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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.03.18 at 16:54, <roger.pau@xxxxxxxxxx> wrote:
> 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?

That (but instead of using void with the actual struct <name>), or e.g.

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

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



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