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

[Peter Maydell]

On Tue, 14 Nov 2023 at 02:10, Joelle van Dyne <j@getutm.app> 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.
>
> 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 (AARCH64 load pair of registers) caused page faults. @agraf suggested that
> we do this to avoid having to do AARCH64 decoding in the HVF backend's fault
> handler.

I had a conversation about this on IRC a week or so back (though I
forget who with, sorry) that made me realise there's a problem with this
approach, and I wanted to write that up for the mailing list.

The problem with turning this into a memory-backed device rather than
an MMIO backed device is that it breaks KVM on Arm CPUs which don't
have FEAT_S2FWB (i.e. anything older than Armv8.4). This is because
without FEAT_S2FWB the guest and host will disagree about the memory
attributes of the region:
 * the host knows this is RAM backed and it's normal-cacheable
   (certainly as far as the mapping that QEMU itself has)
 * the guest thinks it's real hardware device registers and maps it
   as Device
The resulting mismatch in cacheability attributes can cause unexpected
behaviour where the guest and QEMU views of the memory contents don't
necessarily match up. (This is the same underlying issue that means
that you can't use QEMU devices that emulate VGA framebuffers on
AArch64 KVM without FEAT_S2FWB.)

With FEAT_S2FWB the problem goes away because the hypervisor can
override the guest's specified memory attributes to get rid of
the attribute mismatch.

So given that this would cause a regression for KVM, my preference
is to stick with the current "the device is backed by MMIO read
and write functions in the normal way". If a particular guest is
trying to access it with LDP/STP that is best fixed in the guest.

Potentially we could emulate (interpret) some subset of complex
load/store insns in QEMU at the point where we get the "took a
data abort but no syndrome information". This ought to be doable
in a way that's shareable between hvf and KVM, and we can write
a decodetree file for the insns we want to interpret. (I would
not try to share with TCG decodetree, though the patterns can
probably be copied across.)

thanks
-- PMM