Re: [PATCH v4 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region

[Eric Auger <eauger@redhat.com>]

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?

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
>>
>>
>>
>