qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH 0/3] Initial PECI bus support


From: Peter Delevoryas
Subject: Re: [RFC PATCH 0/3] Initial PECI bus support
Date: Fri, 9 Sep 2022 12:54:31 -0700

On Tue, Sep 06, 2022 at 10:05:49PM +0000, Titus Rwantare wrote:
> The Platform Environment Control Interface (PECI), is a way for Intel
> processors to communicate with management controllers.
> 
> This series of patches simulate some PECI subsystem functionality. This
> work is currently used against Nuvoton 7xx BMC, but it can easily be
> extended to support Aspeed BMCs. Most of the functionality is derived
> from PECI support in openbmc. See https://github.com/openbmc/libpeci
> 
> The main consumer of this work is openbmc, so functionality not
> exercised by the openbmc/libpeci is unlikely to be present here.
> 
> peci-core.c is an attempt to split out functionality defined by the
> spec. Anything that is not expected to change between BMC vendors.
> 
> The following commands have some support:
>     Ping()
>     GetDIB()
>     GetTemp()
>     ~RdPkgConfig()
>     ~RdEndPtConfig()
> 
> To be implemented:
>     RdIAMSR()
>     RdPCIConfig()
>     RdPCIConfigLocal()
> 
> Currently, in the board file during bmc_init() one may specify defaults
> as follows:
> 
> static void my_machine_peci_init(NPCM7xxState *soc)
> {
>     PECIBus *peci_bus = npcm7xx_peci_get_bus(soc);
>     DeviceState *dev;
> 
>     /* per socket properties - both sockets are identical in this case */
>     PECIClientProperties peci_props = {
>         .cpu_family = FAM6_SAPPHIRE_RAPIDS_X,
>         .cpus = 56,
>         .dimms = 16
>     };
> 
>     /* socket 0 - with example setting a few of the cpu and dimm temperatures 
> in millidegrees */
>     dev = DEVICE(peci_add_client(peci_bus, 0x30, &peci_props));
>     object_property_set_uint(OBJECT(dev), "cpu_temp[0]", 30000, &error_abort);
>     object_property_set_uint(OBJECT(dev), "cpu_temp[2]", 35000, &error_abort);
>     object_property_set_uint(OBJECT(dev), "dimm_temp[1]", 40000, 
> &error_abort);
>     object_property_set_uint(OBJECT(dev), "dimm_temp[8]", 36000, 
> &error_abort);
> 
>     /* socket 1 */
>     dev = DEVICE(peci_add_client(peci_bus, 0x31, &peci_props));
>     object_property_set_uint(OBJECT(dev), "cpu_temp[9]", 50000, &error_abort);
>     object_property_set_uint(OBJECT(dev), "dimm_temp[0]", 31000, 
> &error_abort);
>     object_property_set_uint(OBJECT(dev), "dimm_temp[14]", 36000, 
> &error_abort);
>     ...
> }
> 
> This is something that can also be extended as other parameters arise that 
> need
> to differ between platforms. So far you can have have different CPUs, DIMM 
> counts,
> DIMM temperatures here. These fields can also be adjusted at runtime through 
> qmp.

That looks good to me, seems like the standard way to do it in QEMU.

> 
> A lot of the registers are hard coded, see hw/peci/peci-client.c. I'd like to
> gauge interest in what potential users would like to be adjustable at runtime.
> I've not written QEMU models that read config files at runtime, something I'd
> appreciate guidance on.

This part I don't totally understand. I also barely know anything about
PECI.

Is the register location for things different between CPU generations?

If so (and I expect it probably is), why is there only a configuration
for Sapphire Rapids, and not for the other ones?

Is that because of PECI protocol changes between generations?

In which case, maybe there needs to be a notion of PECI version
somewhere?

Also, I don't understand why it would be adjustable at runtime, do we
change register locations during execution?

I would expect it to be part of the board definition.

You could provide a bunch of sample configs for the CPU's that you're
testing for, and the board configuration could just select the sample
config it is using (corresponding to the CPU model).

That's the model I would imagine, but I might be missing some important
context here.

Thanks,
Peter

> 
> Thanks all
> 
> Titus Rwantare (3):
>   hw/peci: add initial support for PECI
>   hw/peci: add PECI support for NPCM7xx BMCs
>   hw/peci: add support for EndPointConfig reads
> 
>  MAINTAINERS                    |  10 +-
>  hw/Kconfig                     |   1 +
>  hw/arm/Kconfig                 |   1 +
>  hw/arm/npcm7xx.c               |   9 +
>  hw/meson.build                 |   1 +
>  hw/peci/Kconfig                |   2 +
>  hw/peci/meson.build            |   2 +
>  hw/peci/npcm7xx_peci.c         | 204 +++++++++++++++++++++++
>  hw/peci/peci-client.c          | 293 +++++++++++++++++++++++++++++++++
>  hw/peci/peci-core.c            | 222 +++++++++++++++++++++++++
>  hw/peci/trace-events           |  10 ++
>  hw/peci/trace.h                |   1 +
>  include/hw/arm/npcm7xx.h       |   2 +
>  include/hw/peci/npcm7xx_peci.h |  37 +++++
>  include/hw/peci/peci.h         | 217 ++++++++++++++++++++++++
>  meson.build                    |   1 +
>  16 files changed, 1012 insertions(+), 1 deletion(-)
>  create mode 100644 hw/peci/Kconfig
>  create mode 100644 hw/peci/meson.build
>  create mode 100644 hw/peci/npcm7xx_peci.c
>  create mode 100644 hw/peci/peci-client.c
>  create mode 100644 hw/peci/peci-core.c
>  create mode 100644 hw/peci/trace-events
>  create mode 100644 hw/peci/trace.h
>  create mode 100644 include/hw/peci/npcm7xx_peci.h
>  create mode 100644 include/hw/peci/peci.h
> 
> -- 
> 2.37.2.789.g6183377224-goog
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]