Re: [Xen-devel] [PATCH v9 09/11] vpci/msi: add MSI handlers

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Mar 15, 2018 at 06:44:13AM -0600, Jan Beulich wrote:
> >>> On 15.03.18 at 12:48, <roger.pau@xxxxxxxxxx> wrote:
> > On Wed, Mar 14, 2018 at 10:51:07AM -0600, Jan Beulich wrote:
> >> >>> On 14.03.18 at 15:04, <roger.pau@xxxxxxxxxx> wrote:
> >> > --- a/xen/drivers/vpci/vpci.c
> >> > +++ b/xen/drivers/vpci/vpci.c
> >> > @@ -47,6 +47,10 @@ void vpci_remove_device(struct pci_dev *pdev)
> >> >          xfree(r);
> >> >      }
> >> >      spin_unlock(&pdev->vpci->lock);
> >> > +#ifdef __XEN__
> >> > +    /* NB: fields below are not exposed to the user-space test harness. 
> >> > */
> >> > +    xfree(pdev->vpci->msi);
> >> > +#endif
> >> 
> >> Would it maybe be better to add such dummy field(s), to avoid the
> >> #ifdef here? Anyway, with or without that
> >> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > 
> > For the msi structure that's doable, but for msix it's more complex
> > because it includes the vpci_arch_msix_entry structure, and that would
> > mean exposing more stuff to the user-space test harness.
> 
> I don't understand: These are pointers, hence it suffices to
> use the structures here without actually defining their members
> anywhere; you don't even need to declare them as an empty
> structure. You'd just need to make sure that the test tool fills
> the pointer fields with NULL (so that free()ing them is a no-op).

The issue is that the structure definition and the field declaration
are done at the same time:

struct vpci {
    ...
#ifdef __XEN__
    struct vpci_msi {
        ...
    } *msi;

    struct vpci_msix {
        ...
    } *msix;
#endif
}

Would you like me to add a #else with void * declarations for both msi
and msix?

I could define the structures separately inside of __XEN__ guards, but
I don't see that approach better than the one used here.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel