[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 4/7] hw/ipmi: Refactor IPMI interface
From: |
Corey Minyard |
Subject: |
Re: [PATCH v2 4/7] hw/ipmi: Refactor IPMI interface |
Date: |
Mon, 27 Mar 2023 15:23:36 -0500 |
On Mon, Mar 27, 2023 at 10:11:50AM -0700, Hao Wu wrote:
> Hi, Cedric
>
> The naming scheme is suggested by Corey in a previous review:
>
> https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg02691.html
>
> I originally kept "IpmIBmc" for the host side code talking to BMC but it
> might also cause confusion as well. I'm not sure which name is the best
> here. Maybe Corey can shed some light on this one? Thank you!
I agree with Cédric here, Bmc and BmcClient sound more clear than what I
proposed earlier.
-corey
>
> Best Regards,
>
> On Mon, Mar 27, 2023 at 5:34 AM Cédric Le Goater <clg@kaod.org> wrote:
>
> > Hello Hao,
> >
> > On 3/25/23 00:09, Hao Wu wrote:
> > > This patch refactors the IPMI interface so that it can be used by both
> > > the BMC side and core-side simulation.
> > >
> > > Detail changes:
> > > (1) Split IPMIInterface into IPMIInterfaceHost (for host side
> > > simulation) and IPMIInterfaceClient (for BMC side simulation).
> > > (2) rename handle_rsp -> handle_msg so the name fits both BMC side and
> > > Core side.
> > > (3) Add a new class IPMICore. This class represents a simulator/external
> > > connection for both BMC and Core side emulation.
> > > (4) Change the original IPMIBmc to IPMIBmcHost, representing host side
> > > simulation.
> > > (5) Add a new type IPMIBmcClient representing BMC side simulation.
> > > (6) Appy the changes to the entire IPMI library.
> >
> > 'IPMIBmcHost' is a BMC object model (internal or external) and
> > 'IPMIBmcClient' is a host object model ?
> >
> > [ ... ]
> >
> > > @@ -267,15 +267,15 @@ void pnv_bmc_set_pnor(IPMIBmc *bmc, PnvPnor *pnor)
> > > * Instantiate the machine BMC. PowerNV uses the QEMU internal
> > > * simulator but it could also be external.
> > > */
> > > -IPMIBmc *pnv_bmc_create(PnvPnor *pnor)
> > > +IPMIBmcHost *pnv_bmc_create(PnvPnor *pnor)
> > > {
> > > Object *obj;
> > >
> > > obj = object_new(TYPE_IPMI_BMC_SIMULATOR);
> > > qdev_realize(DEVICE(obj), NULL, &error_fatal);
> > > - pnv_bmc_set_pnor(IPMI_BMC(obj), pnor);
> > > + pnv_bmc_set_pnor(IPMI_BMC_HOST(obj), pnor);
> > >
> > > - return IPMI_BMC(obj);
> > > + return IPMI_BMC_HOST(obj);
> >
> > QEMU PowerNV machines model the host side of OpenPOWER systems which
> > have an Aspeed SoC based BMC for management. The routine above creates
> > an Aspeed *BMC* object model for the PowerNV *host* machine. I find
> > 'IPMIBmcHost' confusing. It shouldn't have a 'Host' suffix I think.
> >
> > 'IPMIBmcClient' sounds ok, or 'IPMIBmcPeer' maybe.
> >
> > Thanks,
> >
> > C.
> >
> >
- Re: [PATCH v2 2/7] docs/specs: IPMI device emulation: main processor, (continued)
[PATCH v2 6/7] hw/ipmi: Add an IPMI external host device, Hao Wu, 2023/03/24
[PATCH v2 7/7] hw/ipmi: Add a KCS Module for NPCM7XX, Hao Wu, 2023/03/24