Hi Stefan,
On 2/16/20 7:47 PM, Stefan Berger wrote:
> On 2/14/20 1:37 PM, Eric Auger wrote:
>> Let the TPM TIS SYSBUS device be dynamically instantiable
>> in ARM virt. A device tree node is dynamically created
>> (TPM via MMIO).
>>
>> The TPM Physical Presence interface (PPI) is not supported.
>>
>> To run with the swtmp TPM emulator, the qemu command line must
>> be augmented with:
>>
>> -chardev socket,id=chrtpm,path=swtpm-sock \
>> -tpmdev emulator,id=tpm0,chardev=chrtpm \
>> -device tpm-tis-device,tpmdev=tpm0 \
>>
>> swtpm/libtpms command line example:
>>
>> swtpm socket --tpm2 -t -d --tpmstate dir=/tmp/tpm \
>> --ctrl type=unixio,path=swtpm-sock
>
>
> Can you also extend docs/spec/tpm.rst for the arm case?
Sure
And the test
> cases should probably also cover the arm sysbus device.
Yes I agree. At the moment I have made sure existing x86-64 qtests were
running as before.
I will investigate what I can do to cover the sysbus device.
>
>
>>
>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>> ---
>> hw/arm/Kconfig | 1 +
>> hw/arm/sysbus-fdt.c | 36 ++++++++++++++++++++++++++++++++++++
>> hw/arm/virt.c | 7 +++++++
>> 3 files changed, 44 insertions(+)
>>
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index 3d86691ae0..b6f03f7f53 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -5,6 +5,7 @@ config ARM_VIRT
>> imply VFIO_AMD_XGBE
>> imply VFIO_PLATFORM
>> imply VFIO_XGMAC
>> + imply TPM_TIS_SYSBUS
>> select A15MPCORE
>> select ACPI
>> select ARM_SMMUV3
>> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
>> index 022fc97ecd..adf50444c2 100644
>> --- a/hw/arm/sysbus-fdt.c
>> +++ b/hw/arm/sysbus-fdt.c
>> @@ -30,6 +30,7 @@
>> #include "hw/arm/sysbus-fdt.h"
>> #include "qemu/error-report.h"
>> #include "sysemu/device_tree.h"
>> +#include "sysemu/tpm.h"
>> #include "hw/platform-bus.h"
>> #include "hw/vfio/vfio-platform.h"
>> #include "hw/vfio/vfio-calxeda-xgmac.h"
>> @@ -434,6 +435,40 @@ static bool vfio_platform_match(SysBusDevice *sbdev,
>> #define VFIO_PLATFORM_BINDING(compat, add_fn) \
>> {TYPE_VFIO_PLATFORM, (compat), (add_fn), vfio_platform_match}
>> +/*
>> + * add_tpm_tis_fdt_node: Create a DT node for TPM TIS
>> + *
>> + * See kernel documentation:
>> + * Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt
>> + * Optional interrupt for command completion is not exposed
>> + */
>> +static int add_tpm_tis_fdt_node(SysBusDevice *sbdev, void *opaque)
>> +{
>> + PlatformBusFDTData *data = opaque;
>> + PlatformBusDevice *pbus = data->pbus;
>> + void *fdt = data->fdt;
>> + const char *parent_node = data->pbus_node_name;
>> + int compat_str_len;
>> + char *nodename;
>> + uint32_t reg_attr[2];
>> + uint64_t mmio_base;
>> +
>> + mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
>
> I suppose any conditional creation of this device tree entry is covered
> with the TYPE_BINDING below, meaning no device tree is created if the
> device wasn't added.
Yes exactly, the dt node is not created if the -device tpm-tis-device is
not passed in the qemu cmd line.
>
>
>> + nodename = g_strdup_printf("%s/tpm_tis@%" PRIx64, parent_node,
>> mmio_base);
>> + qemu_fdt_add_subnode(fdt, nodename);
>> +
>> + compat_str_len = strlen("tcg,tpm-tis-mmio") + 1;
>> + qemu_fdt_setprop(fdt, nodename, "compatible", "tcg,tpm-tis-mmio",
>> + compat_str_len);
>
> You probably can use qemu_fdt_setprop_string()?
indeed!
>
>
>> +
>> + reg_attr[0] = cpu_to_be32(mmio_base);
>> + reg_attr[1] = cpu_to_be32(0x5000);
>> + qemu_fdt_setprop(fdt, nodename, "reg", reg_attr, 2 *
>> sizeof(uint32_t));
>> +
>> + g_free(nodename);
>> + return 0;
>> +}
>> +
>> #endif /* CONFIG_LINUX */
>> static int no_fdt_node(SysBusDevice *sbdev, void *opaque)
>> @@ -455,6 +490,7 @@ static const BindingEntry bindings[] = {
>> TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC,
>> add_calxeda_midway_xgmac_fdt_node),
>> TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
>> VFIO_PLATFORM_BINDING("amd,xgbe-seattle-v1a",
>> add_amd_xgbe_fdt_node),
>> + TYPE_BINDING(TYPE_TPM_TIS_SYSBUS, add_tpm_tis_fdt_node),
>> #endif
>> TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
>> TYPE_BINDING("", NULL), /* last element */
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index f788fe27d6..4b967e39d1 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -47,6 +47,7 @@
>> #include "sysemu/numa.h"
>> #include "sysemu/runstate.h"
>> #include "sysemu/sysemu.h"
>> +#include "sysemu/tpm.h"
>> #include "sysemu/kvm.h"
>> #include "hw/loader.h"
>> #include "exec/address-spaces.h"
>> @@ -2041,6 +2042,7 @@ static void virt_machine_class_init(ObjectClass
>> *oc, void *data)
>> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
>> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
>> + machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
>> mc->block_default_type = IF_VIRTIO;
>> mc->no_cdrom = 1;
>> mc->pci_allow_0_address = true;
>> @@ -2153,6 +2155,11 @@ type_init(machvirt_machine_init);
>> static void virt_machine_5_0_options(MachineClass *mc)
>> {
>> + static GlobalProperty compat[] = {
>> + { TYPE_TPM_TIS_SYSBUS, "ppi", "false" },
>> + };
>> +
>> + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
>> }
>> DEFINE_VIRT_MACHINE_AS_LATEST(5, 0)
>>
>
>
Thanks
Eric
