Re: [PATCH v5 24/26] nvme: change controller pci id
On Tue, 2020-02-04 at 10:52 +0100, Klaus Jensen wrote:
> 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 machines 4.2
> and older.
>
> Signed-off-by: Klaus Jensen
> ---
> hw/block/nvme.c | 13 +
> hw/block/nvme.h | 4 +++-
> hw/core/machine.c | 1 +
> 3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 3a377bc56734..bdef53a590b0 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -2467,8 +2467,15 @@ 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);
> +
> +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);
> +} 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);
>
> @@ -2638,8 +2645,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 c3cef0f024da..6b584f53ed64 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -12,7 +12,8 @@
> DEFINE_PROP_UINT32("num_queues", _state, _props.num_queues, 64), \
> DEFINE_PROP_UINT8("aerl", _state, _props.aerl, 3), \
> DEFINE_PROP_UINT32("aer_max_queued", _state, _props.aer_max_queued, 64),
> \
> -DEFINE_PROP_UINT8("mdts", _state, _props.mdts, 7)
> +DEFINE_PROP_UINT8("mdts", _state, _props.mdts, 7), \
> +DEFINE_PROP_BOOL("x-use-intel-id", _state, _props.use_intel_id, false)
>
> typedef struct NvmeParams {
> char *serial;
> @@ -21,6 +22,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 3e288bfceb7f..984412d98c9d 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -34,6 +34,7 @@ GlobalProperty hw_compat_4_2[] = {
> { "vhost-blk-device", "seg_max_adjust", "off"},
> { "usb-host", "suppress-remote-wake", "off" },
> { "usb-redir", "suppress-remote-wake", "off" },
> +{ "nvme", "x-use-intel-id", "on"},
> };
> const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2);
>
Reviewed-by: Maxim Levitsky
Best regards,
Maxim Levitsky
Re: [PATCH v5 24/26] nvme: change controller pci id
On Feb 5 01:35, Keith Busch wrote: > On Tue, Feb 04, 2020 at 10:52:06AM +0100, Klaus Jensen wrote: > > 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 machines 4.2 > > and older. > > > > Signed-off-by: Klaus Jensen > > Yay, thank you for following through on getting this identifier assigned. > > Reviewed-by: Keith Busch This is technically not "officially" sanctioned yet, but I got an indication from Gerd that we are good to proceed with this.
Re: [PATCH v5 24/26] nvme: change controller pci id
On Tue, Feb 04, 2020 at 10:52:06AM +0100, Klaus Jensen wrote: > 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 machines 4.2 > and older. > > Signed-off-by: Klaus Jensen Yay, thank you for following through on getting this identifier assigned. Reviewed-by: Keith Busch
[PATCH v5 24/26] nvme: change controller pci id
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 machines 4.2
and older.
Signed-off-by: Klaus Jensen
---
hw/block/nvme.c | 13 +
hw/block/nvme.h | 4 +++-
hw/core/machine.c | 1 +
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 3a377bc56734..bdef53a590b0 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -2467,8 +2467,15 @@ 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);
+
+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);
+} 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);
@@ -2638,8 +2645,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 c3cef0f024da..6b584f53ed64 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -12,7 +12,8 @@
DEFINE_PROP_UINT32("num_queues", _state, _props.num_queues, 64), \
DEFINE_PROP_UINT8("aerl", _state, _props.aerl, 3), \
DEFINE_PROP_UINT32("aer_max_queued", _state, _props.aer_max_queued, 64), \
-DEFINE_PROP_UINT8("mdts", _state, _props.mdts, 7)
+DEFINE_PROP_UINT8("mdts", _state, _props.mdts, 7), \
+DEFINE_PROP_BOOL("x-use-intel-id", _state, _props.use_intel_id, false)
typedef struct NvmeParams {
char *serial;
@@ -21,6 +22,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 3e288bfceb7f..984412d98c9d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -34,6 +34,7 @@ GlobalProperty hw_compat_4_2[] = {
{ "vhost-blk-device", "seg_max_adjust", "off"},
{ "usb-host", "suppress-remote-wake", "off" },
{ "usb-redir", "suppress-remote-wake", "off" },
+{ "nvme", "x-use-intel-id", "on"},
};
const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2);
--
2.25.0
