Re: [PATCH v2 2/2] acpi: nfit: Add support for hot-add

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Wed, 2015-10-14 at 17:04 -0600, Vishal Verma wrote:
> Add a .notify callback to the acpi_nfit_driver that gets called on a
> hotplug event. From this, evaluate the _FIT ACPI method which returns
> the updated NFIT with handles for the hot-plugged NVDIMM.
> 
> Iterate over the new NFIT, and add any new tables found, and
> register/enable the corresponding regions.
> 
> In the nfit test framework, after normal initialization, update the
> NFIT
> with a new hot-plugged NVDIMM, and directly call into the driver to
> update its view of the available regions.
> 
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Toshi Kani <toshi.kani@hpe.com>
> Cc: Elliott, Robert <elliott@hpe.com>
> Cc: <linux-acpi@vger.kernel.org>
> Cc: <linux-nvdimm@lists.01.org>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  drivers/acpi/nfit.c              | 197 +++++++++++++++++++++++++++++
> ----------
>  drivers/acpi/nfit.h              |   2 +
>  tools/testing/nvdimm/test/nfit.c | 156
> ++++++++++++++++++++++++++++++-
>  3 files changed, 304 insertions(+), 51 deletions(-)
> 

<snip>

> diff --git a/tools/testing/nvdimm/test/nfit.c
> b/tools/testing/nvdimm/test/nfit.c
> index 021e6f9..d803e68 100644
> --- a/tools/testing/nvdimm/test/nfit.c
> +++ b/tools/testing/nvdimm/test/nfit.c

<snip>

>  
> +	if (t->setup_hotplug) {
> +		offset = offset + sizeof(struct acpi_nfit_flush_address) * 4;
> +		/* dcr-descriptor4 */
> +		dcr = nfit_buf + offset;
> +		dcr->header.type = ACPI_NFIT_TYPE_CONTROL_REGION;
> +		dcr->header.length = sizeof(struct acpi_nfit_control_region);
> +		dcr->region_index = 4+1;
> +		dcr->vendor_id = 0xabcd;
> +		dcr->device_id = 0;
> +		dcr->revision_id = 1;
> +		dcr->serial_number = ~handle[4];
> +		dcr->windows = 0;
> +		dcr->window_size = 0;
> +		dcr->command_offset = 0;
> +		dcr->command_size = 0;
> +		dcr->status_offset = 0;
> +		dcr->status_size = 0;
> +

I just noticed that while we do add a block window for this new hotplug
DIMM, the dcr fields talking about block windows were not initialized
correctly. While this still _works_ because of a correct bdw structure,
we should fix it to be a well formed nfit.

If there's a v3 due to additional feedback, I'll include the following
patch. If not, Dan, perhaps you can squash it in? (Or I could send a v3
regardless if you prefer that).

diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index d803e68..6f695ee 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -978,12 +978,12 @@ static void nfit_test0_setup(struct nfit_test *t)
 		dcr->device_id = 0;
 		dcr->revision_id = 1;
 		dcr->serial_number = ~handle[4];
-		dcr->windows = 0;
-		dcr->window_size = 0;
+		dcr->windows = 1;
+		dcr->window_size = DCR_SIZE;
 		dcr->command_offset = 0;
-		dcr->command_size = 0;
-		dcr->status_offset = 0;
-		dcr->status_size = 0;
+		dcr->command_size = 8;
+		dcr->status_offset = 8;
+		dcr->status_size = 4;
 
 		offset = offset + sizeof(struct acpi_nfit_control_region);
 		/* bdw4 (spa/dcr4, dimm4) */