Re: [PATCH v5 0/2] vfio/common: remove spurious tpm-crb-cmd misalignment warning

From: Eric Auger <eric.auger@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.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 0/2] vfio/common: remove spurious tpm-crb-cmd misalignment warning
Date: Wed, 23 Nov 2022 09:16:05 +0100	[thread overview]
Message-ID: <e48c6636-99da-c607-45ce-dfadfaad8a1f@redhat.com> (raw)
In-Reply-To: <20221123013304-mutt-send-email-mst@kernel.org>

Hi Michael,

On 11/23/22 07:34, Michael S. Tsirkin wrote:
> On Fri, May 06, 2022 at 03:25:08PM +0200, Eric Auger wrote:
>> The CRB command buffer currently is a RAM MemoryRegion and given
>> its base address alignment, it causes an error report on
>> vfio_listener_region_add(). This region could have been a RAM device
>> region, easing the detection of such safe situation but this option
>> was not well received.
> Eric could you point me at this discussion please?
> We are now asked to proliferate stuff like this into vdpa
> as well, this just doesn't scale. I'd like to see whether we
> can make it a RAM device region after all - was a patch
> like that posted?
The bulk of the discussion happened in
https://lore.kernel.org/all/20220208133842.112017-1-eric.auger@redhat.com/#r

See exchanges with Peter who was against turning the CRB cmd/response
buffer into a RAM device region at that time, hence the current workaround.

You will see there also discussions about the buffer size in
https://lore.kernel.org/all/eae7e6e6-2f56-c263-f1d2-19104201c8ec@redhat.com/

Thanks

Eric
>
>> So let's add a helper function that uses the
>> memory region owner type to detect the situation is safe wrt
>> the assignment. Other device types can be checked here if such kind
>> of problem occurs again.
>>
>> As TPM devices can be compiled out we need to introduce a stub
>> for TPM_IS_CRB.
>>
>> Best Regards
>>
>> Eric
>>
>> This series can be found at:
>> https://github.com/eauger/qemu/tree/tpm-crb-vfio-v5
>>
>> History:
>>
>> v4 -> v5:
>> - Add sysemu: tpm: Add a stub function for TPM_IS_CRB to fix
>>   compilation error if CONFIG_TPM is unset
>>
>> Eric Auger (2):
>>   sysemu: tpm: Add a stub function for TPM_IS_CRB
>>   vfio/common: remove spurious tpm-crb-cmd misalignment warning
>>
>>  hw/vfio/common.c     | 27 ++++++++++++++++++++++++++-
>>  hw/vfio/trace-events |  1 +
>>  include/sysemu/tpm.h |  6 ++++++
>>  3 files changed, 33 insertions(+), 1 deletion(-)
>>
>> -- 
>> 2.35.1
>>
>>
>>