Re: [Qemu-devel] [PATCH V5 18/18] virtio-pci: introduce auto_msix_bar_size property

[Jason Wang <jasowang@redhat.com>]

On Wed, Apr 1, 2015 at 5:20 PM, Michael S. Tsirkin <mst@redhat.com> 
wrote:
> On Wed, Apr 01, 2015 at 04:15:12PM +0800, Jason Wang wrote:
>>  Currently we don't support more than 128 MSI-X vectors for a pci
>>  devices, trying to use vector=129 for a virtio-net-pci device may 
>> get:
>>  
>>  qemu-system-x86_64: -device virtio-net-pci,netdev=hn0,vectors=129:
>>  unable to init msix vectors to 129
> 
> So you want to keep producing this error for
> compat machine types? Given that we only support
> up to 64 queues, why is vector=129 worth supporting
> for these machine types?

I'm ok to ignore this issue. As I replied in the previous mail, we'd 
better fail and quit early in this case. But it looks too late to do 
this now.

> 
> 
> 
>>  This this because the MSI-X bar size were hard-coded as 4096. So 
>> this
>>  patch introduces boolean auto_msix_bar_size property for
>>  virito-pci devices. Enable this will let virtio pci device to let 
>> msix
>>  core to calculate the MSI-X bar size dynamically based on the number
>>  of vectors.
>>  
>>  This is a must to let virtio-net can up to 256 queues and each queue
>>  were associated with a specific MSI-X entry.
>>  
>>  Cc: Paolo Bonzini <pbonzini@redhat.com>
>>  Cc: Richard Henderson <rth@twiddle.net>
>>  Cc: Michael S. Tsirkin <mst@redhat.com>
>>  Cc: Alexander Graf <agraf@suse.de>
>>  Cc: qemu-ppc@nongnu.org
>>  Signed-off-by: Jason Wang <jasowang@redhat.com>
>>  ---
>>   hw/i386/pc_piix.c      |  8 ++++++++
>>   hw/i386/pc_q35.c       |  8 ++++++++
>>   hw/ppc/spapr.c         | 11 ++++++++++-
>>   hw/virtio/virtio-pci.c |  6 +++++-
>>   hw/virtio/virtio-pci.h |  3 +++
>>   include/hw/compat.h    | 11 +++++++++++
>>   6 files changed, 45 insertions(+), 2 deletions(-)
>>  
>>  diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>>  index 6e098ce..5da6ef5 100644
>>  --- a/hw/i386/pc_piix.c
>>  +++ b/hw/i386/pc_piix.c
>>  @@ -548,6 +548,10 @@ static QEMUMachine pc_i440fx_machine_v2_3 = {
>>       PC_I440FX_2_3_MACHINE_OPTIONS,
>>       .name = "pc-i440fx-2.3",
>>       .init = pc_init_pci_2_3,
>>  +    .compat_props = (GlobalProperty[]) {
>>  +        HW_COMPAT_2_3,
>>  +        { /* end of list */ }
>>  +    },
>>   };
>>   
>>   #define PC_I440FX_2_2_MACHINE_OPTIONS PC_I440FX_2_3_MACHINE_OPTIONS
>>  @@ -556,6 +560,10 @@ static QEMUMachine pc_i440fx_machine_v2_2 = {
>>       PC_I440FX_2_2_MACHINE_OPTIONS,
>>       .name = "pc-i440fx-2.2",
>>       .init = pc_init_pci_2_2,
>>  +    .compat_props = (GlobalProperty[]) {
>>  +        HW_COMPAT_2_2,
>>  +        { /* end of list */ }
>>  +    },
>>   };
>>   
>>   #define PC_I440FX_2_1_MACHINE_OPTIONS                           \
>>  diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>>  index ff7c414..e38fda0 100644
>>  --- a/hw/i386/pc_q35.c
>>  +++ b/hw/i386/pc_q35.c
>>  @@ -444,6 +444,10 @@ static QEMUMachine pc_q35_machine_v2_3 = {
>>       PC_Q35_2_3_MACHINE_OPTIONS,
>>       .name = "pc-q35-2.3",
>>       .init = pc_q35_init_2_3,
>>  +    .compat_props = (GlobalProperty[]) {
>>  +        HW_COMPAT_2_3,
>>  +        { /* end of list */ }
>>  +    },
>>   };
>>   
>>   #define PC_Q35_2_2_MACHINE_OPTIONS PC_Q35_2_3_MACHINE_OPTIONS
>>  @@ -452,6 +456,10 @@ static QEMUMachine pc_q35_machine_v2_2 = {
>>       PC_Q35_2_2_MACHINE_OPTIONS,
>>       .name = "pc-q35-2.2",
>>       .init = pc_q35_init_2_2,
>>  +    .compat_props = (GlobalProperty[]) {
>>  +        HW_COMPAT_2_2,
>>  +        { /* end of list */ }
>>  +    },
>>   };
>>   
>>   #define PC_Q35_2_1_MACHINE_OPTIONS                      \
>>  diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>  index ee8f6a3..109c4ff 100644
>>  --- a/hw/ppc/spapr.c
>>  +++ b/hw/ppc/spapr.c
>>  @@ -1816,12 +1816,16 @@ static const TypeInfo spapr_machine_info = {
>>       },
>>   };
>>   
>>  +#define SPAPR_COMPAT_2_3 \
>>  +        HW_COMPAT_2_3
>>  +
>>   #define SPAPR_COMPAT_2_2 \
>>  +        SPAPR_COMPAT_2_3, \
>>           {\
>>               .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,\
>>               .property = "mem_win_size",\
>>               .value    = "0x20000000",\
>>  -        }
>>  +        } \
>>   
>>   #define SPAPR_COMPAT_2_1 \
>>           SPAPR_COMPAT_2_2
>>  @@ -1905,10 +1909,15 @@ static const TypeInfo 
>> spapr_machine_2_2_info = {
>>   
>>   static void spapr_machine_2_3_class_init(ObjectClass *oc, void 
>> *data)
>>   {
>>  +    static GlobalProperty compat_props[] = {
>>  +        SPAPR_COMPAT_2_3,
>>  +        { /* end of list */ }
>>  +    };
>>       MachineClass *mc = MACHINE_CLASS(oc);
>>   
>>       mc->name = "pseries-2.3";
>>       mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
>>  +    mc->compat_props = compat_props;
>>   }
>>   
>>   static const TypeInfo spapr_machine_2_3_info = {
>>  diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>>  index 816a706..9c6082a 100644
>>  --- a/hw/virtio/virtio-pci.c
>>  +++ b/hw/virtio/virtio-pci.c
>>  @@ -924,6 +924,7 @@ static void 
>> virtio_pci_device_plugged(DeviceState *d)
>>   {
>>       VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
>>       VirtioBusState *bus = &proxy->bus;
>>  +    bool legacy_layout = !(proxy->flags & 
>> VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE);
>>       uint8_t *config;
>>       uint32_t size;
>>   
>>  @@ -937,7 +938,8 @@ static void 
>> virtio_pci_device_plugged(DeviceState *d)
>>       config[PCI_INTERRUPT_PIN] = 1;
>>   
>>       if (proxy->nvectors &&
>>  -        msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 
>> 1, true)) {
>>  +        msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 
>> 1,
>>  +                                legacy_layout)) {
>>           error_report("unable to init msix vectors to %" PRIu32,
>>                        proxy->nvectors);
>>           proxy->nvectors = 0;
>>  @@ -1370,6 +1372,8 @@ static const TypeInfo virtio_serial_pci_info 
>> = {
>>   static Property virtio_net_properties[] = {
>>       DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
>>                       VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),
>>  +    DEFINE_PROP_BIT("auto_msix_bar_size", VirtIOPCIProxy, flags,
>>  +                    VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE_BIT, true),
>>       DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
>>       DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features),
>>       DEFINE_PROP_END_OF_LIST(),
>>  diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
>>  index 3bac016..82a6782 100644
>>  --- a/hw/virtio/virtio-pci.h
>>  +++ b/hw/virtio/virtio-pci.h
>>  @@ -62,6 +62,9 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
>>    * vcpu thread using ioeventfd for some devices. */
>>   #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
>>   #define VIRTIO_PCI_FLAG_USE_IOEVENTFD   (1 << 
>> VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT)
>>  +#define VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE_BIT 2
>>  +#define VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE \
>>  +    (1 << VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE_BIT)
>>   
>>   typedef struct {
>>       MSIMessage msg;
>>  diff --git a/include/hw/compat.h b/include/hw/compat.h
>>  index 313682a..3186275 100644
>>  --- a/include/hw/compat.h
>>  +++ b/include/hw/compat.h
>>  @@ -1,7 +1,18 @@
>>   #ifndef HW_COMPAT_H
>>   #define HW_COMPAT_H
>>   
>>  +#define HW_COMPAT_2_3 \
>>  +        {\
>>  +            .driver   = "virtio-net-pci",\
>>  +            .property = "auto_msix_bar_size",\
>>  +            .value    = "off",\
>>  +        }
>>  +
>>  +#define HW_COMPAT_2_2 \
>>  +        HW_COMPAT_2_3
>>  +
>>   #define HW_COMPAT_2_1 \
>>  +        HW_COMPAT_2_2, \
>>           {\
>>               .driver   = "intel-hda",\
>>               .property = "old_msi_addr",\
>>  -- 
>>  2.1.0
>