Re: [PATCH 3/5] xen/arm: handle "multiboot, dtb" compatible nodes

[Julien Grall <julien.grall@arm.com>]

Hi,

On 12/5/18 5:28 PM, Stefano Stabellini wrote:
> Detect "multiboot,dtb" compatible nodes. Add them to the bootmod array
> as BOOTMOD_DTB.  In kernel_probe, find the right BOOTMOD_DTB and store a
> pointer to it in dtb_bootmodule.

This is too close to the name BOOTMOD_FDT. So this will confuse more 
than one developer.

This probably want to be renamed to BOOTMOD_GUEST_DTB.

> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> ---
>   xen/arch/arm/bootfdt.c      |  2 ++
>   xen/arch/arm/kernel.c       | 12 +++++++++++-
>   xen/arch/arm/setup.c        |  1 +
>   xen/include/asm-arm/setup.h |  1 +
>   4 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index 72cb8d6..ad4fbac 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -205,6 +205,8 @@ static void __init process_multiboot_node(const void *fdt, int node,
>           kind = BOOTMOD_RAMDISK;
>       else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 )
>           kind = BOOTMOD_XSM;
> +    else if ( fdt_node_check_compatible(fdt, node, "multiboot,dtb") == 0 )

I would prefer an explicit name such as "multiboot,device-tree".

> +        kind = BOOTMOD_DTB;
>       else
>           kind = BOOTMOD_UNKNOWN;
>   
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index d04a862..8918d75 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -426,7 +426,7 @@ int __init kernel_probe(struct kernel_info *info,
>       struct bootmodule *mod = NULL;
>       struct bootcmdline *cmd = NULL;
>       struct dt_device_node *node;
> -    u64 kernel_addr, initrd_addr, size;
> +    u64 kernel_addr = 0, initrd_addr = 0, dtb_addr = 0, size;
>       int rc;
>   
>       /* domain is NULL only for the hardware domain */
> @@ -470,6 +470,16 @@ int __init kernel_probe(struct kernel_info *info,
>                   info->initrd_bootmodule = boot_module_find_by_addr_and_kind(
>                           BOOTMOD_RAMDISK, initrd_addr);
>               }
> +            else if ( dt_device_is_compatible(node, "multiboot,dtb") )
> +            {
> +                u32 len;
> +                const __be32 *val;
> +
> +                val = dt_get_property(node, "reg", &len);
> +                dt_get_range(&val, node, &dtb_addr, &size);
> +                info->dtb_bootmodule = boot_module_find_by_addr_and_kind(
> +                        BOOTMOD_DTB, dtb_addr);
> +            }
>               else
>                   continue;
>           }
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index e83221a..b3de0a2 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -344,6 +344,7 @@ const char * __init boot_module_kind_as_string(bootmodule_kind kind)
>       case BOOTMOD_KERNEL:  return "Kernel";
>       case BOOTMOD_RAMDISK: return "Ramdisk";
>       case BOOTMOD_XSM:     return "XSM";
> +    case BOOTMOD_DTB:     return "DTB";
>       case BOOTMOD_UNKNOWN: return "Unknown";
>       default: BUG();
>       }
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index 48187e1..0ff0768 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -16,6 +16,7 @@ typedef enum {
>       BOOTMOD_KERNEL,
>       BOOTMOD_RAMDISK,
>       BOOTMOD_XSM,
> +    BOOTMOD_DTB,
>       BOOTMOD_UNKNOWN
>   }  bootmodule_kind;
>   
> 

Cheers,

-- 
Julien Grall

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