Re: [PATCH v4 6/6] hw/riscv: Enable TPM backends

[Alistair Francis]

On Wed, Jun 21, 2023 at 4:34 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Mon, Jun 19, 2023 at 01:32:34PM -0700, Guenter Roeck wrote:
> > Hi Alistair,
> >
> > On Wed, Apr 20, 2022 at 03:52:48PM +1000, Alistair Francis wrote:
> > > From: Alistair Francis <alistair.francis@wdc.com>
> > >
> > > Imply the TPM sysbus devices. This allows users to add TPM devices to
> > > the RISC-V virt board.
> > >
> > > This was tested by first creating an emulated TPM device:
> > >
> > >     swtpm socket --tpm2 -t -d --tpmstate dir=/tmp/tpm \
> > >         --ctrl type=unixio,path=swtpm-sock
> > >
> > > Then launching QEMU with:
> > >
> > >     -chardev socket,id=chrtpm,path=swtpm-sock \
> > >     -tpmdev emulator,id=tpm0,chardev=chrtpm \
> > >     -device tpm-tis-device,tpmdev=tpm0
> > >
> > > The TPM device can be seen in the memory tree and the generated device
> > > tree.
> > >
> > I tried to get this working with qemu 8.0, but I did not have any success.
> > I am quite sure I have the above command line correctly, and it does work
> > with arm64. Any idea what I might be missing ?
> >
>
> Answering my own question: Nothing. The problem is that the devicetree
> is created too early, before the tpm device is instantiated/realized in
> qemu. The tpm device therefore does not show up in devicetree, and the
> tom device does not instantiate in Linux. The patch below fixes the problem
> for me.

Thanks for looking into this!

>
> Any comments / thoughts ? Is that change acceptable, or should it be
> implemented differently ?

The diff below looks fine to me. Do you mind sending a patch?

Alistair

>
> Thanks,
> Guenter
>
> ---
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 4e3efbee16..ea259d7ade 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1247,6 +1247,11 @@ static void virt_machine_done(Notifier *notifier, void 
> *data)
>      uint32_t fdt_load_addr;
>      uint64_t kernel_entry;
>
> +    /* create devicetree if not provided */
> +    if (!machine->dtb) {
> +        create_fdt(s, memmap);
> +    }
> +
>      /*
>       * Only direct boot kernel is currently supported for KVM VM,
>       * so the "-bios" parameter is not supported when KVM is enabled.
> @@ -1519,15 +1524,13 @@ static void virt_machine_init(MachineState *machine)
>      }
>      virt_flash_map(s, system_memory);
>
> -    /* load/create device tree */
> +    /* load device tree */
>      if (machine->dtb) {
>          machine->fdt = load_device_tree(machine->dtb, &s->fdt_size);
>          if (!machine->fdt) {
>              error_report("load_device_tree() failed");
>              exit(1);
>          }
> -    } else {
> -        create_fdt(s, memmap);
>      }
>
>      s->machine_done.notify = virt_machine_done;