[PATCH] nvme-pci: expose active quirks in sysfs

[PATCH] nvme-pci: expose active quirks in sysfs

[Maurizio Lombardi]

Currently, there is no straightforward way for a user to inspect
the quirks value from userspace.
Add a new read-only sysfs attribute "quirks";
reading this file will display the hexadecimal
value of the ctrl->quirks bitmask for the given NVMe device.

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/nvme/host/pci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c916176bd9f0..de9febd32e83 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2497,6 +2497,15 @@ static ssize_t cmbsz_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(cmbsz);
 
+static ssize_t quirks_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct nvme_dev *ndev = to_nvme_dev(dev_get_drvdata(dev));
+
+	return sysfs_emit(buf, "0x%08lx\n", ndev->ctrl.quirks);
+}
+static DEVICE_ATTR_RO(quirks);
+
 static ssize_t hmb_show(struct device *dev, struct device_attribute *attr,
 			char *buf)
 {
@@ -2557,6 +2566,7 @@ static struct attribute *nvme_pci_attrs[] = {
 	&dev_attr_cmbloc.attr,
 	&dev_attr_cmbsz.attr,
 	&dev_attr_hmb.attr,
+	&dev_attr_quirks.attr,
 	NULL,
 };
 
-- 
2.47.3

Re: [PATCH] nvme-pci: expose active quirks in sysfs

[Bart Van Assche]

On 10/28/25 7:29 AM, Maurizio Lombardi wrote:
> Currently, there is no straightforward way for a user to inspect
> the quirks value from userspace.
> Add a new read-only sysfs attribute "quirks";
> reading this file will display the hexadecimal
> value of the ctrl->quirks bitmask for the given NVMe device.

This patch changes the constants in enum nvme_quirks from
kernel-internal constants into an ABI. I'm not sure that's what we
want.

Thanks,

Bart.

Re: [PATCH] nvme-pci: expose active quirks in sysfs

[Maurizio Lombardi]

On Tue Oct 28, 2025 at 5:32 PM CET, Bart Van Assche wrote:
> On 10/28/25 7:29 AM, Maurizio Lombardi wrote:
>> Currently, there is no straightforward way for a user to inspect
>> the quirks value from userspace.
>> Add a new read-only sysfs attribute "quirks";
>> reading this file will display the hexadecimal
>> value of the ctrl->quirks bitmask for the given NVMe device.
>
> This patch changes the constants in enum nvme_quirks from
> kernel-internal constants into an ABI. I'm not sure that's what we
> want.

I understand your concern.
I could respin it to export the names of the active quirks
rather than the raw bitmask, if there is an interest in having
this feature.

Maurizio

Re: [PATCH] nvme-pci: expose active quirks in sysfs

[Chaitanya Kulkarni]

On 10/28/25 10:00, Maurizio Lombardi wrote:
> On Tue Oct 28, 2025 at 5:32 PM CET, Bart Van Assche wrote:
>> On 10/28/25 7:29 AM, Maurizio Lombardi wrote:
>>> Currently, there is no straightforward way for a user to inspect
>>> the quirks value from userspace.
>>> Add a new read-only sysfs attribute "quirks";
>>> reading this file will display the hexadecimal
>>> value of the ctrl->quirks bitmask for the given NVMe device.
>> This patch changes the constants in enum nvme_quirks from
>> kernel-internal constants into an ABI. I'm not sure that's what we
>> want.
> I understand your concern.
> I could respin it to export the names of the active quirks
> rather than the raw bitmask, if there is an interest in having
> this feature.
>
> Maurizio
>
question here: do we need to care about some kind of security before we 
expose controller information that is internal to the driver ? 
CAP_SYS_ADMIN ? -ck

Re: [PATCH] nvme-pci: expose active quirks in sysfs

[Maurizio Lombardi]

On Wed Oct 29, 2025 at 6:47 AM CET, Chaitanya Kulkarni wrote:
> On 10/28/25 10:00, Maurizio Lombardi wrote:
>> On Tue Oct 28, 2025 at 5:32 PM CET, Bart Van Assche wrote:
>>> On 10/28/25 7:29 AM, Maurizio Lombardi wrote:
>>>> Currently, there is no straightforward way for a user to inspect
>>>> the quirks value from userspace.
>>>> Add a new read-only sysfs attribute "quirks";
>>>> reading this file will display the hexadecimal
>>>> value of the ctrl->quirks bitmask for the given NVMe device.
>>> This patch changes the constants in enum nvme_quirks from
>>> kernel-internal constants into an ABI. I'm not sure that's what we
>>> want.
>> I understand your concern.
>> I could respin it to export the names of the active quirks
>> rather than the raw bitmask, if there is an interest in having
>> this feature.
>>
>> Maurizio
>>
> question here: do we need to care about some kind of security before we 
> expose controller information that is internal to the driver ? 
> CAP_SYS_ADMIN ? -ck

IMO I don't think it's an information that needs to be hidden.
After all, any user could run "lscpi -nn", get the vendor and device
id and cross-reference them with the pci table in the source code
to get the enabled quirks, this sysfs entry would just make it faster.

Maurizio

Re: [PATCH] nvme-pci: expose active quirks in sysfs

[Chaitanya Kulkarni]

On 10/29/25 01:02, Maurizio Lombardi wrote:
> On Wed Oct 29, 2025 at 6:47 AM CET, Chaitanya Kulkarni wrote:
>> On 10/28/25 10:00, Maurizio Lombardi wrote:
>>> On Tue Oct 28, 2025 at 5:32 PM CET, Bart Van Assche wrote:
>>>> On 10/28/25 7:29 AM, Maurizio Lombardi wrote:
>>>>> Currently, there is no straightforward way for a user to inspect
>>>>> the quirks value from userspace.
>>>>> Add a new read-only sysfs attribute "quirks";
>>>>> reading this file will display the hexadecimal
>>>>> value of the ctrl->quirks bitmask for the given NVMe device.
>>>> This patch changes the constants in enum nvme_quirks from
>>>> kernel-internal constants into an ABI. I'm not sure that's what we
>>>> want.
>>> I understand your concern.
>>> I could respin it to export the names of the active quirks
>>> rather than the raw bitmask, if there is an interest in having
>>> this feature.
>>>
>>> Maurizio
>>>
>> question here: do we need to care about some kind of security before we
>> expose controller information that is internal to the driver ?
>> CAP_SYS_ADMIN ? -ck
> IMO I don't think it's an information that needs to be hidden.
> After all, any user could run "lscpi -nn", get the vendor and device
> id and cross-reference them with the pci table in the source code
> to get the enabled quirks, this sysfs entry would just make it faster.
>
> Maurizio

Yes it is definitely useful after only decoding :-

linux-block (for-next) # cat /sys/class/nvme/nvme0/quirks
0x0000000000040000

Active quirks:
   BOGUS_NID
linux-block (for-next) #


diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c916176bd9f0..1b9ad818b12d 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2497,6 +2497,80 @@ static ssize_t cmbsz_show(struct device *dev, struct device_attribute *attr,
  }
  static DEVICE_ATTR_RO(cmbsz);
  
+static int nvme_get_quirks_string(unsigned long quirks, char *buf, size_t buf_len)
+{
+	size_t len = 0;
+
+	if (!quirks) {
+		len = scnprintf(buf, buf_len, "No quirks enabled\n");
+		return len;
+	}
+
+	len += scnprintf(buf + len, buf_len - len, "Active quirks:\n");
+
+	if (quirks & NVME_QUIRK_STRIPE_SIZE)
+		len += scnprintf(buf + len, buf_len - len, "  STRIPE_SIZE\n");
+	if (quirks & NVME_QUIRK_IDENTIFY_CNS)
+		len += scnprintf(buf + len, buf_len - len, "  IDENTIFY_CNS\n");
+	if (quirks & NVME_QUIRK_DEALLOCATE_ZEROES)
+		len += scnprintf(buf + len, buf_len - len, "  DEALLOCATE_ZEROES\n");
+	if (quirks & NVME_QUIRK_DELAY_BEFORE_CHK_RDY)
+		len += scnprintf(buf + len, buf_len - len, "  DELAY_BEFORE_CHK_RDY\n");
+	if (quirks & NVME_QUIRK_NO_APST)
+		len += scnprintf(buf + len, buf_len - len, "  NO_APST\n");
+	if (quirks & NVME_QUIRK_NO_DEEPEST_PS)
+		len += scnprintf(buf + len, buf_len - len, "  NO_DEEPEST_PS\n");
+	if (quirks & NVME_QUIRK_QDEPTH_ONE)
+		len += scnprintf(buf + len, buf_len - len, "  QDEPTH_ONE\n");
+	if (quirks & NVME_QUIRK_MEDIUM_PRIO_SQ)
+		len += scnprintf(buf + len, buf_len - len, "  MEDIUM_PRIO_SQ\n");
+	if (quirks & NVME_QUIRK_IGNORE_DEV_SUBNQN)
+		len += scnprintf(buf + len, buf_len - len, "  IGNORE_DEV_SUBNQN\n");
+	if (quirks & NVME_QUIRK_DISABLE_WRITE_ZEROES)
+		len += scnprintf(buf + len, buf_len - len, "  DISABLE_WRITE_ZEROES\n");
+	if (quirks & NVME_QUIRK_SIMPLE_SUSPEND)
+		len += scnprintf(buf + len, buf_len - len, "  SIMPLE_SUSPEND\n");
+	if (quirks & NVME_QUIRK_SINGLE_VECTOR)
+		len += scnprintf(buf + len, buf_len - len, "  SINGLE_VECTOR\n");
+	if (quirks & NVME_QUIRK_128_BYTES_SQES)
+		len += scnprintf(buf + len, buf_len - len, "  128_BYTES_SQES\n");
+	if (quirks & NVME_QUIRK_SHARED_TAGS)
+		len += scnprintf(buf + len, buf_len - len, "  SHARED_TAGS\n");
+	if (quirks & NVME_QUIRK_NO_TEMP_THRESH_CHANGE)
+		len += scnprintf(buf + len, buf_len - len, "  NO_TEMP_THRESH_CHANGE\n");
+	if (quirks & NVME_QUIRK_NO_NS_DESC_LIST)
+		len += scnprintf(buf + len, buf_len - len, "  NO_NS_DESC_LIST\n");
+	if (quirks & NVME_QUIRK_DMA_ADDRESS_BITS_48)
+		len += scnprintf(buf + len, buf_len - len, "  DMA_ADDRESS_BITS_48\n");
+	if (quirks & NVME_QUIRK_SKIP_CID_GEN)
+		len += scnprintf(buf + len, buf_len - len, "  SKIP_CID_GEN\n");
+	if (quirks & NVME_QUIRK_BOGUS_NID)
+		len += scnprintf(buf + len, buf_len - len, "  BOGUS_NID\n");
+	if (quirks & NVME_QUIRK_NO_SECONDARY_TEMP_THRESH)
+		len += scnprintf(buf + len, buf_len - len, "  NO_SECONDARY_TEMP_THRESH\n");
+	if (quirks & NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND)
+		len += scnprintf(buf + len, buf_len - len, "  FORCE_NO_SIMPLE_SUSPEND\n");
+	if (quirks & NVME_QUIRK_BROKEN_MSI)
+		len += scnprintf(buf + len, buf_len - len, "  BROKEN_MSI\n");
+	if (quirks & NVME_QUIRK_DMAPOOL_ALIGN_512)
+		len += scnprintf(buf + len, buf_len - len, "  DMAPOOL_ALIGN_512\n");
+
+	return len;
+}
+
+static ssize_t quirks_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct nvme_dev *ndev = to_nvme_dev(dev_get_drvdata(dev));
+	int len;
+
+	len = sysfs_emit(buf, "0x%016lx\n\n", ndev->ctrl.quirks);
+	len += nvme_get_quirks_string(ndev->ctrl.quirks, buf + len, PAGE_SIZE - len);
+
+	return len;
+}
+static DEVICE_ATTR_RO(quirks);
+
  static ssize_t hmb_show(struct device *dev, struct device_attribute *attr,
  			char *buf)
  {
@@ -2557,6 +2631,7 @@ static struct attribute *nvme_pci_attrs[] = {
  	&dev_attr_cmbloc.attr,
  	&dev_attr_cmbsz.attr,
  	&dev_attr_hmb.attr,
+	&dev_attr_quirks.attr,
  	NULL,
  };
  
-- 
2.40.0

Re: [PATCH] nvme-pci: expose active quirks in sysfs

[Maurizio Lombardi]

On Thu Oct 30, 2025 at 12:25 AM CET, Chaitanya Kulkarni wrote:
> On 10/29/25 01:02, Maurizio Lombardi wrote:
>> On Wed Oct 29, 2025 at 6:47 AM CET, Chaitanya Kulkarni wrote:
>>> On 10/28/25 10:00, Maurizio Lombardi wrote:
>>>> On Tue Oct 28, 2025 at 5:32 PM CET, Bart Van Assche wrote:
>>>>> On 10/28/25 7:29 AM, Maurizio Lombardi wrote:
>>>>>> Currently, there is no straightforward way for a user to inspect
>>>>>> the quirks value from userspace.
>>>>>> Add a new read-only sysfs attribute "quirks";
>>>>>> reading this file will display the hexadecimal
>>>>>> value of the ctrl->quirks bitmask for the given NVMe device.
>>>>> This patch changes the constants in enum nvme_quirks from
>>>>> kernel-internal constants into an ABI. I'm not sure that's what we
>>>>> want.
>>>> I understand your concern.
>>>> I could respin it to export the names of the active quirks
>>>> rather than the raw bitmask, if there is an interest in having
>>>> this feature.
>>>>
>>>> Maurizio
>>>>
>>> question here: do we need to care about some kind of security before we
>>> expose controller information that is internal to the driver ?
>>> CAP_SYS_ADMIN ? -ck
>> IMO I don't think it's an information that needs to be hidden.
>> After all, any user could run "lscpi -nn", get the vendor and device
>> id and cross-reference them with the pci table in the source code
>> to get the enabled quirks, this sysfs entry would just make it faster.
>>
>> Maurizio
>
> Yes it is definitely useful after only decoding :-
>
> linux-block (for-next) # cat /sys/class/nvme/nvme0/quirks
> 0x0000000000040000
>
> Active quirks:
>    BOGUS_NID
> linux-block (for-next) #

Ah you anticipated me, I was preparing a somewhat similar patch.

************************

Currently, there is no straightforward way for a user to inspect
which quirks are active for a given device from userspace.

Add a new "quirks" sysfs attribute to the nvme controller device.

Reading this file will display a human-readable list
of all active quirks, with each quirk name on a new line.
If no quirks are active, it will display "none".

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---

V2: Do not expose kernel-internal constants to userspace.
    Move the quirks attribute to the controller device, where it belongs.

 drivers/nvme/host/nvme.h  | 54 +++++++++++++++++++++++++++++++++++++++
 drivers/nvme/host/sysfs.c | 23 +++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 102fae6a231c..647f19dd57e9 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -180,6 +180,60 @@ enum nvme_quirks {
 	NVME_QUIRK_DMAPOOL_ALIGN_512		= (1 << 22),
 };

+static inline char *nvme_quirk_name(enum nvme_quirks q)
+{
+	switch (q) {
+	case NVME_QUIRK_STRIPE_SIZE:
+		return "stripe_size";
+	case NVME_QUIRK_IDENTIFY_CNS:
+		return "identify_cns";
+	case NVME_QUIRK_DEALLOCATE_ZEROES:
+		return "deallocate_zeroes";
+	case NVME_QUIRK_DELAY_BEFORE_CHK_RDY:
+		return "delay_before_chk_rdy";
+	case NVME_QUIRK_NO_APST:
+		return "no_apst";
+	case NVME_QUIRK_NO_DEEPEST_PS:
+		return "no_deepest_ps";
+	case NVME_QUIRK_QDEPTH_ONE:
+		return "qdepth_one";
+	case NVME_QUIRK_MEDIUM_PRIO_SQ:
+		return "medium_prio_sq";
+	case NVME_QUIRK_IGNORE_DEV_SUBNQN:
+		return "ignore_dev_subnqn";
+	case NVME_QUIRK_DISABLE_WRITE_ZEROES:
+		return "disable_write_zeroes";
+	case NVME_QUIRK_SIMPLE_SUSPEND:
+		return "simple_suspend";
+	case NVME_QUIRK_SINGLE_VECTOR:
+		return "single_vector";
+	case NVME_QUIRK_128_BYTES_SQES:
+		return "128_bytes_sqes";
+	case NVME_QUIRK_SHARED_TAGS:
+		return "shared_tags";
+	case NVME_QUIRK_NO_TEMP_THRESH_CHANGE:
+		return "no_temp_thresh_change";
+	case NVME_QUIRK_NO_NS_DESC_LIST:
+		return "no_ns_desc_list";
+	case NVME_QUIRK_DMA_ADDRESS_BITS_48:
+		return "dma_address_bits_48";
+	case NVME_QUIRK_SKIP_CID_GEN:
+		return "skip_cid_gen";
+	case NVME_QUIRK_BOGUS_NID:
+		return "bogus_nid";
+	case NVME_QUIRK_NO_SECONDARY_TEMP_THRESH:
+		return "no_secondary_temp_thresh";
+	case NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND:
+		return "force_no_simple_suspend";
+	case NVME_QUIRK_BROKEN_MSI:
+		return "broken_msi";
+	case NVME_QUIRK_DMAPOOL_ALIGN_512:
+		return "dmapool_align_512";
+	}
+
+	return "unknown";
+}
+
 /*
  * Common request structure for NVMe passthrough.  All drivers must have
  * this structure as the first member of their request-private data.
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 29430949ce2f..16c6fea4b2db 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -601,6 +601,28 @@ static ssize_t dctype_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(dctype);

+static ssize_t quirks_show(struct device *dev, struct device_attribute *attr,
+                char *buf)
+{
+	int count = 0, i;
+	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
+	unsigned long quirks = ctrl->quirks;
+
+	if (!quirks)
+		return sysfs_emit(buf, "none\n");
+
+	for (i = 0; quirks; ++i) {
+		if (quirks & 1) {
+			count += sysfs_emit_at(buf, count, "%s\n",
+					nvme_quirk_name(BIT(i)));
+		}
+		quirks >>= 1;
+	}
+
+	return count;
+}
+static DEVICE_ATTR_RO(quirks);
+
 #ifdef CONFIG_NVME_HOST_AUTH
 static ssize_t nvme_ctrl_dhchap_secret_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
@@ -742,6 +764,7 @@ static struct attribute *nvme_dev_attrs[] = {
 	&dev_attr_kato.attr,
 	&dev_attr_cntrltype.attr,
 	&dev_attr_dctype.attr,
+	&dev_attr_quirks.attr,
 #ifdef CONFIG_NVME_HOST_AUTH
 	&dev_attr_dhchap_secret.attr,
 	&dev_attr_dhchap_ctrl_secret.attr,
--
2.47.3

>
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index c916176bd9f0..1b9ad818b12d 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2497,6 +2497,80 @@ static ssize_t cmbsz_show(struct device *dev, struct device_attribute *attr,
>   }
>   static DEVICE_ATTR_RO(cmbsz);
>   
> +static int nvme_get_quirks_string(unsigned long quirks, char *buf, size_t buf_len)
> +{
> +	size_t len = 0;
> +
> +	if (!quirks) {
> +		len = scnprintf(buf, buf_len, "No quirks enabled\n");
> +		return len;
> +	}
> +
> +	len += scnprintf(buf + len, buf_len - len, "Active quirks:\n");
> +
> +	if (quirks & NVME_QUIRK_STRIPE_SIZE)
> +		len += scnprintf(buf + len, buf_len - len, "  STRIPE_SIZE\n");
> +	if (quirks & NVME_QUIRK_IDENTIFY_CNS)
> +		len += scnprintf(buf + len, buf_len - len, "  IDENTIFY_CNS\n");
> +	if (quirks & NVME_QUIRK_DEALLOCATE_ZEROES)
> +		len += scnprintf(buf + len, buf_len - len, "  DEALLOCATE_ZEROES\n");
> +	if (quirks & NVME_QUIRK_DELAY_BEFORE_CHK_RDY)
> +		len += scnprintf(buf + len, buf_len - len, "  DELAY_BEFORE_CHK_RDY\n");
> +	if (quirks & NVME_QUIRK_NO_APST)
> +		len += scnprintf(buf + len, buf_len - len, "  NO_APST\n");
> +	if (quirks & NVME_QUIRK_NO_DEEPEST_PS)
> +		len += scnprintf(buf + len, buf_len - len, "  NO_DEEPEST_PS\n");
> +	if (quirks & NVME_QUIRK_QDEPTH_ONE)
> +		len += scnprintf(buf + len, buf_len - len, "  QDEPTH_ONE\n");
> +	if (quirks & NVME_QUIRK_MEDIUM_PRIO_SQ)
> +		len += scnprintf(buf + len, buf_len - len, "  MEDIUM_PRIO_SQ\n");
> +	if (quirks & NVME_QUIRK_IGNORE_DEV_SUBNQN)
> +		len += scnprintf(buf + len, buf_len - len, "  IGNORE_DEV_SUBNQN\n");
> +	if (quirks & NVME_QUIRK_DISABLE_WRITE_ZEROES)
> +		len += scnprintf(buf + len, buf_len - len, "  DISABLE_WRITE_ZEROES\n");
> +	if (quirks & NVME_QUIRK_SIMPLE_SUSPEND)
> +		len += scnprintf(buf + len, buf_len - len, "  SIMPLE_SUSPEND\n");
> +	if (quirks & NVME_QUIRK_SINGLE_VECTOR)
> +		len += scnprintf(buf + len, buf_len - len, "  SINGLE_VECTOR\n");
> +	if (quirks & NVME_QUIRK_128_BYTES_SQES)
> +		len += scnprintf(buf + len, buf_len - len, "  128_BYTES_SQES\n");
> +	if (quirks & NVME_QUIRK_SHARED_TAGS)
> +		len += scnprintf(buf + len, buf_len - len, "  SHARED_TAGS\n");
> +	if (quirks & NVME_QUIRK_NO_TEMP_THRESH_CHANGE)
> +		len += scnprintf(buf + len, buf_len - len, "  NO_TEMP_THRESH_CHANGE\n");
> +	if (quirks & NVME_QUIRK_NO_NS_DESC_LIST)
> +		len += scnprintf(buf + len, buf_len - len, "  NO_NS_DESC_LIST\n");
> +	if (quirks & NVME_QUIRK_DMA_ADDRESS_BITS_48)
> +		len += scnprintf(buf + len, buf_len - len, "  DMA_ADDRESS_BITS_48\n");
> +	if (quirks & NVME_QUIRK_SKIP_CID_GEN)
> +		len += scnprintf(buf + len, buf_len - len, "  SKIP_CID_GEN\n");
> +	if (quirks & NVME_QUIRK_BOGUS_NID)
> +		len += scnprintf(buf + len, buf_len - len, "  BOGUS_NID\n");
> +	if (quirks & NVME_QUIRK_NO_SECONDARY_TEMP_THRESH)
> +		len += scnprintf(buf + len, buf_len - len, "  NO_SECONDARY_TEMP_THRESH\n");
> +	if (quirks & NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND)
> +		len += scnprintf(buf + len, buf_len - len, "  FORCE_NO_SIMPLE_SUSPEND\n");
> +	if (quirks & NVME_QUIRK_BROKEN_MSI)
> +		len += scnprintf(buf + len, buf_len - len, "  BROKEN_MSI\n");
> +	if (quirks & NVME_QUIRK_DMAPOOL_ALIGN_512)
> +		len += scnprintf(buf + len, buf_len - len, "  DMAPOOL_ALIGN_512\n");
> +
> +	return len;
> +}
> +
> +static ssize_t quirks_show(struct device *dev, struct device_attribute *attr,
> +			   char *buf)
> +{
> +	struct nvme_dev *ndev = to_nvme_dev(dev_get_drvdata(dev));
> +	int len;
> +
> +	len = sysfs_emit(buf, "0x%016lx\n\n", ndev->ctrl.quirks);
> +	len += nvme_get_quirks_string(ndev->ctrl.quirks, buf + len, PAGE_SIZE - len);
> +
> +	return len;
> +}
> +static DEVICE_ATTR_RO(quirks);
> +
>   static ssize_t hmb_show(struct device *dev, struct device_attribute *attr,
>   			char *buf)
>   {
> @@ -2557,6 +2631,7 @@ static struct attribute *nvme_pci_attrs[] = {
>   	&dev_attr_cmbloc.attr,
>   	&dev_attr_cmbsz.attr,
>   	&dev_attr_hmb.attr,
> +	&dev_attr_quirks.attr,
>   	NULL,
>   };
>   

Re: [PATCH] nvme-pci: expose active quirks in sysfs

[Chaitanya Kulkarni]

On 10/29/25 23:18, Maurizio Lombardi wrote:
> On Thu Oct 30, 2025 at 12:25 AM CET, Chaitanya Kulkarni wrote:
>> On 10/29/25 01:02, Maurizio Lombardi wrote:
>>> On Wed Oct 29, 2025 at 6:47 AM CET, Chaitanya Kulkarni wrote:
>>>> On 10/28/25 10:00, Maurizio Lombardi wrote:
>>>>> On Tue Oct 28, 2025 at 5:32 PM CET, Bart Van Assche wrote:
>>>>>> On 10/28/25 7:29 AM, Maurizio Lombardi wrote:
>>>>>>> Currently, there is no straightforward way for a user to inspect
>>>>>>> the quirks value from userspace.
>>>>>>> Add a new read-only sysfs attribute "quirks";
>>>>>>> reading this file will display the hexadecimal
>>>>>>> value of the ctrl->quirks bitmask for the given NVMe device.
>>>>>> This patch changes the constants in enum nvme_quirks from
>>>>>> kernel-internal constants into an ABI. I'm not sure that's what we
>>>>>> want.
>>>>> I understand your concern.
>>>>> I could respin it to export the names of the active quirks
>>>>> rather than the raw bitmask, if there is an interest in having
>>>>> this feature.
>>>>>
>>>>> Maurizio
>>>>>
>>>> question here: do we need to care about some kind of security before we
>>>> expose controller information that is internal to the driver ?
>>>> CAP_SYS_ADMIN ? -ck
>>> IMO I don't think it's an information that needs to be hidden.
>>> After all, any user could run "lscpi -nn", get the vendor and device
>>> id and cross-reference them with the pci table in the source code
>>> to get the enabled quirks, this sysfs entry would just make it faster.
>>>
>>> Maurizio
>> Yes it is definitely useful after only decoding :-
>>
>> linux-block (for-next) # cat /sys/class/nvme/nvme0/quirks
>> 0x0000000000040000
>>
>> Active quirks:
>>     BOGUS_NID
>> linux-block (for-next) #
> Ah you anticipated me, I was preparing a somewhat similar patch.
>
> ************************


how about something like this totally untested ?

drivers/nvme/host/pci.c | 64 +++++++++++++++++++++++++++++++++++++++++
  1 file changed, 64 insertions(+)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c916176bd9f0..ea9904e2dbd9 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2497,6 +2497,69 @@ static ssize_t cmbsz_show(struct device *dev, 
struct device_attribute *attr,
  }
  static DEVICE_ATTR_RO(cmbsz);

+static const struct {
+    unsigned long quirk;
+    const char *name;
+} nvme_quirk_info[] = {
+    { NVME_QUIRK_STRIPE_SIZE,        "STRIPE_SIZE" },
+    { NVME_QUIRK_IDENTIFY_CNS,        "IDENTIFY_CNS" },
+    { NVME_QUIRK_DEALLOCATE_ZEROES,        "DEALLOCATE_ZEROES" },
+    { NVME_QUIRK_DELAY_BEFORE_CHK_RDY, "DELAY_BEFORE_CHK_RDY" },
+    { NVME_QUIRK_NO_APST,            "NO_APST" },
+    { NVME_QUIRK_NO_DEEPEST_PS,        "NO_DEEPEST_PS" },
+    { NVME_QUIRK_QDEPTH_ONE,        "QDEPTH_ONE" },
+    { NVME_QUIRK_MEDIUM_PRIO_SQ,        "MEDIUM_PRIO_SQ" },
+    { NVME_QUIRK_IGNORE_DEV_SUBNQN,        "IGNORE_DEV_SUBNQN" },
+    { NVME_QUIRK_DISABLE_WRITE_ZEROES, "DISABLE_WRITE_ZEROES" },
+    { NVME_QUIRK_SIMPLE_SUSPEND,        "SIMPLE_SUSPEND" },
+    { NVME_QUIRK_SINGLE_VECTOR,        "SINGLE_VECTOR" },
+    { NVME_QUIRK_128_BYTES_SQES,        "128_BYTES_SQES" },
+    { NVME_QUIRK_SHARED_TAGS,        "SHARED_TAGS" },
+    { NVME_QUIRK_NO_TEMP_THRESH_CHANGE, "NO_TEMP_THRESH_CHANGE" },
+    { NVME_QUIRK_NO_NS_DESC_LIST,        "NO_NS_DESC_LIST" },
+    { NVME_QUIRK_DMA_ADDRESS_BITS_48,    "DMA_ADDRESS_BITS_48" },
+    { NVME_QUIRK_SKIP_CID_GEN,        "SKIP_CID_GEN" },
+    { NVME_QUIRK_BOGUS_NID,            "BOGUS_NID" },
+    { NVME_QUIRK_NO_SECONDARY_TEMP_THRESH, "NO_SECONDARY_TEMP_THRESH" },
+    { NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND, "FORCE_NO_SIMPLE_SUSPEND" },
+    { NVME_QUIRK_BROKEN_MSI,        "BROKEN_MSI" },
+    { NVME_QUIRK_DMAPOOL_ALIGN_512,        "DMAPOOL_ALIGN_512" },
+};
+
+static int nvme_get_quirks_string(unsigned long quirks, char *buf, 
size_t buf_len)
+{
+    size_t len = 0;
+    int i;
+
+    if (!quirks) {
+        len = scnprintf(buf, buf_len, "No quirks enabled\n");
+        return len;
+    }
+
+    len += scnprintf(buf + len, buf_len - len, "Active quirks:\n");
+
+    for (i = 0; i < ARRAY_SIZE(nvme_quirk_info); i++) {
+        if (quirks & nvme_quirk_info[i].quirk)
+            len += scnprintf(buf + len, buf_len - len, " %s\n",
+                     nvme_quirk_info[i].name);
+    }
+
+    return len;
+}
+
+static ssize_t quirks_show(struct device *dev, struct device_attribute 
*attr,
+               char *buf)
+{
+    struct nvme_dev *ndev = to_nvme_dev(dev_get_drvdata(dev));
+    int len;
+
+    len = sysfs_emit(buf, "0x%016lx\n\n", ndev->ctrl.quirks);
+    len += nvme_get_quirks_string(ndev->ctrl.quirks, buf + len, 
PAGE_SIZE - len);
+
+    return len;
+}
+static DEVICE_ATTR_RO(quirks);
+
  static ssize_t hmb_show(struct device *dev, struct device_attribute *attr,
              char *buf)
  {
@@ -2557,6 +2620,7 @@ static struct attribute *nvme_pci_attrs[] = {
      &dev_attr_cmbloc.attr,
      &dev_attr_cmbsz.attr,
      &dev_attr_hmb.attr,
+    &dev_attr_quirks.attr,
      NULL,
  };

-- 
2.40.0

Re: [PATCH] nvme-pci: expose active quirks in sysfs

[Maurizio Lombardi]

On Thu Oct 30, 2025 at 7:46 AM CET, Chaitanya Kulkarni wrote:
> On 10/29/25 23:18, Maurizio Lombardi wrote:
>
> how about something like this totally untested ?
>
> drivers/nvme/host/pci.c | 64 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 64 insertions(+)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index c916176bd9f0..ea9904e2dbd9 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2497,6 +2497,69 @@ static ssize_t cmbsz_show(struct device *dev, 
> struct device_attribute *attr,
>   }
>   static DEVICE_ATTR_RO(cmbsz);
>
> +static const struct {
> +    unsigned long quirk;
> +    const char *name;
> +} nvme_quirk_info[] = {
> +    { NVME_QUIRK_STRIPE_SIZE,        "STRIPE_SIZE" },
> +    { NVME_QUIRK_IDENTIFY_CNS,        "IDENTIFY_CNS" },
> +    { NVME_QUIRK_DEALLOCATE_ZEROES,        "DEALLOCATE_ZEROES" },
> +    { NVME_QUIRK_DELAY_BEFORE_CHK_RDY, "DELAY_BEFORE_CHK_RDY" },
> +    { NVME_QUIRK_NO_APST,            "NO_APST" },
> +    { NVME_QUIRK_NO_DEEPEST_PS,        "NO_DEEPEST_PS" },
> +    { NVME_QUIRK_QDEPTH_ONE,        "QDEPTH_ONE" },
> +    { NVME_QUIRK_MEDIUM_PRIO_SQ,        "MEDIUM_PRIO_SQ" },
> +    { NVME_QUIRK_IGNORE_DEV_SUBNQN,        "IGNORE_DEV_SUBNQN" },
> +    { NVME_QUIRK_DISABLE_WRITE_ZEROES, "DISABLE_WRITE_ZEROES" },
> +    { NVME_QUIRK_SIMPLE_SUSPEND,        "SIMPLE_SUSPEND" },
> +    { NVME_QUIRK_SINGLE_VECTOR,        "SINGLE_VECTOR" },
> +    { NVME_QUIRK_128_BYTES_SQES,        "128_BYTES_SQES" },
> +    { NVME_QUIRK_SHARED_TAGS,        "SHARED_TAGS" },
> +    { NVME_QUIRK_NO_TEMP_THRESH_CHANGE, "NO_TEMP_THRESH_CHANGE" },
> +    { NVME_QUIRK_NO_NS_DESC_LIST,        "NO_NS_DESC_LIST" },
> +    { NVME_QUIRK_DMA_ADDRESS_BITS_48,    "DMA_ADDRESS_BITS_48" },
> +    { NVME_QUIRK_SKIP_CID_GEN,        "SKIP_CID_GEN" },
> +    { NVME_QUIRK_BOGUS_NID,            "BOGUS_NID" },
> +    { NVME_QUIRK_NO_SECONDARY_TEMP_THRESH, "NO_SECONDARY_TEMP_THRESH" },
> +    { NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND, "FORCE_NO_SIMPLE_SUSPEND" },
> +    { NVME_QUIRK_BROKEN_MSI,        "BROKEN_MSI" },
> +    { NVME_QUIRK_DMAPOOL_ALIGN_512,        "DMAPOOL_ALIGN_512" },
> +};

Hmm, NVME_QUIRK_DMAPOOL_ALIGN_512 is "1 << 22", wouldn't this create 
an enormous array?

Maurizio

> +
> +static int nvme_get_quirks_string(unsigned long quirks, char *buf, 
> size_t buf_len)
> +{
> +    size_t len = 0;
> +    int i;
> +
> +    if (!quirks) {
> +        len = scnprintf(buf, buf_len, "No quirks enabled\n");
> +        return len;
> +    }
> +
> +    len += scnprintf(buf + len, buf_len - len, "Active quirks:\n");
> +
> +    for (i = 0; i < ARRAY_SIZE(nvme_quirk_info); i++) {
> +        if (quirks & nvme_quirk_info[i].quirk)
> +            len += scnprintf(buf + len, buf_len - len, " %s\n",
> +                     nvme_quirk_info[i].name);
> +    }
> +
> +    return len;
> +}
> +
> +static ssize_t quirks_show(struct device *dev, struct device_attribute 
> *attr,
> +               char *buf)
> +{
> +    struct nvme_dev *ndev = to_nvme_dev(dev_get_drvdata(dev));
> +    int len;
> +
> +    len = sysfs_emit(buf, "0x%016lx\n\n", ndev->ctrl.quirks);
> +    len += nvme_get_quirks_string(ndev->ctrl.quirks, buf + len, 
> PAGE_SIZE - len);
> +
> +    return len;
> +}
> +static DEVICE_ATTR_RO(quirks);
> +
>   static ssize_t hmb_show(struct device *dev, struct device_attribute *attr,
>               char *buf)
>   {
> @@ -2557,6 +2620,7 @@ static struct attribute *nvme_pci_attrs[] = {
>       &dev_attr_cmbloc.attr,
>       &dev_attr_cmbsz.attr,
>       &dev_attr_hmb.attr,
> +    &dev_attr_quirks.attr,
>       NULL,
>   };

Re: [PATCH] nvme-pci: expose active quirks in sysfs

[Maurizio Lombardi]

On Thu Oct 30, 2025 at 7:55 AM CET, Maurizio Lombardi wrote:
> On Thu Oct 30, 2025 at 7:46 AM CET, Chaitanya Kulkarni wrote:
>> On 10/29/25 23:18, Maurizio Lombardi wrote:
>>
>> how about something like this totally untested ?
>>
>> drivers/nvme/host/pci.c | 64 +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 64 insertions(+)
>>
>> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
>> index c916176bd9f0..ea9904e2dbd9 100644
>> --- a/drivers/nvme/host/pci.c
>> +++ b/drivers/nvme/host/pci.c
>> @@ -2497,6 +2497,69 @@ static ssize_t cmbsz_show(struct device *dev, 
>> struct device_attribute *attr,
>>   }
>>   static DEVICE_ATTR_RO(cmbsz);
>>
>> +static const struct {
>> +    unsigned long quirk;
>> +    const char *name;
>> +} nvme_quirk_info[] = {
>> +    { NVME_QUIRK_STRIPE_SIZE,        "STRIPE_SIZE" },
>> +    { NVME_QUIRK_IDENTIFY_CNS,        "IDENTIFY_CNS" },
>> +    { NVME_QUIRK_DEALLOCATE_ZEROES,        "DEALLOCATE_ZEROES" },
>> +    { NVME_QUIRK_DELAY_BEFORE_CHK_RDY, "DELAY_BEFORE_CHK_RDY" },
>> +    { NVME_QUIRK_NO_APST,            "NO_APST" },
>> +    { NVME_QUIRK_NO_DEEPEST_PS,        "NO_DEEPEST_PS" },
>> +    { NVME_QUIRK_QDEPTH_ONE,        "QDEPTH_ONE" },
>> +    { NVME_QUIRK_MEDIUM_PRIO_SQ,        "MEDIUM_PRIO_SQ" },
>> +    { NVME_QUIRK_IGNORE_DEV_SUBNQN,        "IGNORE_DEV_SUBNQN" },
>> +    { NVME_QUIRK_DISABLE_WRITE_ZEROES, "DISABLE_WRITE_ZEROES" },
>> +    { NVME_QUIRK_SIMPLE_SUSPEND,        "SIMPLE_SUSPEND" },
>> +    { NVME_QUIRK_SINGLE_VECTOR,        "SINGLE_VECTOR" },
>> +    { NVME_QUIRK_128_BYTES_SQES,        "128_BYTES_SQES" },
>> +    { NVME_QUIRK_SHARED_TAGS,        "SHARED_TAGS" },
>> +    { NVME_QUIRK_NO_TEMP_THRESH_CHANGE, "NO_TEMP_THRESH_CHANGE" },
>> +    { NVME_QUIRK_NO_NS_DESC_LIST,        "NO_NS_DESC_LIST" },
>> +    { NVME_QUIRK_DMA_ADDRESS_BITS_48,    "DMA_ADDRESS_BITS_48" },
>> +    { NVME_QUIRK_SKIP_CID_GEN,        "SKIP_CID_GEN" },
>> +    { NVME_QUIRK_BOGUS_NID,            "BOGUS_NID" },
>> +    { NVME_QUIRK_NO_SECONDARY_TEMP_THRESH, "NO_SECONDARY_TEMP_THRESH" },
>> +    { NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND, "FORCE_NO_SIMPLE_SUSPEND" },
>> +    { NVME_QUIRK_BROKEN_MSI,        "BROKEN_MSI" },
>> +    { NVME_QUIRK_DMAPOOL_ALIGN_512,        "DMAPOOL_ALIGN_512" },
>> +};
>
> Hmm, NVME_QUIRK_DMAPOOL_ALIGN_512 is "1 << 22", wouldn't this create 
> an enormous array?

Sorry, my mistake, this is completely ok.

So yes, it would be fine, the only drawback is that with the switch()
the compiler would raise a warning if someone adds a new quirk and
forgets to add its name to the nvme_quirk_name() function.

>
> Maurizio
>
>> +
>> +static int nvme_get_quirks_string(unsigned long quirks, char *buf, 
>> size_t buf_len)
>> +{
>> +    size_t len = 0;
>> +    int i;
>> +
>> +    if (!quirks) {
>> +        len = scnprintf(buf, buf_len, "No quirks enabled\n");
>> +        return len;
>> +    }
>> +
>> +    len += scnprintf(buf + len, buf_len - len, "Active quirks:\n");
>> +
>> +    for (i = 0; i < ARRAY_SIZE(nvme_quirk_info); i++) {
>> +        if (quirks & nvme_quirk_info[i].quirk)
>> +            len += scnprintf(buf + len, buf_len - len, " %s\n",
>> +                     nvme_quirk_info[i].name);
>> +    }
>> +
>> +    return len;
>> +}
>> +
>> +static ssize_t quirks_show(struct device *dev, struct device_attribute 
>> *attr,
>> +               char *buf)
>> +{
>> +    struct nvme_dev *ndev = to_nvme_dev(dev_get_drvdata(dev));
>> +    int len;
>> +
>> +    len = sysfs_emit(buf, "0x%016lx\n\n", ndev->ctrl.quirks);
>> +    len += nvme_get_quirks_string(ndev->ctrl.quirks, buf + len, 
>> PAGE_SIZE - len);
>> +
>> +    return len;
>> +}
>> +static DEVICE_ATTR_RO(quirks);
>> +
>>   static ssize_t hmb_show(struct device *dev, struct device_attribute *attr,
>>               char *buf)
>>   {
>> @@ -2557,6 +2620,7 @@ static struct attribute *nvme_pci_attrs[] = {
>>       &dev_attr_cmbloc.attr,
>>       &dev_attr_cmbsz.attr,
>>       &dev_attr_hmb.attr,
>> +    &dev_attr_quirks.attr,
>>       NULL,
>>   };