Message ID | 20200120140749.69549-3-aneesh.kumar@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Validating namespace size and start address attributes. | expand |
On Mon, Jan 20, 2020 at 6:08 AM Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> wrote: > > Make sure namespace start addr and size are properly aligned as per architecture > restrictions. If the namespace is not aligned kernel mark the namespace 'disabled' > > Architectures like ppc64 use different page size than PAGE_SIZE to map > direct-map address range. The kernel needs to make sure the namespace size is aligned > correctly for the direct-map page size. > > kernel log will contain information as below. > > [ 5.810939] nd_pmem namespace0.1: invalid size/SPA > [ 5.810969] nd_pmem: probe of namespace0.1 failed with error -95 > > and the namespace will be marked 'disabled' > > { > "dev":"namespace0.1", > "mode":"fsdax", > "map":"mem", > "size":1071644672, > "uuid":"25577a00-c012-421d-89ca-3ee189e08848", > "sector_size":512, > "state":"disabled" > }, > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > --- > drivers/nvdimm/namespace_devs.c | 50 +++++++++++++++++++++++++++++++++ > 1 file changed, 50 insertions(+) > > diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c > index 032dc61725ff..0e2c90730ce3 100644 > --- a/drivers/nvdimm/namespace_devs.c > +++ b/drivers/nvdimm/namespace_devs.c > @@ -1113,6 +1113,51 @@ resource_size_t nvdimm_namespace_capacity(struct nd_namespace_common *ndns) > } > EXPORT_SYMBOL(nvdimm_namespace_capacity); > > +static bool nvdimm_valid_namespace(struct device *dev, > + struct nd_namespace_common *ndns, resource_size_t size) > +{ > + struct device *ndns_dev = &ndns->dev; > + struct nd_region *nd_region = to_nd_region(ndns->dev.parent); > + unsigned long map_size = arch_namespace_map_size(); > + struct resource *res; > + u32 remainder; > + > + /* > + * Don't validate the start and size for blk namespace type > + */ If you're going to comment I'd prefer a comment that explains why, not what. E.g. "blk namespaces are ioremap()'d with small page aligned apertures" > + if (is_namespace_blk(ndns_dev)) > + return true; > + > + /* > + * For btt and raw namespace kernel use ioremap. Assume both can work > + * with PAGE_SIZE alignment. > + */ > + if (is_nd_btt(dev) || is_namespace_io(ndns_dev)) > + map_size = PAGE_SIZE; > + > + div_u64_rem(size, map_size * nd_region->ndr_mappings, &remainder); Not all regions have mappings see e820_register_one(), so this is a divide by zero. I think this is mixing 2 different constraints. There's the soft constraint of wanting aliased pmem capacity to remain aligned relative to blk capacity, and then there's the hard constraint where the kernel just can't use devm_memremap_pages() for the given capacity alignment. I think this routine should be called: nvdimm_validate_geometry() ...and only check for: if (pmem_should_map_pages() || is_nd_pfn() || is_nd_dax()) ...i.e namespaces are mapped by memremap_pages() and compare that against memremap_compat_align().
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c index 032dc61725ff..0e2c90730ce3 100644 --- a/drivers/nvdimm/namespace_devs.c +++ b/drivers/nvdimm/namespace_devs.c @@ -1113,6 +1113,51 @@ resource_size_t nvdimm_namespace_capacity(struct nd_namespace_common *ndns) } EXPORT_SYMBOL(nvdimm_namespace_capacity); +static bool nvdimm_valid_namespace(struct device *dev, + struct nd_namespace_common *ndns, resource_size_t size) +{ + struct device *ndns_dev = &ndns->dev; + struct nd_region *nd_region = to_nd_region(ndns->dev.parent); + unsigned long map_size = arch_namespace_map_size(); + struct resource *res; + u32 remainder; + + /* + * Don't validate the start and size for blk namespace type + */ + if (is_namespace_blk(ndns_dev)) + return true; + + /* + * For btt and raw namespace kernel use ioremap. Assume both can work + * with PAGE_SIZE alignment. + */ + if (is_nd_btt(dev) || is_namespace_io(ndns_dev)) + map_size = PAGE_SIZE; + + div_u64_rem(size, map_size * nd_region->ndr_mappings, &remainder); + if (remainder) + return false; + + if (is_namespace_pmem(ndns_dev)) { + struct nd_namespace_pmem *nspm = to_nd_namespace_pmem(ndns_dev); + + res = &nspm->nsio.res; + } else if (is_namespace_io(ndns_dev)) { + struct nd_namespace_io *nsio = to_nd_namespace_io(ndns_dev); + + res = &nsio->res; + } else + /* cannot reach */ + return false; + + div_u64_rem(res->start, map_size * nd_region->ndr_mappings, &remainder); + if (remainder) + return false; + + return true; +} + bool nvdimm_namespace_locked(struct nd_namespace_common *ndns) { int i; @@ -1739,6 +1784,11 @@ struct nd_namespace_common *nvdimm_namespace_common_probe(struct device *dev) return ERR_PTR(-ENODEV); } + if (!nvdimm_valid_namespace(dev, ndns, size)) { + dev_err(&ndns->dev, "invalid size/SPA"); + return ERR_PTR(-EOPNOTSUPP); + } + if (is_namespace_pmem(&ndns->dev)) { struct nd_namespace_pmem *nspm;
Make sure namespace start addr and size are properly aligned as per architecture restrictions. If the namespace is not aligned kernel mark the namespace 'disabled' Architectures like ppc64 use different page size than PAGE_SIZE to map direct-map address range. The kernel needs to make sure the namespace size is aligned correctly for the direct-map page size. kernel log will contain information as below. [ 5.810939] nd_pmem namespace0.1: invalid size/SPA [ 5.810969] nd_pmem: probe of namespace0.1 failed with error -95 and the namespace will be marked 'disabled' { "dev":"namespace0.1", "mode":"fsdax", "map":"mem", "size":1071644672, "uuid":"25577a00-c012-421d-89ca-3ee189e08848", "sector_size":512, "state":"disabled" }, Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> --- drivers/nvdimm/namespace_devs.c | 50 +++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)