Re: [PATCH v5 1/2] sysemu: tpm: Add a stub function for TPM_IS_CRB

[Eric Auger]

On 11/23/22 10:30, Michael S. Tsirkin wrote:
> On Wed, Nov 23, 2022 at 09:18:39AM +0100, Eric Auger wrote:
>> Hi,
>>
>> On 11/23/22 07:36, Michael S. Tsirkin wrote:
>>> On Fri, May 06, 2022 at 09:47:52AM -0400, Stefan Berger wrote:
>>>> On 5/6/22 09:25, Eric Auger wrote:
>>>>> In a subsequent patch, VFIO will need to recognize if
>>>>> a memory region owner is a TPM CRB device. Hence VFIO
>>>>> needs to use TPM_IS_CRB() even if CONFIG_TPM is unset. So
>>>>> let's add a stub function.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>> Suggested-by: Cornelia Huck <cohuck@redhat.com>
>>>> Reviewed-by: Stefan Berger <stefanb@linnux.ibm.com>
>>> ... and now in 7.2 vdpa needs a dependency on tpm too, what a hack :(
>>> And what exactly is it about TPM CRB that everyone needs to
>>> know about it and skip it? The API does not tell ...
>> An excerpt of one reply I made at that time:
>>
>> The spec (CG PC Client Platform TPM Profile (PTP)
>>     Specification Family “2.0” Level 00 Revision 01.03 v22, page 100) 
>> says that the command/response data "may be defined as large as 3968",
>> which is (0x1000 - 0x80), 0x80 being the size of the control struct.
>> so the size of the region logically is less than a 4kB page, hence our
>> trouble.
>>
>> We learnt in the past Windows driver has some stronger expectation wrt
>> memory mapping. I don't know if those latter would work if we were to
>> enlarge the window by some tricks.
>>
>> https://trustedcomputinggroup.org/wp-content/uploads/Mobile-Command-Response-Buffer-Interface-v2-r12-Specification_FINAL2.pdf
>> 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.
>>
>> Thanks
>>
>> Eric
> So we put it in system RAM then? But why isn't DMA there allowed?

I don't think there is any need and since it violates the alignment
check in VFIO we discard the region from DMA mapped ones.

Thanks

Eric
>
>>>>> ---
>>>>>   include/sysemu/tpm.h | 6 ++++++
>>>>>   1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
>>>>> index 68b2206463c..fb40e30ff60 100644
>>>>> --- a/include/sysemu/tpm.h
>>>>> +++ b/include/sysemu/tpm.h
>>>>> @@ -80,6 +80,12 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
>>>>>   #define tpm_init()  (0)
>>>>>   #define tpm_cleanup()
>>>>>
>>>>> +/* needed for an alignment check in non-tpm code */
>>>>> +static inline Object *TPM_IS_CRB(Object *obj)
>>>>> +{
>>>>> +     return NULL;
>>>>> +}
>>>>> +
>>>>>   #endif /* CONFIG_TPM */
>>>>>
>>>>>   #endif /* QEMU_TPM_H */