On Wed, Jun 27, 2018 at 4:19 PM, Igor Mammedov <[email protected]> wrote: > On Wed, 27 Jun 2018 08:53:28 -0400 > Stefan Berger <[email protected]> wrote: > >> On 06/27/2018 07:44 AM, Igor Mammedov wrote: >> > On Tue, 26 Jun 2018 14:23:41 +0200 >> > Marc-André Lureau <[email protected]> wrote: >> > >> >> From: Stefan Berger <[email protected]> >> >> >> >> Implement a virtual memory device for the TPM Physical Presence interface. >> >> The memory is located at 0xFED45000 and used by ACPI to send messages to >> >> the >> >> firmware (BIOS) and by the firmware to provide parameters for each one of >> >> the supported codes. >> >> >> >> This device should be used by all TPM interfaces on x86 and can be added >> >> by calling tpm_ppi_init_io(). >> >> >> >> Signed-off-by: Stefan Berger <[email protected]> >> >> Signed-off-by: Marc-André Lureau <[email protected]> >> >> >> >> --- >> >> >> >> v4 (Marc-André): >> >> - pass TPM_PPI_ADDR_BASE as argument to tpm_ppi_init_io() >> >> - only enable PPI if property is set >> >> >> >> v3 (Marc-André): >> >> - merge CRB support >> >> - use trace events instead of DEBUG printf >> >> - headers inclusion simplification >> >> >> >> v2: >> >> - moved to byte access since an infrequently used device; >> >> this simplifies code >> >> - increase size of device to 0x400 >> >> - move device to 0xfffef000 since SeaBIOS has some code at 0xffff0000: >> >> 'On the emulators, the bios at 0xf0000 is also at 0xffff0000' >> >> --- >> >> hw/tpm/tpm_ppi.h | 27 ++++++++++++++++++++ >> >> include/hw/acpi/tpm.h | 6 +++++ >> >> hw/tpm/tpm_crb.c | 7 ++++++ >> >> hw/tpm/tpm_ppi.c | 57 +++++++++++++++++++++++++++++++++++++++++++ >> >> hw/tpm/tpm_tis.c | 7 ++++++ >> >> hw/tpm/Makefile.objs | 2 +- >> >> hw/tpm/trace-events | 4 +++ >> >> 7 files changed, 109 insertions(+), 1 deletion(-) >> >> create mode 100644 hw/tpm/tpm_ppi.h >> >> create mode 100644 hw/tpm/tpm_ppi.c >> >> >> >> diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h >> >> new file mode 100644 >> >> index 0000000000..ac7ad47238 >> >> --- /dev/null >> >> +++ b/hw/tpm/tpm_ppi.h >> >> @@ -0,0 +1,27 @@ >> >> +/* >> >> + * TPM Physical Presence Interface >> >> + * >> >> + * Copyright (C) 2018 IBM Corporation >> >> + * >> >> + * Authors: >> >> + * Stefan Berger <[email protected]> >> >> + * >> >> + * This work is licensed under the terms of the GNU GPL, version 2 or >> >> later. >> >> + * See the COPYING file in the top-level directory. >> >> + */ >> >> +#ifndef TPM_TPM_PPI_H >> >> +#define TPM_TPM_PPI_H >> >> + >> >> +#include "hw/acpi/tpm.h" >> >> +#include "exec/address-spaces.h" >> >> + >> >> +typedef struct TPMPPI { >> >> + MemoryRegion mmio; >> >> + >> >> + uint8_t ram[TPM_PPI_ADDR_SIZE]; >> >> +} TPMPPI; >> > I probably miss something obvious here, >> > 1st: >> > commit message says that memory reion is supposed to be interface >> > between FW and OSPM (i.e. totally guest internal thingy). >> > So question is: >> > why do we register memory region at all? >> >> One reason for the device itself was being able to debug the interaction >> of the guest with ACPI though I had additional instrumentation for that >> showing register contents. >> We need it to have some memory in the region where we place it. I >> suppose a memory_region_init_ram() would provide migration support >> automatically but cannot be used on memory where we have >> MemoryRegionOps. So we could drop most parts of the device and only run >> memory_region_init_ram() ? > if QEMU doesn't need to touch it ever, you could do even better. > Use bios_linker to make FW allocate a reserved portion if guest RAM and > patch TPM aml code with allocated address. Then you won't have to worry > about migration as reserved area is already migrated as part of RAM.
The location needs to be fixed across reboots (or content moved somehow..) And timing-wise, I am not sure the ACPI tables are installed and processed by firmware before the TPM PPI operation must be handled. > > Huge benefit here is that QEMU doesn't dictate address so no chance > of conflicts and related compat hacks in case we have to move it. > > To do it easily, I'd suggest to extract TPM from DSDT into a dedicated > SSDT table (see vmgenid_build_acpi() as example /you don't need > bios_linker_loader_write_pointer() part, later we can use it if > it would be necessary for QEMU to know address/) > >> Stefan >> > > -- Marc-André Lureau
