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

["Maurizio Lombardi" <mlombard@bsdbackstore.eu>]

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,
>   };
>