[PATCH v4 22/24] nvme: bump controller pci device id

Klaus Jensen posted 24 patches 6 years, 1 month ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Max Reitz <mreitz@redhat.com>, Fam Zheng <fam@euphon.net>, Keith Busch <keith.busch@intel.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
[PATCH v4 22/24] nvme: bump controller pci device id
Posted by Klaus Jensen 6 years, 1 month ago
Since commits 9d6459d21a6e ("nvme: fix write zeroes offset and count")
and c7fe50bcf1f1 ("nvme: support multiple namespaces") the controller
device no longer has the quirks that the Linux kernel think it has.

As the quirks are applied based on pci vendor and device id, bump the
device id to get rid of them.

Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/block/nvme.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index e1bf9bf3798b..68c433415169 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -2480,7 +2480,7 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
     pci_conf[PCI_INTERRUPT_PIN] = 1;
     pci_config_set_prog_interface(pci_conf, 0x2);
     pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
-    pci_config_set_device_id(pci_conf, 0x5845);
+    pci_config_set_device_id(pci_conf, 0x5846);
     pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
     pcie_endpoint_cap_init(pci_dev, 0x80);
 
@@ -2651,7 +2651,7 @@ static void nvme_class_init(ObjectClass *oc, void *data)
     pc->exit = nvme_exit;
     pc->class_id = PCI_CLASS_STORAGE_EXPRESS;
     pc->vendor_id = PCI_VENDOR_ID_INTEL;
-    pc->device_id = 0x5845;
+    pc->device_id = 0x5846;
     pc->revision = 2;
 
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-- 
2.24.1


Re: [PATCH v4 22/24] nvme: bump controller pci device id
Posted by Keith Busch 6 years, 1 month ago
On Thu, Dec 19, 2019 at 02:09:19PM +0100, Klaus Jensen wrote:
> @@ -2480,7 +2480,7 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
>      pci_conf[PCI_INTERRUPT_PIN] = 1;
>      pci_config_set_prog_interface(pci_conf, 0x2);
>      pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
> -    pci_config_set_device_id(pci_conf, 0x5845);
> +    pci_config_set_device_id(pci_conf, 0x5846);
>      pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
>      pcie_endpoint_cap_init(pci_dev, 0x80);

We can't just pick a number here, these are supposed to be assigned by the
vendor. A day will come when I will be in trouble for using the existing
identifier: I found out to late it was supposed to be for internal use
only as it was never officially reserved, so lets not make the same
mistake for some future device.

Re: [PATCH v4 22/24] nvme: bump controller pci device id
Posted by Klaus Birkelund Jensen 6 years, 1 month ago
On Dec 20 01:16, Keith Busch wrote:
> On Thu, Dec 19, 2019 at 02:09:19PM +0100, Klaus Jensen wrote:
> > @@ -2480,7 +2480,7 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
> >      pci_conf[PCI_INTERRUPT_PIN] = 1;
> >      pci_config_set_prog_interface(pci_conf, 0x2);
> >      pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
> > -    pci_config_set_device_id(pci_conf, 0x5845);
> > +    pci_config_set_device_id(pci_conf, 0x5846);
> >      pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
> >      pcie_endpoint_cap_init(pci_dev, 0x80);
> 
> We can't just pick a number here, these are supposed to be assigned by the
> vendor. A day will come when I will be in trouble for using the existing
> identifier: I found out to late it was supposed to be for internal use
> only as it was never officially reserved, so lets not make the same
> mistake for some future device.
> 

Makes sense. And there is no "QEMU" vendor, is there? But it would be
really nice to get rid of the quirks.

Re: [PATCH v4 22/24] nvme: bump controller pci device id
Posted by Keith Busch 6 years, 1 month ago
On Thu, Dec 19, 2019 at 06:24:57PM +0100, Klaus Birkelund Jensen wrote:
> On Dec 20 01:16, Keith Busch wrote:
> > On Thu, Dec 19, 2019 at 02:09:19PM +0100, Klaus Jensen wrote:
> > > @@ -2480,7 +2480,7 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
> > >      pci_conf[PCI_INTERRUPT_PIN] = 1;
> > >      pci_config_set_prog_interface(pci_conf, 0x2);
> > >      pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
> > > -    pci_config_set_device_id(pci_conf, 0x5845);
> > > +    pci_config_set_device_id(pci_conf, 0x5846);
> > >      pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
> > >      pcie_endpoint_cap_init(pci_dev, 0x80);
> > 
> > We can't just pick a number here, these are supposed to be assigned by the
> > vendor. A day will come when I will be in trouble for using the existing
> > identifier: I found out to late it was supposed to be for internal use
> > only as it was never officially reserved, so lets not make the same
> > mistake for some future device.
> > 
> 
> Makes sense. And there is no "QEMU" vendor, is there?

I'm not sure if we can use this, but there is a PCI_VENDOR_ID_QEMU,
0x1234, defined in include/hw/pci/pci.h.

> But it would be really nice to get rid of the quirks.

Since you're bumping the nvme version, Maybe we can change the quirk to
include the nvme version number

Re: [PATCH v4 22/24] nvme: bump controller pci device id
Posted by Klaus Birkelund Jensen 6 years, 1 month ago
On Dec 20 02:46, Keith Busch wrote:
> On Thu, Dec 19, 2019 at 06:24:57PM +0100, Klaus Birkelund Jensen wrote:
> > On Dec 20 01:16, Keith Busch wrote:
> > > On Thu, Dec 19, 2019 at 02:09:19PM +0100, Klaus Jensen wrote:
> > > > @@ -2480,7 +2480,7 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
> > > >      pci_conf[PCI_INTERRUPT_PIN] = 1;
> > > >      pci_config_set_prog_interface(pci_conf, 0x2);
> > > >      pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
> > > > -    pci_config_set_device_id(pci_conf, 0x5845);
> > > > +    pci_config_set_device_id(pci_conf, 0x5846);
> > > >      pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
> > > >      pcie_endpoint_cap_init(pci_dev, 0x80);
> > > 
> > > We can't just pick a number here, these are supposed to be assigned by the
> > > vendor. A day will come when I will be in trouble for using the existing
> > > identifier: I found out to late it was supposed to be for internal use
> > > only as it was never officially reserved, so lets not make the same
> > > mistake for some future device.
> > > 
> > 
> > Makes sense. And there is no "QEMU" vendor, is there?
> 
> I'm not sure if we can use this, but there is a PCI_VENDOR_ID_QEMU,
> 0x1234, defined in include/hw/pci/pci.h.
> 

Maybe it's possible to use PCI_VENDOR_ID_REDHAT?