Re: [PATCH v2 4/7] hw/ipmi: Refactor IPMI interface

[Hao Wu]

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!

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.