Re: [PATCH] x86/passthrough: Fix hvm_gsi_eoi() build with GCC 12

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/10/2021 08:31, Roger Pau Monné wrote:
> On Wed, Oct 27, 2021 at 09:07:13PM +0100, Andrew Cooper wrote:
>> GCC master (nearly version 12) complains:
>>
>>   hvm.c: In function 'hvm_gsi_eoi':
>>   hvm.c:905:10: error: the comparison will always evaluate as 'true' for the
>>   address of 'dpci' will never be NULL [-Werror=address]
>>     905 |     if ( !pirq_dpci(pirq) )
>>         |          ^
>>   In file included from /local/xen.git/xen/include/xen/irq.h:73,
>>                    from /local/xen.git/xen/include/xen/pci.h:13,
>>                    from /local/xen.git/xen/include/asm/hvm/io.h:22,
>>                    from /local/xen.git/xen/include/asm/hvm/domain.h:27,
>>                    from /local/xen.git/xen/include/asm/domain.h:7,
>>                    from /local/xen.git/xen/include/xen/domain.h:8,
>>                    from /local/xen.git/xen/include/xen/sched.h:11,
>>                    from /local/xen.git/xen/include/xen/event.h:12,
>>                    from hvm.c:20:
>>   /local/xen.git/xen/include/asm/irq.h:140:34: note: 'dpci' declared here
>>     140 |             struct hvm_pirq_dpci dpci;
>>         |                                  ^~~~
>>
>> The location marker is unhelpfully positioned and upstream may get around to
>> fixing it.  The complaint is intended to be:
>>
>>   if ( !((pirq) ? &(pirq)->arch.hvm.dpci : NULL) )
>>                   ^~~~~~~~~~~~~~~~~~~~~~
>>
>> which is a hint that the code is should be simplified to just:
>>
>>   if ( !pirq )
> I likely need more coffee, but doesn't this exploit how the macro
> (pirq_dpci) is currently coded?

The way pirq_dpci() is currently coded, this is nonsense, which GCC is
now highlighting.

It would be a very different thing if the logic said:

struct pirq *pirq = pirq_info(d, gsi);
struct hvm_pirq_dpci *dpci = pirq_dpci(pirq);

/* Check if GSI is actually mapped. */
if ( !dpci )
    return;

but it doesn't. Getting a non-null pirq pointer from pirq_info(d, gsi)
does identify that the GSI is mapped, and the dpci nested structure is
not used in this function.  I would expect this property to remain true
even if we alter the data layout.

~Andrew