[PATCH 2/2] nvme: Expose cntrltype and dctype through sysfs.

[Belanger, Martin]

> On 2/2/22 22:34, Martin Belanger wrote:
> > From: Martin Belanger <martin.belanger at dell.com>
> >
> > TP8010 introduces the Discovery Controller Type (dctype) attribute.
> > The dctype is returned in the response to the Identify command. This
> > patch exposes the dctype through the sysfs. Since the dctype depends
> > on the Controller Type (cntrltype), another attribute of the Identify
> > response, the patch also exposes the cntrltype as well.
> >
> > Signed-off-by: Martin Belanger <martin.belanger at dell.com>
> > ---
> >   drivers/nvme/host/core.c | 35
> +++++++++++++++++++++++++++++++++++
> >   drivers/nvme/host/nvme.h |  3 +++
> >   include/linux/nvme.h     | 10 +++++++++-
> >   3 files changed, 47 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index
> > 872c389dc6d3..c65d44fb65c4 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -2992,6 +2992,9 @@ static int nvme_init_identify(struct nvme_ctrl
> *ctrl)
> >   	ctrl->max_namespaces = le32_to_cpu(id->mnan);
> >   	ctrl->ctratt = le32_to_cpu(id->ctratt);
> >
> > +	ctrl->cntrltype = id->cntrltype;
> > +	ctrl->dctype = id->dctype;
> > +
> >   	if (id->rtd3e) {
> >   		/* us -> s */
> >   		u32 transition_time = le32_to_cpu(id->rtd3e) /
> USEC_PER_SEC; @@
> > -3525,6 +3528,34 @@ static ssize_t
> nvme_ctrl_fast_io_fail_tmo_store(struct device *dev,
> >   static DEVICE_ATTR(fast_io_fail_tmo, S_IRUGO | S_IWUSR,
> >   	nvme_ctrl_fast_io_fail_tmo_show,
> nvme_ctrl_fast_io_fail_tmo_store);
> >
> > +static ssize_t nvme_ctrl_cntrltype_show(struct device *dev,
> > +				     struct device_attribute *attr, char *buf) {
> > +	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> > +
> > +	switch (ctrl->cntrltype) {
> > +	case NVME_CTRL_IO:	return sysfs_emit(buf, "io\n");
> > +	case NVME_CTRL_DISC:	return sysfs_emit(buf, "disc\n");
> 
> 'discovery', please. 'disc' is misleading here.
> 
> > +	case NVME_CTRL_ADMIN:	return sysfs_emit(buf, "admin\n");
> > +	default:		return sysfs_emit(buf, "reserved\n");
> > +	}
> > +}
> > +static DEVICE_ATTR(cntrltype, S_IRUGO, nvme_ctrl_cntrltype_show,
> > +NULL);
> > +
> > +static ssize_t nvme_ctrl_dctype_show(struct device *dev,
> > +				     struct device_attribute *attr, char *buf) {
> > +	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> > +
> > +	switch (ctrl->dctype) {
> > +	case NVME_DCTYPE_NOT_REPORTED:	return sysfs_emit(buf,
> "nr\n");
> 
> Please use 'none' here; 'nr' is ambiguous ('Number'?)

Thanks for the review. I will apply the changes.

Question: Since this was part of a set of patches, do I need to resubmit
the whole set or can I just resubmit this one patch?

Also, Sagi mentioned that the dctype should only be exposed for
Discovery Controllers, which makes sense. However, I found that
when  nvme_dev_attrs_are_visible() is invoked the ctrl object is
not fully configured yet (i.e. the controller is not yet identified).
Is there any other way to determine the controller type prior to
receiving the response to the Identify command?

> 
> > +	case NVME_DCTYPE_DDC:		return sysfs_emit(buf,
> "ddc\n");
> > +	case NVME_DCTYPE_CDC:		return sysfs_emit(buf,
> "cdc\n");
> > +	default:			return sysfs_emit(buf, "reserved\n");
> > +	}
> > +}
> > +static DEVICE_ATTR(dctype, S_IRUGO, nvme_ctrl_dctype_show, NULL);
> > +
> >   static struct attribute *nvme_dev_attrs[] = {
> >   	&dev_attr_reset_controller.attr,
> >   	&dev_attr_rescan_controller.attr,
> > @@ -3546,6 +3577,8 @@ static struct attribute *nvme_dev_attrs[] = {
> >   	&dev_attr_reconnect_delay.attr,
> >   	&dev_attr_fast_io_fail_tmo.attr,
> >   	&dev_attr_kato.attr,
> > +	&dev_attr_cntrltype.attr,
> > +	&dev_attr_dctype.attr,
> >   	NULL
> >   };
> >
> > @@ -3569,6 +3602,8 @@ static umode_t
> nvme_dev_attrs_are_visible(struct kobject *kobj,
> >   		return 0;
> >   	if (a == &dev_attr_fast_io_fail_tmo.attr && !ctrl->opts)
> >   		return 0;
> > +	if (a == &dev_attr_dctype.attr && ctrl->cntrltype ==
> NVME_CTRL_DISC)
> > +		return 0;
> >
> >   	return a->mode;
> >   }
> > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index
> > a162f6c6da6e..e30626c00791 100644
> > --- a/drivers/nvme/host/nvme.h
> > +++ b/drivers/nvme/host/nvme.h
> > @@ -349,6 +349,9 @@ struct nvme_ctrl {
> >   	unsigned long discard_page_busy;
> >
> >   	struct nvme_fault_inject fault_inject;
> > +
> > +	enum nvme_ctrl_type cntrltype;
> > +	enum nvme_dctype dctype;
> >   };
> >
> >   enum nvme_iopolicy {
> > diff --git a/include/linux/nvme.h b/include/linux/nvme.h index
> > 855dd9b3e84b..af1494d83a52 100644
> > --- a/include/linux/nvme.h
> > +++ b/include/linux/nvme.h
> > @@ -43,6 +43,12 @@ enum nvme_ctrl_type {
> >   	NVME_CTRL_ADMIN	= 3,		/* Administrative controller
> */
> >   };
> >
> > +enum nvme_dctype {
> > +	NVME_DCTYPE_NOT_REPORTED = 0,
> > +	NVME_DCTYPE_DDC = 1,		/* Direct Discovery Controller
> */
> > +	NVME_DCTYPE_CDC = 2,		/* Central Discovery
> Controller */
> > +};
> > +
> >   /* Address Family codes for Discovery Log Page entry ADRFAM field */
> >   enum {
> >   	NVMF_ADDR_FAMILY_PCI	= 0,	/* PCIe */
> > @@ -320,7 +326,9 @@ struct nvme_id_ctrl {
> >   	__le16			icdoff;
> >   	__u8			ctrattr;
> >   	__u8			msdbd;
> > -	__u8			rsvd1804[244];
> > +	__u8			rsvd1804[2];
> > +	__u8			dctype;
> > +	__u8			rsvd1807[241];
> >   	struct nvme_id_power_state	psd[32];
> >   	__u8			vs[1024];
> >   };
> 
> Cheers,
> 
> Hannes
> --
> Dr. Hannes Reinecke                Kernel Storage Architect
> hare at suse.de                              +49 911 74053 688
> SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809
> (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Internal Use - Confidential