[RFC PATCH 0/3] Initial PECI bus support

Titus Rwantare posted 3 patches 1 year, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220906220552.1243998-1-titusr@google.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Havard Skinnemoen <hskinnemoen@google.com>, Tyrone Ting <kfting@nuvoton.com>, Titus Rwantare <titusr@google.com>
There is a newer version of this series
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
[RFC PATCH 0/3] Initial PECI bus support
Posted by Titus Rwantare 1 year, 7 months ago
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.

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.

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
Re: [RFC PATCH 0/3] Initial PECI bus support
Posted by Peter Delevoryas 1 year, 7 months ago
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
>
Re: [RFC PATCH 0/3] Initial PECI bus support
Posted by Titus Rwantare 1 year, 7 months ago
On Fri, 9 Sept 2022 at 12:54, Peter Delevoryas <peter@pjd.dev> wrote:
>
> On Tue, Sep 06, 2022 at 10:05:49PM +0000, Titus Rwantare wrote:
...
> >
> > 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?

Some things seem to move between generations and others don't move, someone at
Intel would know better than I do.



> 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?

I haven't dug into the other machines because of internal demand, but
I've found that
with newer generations, more features get used in addition to existing
ones. It's
possible these features existed on older machines.



> 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.

I think it would be nice to have additional registers at runtime, at
the time of writing,
I don't know how much of the internal workings of Sapphire Rapids
Intel is willing to
share publicly. If users are free to separately define registers, I
don't then get to
worry about this. e.g. I'd like to simulate errors from the memory controllers.



Titus
Re: [RFC PATCH 0/3] Initial PECI bus support
Posted by Peter Delevoryas 1 year, 7 months ago
On Tue, Sep 13, 2022 at 11:20:57AM -0700, Titus Rwantare wrote:
> On Fri, 9 Sept 2022 at 12:54, Peter Delevoryas <peter@pjd.dev> wrote:
> >
> > On Tue, Sep 06, 2022 at 10:05:49PM +0000, Titus Rwantare wrote:
> ...
> > >
> > > 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?
> 
> Some things seem to move between generations and others don't move, someone at
> Intel would know better than I do.
> 
> 
> 
> > 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?
> 
> I haven't dug into the other machines because of internal demand, but
> I've found that
> with newer generations, more features get used in addition to existing
> ones. It's
> possible these features existed on older machines.
> 
> 
> 
> > 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.
> 
> I think it would be nice to have additional registers at runtime, at
> the time of writing,
> I don't know how much of the internal workings of Sapphire Rapids
> Intel is willing to
> share publicly. If users are free to separately define registers, I
> don't then get to
> worry about this. e.g. I'd like to simulate errors from the memory controllers.

Oh ok, yeah I guess making it more dynamic shouldn't really hurt
anything. That sounds ok then.  Also yeah, perhaps keeping the register
definitions separate for privacy concerns is necessary.

Reviewed-by: Peter Delevoryas <peter@pjd.dev>

> 
> 
> Titus