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

[Cornelia Huck]

On Fri, May 06 2022, Eric Auger <eric.auger@redhat.com> wrote:

> Hi Alex,
>
> On 4/28/22 22:14, Alex Williamson wrote:
>> On Thu, 28 Apr 2022 15:49:45 +0200
>> Eric Auger <eric.auger@redhat.com> wrote:

>>> +static bool vfio_known_safe_misalignment(MemoryRegionSection *section)
>>> +{
>>> +    MemoryRegion *mr = section->mr;
>>> +
>>> +    if (!TPM_IS_CRB(mr->owner)) {
>>> +        return false;
>>> +    }
>> It looks like this test is going to need to be wrapped in #ifdef
>> CONFIG_TPM:
>
> sorry for the delay. Your message fell though the cracks :-(
>
> if I put an '#ifdef CONFIG_TPM' I need to inverse the logic because by
> default the function shall return false.
>
> solution #1
>
> #ifdef CONFIG_TPM  
>  if (TPM_IS_CRB(mr->owner)) {
>
>     /* this is a known safe misaligned region, just trace for debug purpose */
>     trace_vfio_known_safe_misalignment(memory_region_name(mr),
>                                        section->offset_within_address_space,
>                                        section->offset_within_region,
>                                        qemu_real_host_page_size());
>
>     return true;
>    }
>
> #endif
> return false;
>
> This looks weird to me.
>
> +    if (!object_dynamic_cast(mr->owner, TYPE_TPM_CRB)) {
> +        return false;
> +    }
>
>
> solution #2
> replace !object_dynamic_cast(mr->owner, TYPE_TPM_CRB) by
> !object_dynamic_cast(mr->owner, "tpm-crb")
> and add a comment saying that we don't use TYPE_TPM_CRB on purpose
>
> solution #3
> Move #define TPM_IS_CRB(chr) and related defined out of
> #ifdef CONFIG_TPM hoping it does not have other side effects
>
> Thoughts?
> Eric

solution #4

#ifndef CONFIG_TPM
/* needed for an alignment check in non-tpm code */
static inline Object *TPM_IS_CRB(Object *obj)
{
    return NULL;
}
#endif

I think it would be good if we could hide the configuration details in
the header.