Re: [PATCH v2] xen/pci: Refactor PCI MSI interrupts related code

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 14/04/2021 08:08, Jan Beulich wrote:
> On 13.04.2021 19:12, Julien Grall wrote:
> > On 12/04/2021 11:49, Roger Pau Monné wrote:
> > > On Fri, Apr 09, 2021 at 05:00:41PM +0100, Rahul Singh wrote:
> > > > --- a/xen/include/xen/vpci.h
> > > > +++ b/xen/include/xen/vpci.h
> > > > @@ -91,6 +91,7 @@ struct vpci {
> > > >            /* FIXME: currently there's no support for SR-IOV. */
> > > >        } header;
> > > >    
> > > > +#ifdef CONFIG_PCI_MSI_INTERCEPT
> > > >        /* MSI data. */
> > > >        struct vpci_msi {
> > > >          /* Address. */
> > > > @@ -136,6 +137,7 @@ struct vpci {
> > > >                struct vpci_arch_msix_entry arch;
> > > >            } entries[];
> > > >        } *msix;
> > > > +#endif /* CONFIG_PCI_MSI_INTERCEPT */
> > > Note that here you just remove two pointers from the struct, not that
> > > I'm opposed to it, but it's not that much space that's saved anyway.
> > > Ie: it might also be fine to just leave them as NULL unconditionally
> > > on Arm.
> > Can the two pointers be NULL on x86? If not, then I would prefer if they
> > disappear on Arm so there is less chance to make any mistake (such as
> > unconditionally accessing the pointer in common code).
> Alternative proposal: How about making it effectively impossible to
> de-reference the pointer on Arm by leaving the field there, but having
> the struct definition available on non-Arm only?
Meh, it making more difficult for the reader because it has to go 
through one more hop to find out why the structure is not defined.
IOW, I still prefer Rahul's version.

Cheers,

--
Julien Grall