On 2/8/2016 1:30 PM, Dan Williams wrote:
> ACPI 6.1 clarified that multi-interface dimms require multiple control
> region entries (DCRs) per dimm.  Previously we were assuming that a
> control region is only present when block-data-windows are present.

We need to give this a quick test with NVDIMM-N because those types of
NVDIMMs have control regions without block-data-windows.  We've fixed
bugs related to that assumption a couple of times.

> This implementation was done with an eye to be compatibility with the
> looser ACPI 6.0 interpretation of this table.
> 
> 1/ When coalescing the memory device (MEMDEV) tables for a single dimm,
> coalesce on device_handle rather than control region index.
> 
> 2/ Whenever we disocver a control region with non-zero block windows
discover

> re-scan for block-data-window (BDW) entries.
> 
> We may need to revisit this if a DIMM ever implements a format interface
> outside of blk or pmem, but that is not on the foreseeable horizon.
> 
> Cc: <sta...@vger.kernel.org>
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> ---
>  drivers/acpi/nfit.c |   71 
> +++++++++++++++++++++++++--------------------------
>  1 file changed, 35 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
> index ad6d8c6b777e..424b362e8fdc 100644
> --- a/drivers/acpi/nfit.c
> +++ b/drivers/acpi/nfit.c
> @@ -469,37 +469,16 @@ static void nfit_mem_find_spa_bdw(struct acpi_nfit_desc 
> *acpi_desc,
>       nfit_mem->bdw = NULL;
>  }
>  
> -static int nfit_mem_add(struct acpi_nfit_desc *acpi_desc,
> +static void nfit_mem_init_bdw(struct acpi_nfit_desc *acpi_desc,
>               struct nfit_mem *nfit_mem, struct acpi_nfit_system_address *spa)
>  {
>       u16 dcr = __to_nfit_memdev(nfit_mem)->region_index;
>       struct nfit_memdev *nfit_memdev;
>       struct nfit_flush *nfit_flush;
> -     struct nfit_dcr *nfit_dcr;
>       struct nfit_bdw *nfit_bdw;
>       struct nfit_idt *nfit_idt;
>       u16 idt_idx, range_index;
>  
> -     list_for_each_entry(nfit_dcr, &acpi_desc->dcrs, list) {
> -             if (nfit_dcr->dcr->region_index != dcr)
> -                     continue;
> -             nfit_mem->dcr = nfit_dcr->dcr;
> -             break;
> -     }
> -
> -     if (!nfit_mem->dcr) {
> -             dev_dbg(acpi_desc->dev, "SPA %d missing:%s%s\n",
> -                             spa->range_index, __to_nfit_memdev(nfit_mem)
> -                             ? "" : " MEMDEV", nfit_mem->dcr ? "" : " DCR");
> -             return -ENODEV;
> -     }
> -
> -     /*
> -      * We've found enough to create an nvdimm, optionally
> -      * find an associated BDW
> -      */
> -     list_add(&nfit_mem->list, &acpi_desc->dimms);
> -
>       list_for_each_entry(nfit_bdw, &acpi_desc->bdws, list) {
>               if (nfit_bdw->bdw->region_index != dcr)
>                       continue;
> @@ -508,12 +487,12 @@ static int nfit_mem_add(struct acpi_nfit_desc 
> *acpi_desc,
>       }
>  
>       if (!nfit_mem->bdw)
> -             return 0;
> +             return;
>  
>       nfit_mem_find_spa_bdw(acpi_desc, nfit_mem);
>  
>       if (!nfit_mem->spa_bdw)
> -             return 0;
> +             return;
>  
>       range_index = nfit_mem->spa_bdw->range_index;
>       list_for_each_entry(nfit_memdev, &acpi_desc->memdevs, list) {
> @@ -538,8 +517,6 @@ static int nfit_mem_add(struct acpi_nfit_desc *acpi_desc,
>               }
>               break;
>       }
> -
> -     return 0;
>  }
>  
>  static int nfit_mem_dcr_init(struct acpi_nfit_desc *acpi_desc,
> @@ -548,7 +525,6 @@ static int nfit_mem_dcr_init(struct acpi_nfit_desc 
> *acpi_desc,
>       struct nfit_mem *nfit_mem, *found;
>       struct nfit_memdev *nfit_memdev;
>       int type = nfit_spa_type(spa);
> -     u16 dcr;
>  
>       switch (type) {
>       case NFIT_SPA_DCR:
> @@ -559,14 +535,18 @@ static int nfit_mem_dcr_init(struct acpi_nfit_desc 
> *acpi_desc,
>       }
>  
>       list_for_each_entry(nfit_memdev, &acpi_desc->memdevs, list) {
> -             int rc;
> +             struct nfit_dcr *nfit_dcr;
> +             u32 device_handle;
> +             u16 dcr;
>  
>               if (nfit_memdev->memdev->range_index != spa->range_index)
>                       continue;
>               found = NULL;
>               dcr = nfit_memdev->memdev->region_index;
> +             device_handle = nfit_memdev->memdev->device_handle;
>               list_for_each_entry(nfit_mem, &acpi_desc->dimms, list)
> -                     if (__to_nfit_memdev(nfit_mem)->region_index == dcr) {
> +                     if (__to_nfit_memdev(nfit_mem)->device_handle
> +                                     == device_handle) {
>                               found = nfit_mem;
>                               break;
>                       }
> @@ -579,6 +559,31 @@ static int nfit_mem_dcr_init(struct acpi_nfit_desc 
> *acpi_desc,
>                       if (!nfit_mem)
>                               return -ENOMEM;
>                       INIT_LIST_HEAD(&nfit_mem->list);
> +                     list_add(&nfit_mem->list, &acpi_desc->dimms);
> +             }
> +
> +             list_for_each_entry(nfit_dcr, &acpi_desc->dcrs, list) {
> +                     if (nfit_dcr->dcr->region_index != dcr)
> +                             continue;
> +                     /*
> +                      * Record the control region for the dimm.  For
> +                      * the ACPI 6.1 case, where there are separate
> +                      * control regions for the pmem vs blk
> +                      * interfaces, be sure to record the extended
> +                      * blk details.
> +                      */
> +                     if (!nfit_mem->dcr)
> +                             nfit_mem->dcr = nfit_dcr->dcr;
> +                     else if (nfit_mem->dcr->windows == 0
> +                                     && nfit_dcr->dcr->windows)
> +                             nfit_mem->dcr = nfit_dcr->dcr;
> +                     break;
> +             }
> +
> +             if (dcr && !nfit_mem->dcr) {
> +                     dev_err(acpi_desc->dev, "SPA %d missing DCR %d\n",
> +                                     spa->range_index, dcr);
> +                     return -ENODEV;
>               }
>  
>               if (type == NFIT_SPA_DCR) {
> @@ -595,6 +600,7 @@ static int nfit_mem_dcr_init(struct acpi_nfit_desc 
> *acpi_desc,
>                               nfit_mem->idt_dcr = nfit_idt->idt;
>                               break;
>                       }
> +                     nfit_mem_init_bdw(acpi_desc, nfit_mem, spa);
>               } else {
>                       /*
>                        * A single dimm may belong to multiple SPA-PM
> @@ -603,13 +609,6 @@ static int nfit_mem_dcr_init(struct acpi_nfit_desc 
> *acpi_desc,
>                        */
>                       nfit_mem->memdev_pmem = nfit_memdev->memdev;
>               }
> -
> -             if (found)
> -                     continue;
> -
> -             rc = nfit_mem_add(acpi_desc, nfit_mem, spa);
> -             if (rc)
> -                     return rc;
>       }
>  
>       return 0;
> 
> _______________________________________________
> Linux-nvdimm mailing list
> linux-nvd...@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
> 

Reply via email to