Re: [Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Igor Mammedov]

On Tue, 17 Jul 2018 15:04:25 +0200
Laszlo Ersek <address@hidden> wrote:

> On 07/17/18 12:39, Marc-André Lureau wrote:
> > Hi
> > 
> > On Tue, Jul 17, 2018 at 12:03 PM, Igor Mammedov <address@hidden> wrote:  
> >> On Mon, 16 Jul 2018 14:59:45 +0200
> >> Marc-André Lureau <address@hidden> wrote:
> >>  
> >>> From: Stefan Berger <address@hidden>
> >>>
> >>> 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.  
> >> ...
> >> another question wrt E820 table, is firmware putting a reservation record
> >> for this RAM area overlay or should we do it QEMU?  
> 
> What OS requires this memmap entry?
> 
> > There doesn't seem to be any reservation done in ovmf, or on my
> > laptop.  
> 
> Relevant point, about your laptop.
> 
> > I also wonder if it should be done.  
> 
> IMO: no, it shouldn't.
> 
> > And how OS deal with that today (/proc/iomem is correct).
> > 
> > Laszlo, any idea?  
> 
> In my experience, exposing such MMIO areas that belong to *platform*
> devices, in the UEFI memory map, is an inconsistent practice. I can list
> at least four different cases:
> 
> (1) A UEFI runtime driver is using the MMIO area for providing a UEFI
> runtime service. In this case, the firmware must expose the area as
> "MMIO" type memory, and set the "runtime" attribute on it. In response,
> the OS will map the area with virtual addresses, directly in response to
> the memmap entry.
> 
> (2) A firmware SMM driver is using the MMIO area for providing a
> privileged part of a UEFI runtime service. Because SMM uses its own page
> table anyway (protected from the OS), advertizing the area in the UEFI
> memmap is unneeded. It can simply be left as a gap.
> 
> (3) Linux requires the firmware to cover every MMCONFIG area (each of
> which belongs to a PCIe host bridge, which is a platform device) with a
> "reserved" type memmap entry. To me this looks like an outlier, although
> I *very* vaguely recall that some spec recommended this.
> 
> Ah, right, the PCI Firmware Specification says, under "MCFG Table
> Description": "*If* the operating system does not natively comprehend
> reserving the MMCFG region, the MMCFG region must be reserved by
> firmware." (Emphasis mine.) In other words, this is a provision in the
> PCI firmware spec so that platform firmware paper over the fact that the
> ACPI subsystem in the OS kernel is limited. Linux, given that it
> understand MCFG just fine, totally should not *require* the firmware to
> reserve this region in the UEFI memory map as well. Anyway, Linux does
> require it (incorrectly, in my opinion), so OVMF complies.
> 
> (4) To my understanding, unless given Linux or Windows quirks like (3),
> or UEFI runtime drivers like (1), we shouldn't expose, in the UEFI
> memmap, MMIO areas related to platform devices. In particular, the
> "MMIO" type memory *without* the "runtime" attribute (see (1)) should
> never be used in the UEFI memmap (and edk2 will never create such
> entries) -- the UEFI spec says that all such areas (resources) should be
> communicated through ACPI tables ("All system memory-mapped IO
> information should come from ACPI tables" -- see EfiMemoryMappedIO in
> "Table 26. Memory Type Usage after ExitBootServices()").
> 
> 
> In brief, the TPM PPI MMIO area should just be left as a gap in the UEFI
> memmap. (Case (4) applies.)
so I guess we are good as the patch should fit the bill for (4)

> 
> Thanks,
> Laszlo