| Message ID | Z-8ujYWA8yBATtYK@mail.minyard.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hw/ipmi: Allow multiple BMC instances | expand |
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 <corey@minyard.net> > --- > 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. Preferably s/instance/id/: Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > }; > > 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. >
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 <corey@minyard.net> > > --- > > 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é <philmd@linaro.org> > > > }; > > 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. >
On 4/4/25 15:04, Corey Minyard wrote: > 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 <corey@minyard.net> >>> --- >>> 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(). Note, vmstate_register() is a legacy API, with only 20 cases left to update. See commit 6caf1571a97 ("include/migration: mark vmstate_register() as a legacy function"): /** * vmstate_register() - legacy function to register state * serialisation description * * New code shouldn't be using this function as QOM-ified devices * have dc->vmsd to store the serialisation description. * * Returns: 0 on success, -1 on failure */ > Maybe it's better to just have a global variable that increments and not > pass it in? That way it would work automatically. Global variables often hide suble problems. We have a list of some used in qdev / qbus / pci that we plan to remove, because it makes command line not reproducible. See this thread: https://lore.kernel.org/qemu-devel/87czq0l2mn.fsf@dusky.pond.sub.org/ > > -corey > >> >> Preferably s/instance/id/: >> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> >>> }; >>> 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. >>
On Fri, Apr 04, 2025 at 03:21:09PM +0200, Philippe Mathieu-Daudé wrote: > On 4/4/25 15:04, Corey Minyard wrote: > > 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 <corey@minyard.net> > > > > --- > > > > 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(). > > Note, vmstate_register() is a legacy API, with only 20 cases left to > update. See commit 6caf1571a97 ("include/migration: mark > vmstate_register() as a legacy function"): > > /** > * vmstate_register() - legacy function to register state > * serialisation description > * > * New code shouldn't be using this function as QOM-ified devices > * have dc->vmsd to store the serialisation description. > * > * Returns: 0 on success, -1 on failure > */ > > > Maybe it's better to just have a global variable that increments and not > > pass it in? That way it would work automatically. > > Global variables often hide suble problems. We have a list of some used > in qdev / qbus / pci that we plan to remove, because it makes command > line not reproducible. Yeah, I was guessing that, and I also just found that the vmstate_register() was deprecated. I'll work on switching to the new API. Thanks, -corey > > See this thread: > https://lore.kernel.org/qemu-devel/87czq0l2mn.fsf@dusky.pond.sub.org/ > > > > > -corey > > > > > > > > Preferably s/instance/id/: > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > > > > > > }; > > > > 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. > > > >
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), }; 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.
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 <corey@minyard.net> --- 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(-)