Hi Leif & Peter, Thanks for the comments. I will address them in a v2 patch.
Regards, Kun On Mon, Mar 3, 2025 at 12:44 PM Leif Lindholm < [email protected]> wrote: > Doh! Add the lists back in. (No idea how I dropped them off.) > > On Mon, 3 Mar 2025 at 17:02, Leif Lindholm > <[email protected]> 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 <[email protected]> wrote: > > > > > > From: Kun Qin <[email protected]> > > > > > > Resolves: 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 <[email protected]> > > > --- > > > 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 > > > >
