[PATCH V9 3/9] nvmet: add NVM command set identifier support

[Chaitanya Kulkarni]

On 1/11/21 23:27, Christoph Hellwig wrote:
> The Command Set Identifier has no "NVM" in its name.
>
>
>> +static inline bool nvmet_cc_css_check(u8 cc_css)
>> +{
>> +	switch (cc_css <<= NVME_CC_CSS_SHIFT) {
>> +	case NVME_CC_CSS_NVM:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
> This hunk looks misplaced, it isn't very useful on its own, but
> should go together with the multiple command set support.
>
We advertise the support for command sets supported in
nvmet_init_cap() -> ctrl->cap = (1ULL << 37). This results in
nvme_enable_ctrl() setting the ctrl->ctrl_config -> NVME_CC_CSS_NVM.
In current code in nvmet_start_ctrl() ->  nvmet_cc_css(ctrl->cc) != 0
checks if value is not = 0 but doesn't use the macro used by the host.
Above function does that also makes it helper that we use in the next
patch where cc_css value is != 0 but NVME_CC_CSS_CSI with
ctrl->cap set to 1ULL << 43.

With code flow in [1] above function is needed to make sure css value
matches the value set by the host using the same macro in
nvme_enable_ctrl() NVME_CC_CSS_NVM. Otherwise patch looks incomplete
and adding check for the CSS NVM with CSS_CSI looks mixing up things
to me.

Are you okay with that ?

[1]
nvme_enable_ctrl()
 ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config)
  nvmf_reg_write32()
   nvmet_parse_fabrics_cmd()
    nvmet_execute_prop_set()
     nvmet_update_ctrl()
      new cc != old cc == true -> nvmet_start_ctrl()
       nvmet_cc_css_check(ctrl->css)
        Check if host has set the for controller config NVME_CC_CSS_NVM
        as we are supporting default CSS_NVM which ctrl needs to set
        irrespective of other CC_CSS values.