All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joao Martins <joao.m.martins@oracle.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Linux ACPI <linux-acpi@vger.kernel.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Brice Goglin <Brice.Goglin@inria.fr>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	linux-nvdimm <linux-nvdimm@lists.01.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	X86 ML <x86@kernel.org>
Subject: Re: [PATCH v2 6/6] ACPI: HMAT: Attach a device for each soft-reserved range
Date: Tue, 24 Mar 2020 21:30:42 +0000	[thread overview]
Message-ID: <ecfd5f74-cc03-5590-82df-6f7a3dbcdb50@oracle.com> (raw)
In-Reply-To: <CAPcyv4iyfP88KXaK4VbaUgFWRjsRutdFF8OH7nwT-zUiv3fV7Q@mail.gmail.com>

On 3/24/20 9:06 PM, Dan Williams wrote:
> On Tue, Mar 24, 2020 at 12:41 PM Joao Martins <joao.m.martins@oracle.com> wrote:
>>
>> On 3/22/20 4:12 PM, Dan Williams wrote:
>>> The hmem enabling in commit 'cf8741ac57ed ("ACPI: NUMA: HMAT: Register
>>> "soft reserved" memory as an "hmem" device")' only registered ranges to
>>> the hmem driver for each soft-reservation that also appeared in the
>>> HMAT. While this is meant to encourage platform firmware to "do the
>>> right thing" and publish an HMAT, the corollary is that platforms that
>>> fail to publish an accurate HMAT will strand memory from Linux usage.
>>> Additionally, the "efi_fake_mem" kernel command line option enabling
>>> will strand memory by default without an HMAT.
>>>
>>> Arrange for "soft reserved" memory that goes unclaimed by HMAT entries
>>> to be published as raw resource ranges for the hmem driver to consume.
>>>
>>> Include a module parameter to disable either this fallback behavior, or
>>> the hmat enabling from creating hmem devices. The module parameter
>>> requires the hmem device enabling to have unique name in the module
>>> namespace: "device_hmem".
>>>
>>> Rather than mark this x86-only, include an interim phys_to_target_node()
>>> implementation for arm64.
>>>
>>> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>> Cc: Brice Goglin <Brice.Goglin@inria.fr>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>> Cc: Jeff Moyer <jmoyer@redhat.com>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>> ---
>>>  arch/arm64/mm/numa.c      |   13 +++++++++++++
>>>  drivers/dax/Kconfig       |    1 +
>>>  drivers/dax/hmem/Makefile |    3 ++-
>>>  drivers/dax/hmem/device.c |   33 +++++++++++++++++++++++++++++++++
>>>  4 files changed, 49 insertions(+), 1 deletion(-)
>>>
>>
>> [...]
>>
>>> diff --git a/drivers/dax/hmem/device.c b/drivers/dax/hmem/device.c
>>> index 99bc15a8b031..f9c5fa8b1880 100644
>>> --- a/drivers/dax/hmem/device.c
>>> +++ b/drivers/dax/hmem/device.c
>>> @@ -4,6 +4,9 @@
>>>  #include <linux/module.h>
>>>  #include <linux/mm.h>
>>>
>>> +static bool nohmem;
>>> +module_param_named(disable, nohmem, bool, 0444);
>>> +
>>>  void hmem_register_device(int target_nid, struct resource *r)
>>>  {
>>>       /* define a clean / non-busy resource for the platform device */
>>> @@ -16,6 +19,9 @@ void hmem_register_device(int target_nid, struct resource *r)
>>>       struct memregion_info info;
>>>       int rc, id;
>>>
>>> +     if (nohmem)
>>> +             return;
>>> +
>>>       rc = region_intersects(res.start, resource_size(&res), IORESOURCE_MEM,
>>>                       IORES_DESC_SOFT_RESERVED);
>>>       if (rc != REGION_INTERSECTS)
>>> @@ -62,3 +68,30 @@ void hmem_register_device(int target_nid, struct resource *r)
>>>  out_pdev:
>>>       memregion_free(id);
>>>  }
>>> +
>>> +static __init int hmem_register_one(struct resource *res, void *data)
>>> +{
>>> +     /*
>>> +      * If the resource is not a top-level resource it was already
>>> +      * assigned to a device by the HMAT parsing.
>>> +      */
>>> +     if (res->parent != &iomem_resource)
>>> +             return 0;
>>> +
>>> +     hmem_register_device(phys_to_target_node(res->start), res);
>>> +
>>> +     return 0;
>>
>> Should we add an error returning value to hmem_register_device() perhaps this
>> ought to be reflected in hmem_register_one().
>>
>>> +}
>>> +
>>> +static __init int hmem_init(void)
>>> +{
>>> +     walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED,
>>> +                     IORESOURCE_MEM, 0, -1, NULL, hmem_register_one);
>>> +     return 0;
>>> +}
>>> +
>>
>> (...) and then perhaps here returning in the initcall if any of the resources
>> failed hmem registration?
> 
> Except that hmem_register_one() is a stop-gap to collect soft-reserved
> ranges that were not already registered, and it's not an error to find
> already registered devices. 
> 
/nods

And if we were to return an error (say for hmem0 out of 4 hmem ones)  before
walking through all soft-reserved found resources, if would skip registration
for the remaining ones.

  Joao
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

WARNING: multiple messages have this Message-ID (diff)
From: Joao Martins <joao.m.martins@oracle.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Linux ACPI <linux-acpi@vger.kernel.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Brice Goglin <Brice.Goglin@inria.fr>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	linux-nvdimm <linux-nvdimm@lists.01.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	X86 ML <x86@kernel.org>
Subject: Re: [PATCH v2 6/6] ACPI: HMAT: Attach a device for each soft-reserved range
Date: Tue, 24 Mar 2020 21:30:42 +0000	[thread overview]
Message-ID: <ecfd5f74-cc03-5590-82df-6f7a3dbcdb50@oracle.com> (raw)
In-Reply-To: <CAPcyv4iyfP88KXaK4VbaUgFWRjsRutdFF8OH7nwT-zUiv3fV7Q@mail.gmail.com>

On 3/24/20 9:06 PM, Dan Williams wrote:
> On Tue, Mar 24, 2020 at 12:41 PM Joao Martins <joao.m.martins@oracle.com> wrote:
>>
>> On 3/22/20 4:12 PM, Dan Williams wrote:
>>> The hmem enabling in commit 'cf8741ac57ed ("ACPI: NUMA: HMAT: Register
>>> "soft reserved" memory as an "hmem" device")' only registered ranges to
>>> the hmem driver for each soft-reservation that also appeared in the
>>> HMAT. While this is meant to encourage platform firmware to "do the
>>> right thing" and publish an HMAT, the corollary is that platforms that
>>> fail to publish an accurate HMAT will strand memory from Linux usage.
>>> Additionally, the "efi_fake_mem" kernel command line option enabling
>>> will strand memory by default without an HMAT.
>>>
>>> Arrange for "soft reserved" memory that goes unclaimed by HMAT entries
>>> to be published as raw resource ranges for the hmem driver to consume.
>>>
>>> Include a module parameter to disable either this fallback behavior, or
>>> the hmat enabling from creating hmem devices. The module parameter
>>> requires the hmem device enabling to have unique name in the module
>>> namespace: "device_hmem".
>>>
>>> Rather than mark this x86-only, include an interim phys_to_target_node()
>>> implementation for arm64.
>>>
>>> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>> Cc: Brice Goglin <Brice.Goglin@inria.fr>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>> Cc: Jeff Moyer <jmoyer@redhat.com>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>> ---
>>>  arch/arm64/mm/numa.c      |   13 +++++++++++++
>>>  drivers/dax/Kconfig       |    1 +
>>>  drivers/dax/hmem/Makefile |    3 ++-
>>>  drivers/dax/hmem/device.c |   33 +++++++++++++++++++++++++++++++++
>>>  4 files changed, 49 insertions(+), 1 deletion(-)
>>>
>>
>> [...]
>>
>>> diff --git a/drivers/dax/hmem/device.c b/drivers/dax/hmem/device.c
>>> index 99bc15a8b031..f9c5fa8b1880 100644
>>> --- a/drivers/dax/hmem/device.c
>>> +++ b/drivers/dax/hmem/device.c
>>> @@ -4,6 +4,9 @@
>>>  #include <linux/module.h>
>>>  #include <linux/mm.h>
>>>
>>> +static bool nohmem;
>>> +module_param_named(disable, nohmem, bool, 0444);
>>> +
>>>  void hmem_register_device(int target_nid, struct resource *r)
>>>  {
>>>       /* define a clean / non-busy resource for the platform device */
>>> @@ -16,6 +19,9 @@ void hmem_register_device(int target_nid, struct resource *r)
>>>       struct memregion_info info;
>>>       int rc, id;
>>>
>>> +     if (nohmem)
>>> +             return;
>>> +
>>>       rc = region_intersects(res.start, resource_size(&res), IORESOURCE_MEM,
>>>                       IORES_DESC_SOFT_RESERVED);
>>>       if (rc != REGION_INTERSECTS)
>>> @@ -62,3 +68,30 @@ void hmem_register_device(int target_nid, struct resource *r)
>>>  out_pdev:
>>>       memregion_free(id);
>>>  }
>>> +
>>> +static __init int hmem_register_one(struct resource *res, void *data)
>>> +{
>>> +     /*
>>> +      * If the resource is not a top-level resource it was already
>>> +      * assigned to a device by the HMAT parsing.
>>> +      */
>>> +     if (res->parent != &iomem_resource)
>>> +             return 0;
>>> +
>>> +     hmem_register_device(phys_to_target_node(res->start), res);
>>> +
>>> +     return 0;
>>
>> Should we add an error returning value to hmem_register_device() perhaps this
>> ought to be reflected in hmem_register_one().
>>
>>> +}
>>> +
>>> +static __init int hmem_init(void)
>>> +{
>>> +     walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED,
>>> +                     IORESOURCE_MEM, 0, -1, NULL, hmem_register_one);
>>> +     return 0;
>>> +}
>>> +
>>
>> (...) and then perhaps here returning in the initcall if any of the resources
>> failed hmem registration?
> 
> Except that hmem_register_one() is a stop-gap to collect soft-reserved
> ranges that were not already registered, and it's not an error to find
> already registered devices. 
> 
/nods

And if we were to return an error (say for hmem0 out of 4 hmem ones)  before
walking through all soft-reserved found resources, if would skip registration
for the remaining ones.

  Joao

  reply	other threads:[~2020-03-24 21:31 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-22 16:12 [PATCH v2 0/6] Manual definition of Soft Reserved memory devices Dan Williams
2020-03-22 16:12 ` Dan Williams
2020-03-22 16:12 ` [PATCH v2 1/6] x86/numa: Cleanup configuration dependent command-line options Dan Williams
2020-03-22 16:12   ` Dan Williams
2020-03-22 16:12 ` [PATCH v2 2/6] x86/numa: Add 'nohmat' option Dan Williams
2020-03-22 16:12   ` Dan Williams
2020-03-22 16:12 ` [PATCH v2 3/6] efi/fake_mem: Arrange for a resource entry per efi_fake_mem instance Dan Williams
2020-03-22 16:12   ` Dan Williams
2020-03-22 16:12 ` [PATCH v2 4/6] ACPI: HMAT: Refactor hmat_register_target_device to hmem_register_device Dan Williams
2020-03-22 16:12   ` Dan Williams
2020-03-24 19:40   ` Joao Martins
2020-03-24 19:40     ` Joao Martins
2020-03-24 21:04     ` Dan Williams
2020-03-24 21:04       ` Dan Williams
2020-03-25 22:32       ` Dan Williams
2020-03-25 22:32         ` Dan Williams
2020-03-22 16:12 ` [PATCH v2 5/6] resource: Report parent to walk_iomem_res_desc() callback Dan Williams
2020-03-22 16:12   ` Dan Williams
2020-03-22 16:12 ` [PATCH v2 6/6] ACPI: HMAT: Attach a device for each soft-reserved range Dan Williams
2020-03-22 16:12   ` Dan Williams
2020-03-24 19:41   ` Joao Martins
2020-03-24 19:41     ` Joao Martins
2020-03-24 21:06     ` Dan Williams
2020-03-24 21:06       ` Dan Williams
2020-03-24 21:30       ` Joao Martins [this message]
2020-03-24 21:30         ` Joao Martins
2020-03-25 11:10   ` Will Deacon
2020-03-25 11:10     ` Will Deacon
2020-03-25 17:10     ` Dan Williams
2020-03-25 17:10       ` Dan Williams
2020-03-25 10:02 ` [PATCH v2 0/6] Manual definition of Soft Reserved memory devices Rafael J. Wysocki
2020-03-25 10:02   ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ecfd5f74-cc03-5590-82df-6f7a3dbcdb50@oracle.com \
    --to=joao.m.martins@oracle.com \
    --cc=Brice.Goglin@inria.fr \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=peterz@infradead.org \
    --cc=rjw@rjwysocki.net \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.