qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Auger Eric <eric.auger@redhat.com>
To: Stefan Berger <stefanb@linux.ibm.com>,
	eric.auger.pro@gmail.com, qemu-devel@nongnu.org,
	qemu-arm@nongnu.org, peter.maydell@linaro.org
Cc: marcandre.lureau@redhat.com, lersek@redhat.com, ardb@kernel.org,
	philmd@redhat.com
Subject: Re: [RFC v2 6/6] hw/arm/virt: vTPM support
Date: Mon, 17 Feb 2020 19:26:10 +0100	[thread overview]
Message-ID: <b640344f-ec88-ed79-3623-dcfc20d94541@redhat.com> (raw)
In-Reply-To: <17dfaa9b-dd1a-9654-c59c-391a70d0327e@linux.ibm.com>

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



  reply	other threads:[~2020-02-17 18:27 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-14 18:36 [RFC v2 0/6] vTPM for aarch64 Eric Auger
2020-02-14 18:36 ` [RFC v2 1/6] tpm: rename TPM_TIS into TPM_TIS_ISA Eric Auger
2020-02-14 18:55   ` Philippe Mathieu-Daudé
2020-02-25 10:16     ` Auger Eric
2020-02-25 10:22       ` Philippe Mathieu-Daudé
2020-02-25 10:28         ` Auger Eric
2020-02-16 18:14   ` Stefan Berger
2020-02-14 18:37 ` [RFC v2 2/6] tpm: Use TPMState as a common struct Eric Auger
2020-02-14 19:01   ` Philippe Mathieu-Daudé
2020-02-16 18:22   ` Stefan Berger
2020-02-17  9:21   ` Philippe Mathieu-Daudé
2020-02-17 18:01     ` Auger Eric
2020-02-25 10:18       ` Auger Eric
2020-02-14 18:37 ` [RFC v2 3/6] tpm: Separate tpm_tis common functions from isa code Eric Auger
2020-02-16 18:27   ` Stefan Berger
2020-02-14 18:37 ` [RFC v2 4/6] tpm: Separate TPM_TIS and TPM_TIS_ISA configs Eric Auger
2020-02-14 19:03   ` Philippe Mathieu-Daudé
2020-02-25 10:05     ` Auger Eric
2020-02-14 18:37 ` [RFC v2 5/6] tpm: Add the SysBus TPM TIS device Eric Auger
2020-02-16 18:32   ` Stefan Berger
2020-02-17 18:13     ` Auger Eric
2020-02-25  9:18       ` Auger Eric
2020-02-25  9:52         ` Ard Biesheuvel
2020-02-25 10:18           ` Philippe Mathieu-Daudé
2020-02-26  8:38           ` Auger Eric
2020-02-14 18:37 ` [RFC v2 6/6] hw/arm/virt: vTPM support Eric Auger
2020-02-16 18:47   ` Stefan Berger
2020-02-17 18:26     ` Auger Eric [this message]
2020-02-16 16:35 ` [RFC v2 0/6] vTPM for aarch64 Ard Biesheuvel
2020-02-17 18:03   ` Auger Eric

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b640344f-ec88-ed79-3623-dcfc20d94541@redhat.com \
    --to=eric.auger@redhat.com \
    --cc=ardb@kernel.org \
    --cc=eric.auger.pro@gmail.com \
    --cc=lersek@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanb@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).