[Skiboot] [PATCH] ipmi: Sending a list of PCI devices via IPMI OEM messages
Stewart Smith
stewart at linux.ibm.com
Thu May 3 19:52:43 AEST 2018
Artem Senichev <a.senichev at yadro.com> writes:
> Implements sending a list of installed PCI devices through IPMI
> protocol - OEM message (NetFn 0x3a (IBM), Command 0x2a).
Is the handling of this something you're looking at upstreaming on
OpenBMC?
A minor change, I'd suggest adding an ipmi_oem_pci_device_list to struct
bmc_platform (like we have today for ipmi_oem_partial_add_esel and
ipmi_oem_pnor_access_status) and have the code to send the detected PCI
devices in common PCI code and the OEM command in the common
astbmc_openbmc struct in platforms/astbmc/common.c.
(this would mean there's less vesnin specific code which is always nice)
If the change isn't going to go upstream to openbmc, I wonder if the
vendor code here is correct? It may be worthwhile to constrain that to a
struct bmc_platform for vesnin until it's upstream.
> The synchronization session can be started from pci_probe_complete function to send PCI device list to the BMC.
>
> Signed-off-by: Artem Senichev <a.senichev at yadro.com>
> ---
> hw/ipmi/Makefile.inc | 2 +-
> hw/ipmi/ipmi-pciinv.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++
> include/ipmi.h | 7 +++
> platforms/astbmc/vesnin.c | 11 +++-
> 4 files changed, 167 insertions(+), 2 deletions(-)
> create mode 100644 hw/ipmi/ipmi-pciinv.c
>
> diff --git a/hw/ipmi/Makefile.inc b/hw/ipmi/Makefile.inc
> index 34d6bd31..78350907 100644
> --- a/hw/ipmi/Makefile.inc
> +++ b/hw/ipmi/Makefile.inc
> @@ -1,7 +1,7 @@
> SUBDIRS += hw/ipmi
>
> IPMI_OBJS = ipmi-rtc.o ipmi-power.o ipmi-fru.o ipmi-sel.o
> -IPMI_OBJS += ipmi-watchdog.o ipmi-sensor.o ipmi-attn.o
> +IPMI_OBJS += ipmi-watchdog.o ipmi-sensor.o ipmi-attn.o ipmi-pciinv.o
>
> IPMI = hw/ipmi/built-in.a
> $(IPMI): $(IPMI_OBJS:%=hw/ipmi/%)
> diff --git a/hw/ipmi/ipmi-pciinv.c b/hw/ipmi/ipmi-pciinv.c
> new file mode 100644
> index 00000000..33f1f92b
> --- /dev/null
> +++ b/hw/ipmi/ipmi-pciinv.c
> @@ -0,0 +1,149 @@
> +/**
> + * ipmi-pciinv.c -- PCI device inventory support.
> + *
> + * Implements sending a list of installed PCI devices through IPMI protocol.
> + * Each PCI device description is send as standalone IPMI message.
> + * A list of devices can be collected from separated messages using the
> + * session identifier. The session Id is an incremental counter that updates at
> + * start of synchronization session.
> + *
> + * Copyright (c) 2018 YADRO (KNS Group LLC)
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + *
> + * Written by Artem Senichev <a.senichev at yadro.com>.
> + */
> +
> +#include <ipmi.h>
> +#include <pci.h>
> +#include <pci-cfg.h>
> +
> +/* Current version of the PCI inventory synchronization packet */
> +#define PCIINV_PACK_VERSION 1
> +
> +/**
> + * struct pciinv_device - PCI device inventory description.
> + * @domain_num: Domain number.
> + * @bus_num: Bus number.
> + * @device_num: Device number.
> + * @func_num: Function number.
> + * @vendor_id: Vendor Id.
> + * @device_id: Device Id.
> + * @class_code: Device class code.
> + * @revision: Revision number.
> + *
> + * All fields have Big Endian byte order.
> + */
> +struct pciinv_device {
> + uint16_t domain_num;
> + uint8_t bus_num;
> + uint8_t device_num;
> + uint8_t func_num;
> + uint16_t vendor_id;
> + uint16_t device_id;
> + uint32_t class_code;
> + uint8_t revision;
> +} __packed;
> +
> +/**
> + * struct pciinv_packet - IPMI message packet data.
> + * @version: Packet version, must be set to %PCIINV_PACK_VERSION.
> + * @session: Sync session Id.
> + * @device: PCI device description.
> + */
> +struct pciinv_packet {
> + uint8_t version;
> + uint8_t session;
> + struct pciinv_device device;
> +} __packed;
> +
> +
> +/* Id of the current sync session. */
> +static uint8_t session_id;
> +
> +
> +/**
> + * pciinv_fill() - Fill the PCI device inventory description.
> + * @phb: PHB descriptor.
> + * @pd: PCE device descriptor.
> + * @dev: Inventory device description to fill.
> + */
> +static void pciinv_fill(struct phb *phb, struct pci_device *pd, struct pciinv_device* dev)
> +{
> + dev->domain_num = cpu_to_be16(phb->opal_id & 0xffff);
> + dev->bus_num = (pd->bdfn >> 8) & 0xff;
> + dev->device_num = (pd->bdfn >> 3) & 0x1f;
> + dev->func_num = pd->bdfn & 0x7;
> +
> + pci_cfg_read16(phb, pd->bdfn, PCI_CFG_VENDOR_ID, &dev->vendor_id);
> + dev->vendor_id = cpu_to_be16(dev->vendor_id);
> +
> + pci_cfg_read16(phb, pd->bdfn, PCI_CFG_DEVICE_ID, &dev->device_id);
> + dev->device_id = cpu_to_be16(dev->device_id);
I think these two could be replaced by PCI_VENDOR_ID(pd->vdid) and
PCI_DEVICE_ID(pd->vdid) ?
> +
> + dev->class_code = cpu_to_be32(pd->class & 0xffffff);
> +
> + pci_cfg_read8(phb, pd->bdfn, PCI_CFG_REV_ID, &dev->revision);
> +}
> +
> +
> +/**
> + * pci_walk_dev_cb() - Callback from PCI enumerator, see :c:func:`pci_walk_dev`.
> + */
> +static int pci_walk_dev_cb(struct phb *phb, struct pci_device *pd, void *filter)
> +{
> + struct ipmi_msg *msg;
> + struct pciinv_packet pack = {
> + .version = PCIINV_PACK_VERSION,
> + .session = session_id
> + };
> +
> + /* PCI device filter */
> + if (filter) {
> + /* Skip non-EP devices */
> + if (pci_has_cap(pd, PCI_CFG_CAP_ID_EXP, false)) {
> + if (pd->dev_type != PCIE_TYPE_ENDPOINT)
> + return OPAL_SUCCESS;
> + }
> + else if (pd->is_bridge)
> + return OPAL_SUCCESS;
> + }
> +
> + pciinv_fill(phb, pd, &pack.device);
> +
> + msg = ipmi_mkmsg_simple(IPMI_PCI_INVENTORY, &pack, sizeof(pack));
> + if (!msg)
> + return OPAL_HARDWARE;
> +
> + /* Synchronously send the IPMI message, the queue is too small */
> + ipmi_queue_msg_sync(msg);
I guess in an ideal world we'd walk the hierarchy in the callback when
the message is done, but seeing as where we are in boot, I think this
ends up being okay.... Or, at least, I'm not going to worry about it
taking too long.
> diff --git a/include/ipmi.h b/include/ipmi.h
> index 3389eaf5..a9999a70 100644
> --- a/include/ipmi.h
> +++ b/include/ipmi.h
> @@ -101,6 +101,7 @@
> #define IPMI_NETFN_SE 0x04
> #define IPMI_NETFN_STORAGE 0x0a
> #define IPMI_NETFN_APP 0x06
> +#define IPMI_NETFN_IBM 0x3a
Hrm.. this makes me wonder if we have the ipmi_oem_partial_add_esel
incorrect for OpenBMC.
Tom, is this right for current OpenBMC?
const struct bmc_platform astbmc_openbmc = {
.name = "OpenBMC",
.ipmi_oem_partial_add_esel = IPMI_CODE(0x32, 0xf0),
};
(It's distinctly possible it isn't and this explains a bug report that
we have laying around internally)
> #define IPMI_WRITE_FRU IPMI_CODE(IPMI_NETFN_STORAGE, 0x12)
> #define IPMI_GET_SEL_INFO IPMI_CODE(IPMI_NETFN_STORAGE, 0x40)
> @@ -121,6 +122,7 @@
> #define IPMI_READ_EVENT IPMI_CODE(IPMI_NETFN_APP, 0x35)
> #define IPMI_GET_BT_CAPS IPMI_CODE(IPMI_NETFN_APP, 0x36)
> #define IPMI_SET_SENSOR_READING IPMI_CODE(IPMI_NETFN_SE, 0x30)
> +#define IPMI_PCI_INVENTORY IPMI_CODE(IPMI_NETFN_IBM, 0x2a)
>
> /*
> * IPMI response codes.
> @@ -275,6 +277,11 @@ uint8_t ipmi_get_sensor_number(uint8_t sensor_type);
> /* Set the boot count once the OS is up and running */
> int ipmi_set_boot_count(void);
>
> +/* Report an inventory of PCI devices.
> + * @ep_only: Device filter, true to handle End Point devices only.
> + */
> +void ipmi_inventory_pci(bool ep_only);
I'd prefer just naming the parameter endpoint_only to make it obvious.
--
Stewart Smith
OPAL Architect, IBM.
More information about the Skiboot
mailing list