Re: [PATCH] IOMMU/x86: work around bogus gcc12 warning in hvm_gsi_eoi()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.06.2022 09:20, Roger Pau Monné wrote:
> On Fri, May 27, 2022 at 12:37:19PM +0200, Jan Beulich wrote:
>> As per [1] the expansion of the pirq_dpci() macro causes a -Waddress
>> controlled warning (enabled implicitly in our builds, if not by default)
>> tying the middle part of the involved conditional expression to the
>> surrounding boolean context. Work around this by introducing a local
>> inline function in the affected source file.
>>
>> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967
>> ---
>> This is intended to replace an earlier patch by Andrew [2], open-coding
>> and then simplifying the macro in the one problematic place.
>>
>> Note that, with pirq_dpci() presently used solely in the one file being
>> changed here, we could in principle also remove the #define and use just
>> an inline(?) function in this file. But then the macro would need
>> reinstating as soon as a use elsewhere would become necessary.

Did you read this before ...

>> As to the inline - I think it's warranted here, but it goes against our
>> general policy of using inline only in header files. Hence I'd be okay
>> to drop it to avoid controversy.
>>
>> [2] https://lists.xen.org/archives/html/xen-devel/2021-10/msg01635.html
>>
>> --- a/xen/drivers/passthrough/x86/hvm.c
>> +++ b/xen/drivers/passthrough/x86/hvm.c
>> @@ -25,6 +25,18 @@
>>  #include <asm/hvm/support.h>
>>  #include <asm/io_apic.h>
>>  
>> +/*
>> + * Gcc12 takes issue with pirq_dpci() being used in boolean context (see gcc
>> + * bug 102967). While we can't replace the macro definition in the header 
>> by an
>> + * inline function, we can do so here.
>> + */
>> +static inline struct hvm_pirq_dpci *_pirq_dpci(struct pirq *pirq)
>> +{
>> +    return pirq_dpci(pirq);
>> +}
>> +#undef pirq_dpci
>> +#define pirq_dpci(pirq) _pirq_dpci(pirq)
> 
> That's fairly ugly.  Seeing as pirq_dpci is only used in hvm.c, would
> it make sense to just convert the macro to be a static inline in that
> file? (and remove pirq_dpci() from irq.h).

... saying so? IOW I'm not entirely opposed, but I'm a little afraid we might
be setting us up for later trouble. 

Jan