qemu-devel
[Top][All Lists]
Advanced

[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: Marc-André Lureau
Subject: Re: [PATCH v4 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region
Date: Thu, 3 Mar 2022 20:16:53 +0400

Hi

On Thu, Mar 3, 2022 at 6:41 PM Eric Auger <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>
>>> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
>>> Acked-by: Stefan Berger <stefanb@linux.ibm.com>
>>> [PMD: Keep tpm_crb.c in meson's softmmu_ss]
>>> Signed-off-by: Philippe Mathieu-Daudé <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)

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

    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.
 

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]