| Message ID | 1447930344-17625-2-git-send-email-shmulik.ladkani@ravellosystems.com |
|---|---|
| State | New |
| Headers | show |
On 11/19/2015 06:52 PM, Shmulik Ladkani wrote: > Report the 'express endpoint' capability if on a PCIE bus. > > Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com> > --- > hw/net/vmxnet3.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index 5e3a233..ed286cc 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -2233,6 +2233,10 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp) > VMW_WRPRN("Failed to initialize MSI, configuration is inconsistent."); > } > > + if (pci_bus_is_express(pci_dev->bus)) { > + pcie_endpoint_cap_init(pci_dev, 0); > + } > + > vmxnet3_net_init(s); > > register_savevm(dev, "vmxnet3-msix", -1, 1, > @@ -2568,6 +2572,7 @@ static void vmxnet3_class_init(ObjectClass *class, void *data) > c->class_id = PCI_CLASS_NETWORK_ETHERNET; > c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE; > c->subsystem_id = PCI_DEVICE_ID_VMWARE_VMXNET3; > + c->is_express = 1; Should we do this conditionally? And how about the migration compatibility? Looks like pcie device is using vmstate_pcie_device instead of vmstate_pci_device, maybe need a new property bit for this. > > dc->reset = vmxnet3_qdev_reset; > dc->vmsd = &vmstate_vmxnet3;
Thanks Jason, On Wed, 25 Nov 2015 10:38:51 +0800, jasowang@redhat.com wrote: > > @@ -2568,6 +2572,7 @@ static void vmxnet3_class_init(ObjectClass *class, void *data) > > c->class_id = PCI_CLASS_NETWORK_ETHERNET; > > c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE; > > c->subsystem_id = PCI_DEVICE_ID_VMWARE_VMXNET3; > > + c->is_express = 1; > > Should we do this conditionally? And how about the migration > compatibility? Looks like pcie device is using vmstate_pcie_device > instead of vmstate_pci_device, maybe need a new property bit for this. (Responding for the entire series) Agreed. Will limit these changes for new versions. What's your suggested plan? Does it make sense to have a property for each change (as they are not necessarily related), or is it too tedious and one property will suffice? Regards, Shmulik
On 11/25/2015 02:18 PM, Shmulik Ladkani wrote: > Thanks Jason, > > On Wed, 25 Nov 2015 10:38:51 +0800, jasowang@redhat.com wrote: >>> @@ -2568,6 +2572,7 @@ static void vmxnet3_class_init(ObjectClass *class, void *data) >>> c->class_id = PCI_CLASS_NETWORK_ETHERNET; >>> c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE; >>> c->subsystem_id = PCI_DEVICE_ID_VMWARE_VMXNET3; >>> + c->is_express = 1; >> Should we do this conditionally? And how about the migration >> compatibility? Looks like pcie device is using vmstate_pcie_device >> instead of vmstate_pci_device, maybe need a new property bit for this. > (Responding for the entire series) > > Agreed. Will limit these changes for new versions. > > What's your suggested plan? > Does it make sense to have a property for each change (as they are not > necessarily related), or is it too tedious and one property will suffice? Since they are not necessarily related, we'd better use a property for each change. Thanks > > Regards, > Shmulik >
Hi, On Wed, 25 Nov 2015 16:24:39 +0800 Jason Wang <jasowang@redhat.com> wrote: > >>> @@ -2568,6 +2572,7 @@ static void vmxnet3_class_init(ObjectClass *class, void *data) > >>> c->class_id = PCI_CLASS_NETWORK_ETHERNET; > >>> c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE; > >>> c->subsystem_id = PCI_DEVICE_ID_VMWARE_VMXNET3; > >>> + c->is_express = 1; > >> Should we do this conditionally? And how about the migration > >> compatibility? Looks like pcie device is using vmstate_pcie_device > >> instead of vmstate_pci_device, maybe need a new property bit for this. > > (Responding for the entire series) > > > > Agreed. Will limit these changes for new versions. > > > > What's your suggested plan? > > Does it make sense to have a property for each change (as they are not > > necessarily related), or is it too tedious and one property will suffice? > > Since they are not necessarily related, we'd better use a property for > each change. Would it make sense if we expose a new vmxnet3 type to differenciate pcie vs pci instances of vmxnet3? Otherwise, migration gets more complicated, as we need to use either vmstate_pci_device or vmstate_pcie_device; also, upon vm load, we need to preserve the semantics saved (whether the instance was pci or pcie). I have managed to do so, but is a bit tedious; Exposing a new type seems cleaner.
On 11/30/2015 05:07 AM, Shmulik Ladkani wrote: > Hi, > > On Wed, 25 Nov 2015 16:24:39 +0800 Jason Wang <jasowang@redhat.com> wrote: >>>>> @@ -2568,6 +2572,7 @@ static void vmxnet3_class_init(ObjectClass *class, void *data) >>>>> c->class_id = PCI_CLASS_NETWORK_ETHERNET; >>>>> c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE; >>>>> c->subsystem_id = PCI_DEVICE_ID_VMWARE_VMXNET3; >>>>> + c->is_express = 1; >>>> Should we do this conditionally? And how about the migration >>>> compatibility? Looks like pcie device is using vmstate_pcie_device >>>> instead of vmstate_pci_device, maybe need a new property bit for this. >>> (Responding for the entire series) >>> >>> Agreed. Will limit these changes for new versions. >>> >>> What's your suggested plan? >>> Does it make sense to have a property for each change (as they are not >>> necessarily related), or is it too tedious and one property will suffice? >> Since they are not necessarily related, we'd better use a property for >> each change. > Would it make sense if we expose a new vmxnet3 type to differenciate > pcie vs pci instances of vmxnet3? > > Otherwise, migration gets more complicated, as we need to use either > vmstate_pci_device or vmstate_pcie_device; also, upon vm load, we need > to preserve the semantics saved (whether the instance was pci or pcie). > > I have managed to do so, but is a bit tedious; Exposing a new type seems > cleaner. Yes, it's a good idea to have a new type. Thanks
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index 5e3a233..ed286cc 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -2233,6 +2233,10 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp) VMW_WRPRN("Failed to initialize MSI, configuration is inconsistent."); } + if (pci_bus_is_express(pci_dev->bus)) { + pcie_endpoint_cap_init(pci_dev, 0); + } + vmxnet3_net_init(s); register_savevm(dev, "vmxnet3-msix", -1, 1, @@ -2568,6 +2572,7 @@ static void vmxnet3_class_init(ObjectClass *class, void *data) c->class_id = PCI_CLASS_NETWORK_ETHERNET; c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE; c->subsystem_id = PCI_DEVICE_ID_VMWARE_VMXNET3; + c->is_express = 1; dc->desc = "VMWare Paravirtualized Ethernet v3"; dc->reset = vmxnet3_qdev_reset; dc->vmsd = &vmstate_vmxnet3;
Report the 'express endpoint' capability if on a PCIE bus. Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com> --- hw/net/vmxnet3.c | 5 +++++ 1 file changed, 5 insertions(+)