ipmi: Sending a list of PCI devices via IPMI OEM messages

Implements sending a list of installed PCI devices through IPMI protocol - OEM message (NetFn 0x3a (IBM), Command 0x2a).

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@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

Hi guys,

On April 20, I sent a small patch to support inventory of PCI devices.
But I don't get any feedback :(
Do I need to create a pull request or register an issue with github?
Or maybe it's better to put this patch in op-build as a vesnin platform specific?

--
Regards,
Artem Senichev
Software Engineer, YADRO.

On Fri, Apr 20, 2018 at 02:11:50PM +0300, Artem Senichev wrote:
> Implements sending a list of installed PCI devices through IPMI protocol - OEM message (NetFn 0x3a (IBM), Command 0x2a).
> 
> 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@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@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);
> +
> +	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);
> +
> +	return OPAL_SUCCESS;
> +}
> +
> +
> +void ipmi_inventory_pci(bool ep_only)
> +{
> +	struct phb *phb;
> +
> +	/* User data passed to the pci_walk_dev is a filter flag, any
> +	   non-NULL values will be interpreted as EP filter. */
> +	void *filter = ep_only ? &ep_only : NULL;
> +
> +	/* Start new sync session */
> +	++session_id;
> +
> +	for_each_phb(phb) {
> +		pci_walk_dev(phb, NULL, &pci_walk_dev_cb, filter);
> +	}
> +}
> 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
>  
>  #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);
> +
>  /* Terminate immediate */
>  void __attribute__((noreturn)) ipmi_terminate(const char *msg);
>  
> diff --git a/platforms/astbmc/vesnin.c b/platforms/astbmc/vesnin.c
> index d7df191b..17d29769 100644
> --- a/platforms/astbmc/vesnin.c
> +++ b/platforms/astbmc/vesnin.c
> @@ -233,6 +233,14 @@ static const struct slot_table_entry vesnin_phb_table[] = {
>  	{ .etype = st_end }
>  };
>  
> +
> +static void vesnin_pci_probe_complete(void)
> +{
> +	check_all_slot_table();
> +	ipmi_inventory_pci(true);
> +}
> +
> +
>  static bool vesnin_probe(void)
>  {
>  	if (!dt_node_is_compatible(dt_root, "YADRO,vesnin"))
> @@ -245,13 +253,14 @@ static bool vesnin_probe(void)
>  	return true;
>  }
>  
> +
>  DECLARE_PLATFORM(vesnin) = {
>  	.name			= "vesnin",
>  	.bmc			= &astbmc_ami,
>  	.probe			= vesnin_probe,
>  	.init			= astbmc_init,
>  	.pci_get_slot_info	= slot_table_get_slot_info,
> -	.pci_probe_complete	= check_all_slot_table,
> +	.pci_probe_complete	= vesnin_pci_probe_complete,
>  	.external_irq		= astbmc_ext_irq_serirq_cpld,
>  	.cec_power_down		= astbmc_ipmi_power_down,
>  	.cec_reboot		= astbmc_ipmi_reboot,
> -- 
> 2.14.1
>

Artem Senichev <a.senichev@yadro.com> writes:
> On April 20, I sent a small patch to support inventory of PCI devices.
> But I don't get any feedback :(
> Do I need to create a pull request or register an issue with github?
> Or maybe it's better to put this patch in op-build as a vesnin
> platform specific?

We just need more hours in the day on our side :) Taking a look now.

Artem Senichev <a.senichev@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@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@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.

On Thu, May 03, 2018 at 07:52:43PM +1000, Stewart Smith wrote:
> Artem Senichev <a.senichev@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?
It will be, actually we have a patch to add PCI device handling:
https://github.com/YADRO-KNS/openpower-host-ipmi-oem/pull/1/files
I will send it to OpenBMC's gerrit as soon as we have PCI list support from OPAL.

> 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.
Perhaps I misunderstand something, but I don't have any specific code for PCI
common code, there is just a for_each_phb() call that generates an OEM IPMI
message for each device. Currently I have defined Vesnin's own
pci_probe_complete() callback that initiates sending the PCI device list through
IPMI.
Something like this:
OPAL's PCI subsystem -> platform.pci_probe_complete() -> ipmi_inventory_pci() -> for_each_phb -> ipmi -> BMC

What do you mean by "have the code to send the detected PCI devices in
common PCI code"?

I didn't put the OEM command to astbmc because there is no any AST specific,
the command can be handled with any other BMC implementation that knows about
the message format (for example, qemu with IPMI emulator).
Do you really want to move the IPMI-OEM code to platforms/astbmc/common.c?

> 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.
I don't know which code to use :)
I chose 0x3a (IBM) as vendor code because IBM is a founder of OpenPOWER.
The command id 0x2a it is just 42 - "The Ultimate Question of Life, the Universe, and Everything".
I will change them if you specify the correct identifiers.

> > +	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) ?
Yes, thanks!

> > +	/* 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.
In an ideal world we have ipmi_queue_msg_sync() function with additional
parameter that specified the timeout.
I can do it later, but not as part of this patch.

> > +#define IPMI_NETFN_IBM			0x3a
> Hrm.. this makes me wonder if we have the ipmi_oem_partial_add_esel
> incorrect for OpenBMC.
Yea, there is something strange with this id's. We have a patch for hostboot
that makes partial_add_esel() function to use NETFUN_IBM instead of NETFUN_AMI,
in skiboot our platform uses astbmc_ami (not astbmc_openbmc).

> > +/* 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.
Ok, I'll change it.

--
Regards,
Artem Senichev
Software Engineer, YADRO.