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? IOW we could change how pirq_dpci is implemented and then that relation might no longer be true. What we care in hvm_gsi_eoi is not only having a valid pirq, but also having a valid dpci struct which will only be the case if the PIRQ is bound to an HVM domain, and that is what the check tries to represent. I know this is not how pirq_dpci is currently implemented, but I don't see why it couldn't change in the future. Thanks, Roger.
