| Message ID | 512E1D54.5040607@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Feb 27, 2013 at 04:51:00PM +0200, Gal Hammer wrote: > On 27/02/2013 16:03, Michael S. Tsirkin wrote: > >On Wed, Feb 27, 2013 at 03:46:25PM +0200, Gal Hammer wrote: > >>On 27/02/2013 15:28, Paolo Bonzini wrote: > >>>Il 27/02/2013 14:24, Gal Hammer ha scritto: > >>>>On 27/02/2013 14:35, Amit Shah wrote: > >>>>>On (Mon) 04 Feb 2013 [11:59:02], Michael S. Tsirkin wrote: > >>>>>>On Mon, Feb 04, 2013 at 10:42:32AM +0100, Paolo Bonzini wrote: > >>>>>>>Il 03/02/2013 13:11, Michael S. Tsirkin ha scritto: > >>>>>>>>>> static Property virtio_serial_properties[] = { > >>>>>>>>>> DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, > >>>>>>>>>>VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > >>>>>>>>>>- DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, > >>>>>>>>>>DEV_NVECTORS_UNSPECIFIED), > >>>>>>>>>>+ DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), > >>>>>>>>>> DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), > >>>>>>>>>> DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > >>>>>>>>>> DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, > >>>>>>>>>>serial.max_virtserial_ports, 31), > >>>>>>>>>> > >>>>>>>>>>plus the backwards-compatibility stuff. > >>>>>>>>>> > >>>>>>>>>>Paolo > >>>>>>>>Makes sense, but the logic in virtio_serial_init_pci is > >>>>>>>>then dead code and should go away. > >>>>>>>> > >>>>>>> > >>>>>>>It won't be dead code for the backwards-compatible machine types (that > >>>>>>>use DEV_NVECTORS_UNSPECIFIED). > >>>>>>> > >>>>>>>Paolo > >>>>>> > >>>>>>Good point. Ack. Want to post this with proper signature etc? > >>>>> > >>>>>Gal, do you want to submit a patch for this? > >>>>> > >>>>> Amit > >>>>> > >>>> > >>>>Attached. > >>> > >>>You need to add backwards-compatibility properties in hw/pc.h. > >>> > >>>Paolo > >>> > >> > >>I've added the BC property. I hope it is in the right format. > >> > >>Thanks, > >> > >> Gal. > >> > > > >>>From db49a36ecc4de5744415357691140fb68df4fae2 Mon Sep 17 00:00:00 2001 > >>From: Gal Hammer <ghammer@redhat.com> > >>Date: Wed, 27 Feb 2013 15:15:31 +0200 > >>Subject: [PATCH] Set virtio-serial device to have a default of 2 MSI vectors. > >> > >>The virtio-serial device is expected to use 2 MSI vectors, one for > >>control queue and a second shared for all queues. > >> > >>Signed-off-by: Gal Hammer <ghammer@redhat.com> > >>--- > >> hw/pc.h | 4 ++++ > >> hw/virtio-pci.c | 5 ++++- > >> 2 files changed, 8 insertions(+), 1 deletion(-) > >> > >>diff --git a/hw/pc.h b/hw/pc.h > >>index da1b102..d63d411 100644 > >>--- a/hw/pc.h > >>+++ b/hw/pc.h > >>@@ -216,6 +216,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > >> .driver = "virtio-blk-pci",\ > >> .property = "discard_granularity",\ > >> .value = stringify(0),\ > >>+ },{\ > >>+ .driver = "virtio-serial-pci",\ > >>+ .property = "vectors",\ > >>+ .value = stringify(DEV_NVECTORS_UNSPECIFIED),\ > >> } > >> > >> #endif > >>diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > >>index a869f53..ba56ab2 100644 > >>--- a/hw/virtio-pci.c > >>+++ b/hw/virtio-pci.c > >>@@ -975,6 +975,9 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev) > >> if (!vdev) { > >> return -1; > >> } > >>+ > >>+ /* backwards-compatibility with machines that were created with > >>+ DEV_NVECTORS_UNSPECIFIED */ > >> vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED > >> ? proxy->serial.max_virtserial_ports + 1 > >> : proxy->nvectors; > >>@@ -1155,7 +1158,7 @@ static const TypeInfo virtio_net_info = { > >> > >> static Property virtio_serial_properties[] = { > >> DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > >>- DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED), > >>+ DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), > >> DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), > >> DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > >> DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, serial.max_virtserial_ports, 31), > >>-- > >>1.8.1.2 > >> > > > > > >Looks good but just making sure - you did test with compat machine type, > >yes? > > > > After a small fix (attached), I've executed: > > $ x86_64-softmmu/qemu-system-x86_64 -device virtio-serial-pci > vectors = 2 > > $ x86_64-softmmu/qemu-system-x86_64 -device virtio-serial-pci -M pc-0.14 > vectors = 32 > > Is this test is enough? > > Gal. > You'll want to test 1.4 I guess? > >From fddf3136cc2cd0372c52a09424715b15c6ad131f Mon Sep 17 00:00:00 2001 > From: Gal Hammer <ghammer@redhat.com> > Date: Wed, 27 Feb 2013 15:15:31 +0200 > Subject: [PATCH] Set virtio-serial device to have a default of 2 MSI vectors. > > The virtio-serial device is expected to use 2 MSI vectors, one for > control queue and a second shared for all queues. > > Signed-off-by: Gal Hammer <ghammer@redhat.com> > --- > hw/pc.h | 5 +++++ > hw/virtio-pci.c | 5 ++++- > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/hw/pc.h b/hw/pc.h > index da1b102..f2c1b1c 100644 > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -216,6 +216,11 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > .driver = "virtio-blk-pci",\ > .property = "discard_granularity",\ > .value = stringify(0),\ > + },{\ > + .driver = "virtio-serial-pci",\ > + .property = "vectors",\ > + /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\ > + .value = stringify(0xFFFFFFFF),\ > } > > #endif > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index a869f53..ba56ab2 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -975,6 +975,9 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev) > if (!vdev) { > return -1; > } > + > + /* backwards-compatibility with machines that were created with > + DEV_NVECTORS_UNSPECIFIED */ > vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED > ? proxy->serial.max_virtserial_ports + 1 > : proxy->nvectors; > @@ -1155,7 +1158,7 @@ static const TypeInfo virtio_net_info = { > > static Property virtio_serial_properties[] = { > DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > - DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED), > + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), > DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), > DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, serial.max_virtserial_ports, 31), > -- > 1.8.1.2 >
> >From fddf3136cc2cd0372c52a09424715b15c6ad131f Mon Sep 17 00:00:00 2001 > From: Gal Hammer <ghammer@redhat.com> > Date: Wed, 27 Feb 2013 15:15:31 +0200 > Subject: [PATCH] Set virtio-serial device to have a default of 2 MSI vectors. > > The virtio-serial device is expected to use 2 MSI vectors, one for > control queue and a second shared for all queues. > > Signed-off-by: Gal Hammer <ghammer@redhat.com> Thanks, applied. > --- > hw/pc.h | 5 +++++ > hw/virtio-pci.c | 5 ++++- > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/hw/pc.h b/hw/pc.h > index da1b102..f2c1b1c 100644 > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -216,6 +216,11 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > .driver = "virtio-blk-pci",\ > .property = "discard_granularity",\ > .value = stringify(0),\ > + },{\ > + .driver = "virtio-serial-pci",\ > + .property = "vectors",\ > + /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\ > + .value = stringify(0xFFFFFFFF),\ > } > > #endif > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index a869f53..ba56ab2 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -975,6 +975,9 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev) > if (!vdev) { > return -1; > } > + > + /* backwards-compatibility with machines that were created with > + DEV_NVECTORS_UNSPECIFIED */ > vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED > ? proxy->serial.max_virtserial_ports + 1 > : proxy->nvectors; > @@ -1155,7 +1158,7 @@ static const TypeInfo virtio_net_info = { > > static Property virtio_serial_properties[] = { > DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > - DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED), > + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), > DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), > DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, serial.max_virtserial_ports, 31), > -- > 1.8.1.2 >
diff --git a/hw/pc.h b/hw/pc.h index da1b102..f2c1b1c 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -216,6 +216,11 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); .driver = "virtio-blk-pci",\ .property = "discard_granularity",\ .value = stringify(0),\ + },{\ + .driver = "virtio-serial-pci",\ + .property = "vectors",\ + /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\ + .value = stringify(0xFFFFFFFF),\ } #endif diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index a869f53..ba56ab2 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -975,6 +975,9 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev) if (!vdev) { return -1; } + + /* backwards-compatibility with machines that were created with + DEV_NVECTORS_UNSPECIFIED */ vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED ? proxy->serial.max_virtserial_ports + 1 : proxy->nvectors; @@ -1155,7 +1158,7 @@ static const TypeInfo virtio_net_info = { static Property virtio_serial_properties[] = { DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), - DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED), + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, serial.max_virtserial_ports, 31),