| Message ID | 20230928143110.248221-1-leo.yan@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] xen/arm: Skip memory nodes if not enabled | expand |
Hi Leo, On 28/09/2023 16:31, Leo Yan wrote: > > > Currently, the Xen hypervisor doesn't handle the status, the issue can > be described from two perspectives: the memory nodes and the reserved > memory nodes. > > - 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> > --- > > 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 | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > index 2673ad17a1..1b80d2d622 100644 > --- a/xen/arch/arm/bootfdt.c > +++ b/xen/arch/arm/bootfdt.c > @@ -16,6 +16,19 @@ > #include <xsm/xsm.h> > #include <asm/setup.h> > > +static bool __init device_tree_node_is_available(const void *fdt, int node) > +{ > + const char *status = fdt_getprop(fdt, node, "status", NULL); Please see Julien's comment for v1. To save some jumps,instructions we should also check for length of the property to be > 0, just like we do in dt_device_is_available(). Apart from that the patch looks good. ~Michal
Hi Michal, On Fri, Sep 29, 2023 at 10:11:19AM +0200, Michal Orzel wrote: [...] > > +static bool __init device_tree_node_is_available(const void *fdt, int node) > > +{ > > + const char *status = fdt_getprop(fdt, node, "status", NULL); > Please see Julien's comment for v1. To save some jumps,instructions > we should also check for length of the property to be > 0, just like we do in dt_device_is_available(). Sorry for I missed Julien's comment. Have sent patch v3 for review. And apologize for late replying. Leo
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index 2673ad17a1..1b80d2d622 100644 --- a/xen/arch/arm/bootfdt.c +++ b/xen/arch/arm/bootfdt.c @@ -16,6 +16,19 @@ #include <xsm/xsm.h> #include <asm/setup.h> +static bool __init device_tree_node_is_available(const void *fdt, int node) +{ + const char *status = fdt_getprop(fdt, node, "status", NULL); + + if ( !status ) + return true; + + 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 +110,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",
Currently, the Xen hypervisor doesn't handle the status, the issue can be described from two perspectives: the memory nodes and the reserved memory nodes. - 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> --- 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 | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)