[PATCH v4] xen/arm: Skip memory nodes if not enabled

Leo Yan posted 1 patch 6 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20231013120442.1267488-1-leo.yan@linaro.org
xen/arch/arm/bootfdt.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
[PATCH v4] xen/arm: Skip memory nodes if not enabled
Posted by Leo Yan 6 months, 2 weeks ago
Currently, Xen doesn't check the status property of memory/reserved
memory nodes, which may lead to the following issues:

- If a memory node has a status "disabled" it implies that it should
  not be used. Xen does not handle the status property for the memory
  node and ends up using it.

- If a reserved memory node has a status "disabled", it means that this
  region is no longer reserved and can be used, but the "disabled"
  status is not handled by Xen.

  Xen passes the intact device tree binding of the reserved memory nodes
  to Dom0 and creates a memory node to cover reserved regions. Disabled
  reserved memory nodes are ignored by the Dom0 Linux kernel, thus the
  Dom0 Linux kernel will continue to allocate pages from such a region.

  On the other hand, since the disabled status is not handled by Xen,
  the disabled reserved memory regions are excluded from the page
  management in Xen which results in Xen being unable to obtain the
  corresponding MFN, in the end, Xen reports error like:

  (XEN) arch/arm/p2m.c:2202: d0v0: Failing to acquire the MFN 0x1a02dc

This patch introduces a function device_tree_node_is_available(). If it
detects a memory node is not enabled, Xen will not add the memory region
into the memory lists. In the end, this avoids to generate the memory
node for the disabled memory regions sent to the kernel and the kernel
cannot use the disabled memory nodes any longer.

Since this patch adds checking device node's status in the
device_tree_get_meminfo() function, except it checks for memory nodes
and reserved memory nodes, it also supports status for static memory
and static heap.

Suggested-by: Michal Orzel <michal.orzel@amd.com>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
---

Changes from v3:
- Refined the commit log to avoid ambiguous description (Michal Orzel).

Changes from v2:
- Added checking for the DT property length (Julien Grall, Michal Orzel).

Changes from v1:
- Renamed function to device_tree_node_is_available() (Michal Orzel);
- Polished coding style (Michal Orzel);
- Refined commit log (Michal Orzel, Julien Grall).

 xen/arch/arm/bootfdt.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 2673ad17a1..d73f8e4971 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -16,6 +16,24 @@
 #include <xsm/xsm.h>
 #include <asm/setup.h>
 
+static bool __init device_tree_node_is_available(const void *fdt, int node)
+{
+    const char *status;
+    int len;
+
+    status = fdt_getprop(fdt, node, "status", &len);
+    if ( !status )
+        return true;
+
+    if ( len > 0 )
+    {
+        if ( !strcmp(status, "ok") || !strcmp(status, "okay") )
+            return true;
+    }
+
+    return false;
+}
+
 static bool __init device_tree_node_matches(const void *fdt, int node,
                                             const char *match)
 {
@@ -97,6 +115,9 @@ static int __init device_tree_get_meminfo(const void *fdt, int node,
     paddr_t start, size;
     struct meminfo *mem = data;
 
+    if ( !device_tree_node_is_available(fdt, node) )
+        return 0;
+
     if ( address_cells < 1 || size_cells < 1 )
     {
         printk("fdt: property `%s': invalid #address-cells or #size-cells",
-- 
2.39.2
Re: [PATCH v4] xen/arm: Skip memory nodes if not enabled
Posted by Leo Yan 5 months, 3 weeks ago
Hi maintainers,

On Fri, Oct 13, 2023 at 08:04:42PM +0800, Leo Yan wrote:
> Currently, Xen doesn't check the status property of memory/reserved
> memory nodes, which may lead to the following issues:
> 
> - If a memory node has a status "disabled" it implies that it should
>   not be used. Xen does not handle the status property for the memory
>   node and ends up using it.
> 
> - If a reserved memory node has a status "disabled", it means that this
>   region is no longer reserved and can be used, but the "disabled"
>   status is not handled by Xen.
> 
>   Xen passes the intact device tree binding of the reserved memory nodes
>   to Dom0 and creates a memory node to cover reserved regions. Disabled
>   reserved memory nodes are ignored by the Dom0 Linux kernel, thus the
>   Dom0 Linux kernel will continue to allocate pages from such a region.
> 
>   On the other hand, since the disabled status is not handled by Xen,
>   the disabled reserved memory regions are excluded from the page
>   management in Xen which results in Xen being unable to obtain the
>   corresponding MFN, in the end, Xen reports error like:
> 
>   (XEN) arch/arm/p2m.c:2202: d0v0: Failing to acquire the MFN 0x1a02dc
> 
> This patch introduces a function device_tree_node_is_available(). If it
> detects a memory node is not enabled, Xen will not add the memory region
> into the memory lists. In the end, this avoids to generate the memory
> node for the disabled memory regions sent to the kernel and the kernel
> cannot use the disabled memory nodes any longer.
> 
> Since this patch adds checking device node's status in the
> device_tree_get_meminfo() function, except it checks for memory nodes
> and reserved memory nodes, it also supports status for static memory
> and static heap.
> 
> Suggested-by: Michal Orzel <michal.orzel@amd.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Gentle ping.

I don't see this patch is landed in Xen master or staging branch, should
anything I need to follow up?

Thanks,
Leo
Re: [PATCH v4] xen/arm: Skip memory nodes if not enabled
Posted by Julien Grall 5 months, 3 weeks ago

On 06/11/2023 02:18, Leo Yan wrote:
> Hi maintainers,

Hi Leo,

> On Fri, Oct 13, 2023 at 08:04:42PM +0800, Leo Yan wrote:
>> Currently, Xen doesn't check the status property of memory/reserved
>> memory nodes, which may lead to the following issues:
>>
>> - If a memory node has a status "disabled" it implies that it should
>>    not be used. Xen does not handle the status property for the memory
>>    node and ends up using it.
>>
>> - If a reserved memory node has a status "disabled", it means that this
>>    region is no longer reserved and can be used, but the "disabled"
>>    status is not handled by Xen.
>>
>>    Xen passes the intact device tree binding of the reserved memory nodes
>>    to Dom0 and creates a memory node to cover reserved regions. Disabled
>>    reserved memory nodes are ignored by the Dom0 Linux kernel, thus the
>>    Dom0 Linux kernel will continue to allocate pages from such a region.
>>
>>    On the other hand, since the disabled status is not handled by Xen,
>>    the disabled reserved memory regions are excluded from the page
>>    management in Xen which results in Xen being unable to obtain the
>>    corresponding MFN, in the end, Xen reports error like:
>>
>>    (XEN) arch/arm/p2m.c:2202: d0v0: Failing to acquire the MFN 0x1a02dc
>>
>> This patch introduces a function device_tree_node_is_available(). If it
>> detects a memory node is not enabled, Xen will not add the memory region
>> into the memory lists. In the end, this avoids to generate the memory
>> node for the disabled memory regions sent to the kernel and the kernel
>> cannot use the disabled memory nodes any longer.
>>
>> Since this patch adds checking device node's status in the
>> device_tree_get_meminfo() function, except it checks for memory nodes
>> and reserved memory nodes, it also supports status for static memory
>> and static heap.
>>
>> Suggested-by: Michal Orzel <michal.orzel@amd.com>
>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
>> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> 
> Gentle ping.
> 
> I don't see this patch is landed in Xen master or staging branch, should
> anything I need to follow up?

The tree has been frozen for the past few weeks for any patches that are 
not meant for 4.18. We branched for 4.18 last week so staging is now in 
soft-reopening until 4.18 is released.

I am aware of few patches that are ready to be merged. But I haven't yet 
gone through all of them and merge to 4.19. It will probably be done 
once 4.18 is released.

Cheers,

-- 
Julien Grall
Re: [PATCH v4] xen/arm: Skip memory nodes if not enabled
Posted by Leo Yan 5 months, 3 weeks ago
Hi Julien,

On Mon, Nov 06, 2023 at 09:52:45AM +0000, Julien Grall wrote:

[...]

> > Gentle ping.
> > 
> > I don't see this patch is landed in Xen master or staging branch, should
> > anything I need to follow up?
> 
> The tree has been frozen for the past few weeks for any patches that are not
> meant for 4.18. We branched for 4.18 last week so staging is now in
> soft-reopening until 4.18 is released.

Thank you for the info.

> I am aware of few patches that are ready to be merged. But I haven't yet
> gone through all of them and merge to 4.19. It will probably be done once
> 4.18 is released.

Sure, good to know this.  I will wait a bit and just let me know if
I need to follow up anything.

Thanks,
Leo
Re: [PATCH v4] xen/arm: Skip memory nodes if not enabled
Posted by Julien Grall 5 months, 3 weeks ago

On 06/11/2023 10:32, Leo Yan wrote:
> Hi Julien,
> 
> On Mon, Nov 06, 2023 at 09:52:45AM +0000, Julien Grall wrote:
> 
> [...]
> 
>>> Gentle ping.
>>>
>>> I don't see this patch is landed in Xen master or staging branch, should
>>> anything I need to follow up?
>>
>> The tree has been frozen for the past few weeks for any patches that are not
>> meant for 4.18. We branched for 4.18 last week so staging is now in
>> soft-reopening until 4.18 is released.
> 
> Thank you for the info.
> 
>> I am aware of few patches that are ready to be merged. But I haven't yet
>> gone through all of them and merge to 4.19. It will probably be done once
>> 4.18 is released.
> 
> Sure, good to know this.  I will wait a bit and just let me know if
> I need to follow up anything.

Please ping me in ~ 2 weeks time if your patch is still not merged.

@Stefano, did you add this patch in your for-next branch?

-- 
Julien Grall
Re: [PATCH v4] xen/arm: Skip memory nodes if not enabled
Posted by Julien Grall 5 months, 2 weeks ago
Hi,

On 06/11/2023 11:31, Julien Grall wrote:
> On 06/11/2023 10:32, Leo Yan wrote:
>> Hi Julien,
>>
>> On Mon, Nov 06, 2023 at 09:52:45AM +0000, Julien Grall wrote:
>>
>> [...]
>>
>>>> Gentle ping.
>>>>
>>>> I don't see this patch is landed in Xen master or staging branch, 
>>>> should
>>>> anything I need to follow up?
>>>
>>> The tree has been frozen for the past few weeks for any patches that 
>>> are not
>>> meant for 4.18. We branched for 4.18 last week so staging is now in
>>> soft-reopening until 4.18 is released.
>>
>> Thank you for the info.
>>
>>> I am aware of few patches that are ready to be merged. But I haven't yet
>>> gone through all of them and merge to 4.19. It will probably be done 
>>> once
>>> 4.18 is released.
>>
>> Sure, good to know this.  I will wait a bit and just let me know if
>> I need to follow up anything.
> 
> Please ping me in ~ 2 weeks time if your patch is still not merged.
> 
> @Stefano, did you add this patch in your for-next branch?

It is now committed.

Cheers,

-- 
Julien Grall