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

From: Eric Auger <eric.auger@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>,
	Stefan Berger <stefanb@linux.ibm.com>
Cc: eric.auger.pro@gmail.com, qemu-devel@nongnu.org,
	alex.williamson@redhat.com, stefanb@linux.vnet.ibm.com,
	cohuck@redhat.com, f4bug@amsat.org
Subject: Re: [PATCH v5 1/2] sysemu: tpm: Add a stub function for TPM_IS_CRB
Date: Wed, 23 Nov 2022 09:18:39 +0100	[thread overview]
Message-ID: <45e916e5-9297-a8e4-c539-123e0ee6347c@redhat.com> (raw)
In-Reply-To: <20221123013441-mutt-send-email-mst@kernel.org>

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

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