Re: [Xen-devel] [PATCH v6 11/11] vpci/msix: add MSI-X handlers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.10.17 at 17:04, <roger.pau@xxxxxxxxxx> wrote:
> On Wed, Oct 04, 2017 at 08:34:43AM +0000, Jan Beulich wrote:
>> >>> On 19.09.17 at 17:29, <roger.pau@xxxxxxxxxx> wrote:
>> > --- a/xen/include/xen/vpci.h
>> > +++ b/xen/include/xen/vpci.h
>> > @@ -100,6 +100,40 @@ struct vpci {
>> >          /* 64-bit address capable? */
>> >          bool address64;
>> >      } *msi;
>> > +
>> > +    /* MSI-X data. */
>> > +    struct vpci_msix {
>> > +        struct pci_dev *pdev;
>> > +        /* List link. */
>> > +        struct list_head next;
>> > +        /* Table information. */
>> > +        struct vpci_msix_mem {
>> > +            /* MSI-X table offset. */
>> > +            unsigned int offset;
>> > +            /* MSI-X table BIR. */
>> > +            unsigned int bir;
>> > +            /* Table size. */
>> > +            unsigned int size;
>> > +#define VPCI_MSIX_TABLE     0
>> > +#define VPCI_MSIX_PBA       1
>> > +#define VPCI_MSIX_MEM_NUM   2
>> > +        } mem[VPCI_MSIX_MEM_NUM];
>> > +        /* Maximum number of vectors supported by the device. */
>> > +        unsigned int max_entries;
>> > +        /* MSI-X enabled? */
>> > +        bool enabled;
>> > +        /* Masked? */
>> > +        bool masked;
>> > +        /* Entries. */
>> > +        struct vpci_msix_entry {
>> > +            uint64_t addr;
>> > +            uint32_t data;
>> > +            unsigned int nr;
>> > +            struct vpci_arch_msix_entry arch;
>> > +            bool masked;
>> > +            bool updated;
>> > +        } entries[];
>> > +    } *msix;
>> 
>> Same remark as for MSI regarding optimizing structure size.
> 
> Going over the fields, bir can be turned into a uint8_t, and size into
> a uint16_t. max_entries can also be converted to a uint16_t together
> with nr.
> 
> Apart from that I don't see much more optimization, unless we start
> packaging fields (ie: offset and bir could reside in a uint32_t), but
> IMHO that's going to make the code harder to parse for little gain,
> and will involve more calculations in the handlers.

The more instances of a structure there may be, the more
relevant it is to pack them tightly. I.e. primary focus needs
to be on struct vpci_msix_entry, but since - as indicated -
there may be many devices supporting MSI-X, struct vpci_msix
as a whole should be reasonably well packed as well. I don't
think more calculation in the handlers is an argument - the
compiler will do it for you, and the affected code shouldn't
really be performance critical (it's involved in setting up
interrupts, not delivering them).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel