| Message ID | 20220906220552.1243998-1-titusr@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Initial PECI bus support | expand |
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 >
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
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
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