* [PATCH] hw/ipmi: Allow multiple BMC instances @ 2025-04-04 0:57 Corey Minyard 2025-04-04 12:41 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 5+ messages in thread From: Corey Minyard @ 2025-04-04 0:57 UTC (permalink / raw) To: qemu-devel 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), }; 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. -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/ipmi: Allow multiple BMC instances 2025-04-04 0:57 [PATCH] hw/ipmi: Allow multiple BMC instances Corey Minyard @ 2025-04-04 12:41 ` Philippe Mathieu-Daudé 2025-04-04 13:04 ` Corey Minyard 0 siblings, 1 reply; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2025-04-04 12:41 UTC (permalink / raw) To: corey, qemu-devel 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. > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/ipmi: Allow multiple BMC instances 2025-04-04 12:41 ` Philippe Mathieu-Daudé @ 2025-04-04 13:04 ` Corey Minyard 2025-04-04 13:21 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 5+ messages in thread From: Corey Minyard @ 2025-04-04 13:04 UTC (permalink / raw) To: Philippe Mathieu-Daudé; +Cc: qemu-devel 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. > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/ipmi: Allow multiple BMC instances 2025-04-04 13:04 ` Corey Minyard @ 2025-04-04 13:21 ` Philippe Mathieu-Daudé 2025-04-04 15:25 ` Corey Minyard 0 siblings, 1 reply; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2025-04-04 13:21 UTC (permalink / raw) To: corey Cc: qemu-devel, Alex Bennée, Peter Xu, Fabiano Rosas, Markus Armbruster 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. >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/ipmi: Allow multiple BMC instances 2025-04-04 13:21 ` Philippe Mathieu-Daudé @ 2025-04-04 15:25 ` Corey Minyard 0 siblings, 0 replies; 5+ messages in thread From: Corey Minyard @ 2025-04-04 15:25 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: qemu-devel, Alex Bennée, Peter Xu, Fabiano Rosas, Markus Armbruster 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. > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-04-04 15:25 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-04 0:57 [PATCH] hw/ipmi: Allow multiple BMC instances Corey Minyard 2025-04-04 12:41 ` Philippe Mathieu-Daudé 2025-04-04 13:04 ` Corey Minyard 2025-04-04 13:21 ` Philippe Mathieu-Daudé 2025-04-04 15:25 ` Corey Minyard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).