Neeraj Kumar wrote: > Prior to LSA 2.1 Support, label in slot means only namespace > label. But with LSA 2.1 a label can be either namespace or > region label. > > Slot validation routine validates label slot by calculating > label checksum. It was only validating namespace label. > This changeset also validates region label if present. > > In previous patch to_lsa_label() was introduced along with > to_label(). to_label() returns only namespace label whereas > to_lsa_label() returns union nd_lsa_label* > > In this patch We have converted all usage of to_label()
NIT: don't use 'We' > to to_lsa_label() > > Reviewed-by: Jonathan Cameron <[email protected]> > Reviewed-by: Dave Jiang <[email protected]> > Signed-off-by: Neeraj Kumar <[email protected]> > --- > drivers/nvdimm/label.c | 94 ++++++++++++++++++++++++++++-------------- > 1 file changed, 64 insertions(+), 30 deletions(-) > > diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c > index 17e2a1f5a6da..9854cb45fb62 100644 > --- a/drivers/nvdimm/label.c > +++ b/drivers/nvdimm/label.c > @@ -312,16 +312,6 @@ static union nd_lsa_label *to_lsa_label(struct > nvdimm_drvdata *ndd, int slot) > return (union nd_lsa_label *) label; > } > > -static struct nd_namespace_label *to_label(struct nvdimm_drvdata *ndd, int > slot) > -{ > - unsigned long label, base; > - > - base = (unsigned long) nd_label_base(ndd); > - label = base + sizeof_namespace_label(ndd) * slot; > - > - return (struct nd_namespace_label *) label; > -} > - > #define for_each_clear_bit_le(bit, addr, size) \ > for ((bit) = find_next_zero_bit_le((addr), (size), 0); \ > (bit) < (size); \ > @@ -382,7 +372,7 @@ static bool nsl_validate_checksum(struct nvdimm_drvdata > *ndd, > { > u64 sum, sum_save; > > - if (!ndd->cxl && !efi_namespace_label_has(ndd, checksum)) > + if (!efi_namespace_label_has(ndd, checksum)) What does this change have to do with region label validation during slot validation? > return true; > > sum_save = nsl_get_checksum(ndd, nd_label); > @@ -397,13 +387,25 @@ static void nsl_calculate_checksum(struct > nvdimm_drvdata *ndd, > { > u64 sum; > > - if (!ndd->cxl && !efi_namespace_label_has(ndd, checksum)) > + if (!efi_namespace_label_has(ndd, checksum)) This and the above seem like cleanups because efi_namespace_label_has() already checks !ndd->cxl? Was that the intent? Perhaps as a separate cleanup? Rest of the patch looks reasonable, Ira > return; > nsl_set_checksum(ndd, nd_label, 0); > sum = nd_fletcher64(nd_label, sizeof_namespace_label(ndd), 1); > nsl_set_checksum(ndd, nd_label, sum); > } > > +static bool region_label_validate_checksum(struct nvdimm_drvdata *ndd, > + struct cxl_region_label *region_label) > +{ > + u64 sum, sum_save; > + > + sum_save = __le64_to_cpu(region_label->checksum); > + region_label->checksum = __cpu_to_le64(0); > + sum = nd_fletcher64(region_label, sizeof_namespace_label(ndd), 1); > + region_label->checksum = __cpu_to_le64(sum_save); > + return sum == sum_save; > +} > + > static void region_label_calculate_checksum(struct nvdimm_drvdata *ndd, > struct cxl_region_label *region_label) > { > @@ -415,16 +417,34 @@ static void region_label_calculate_checksum(struct > nvdimm_drvdata *ndd, > } > > static bool slot_valid(struct nvdimm_drvdata *ndd, > - struct nd_namespace_label *nd_label, u32 slot) > + union nd_lsa_label *lsa_label, u32 slot) > { > + struct cxl_region_label *region_label = &lsa_label->region_label; > + struct nd_namespace_label *nd_label = &lsa_label->ns_label; > + enum label_type type; > bool valid; > + static const char * const label_name[] = { > + [RG_LABEL_TYPE] = "region", > + [NS_LABEL_TYPE] = "namespace", > + }; > > /* check that we are written where we expect to be written */ > - if (slot != nsl_get_slot(ndd, nd_label)) > - return false; > - valid = nsl_validate_checksum(ndd, nd_label); > + if (is_region_label(ndd, lsa_label)) { > + type = RG_LABEL_TYPE; > + if (slot != __le32_to_cpu(region_label->slot)) > + return false; > + valid = region_label_validate_checksum(ndd, region_label); > + } else { > + type = NS_LABEL_TYPE; > + if (slot != nsl_get_slot(ndd, nd_label)) > + return false; > + valid = nsl_validate_checksum(ndd, nd_label); > + } > + > if (!valid) > - dev_dbg(ndd->dev, "fail checksum. slot: %d\n", slot); > + dev_dbg(ndd->dev, "%s label checksum fail. slot: %d\n", > + label_name[type], slot); > + > return valid; > } > > @@ -440,14 +460,16 @@ int nd_label_reserve_dpa(struct nvdimm_drvdata *ndd) > for_each_clear_bit_le(slot, free, nslot) { > struct nd_namespace_label *nd_label; > struct nd_region *nd_region = NULL; > + union nd_lsa_label *lsa_label; > struct nd_label_id label_id; > struct resource *res; > uuid_t label_uuid; > u32 flags; > > - nd_label = to_label(ndd, slot); > + lsa_label = to_lsa_label(ndd, slot); > + nd_label = &lsa_label->ns_label; > > - if (!slot_valid(ndd, nd_label, slot)) > + if (!slot_valid(ndd, lsa_label, slot)) > continue; > > nsl_get_uuid(ndd, nd_label, &label_uuid); > @@ -598,18 +620,30 @@ int nd_label_active_count(struct nvdimm_drvdata *ndd) > return 0; > > for_each_clear_bit_le(slot, free, nslot) { > + struct cxl_region_label *region_label; > struct nd_namespace_label *nd_label; > - > - nd_label = to_label(ndd, slot); > - > - if (!slot_valid(ndd, nd_label, slot)) { > - u32 label_slot = nsl_get_slot(ndd, nd_label); > - u64 size = nsl_get_rawsize(ndd, nd_label); > - u64 dpa = nsl_get_dpa(ndd, nd_label); > + union nd_lsa_label *lsa_label; > + u32 lslot; > + u64 size, dpa; > + > + lsa_label = to_lsa_label(ndd, slot); > + nd_label = &lsa_label->ns_label; > + region_label = &lsa_label->region_label; > + > + if (!slot_valid(ndd, lsa_label, slot)) { > + if (is_region_label(ndd, lsa_label)) { > + lslot = __le32_to_cpu(region_label->slot); > + size = __le64_to_cpu(region_label->rawsize); > + dpa = __le64_to_cpu(region_label->dpa); > + } else { > + lslot = nsl_get_slot(ndd, nd_label); > + size = nsl_get_rawsize(ndd, nd_label); > + dpa = nsl_get_dpa(ndd, nd_label); > + } > > dev_dbg(ndd->dev, > "slot%d invalid slot: %d dpa: %llx size: > %llx\n", > - slot, label_slot, dpa, size); > + slot, lslot, dpa, size); > continue; > } > count++; > @@ -627,10 +661,10 @@ union nd_lsa_label *nd_label_active(struct > nvdimm_drvdata *ndd, int n) > return NULL; > > for_each_clear_bit_le(slot, free, nslot) { > - struct nd_namespace_label *nd_label; > + union nd_lsa_label *lsa_label; > > - nd_label = to_label(ndd, slot); > - if (!slot_valid(ndd, nd_label, slot)) > + lsa_label = to_lsa_label(ndd, slot); > + if (!slot_valid(ndd, lsa_label, slot)) > continue; > > if (n-- == 0) > -- > 2.34.1 > >
