Re: [ndctl PATCH v2 2/2] daxctl: Remove unimplemented create-device options

2024-05-31 Thread Verma, Vishal L
On Fri, 2024-05-31 at 14:29 +0800, Li Zhijian wrote: > RECONFIG_OPTIONS and ZONE_OPTIONS are not implemented for create- > device > and they will be ignored by create-device. Remove them so that the > usage > message is identical to the manual. > > Signed-off-by: Li Zhijian Reviewed-by: Vishal

Re: [ndctl PATCH v2 1/2] daxctl: Fix create-device parameters parsing

2024-05-31 Thread Verma, Vishal L
On Fri, 2024-05-31 at 14:29 +0800, Li Zhijian wrote: > Previously, the extra parameters will be ignored quietly, which is a bit > weird and confusing. > $ daxctl create-device region0 > [ >   { >     "chardev":"dax0.1", >     "size":268435456, >     "target_node":1, >     "align":2097152, >    

Re: [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned

2024-05-06 Thread Verma, Vishal L
On Fri, 2024-05-03 at 16:54 -0400, jmo...@redhat.com wrote: > From: Jeff Moyer > > Static analysis points out that the cast of bus->major and bus->minor > to a signed type in the call to parent_dev_path could result in a > negative number.  I sincerely doubt we'll see major and minor numbers >

Re: [PATCH ndctl 1/2] ndctl/keys.c: don't leak fd in error cases

2024-05-06 Thread Verma, Vishal L
On Fri, 2024-05-03 at 16:54 -0400, jmo...@redhat.com wrote: > From: Jeff Moyer > > Static analysis points out that fd is leaked in some cases.  The > change to the while loop is optional.  I only did that to make the > code consistent. > > Signed-off-by: Jeff Moyer Looks good, Reviewed-by:

[ANNOUNCE] ndctl v79

2024-05-02 Thread Verma, Vishal L
A new ndctl release is available[1]. Highlights include test and build fixes, a new cxl-wait-sanitize command, support for QOS Class in cxl-create-region, and a new cxl-set-alert-config command. A shortlog is appended below. [1]: https://github.com/pmem/ndctl/releases/tag/v79 Alison Schofield

Re: [ndctl PATCH] cxl/test: use max_available_extent in cxl-destroy-region

2024-04-22 Thread Verma, Vishal L
On Wed, 2024-03-27 at 11:46 -0700, alison.schofi...@intel.com wrote: > From: Alison Schofield > > Using .size in decoder selection can lead to a set_size failure with > these error messages: > > cxl region: create_region: region8: set_size failed: Numerical result out of > range > > []

Re: [NDCTL PATCH v2] ndctl: cxl: Remove dependency for attributes derived from IDENTIFY command

2024-04-17 Thread Verma, Vishal L
On Thu, 2024-02-15 at 09:16 -0700, Dave Jiang wrote: > A memdev may optionally not host a mailbox and therefore not able to execute > the IDENTIFY command. Currently the kernel emits empty strings for some of > the attributes instead of making them invisible in order to keep backward >

Re:

2024-04-17 Thread Verma, Vishal L
On Wed, 2024-04-17 at 02:46 -0400, Yao Xingtao wrote: > > Hi Dave, >   I have applied this patch in my env, and done a lot of testing, > this > feature is currently working fine. >   But it is not merged into master branch yet, are there any updates > on this feature? Hi Xingtao, Turns out

Re: [PATCH ndctl 2/2] daxctl/device.c: Fix error propagation in do_xaction_device()

2024-04-12 Thread Verma, Vishal L
On Fri, 2024-04-12 at 14:30 -0700, Dan Williams wrote: > Vishal Verma wrote: > > The loop through the provided list of devices in do_xaction_device() > > returns the status based on whatever the last device did. Since the > > order of processing devices, especially in cases like the 'all' keyword,

Re: [NDCTL PATCH v8 3/4] ndctl: cxl: add QoS class check for CXL region creation

2024-03-01 Thread Verma, Vishal L
On Fri, 2024-03-01 at 15:36 -0700, Dave Jiang wrote: > The CFMWS provides a QTG ID. The kernel driver creates a root decoder that > represents the CFMWS. A qos_class attribute is exported via sysfs for the root > decoder. > > One or more qos_class tokens are retrieved via QTG ID _DSM from the

Re: [ndctl PATCH] cxl/event_trace: parse arrays separately from strings

2024-02-28 Thread Verma, Vishal L
On Thu, 2024-02-15 at 22:06 -0800, alison.schofi...@intel.com wrote: > From: Alison Schofield > > Arrays are being parsed as strings based on a flag that seems like > it would be the differentiator, ARRAY and STRING, but it is not. > > libtraceevent sets the flags for arrays and strings like

Re: [NDCTL PATCH v7 3/4] ndctl: cxl: add QoS class check for CXL region creation

2024-02-23 Thread Verma, Vishal L
On Thu, 2024-02-08 at 13:11 -0700, Dave Jiang wrote: > The CFMWS provides a QTG ID. The kernel driver creates a root decoder that > represents the CFMWS. A qos_class attribute is exported via sysfs for the root > decoder. > > One or more qos_class tokens are retrieved via QTG ID _DSM from the

Re: [NDCTL PATCH v7 4/4] ndctl: add test for qos_class in CXL test suite

2024-02-23 Thread Verma, Vishal L
On Thu, 2024-02-08 at 13:11 -0700, Dave Jiang wrote: > Add tests in cxl-qos-class.sh to verify qos_class are set with the fake > qos_class create by the kernel.  Root decoders should have qos_class > attribute set. Memory devices should have ram_qos_class or pmem_qos_class > set depending on which

Re: [ndctl PATCH v7 7/7] cxl/test: add cxl-poison.sh unit test

2024-02-22 Thread Verma, Vishal L
On Wed, 2024-02-07 at 17:01 -0800, alison.schofi...@intel.com wrote: > From: Alison Schofield > > Exercise cxl list, libcxl, and driver pieces of the get poison list > pathway. Inject and clear poison using debugfs and use cxl-cli to > read the poison list by memdev and by region. > >

Re: [NDCTL PATCH v7 4/4] ndctl: add test for qos_class in CXL test suite

2024-02-21 Thread Verma, Vishal L
On Thu, 2024-02-08 at 13:11 -0700, Dave Jiang wrote: > Add tests in cxl-qos-class.sh to verify qos_class are set with the fake > qos_class create by the kernel.  Root decoders should have qos_class > attribute set. Memory devices should have ram_qos_class or pmem_qos_class > set depending on which

Re: [ndctl PATCH] cxl/test: Add 3-way HB interleave testcase to cxl-xor-region.sh

2024-02-14 Thread Verma, Vishal L
On Tue, 2024-02-13 at 23:14 -0800, alison.schofi...@intel.com wrote: > From: Alison Schofield > > cxl-xor-region.sh includes test cases for 1 & 2 way host bridge > interleaves. Add a new test case to exercise the modulo math > function the CXL driver uses to find positions in a 3-way host >

Re: [NDCTL PATCH v6 4/4] ndctl: add test for qos_class in CXL test suite

2024-02-07 Thread Verma, Vishal L
On Wed, 2024-02-07 at 10:19 -0700, Dave Jiang wrote: > Add tests in cxl-qos-class.sh to verify qos_class are set with the fake > qos_class create by the kernel.  Root decoders should have qos_class > attribute set. Memory devices should have ram_qos_class or pmem_qos_class > set depending on which

Re: [NDCTL PATCH v6 3/4] ndctl: cxl: add QoS class check for CXL region creation

2024-02-07 Thread Verma, Vishal L
On Wed, 2024-02-07 at 10:19 -0700, Dave Jiang wrote: > The CFMWS provides a QTG ID. The kernel driver creates a root decoder that > represents the CFMWS. A qos_class attribute is exported via sysfs for the root > decoder. > > One or more QoS class tokens are retrieved via QTG ID _DSM from the

Re: [NDCTL PATCH v6 1/4] ndctl: cxl: Add QoS class retrieval for the root decoder

2024-02-07 Thread Verma, Vishal L
On Wed, 2024-02-07 at 13:16 -0700, Dave Jiang wrote: > > > On 2/7/24 1:13 PM, Alison Schofield wrote: > > On Wed, Feb 07, 2024 at 12:05:17PM -0800, Alison Schofield wrote: > > > On Wed, Feb 07, 2024 at 10:19:36AM -0700, Dave Jiang wrote: > > > > Add libcxl API to retrieve the QoS class for the

Re: [PATCH ndctl] test/daxctl-create.sh: remove region and dax device assumptions

2024-01-11 Thread Verma, Vishal L
On Thu, 2024-01-11 at 11:23 -0800, Dan Williams wrote: > Vishal Verma wrote: > > The daxctl-create.sh test had some hard-coded assumptions about what dax > > device it expects to find, and what region number it will be under. This > > usually worked when the unit test environment only had

Re: [PATCH ndctl RESEND 1/2] ndctl/test: Add destroy region test

2023-12-05 Thread Verma, Vishal L
On Tue, 2023-12-05 at 13:22 -0800, Ira Weiny wrote: > Verma, Vishal L wrote: > > [snip] > > > > > > > > > > Correct, the set -e will cause the script to abort with an error exit > > code whenever a command fails. > > > > I do w

Re: [PATCH ndctl RESEND 1/2] ndctl/test: Add destroy region test

2023-12-04 Thread Verma, Vishal L
On Mon, 2023-12-04 at 10:05 -0800, Ira Weiny wrote: > Alison Schofield wrote: > > On Thu, Nov 30, 2023 at 08:06:13PM -0800, Ira Weiny wrote: > > [snip] > > > > + > > > +check_destroy_devdax() > > > +{ > > > +   mem=$1 > > > +   decoder=$2 > > > + > > > +   region=$($CXL create-region

Re: [ndctl PATCH 3/3] cxl/test: use an explicit --since time in journalctl

2023-11-28 Thread Verma, Vishal L
On Mon, 2023-11-27 at 20:11 -0800, alison.schofi...@intel.com wrote: > From: Alison Schofield > > Using the bash variable 'SECONDS' plus 1 for searching the > dmesg log sometimes led to one test picking up error messages > from the previous test when run as a suite. SECONDS alone may > miss some

Re: [ndctl PATCH 2/3] cxl/test: add a cxl_ derivative of check_dmesg()

2023-11-28 Thread Verma, Vishal L
On Mon, 2023-11-27 at 20:11 -0800, alison.schofi...@intel.com wrote: > From: Alison Schofield > > check_dmesg() is used by CXL unit tests as well as by a few > DAX unit tests. Add a cxl_check_dmesg() version that can be > expanded for CXL special checks like this: > > Add a check for an

Re: [ndctl PATCH 1/3] cxl/test: add and use cxl_common_[start|stop] helpers

2023-11-28 Thread Verma, Vishal L
On Mon, 2023-11-27 at 20:11 -0800, alison.schofi...@intel.com wrote: > From: Alison Schofield > > CXL unit tests use a mostly common set of commands to setup and tear down > their test environments. Standardize on a common set and make all unit > tests that run as part of the CXL suite use the

Re: [NDCTL PATCH 2/2] cxl: Add check for regions before disabling memdev

2023-11-28 Thread Verma, Vishal L
On Tue, 2023-11-28 at 14:35 +0800, Cao, Quanquan/曹 全全 wrote: > > > Add a check for memdev disable to see if there are active regions present > > before disabling the device. This is necessary now regions are present to > > fulfill the TODO that was left there. The best way to determine if a > >

Re: [NDCTL PATCH v3] cxl/region: Add -f option for disable-region

2023-11-27 Thread Verma, Vishal L
On Mon, 2023-11-27 at 10:13 -0700, Dave Jiang wrote: > On 11/27/23 02:34, Cao, Quanquan/曹 全全 wrote: > > > > > > 1.Assuming the user hasn't executed the 'cxl disable-region > > region0' command and directly runs 'cxl destroy-region region0 -f', > > using the 'disable_region(region)' function to

Re: [ndctl PATCH] cxl/test: replace a bad root decoder usage in cxl-xor-region.sh

2023-11-22 Thread Verma, Vishal L
On Tue, 2023-11-21 at 18:57 -0800, alison.schofi...@intel.com wrote: > From: Alison Schofield > > The 4-way XOR region as defined in this test uses a root decoder that > is created using an improperly defined CFMWS. The problem with the > CFMWS is that Host Bridges repeat in the target list like

Re: [ndctl PATCH v3 5/5] cxl/test: add cxl-poison.sh unit test

2023-11-17 Thread Verma, Vishal L
On Fri, 2023-11-17 at 14:35 -0800, alison.schofi...@intel.com wrote: > From: Alison Schofield > [..] Rest of the series is looking good, just a few minor things below. > > + > +find_media_errors() > +{ > +   local json="$1" > + > +   nr="$(jq -r ".nr_records" <<< "$json")" > +   

Re: [ndctl PATCH v2 5/5] cxl/test: add cxl-poison.sh unit test

2023-11-15 Thread Verma, Vishal L
On Sun, 2023-10-01 at 15:31 -0700, alison.schofi...@intel.com wrote: > From: Alison Schofield > > Exercise cxl list, libcxl, and driver pieces of the get poison list > pathway. Inject and clear poison using debugfs and use cxl-cli to > read the poison list by memdev and by region. > >

Re: [ndctl PATCH v2 3/5] cxl/list: collect and parse the poison list records

2023-11-15 Thread Verma, Vishal L
On Sun, 2023-10-01 at 15:31 -0700, alison.schofi...@intel.com wrote: > From: Alison Schofield > > Poison list records are logged as events in the kernel tracing > subsystem. To prepare the poison list for cxl list, enable tracing, > trigger the poison list read, and parse the generated

Re: [ndctl PATCH v2 1/5] libcxl: add interfaces for GET_POISON_LIST mailbox commands

2023-11-15 Thread Verma, Vishal L
On Sun, 2023-10-01 at 15:31 -0700, alison.schofi...@intel.com wrote: > From: Alison Schofield > > CXL devices maintain a list of locations that are poisoned or result > in poison if the addresses are accessed by the host. > > Per the spec (CXL 3.0 8.2.9.8.4.1), the device returns the Poison >

Re: [NDCTL PATCH] cxl: Augment documentation on cxl operational behavior

2023-11-14 Thread Verma, Vishal L
On Tue, 2023-10-31 at 13:33 -0700, Dave Jiang wrote: I think a more specific subject like: cxl/Documentation: Clarify that no-op is a success for xable commands is better? > If a cxl operation is executed resulting in no-op, the tool will still .. cxl enable or disable operation .. > emit

Re: [ISSUE] `cxl disable-region region0` twice but got same output

2023-10-31 Thread Verma, Vishal L
On Mon, 2023-10-30 at 14:41 +0800, Cao, Quanquan/曹 全全 wrote: > [..] > After investigation, it was found that when disabling the region and > attempting to disable the same region again, the message "cxl region: > cmd_disable_region: disabled 1 region" is still returned. > I consider this to be

Re: [PATCH] dax: remove unnecessary check

2023-10-05 Thread Verma, Vishal L
On Thu, 2023-10-05 at 16:58 +0300, Dan Carpenter wrote: > We know that "rc" is zero so there is no need to check. > > Signed-off-by: Dan Carpenter > --- >  drivers/dax/bus.c | 2 +- >  1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c > index

Re: [NDCTL PATCH] cxl/region: Add -f option for disable-region

2023-09-20 Thread Verma, Vishal L
Hi Dave, Looks good apart from a couple minor nits - On Tue, 2023-09-19 at 16:52 -0700, Dave Jiang wrote: > The current operation for disable_region does not check if the memory > covered by a region is online before attempting to disable the cxl region. > Provide a -f option for the region to

[ANNOUNCE] ndctl v78

2023-08-03 Thread Verma, Vishal L
A new ndctl release is available[1]. This release incorporates functionality up to the 6.5 kernel. Highlights include fixes to cxl-monitor and ndctl-monitor, a new cxl-update-firmware command, various documentation fixes, and a new unit test for cxl events. A shortlog is appended below. [1]:

Re: [PATCH ndctl v2] ndctl/cxl/test: Add CXL event test

2023-07-31 Thread Verma, Vishal L
On Mon, 2023-07-31 at 16:53 -0700, Ira Weiny wrote: > Previously CXL event testing was run by hand.  This reduces testing Reduces or increases / improves? Or did you mean running by hand reduced coverage. Maybe this can read "Improve testing coverage and address a lack of automated regression

Re: [PATCH ndctl] cxl/memdev: initialize 'rc' in action_update_fw()

2023-07-31 Thread Verma, Vishal L
On Mon, 2023-07-31 at 14:18 -0600, Vishal Verma wrote: > Static analysis complains that in some cases, an uninitialized 'rc' can > get returned from action_update_fw(). Since this can only happen in a > 'no-op' case, initialize rc to 0. > > Signed-off-by: Vishal Verma This should've included a

Re: [PATCH ndctl] ndctl/cxl/test: Add CXL event test

2023-07-31 Thread Verma, Vishal L
On Thu, 2023-07-27 at 14:21 -0700, Ira Weiny wrote: > Previously CXL event testing was run by hand.  This reduces testing > coverage including a lack of regression testing. > > Add a CXL test as part of the meson test infrastructure.  Passing is > predicated on receiving the appropriate number of

Re: [ndctl PATCH RESEND 2/2] libcxl: Fix accessors for temperature field to support negative value

2023-07-31 Thread Verma, Vishal L
On Mon, 2023-07-31 at 12:17 +0900, Jehoon Park wrote: > On Mon, Jul 24, 2023 at 09:08:21PM +0000, Verma, Vishal L wrote: > > On Mon, 2023-07-17 at 15:29 +0900, Jehoon Park wrote: > > > Add a new macro function to retrieve a signed value such as a temperature. > > > Repl

Re: [ndctl PATCH 2/2] cxl: add 'set-alert-config' command to cxl tool

2023-07-24 Thread Verma, Vishal L
Hi Jehoon, Thanks for adding this. A few minor comments below, otherwise these look good. On Tue, 2023-07-11 at 16:10 +0900, Jehoon Park wrote: > Add a new command: 'set-alert-config', which configures device's warning alert > >  usage: cxl set-alert-config [..] [] > >     -v, --verbose   

Re: [NDCTL PATCH] cxl/region: Always use the correct target position

2023-07-24 Thread Verma, Vishal L
On Sun, 2023-07-09 at 19:50 +0800, Xiao Yang wrote: > Hi all, > > Kindly ping. > > This patch can only fixes the case that 2 way interleave is enabled > across 2 CXL host bridges and each host bridge has 1 CXL Root Port. > PS: In other word, this patch is wrong when 2 way interleave is enabled

Re: [ndctl PATCH RESEND 2/2] libcxl: Fix accessors for temperature field to support negative value

2023-07-24 Thread Verma, Vishal L
On Mon, 2023-07-17 at 15:29 +0900, Jehoon Park wrote: > Add a new macro function to retrieve a signed value such as a temperature. > Replace indistinguishable error numbers with debug message. > > Signed-off-by: Jehoon Park > --- >  cxl/lib/libcxl.c | 36 ++-- >  1

Re: [ndctl PATCH v3 1/6] cxl/monitor: Enable default_log and refactor sanity check

2023-07-10 Thread Verma, Vishal L
On Mon, 2023-07-10 at 10:53 +, Zhijian Li (Fujitsu) wrote: > > > > > >  log_init(, "cxl/monitor", > > > "CXL_MONITOR_LOG"); > > > -   monitor.ctx.log_fn = log_standard; > > > +   if (monitor.log) > > > +   log = monitor.log; > > > +   else > > > +   

Re: [ndctl PATCH v3 0/6] cxl/monitor and ndctl/monitor fixes

2023-07-05 Thread Verma, Vishal L
On Tue, 2023-06-27 at 10:17 +, Zhijian Li (Fujitsu) wrote: > kindly ping > > It has been almost a month, and it's doing some bugfix. The progress > is a little bit slow anyway :) > Hi Li, Sorry for the delay - the patches are mostly fine with the exception of a few things I pointed out.

Re: [ndctl PATCH v3 4/6] cxl/monitor: always log started message

2023-07-05 Thread Verma, Vishal L
On Wed, 2023-05-31 at 10:19 +0800, Li Zhijian wrote: > Tell people monitor is starting rather only daemon mode will log this > message before. It's a minor fix so that whatever stdout or logfile > is specified, people can see a *normal* message. > > After this patch >  # cxl monitor >  cxl

Re: [ndctl PATCH v3 3/6] cxl/monitor: use strcmp to compare the reserved word

2023-07-05 Thread Verma, Vishal L
On Wed, 2023-05-31 at 10:19 +0800, Li Zhijian wrote: > According to the tool's documentation, when '-l standard' is > specified, > log would be output to the stdout. But since it's using strncmp(a, b, > 10) > to compare the former 10 characters, it will also wrongly detect a > filename > starting

Re: [ndctl PATCH v3 1/6] cxl/monitor: Enable default_log and refactor sanity check

2023-07-05 Thread Verma, Vishal L
On Wed, 2023-05-31 at 10:19 +0800, Li Zhijian wrote: > The default_log(/var/log/cxl-monitor.log) should be used when no '-l' > argument is specified in daemon mode, but it was not working at all. > > Here we assigned it a default log per its arguments, and simplify the > sanity check so that it

Re: ndctl-v77: LIBCXL_5 not found

2023-05-09 Thread Verma, Vishal L
On Wed, 2023-05-10 at 00:30 +0900, Minwoo Im wrote: > Hello ndctl, > > With the recent tag (v77), after building it with the following > commands, `cxl` command is not albe to find `LIBCXL_5` version from the > /lib/libcxl.so.1 installed. > > meson build > ninja -C build >    

[ANNOUNCE] ndctl v77

2023-05-02 Thread Verma, Vishal L
A new ndctl release is available[1]. This release incorporates functionality up to the 6.3 kernel. Highlights include some build fixes around cx-monitor and event tracing, the ability to create ram type regions, some fixes to create-region to allow a user-supplied UUID, cxl-list fixes for RCD

Re: [PATCH ndctl 4/5] cxl: add an update-firmware command

2023-04-24 Thread Verma, Vishal L
On Fri, 2023-04-21 at 21:10 -0600, Vishal Verma wrote: > Add a new cxl-update-firmware command to initiate a firmware update on a > given memdev. This allows using a specified file to pass in as the > firmware binary for one or more memdevs, allows for a blocking mode, > where the command only

[ANNOUNCE] ndctl v76

2023-02-22 Thread Verma, Vishal L
A new ndctl release is available[1]. Highlights include the new cxl-monitor command that uses CXL trace events, fix for a long-standing off-by-one in memblock enumeration in libdaxctl, another daxctl fix to be tolerant of new sysfs attributes starting with 'memory_', that are not regular

Re: [PATCH ndctl 2/3] cxl/monitor: retain error code in monitor_event()

2023-02-21 Thread Verma, Vishal L
On Tue, 2023-02-21 at 17:56 -0800, Ira Weiny wrote: > Vishal Verma wrote: > > Static analysis reports that the error unwinding path in monitor_event() > > overwrites 'rc' with the return from cxl_event_tracing_disable(). This > > masks the actual error code from either epoll_wait() or > >

Re: [PATCH ndctl 3/3] test/cxl-security.sh: avoid intermittent failures due to sasync probe

2023-02-21 Thread Verma, Vishal L
On Tue, 2023-02-21 at 09:22 -0800, Alison Schofield wrote: > On Fri, Feb 17, 2023 at 05:40:24PM -0700, Vishal Verma wrote: > > This test failed intermittently because the ndctl-list operation right > > after a 'modprobe cxl_test' could race the actual nmem devices getting > > loaded. > > > >

Re: [ndctl PATCH 2/4 v3] libndctl/msft: Replace nonsense NDN_MSFT_CMD_SMART command

2023-02-15 Thread Verma, Vishal L
On Wed, 2023-02-15 at 11:49 -0500, Alexander Motin wrote: > There is no NDN_MSFT_CMD_SMART command.  There are 3 relevant ones, > reporting different aspects of the module health.  Define those and > use NDN_MSFT_CMD_NHEALTH, while making the code more universal to > allow use of others later. >

Re: [ndctl PATCH 2/2 v2 RESEND] libndctl/msft: Improve "smart" state reporting

2023-02-14 Thread Verma, Vishal L
[Dropping Lijun from CC due to a bounce] On Fri, 2023-01-13 at 12:27 -0500, Alexander Motin wrote: > Previous code reported "smart" state based on number of bits > set in the module health field.  But actually any single bit > set there already means critical failure.  Rework the logic >

Re: [ndctl PATCH 1/2 v2 RESEND] libndctl/msft: Cleanup the code

2023-02-14 Thread Verma, Vishal L
On Fri, 2023-01-13 at 12:27 -0500, Alexander Motin wrote: > Clean up the code, making it more uniform with others and > allowing more methods to be implemented later: >  - remove nonsense NDN_MSFT_CMD_SMART definition, replacing it > with real commands, primarity NDN_MSFT_CMD_NHEALTH; >  - allow

Re: [ndctl PATCH] daxctl: Skip over memory failure node status

2023-02-14 Thread Verma, Vishal L
On Mon, 2023-02-13 at 21:39 +, Adam Manzanares wrote: > When trying to match a dax device to a memblock physical address > memblock_in_dev will fail if the the phys_index sysfs file does > not exist in the memblock. Currently the memory failure directory > associated with a node is currently

Re: [PATCH ndctl 3/7] cxl: add core plumbing for creation of ram regions

2023-02-09 Thread Verma, Vishal L
On Fri, 2023-02-10 at 01:04 +, Fan Ni wrote: > On Tue, Feb 07, 2023 at 12:16:29PM -0700, Vishal Verma wrote: > > Add support in libcxl to create ram regions through a new > > cxl_decoder_create_ram_region() API, which works similarly to its pmem > > sibling. > > > > Enable ram region creation

Re: [PATCH ndctl v2 0/7] cxl: add support for listing and creating volatile regions

2023-02-09 Thread Verma, Vishal L
On Thu, 2023-02-09 at 12:04 +0100, Brice Goglin wrote: > > Le 08/02/2023 à 21:00, Vishal Verma a écrit : > > While enumeration of ram type regions already works in libcxl and > > cxl-cli, it lacked an attribute to indicate pmem vs. ram. Add a new > > 'type' attribute to region listings to address

Re: [PATCH ndctl 7/7] cxl/list: Enumerate device-dax properties for regions

2023-02-07 Thread Verma, Vishal L
On Tue, 2023-02-07 at 22:00 -0800, Dan Williams wrote: > Vishal Verma wrote: > > From: Dan Williams > > > > Recently the kernel added support for routing newly mapped CXL regions to > > device-dax. Include the json representation of a DAX region beneath its > > host CXL region. > > > >

Re: [PATCH ndctl 5/7] cxl/region: determine region type based on root decoder capability

2023-02-07 Thread Verma, Vishal L
On Tue, 2023-02-07 at 21:55 -0800, Dan Williams wrote: > Vishal Verma wrote: > > <..>  > > +static void set_type_from_decoder(struct cxl_ctx *ctx, struct > > parsed_params *p) > > +{ > > +   int num_cap = 0; > > + > > +   /* if param.type was explicitly specified, nothing to do here */ >

Re: [PATCH ndctl 5/7] cxl/region: determine region type based on root decoder capability

2023-02-07 Thread Verma, Vishal L
On Tue, 2023-02-07 at 20:07 -0800, Ira Weiny wrote: > Vishal Verma wrote: > > > > diff --git a/cxl/region.c b/cxl/region.c > > index 9079b2d..1c8ccc7 100644 > > --- a/cxl/region.c > > +++ b/cxl/region.c > > @@ -448,6 +448,31 @@ static int validate_decoder(struct cxl_decoder > > *decoder, > >

Re: [PATCH ndctl 3/7] cxl: add core plumbing for creation of ram regions

2023-02-07 Thread Verma, Vishal L
On Tue, 2023-02-07 at 19:55 -0800, Ira Weiny wrote: > Vishal Verma wrote: <..> > > > > diff --git a/cxl/region.c b/cxl/region.c > > index 38aa142..0945a14 100644 > > --- a/cxl/region.c > > +++ b/cxl/region.c > > @@ -380,7 +380,22 @@ static void collect_minsize(struct cxl_ctx *ctx, > > struct

Re: [PATCH ndctl 2/7] cxl: add a type attribute to region listings

2023-02-07 Thread Verma, Vishal L
On Tue, 2023-02-07 at 21:47 -0800, Dan Williams wrote: > Vishal Verma wrote: <..> > > > > @@ -853,6 +854,10 @@ struct json_object *util_cxl_region_to_json(struct > > cxl_region *region, > > json_object_object_add(jregion, "size", jobj); > > } > >   > > +   

Re: [PATCH ndctl 1/7] cxl/region: skip region_actions for region creation

2023-02-07 Thread Verma, Vishal L
On Tue, 2023-02-07 at 22:07 +, Fan Ni wrote: > On Tue, Feb 07, 2023 at 12:16:27PM -0700, Vishal Verma wrote: > > Commit 3d6cd829ec08 ("cxl/region: Use cxl_filter_walk() to gather > > create-region targets") > > removed the early return for create-region, and this caused a > > create-region

Re: [PATCH] daxctl: Fix memblock enumeration off-by-one

2023-02-03 Thread Verma, Vishal L
On Thu, 2023-02-02 at 12:56 -0800, Dan Williams wrote: > A memblock is an inclusive memory range. Bound the search by the last > address in the memory block. > > Found by wondering why an offline 32-block (at 128MB == 4GB) range was > reported as 33 blocks with one online. > > Signed-off-by: Dan

[ANNOUNCE] ndctl v75

2023-01-12 Thread Verma, Vishal L
A new ndctl release is available[1]. Highlights include cxl region management usability improvements, further cxl-list reworks and enhancements, support for RCH topologies, a unit test for XOR arithmetic based regions, a new cxl-security unit test, support for master-passphrase removal, retrieval

Re: [ndctl PATCH v2 07/18] cxl/list: Add parent_dport attribute to port listings

2023-01-04 Thread Verma, Vishal L
On Fri, 2022-12-16 at 17:36 -0800, Dan Williams wrote: > Dan Williams wrote: > > > > --- > >  cxl/json.c |    8 > >  cxl/lib/libcxl.c   |   38 ++ > >  cxl/lib/libcxl.sym |    1 + > >  cxl/lib/private.h  |    2 ++ > >  cxl/libcxl.h   |    1

Re: [ndctl PATCH 1/2 v2] libndctl/msft: Cleanup the code

2022-12-13 Thread Verma, Vishal L
On Thu, 2022-11-17 at 17:37 -0500, Alexander Motin wrote: > Clean up the code, making it more uniform with others and > allowing more methods to be implemented later. Hi Alexander, I think this would be easier to review if the the cleanups were summarized in the commit message - and if there are

Re: [PATCH 4/4] ndctl/test: Add CXL test for security

2022-12-13 Thread Verma, Vishal L
On Wed, 2022-09-21 at 14:02 -0700, Dave Jiang wrote: > Create security-cxl.sh based off of security.sh for nfit security testing. > The test will test a cxl_test based security commands enabling through > nvdimm. Since the new test is largely a copy of the NFIT security test, I think it might be

Re: [PATCH 3/4] ndctl/libndctl: Add retrieving of unique_id for cxl mem dev

2022-12-13 Thread Verma, Vishal L
Also s/cxl/CXL/On Wed, 2022-09-21 at 14:02 -0700, Dave Jiang wrote: > Subject: [PATCH 3/4] ndctl/libndctl: Add retrieving of unique_id for cxl mem > dev "Allow retrieving" or just "Retrieve unique_id for.." > With bus_prefix, retrieve the unique_id of cxl mem device. This will > allow

Re: [PATCH 2/4] ndctl/libndctl: Add bus_prefix for cxl

2022-12-13 Thread Verma, Vishal L
On Wed, 2022-09-21 at 14:02 -0700, Dave Jiang wrote: > With support of being able to detect the cxl bus, setup the bus_prefix > for cxl bus. Same comment as patch 1 about 'CXL'. This might also be reworded as: When the 'ndbus' is backed by CXL, set up the bus_prefix for the dimm object

Re: [PATCH 1/4] ndctl: add cxl bus detection

2022-12-13 Thread Verma, Vishal L
On Wed, 2022-09-21 at 14:02 -0700, Dave Jiang wrote: > Add helper function to detect that the bus is cxl based. Capitalize CXL in subject and here. This can probably be "Add a CXL bus type, and detect whether a 'dimm' is backed by the CXL subsystem" Otherwise looks good. > > Signed-off-by:

Re: [ndctl PATCH v2 15/18] cxl/Documentation: Fix whitespace typos in create-region man page

2022-12-13 Thread Verma, Vishal L
On Fri, 2022-12-09 at 10:06 -0800, Dan Williams wrote: > Alison Schofield wrote: > > On Thu, Dec 08, 2022 at 01:29:26PM -0800, Dan Williams wrote: > > > > Missing SOB and commit log > > Whoops, moved too fast and broke things. > > Vishal, feel free to add: No problem, done. Patch 7 was missing

Re: [ndctl PATCH v2 04/18] ndctl/clang-format: Fix space after for_each macros

2022-12-12 Thread Verma, Vishal L
On Fri, 2022-12-09 at 09:22 -0800, Alison Schofield wrote: > On Thu, Dec 08, 2022 at 01:28:21PM -0800, Dan Williams wrote: > > Copy the approach taken in the kernel via: > > > > commit 781121a7f6d1 ("clang-format: Fix space after for_each > > macros") > > On a related note -

Re: [ndctl PATCH v2] ndctl: create disable master passphrase support

2022-12-08 Thread Verma, Vishal L
On Thu, 2022-12-08 at 09:58 -0700, Dave Jiang wrote: > The cxl spec supports disabling of master passphrase. This is a new command > that previously was not supported through nvdimm. Add support command to > support "master passhprase disable". > > Signed-off-by: Dave Jiang > --- > > v2: > -

Re: [ndctl PATCH v2] ndctl: create disable master passphrase support

2022-12-08 Thread Verma, Vishal L
On Thu, 2022-12-08 at 09:58 -0700, Dave Jiang wrote: > The cxl spec supports disabling of master passphrase. This is a new command > that previously was not supported through nvdimm. Add support command to > support "master passhprase disable". Extra 'support' in this sentence? > >

Re: [PATCH] ndctl: create disable master passphrase support

2022-12-07 Thread Verma, Vishal L
On Wed, 2022-09-21 at 13:58 -0700, Dave Jiang wrote: > The cxl spec supports disabling of master passphrase. This is a new command > that previously was not supported through nvdimm. Add support command to > support "master passhprase disable". > > Signed-off-by: Dave Jiang > --- >  

Re: [ndctl PATCH 14/15] cxl/test: Extend cxl-topology.sh for a single root-port host-bridge

2022-11-08 Thread Verma, Vishal L
On Sun, 2022-11-06 at 15:48 -0800, Dan Williams wrote: > A recent extension of cxl_test adds 2 memory devices attached through a > switch to a single ported host-bridge to reproduce a bug report. > > Reported-by: Jonathan Cameron > Link:

Re: [ndctl PATCH 09/15] cxl/region: Make ways an integer argument

2022-11-08 Thread Verma, Vishal L
On Sun, 2022-11-06 at 15:47 -0800, Dan Williams wrote: > Since --ways does not take a unit value like --size, just make it an > integer argument directly and skip the hand coded conversion. Ah I had gone back and forth between using an int vs. unsigned. I had gone with unsigned because the kernel

Re: [ndctl PATCH 11/15] cxl/region: Use cxl_filter_walk() to gather create-region targets

2022-11-08 Thread Verma, Vishal L
On Sun, 2022-11-06 at 15:47 -0800, Dan Williams wrote: > > The core of 'cxl list' knows, among other things, how to filter memdevs by > > their connectivity to a root decoder, enabled status, and how to identify > > memdevs by name, id, serial number. Use the fact that the json-c object > > array

Re: [PATCH v4 0/2]ndctl/namespace: Fix and improve write-infoblock

2022-09-12 Thread Verma, Vishal L
On Fri, 2022-05-27 at 16:00 +0530, Tarun Sahu wrote: > This series resolves some issues with write-infoblock > command and provide support to write-infoblock for sector > mode namespace > > write-infoblock command has issues regarding updating the > align, uuid, parent_uuid. In case of no

Re: [ndctl PATCH] cxl/test: Use interleave arithmetic to sort memdevs for a region

2022-08-24 Thread Verma, Vishal L
On Wed, 2022-08-24 at 16:09 -0700, alison.schofi...@intel.com wrote: > From: Alison Schofield > > Test cxl-region-sysfs.sh assumes Modulo arithmetic. XOR arithmetic > is being introduced and requires a different ordering of the memdevs > in the region. Instead of 'is being introduced', maybe

[ANNOUNCE] ndctl v74

2022-08-24 Thread Verma, Vishal L
A new ndctl release is available[1]. Highlights include CXL region management, enhancements to cxl-list, cxl_test based unit tests for topology enumeration, and region and label operations, misc build fixes, iniparser include resolution, fixes in config parsing for ndctl-monitor, and misc

Re: [ndctl PATCH 1/3] cxl/region: fix a dereferecnce after NULL check

2022-08-23 Thread Verma, Vishal L
On Tue, 2022-08-23 at 01:21 -0600, Vishal Verma wrote: > A NULL check in region_action() implies that 'decoder' might be NULL, but > later we dereference it during cxl_decoder_foreach(). > > Since cxl_decoder_foreach() won't ever enter the loop with a NULL decoder, > the check was superfluous.

Re: [ndctl PATCH] meson.build: be specific for library path

2022-08-19 Thread Verma, Vishal L
On Wed, 2022-08-17 at 18:23 -0700, Luis Chamberlain wrote: > If you run the typical configure script on a typical linux software > project say with ./configure --prefix=/usr/ then the libdir defaults > to /usr/lib/ however this is not true with meson. > > With meson the current libdir path

Re: [ndctl PATCH v2 10/10] cxl/decoder: add a max_available_extent attribute

2022-08-11 Thread Verma, Vishal L
On Thu, 2022-08-11 at 13:22 -0700, Dan Williams wrote: > Vishal Verma wrote: > > Add a max_available_extent attribute to cxl_decoder. In order to aid in > > its calculation, change the order of regions in the root decoder's list > > to be sorted by start HPA of the region. > > > > Additionally,

Re: [ndctl PATCH v2 06/10] cxl: add a 'create-region' command

2022-08-11 Thread Verma, Vishal L
On Thu, 2022-08-11 at 12:34 -0700, Dan Williams wrote: > Vishal Verma wrote: > > Add a 'create-region' command to cxl-cli that walks the platform's CXL > > hierarchy to find an appropriate root decoder based on any options > > provided, and uses libcxl APIs to create a 'region' that is

Re: [ndctl PATCH v2 05/10] libcxl: add low level APIs for region creation

2022-08-11 Thread Verma, Vishal L
On Thu, 2022-08-11 at 11:42 -0700, Dan Williams wrote: > Verma, Vishal L wrote: > > On Wed, 2022-08-10 at 20:05 -0700, Dan Williams wrote: > > > > > > [snip] > > > > > > > > > +CXL_EXPORT struct cxl_region * > > >

Re: [ndctl PATCH v2 05/10] libcxl: add low level APIs for region creation

2022-08-10 Thread Verma, Vishal L
On Wed, 2022-08-10 at 20:05 -0700, Dan Williams wrote: > Vishal Verma wrote: > > Add libcxl APIs to create a region under a given root decoder, and to > > set different attributes for the new region. These allow setting the > > size, interleave_ways, interleave_granularity, uuid, and the target >

Re: [PATCH] libcxl: Fix memory leakage in cxl_port_init()

2022-07-19 Thread Verma, Vishal L
On Mon, 2022-07-18 at 13:53 +0530, Shivaprasad G Bhat wrote: > The local variable 'path' is not freed in cxl_port_init() for success > case. > The patch fixes that. > > Signed-off-by: Shivaprasad G Bhat > --- >  cxl/lib/libcxl.c |    1 + >  1 file changed, 1 insertion(+) > > diff --git

Re: [ndctl PATCH v2 12/12] cxl/test: Checkout region setup/teardown

2022-07-14 Thread Verma, Vishal L
On Thu, 2022-07-14 at 14:13 -0700, Dan Williams wrote: > Verma, Vishal L wrote: > > > > > > +# grab the list of memdevs grouped by host-bridge interleave position > > > +port_dev0=$($CXL list -T -d $decoder | jq -r ".[] | > > > +   .targ

Re: [ndctl PATCH v2 12/12] cxl/test: Checkout region setup/teardown

2022-07-14 Thread Verma, Vishal L
On Thu, 2022-07-14 at 10:02 -0700, Dan Williams wrote: > Exercise the fundamental region provisioning sysfs mechanisms of discovering > available DPA capacity, allocating DPA to a region, and programming HDM > decoders. > > Signed-off-by: Dan Williams > --- >  test/cxl-region-sysfs.sh |  122 >

Re: [ndctl PATCH] cxl/test: add a test to {read,write,zero}-labels

2022-07-13 Thread Verma, Vishal L
On Wed, 2022-07-13 at 11:50 -0700, Davidlohr Bueso wrote: > On Wed, 13 Jul 2022, Vishal Verma wrote: > > > Add a unit test to test writing, reading, and zeroing LSA aread for > > cxl_test based memdevs using ndctl commands. > > > > Cc: Dan Williams > > Signed-off-by: Vishal Verma > > --- > >

Re: [ndctl PATCH 09/11] cxl/memdev: Add {reserve,free}-dpa commands

2022-07-13 Thread Verma, Vishal L
On Wed, 2022-07-13 at 08:22 -0700, Dan Williams wrote: > Verma, Vishal L wrote: > > On Tue, 2022-07-12 at 12:08 -0700, Dan Williams wrote: > > > > > > > > + > > > +   if (cxl_decoder_get_mode(target) != mode) { > > > +   

Re: [ndctl PATCH] util/wrapper.c: Fix gcc warning in xrealloc()

2022-06-16 Thread Verma, Vishal L
On Thu, 2022-06-16 at 13:30 -0700, Dan Williams wrote: > Vishal Verma wrote: > > A GCC update (12.1.1) now produces a warning in the xrealloc() > > wrapper > > (originally copied from git, and used in strbuf operations): > > > >   ../util/wrapper.c: In function ‘xrealloc’: > >  

Re: [PATCH] nvdimm: Fix badblocks clear off-by-one error

2022-06-01 Thread Verma, Vishal L
On Tue, 2022-05-31 at 17:09 -0700, Dan Williams wrote: > From: Chris Ye > > nvdimm_clear_badblocks_region() validates badblock clearing requests > against the span of the region, however it compares the inclusive > badblock request range to the exclusive region range. Fix up the > off-by-one

Re: [RFC PATCH 12/15] cxl/region: Add region creation ABI

2022-05-04 Thread Verma, Vishal L
On Wed, 2022-04-13 at 11:37 -0700, Ben Widawsky wrote: > Regions are created as a child of the decoder that encompasses an > address space with constraints. Regions have a number of attributes that > must be configured before the region can be activated. > > Multiple processes which are trying

  1   2   >