On Fri, Apr 04, 2025 at 02:41:46PM +0200, Philippe Mathieu-Daudé wrote: > Hi Corey, > > On 4/4/25 02:57, Corey Minyard wrote: > > Allow a system to have multiple BMC connections to the same BMC and > > multiple different BMCs. This can happen on real systems, and is > > useful for testing the IPMI driver on Linux. > > > > Signed-off-by: Corey Minyard <[email protected]> > > --- > > I'm working on a fairly extensive test suite for IPMI, the Linux > > driver and qemu, and this is necessary for some driver tests. > > > > hw/ipmi/ipmi.c | 1 + > > hw/ipmi/ipmi_bmc_extern.c | 5 +++-- > > hw/ipmi/ipmi_bmc_sim.c | 2 +- > > include/hw/ipmi/ipmi.h | 1 + > > qemu-options.hx | 9 ++++++++- > > 5 files changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c > > index fdeaa5269f..ffd972f78b 100644 > > --- a/hw/ipmi/ipmi.c > > +++ b/hw/ipmi/ipmi.c > > @@ -110,6 +110,7 @@ void ipmi_bmc_find_and_link(Object *obj, Object **bmc) > > static const Property ipmi_bmc_properties[] = { > > DEFINE_PROP_UINT8("slave_addr", IPMIBmc, slave_addr, 0x20), > > + DEFINE_PROP_UINT8("instance", IPMIBmc, instance, 0), > > Can we use "id" instead of "instance"? The latter confuses me, but > maybe a matter of taste.
"id" means "identifier", not "instance". The error log mentions "instance", that that is what is passed to vmstate_register(). Maybe it's better to just have a global variable that increments and not pass it in? That way it would work automatically. -corey > > Preferably s/instance/id/: > Reviewed-by: Philippe Mathieu-Daudé <[email protected]> > > > }; > > static void bmc_class_init(ObjectClass *oc, void *data) > > diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c > > index d015500254..11c28d03ab 100644 > > --- a/hw/ipmi/ipmi_bmc_extern.c > > +++ b/hw/ipmi/ipmi_bmc_extern.c > > @@ -488,7 +488,8 @@ static const VMStateDescription vmstate_ipmi_bmc_extern > > = { > > static void ipmi_bmc_extern_realize(DeviceState *dev, Error **errp) > > { > > - IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(dev); > > + IPMIBmc *b = IPMI_BMC(dev); > > + IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(b); > > if (!qemu_chr_fe_backend_connected(&ibe->chr)) { > > error_setg(errp, "IPMI external bmc requires chardev attribute"); > > @@ -498,7 +499,7 @@ static void ipmi_bmc_extern_realize(DeviceState *dev, > > Error **errp) > > qemu_chr_fe_set_handlers(&ibe->chr, can_receive, receive, > > chr_event, NULL, ibe, NULL, true); > > - vmstate_register(NULL, 0, &vmstate_ipmi_bmc_extern, ibe); > > + vmstate_register(NULL, b->instance, &vmstate_ipmi_bmc_extern, ibe); > > } > > static void ipmi_bmc_extern_init(Object *obj) > > diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c > > index 6157ac7120..c1b39dbdc5 100644 > > --- a/hw/ipmi/ipmi_bmc_sim.c > > +++ b/hw/ipmi/ipmi_bmc_sim.c > > @@ -2188,7 +2188,7 @@ static void ipmi_sim_realize(DeviceState *dev, Error > > **errp) > > ibs->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ipmi_timeout, ibs); > > - vmstate_register(NULL, 0, &vmstate_ipmi_sim, ibs); > > + vmstate_register(NULL, b->instance, &vmstate_ipmi_sim, ibs); > > } > > static const Property ipmi_sim_properties[] = { > > diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h > > index 77a7213ed9..4436d70842 100644 > > --- a/include/hw/ipmi/ipmi.h > > +++ b/include/hw/ipmi/ipmi.h > > @@ -183,6 +183,7 @@ struct IPMIBmc { > > DeviceState parent; > > uint8_t slave_addr; > > + uint8_t instance; > > IPMIInterface *intf; > > }; > > diff --git a/qemu-options.hx b/qemu-options.hx > > index dc694a99a3..186433ac13 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -1120,6 +1120,10 @@ SRST > > ``slave_addr=val`` > > Define slave address to use for the BMC. The default is 0x20. > > + ``instance=val`` > > + For more than one BMC on the same system, each instance needs > > + a unique number. The default is 0. > > + > > ``sdrfile=file`` > > file containing raw Sensor Data Records (SDR) data. The default > > is none. > > @@ -1137,7 +1141,7 @@ SRST > > is set, get "Get GUID" command to the BMC will return it. > > Otherwise "Get GUID" will return an error. > > -``-device ipmi-bmc-extern,id=id,chardev=id[,slave_addr=val]`` > > +``-device ipmi-bmc-extern,id=id,chardev=id[,slave_addr=val][,instance=id]`` > > Add a connection to an external IPMI BMC simulator. Instead of > > locally emulating the BMC like the above item, instead connect to an > > external entity that provides the IPMI services. > > @@ -1151,6 +1155,9 @@ SRST > > simulator running on a secure port on localhost, so neither the > > simulator nor QEMU is exposed to any outside network. > > + You can have more than one external BMC connection with this, but > > + you must set a unique instance for each BMC. > > + > > See the "lanserv/README.vm" file in the OpenIPMI library for more > > details on the external interface. >
