[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 17/17] hw/block/nvme: change controller pci id
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH 17/17] hw/block/nvme: change controller pci id |
Date: |
Mon, 7 Sep 2020 11:37:08 +0100 |
User-agent: |
Mutt/1.14.6 (2020-07-11) |
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> +David in case
>
> On 9/4/20 4:19 PM, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> >
> > There are two reasons for changing this:
> >
> > 1. The nvme device currently uses an internal Intel device id.
> >
> > 2. Since commits "nvme: fix write zeroes offset and count" and "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, change
> > them to get rid of the quirks.
> >
> > To keep backward compatibility, add a new 'x-use-intel-id' parameter to
> > the nvme device to force use of the Intel vendor and device id. This is
> > off by default but add a compat property to set this for 5.1 machines
> > and older.
>
> So now what happens if you start a 5.1 machine with a recent kernel?
> Simply the kernel will use unnecessary quirks, or are there more
> changes in behavior?
Seems reasonable to me...but...
> >
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > Reviewed-by: Keith Busch <kbusch@kernel.org>
> > Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> > hw/block/nvme.c | 12 ++++++++++--
> > hw/block/nvme.h | 1 +
> > hw/core/machine.c | 1 +
> > 3 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index 453d3a89d475..8018f8679366 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -2749,6 +2749,15 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice
> > *pci_dev, Error **errp)
> >
> > pci_conf[PCI_INTERRUPT_PIN] = 1;
> > pci_config_set_prog_interface(pci_conf, 0x2);
> > +
> > + if (n->params.use_intel_id) {
> > + pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
> > + pci_config_set_device_id(pci_conf, 0x5846);
Wasn't that magic number 5845 down there ?
Dave
> > + } else {
> > + pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_REDHAT);
> > + pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_REDHAT_NVME);
> > + }
> > +
> > pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
> > pcie_endpoint_cap_init(pci_dev, 0x80);
> >
> > @@ -2903,6 +2912,7 @@ static Property nvme_props[] = {
> > DEFINE_PROP_UINT8("aerl", NvmeCtrl, params.aerl, 3),
> > DEFINE_PROP_UINT32("aer_max_queued", NvmeCtrl, params.aer_max_queued,
> > 64),
> > DEFINE_PROP_UINT8("mdts", NvmeCtrl, params.mdts, 7),
> > + DEFINE_PROP_BOOL("x-use-intel-id", NvmeCtrl, params.use_intel_id,
> > false),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > @@ -2919,8 +2929,6 @@ static void nvme_class_init(ObjectClass *oc, void
> > *data)
> > pc->realize = nvme_realize;
> > pc->exit = nvme_exit;
> > pc->class_id = PCI_CLASS_STORAGE_EXPRESS;
> > - pc->vendor_id = PCI_VENDOR_ID_INTEL;
> > - pc->device_id = 0x5845;
> > pc->revision = 2;
> >
> > set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> > diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> > index 72260f2e8ea9..a734a5e1370d 100644
> > --- a/hw/block/nvme.h
> > +++ b/hw/block/nvme.h
> > @@ -15,6 +15,7 @@ typedef struct NvmeParams {
> > uint8_t aerl;
> > uint32_t aer_max_queued;
> > uint8_t mdts;
> > + bool use_intel_id;
> > } NvmeParams;
> >
> > typedef struct NvmeAsyncEvent {
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index ea26d612374d..67990232528c 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -34,6 +34,7 @@ GlobalProperty hw_compat_5_1[] = {
> > { "vhost-user-scsi", "num_queues", "1"},
> > { "virtio-blk-device", "num-queues", "1"},
> > { "virtio-scsi-device", "num_queues", "1"},
> > + { "nvme", "x-use-intel-id", "on"},
> > };
> > const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
> >
> >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
- [PATCH 15/17] hw/block/nvme: support multiple namespaces, (continued)
- [PATCH 15/17] hw/block/nvme: support multiple namespaces, Klaus Jensen, 2020/09/04
- [PATCH 14/17] hw/block/nvme: refactor identify active namespace id list, Klaus Jensen, 2020/09/04
- [PATCH 12/17] hw/block/nvme: add support for scatter gather lists, Klaus Jensen, 2020/09/04
- [PATCH 17/17] hw/block/nvme: change controller pci id, Klaus Jensen, 2020/09/04
- Re: [PATCH 17/17] hw/block/nvme: change controller pci id, Philippe Mathieu-Daudé, 2020/09/06
- Re: [PATCH 17/17] hw/block/nvme: change controller pci id, Klaus Jensen, 2020/09/07
- Re: [PATCH 17/17] hw/block/nvme: change controller pci id, Philippe Mathieu-Daudé, 2020/09/07
- Re: [PATCH 17/17] hw/block/nvme: change controller pci id, Klaus Jensen, 2020/09/07
- Re: [PATCH 17/17] hw/block/nvme: change controller pci id, Klaus Jensen, 2020/09/07
- Re: [PATCH 17/17] hw/block/nvme: change controller pci id, Philippe Mathieu-Daudé, 2020/09/07
- Re: [PATCH 17/17] hw/block/nvme: change controller pci id,
Dr. David Alan Gilbert <=
- Re: [PATCH 17/17] hw/block/nvme: change controller pci id, Klaus Jensen, 2020/09/07
- Re: [PATCH 17/17] hw/block/nvme: change controller pci id, Dr. David Alan Gilbert, 2020/09/07
- Re: [PATCH 17/17] hw/block/nvme: change controller pci id, Klaus Jensen, 2020/09/07
- Re: [PATCH 17/17] hw/block/nvme: change controller pci id, Keith Busch, 2020/09/08
Re: [PATCH 00/17] hw/block/nvme: multiple namespaces support, Philippe Mathieu-Daudé, 2020/09/04