Re: [Xen-devel] [PATCH] xen: arm: implement generic multiboot compatibility strings (Was: Re: [Linaro-uefi] The GRUB multiboot support patch for aarch64(V3.1))

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Ian,

On 06/05/2014 12:56 PM, Ian Campbell wrote:
> On Thu, 2014-06-05 at 15:46 +0800, Fu Wei wrote:
>>> I can see why you did this (it's what Xen actually supports today) but I
>>> wonder if in the interests of reducing the special cases I should create
>>> a Xen patch which causes it to accept both names so that you can just
>>> implement the "multiboot,*" stuff in common code without the special
>>> cases? (Although that depends on the reason for the other non-compat
>>> string special cases too)
>>
>> I think the "multiboot,*" stuff will be better, once Xen supports
>> this, I will update this code.
> 
> I've only compile tested this, but I *think* this does the right thing.
> 
> Ian.
> 
> 8<-----------------
> 
> From d0acce53a086869420c2d8870d1a8a058013d6b5 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Date: Wed, 4 Jun 2014 18:17:10 +0100
> Subject: [PATCH] xen: arm: implement generic multiboot compatibility strings
> 
> This causes Xen to accept the more generic names originally proposed by
> Andre in http://thread.gmane.org/gmane.linux.linaro.announce.boot/326 and
> incorporated into the proposal in
> http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/Multiboot
> 
> This will allow bootloaders to not special case Xen (or at least to reduce
> the amount which is required).
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
>  xen/common/device_tree.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index f0b17a3..5040097 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -338,9 +338,11 @@ static void __init process_multiboot_node(const void 
> *fdt, int node,
>      struct dt_mb_module *mod;
>      int len;
>  
> -    if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 )
> +    if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 ||
> +         fdt_node_check_compatible(fdt, node, "multiboot,linux-zimage") == 0 
> )

While we are modifying the protocol, "linux-zImage" is confusing in the
name. Actually we can use it for an ELF, another OS... I don't think Xen
will change his behavior depending of the DOM0 image.

I would rename it to something more generic such as "multiboot,kernel".
This will avoid adding a new compatible string every time we support a
new format/OS.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel