| Message ID | 1454439311-23690-4-git-send-email-toshi.kani@hpe.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, Feb 2, 2016 at 10:55 AM, Toshi Kani <toshi.kani@hpe.com> wrote: > Change acpi_nfit_register_region() to call iomem_set_desc() with > IORES_DESC_PERSISTENT_MEMORY for NFIT_SPA_PM ranges found in ACPI > NFIT table. > > When FW sets E820_PMEM in e820 and EFI_PERSISTENT_MEMORY in EFI, > this code simply sets PMEM type again to "Persistent Memory" entries > in the iomem table. When FW sets reserved type for persistent > memory ranges, it sets PMEM type to "reserved" entries covering > PMEM ranges. > > This allows the EINJ driver, which calls region_intersects() with > IORES_DESC_PERSISTENT_MEMORY to check persistent memory ranges, > to work continuously even if FW sets reserved type to persistent > memory in e820 and EFI. > > Signed-off-by: Toshi Kani <toshi.kani@hpe.com> > Cc: Rafael J. Wysocki <rjw@rjwysocki.net> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Borislav Petkov <bp@suse.de> > Cc: Andrew Morton <akpm@linux-foundation.org> > --- > drivers/acpi/nfit.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c > index ad6d8c6..add04f0 100644 > --- a/drivers/acpi/nfit.c > +++ b/drivers/acpi/nfit.c > @@ -1781,6 +1781,12 @@ static int acpi_nfit_register_region(struct acpi_nfit_desc *acpi_desc, > > nvdimm_bus = acpi_desc->nvdimm_bus; > if (nfit_spa_type(spa) == NFIT_SPA_PM) { > + rc = iomem_set_desc(spa->address, spa->length, > + IORES_DESC_PERSISTENT_MEMORY); > + if (rc) > + dev_dbg(acpi_desc->dev, > + "error setting iomem desc: %d\n", rc); > + Hmm, if we set the type on driver load, should we clear the type on driver unload? Actually it might be more straightforward to specify a type at request_region() time. That way it gets released at release_region(). We're already setting a resource name at request_region time, adding a type annotation at the time seems appropriate. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2016-02-12 at 11:41 -0800, Dan Williams wrote: > On Tue, Feb 2, 2016 at 10:55 AM, Toshi Kani <toshi.kani@hpe.com> wrote: > > Change acpi_nfit_register_region() to call iomem_set_desc() with > > IORES_DESC_PERSISTENT_MEMORY for NFIT_SPA_PM ranges found in ACPI > > NFIT table. > > > > When FW sets E820_PMEM in e820 and EFI_PERSISTENT_MEMORY in EFI, > > this code simply sets PMEM type again to "Persistent Memory" entries > > in the iomem table. When FW sets reserved type for persistent > > memory ranges, it sets PMEM type to "reserved" entries covering > > PMEM ranges. > > > > This allows the EINJ driver, which calls region_intersects() with > > IORES_DESC_PERSISTENT_MEMORY to check persistent memory ranges, > > to work continuously even if FW sets reserved type to persistent > > memory in e820 and EFI. > > > > Signed-off-by: Toshi Kani <toshi.kani@hpe.com> > > Cc: Rafael J. Wysocki <rjw@rjwysocki.net> > > Cc: Dan Williams <dan.j.williams@intel.com> > > Cc: Ingo Molnar <mingo@kernel.org> > > Cc: Borislav Petkov <bp@suse.de> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > --- > > drivers/acpi/nfit.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c > > index ad6d8c6..add04f0 100644 > > --- a/drivers/acpi/nfit.c > > +++ b/drivers/acpi/nfit.c > > @@ -1781,6 +1781,12 @@ static int acpi_nfit_register_region(struct > > acpi_nfit_desc *acpi_desc, > > > > nvdimm_bus = acpi_desc->nvdimm_bus; > > if (nfit_spa_type(spa) == NFIT_SPA_PM) { > > + rc = iomem_set_desc(spa->address, spa->length, > > + IORES_DESC_PERSISTENT_MEMORY); > > + if (rc) > > + dev_dbg(acpi_desc->dev, > > + "error setting iomem desc: %d\n", rc); > > + > > Hmm, if we set the type on driver load, should we clear the type on > driver unload? I think this type update should stay for the life-cycle of this iomem entry itself since this range is PMEM even after the driver is unloaded. This is an extension of the boot-time iomem table initialization from e820/EFI, which allows ACPI to set a correct type. This is independent from driver's resource allocations. > Actually it might be more straightforward to specify a type at > request_region() time. That way it gets released at release_region(). > We're already setting a resource name at request_region time, adding a > type annotation at the time seems appropriate. I first considered simply setting "namespaceX.X" as PMEM. However, region_intersects() and its friends only check the top-level entries, not their children, of the iomem table. And I think a child should have the same type as the parent as I fixed it in patch 1/3. Thanks, -Toshi -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 12, 2016 at 2:30 PM, Toshi Kani <toshi.kani@hpe.com> wrote: > On Fri, 2016-02-12 at 11:41 -0800, Dan Williams wrote: >> On Tue, Feb 2, 2016 at 10:55 AM, Toshi Kani <toshi.kani@hpe.com> wrote: >> > Change acpi_nfit_register_region() to call iomem_set_desc() with >> > IORES_DESC_PERSISTENT_MEMORY for NFIT_SPA_PM ranges found in ACPI >> > NFIT table. >> > >> > When FW sets E820_PMEM in e820 and EFI_PERSISTENT_MEMORY in EFI, >> > this code simply sets PMEM type again to "Persistent Memory" entries >> > in the iomem table. When FW sets reserved type for persistent >> > memory ranges, it sets PMEM type to "reserved" entries covering >> > PMEM ranges. >> > >> > This allows the EINJ driver, which calls region_intersects() with >> > IORES_DESC_PERSISTENT_MEMORY to check persistent memory ranges, >> > to work continuously even if FW sets reserved type to persistent >> > memory in e820 and EFI. >> > >> > Signed-off-by: Toshi Kani <toshi.kani@hpe.com> >> > Cc: Rafael J. Wysocki <rjw@rjwysocki.net> >> > Cc: Dan Williams <dan.j.williams@intel.com> >> > Cc: Ingo Molnar <mingo@kernel.org> >> > Cc: Borislav Petkov <bp@suse.de> >> > Cc: Andrew Morton <akpm@linux-foundation.org> >> > --- >> > drivers/acpi/nfit.c | 6 ++++++ >> > 1 file changed, 6 insertions(+) >> > >> > diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c >> > index ad6d8c6..add04f0 100644 >> > --- a/drivers/acpi/nfit.c >> > +++ b/drivers/acpi/nfit.c >> > @@ -1781,6 +1781,12 @@ static int acpi_nfit_register_region(struct >> > acpi_nfit_desc *acpi_desc, >> > >> > nvdimm_bus = acpi_desc->nvdimm_bus; >> > if (nfit_spa_type(spa) == NFIT_SPA_PM) { >> > + rc = iomem_set_desc(spa->address, spa->length, >> > + IORES_DESC_PERSISTENT_MEMORY); >> > + if (rc) >> > + dev_dbg(acpi_desc->dev, >> > + "error setting iomem desc: %d\n", rc); >> > + >> >> Hmm, if we set the type on driver load, should we clear the type on >> driver unload? > > I think this type update should stay for the life-cycle of this iomem entry > itself since this range is PMEM even after the driver is unloaded. This is > an extension of the boot-time iomem table initialization from e820/EFI, > which allows ACPI to set a correct type. This is independent from driver's > resource allocations. > >> Actually it might be more straightforward to specify a type at >> request_region() time. That way it gets released at release_region(). >> We're already setting a resource name at request_region time, adding a >> type annotation at the time seems appropriate. > > I first considered simply setting "namespaceX.X" as PMEM. However, > region_intersects() and its friends only check the top-level entries, not > their children, of the iomem table. And I think a child should have the > same type as the parent as I fixed it in patch 1/3. Did we investigate updating region_intersects() to check children? When a child sub-divides a region with different types it may be the wrong answer to check the parent. Is there a problem with moving checking to the child? -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2016-02-12 at 15:32 -0800, Dan Williams wrote: > On Fri, Feb 12, 2016 at 2:30 PM, Toshi Kani <toshi.kani@hpe.com> wrote: > > On Fri, 2016-02-12 at 11:41 -0800, Dan Williams wrote: > > > On Tue, Feb 2, 2016 at 10:55 AM, Toshi Kani <toshi.kani@hpe.com> wro : > > > Hmm, if we set the type on driver load, should we clear the type on > > > driver unload? > > > > I think this type update should stay for the life-cycle of this iomem > > entry itself since this range is PMEM even after the driver is > > unloaded. This is an extension of the boot-time iomem table > > initialization from e820/EFI, which allows ACPI to set a correct > > type. This is independent from driver's resource allocations. > > > > > Actually it might be more straightforward to specify a type at > > > request_region() time. That way it gets released at > > > release_region(). We're already setting a resource name at > > > request_region time, adding a type annotation at the time seems > > > appropriate. > > > > I first considered simply setting "namespaceX.X" as PMEM. However, > > region_intersects() and its friends only check the top-level entries, > > not their children, of the iomem table. And I think a child should > > have the same type as the parent as I fixed it in patch 1/3. > > Did we investigate updating region_intersects() to check children? > When a child sub-divides a region with different types it may be the > wrong answer to check the parent. Is there a problem with moving > checking to the child? Here are three options I can think of. 1) Set pmem type to "reserved" (This patch-set) - Add a new iomem_set_desc(), which sets a given type to a top-level entry. Change the ACPI NFIT driver to call it to set pmem type to "reserved" entry. - region_intersects() finds a pmem entry by checking top-level entries (no change). 2) Change region_intersects() to check children's type - Add a new request_region_ext(), which is an extension to request_region() to allow specifying a type of resource. It puts a new child entry under "reserved". Change the pmem driver to call this func. - Change region_intersects() to check children's type for finding this child pmem entry. 3) Pmem driver to call insert_resource() - Change the pmem driver to call insert_resource(), which puts a new pmem entry as the parent of "reserved". - region_intersects() finds a pmem entry by checking top-level entries (no change). - Add a new release_resource_self(), which releases a given entry and keeps its children if any. Change the pmem driver to call it for release. This patch-set implements 1). The pmem type is set to "reserved" for its life-cycle. This option is simplest. For 2), the changes to region_intersects() may be too complex for maintenance. Here are a few examples when region_intersects() is called with addr [1-10] where iomem has entry P and its children. Case A: P is fully covered by children C1 & C2. region_intersects() ignores P's type, but checks C1 and C2's. P [1-10] + C1 [1-5] + C2 [6-10] Case B: C2 is fully covered by C3, but P is not. region_intersects() ignores C2's type, but checks P, C1, C3's. P [1-10] + C1 [1-2] + C2 [6-10] + C3 [6-10] I think region_intersects() will need to construct a flat table from the tree while making recursive calls to walk thru all children. 3) is similar to 2), but avoids the changes to region_intersects() since insert_resource() inserts a new entry as the parent to "reserved". However, a new interface is necessary to put "reserved" back to top-level when releasing the added entry. My recommendation is go with either 1) or 3). What do you think? Thanks, -Toshi -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2016-02-16 at 19:00 -0700, Toshi Kani wrote: > On Fri, 2016-02-12 at 15:32 -0800, Dan Williams wrote: > > On Fri, Feb 12, 2016 at 2:30 PM, Toshi Kani <toshi.kani@hpe.com> wrote: > > > On Fri, 2016-02-12 at 11:41 -0800, Dan Williams wrote: > > > > On Tue, Feb 2, 2016 at 10:55 AM, Toshi Kani <toshi.kani@hpe.com> > > > > wro > : > > > > Hmm, if we set the type on driver load, should we clear the type on > > > > driver unload? > > > > > > I think this type update should stay for the life-cycle of this iomem > > > entry itself since this range is PMEM even after the driver is > > > unloaded. This is an extension of the boot-time iomem table > > > initialization from e820/EFI, which allows ACPI to set a correct > > > type. This is independent from driver's resource allocations. > > > > > > > Actually it might be more straightforward to specify a type at > > > > request_region() time. That way it gets released at > > > > release_region(). We're already setting a resource name at > > > > request_region time, adding a type annotation at the time seems > > > > appropriate. > > > > > > I first considered simply setting "namespaceX.X" as PMEM. However, > > > region_intersects() and its friends only check the top-level entries, > > > not their children, of the iomem table. And I think a child should > > > have the same type as the parent as I fixed it in patch 1/3. > > > > Did we investigate updating region_intersects() to check children? > > When a child sub-divides a region with different types it may be the > > wrong answer to check the parent. Is there a problem with moving > > checking to the child? > > Here are three options I can think of. > > 1) Set pmem type to "reserved" (This patch-set) > - Add a new iomem_set_desc(), which sets a given type to a top-level > entry. Change the ACPI NFIT driver to call it to set pmem type to > "reserved" entry. > - region_intersects() finds a pmem entry by checking top-level entries > (no change). > > 2) Change region_intersects() to check children's type > - Add a new request_region_ext(), which is an extension to > request_region() to allow specifying a type of resource. It puts a new > child entry under "reserved". Change the pmem driver to call this func. > - Change region_intersects() to check children's type for finding this > child pmem entry. > > 3) Pmem driver to call insert_resource() > - Change the pmem driver to call insert_resource(), which puts a new > pmem entry as the parent of "reserved". > - region_intersects() finds a pmem entry by checking top-level entries > (no change). > - Add a new release_resource_self(), which releases a given entry and > keeps its children if any. Change the pmem driver to call it for > release. Thinking further, 3) needs to be modified as follows. insert_resource() should only be allowed for producers of resource (ex. nfit), not consumers (ex. pmem). It also needs to export insert_resource(). 3) NFIT driver to call insert_resource() - Change the ACPI nfit driver to call insert_resource() when a target range is not marked as PMEM (i.e. "reserved") or not present in iomem. This puts a new PMEM entry as the parent of "reserved". - region_intersects() finds a pmem entry by checking top-level entries (no change). - Add a new release_resource_self(), which releases a given entry and keeps its children if any. Change the nfit driver to call it for release. > This patch-set implements 1). The pmem type is set to "reserved" for its > life-cycle. This option is simplest. > > For 2), the changes to region_intersects() may be too complex for > maintenance. I should have said "region_intersects() may be overly complicated for this purpose and maintenance". > Here are a few examples when region_intersects() is called > with addr [1-10] where iomem has entry P and its children. > > Case A: P is fully covered by children C1 & C2. region_intersects() > ignores P's type, but checks C1 and C2's. > > P [1-10] + C1 [1-5] > + C2 [6-10] > > Case B: C2 is fully covered by C3, but P is not. region_intersects() > ignores C2's type, but checks P, C1, C3's. > > P [1-10] + C1 [1-2] > + C2 [6-10] + C3 [6-10] > > I think region_intersects() will need to construct a flat table from the > tree while making recursive calls to walk thru all children. > > 3) is similar to 2), but avoids the changes to region_intersects() since > insert_resource() inserts a new entry as the parent to "reserved". 3) is actually similar to 1) as both options change the producer side. > However, a new interface is necessary to put "reserved" back to top- > level when releasing the added entry. > > My recommendation is go with either 1) or 3). What do you think? I think we should modify the producer side, so 1) or 3) are still my recommendation. Thanks, -Toshi -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 17, 2016 at 10:00 AM, Toshi Kani <toshi.kani@hpe.com> wrote: > On Tue, 2016-02-16 at 19:00 -0700, Toshi Kani wrote: >> On Fri, 2016-02-12 at 15:32 -0800, Dan Williams wrote: >> > On Fri, Feb 12, 2016 at 2:30 PM, Toshi Kani <toshi.kani@hpe.com> wrote: >> > > On Fri, 2016-02-12 at 11:41 -0800, Dan Williams wrote: >> > > > On Tue, Feb 2, 2016 at 10:55 AM, Toshi Kani <toshi.kani@hpe.com> >> > > > wro >> : >> > > > Hmm, if we set the type on driver load, should we clear the type on >> > > > driver unload? >> > > >> > > I think this type update should stay for the life-cycle of this iomem >> > > entry itself since this range is PMEM even after the driver is >> > > unloaded. This is an extension of the boot-time iomem table >> > > initialization from e820/EFI, which allows ACPI to set a correct >> > > type. This is independent from driver's resource allocations. >> > > >> > > > Actually it might be more straightforward to specify a type at >> > > > request_region() time. That way it gets released at >> > > > release_region(). We're already setting a resource name at >> > > > request_region time, adding a type annotation at the time seems >> > > > appropriate. >> > > >> > > I first considered simply setting "namespaceX.X" as PMEM. However, >> > > region_intersects() and its friends only check the top-level entries, >> > > not their children, of the iomem table. And I think a child should >> > > have the same type as the parent as I fixed it in patch 1/3. >> > >> > Did we investigate updating region_intersects() to check children? >> > When a child sub-divides a region with different types it may be the >> > wrong answer to check the parent. Is there a problem with moving >> > checking to the child? >> >> Here are three options I can think of. >> >> 1) Set pmem type to "reserved" (This patch-set) >> - Add a new iomem_set_desc(), which sets a given type to a top-level >> entry. Change the ACPI NFIT driver to call it to set pmem type to >> "reserved" entry. >> - region_intersects() finds a pmem entry by checking top-level entries >> (no change). >> >> 2) Change region_intersects() to check children's type >> - Add a new request_region_ext(), which is an extension to >> request_region() to allow specifying a type of resource. It puts a new >> child entry under "reserved". Change the pmem driver to call this func. >> - Change region_intersects() to check children's type for finding this >> child pmem entry. >> >> 3) Pmem driver to call insert_resource() >> - Change the pmem driver to call insert_resource(), which puts a new >> pmem entry as the parent of "reserved". >> - region_intersects() finds a pmem entry by checking top-level entries >> (no change). >> - Add a new release_resource_self(), which releases a given entry and >> keeps its children if any. Change the pmem driver to call it for >> release. > > Thinking further, 3) needs to be modified as follows. insert_resource() > should only be allowed for producers of resource (ex. nfit), not consumers > (ex. pmem). It also needs to export insert_resource(). > > 3) NFIT driver to call insert_resource() > - Change the ACPI nfit driver to call insert_resource() when a target > range is not marked as PMEM (i.e. "reserved") or not present in iomem. > This puts a new PMEM entry as the parent of "reserved". > - region_intersects() finds a pmem entry by checking top-level entries (no > change). > - Add a new release_resource_self(), which releases a given entry and > keeps its children if any. Change the nfit driver to call it for release. > >> This patch-set implements 1). The pmem type is set to "reserved" for its >> life-cycle. This option is simplest. >> >> For 2), the changes to region_intersects() may be too complex for >> maintenance. > > I should have said "region_intersects() may be overly complicated for this > purpose and maintenance". > >> Here are a few examples when region_intersects() is called >> with addr [1-10] where iomem has entry P and its children. >> >> Case A: P is fully covered by children C1 & C2. region_intersects() >> ignores P's type, but checks C1 and C2's. >> >> P [1-10] + C1 [1-5] >> + C2 [6-10] >> >> Case B: C2 is fully covered by C3, but P is not. region_intersects() >> ignores C2's type, but checks P, C1, C3's. >> >> P [1-10] + C1 [1-2] >> + C2 [6-10] + C3 [6-10] >> >> I think region_intersects() will need to construct a flat table from the >> tree while making recursive calls to walk thru all children. >> >> 3) is similar to 2), but avoids the changes to region_intersects() since >> insert_resource() inserts a new entry as the parent to "reserved". > > 3) is actually similar to 1) as both options change the producer side. > >> However, a new interface is necessary to put "reserved" back to top- >> level when releasing the added entry. >> >> My recommendation is go with either 1) or 3). What do you think? > > I think we should modify the producer side, so 1) or 3) are still my > recommendation. > I think 3 is the most promising option. It aligns the acpi/nfit driver closer with the acpi/pci_root driver that is also doing insert_resource() for each root bridge, and removing those resources when the bridge is disabled/removed. I'd still want to maintain the ability of nfit to be built as a module, so we would need to split the resource registration into a separate built-in object file from nfit.ko. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2016-02-17 at 11:34 -0800, Dan Williams wrote: > On Wed, Feb 17, 2016 at 10:00 AM, Toshi Kani <toshi.kani@hpe.com> wrote: > > On Tue, 2016-02-16 at 19:00 -0700, Toshi Kani wrote: > > > On Fri, 2016-02-12 at 15:32 -0800, Dan Williams wrote: > > > > On Fri, Feb 12, 2016 at 2:30 PM, Toshi Kani <toshi.kani@hpe.com> > > > > wrote: : > > > Here are three options I can think of. > > > > > > 1) Set pmem type to "reserved" (This patch-set) > > > - Add a new iomem_set_desc(), which sets a given type to a top-level > > > entry. Change the ACPI NFIT driver to call it to set pmem type to > > > "reserved" entry. > > > - region_intersects() finds a pmem entry by checking top-level > > > entries (no change). > > > > > > 2) Change region_intersects() to check children's type > > > - Add a new request_region_ext(), which is an extension to > > > request_region() to allow specifying a type of resource. It puts a > > > new child entry under "reserved". Change the pmem driver to call > > > this func. > > > - Change region_intersects() to check children's type for finding > > > this child pmem entry. > > > > > > 3) Pmem driver to call insert_resource() > > > - Change the pmem driver to call insert_resource(), which puts a new > > > pmem entry as the parent of "reserved". > > > - region_intersects() finds a pmem entry by checking top-level > > > entries (no change). > > > - Add a new release_resource_self(), which releases a given entry > > > and keeps its children if any. Change the pmem driver to call it for > > > release. > > > > Thinking further, 3) needs to be modified as follows. insert_resource( > > ) should only be allowed for producers of resource (ex. nfit), not > > consumers (ex. pmem). It also needs to export insert_resource(). > > > > 3) NFIT driver to call insert_resource() > > - Change the ACPI nfit driver to call insert_resource() when a target > > range is not marked as PMEM (i.e. "reserved") or not present in iomem. > > This puts a new PMEM entry as the parent of "reserved". > > - region_intersects() finds a pmem entry by checking top-level entries > > (no change). > > - Add a new release_resource_self(), which releases a given entry and > > keeps its children if any. Change the nfit driver to call it for > > release. > > > > > This patch-set implements 1). The pmem type is set to "reserved" for > > > its life-cycle. This option is simplest. > > > > > > For 2), the changes to region_intersects() may be too complex for > > > maintenance. > > > > I should have said "region_intersects() may be overly complicated for > > this purpose and maintenance". > > > > > Here are a few examples when region_intersects() is called > > > with addr [1-10] where iomem has entry P and its children. > > > > > > Case A: P is fully covered by children C1 & C2. region_intersects() > > > ignores P's type, but checks C1 and C2's. > > > > > > P [1-10] + C1 [1-5] > > > + C2 [6-10] > > > > > > Case B: C2 is fully covered by C3, but P is not. region_intersects() > > > ignores C2's type, but checks P, C1, C3's. > > > > > > P [1-10] + C1 [1-2] > > > + C2 [6-10] + C3 [6-10] > > > > > > I think region_intersects() will need to construct a flat table from > > > the tree while making recursive calls to walk thru all children. > > > > > > 3) is similar to 2), but avoids the changes to region_intersects() > > > since insert_resource() inserts a new entry as the parent to > > > "reserved". > > > > 3) is actually similar to 1) as both options change the producer side. > > > > > However, a new interface is necessary to put "reserved" back to top- > > > level when releasing the added entry. > > > > > > My recommendation is go with either 1) or 3). What do you think? > > > > I think we should modify the producer side, so 1) or 3) are still my > > recommendation. > > > > I think 3 is the most promising option. It aligns the acpi/nfit > driver closer with the acpi/pci_root driver that is also doing > insert_resource() for each root bridge, and removing those resources > when the bridge is disabled/removed. > > I'd still want to maintain the ability of nfit to be built as a > module, so we would need to split the resource registration into a > separate built-in object file from nfit.ko. Sounds good. I will implement 3), and add an ACPI built-in wrapper function to call insert_resource(). Thanks! -Toshi -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c index ad6d8c6..add04f0 100644 --- a/drivers/acpi/nfit.c +++ b/drivers/acpi/nfit.c @@ -1781,6 +1781,12 @@ static int acpi_nfit_register_region(struct acpi_nfit_desc *acpi_desc, nvdimm_bus = acpi_desc->nvdimm_bus; if (nfit_spa_type(spa) == NFIT_SPA_PM) { + rc = iomem_set_desc(spa->address, spa->length, + IORES_DESC_PERSISTENT_MEMORY); + if (rc) + dev_dbg(acpi_desc->dev, + "error setting iomem desc: %d\n", rc); + rc = acpi_nfit_find_poison(acpi_desc, ndr_desc); if (rc) { dev_err(acpi_desc->dev,
Change acpi_nfit_register_region() to call iomem_set_desc() with IORES_DESC_PERSISTENT_MEMORY for NFIT_SPA_PM ranges found in ACPI NFIT table. When FW sets E820_PMEM in e820 and EFI_PERSISTENT_MEMORY in EFI, this code simply sets PMEM type again to "Persistent Memory" entries in the iomem table. When FW sets reserved type for persistent memory ranges, it sets PMEM type to "reserved" entries covering PMEM ranges. This allows the EINJ driver, which calls region_intersects() with IORES_DESC_PERSISTENT_MEMORY to check persistent memory ranges, to work continuously even if FW sets reserved type to persistent memory in e820 and EFI. Signed-off-by: Toshi Kani <toshi.kani@hpe.com> Cc: Rafael J. Wysocki <rjw@rjwysocki.net> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Ingo Molnar <mingo@kernel.org> Cc: Borislav Petkov <bp@suse.de> Cc: Andrew Morton <akpm@linux-foundation.org> --- drivers/acpi/nfit.c | 6 ++++++ 1 file changed, 6 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html