Re: 回复: [PATCH v2] libnvdimm: KASAN: global-out-of-bounds Read in internal_create_group
by Verma, Vishal L
On Wed, 2020-08-19 at 03:23 +0000, Zhang, Qiang wrote:
> cc: Dan Williams
> Please review.
Hi Qiang,
I've got this queued up, I'll submit it for -rc2.
Thanks,
-Vishal
>
> ________________________________________
> 发件人: linux-kernel-owner(a)vger.kernel.org <linux-kernel-owner(a)vger.kernel.org> 代表 qiang.zhang(a)windriver.com <qiang.zhang(a)windriver.com>
> 发送时间: 2020年8月12日 16:55
> 收件人: dan.j.williams(a)intel.com; vishal.l.verma(a)intel.com; dave.jiang(a)intel.com; ira.weiny(a)intel.com
> 抄送: linux-nvdimm(a)lists.01.org; linux-kernel(a)vger.kernel.org
> 主题: [PATCH v2] libnvdimm: KASAN: global-out-of-bounds Read in internal_create_group
>
> From: Zqiang <qiang.zhang(a)windriver.com>
>
> Because the last member of the "nvdimm_firmware_attributes" array
> was not assigned a null ptr, when traversal of "grp->attrs" array
> is out of bounds in "create_files" func.
>
> func:
> create_files:
> ->for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++)
> ->....
>
> BUG: KASAN: global-out-of-bounds in create_files fs/sysfs/group.c:43 [inline]
> BUG: KASAN: global-out-of-bounds in internal_create_group+0x9d8/0xb20
> fs/sysfs/group.c:149
> Read of size 8 at addr ffffffff8a2e4cf0 by task kworker/u17:10/959
>
> CPU: 2 PID: 959 Comm: kworker/u17:10 Not tainted 5.8.0-syzkaller #0
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
> BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
> Workqueue: events_unbound async_run_entry_fn
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x18f/0x20d lib/dump_stack.c:118
> print_address_description.constprop.0.cold+0x5/0x497 mm/kasan/report.c:383
> __kasan_report mm/kasan/report.c:513 [inline]
> kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
> create_files fs/sysfs/group.c:43 [inline]
> internal_create_group+0x9d8/0xb20 fs/sysfs/group.c:149
> internal_create_groups.part.0+0x90/0x140 fs/sysfs/group.c:189
> internal_create_groups fs/sysfs/group.c:185 [inline]
> sysfs_create_groups+0x25/0x50 fs/sysfs/group.c:215
> device_add_groups drivers/base/core.c:2024 [inline]
> device_add_attrs drivers/base/core.c:2178 [inline]
> device_add+0x7fd/0x1c40 drivers/base/core.c:2881
> nd_async_device_register+0x12/0x80 drivers/nvdimm/bus.c:506
> async_run_entry_fn+0x121/0x530 kernel/async.c:123
> process_one_work+0x94c/0x1670 kernel/workqueue.c:2269
> worker_thread+0x64c/0x1120 kernel/workqueue.c:2415
> kthread+0x3b5/0x4a0 kernel/kthread.c:292
> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
>
> The buggy address belongs to the variable:
> nvdimm_firmware_attributes+0x10/0x40
>
> Reported-by: syzbot+1cf0ffe61aecf46f588f(a)syzkaller.appspotmail.com
> Signed-off-by: Zqiang <qiang.zhang(a)windriver.com>
> ---
> v1->v2:
> Modify the description of the error.
>
> drivers/nvdimm/dimm_devs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
> index 61374def5155..b59032e0859b 100644
> --- a/drivers/nvdimm/dimm_devs.c
> +++ b/drivers/nvdimm/dimm_devs.c
> @@ -529,6 +529,7 @@ static DEVICE_ATTR_ADMIN_RW(activate);
> static struct attribute *nvdimm_firmware_attributes[] = {
> &dev_attr_activate.attr,
> &dev_attr_result.attr,
> + NULL,
> };
>
> static umode_t nvdimm_firmware_visible(struct kobject *kobj, struct attribute *a, int n)
> --
> 2.17.1
>
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm(a)lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave(a)lists.01.org
1 year, 10 months
[PATCH v2] libnvdimm: KASAN: global-out-of-bounds Read in internal_create_group
by qiang.zhang＠windriver.com
From: Zqiang <qiang.zhang(a)windriver.com>
Because the last member of the "nvdimm_firmware_attributes" array
was not assigned a null ptr, when traversal of "grp->attrs" array
is out of bounds in "create_files" func.
func:
create_files:
->for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++)
->....
BUG: KASAN: global-out-of-bounds in create_files fs/sysfs/group.c:43 [inline]
BUG: KASAN: global-out-of-bounds in internal_create_group+0x9d8/0xb20
fs/sysfs/group.c:149
Read of size 8 at addr ffffffff8a2e4cf0 by task kworker/u17:10/959
CPU: 2 PID: 959 Comm: kworker/u17:10 Not tainted 5.8.0-syzkaller #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
Workqueue: events_unbound async_run_entry_fn
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x18f/0x20d lib/dump_stack.c:118
print_address_description.constprop.0.cold+0x5/0x497 mm/kasan/report.c:383
__kasan_report mm/kasan/report.c:513 [inline]
kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
create_files fs/sysfs/group.c:43 [inline]
internal_create_group+0x9d8/0xb20 fs/sysfs/group.c:149
internal_create_groups.part.0+0x90/0x140 fs/sysfs/group.c:189
internal_create_groups fs/sysfs/group.c:185 [inline]
sysfs_create_groups+0x25/0x50 fs/sysfs/group.c:215
device_add_groups drivers/base/core.c:2024 [inline]
device_add_attrs drivers/base/core.c:2178 [inline]
device_add+0x7fd/0x1c40 drivers/base/core.c:2881
nd_async_device_register+0x12/0x80 drivers/nvdimm/bus.c:506
async_run_entry_fn+0x121/0x530 kernel/async.c:123
process_one_work+0x94c/0x1670 kernel/workqueue.c:2269
worker_thread+0x64c/0x1120 kernel/workqueue.c:2415
kthread+0x3b5/0x4a0 kernel/kthread.c:292
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
The buggy address belongs to the variable:
nvdimm_firmware_attributes+0x10/0x40
Reported-by: syzbot+1cf0ffe61aecf46f588f(a)syzkaller.appspotmail.com
Signed-off-by: Zqiang <qiang.zhang(a)windriver.com>
---
v1->v2:
Modify the description of the error.
drivers/nvdimm/dimm_devs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 61374def5155..b59032e0859b 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -529,6 +529,7 @@ static DEVICE_ATTR_ADMIN_RW(activate);
static struct attribute *nvdimm_firmware_attributes[] = {
&dev_attr_activate.attr,
&dev_attr_result.attr,
+ NULL,
};
static umode_t nvdimm_firmware_visible(struct kobject *kobj, struct attribute *a, int n)
--
2.17.1
1 year, 10 months
Re: [PATCH 1/3] libnvdimm: Fix memory leaks in of_pmem.c
by Markus Elfring
> The memory priv->bus_desc.provider_name allocated by kstrdup() should be
> freed.
* Would an imperative wording be preferred for the change description?
* I propose to add the tag “Fixes” to the commit message.
Regards,
Markus
1 year, 10 months
Can we change the S_DAX flag immediately on XFS without dropping
caches?
by Li, Hao
Hi,
I have noticed that we have to drop caches to make the changing of S_DAX
flag take effect after using chattr +x to turn on DAX for a existing
regular file. The related function is xfs_diflags_to_iflags, whose
second parameter determines whether we should set S_DAX immediately.
I can't figure out why we do this. Is this because the page caches in
address_space->i_pages are hard to deal with? I also wonder what will
happen if we set S_DAX unconditionally. Thanks!
Regards,
Hao Li
1 year, 10 months
[PATCH 0/3] bugfix and optimize for drivers/nvdimm
by Zhen Lei
I found a memleak when I learned the drivers/nvdimm code today. And I also
added a sanity check for priv->bus_desc.provider_name, because strdup()
maybe failed. Patch 3 is a trivial source code optimization.
Zhen Lei (3):
libnvdimm: fix memleak in of_pmem.c
libnvdimm: add sanity check for provider_name in
of_pmem_region_probe()
libnvdimm/bus: simplify walk_to_nvdimm_bus()
drivers/nvdimm/bus.c | 7 +++----
drivers/nvdimm/of_pmem.c | 7 +++++++
2 files changed, 10 insertions(+), 4 deletions(-)
--
1.8.3
1 year, 10 months
[PATCH 1/1] device-dax: fix mismatches of request_mem_region()
by Zhen Lei
The resources allocated by request_mem_region() is better to use
release_mem_region() to free. These two functions are paired.
Signed-off-by: Zhen Lei <thunder.leizhen(a)huawei.com>
---
drivers/dax/kmem.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
index 275aa5f873991af..9e38f9c2b6d7f02 100644
--- a/drivers/dax/kmem.c
+++ b/drivers/dax/kmem.c
@@ -82,8 +82,7 @@ int dev_dax_kmem_probe(struct device *dev)
rc = add_memory_driver_managed(numa_node, new_res->start,
resource_size(new_res), kmem_name);
if (rc) {
- release_resource(new_res);
- kfree(new_res);
+ release_mem_region(kmem_start, kmem_size);
kfree(new_res_name);
return rc;
}
@@ -118,8 +117,7 @@ static int dev_dax_kmem_remove(struct device *dev)
}
/* Release and free dax resources */
- release_resource(res);
- kfree(res);
+ release_mem_region(kmem_start, kmem_size);
kfree(res_name);
dev_dax->dax_kmem_res = NULL;
--
1.8.3
1 year, 10 months
[PATCH v2 02/20] dax: Create a range version of dax_layout_busy_page()
by Vivek Goyal
virtiofs device has a range of memory which is mapped into file inodes
using dax. This memory is mapped in qemu on host and maps different
sections of real file on host. Size of this memory is limited
(determined by administrator) and depending on filesystem size, we will
soon reach a situation where all the memory is in use and we need to
reclaim some.
As part of reclaim process, we will need to make sure that there are
no active references to pages (taken by get_user_pages()) on the memory
range we are trying to reclaim. I am planning to use
dax_layout_busy_page() for this. But in current form this is per inode
and scans through all the pages of the inode.
We want to reclaim only a portion of memory (say 2MB page). So we want
to make sure that only that 2MB range of pages do not have any
references (and don't want to unmap all the pages of inode).
Hence, create a range version of this function named
dax_layout_busy_page_range() which can be used to pass a range which
needs to be unmapped.
Cc: Dan Williams <dan.j.williams(a)intel.com>
Cc: linux-nvdimm(a)lists.01.org
Signed-off-by: Vivek Goyal <vgoyal(a)redhat.com>
---
fs/dax.c | 66 ++++++++++++++++++++++++++++++++-------------
include/linux/dax.h | 6 +++++
2 files changed, 54 insertions(+), 18 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index 11b16729b86f..0d51b0fbb489 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -558,27 +558,20 @@ static void *grab_mapping_entry(struct xa_state *xas,
return xa_mk_internal(VM_FAULT_FALLBACK);
}
-/**
- * dax_layout_busy_page - find first pinned page in @mapping
- * @mapping: address space to scan for a page with ref count > 1
- *
- * DAX requires ZONE_DEVICE mapped pages. These pages are never
- * 'onlined' to the page allocator so they are considered idle when
- * page->count == 1. A filesystem uses this interface to determine if
- * any page in the mapping is busy, i.e. for DMA, or other
- * get_user_pages() usages.
- *
- * It is expected that the filesystem is holding locks to block the
- * establishment of new mappings in this address_space. I.e. it expects
- * to be able to run unmap_mapping_range() and subsequently not race
- * mapping_mapped() becoming true.
+/*
+ * Partial pages are included. If end is LLONG_MAX, pages in the range from
+ * start to end of the file are inluded.
*/
-struct page *dax_layout_busy_page(struct address_space *mapping)
+struct page *dax_layout_busy_page_range(struct address_space *mapping,
+ loff_t start, loff_t end)
{
- XA_STATE(xas, &mapping->i_pages, 0);
void *entry;
unsigned int scanned = 0;
struct page *page = NULL;
+ pgoff_t start_idx = start >> PAGE_SHIFT;
+ pgoff_t end_idx = end >> PAGE_SHIFT;
+ XA_STATE(xas, &mapping->i_pages, start_idx);
+ loff_t len, lstart = round_down(start, PAGE_SIZE);
/*
* In the 'limited' case get_user_pages() for dax is disabled.
@@ -589,6 +582,22 @@ struct page *dax_layout_busy_page(struct address_space *mapping)
if (!dax_mapping(mapping) || !mapping_mapped(mapping))
return NULL;
+ /* If end == LLONG_MAX, all pages from start to till end of file */
+ if (end == LLONG_MAX) {
+ end_idx = ULONG_MAX;
+ len = 0;
+ } else {
+ /* length is being calculated from lstart and not start.
+ * This is due to behavior of unmap_mapping_range(). If
+ * start is say 4094 and end is on 4096 then we want to
+ * unamp two pages, idx 0 and 1. But unmap_mapping_range()
+ * will unmap only page at idx 0. If we calculate len
+ * from the rounded down start, this problem should not
+ * happen.
+ */
+ len = end - lstart + 1;
+ }
+
/*
* If we race get_user_pages_fast() here either we'll see the
* elevated page count in the iteration and wait, or
@@ -601,10 +610,10 @@ struct page *dax_layout_busy_page(struct address_space *mapping)
* guaranteed to either see new references or prevent new
* references from being established.
*/
- unmap_mapping_range(mapping, 0, 0, 0);
+ unmap_mapping_range(mapping, start, len, 0);
xas_lock_irq(&xas);
- xas_for_each(&xas, entry, ULONG_MAX) {
+ xas_for_each(&xas, entry, end_idx) {
if (WARN_ON_ONCE(!xa_is_value(entry)))
continue;
if (unlikely(dax_is_locked(entry)))
@@ -625,6 +634,27 @@ struct page *dax_layout_busy_page(struct address_space *mapping)
xas_unlock_irq(&xas);
return page;
}
+EXPORT_SYMBOL_GPL(dax_layout_busy_page_range);
+
+/**
+ * dax_layout_busy_page - find first pinned page in @mapping
+ * @mapping: address space to scan for a page with ref count > 1
+ *
+ * DAX requires ZONE_DEVICE mapped pages. These pages are never
+ * 'onlined' to the page allocator so they are considered idle when
+ * page->count == 1. A filesystem uses this interface to determine if
+ * any page in the mapping is busy, i.e. for DMA, or other
+ * get_user_pages() usages.
+ *
+ * It is expected that the filesystem is holding locks to block the
+ * establishment of new mappings in this address_space. I.e. it expects
+ * to be able to run unmap_mapping_range() and subsequently not race
+ * mapping_mapped() becoming true.
+ */
+struct page *dax_layout_busy_page(struct address_space *mapping)
+{
+ return dax_layout_busy_page_range(mapping, 0, 0);
+}
EXPORT_SYMBOL_GPL(dax_layout_busy_page);
static int __dax_invalidate_entry(struct address_space *mapping,
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 6904d4e0b2e0..9016929db4c6 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -141,6 +141,7 @@ int dax_writeback_mapping_range(struct address_space *mapping,
struct dax_device *dax_dev, struct writeback_control *wbc);
struct page *dax_layout_busy_page(struct address_space *mapping);
+struct page *dax_layout_busy_page_range(struct address_space *mapping, loff_t start, loff_t end);
dax_entry_t dax_lock_page(struct page *page);
void dax_unlock_page(struct page *page, dax_entry_t cookie);
#else
@@ -171,6 +172,11 @@ static inline struct page *dax_layout_busy_page(struct address_space *mapping)
return NULL;
}
+static inline struct page *dax_layout_busy_page_range(struct address_space *mapping, pgoff_t start, pgoff_t nr_pages)
+{
+ return NULL;
+}
+
static inline int dax_writeback_mapping_range(struct address_space *mapping,
struct dax_device *dax_dev, struct writeback_control *wbc)
{
--
2.25.4
1 year, 10 months
[PATCH v2 01/20] dax: Modify bdev_dax_pgoff() to handle NULL bdev
by Vivek Goyal
virtiofs does not have a block device but it has dax device.
Modify bdev_dax_pgoff() to be able to handle that.
If there is no bdev, that means dax offset is 0. (It can't be a partition
block device starting at an offset in dax device).
This is little hackish. There have been discussions about getting rid
of dax not supporting partitions.
https://lore.kernel.org/linux-fsdevel/20200107125159.GA15745@infradead.org/
IMHO, this path can easily break exisitng users. For example
ioctl(BLKPG_ADD_PARTITION) will start breaking on block devices
supporting DAX. Also, I personally find it very useful to be able to
partition dax devices and still be able to use DAX.
Alternatively, I tried to store offset into dax device information in iomap
interface, but that got NACKed.
https://lore.kernel.org/linux-fsdevel/20200217133117.GB20444@infradead.org/
I can't think of a good path to solve this issue properly. So to make
progress, it seems this patch is least bad option for now and I hope
we can take it.
Signed-off-by: Stefan Hajnoczi <stefanha(a)redhat.com>
Signed-off-by: Vivek Goyal <vgoyal(a)redhat.com>
Cc: Christoph Hellwig <hch(a)infradead.org>
Cc: Dan Williams <dan.j.williams(a)intel.com>
Cc: linux-nvdimm(a)lists.01.org
---
drivers/dax/super.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 8e32345be0f7..c4bec437e88b 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -46,7 +46,8 @@ EXPORT_SYMBOL_GPL(dax_read_unlock);
int bdev_dax_pgoff(struct block_device *bdev, sector_t sector, size_t size,
pgoff_t *pgoff)
{
- phys_addr_t phys_off = (get_start_sect(bdev) + sector) * 512;
+ sector_t start_sect = bdev ? get_start_sect(bdev) : 0;
+ phys_addr_t phys_off = (start_sect + sector) * 512;
if (pgoff)
*pgoff = PHYS_PFN(phys_off);
--
2.25.4
1 year, 10 months
[PATCH] error-inject: Remove redundant has_nfit check
by Santosh Sivaraj
ndctl_bus_has_error_injection already has the ndctl_bus_has_nfit
check. Remove the redundant checks.
Signed-off-by: Santosh Sivaraj <santosh(a)fossix.org>
---
ndctl/lib/inject.c | 73 +++++++++++++++++++++-------------------------
1 file changed, 33 insertions(+), 40 deletions(-)
diff --git a/ndctl/lib/inject.c b/ndctl/lib/inject.c
index ab0bee5..509ba27 100644
--- a/ndctl/lib/inject.c
+++ b/ndctl/lib/inject.c
@@ -190,8 +190,6 @@ NDCTL_EXPORT int ndctl_namespace_inject_error2(struct ndctl_namespace *ndns,
if (!ndctl_bus_has_error_injection(bus))
return -EOPNOTSUPP;
- if (!ndctl_bus_has_nfit(bus))
- return -EOPNOTSUPP;
for (i = 0; i < count; i++) {
rc = ndctl_namespace_inject_one_error(ndns, block + i, flags);
@@ -268,8 +266,6 @@ NDCTL_EXPORT int ndctl_namespace_uninject_error2(struct ndctl_namespace *ndns,
if (!ndctl_bus_has_error_injection(bus))
return -EOPNOTSUPP;
- if (!ndctl_bus_has_nfit(bus))
- return -EOPNOTSUPP;
for (i = 0; i < count; i++) {
rc = ndctl_namespace_uninject_one_error(ndns, block + i,
@@ -450,46 +446,43 @@ NDCTL_EXPORT int ndctl_namespace_injection_status(struct ndctl_namespace *ndns)
if (!ndctl_bus_has_error_injection(bus))
return -EOPNOTSUPP;
- if (ndctl_bus_has_nfit(bus)) {
- rc = ndctl_namespace_get_injection_bounds(ndns, &ns_offset,
- &ns_size);
- if (rc)
- return rc;
+ rc = ndctl_namespace_get_injection_bounds(ndns, &ns_offset,
+ &ns_size);
+ if (rc)
+ return rc;
- cmd = ndctl_bus_cmd_new_ars_cap(bus, ns_offset, ns_size);
- rc = ndctl_cmd_submit(cmd);
- if (rc < 0) {
- dbg(ctx, "Error submitting ars_cap: %d\n", rc);
- goto out;
- }
- buf_size = ndctl_cmd_ars_cap_get_size(cmd);
- if (buf_size == 0) {
- dbg(ctx, "Got an invalid max_ars_out from ars_cap\n");
- rc = -EINVAL;
- goto out;
- }
- ndctl_cmd_unref(cmd);
+ cmd = ndctl_bus_cmd_new_ars_cap(bus, ns_offset, ns_size);
+ rc = ndctl_cmd_submit(cmd);
+ if (rc < 0) {
+ dbg(ctx, "Error submitting ars_cap: %d\n", rc);
+ goto out;
+ }
+ buf_size = ndctl_cmd_ars_cap_get_size(cmd);
+ if (buf_size == 0) {
+ dbg(ctx, "Got an invalid max_ars_out from ars_cap\n");
+ rc = -EINVAL;
+ goto out;
+ }
+ ndctl_cmd_unref(cmd);
- cmd = ndctl_bus_cmd_new_err_inj_stat(bus, buf_size);
- if (!cmd)
- return -ENOMEM;
+ cmd = ndctl_bus_cmd_new_err_inj_stat(bus, buf_size);
+ if (!cmd)
+ return -ENOMEM;
- pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
- err_inj_stat =
- (struct nd_cmd_ars_err_inj_stat *)&pkg->nd_payload[0];
+ pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+ err_inj_stat =
+ (struct nd_cmd_ars_err_inj_stat *)&pkg->nd_payload[0];
- rc = ndctl_cmd_submit(cmd);
- if (rc < 0) {
- dbg(ctx, "Error submitting command: %d\n", rc);
- goto out;
- }
- rc = injection_status_to_bb(ndns, err_inj_stat,
- ns_offset, ns_size);
- if (rc) {
- dbg(ctx, "Error converting status to badblocks: %d\n",
- rc);
- goto out;
- }
+ rc = ndctl_cmd_submit(cmd);
+ if (rc < 0) {
+ dbg(ctx, "Error submitting command: %d\n", rc);
+ goto out;
+ }
+ rc = injection_status_to_bb(ndns, err_inj_stat,
+ ns_offset, ns_size);
+ if (rc) {
+ dbg(ctx, "Error converting status to badblocks: %d\n", rc);
+ goto out;
}
out:
--
2.26.2
1 year, 10 months
[PATCH] Don't treat DSM commands as NFIT specific
by Santosh Sivaraj
The DSM commands are treated exclusively as NFIT commands, remove NFIT
dependency on using DSM commands.
Signed-off-by: Santosh Sivaraj <santosh(a)fossix.org>
---
ndctl/lib/inject.c | 6 +++---
ndctl/lib/libndctl.c | 29 ++++++++++++++++++++++-------
ndctl/lib/libndctl.sym | 4 ++++
ndctl/lib/nfit.c | 35 +++++++++--------------------------
ndctl/lib/private.h | 2 +-
ndctl/libndctl-nfit.h | 8 --------
ndctl/libndctl.h | 9 +++++++++
7 files changed, 48 insertions(+), 45 deletions(-)
diff --git a/ndctl/lib/inject.c b/ndctl/lib/inject.c
index 815f254..ab0bee5 100644
--- a/ndctl/lib/inject.c
+++ b/ndctl/lib/inject.c
@@ -26,9 +26,9 @@ NDCTL_EXPORT int ndctl_bus_has_error_injection(struct ndctl_bus *bus)
if (!bus || !ndctl_bus_has_nfit(bus))
return 0;
- if (ndctl_bus_is_nfit_cmd_supported(bus, NFIT_CMD_ARS_INJECT_SET) &&
- ndctl_bus_is_nfit_cmd_supported(bus, NFIT_CMD_ARS_INJECT_GET) &&
- ndctl_bus_is_nfit_cmd_supported(bus, NFIT_CMD_ARS_INJECT_CLEAR))
+ if (ndctl_bus_is_dsm_supported(bus, DSM_CMD_ARS_INJECT_SET) &&
+ ndctl_bus_is_dsm_supported(bus, DSM_CMD_ARS_INJECT_GET) &&
+ ndctl_bus_is_dsm_supported(bus, DSM_CMD_ARS_INJECT_CLEAR))
return 1;
return 0;
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 952192c..410fd16 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -884,12 +884,6 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
else
bus->has_of_node = 1;
- sprintf(path, "%s/device/nfit/dsm_mask", ctl_base);
- if (sysfs_read_attr(ctx, path, buf) < 0)
- bus->nfit_dsm_mask = 0;
- else
- bus->nfit_dsm_mask = strtoul(buf, NULL, 0);
-
sprintf(path, "%s/device/provider", ctl_base);
if (sysfs_read_attr(ctx, path, buf) < 0)
goto err_read;
@@ -898,12 +892,20 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
if (!bus->provider)
goto err_read;
+ sprintf(path, "%s/device/%s/dsm_mask", ctl_base,
+ ndctl_bus_has_nfit(bus) ? "nfit" : bus->provider);
+ if (sysfs_read_attr(ctx, path, buf) < 0)
+ bus->dsm_mask = 0;
+ else
+ bus->dsm_mask = strtoul(buf, NULL, 0);
+
sprintf(path, "%s/device/wait_probe", ctl_base);
bus->wait_probe_path = strdup(path);
if (!bus->wait_probe_path)
goto err_read;
- sprintf(path, "%s/device/nfit/scrub", ctl_base);
+ sprintf(path, "%s/device/%s/scrub", ctl_base,
+ ndctl_bus_has_nfit(bus) ? "nfit" : bus->provider);
bus->scrub_path = strdup(path);
if (!bus->scrub_path)
goto err_read;
@@ -1236,6 +1238,19 @@ NDCTL_EXPORT int ndctl_bus_is_cmd_supported(struct ndctl_bus *bus,
return !!(bus->cmd_mask & (1ULL << cmd));
}
+/**
+ * ndctl_bus_is_cmd_supported - ask if command is supported on @bus.
+ * @bus: ndctl_bus instance
+ * @cmd: command number (defined as NFIT_CMD_XXX in libndctl-nfit.h)
+ *
+ * Return 1: command is supported. Return 0: command is not supported.
+ *
+ */
+NDCTL_EXPORT int ndctl_bus_is_dsm_supported(struct ndctl_bus *bus, int cmd)
+{
+ return !!(bus->dsm_mask & (1ULL << cmd));
+}
+
NDCTL_EXPORT unsigned int ndctl_bus_get_revision(struct ndctl_bus *bus)
{
return bus->revision;
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 7ba1dcc..640342c 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -437,3 +437,7 @@ LIBNDCTL_24 {
ndctl_bus_is_papr_scm;
ndctl_region_has_numa;
} LIBNDCTL_23;
+
+LIBNDCTL_25 {
+ ndctl_bus_is_dsm_supported;
+} LIBNDCTL_24;
diff --git a/ndctl/lib/nfit.c b/ndctl/lib/nfit.c
index f9fbe73..be252e0 100644
--- a/ndctl/lib/nfit.c
+++ b/ndctl/lib/nfit.c
@@ -20,39 +20,22 @@ static u32 bus_get_firmware_status(struct ndctl_cmd *cmd)
struct nd_cmd_bus *cmd_bus = cmd->cmd_bus;
switch (cmd_bus->gen.nd_command) {
- case NFIT_CMD_TRANSLATE_SPA:
+ case DSM_CMD_TRANSLATE_SPA:
return cmd_bus->xlat_spa.status;
- case NFIT_CMD_ARS_INJECT_SET:
+ case DSM_CMD_ARS_INJECT_SET:
return cmd_bus->err_inj.status;
- case NFIT_CMD_ARS_INJECT_CLEAR:
+ case DSM_CMD_ARS_INJECT_CLEAR:
return cmd_bus->err_inj_clr.status;
- case NFIT_CMD_ARS_INJECT_GET:
+ case DSM_CMD_ARS_INJECT_GET:
return cmd_bus->err_inj_stat.status;
}
return -1U;
}
-/**
- * ndctl_bus_is_nfit_cmd_supported - ask nfit command is supported on @bus.
- * @bus: ndctl_bus instance
- * @cmd: nfit command number (defined as NFIT_CMD_XXX in libndctl-nfit.h)
- *
- * Return 1: command is supported. Return 0: command is not supported.
- *
- */
-NDCTL_EXPORT int ndctl_bus_is_nfit_cmd_supported(struct ndctl_bus *bus,
- int cmd)
-{
- return !!(bus->nfit_dsm_mask & (1ULL << cmd));
-}
-
static int bus_has_translate_spa(struct ndctl_bus *bus)
{
- if (!ndctl_bus_has_nfit(bus))
- return 0;
-
- return ndctl_bus_is_nfit_cmd_supported(bus, NFIT_CMD_TRANSLATE_SPA);
+ return ndctl_bus_is_dsm_supported(bus, DSM_CMD_TRANSLATE_SPA);
}
static struct ndctl_cmd *ndctl_bus_cmd_new_translate_spa(struct ndctl_bus *bus)
@@ -76,7 +59,7 @@ static struct ndctl_cmd *ndctl_bus_cmd_new_translate_spa(struct ndctl_bus *bus)
cmd->size = size;
cmd->status = 1;
pkg = &cmd->cmd_bus->gen;
- pkg->nd_command = NFIT_CMD_TRANSLATE_SPA;
+ pkg->nd_command = DSM_CMD_TRANSLATE_SPA;
pkg->nd_size_in = sizeof(unsigned long long);
pkg->nd_size_out = spa_length;
pkg->nd_fw_size = spa_length;
@@ -181,7 +164,7 @@ struct ndctl_cmd *ndctl_bus_cmd_new_err_inj(struct ndctl_bus *bus)
cmd->size = size;
cmd->status = 1;
pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
- pkg->nd_command = NFIT_CMD_ARS_INJECT_SET;
+ pkg->nd_command = DSM_CMD_ARS_INJECT_SET;
pkg->nd_size_in = offsetof(struct nd_cmd_ars_err_inj, status);
pkg->nd_size_out = cmd_length - pkg->nd_size_in;
pkg->nd_fw_size = pkg->nd_size_out;
@@ -208,7 +191,7 @@ struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_clr(struct ndctl_bus *bus)
cmd->size = size;
cmd->status = 1;
pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
- pkg->nd_command = NFIT_CMD_ARS_INJECT_CLEAR;
+ pkg->nd_command = DSM_CMD_ARS_INJECT_CLEAR;
pkg->nd_size_in = offsetof(struct nd_cmd_ars_err_inj_clr, status);
pkg->nd_size_out = cmd_length - pkg->nd_size_in;
pkg->nd_fw_size = pkg->nd_size_out;
@@ -237,7 +220,7 @@ struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_stat(struct ndctl_bus *bus,
cmd->size = size;
cmd->status = 1;
pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
- pkg->nd_command = NFIT_CMD_ARS_INJECT_GET;
+ pkg->nd_command = DSM_CMD_ARS_INJECT_GET;
pkg->nd_size_in = 0;
pkg->nd_size_out = cmd_length + buf_size;
pkg->nd_fw_size = pkg->nd_size_out;
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index c3d5fd7..a498542 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -175,7 +175,7 @@ struct ndctl_bus {
char *wait_probe_path;
char *scrub_path;
unsigned long cmd_mask;
- unsigned long nfit_dsm_mask;
+ unsigned long dsm_mask;
};
/**
diff --git a/ndctl/libndctl-nfit.h b/ndctl/libndctl-nfit.h
index 8c4f72d..9c91d4a 100644
--- a/ndctl/libndctl-nfit.h
+++ b/ndctl/libndctl-nfit.h
@@ -23,14 +23,6 @@
* libndctl-nfit.h : definitions for NFIT related commands/functions.
*/
-/* nfit command numbers which are called via ND_CMD_CALL */
-enum {
- NFIT_CMD_TRANSLATE_SPA = 5,
- NFIT_CMD_ARS_INJECT_SET = 7,
- NFIT_CMD_ARS_INJECT_CLEAR = 8,
- NFIT_CMD_ARS_INJECT_GET = 9,
-};
-
/* error number of Translate SPA by firmware */
#define ND_TRANSLATE_SPA_STATUS_INVALID_SPA 2
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index dd21e2e..6390b2a 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -128,6 +128,7 @@ struct ndctl_bus *ndctl_bus_get_by_provider(struct ndctl_ctx *ctx,
const char *provider);
const char *ndctl_bus_get_cmd_name(struct ndctl_bus *bus, int cmd);
int ndctl_bus_is_cmd_supported(struct ndctl_bus *bus, int cmd);
+int ndctl_bus_is_dsm_supported(struct ndctl_bus *bus, int cmd);
unsigned int ndctl_bus_get_revision(struct ndctl_bus *bus);
unsigned int ndctl_bus_get_id(struct ndctl_bus *bus);
const char *ndctl_bus_get_provider(struct ndctl_bus *bus);
@@ -740,6 +741,14 @@ int ndctl_dimm_master_secure_erase(struct ndctl_dimm *dimm, long key);
#define NUMA_NO_NODE (-1)
#define NUMA_NO_ATTR (-2)
+/* DSM numbers which are called via ND_CMD_CALL */
+enum {
+ DSM_CMD_TRANSLATE_SPA = 5,
+ DSM_CMD_ARS_INJECT_SET = 7,
+ DSM_CMD_ARS_INJECT_CLEAR = 8,
+ DSM_CMD_ARS_INJECT_GET = 9,
+};
+
#ifdef __cplusplus
} /* extern "C" */
#endif
--
2.26.2
1 year, 10 months
