On Wed, Feb 02, 2022 at 02:28:53PM +0100, Hannes Reinecke wrote:
> On 2/1/22 20:01, Keith Busch wrote:
> > + integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
> > + break;
> > + case NVME_NVM_NS_64B_GUARD:
> > + integrity.profile = &nvme_pi_type1_crc64
I just noticed this should be type3, not type1...
> > + integrity.tag_size = sizeof(u16) + 6;
> > + integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
> > + break;
> > + default:
> > + integrity.profile = NULL;
> > + break;
> > + }
> > break;
> > case NVME_NS_DPS_PI_TYPE1:
> > case NVME_NS_DPS_PI_TYPE2:
> > - integrity.profile = &t10_pi_type1_crc;
> > - integrity.tag_size = sizeof(u16);
> > - integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
> > + switch (ns->guard_type) {
> > + case NVME_NVM_NS_16B_GUARD:
> > + integrity.profile = &t10_pi_type1_crc;
> > + integrity.tag_size = sizeof(u16);
> > + integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
> > + break;
> > + case NVME_NVM_NS_64B_GUARD:
> > + integrity.profile = &nvme_pi_type1_crc64;
> > + integrity.tag_size = sizeof(u16);
>
> Is that correct? Shouldn't it be '8' like in the above case?
For type1 and 2, I believe tag_size refers to the "application" tag,
which is 2 bytes here.
The reason it is 8 bytes for type3 is because there is no ref tag, so
that portion of the metadata becomes part of the opaque application
tag_size.
> > @@ -3104,7 +3218,7 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
> > if (ret < 0)
> > return ret;
> > - ret = nvme_configure_acre(ctrl);
> > + ret = nvme_configure_host_options(ctrl);
> > if (ret < 0)
> > return ret;
>
> This could be made into a separate patch, is it's not directly related to PI
> support.
Well, the driver can't read the new PI formats without enabling host
supported features for it. Enabling the feature tells the controller
we're going to check for it, so I don't think we could reasonably split
this part into a prep patch from the part that sets up the PI formats.
