[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region
From: |
Eric Auger |
Subject: |
Re: [PATCH v4 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region |
Date: |
Fri, 4 Mar 2022 10:32:30 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 |
Hi Marc-André,
On 3/3/22 5:16 PM, Marc-André Lureau wrote:
> Hi
>
> On Thu, Mar 3, 2022 at 6:41 PM Eric Auger <eauger@redhat.com
> <mailto:eauger@redhat.com>> wrote:
>
> Hi Stefan,
>
> On 2/8/22 6:58 PM, Eric Auger wrote:
> > Hi Stefan,
> >
> > On 2/8/22 6:16 PM, Stefan Berger wrote:
> >>
> >> On 2/8/22 08:38, Eric Auger wrote:
> >>> Representing the CRB cmd/response buffer as a standard
> >>> RAM region causes some trouble when the device is used
> >>> with VFIO. Indeed VFIO attempts to DMA_MAP this region
> >>> as usual RAM but this latter does not have a valid page
> >>> size alignment causing such an error report:
> >>> "vfio_listener_region_add received unaligned region".
> >>> To allow VFIO to detect that failing dma mapping
> >>> this region is not an issue, let's use a ram_device
> >>> memory region type instead.
> >>>
> >>> Signed-off-by: Eric Auger <eric.auger@redhat.com
> <mailto:eric.auger@redhat.com>>
> >>> Tested-by: Stefan Berger <stefanb@linux.ibm.com
> <mailto:stefanb@linux.ibm.com>>
> >>> Acked-by: Stefan Berger <stefanb@linux.ibm.com
> <mailto:stefanb@linux.ibm.com>>
> >>> [PMD: Keep tpm_crb.c in meson's softmmu_ss]
> >>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org
> <mailto:f4bug@amsat.org>>
> >>
> >>
> >> v4 doesn't build for me:
> >>
> >> ../hw/tpm/tpm_crb.c: In function ?tpm_crb_realize?:
> >> ../hw/tpm/tpm_crb.c:297:33: error: implicit declaration of function
> >> ?HOST_PAGE_ALIGN? [-Werror=implicit-function-declaration]
> >> 297 | HOST_PAGE_ALIGN(CRB_CTRL_CMD_SIZE));
> >> | ^~~~~~~~~~~~~~~
> >> ../hw/tpm/tpm_crb.c:297:33: error: nested extern declaration of
> >> ?HOST_PAGE_ALIGN? [-Werror=nested-externs]
> >> cc1: all warnings being treated as errors
> >
> > Do you have
> > b269a70810a exec/cpu: Make host pages variables / macros 'target
> > agnostic' in your tree?
> I may have missed your reply. Did you have that dependency? Were you
> able to compile eventually?
>
> Besides, do you have any opinion overall about the relevance of
> transforming the CRB ctrl cmd region into a RAM device wrt the TPM spec?
>
> Again spec says:
>
> "
> Including the control structure, the three memory areas comprise the
> entirety of the CRB. There are no constraints on how those three memory
> areas are provided. They can all be in system RAM, or all be in device
> memory, or any combination.
> "
>
> (https://trustedcomputinggroup.org/wp-content/uploads/Mobile-Command-Response-Buffer-Interface-v2-r12-Specification_FINAL2.pdf
>
> <https://trustedcomputinggroup.org/wp-content/uploads/Mobile-Command-Response-Buffer-Interface-v2-r12-Specification_FINAL2.pdf>)
>
> What was the rationale behind using RAM device for the PPI region?
>
>
> Is this the rationale you are looking for?
> https://gitlab.com/qemu-project/qemu/-/commit/3b97c01e9ccdfbd517a0fd631838d6252dbfa692
> <https://gitlab.com/qemu-project/qemu/-/commit/3b97c01e9ccdfbd517a0fd631838d6252dbfa692>
>
> Note: bios_linker cannot be used to allocate the PPI memory region,
> since the reserved memory should stay stable across reboots, and might
> be needed before the ACPI tables are installed.
And did this mandate to use "ram_device" memory type instead of standard
system RAM.
As I understand the spec (statement above), the CRB areas can be
implemented as system RAM or device memory. So I want to understand why
using RAM device for the CRB is not a reasonable choice. By the way I
understand my motivation behind that change is a bit far-fetched and
aiming at fixing another issue, but well ;-)
Thanks
Eric
>
>
>
> There are some spurious warnings when using CRB with VFIO and that would
> be nice to remove them one way or the other.
>
> Thanks
>
> Eric
> >
> > Thanks
> >
> > Eric
> >>
> >>
> >>
> >
>
>
>
>
> --
> Marc-André Lureau