Re: [Xen-devel] [PATCH v3 3/6] xen/arm: keep track of reserved-memory regions

[Oleksandr <olekstysh@xxxxxxxxx>]

On 22.06.19 02:56, Stefano Stabellini wrote:

Hi Stefano

> As we parse the device tree in Xen, keep track of the reserved-memory
> regions as they need special treatment (follow-up patches will make use
> of the stored information.)
>
> Reuse process_memory_node to add reserved-memory regions to the
> bootinfo.reserved_mem array.
>
> Refuse to continue once we reach the max number of reserved memory
> regions to avoid accidentally mapping any portions of them into a VM.
>
> Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
>
> ---
> It is cleaner to avoid sharing the whole function process_memory_node
> between the normal memory case and the reserved-memory case. I'll do it
> in the next version once I understand the best way do to it.
>
> ---
> Changes in v3:
> - match only /reserved-memory
> - put the warning back in place for reg not present on a normal memory
>    region
> - refuse to continue once we reach the max number of reserved memory
>    regions
>
> Changes in v2:
> - call process_memory_node from process_reserved_memory_node to avoid
>    duplication
> ---
>   xen/arch/arm/bootfdt.c      | 38 +++++++++++++++++++++++++++++++------
>   xen/include/asm-arm/setup.h |  1 +
>   2 files changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index 611724433b..b24ab10cb9 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -135,6 +135,8 @@ static int __init process_memory_node(const void *fdt, int 
> node,
>       const __be32 *cell;
>       paddr_t start, size;
>       u32 reg_cells = address_cells + size_cells;
> +    struct meminfo *mem;
> +    bool reserved = (bool)data;
>   
>       if ( address_cells < 1 || size_cells < 1 )
>       {
> @@ -143,29 +145,49 @@ static int __init process_memory_node(const void *fdt, 
> int node,
>           return 0;
>       }
>   
> +    if ( reserved )
> +        mem = &bootinfo.reserved_mem;
> +    else
> +        mem = &bootinfo.mem;
> +
>       prop = fdt_get_property(fdt, node, "reg", NULL);
>       if ( !prop )
>       {
> -        printk("fdt: node `%s': missing `reg' property\n", name);
> +        if ( !reserved )
> +            printk("fdt: node `%s': missing `reg' property\n", name);
>           return 0;
>       }
>   
>       cell = (const __be32 *)prop->data;
>       banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
>   
> -    for ( i = 0; i < banks && bootinfo.mem.nr_banks < NR_MEM_BANKS; i++ )
> +    for ( i = 0; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ )
>       {
>           device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
>           if ( !size )
>               continue;
> -        bootinfo.mem.bank[bootinfo.mem.nr_banks].start = start;
> -        bootinfo.mem.bank[bootinfo.mem.nr_banks].size = size;
> -        bootinfo.mem.nr_banks++;
> +        mem->bank[mem->nr_banks].start = start;
> +        mem->bank[mem->nr_banks].size = size;
> +        mem->nr_banks++;
>       }
> +    /*
> +     * We reached the max number of supported reserved-memory regions.
> +     * Stop and refuse to continue. We don't want to risk Xen allocating
> +     * those regions as normal memory to a VM.
> +     */
> +    BUG_ON(reserved && mem->nr_banks == NR_MEM_BANKS);
>   
>       return 0;
>   }
>   
> +static int __init process_reserved_memory_node(const void *fdt, int node,
> +                                               const char *name, int depth,
> +                                               u32 address_cells, u32 
> size_cells)
> +{
> +    device_tree_for_each_node(fdt, node, depth, process_memory_node, 
> (void*)true);
> +    return 0;
> +}
I have tested this series and got the same behavior as with V2 [1].

The "non-reserved-memory" node in my device-tree 
(sram@47FFF000->scp_shmem@0) is still interpreted as a "reserved-memory".
I think, this takes place because current implementation iterates over 
all device tree nodes starting
from real "reserved-memory" node up to the end of the device tree. And 
if there is at least one "non-reserved-memory" node (with a suitable 
depth and valid "reg" property)
located down the device tree, it will be mistakenly inserted to 
bootinfo.reserved_mem (as in my case).
----------

Unpacked device tree:
{
        ...

        memory@48000000 {
                device_type = "memory";
                reg = <0x00000000 0x48000000 0x00000000 0x78000000 
0x00000005 0x00000000 0x00000000 0x80000000 0x00000006 0x00000000 
0x00000000 0x80000000 0x00000007 0x00000000 0x00000000 0x80000000>;
        };
        reserved-memory {
                #address-cells = <0x00000002>;
                #size-cells = <0x00000002>;
                ranges;
                linux,lossy_decompress@54000000 {
                        no-map;
                        reg = <0x00000000 0x54000000 0x00000000 
0x03000000>;
                        linux,phandle = <0x000000b0>;
                        phandle = <0x000000b0>;
                };
                linux,adsp@57000000 {
                        compatible = "shared-dma-pool";
                        reusable;
                        reg = <0x00000000 0x57000000 0x00000000 
0x01000000>;
                };
                linux,cma@58000000 {
                        compatible = "shared-dma-pool";
                        reusable;
                        reg = <0x00000000 0x58000000 0x00000000 
0x18000000>;
                        linux,cma-default;
                };
                linux,multimedia@70000000 {
                        compatible = "shared-dma-pool";
                        reusable;
                        reg = <0x00000000 0x70000000 0x00000000 
0x10000000>;
                        linux,phandle = <0x000000af>;
                        phandle = <0x000000af>;
                };
        };

        ...

        sram@47FFF000 {
                compatible = "mmio-sram";
                reg = <0x00000000 0x47fff000 0x00000000 0x00001000>;
                #address-cells = <0x00000001>;
                #size-cells = <0x00000001>;
                ranges = <0x00000000 0x00000000 0x47fff000 0x00001000>;
                scp_shmem@0 {
                        compatible = "mmio-sram";
                        reg = <0x00000000 0x00000200>;
                        linux,phandle = <0x000000b2>;
                        phandle = <0x000000b2>;
                };
        };

        ...
};

----------

Xen log:

Starting kernel ...

- UART enabled -
- CPU 00000000 booting -
- Current EL 00000008 -
- Xen starting at EL2 -
- Zero BSS -
- Setting up control registers -
- Turning on paging -
- Ready -
(XEN) Checking for initrd in /chosen
(XEN) Initrd 0000000074000040-0000000076252c54
(XEN) node memory@48000000: insert bank 0: 0x48000000->0xc0000000 type: 
normal
(XEN) node memory@48000000: insert bank 1: 0x500000000->0x580000000 
type: normal
(XEN) node memory@48000000: insert bank 2: 0x600000000->0x680000000 
type: normal
(XEN) node memory@48000000: insert bank 3: 0x700000000->0x780000000 
type: normal
(XEN) node linux,lossy_decompress@54000000: insert bank 0: 
0x54000000->0x57000000 type: reserved
(XEN) node linux,adsp@57000000: insert bank 0: 0x57000000->0x58000000 
type: reserved
(XEN) node linux,cma@58000000: insert bank 0: 0x58000000->0x70000000 
type: reserved
(XEN) node linux,multimedia@70000000: insert bank 0: 
0x70000000->0x80000000 type: reserved
(XEN) node scp_shmem@0: insert bank 0: 0->0x200 type: reserved 
<--------------- Not a reserved memory
(XEN) RAM: 0000000048000000 - 00000000bfffffff
(XEN) RAM: 0000000500000000 - 000000057fffffff
(XEN) RAM: 0000000600000000 - 000000067fffffff
(XEN) RAM: 0000000700000000 - 000000077fffffff
(XEN)
(XEN) MODULE[0]: 0000000048000000 - 0000000048014080 Device Tree
(XEN) MODULE[1]: 0000000074000040 - 0000000076252c54 Ramdisk
(XEN) MODULE[2]: 000000007a000000 - 000000007c000000 Kernel
(XEN) MODULE[3]: 000000007c000000 - 000000007c010000 XSM
(XEN)  RESVD[0]: 0000000048000000 - 0000000048014000
(XEN)  RESVD[1]: 0000000074000040 - 0000000076252c54

----------

[1] 
https://www.mail-archive.com/xen-devel@xxxxxxxxxxxxxxxxxxxx/msg45632.html

--
Regards,

Oleksandr Tyshchenko


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel