[PATCH 0/1] nvme: Export CSTS register via sysfs

[PATCH 0/1] nvme: Export CSTS register via sysfs

[Alan Adamson]

This patch exports the NVMe Controller CSTS register via sysfs.  This
feature can be used by userland executables that accessed CSTS and
possibly other registers by mapping them into user space.  Since this ability
may not always available, exporting certain registers via sysfs provides
a safe/read-only way to access registers from outside the kernel.

Testing:

# Controller in the RDY state
# cat /sys/devices/pci0000:00/0000:00:04.0/nvme/nvme0/csts
1

# Controller in the FAILED state
# cat /sys/devices/pci0000:00/0000:00:04.0/nvme/nvme0/csts
2

# A virtualized csts register can be accessed for fabric devices 
# cat /sys/devices/virtual/nvme-fabrics/ctl/nvme1/csts
1

Alan Adamson (1):
  Export CSTS register via sysfs

 drivers/nvme/host/core.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

-- 
2.18.4


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH 1/1] nvme: Export CSTS register via sysfs

[Alan Adamson]

This exposes a sysfs method a user can invoke to request read-only
access to a NVMe Controller's CSTS (Controller Status) Register.

Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
---
 drivers/nvme/host/core.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e68a8c4ac5a6..7736b8b0891c 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3683,9 +3683,24 @@ static ssize_t nvme_ctrl_reconnect_delay_store(struct device *dev,
 	ctrl->opts->reconnect_delay = v;
 	return count;
 }
+
+static ssize_t nvme_ctrl_csts_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
+	u32 csts;
+
+	ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts);
+
+	return sprintf(buf, "%d\n", csts);
+}
+
 static DEVICE_ATTR(reconnect_delay, S_IRUGO | S_IWUSR,
 	nvme_ctrl_reconnect_delay_show, nvme_ctrl_reconnect_delay_store);
 
+static DEVICE_ATTR(csts, S_IRUGO | S_IWUSR,
+	nvme_ctrl_csts_show, NULL);
+
 static struct attribute *nvme_dev_attrs[] = {
 	&dev_attr_reset_controller.attr,
 	&dev_attr_rescan_controller.attr,
@@ -3705,6 +3720,7 @@ static struct attribute *nvme_dev_attrs[] = {
 	&dev_attr_hostid.attr,
 	&dev_attr_ctrl_loss_tmo.attr,
 	&dev_attr_reconnect_delay.attr,
+	&dev_attr_csts.attr,
 	NULL
 };
 
-- 
2.18.4


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Chaitanya Kulkarni]

On 3/17/21 14:05, Alan Adamson wrote:
> This patch exports the NVMe Controller CSTS register via sysfs.  This
> feature can be used by userland executables that accessed CSTS and
> possibly other registers by mapping them into user space.  Since this ability
> may not always available, exporting certain registers via sysfs provides
> a safe/read-only way to access registers from outside the kernel.
>
> Testing:
>
> # Controller in the RDY state
> # cat /sys/devices/pci0000:00/0000:00:04.0/nvme/nvme0/csts
> 1
>
> # Controller in the FAILED state
> # cat /sys/devices/pci0000:00/0000:00:04.0/nvme/nvme0/csts
> 2
>
> # A virtualized csts register can be accessed for fabric devices 
> # cat /sys/devices/virtual/nvme-fabrics/ctl/nvme1/csts
> 1
>
> Alan Adamson (1):
>   Export CSTS register via sysfs
>
>  drivers/nvme/host/core.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>

This patch looks good the usecase mentioned, but I'd really like to
see blktests for this since we are exporting it to userspace.



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Christoph Hellwig]

On Wed, Mar 17, 2021 at 04:46:14PM -0400, Alan Adamson wrote:
> This patch exports the NVMe Controller CSTS register via sysfs.  This
> feature can be used by userland executables that accessed CSTS and
> possibly other registers by mapping them into user space.  Since this ability
> may not always available, exporting certain registers via sysfs provides
> a safe/read-only way to access registers from outside the kernel.

So what is this application doings with it?  Should we just have a
ready attribute instead of exporting a raw register encoding?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Keith Busch]

On Thu, Mar 18, 2021 at 05:38:28AM +0100, Christoph Hellwig wrote:
> On Wed, Mar 17, 2021 at 04:46:14PM -0400, Alan Adamson wrote:
> > This patch exports the NVMe Controller CSTS register via sysfs.  This
> > feature can be used by userland executables that accessed CSTS and
> > possibly other registers by mapping them into user space.  Since this ability
> > may not always available, exporting certain registers via sysfs provides
> > a safe/read-only way to access registers from outside the kernel.
> 
> So what is this application doings with it?  Should we just have a
> ready attribute instead of exporting a raw register encoding?

Isn't that captured by the existing 'state' sysfs attribute?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Alan Adamson]

> On Mar 17, 2021, at 9:38 PM, Christoph Hellwig <hch@lst.de> wrote:
> 
> On Wed, Mar 17, 2021 at 04:46:14PM -0400, Alan Adamson wrote:
>> This patch exports the NVMe Controller CSTS register via sysfs.  This
>> feature can be used by userland executables that accessed CSTS and
>> possibly other registers by mapping them into user space.  Since this ability
>> may not always available, exporting certain registers via sysfs provides
>> a safe/read-only way to access registers from outside the kernel.
> 
> So what is this application doings with it?  Should we just have a
> ready attribute instead of exporting a raw register encoding?


Was using nvme-cli show-regs to manage nvme devices. This is no longer an option.

Alan




_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Keith Busch]

On Thu, Mar 18, 2021 at 04:28:24PM +0000, Alan Adamson wrote:
> 
> 
> > On Mar 17, 2021, at 9:38 PM, Christoph Hellwig <hch@lst.de> wrote:
> > 
> > On Wed, Mar 17, 2021 at 04:46:14PM -0400, Alan Adamson wrote:
> >> This patch exports the NVMe Controller CSTS register via sysfs.  This
> >> feature can be used by userland executables that accessed CSTS and
> >> possibly other registers by mapping them into user space.  Since this ability
> >> may not always available, exporting certain registers via sysfs provides
> >> a safe/read-only way to access registers from outside the kernel.
> > 
> > So what is this application doings with it?  Should we just have a
> > ready attribute instead of exporting a raw register encoding?
> 
> 
> Was using nvme-cli show-regs to manage nvme devices. This is no longer an option.

Most distributions ship with kernel CONFIG_IO_STRICT_DEVMEM these days,
so that user command will very rarely work on PCIe targets anymore.

Perhaps we could introduce a driver option allowing read-only mmap on
this memory?  While you're currently asking for just one register
attribute, it would be nice if we can make all future requests available
without piling on more sysfs properties.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Alan Adamson]

> On Mar 18, 2021, at 9:52 AM, Keith Busch <kbusch@kernel.org> wrote:
> 
> On Thu, Mar 18, 2021 at 04:28:24PM +0000, Alan Adamson wrote:
>> 
>> 
>>> On Mar 17, 2021, at 9:38 PM, Christoph Hellwig <hch@lst.de> wrote:
>>> 
>>> On Wed, Mar 17, 2021 at 04:46:14PM -0400, Alan Adamson wrote:
>>>> This patch exports the NVMe Controller CSTS register via sysfs.  This
>>>> feature can be used by userland executables that accessed CSTS and
>>>> possibly other registers by mapping them into user space.  Since this ability
>>>> may not always available, exporting certain registers via sysfs provides
>>>> a safe/read-only way to access registers from outside the kernel.
>>> 
>>> So what is this application doings with it?  Should we just have a
>>> ready attribute instead of exporting a raw register encoding?
>> 
>> 
>> Was using nvme-cli show-regs to manage nvme devices. This is no longer an option.
> 
> Most distributions ship with kernel CONFIG_IO_STRICT_DEVMEM these days,
> so that user command will very rarely work on PCIe targets anymore.
> 
> Perhaps we could introduce a driver option allowing read-only mmap on
> this memory?  While you're currently asking for just one register
> attribute, it would be nice if we can make all future requests available
> without piling on more sysfs properties.

This could be a good solution (along with a nvme-cli change).  I’ll code this up.

Thanks,

Alan

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Chaitanya Kulkarni]

On 3/18/21 09:59, Keith Busch wrote:
>>> So what is this application doings with it?  Should we just have a
>>> ready attribute instead of exporting a raw register encoding?
>> Was using nvme-cli show-regs to manage nvme devices. This is no longer an option.
> Most distributions ship with kernel CONFIG_IO_STRICT_DEVMEM these days,
> so that user command will very rarely work on PCIe targets anymore.
>
> Perhaps we could introduce a driver option allowing read-only mmap on
> this memory?  While you're currently asking for just one register
> attribute, it would be nice if we can make all future requests available
> without piling on more sysfs properties.

This will also avoid further code changes seems very generic to me.



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Keith Busch]

On Thu, Mar 18, 2021 at 06:39:37PM +0000, Alan Adamson wrote:
> 
> 
> > On Mar 18, 2021, at 9:52 AM, Keith Busch <kbusch@kernel.org> wrote:
> > 
> > On Thu, Mar 18, 2021 at 04:28:24PM +0000, Alan Adamson wrote:
> >> 
> >> 
> >>> On Mar 17, 2021, at 9:38 PM, Christoph Hellwig <hch@lst.de> wrote:
> >>> 
> >>> On Wed, Mar 17, 2021 at 04:46:14PM -0400, Alan Adamson wrote:
> >>>> This patch exports the NVMe Controller CSTS register via sysfs.  This
> >>>> feature can be used by userland executables that accessed CSTS and
> >>>> possibly other registers by mapping them into user space.  Since this ability
> >>>> may not always available, exporting certain registers via sysfs provides
> >>>> a safe/read-only way to access registers from outside the kernel.
> >>> 
> >>> So what is this application doings with it?  Should we just have a
> >>> ready attribute instead of exporting a raw register encoding?
> >> 
> >> 
> >> Was using nvme-cli show-regs to manage nvme devices. This is no longer an option.
> > 
> > Most distributions ship with kernel CONFIG_IO_STRICT_DEVMEM these days,
> > so that user command will very rarely work on PCIe targets anymore.
> > 
> > Perhaps we could introduce a driver option allowing read-only mmap on
> > this memory?  While you're currently asking for just one register
> > attribute, it would be nice if we can make all future requests available
> > without piling on more sysfs properties.
> 
> This could be a good solution (along with a nvme-cli change).  I’ll code this up.

I don't think it should require user space modifications. Just mmap the
current resource as-is but let the driver allow read-only in the
precense of CONFIG_IO_STRICT_DEVMEM. The below is a rough idea of what I
had in mind (compile tested only, and a proper solution should restrict
access to a specific address range)

---
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index d47bb18b976a..c70965135276 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2362,6 +2362,7 @@ static int nvme_pci_enable(struct nvme_dev *dev)
 		return result;
 
 	pci_set_master(pdev);
+	pdev->allow_ro_mmap = true;
 
 	if (dev->ctrl.quirks & NVME_QUIRK_DMA_ADDRESS_BITS_48)
 		dma_address_bits = 48;
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index f8afd54ca3e1..b92f2172497c 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1028,7 +1028,10 @@ static int pci_mmap_resource(struct kobject *kobj, struct bin_attribute *attr,
 	if (ret)
 		return ret;
 
-	if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
+	if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start) &&
+	    !(pdev->allow_ro_mmap &&
+			!((vma->vm_flags & VM_SHARED) ||
+			  (vma->vm_flags & VM_MAYWRITE))))
 		return -EINVAL;
 
 	if (!pci_mmap_fits(pdev, bar, vma, PCI_MMAP_SYSFS))
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 86c799c97b77..ca1598fa46d5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -379,6 +379,7 @@ struct pci_dev {
 						      user sysfs */
 	unsigned int	clear_retrain_link:1;	/* Need to clear Retrain Link
 						   bit manually */
+	unsigned int	allow_ro_mmap;	/* allow read only even if CONFIG_IO_STRICT_DEVMEM */
 	unsigned int	d3hot_delay;	/* D3hot->D0 transition time in ms */
 	unsigned int	d3cold_delay;	/* D3cold->D0 transition time in ms */
 
--

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Christoph Hellwig]

On Thu, Mar 18, 2021 at 12:46:53PM -0700, Keith Busch wrote:
> I don't think it should require user space modifications. Just mmap the
> current resource as-is but let the driver allow read-only in the
> precense of CONFIG_IO_STRICT_DEVMEM. The below is a rough idea of what I
> had in mind (compile tested only, and a proper solution should restrict
> access to a specific address range)

I think this is a horrible idea.  Userspace has no business touching
registers even read-only.  MMIO reads can have side effects as well,
intentional or unintentional, and we also open up a whole can of worms
of mismatched memory attributes.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Keith Busch]

On Fri, Mar 19, 2021 at 07:51:58AM +0100, Christoph Hellwig wrote:
> On Thu, Mar 18, 2021 at 12:46:53PM -0700, Keith Busch wrote:
> > I don't think it should require user space modifications. Just mmap the
> > current resource as-is but let the driver allow read-only in the
> > precense of CONFIG_IO_STRICT_DEVMEM. The below is a rough idea of what I
> > had in mind (compile tested only, and a proper solution should restrict
> > access to a specific address range)
> 
> I think this is a horrible idea.  Userspace has no business touching
> registers even read-only.  MMIO reads can have side effects as well,
> intentional or unintentional, and we also open up a whole can of worms
> of mismatched memory attributes.

I was thinking the driver wouldn't opt-in if there were read side
effects, but yeah, it's too fragile. I withdraw the suggestion.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Christoph Hellwig]

On Sat, Mar 20, 2021 at 12:22:08AM +0900, Keith Busch wrote:
> > I think this is a horrible idea.  Userspace has no business touching
> > registers even read-only.  MMIO reads can have side effects as well,
> > intentional or unintentional, and we also open up a whole can of worms
> > of mismatched memory attributes.
> 
> I was thinking the driver wouldn't opt-in if there were read side
> effects, but yeah, it's too fragile. I withdraw the suggestion.

I'd still like to understand what values in CSTS Alan cares about.  I
don't think just dumping a register with somewhat awkward encodings
is a good idea.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Alan Adamson]

> On Mar 19, 2021, at 8:30 AM, Christoph Hellwig <hch@lst.de> wrote:
> 
> On Sat, Mar 20, 2021 at 12:22:08AM +0900, Keith Busch wrote:
>>> I think this is a horrible idea.  Userspace has no business touching
>>> registers even read-only.  MMIO reads can have side effects as well,
>>> intentional or unintentional, and we also open up a whole can of worms
>>> of mismatched memory attributes.
>> 
>> I was thinking the driver wouldn't opt-in if there were read side
>> effects, but yeah, it's too fragile. I withdraw the suggestion.
> 
> I'd still like to understand what values in CSTS Alan cares about.  I
> don't think just dumping a register with somewhat awkward encodings
> is a good idea.

Primarily Ready and Failed. I’m reaching out to the requesting team to see if the ’state’ attribute is sufficient.

Back to nvme-cli show-regs, do we just expect this to always fail now our should we be returning good
values?

Alan

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Keith Busch]

On Fri, Mar 19, 2021 at 05:21:22PM +0000, Alan Adamson wrote:
> 
> 
> > On Mar 19, 2021, at 8:30 AM, Christoph Hellwig <hch@lst.de> wrote:
> > 
> > On Sat, Mar 20, 2021 at 12:22:08AM +0900, Keith Busch wrote:
> >>> I think this is a horrible idea.  Userspace has no business touching
> >>> registers even read-only.  MMIO reads can have side effects as well,
> >>> intentional or unintentional, and we also open up a whole can of worms
> >>> of mismatched memory attributes.
> >> 
> >> I was thinking the driver wouldn't opt-in if there were read side
> >> effects, but yeah, it's too fragile. I withdraw the suggestion.
> > 
> > I'd still like to understand what values in CSTS Alan cares about.  I
> > don't think just dumping a register with somewhat awkward encodings
> > is a good idea.
> 
> Primarily Ready and Failed. I’m reaching out to the requesting team to see if the ’state’ attribute is sufficient.
> 
> Back to nvme-cli show-regs, do we just expect this to always fail now our should we be returning good
> values?

For PCIe, that shell command returns good values only if the kernel
wasn't compiled with CONFIG_IO_STRICT_DEVMEM. If the kernel was compiled
with that option (most are), user space will not be able to access the
values.

Fabrics should always work, though, because we retrieve CSTS through an
admin command rather than memory mapped IO.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Alan Adamson]

> On Mar 19, 2021, at 10:30 AM, Keith Busch <kbusch@kernel.org> wrote:
> 
> On Fri, Mar 19, 2021 at 05:21:22PM +0000, Alan Adamson wrote:
>> 
>> 
>>> On Mar 19, 2021, at 8:30 AM, Christoph Hellwig <hch@lst.de> wrote:
>>> 
>>> On Sat, Mar 20, 2021 at 12:22:08AM +0900, Keith Busch wrote:
>>>>> I think this is a horrible idea.  Userspace has no business touching
>>>>> registers even read-only.  MMIO reads can have side effects as well,
>>>>> intentional or unintentional, and we also open up a whole can of worms
>>>>> of mismatched memory attributes.
>>>> 
>>>> I was thinking the driver wouldn't opt-in if there were read side
>>>> effects, but yeah, it's too fragile. I withdraw the suggestion.
>>> 
>>> I'd still like to understand what values in CSTS Alan cares about.  I
>>> don't think just dumping a register with somewhat awkward encodings
>>> is a good idea.
>> 
>> Primarily Ready and Failed. I’m reaching out to the requesting team to see if the ’state’ attribute is sufficient.
>> 
>> Back to nvme-cli show-regs, do we just expect this to always fail now our should we be returning good
>> values?
> 
> For PCIe, that shell command returns good values only if the kernel
> wasn't compiled with CONFIG_IO_STRICT_DEVMEM. If the kernel was compiled
> with that option (most are), user space will not be able to access the
> values.

I guess the question is: should we get  show-regs to work without having to map in
the registers?

Alan


> 
> Fabrics should always work, though, because we retrieve CSTS through an
> admin command rather than memory mapped IO.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Alan Adamson]

Resending this due to some formatting issues.

Need to revisit this issue again.  In the original case we needed to get access to the csts register. We were able to move to using the ’state’ sysfs attribute.

We are now getting a similar request from our manufacturing team. They are using nvme-cli show-regs to assist debug. The ’state’ attribute is not sufficient and they want to use the production version of the kernel (which enables CONFIG_IO_STRICT_DEVMEM) rather than a special kernel. Ideally, we would want nvme-cli show-regs to work whether or not CONFIG_IO_STRICT_DEVMEM is configured. Other than exporting the registers thru sysfs attributes, is there any other mechanism to get show-regs the data it needs?

Alan 

> On Mar 19, 2021, at 10:30 AM, Keith Busch <kbusch@kernel.org> wrote:
> 
> On Fri, Mar 19, 2021 at 05:21:22PM +0000, Alan Adamson wrote:
>> 
>> 
>>> On Mar 19, 2021, at 8:30 AM, Christoph Hellwig <hch@lst.de> wrote:
>>> 
>>> On Sat, Mar 20, 2021 at 12:22:08AM +0900, Keith Busch wrote:
>>>>> I think this is a horrible idea.  Userspace has no business touching
>>>>> registers even read-only.  MMIO reads can have side effects as well,
>>>>> intentional or unintentional, and we also open up a whole can of worms
>>>>> of mismatched memory attributes.
>>>> 
>>>> I was thinking the driver wouldn't opt-in if there were read side
>>>> effects, but yeah, it's too fragile. I withdraw the suggestion.
>>> 
>>> I'd still like to understand what values in CSTS Alan cares about.  I
>>> don't think just dumping a register with somewhat awkward encodings
>>> is a good idea.
>> 
>> Primarily Ready and Failed. I’m reaching out to the requesting team to see if the ’state’ attribute is sufficient.
>> 
>> Back to nvme-cli show-regs, do we just expect this to always fail now our should we be returning good
>> values?
> 
> For PCIe, that shell command returns good values only if the kernel
> wasn't compiled with CONFIG_IO_STRICT_DEVMEM. If the kernel was compiled
> with that option (most are), user space will not be able to access the
> values.
> 
> Fabrics should always work, though, because we retrieve CSTS through an
> admin command rather than memory mapped IO.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Keith Busch]

On Wed, May 05, 2021 at 06:40:54PM +0000, Alan Adamson wrote:
> Need to revisit this issue again.  In the original case we needed to
> get access to the csts register. We were able to move to using the
> ’state’ sysfs attribute.
> 
> We are now getting a similar request from our manufacturing team. They
> are using nvme-cli show-regs to assist debug. The ’state’ attribute is
> not sufficient and they want to use the production version of the
> kernel (which enables CONFIG_IO_STRICT_DEVMEM) rather than a special
> kernel. Ideally, we would want nvme-cli show-regs to work whether or
> not CONFIG_IO_STRICT_DEVMEM is configured. Other than exporting the
> registers thru sysfs attributes, is there any other mechanism to get
> show-regs the data it needs?

Append kernel parameter "iomem=relaxed".

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Alan Adamson]

> On May 5, 2021, at 1:11 PM, Keith Busch <kbusch@kernel.org> wrote:
> 
> On Wed, May 05, 2021 at 06:40:54PM +0000, Alan Adamson wrote:
>> Need to revisit this issue again.  In the original case we needed to
>> get access to the csts register. We were able to move to using the
>> ’state’ sysfs attribute.
>> 
>> We are now getting a similar request from our manufacturing team. They
>> are using nvme-cli show-regs to assist debug. The ’state’ attribute is
>> not sufficient and they want to use the production version of the
>> kernel (which enables CONFIG_IO_STRICT_DEVMEM) rather than a special
>> kernel. Ideally, we would want nvme-cli show-regs to work whether or
>> not CONFIG_IO_STRICT_DEVMEM is configured. Other than exporting the
>> registers thru sysfs attributes, is there any other mechanism to get
>> show-regs the data it needs?
> 
> Append kernel parameter "iomem=relaxed”.

That requires the OS disk to be modified.  We would rather not do that.

Alan



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 0/1] nvme: Export CSTS register via sysfs

[Keith Busch]

On Wed, May 05, 2021 at 08:23:43PM +0000, Alan Adamson wrote:
> 
> 
> > On May 5, 2021, at 1:11 PM, Keith Busch <kbusch@kernel.org> wrote:
> > 
> > On Wed, May 05, 2021 at 06:40:54PM +0000, Alan Adamson wrote:
> >> Need to revisit this issue again.  In the original case we needed to
> >> get access to the csts register. We were able to move to using the
> >> ’state’ sysfs attribute.
> >> 
> >> We are now getting a similar request from our manufacturing team. They
> >> are using nvme-cli show-regs to assist debug. The ’state’ attribute is
> >> not sufficient and they want to use the production version of the
> >> kernel (which enables CONFIG_IO_STRICT_DEVMEM) rather than a special
> >> kernel. Ideally, we would want nvme-cli show-regs to work whether or
> >> not CONFIG_IO_STRICT_DEVMEM is configured. Other than exporting the
> >> registers thru sysfs attributes, is there any other mechanism to get
> >> show-regs the data it needs?
> > 
> > Append kernel parameter "iomem=relaxed”.
> 
> That requires the OS disk to be modified.  We would rather not do that.

Oh, that seems like a really good idea to me for a manufacturing
enviroment. There really is no other option other than modify the
kernel, which sounds much more invasive.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme