Re: svn commit: r330759 - in head: sbin/nvmecontrol sys/dev/nvme

2018-03-11 Thread Alexander Motin
On 11.03.2018 01:12, Conrad Meyer wrote:
> On Sat, Mar 10, 2018 at 9:09 PM, Alexander Motin  wrote:
>> Author: mav
>> Date: Sun Mar 11 05:09:02 2018
>> New Revision: 330759
>> URL: https://svnweb.freebsd.org/changeset/base/330759
>>
>> Log:
>>   Add new identify data structures fields from NVMe 1.3a.
>>
>> ...
>> Modified: head/sbin/nvmecontrol/identify.c
>> ==
>> --- head/sbin/nvmecontrol/identify.cSun Mar 11 04:37:05 2018
>> (r330758)
>> +++ head/sbin/nvmecontrol/identify.cSun Mar 11 05:09:02 2018
>> (r330759)
>> @@ -95,25 +95,35 @@ print_controller(struct nvme_controller_data *cdata)
>> ...
>> +   ((cdata->mic >> NVME_CTRLR_DATA_MIC_SRIOVVF_SHIFT) &
>> +NVME_CTRLR_DATA_MIC_SRIOVVF_MASK) ? "SR-IOV VF, " : "",
>> +   ((cdata->mic >> NVME_CTRLR_DATA_MIC_MCTRLRS_SHIFT) &
>> +NVME_CTRLR_DATA_MIC_MCTRLRS_MASK) ? "Multiple controllers, " : 
>> "",
>> +   ((cdata->mic >> NVME_CTRLR_DATA_MIC_MPORTS_SHIFT) &
>> +NVME_CTRLR_DATA_MIC_MPORTS_MASK) ? "Multiple ports" : "");
>> ...
>> Modified: head/sys/dev/nvme/nvme.h
>> ==
>> --- head/sys/dev/nvme/nvme.hSun Mar 11 04:37:05 2018(r330758)
>> +++ head/sys/dev/nvme/nvme.hSun Mar 11 05:09:02 2018(r330759)
>> @@ -153,6 +153,17 @@
>>  #define NVME_PWR_ST_APS_SHIFT  (6)
>>  #define NVME_PWR_ST_APS_MASK   (0x3)
>>
>> +/** Controller Multi-path I/O and Namespace Sharing Capabilities */
>> +/* More then one port */
>> +#define NVME_CTRLR_DATA_MIC_MPORTS_SHIFT   (0)
>> +#define NVME_CTRLR_DATA_MIC_MPORTS_MASK(0x1)
>> +/* More then one controller */
>> +#define NVME_CTRLR_DATA_MIC_MCTRLRS_SHIFT  (1)
>> +#define NVME_CTRLR_DATA_MIC_MCTRLRS_MASK   (0x1)
>> +/* SR-IOV Virtual Function */
>> +#define NVME_CTRLR_DATA_MIC_SRIOVVF_SHIFT  (2)
>> +#define NVME_CTRLR_DATA_MIC_SRIOVVF_MASK   (0x1)
>> +
>>  /** OACS - optional admin command support */
>>  /* supports security send/receive commands */
>>  #define NVME_CTRLR_DATA_OACS_SECURITY_SHIFT(0)
>> @@ -166,6 +177,21 @@
>>  /* supports namespace management commands */
>>  #define NVME_CTRLR_DATA_OACS_NSMGMT_SHIFT  (3)
>>  #define NVME_CTRLR_DATA_OACS_NSMGMT_MASK   (0x1)
>> +/* supports Device Self-test command */
>> +#define NVME_CTRLR_DATA_OACS_SELFTEST_SHIFT(4)
>> +#define NVME_CTRLR_DATA_OACS_SELFTEST_MASK (0x1)
>> +/* supports Directives */
>> +#define NVME_CTRLR_DATA_OACS_DIRECTIVES_SHIFT  (5)
>> +#define NVME_CTRLR_DATA_OACS_DIRECTIVES_MASK   (0x1)
>> +/* supports NVMe-MI Send/Receive */
>> +#define NVME_CTRLR_DATA_OACS_NVMEMI_SHIFT  (6)
>> +#define NVME_CTRLR_DATA_OACS_NVMEMI_MASK   (0x1)
>> +/* supports Virtualization Management */
>> +#define NVME_CTRLR_DATA_OACS_VM_SHIFT  (7)
>> +#define NVME_CTRLR_DATA_OACS_VM_MASK   (0x1)
>> +/* supports Doorbell Buffer Config */
>> +#define NVME_CTRLR_DATA_OACS_DBBUFFER_SHIFT(8)
>> +#define NVME_CTRLR_DATA_OACS_DBBUFFER_MASK (0x1)
> 
> This seems like a suboptimal way to represent and check single flag
> bits.  Is there a reason the conventional (1

Re: svn commit: r330759 - in head: sbin/nvmecontrol sys/dev/nvme

2018-03-10 Thread Conrad Meyer
On Sat, Mar 10, 2018 at 9:09 PM, Alexander Motin  wrote:
> Author: mav
> Date: Sun Mar 11 05:09:02 2018
> New Revision: 330759
> URL: https://svnweb.freebsd.org/changeset/base/330759
>
> Log:
>   Add new identify data structures fields from NVMe 1.3a.
>
> ...
> Modified: head/sbin/nvmecontrol/identify.c
> ==
> --- head/sbin/nvmecontrol/identify.cSun Mar 11 04:37:05 2018
> (r330758)
> +++ head/sbin/nvmecontrol/identify.cSun Mar 11 05:09:02 2018
> (r330759)
> @@ -95,25 +95,35 @@ print_controller(struct nvme_controller_data *cdata)
> ...
> +   ((cdata->mic >> NVME_CTRLR_DATA_MIC_SRIOVVF_SHIFT) &
> +NVME_CTRLR_DATA_MIC_SRIOVVF_MASK) ? "SR-IOV VF, " : "",
> +   ((cdata->mic >> NVME_CTRLR_DATA_MIC_MCTRLRS_SHIFT) &
> +NVME_CTRLR_DATA_MIC_MCTRLRS_MASK) ? "Multiple controllers, " : 
> "",
> +   ((cdata->mic >> NVME_CTRLR_DATA_MIC_MPORTS_SHIFT) &
> +NVME_CTRLR_DATA_MIC_MPORTS_MASK) ? "Multiple ports" : "");
> ...
> Modified: head/sys/dev/nvme/nvme.h
> ==
> --- head/sys/dev/nvme/nvme.hSun Mar 11 04:37:05 2018(r330758)
> +++ head/sys/dev/nvme/nvme.hSun Mar 11 05:09:02 2018(r330759)
> @@ -153,6 +153,17 @@
>  #define NVME_PWR_ST_APS_SHIFT  (6)
>  #define NVME_PWR_ST_APS_MASK   (0x3)
>
> +/** Controller Multi-path I/O and Namespace Sharing Capabilities */
> +/* More then one port */
> +#define NVME_CTRLR_DATA_MIC_MPORTS_SHIFT   (0)
> +#define NVME_CTRLR_DATA_MIC_MPORTS_MASK(0x1)
> +/* More then one controller */
> +#define NVME_CTRLR_DATA_MIC_MCTRLRS_SHIFT  (1)
> +#define NVME_CTRLR_DATA_MIC_MCTRLRS_MASK   (0x1)
> +/* SR-IOV Virtual Function */
> +#define NVME_CTRLR_DATA_MIC_SRIOVVF_SHIFT  (2)
> +#define NVME_CTRLR_DATA_MIC_SRIOVVF_MASK   (0x1)
> +
>  /** OACS - optional admin command support */
>  /* supports security send/receive commands */
>  #define NVME_CTRLR_DATA_OACS_SECURITY_SHIFT(0)
> @@ -166,6 +177,21 @@
>  /* supports namespace management commands */
>  #define NVME_CTRLR_DATA_OACS_NSMGMT_SHIFT  (3)
>  #define NVME_CTRLR_DATA_OACS_NSMGMT_MASK   (0x1)
> +/* supports Device Self-test command */
> +#define NVME_CTRLR_DATA_OACS_SELFTEST_SHIFT(4)
> +#define NVME_CTRLR_DATA_OACS_SELFTEST_MASK (0x1)
> +/* supports Directives */
> +#define NVME_CTRLR_DATA_OACS_DIRECTIVES_SHIFT  (5)
> +#define NVME_CTRLR_DATA_OACS_DIRECTIVES_MASK   (0x1)
> +/* supports NVMe-MI Send/Receive */
> +#define NVME_CTRLR_DATA_OACS_NVMEMI_SHIFT  (6)
> +#define NVME_CTRLR_DATA_OACS_NVMEMI_MASK   (0x1)
> +/* supports Virtualization Management */
> +#define NVME_CTRLR_DATA_OACS_VM_SHIFT  (7)
> +#define NVME_CTRLR_DATA_OACS_VM_MASK   (0x1)
> +/* supports Doorbell Buffer Config */
> +#define NVME_CTRLR_DATA_OACS_DBBUFFER_SHIFT(8)
> +#define NVME_CTRLR_DATA_OACS_DBBUFFER_MASK (0x1)

This seems like a suboptimal way to represent and check single flag
bits.  Is there a reason the conventional (1