Re: [PATCH 00/11] tpm: introduce TPM CRB SysBus device

2023-07-13 Thread Joelle van Dyne
On Thu, Jul 13, 2023 at 6:07 AM Stefan Berger  wrote:
>
>
>
> On 7/12/23 23:51, Joelle van Dyne wrote:
> > The impetus for this patch set is to get TPM 2.0 working on Windows 11 
> > ARM64.
> > Windows' tpm.sys does not seem to work on a TPM TIS device (as verified with
> > VMWare's implementation). However, the current TPM CRB device uses a fixed
> > system bus address that is reserved for RAM in ARM64 Virt machines.
>
> Thanks a lot for this work. The last sentence seems to hint at the current 
> issue
> with TPM CRB on ARM64 and seems to be the only way forward there. You may want
> to reformulate it a bit because it's not clear how the 'however' related to
> CRB relates to TIS.
>
> >
> > In the process of adding the TPM CRB SysBus device, we also went ahead and
> > cleaned up some of the existing TPM hardware code and fixed some bugs. We 
> > used
>
> Please reorder bugs to the beginning of the series or submit in an extra 
> patch set
> so we can backport them. Ideal would be description(s) for how to trigger the 
> bug(s).
>
> > the TPM TIS devices as a template for the TPM CRB devices and refactored out
> > common code. We moved the ACPI DSDT generation to the device in order to 
> > handle
> > dynamic base address requirements as well as reduce redundent code in 
> > different
> s/redundent/redundant
>
>
> > machine ACPI generation. We also changed the tpm_crb device to use the ISA 
> > bus
> > instead of depending on the default system bus as the device only was built 
> > for
> > the PC configuration.
> >
> > Another change is that the TPM CRB registers are now mapped in the same way 
> > that
> > the pflash ROM devices are mapped. It is a memory region whose writes are
> > trapped as MMIO accesses. This was needed because Apple Silicon does not 
> > decode
> > LDP caused page faults. @agraf suggested that we do this to avoid having to
>
> Afaik, LDP is an ARM assembly instruction that loads two 32bit or 64bit 
> registers from
> consecutive addresses. May be worth mentioning for those wondering about it...
>
> > do AARCH64 decoding in the HVF fault handler.
>
> What is HVF?

Sorry, HVF is the QEMU backend for Apple's Hypervisor.framework which
runs on macOS including on Apple Silicon.

>
> Regards,
> Stefan
> >
> > Unfortunately, it seems like the LDP fault still happens on HVF but the 
> > issue
> > seems to be in the HVF backend which needs to be fixed in a separate patch.
> >
> > One last thing that's needed to get Windows 11 to recognize the TPM 2.0 
> > device
> > is for the OVMF firmware to setup the TPM device. Currently, OVMF for ARM64 
> > Virt
> > only recognizes the TPM TIS device through a FDT entry. A workaround is to
> > falsely identify the TPM CRB device as a TPM TIS device in the FDT node but 
> > this
> > causes issues for Linux. A proper fix would involve adding an ACPI device 
> > driver
> > in OVMF.
> >
> > Joelle van Dyne (11):
> >tpm_crb: refactor common code
> >tpm_crb: CTRL_RSP_ADDR is 64-bits wide
> >tpm_ppi: refactor memory space initialization
> >tpm_crb: use a single read-as-mem/write-as-mmio mapping
> >tpm_crb: use the ISA bus
> >tpm_crb: move ACPI table building to device interface
> >hw/arm/virt: add plug handler for TPM on SysBus
> >hw/loongarch/virt: add plug handler for TPM on SysBus
> >tpm_tis_sysbus: fix crash when PPI is enabled
> >tpm_tis_sysbus: move DSDT AML generation to device
> >tpm_crb_sysbus: introduce TPM CRB SysBus device
> >
> >   docs/specs/tpm.rst  |   2 +
> >   hw/tpm/tpm_crb.h|  74 +
> >   hw/tpm/tpm_ppi.h|  10 +-
> >   include/hw/acpi/aml-build.h |   1 +
> >   include/hw/acpi/tpm.h   |   3 +-
> >   include/sysemu/tpm.h|   3 +
> >   hw/acpi/aml-build.c |   7 +-
> >   hw/arm/virt-acpi-build.c|  38 +
> >   hw/arm/virt.c   |  38 +
> >   hw/core/sysbus-fdt.c|   1 +
> >   hw/i386/acpi-build.c|  23 ---
> >   hw/loongarch/acpi-build.c   |  38 +
> >   hw/loongarch/virt.c |  38 +
> >   hw/riscv/virt.c |   1 +
> >   hw/tpm/tpm_crb.c| 307 
> >   hw/tpm/tpm_crb_common.c | 224 ++
> >   hw/tpm/tpm_crb_sysbus.c | 178 +
> >   hw/tpm/tpm_ppi.c|   5 +-
> >   hw/tpm/tpm_tis_isa.c|   5 +-
> >   hw/tpm/tpm_tis_sysbus.c |  43 +
> >   tests/qtest/tpm-crb-test.c  |   2 +-
> >   tests/qtest/tpm-util.c  |   2 +-
> >   hw/arm/Kconfig  |   1 +
> >   hw/riscv/Kconfig|   1 +
> >   hw/tpm/Kconfig  |   7 +-
> >   hw/tpm/meson.build  |   3 +
> >   hw/tpm/trace-events |   2 +-
> >   27 files changed, 703 insertions(+), 354 deletions(-)
> >   create mode 100644 hw/tpm/tpm_crb.h
> >   create mode 100644 hw/tpm/tpm_crb_common.c
> >   create mode 100644 hw/tpm/tpm_crb_sysbus.c
> >



Re: [PATCH 00/11] tpm: introduce TPM CRB SysBus device

2023-07-13 Thread Stefan Berger




On 7/12/23 23:51, Joelle van Dyne wrote:

The impetus for this patch set is to get TPM 2.0 working on Windows 11 ARM64.
Windows' tpm.sys does not seem to work on a TPM TIS device (as verified with
VMWare's implementation). However, the current TPM CRB device uses a fixed
system bus address that is reserved for RAM in ARM64 Virt machines.


Thanks a lot for this work. The last sentence seems to hint at the current issue
with TPM CRB on ARM64 and seems to be the only way forward there. You may want
to reformulate it a bit because it's not clear how the 'however' related to
CRB relates to TIS.



In the process of adding the TPM CRB SysBus device, we also went ahead and
cleaned up some of the existing TPM hardware code and fixed some bugs. We used


Please reorder bugs to the beginning of the series or submit in an extra patch 
set
so we can backport them. Ideal would be description(s) for how to trigger the 
bug(s).


the TPM TIS devices as a template for the TPM CRB devices and refactored out
common code. We moved the ACPI DSDT generation to the device in order to handle
dynamic base address requirements as well as reduce redundent code in different

s/redundent/redundant



machine ACPI generation. We also changed the tpm_crb device to use the ISA bus
instead of depending on the default system bus as the device only was built for
the PC configuration.

Another change is that the TPM CRB registers are now mapped in the same way that
the pflash ROM devices are mapped. It is a memory region whose writes are
trapped as MMIO accesses. This was needed because Apple Silicon does not decode
LDP caused page faults. @agraf suggested that we do this to avoid having to


Afaik, LDP is an ARM assembly instruction that loads two 32bit or 64bit 
registers from
consecutive addresses. May be worth mentioning for those wondering about it...


do AARCH64 decoding in the HVF fault handler.


What is HVF?

Regards,
   Stefan


Unfortunately, it seems like the LDP fault still happens on HVF but the issue
seems to be in the HVF backend which needs to be fixed in a separate patch.

One last thing that's needed to get Windows 11 to recognize the TPM 2.0 device
is for the OVMF firmware to setup the TPM device. Currently, OVMF for ARM64 Virt
only recognizes the TPM TIS device through a FDT entry. A workaround is to
falsely identify the TPM CRB device as a TPM TIS device in the FDT node but this
causes issues for Linux. A proper fix would involve adding an ACPI device driver
in OVMF.

Joelle van Dyne (11):
   tpm_crb: refactor common code
   tpm_crb: CTRL_RSP_ADDR is 64-bits wide
   tpm_ppi: refactor memory space initialization
   tpm_crb: use a single read-as-mem/write-as-mmio mapping
   tpm_crb: use the ISA bus
   tpm_crb: move ACPI table building to device interface
   hw/arm/virt: add plug handler for TPM on SysBus
   hw/loongarch/virt: add plug handler for TPM on SysBus
   tpm_tis_sysbus: fix crash when PPI is enabled
   tpm_tis_sysbus: move DSDT AML generation to device
   tpm_crb_sysbus: introduce TPM CRB SysBus device

  docs/specs/tpm.rst  |   2 +
  hw/tpm/tpm_crb.h|  74 +
  hw/tpm/tpm_ppi.h|  10 +-
  include/hw/acpi/aml-build.h |   1 +
  include/hw/acpi/tpm.h   |   3 +-
  include/sysemu/tpm.h|   3 +
  hw/acpi/aml-build.c |   7 +-
  hw/arm/virt-acpi-build.c|  38 +
  hw/arm/virt.c   |  38 +
  hw/core/sysbus-fdt.c|   1 +
  hw/i386/acpi-build.c|  23 ---
  hw/loongarch/acpi-build.c   |  38 +
  hw/loongarch/virt.c |  38 +
  hw/riscv/virt.c |   1 +
  hw/tpm/tpm_crb.c| 307 
  hw/tpm/tpm_crb_common.c | 224 ++
  hw/tpm/tpm_crb_sysbus.c | 178 +
  hw/tpm/tpm_ppi.c|   5 +-
  hw/tpm/tpm_tis_isa.c|   5 +-
  hw/tpm/tpm_tis_sysbus.c |  43 +
  tests/qtest/tpm-crb-test.c  |   2 +-
  tests/qtest/tpm-util.c  |   2 +-
  hw/arm/Kconfig  |   1 +
  hw/riscv/Kconfig|   1 +
  hw/tpm/Kconfig  |   7 +-
  hw/tpm/meson.build  |   3 +
  hw/tpm/trace-events |   2 +-
  27 files changed, 703 insertions(+), 354 deletions(-)
  create mode 100644 hw/tpm/tpm_crb.h
  create mode 100644 hw/tpm/tpm_crb_common.c
  create mode 100644 hw/tpm/tpm_crb_sysbus.c





[PATCH 00/11] tpm: introduce TPM CRB SysBus device

2023-07-12 Thread Joelle van Dyne
The impetus for this patch set is to get TPM 2.0 working on Windows 11 ARM64.
Windows' tpm.sys does not seem to work on a TPM TIS device (as verified with
VMWare's implementation). However, the current TPM CRB device uses a fixed
system bus address that is reserved for RAM in ARM64 Virt machines.

In the process of adding the TPM CRB SysBus device, we also went ahead and
cleaned up some of the existing TPM hardware code and fixed some bugs. We used
the TPM TIS devices as a template for the TPM CRB devices and refactored out
common code. We moved the ACPI DSDT generation to the device in order to handle
dynamic base address requirements as well as reduce redundent code in different
machine ACPI generation. We also changed the tpm_crb device to use the ISA bus
instead of depending on the default system bus as the device only was built for
the PC configuration.

Another change is that the TPM CRB registers are now mapped in the same way that
the pflash ROM devices are mapped. It is a memory region whose writes are
trapped as MMIO accesses. This was needed because Apple Silicon does not decode
LDP caused page faults. @agraf suggested that we do this to avoid having to
do AARCH64 decoding in the HVF fault handler.

Unfortunately, it seems like the LDP fault still happens on HVF but the issue
seems to be in the HVF backend which needs to be fixed in a separate patch.

One last thing that's needed to get Windows 11 to recognize the TPM 2.0 device
is for the OVMF firmware to setup the TPM device. Currently, OVMF for ARM64 Virt
only recognizes the TPM TIS device through a FDT entry. A workaround is to
falsely identify the TPM CRB device as a TPM TIS device in the FDT node but this
causes issues for Linux. A proper fix would involve adding an ACPI device driver
in OVMF.

Joelle van Dyne (11):
  tpm_crb: refactor common code
  tpm_crb: CTRL_RSP_ADDR is 64-bits wide
  tpm_ppi: refactor memory space initialization
  tpm_crb: use a single read-as-mem/write-as-mmio mapping
  tpm_crb: use the ISA bus
  tpm_crb: move ACPI table building to device interface
  hw/arm/virt: add plug handler for TPM on SysBus
  hw/loongarch/virt: add plug handler for TPM on SysBus
  tpm_tis_sysbus: fix crash when PPI is enabled
  tpm_tis_sysbus: move DSDT AML generation to device
  tpm_crb_sysbus: introduce TPM CRB SysBus device

 docs/specs/tpm.rst  |   2 +
 hw/tpm/tpm_crb.h|  74 +
 hw/tpm/tpm_ppi.h|  10 +-
 include/hw/acpi/aml-build.h |   1 +
 include/hw/acpi/tpm.h   |   3 +-
 include/sysemu/tpm.h|   3 +
 hw/acpi/aml-build.c |   7 +-
 hw/arm/virt-acpi-build.c|  38 +
 hw/arm/virt.c   |  38 +
 hw/core/sysbus-fdt.c|   1 +
 hw/i386/acpi-build.c|  23 ---
 hw/loongarch/acpi-build.c   |  38 +
 hw/loongarch/virt.c |  38 +
 hw/riscv/virt.c |   1 +
 hw/tpm/tpm_crb.c| 307 
 hw/tpm/tpm_crb_common.c | 224 ++
 hw/tpm/tpm_crb_sysbus.c | 178 +
 hw/tpm/tpm_ppi.c|   5 +-
 hw/tpm/tpm_tis_isa.c|   5 +-
 hw/tpm/tpm_tis_sysbus.c |  43 +
 tests/qtest/tpm-crb-test.c  |   2 +-
 tests/qtest/tpm-util.c  |   2 +-
 hw/arm/Kconfig  |   1 +
 hw/riscv/Kconfig|   1 +
 hw/tpm/Kconfig  |   7 +-
 hw/tpm/meson.build  |   3 +
 hw/tpm/trace-events |   2 +-
 27 files changed, 703 insertions(+), 354 deletions(-)
 create mode 100644 hw/tpm/tpm_crb.h
 create mode 100644 hw/tpm/tpm_crb_common.c
 create mode 100644 hw/tpm/tpm_crb_sysbus.c

-- 
2.39.2 (Apple Git-143)