Re: [PATCH 1/1] hw/arm/sbsa-ref: Adding TPM support for ARM SBSA-Ref machine

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Kun,

On 3/3/25 23:55, Kun Qin wrote:
> Hi Leif & Peter,
> 
> Thanks for the comments. I will address them in a v2 patch.

Please also Cc me in your v2 :)

Regards,

Phil.

> 
> Regards,
> Kun
> 
> On Mon, Mar 3, 2025 at 12:44 PM Leif Lindholm 
> <leif.lindholm@oss.qualcomm.com <mailto:leif.lindholm@oss.qualcomm.com>> 
> wrote:
> 
>     Doh! Add the lists back in. (No idea how I dropped them off.)
> 
>     On Mon, 3 Mar 2025 at 17:02, Leif Lindholm
>     <leif.lindholm@oss.qualcomm.com
>     <mailto:leif.lindholm@oss.qualcomm.com>> wrote:
>      >
>      > Hi Kun,
>      >
>      > Apologies for delay in responding - I was out last week.
>      > I agree with this addition, since a TPM is a requirement for servers.
>      >
>      > However, to help simplify review, could you add some detail in the
>      > commit message
>      > as to which SystemReady requirements this resolves and whether this
>      > implementation
>      > fulfills all requirements across BSA/SBSA/BBSA?
>      >
>      > I agree with Peter that since this is a non-discoverable
>     component, it
>      > would make sense
>      > to step the machine minor version number. A major version bump would
>      > not be required
>      > since simply adding this component will not break any existing
>      > firmware (which will have
>      > no way of knowing it even exists).
>      >
>      > Regards,
>      >
>      > Leif
>      >
>      > On Tue, 25 Feb 2025 at 07:41, Kun Qin <kuqin12@gmail.com
>     <mailto:kuqin12@gmail.com>> wrote:
>      > >
>      > > From: Kun Qin <kuqin@microsoft.com <mailto:kuqin@microsoft.com>>
>      > >
>      > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2625
>     <https://gitlab.com/qemu-project/qemu/-/issues/2625>
>      > >
>      > > This change aims to add a TPM device for SBSA ref machine.
>      > >
>      > > The implementation adds a TPM create routine during machine
>      > > initialization.
>      > >
>      > > The backend can be the same as the rest of TPM support, by
>     using swtpm.
>      > >
>      > > Signed-off-by: Kun Qin <kuqin12@gmail.com
>     <mailto:kuqin12@gmail.com>>
>      > > ---
>      > >  hw/arm/sbsa-ref.c | 24 ++++++++++++++++++++++++
>      > >  1 file changed, 24 insertions(+)
>      > >
>      > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
>      > > index e720de306419..93eb3d1e363b 100644
>      > > --- a/hw/arm/sbsa-ref.c
>      > > +++ b/hw/arm/sbsa-ref.c
>      > > @@ -28,6 +28,8 @@
>      > >  #include "system/numa.h"
>      > >  #include "system/runstate.h"
>      > >  #include "system/system.h"
>      > > +#include "system/tpm.h"
>      > > +#include "system/tpm_backend.h"
>      > >  #include "exec/hwaddr.h"
>      > >  #include "kvm_arm.h"
>      > >  #include "hw/arm/boot.h"
>      > > @@ -94,6 +96,7 @@ enum {
>      > >      SBSA_SECURE_MEM,
>      > >      SBSA_AHCI,
>      > >      SBSA_XHCI,
>      > > +    SBSA_TPM,
>      > >  };
>      > >
>      > >  struct SBSAMachineState {
>      > > @@ -132,6 +135,7 @@ static const MemMapEntry sbsa_ref_memmap[] = {
>      > >      /* Space here reserved for more SMMUs */
>      > >      [SBSA_AHCI] =               { 0x60100000, 0x00010000 },
>      > >      [SBSA_XHCI] =               { 0x60110000, 0x00010000 },
>      > > +    [SBSA_TPM] =                { 0x60120000, 0x00010000 },
>      > >      /* Space here reserved for other devices */
>      > >      [SBSA_PCIE_PIO] =           { 0x7fff0000, 0x00010000 },
>      > >      /* 32-bit address PCIE MMIO space */
>      > > @@ -629,6 +633,24 @@ static void create_smmu(const
>     SBSAMachineState *sms, PCIBus *bus)
>      > >      }
>      > >  }
>      > >
>      > > +static void create_tpm(SBSAMachineState *sbsa, PCIBus *bus)
>      > > +{
>      > > +    Error *errp = NULL;
>      > > +    DeviceState *dev;
>      > > +
>      > > +    TPMBackend *be = qemu_find_tpm_be("tpm0");
>      > > +    if (be == NULL) {
>      > > +        error_report("Couldn't find tmp0 backend");
>      > > +        return;
>      > > +    }
>      > > +
>      > > +    dev = qdev_new(TYPE_TPM_TIS_SYSBUS);
>      > > +    object_property_set_link(OBJECT(dev), "tpmdev",
>     OBJECT(be), &errp);
>      > > +    object_property_set_str(OBJECT(dev), "tpmdev", be->id, &errp);
>      > > +    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>      > > +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0,
>     sbsa_ref_memmap[SBSA_TPM].base);
>      > > +}
>      > > +
>      > >  static void create_pcie(SBSAMachineState *sms)
>      > >  {
>      > >      hwaddr base_ecam = sbsa_ref_memmap[SBSA_PCIE_ECAM].base;
>      > > @@ -686,6 +708,8 @@ static void create_pcie(SBSAMachineState *sms)
>      > >      pci_create_simple(pci->bus, -1, "bochs-display");
>      > >
>      > >      create_smmu(sms, pci->bus);
>      > > +
>      > > +    create_tpm(sms, pci->bus);
>      > >  }
>      > >
>      > >  static void *sbsa_ref_dtb(const struct arm_boot_info *binfo,
>     int *fdt_size)
>      > > --
>      > > 2.43.0
>      > >
>