[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Roger Pau Monné writes ("Re: [PATCH for-4.16] x86/passthrough: Fix 
hvm_gsi_eoi() build with GCC 12"):
> I honestly don't think we expect any property of pirq_dpci, it just
> tells whether a pirq has a dpci associated with it or not. As I said
> on my previous replies I think GCC is not correct in doing the check
> after expanding the macro.
> 
> The relation between a pirq and it's dpci is an implementation detail
> that could change at any point, and hence the complain by GCC would no
> longer be true. That's exactly why we use a macro to get the dpci out
> of a pirq, because how that's obtained is opaque to the caller.
> 
> So while it's true that a NULL pirq will always result in a NULL dpci,
> a non-NULL pirq should not be assumed to result in a non-NULL dpci,
> which is inferred by GCC by expanding the macro.
> 
> In this specific case I think the change is fine for two reasons:
> 
>  * The pirq is obtained from the domain struct.
>  * The domain is known to be HVM because the only caller of
>    hvm_gsi_eoi is hvm_dpci_eoi which in
>    turn is only called by the vIO-APIC and the vPIC emulation code.
> 
> I dislike of GCC doing those checks to expanded macros. In this case I
> think the change is fine due to my reasoning above.
> 
> It might be appropriate to switch pirq_dpci to:
> 
> #define pirq_dpci(d, pirq) \
>     ((is_hvm_domain(d) && (pirq)) ? &(pirq)->arch.hvm.dpci : NULL)
> 
> Or to an inline helper.

Anyway, I don't think there are any functional implications.

Release-Acked-by: Ian Jackson <iwj@xxxxxxxxxxxxxx>



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.