[Xen-devel] [PATCH v2 2/3] util/grub.d/20_linux_xen.in: Add arm64 support

2015-07-13 Thread fu . wei
From: Fu Wei 

This patch adds the support of boot command on arm64 for XEN:
xen_hypervisor
xen_module

Signed-off-by: Fu Wei 
---
 util/grub.d/20_linux_xen.in | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index f532fb9..b52c50d 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -120,16 +120,16 @@ linux_entry ()
 else
 xen_rm_opts="no-real-mode edd=off"
 fi
-   multiboot   ${rel_xen_dirname}/${xen_basename} placeholder 
${xen_args} \${xen_rm_opts}
+   ${multiboot_cmd}${rel_xen_dirname}/${xen_basename} placeholder 
${xen_args} \${xen_rm_opts}
echo'$(echo "$lmessage" | grub_quote)'
-   module  ${rel_dirname}/${basename} placeholder 
root=${linux_root_device_thisversion} ro ${args}
+   ${module_cmd}   ${rel_dirname}/${basename} placeholder 
root=${linux_root_device_thisversion} ro ${args}
 EOF
   if test -n "${initrd}" ; then
 # TRANSLATORS: ramdisk isn't identifier. Should be translated.
 message="$(gettext_printf "Loading initial ramdisk ...")"
 sed "s/^/$submenu_indentation/" << EOF
echo'$(echo "$message" | grub_quote)'
-   module  --nounzip   ${rel_dirname}/${initrd}
+   ${module_cmd}   --nounzip   ${rel_dirname}/${initrd}
 EOF
   fi
   sed "s/^/$submenu_indentation/" << EOF
@@ -185,6 +185,14 @@ case "$machine" in
 *) GENKERNEL_ARCH="$machine" ;;
 esac
 
+if [ "x$machine" != xaarch64 ]; then
+   multiboot_cmd="multiboot"
+   module_cmd="module"
+else
+   multiboot_cmd="xen_hypervisor"
+   module_cmd="xen_module"
+fi
+
 # Extra indentation to add to menu entries in a submenu. We're not in a submenu
 # yet, so it's empty. In a submenu it will be equal to '\t' (one tab).
 submenu_indentation=""
-- 
1.8.3.1


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


Re: [Xen-devel] [PATCH v2 2/3] util/grub.d/20_linux_xen.in: Add arm64 support

2015-07-13 Thread Andrei Borzenkov
В Mon, 13 Jul 2015 16:53:59 +0800
fu@linaro.org пишет:

> From: Fu Wei 
> 
> This patch adds the support of boot command on arm64 for XEN:
> xen_hypervisor
> xen_module
> 
> Signed-off-by: Fu Wei 
> ---
>  util/grub.d/20_linux_xen.in | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
> index f532fb9..b52c50d 100644
> --- a/util/grub.d/20_linux_xen.in
> +++ b/util/grub.d/20_linux_xen.in
> @@ -120,16 +120,16 @@ linux_entry ()
>  else
>  xen_rm_opts="no-real-mode edd=off"
>  fi
> - multiboot   ${rel_xen_dirname}/${xen_basename} placeholder 
> ${xen_args} \${xen_rm_opts}
> + ${multiboot_cmd}${rel_xen_dirname}/${xen_basename} placeholder 
> ${xen_args} \${xen_rm_opts}
>   echo'$(echo "$lmessage" | grub_quote)'
> - module  ${rel_dirname}/${basename} placeholder 
> root=${linux_root_device_thisversion} ro ${args}
> + ${module_cmd}   ${rel_dirname}/${basename} placeholder 
> root=${linux_root_device_thisversion} ro ${args}
>  EOF
>if test -n "${initrd}" ; then
>  # TRANSLATORS: ramdisk isn't identifier. Should be translated.
>  message="$(gettext_printf "Loading initial ramdisk ...")"
>  sed "s/^/$submenu_indentation/" << EOF
>   echo'$(echo "$message" | grub_quote)'
> - module  --nounzip   ${rel_dirname}/${initrd}
> + ${module_cmd}   --nounzip   ${rel_dirname}/${initrd}
>  EOF
>fi
>sed "s/^/$submenu_indentation/" << EOF
> @@ -185,6 +185,14 @@ case "$machine" in
>  *) GENKERNEL_ARCH="$machine" ;;
>  esac
>  
> +if [ "x$machine" != xaarch64 ]; then
> + multiboot_cmd="multiboot"
> + module_cmd="module"
> +else
> + multiboot_cmd="xen_hypervisor"
> + module_cmd="xen_module"
> +fi
> +

Strictly speaking, this is boot-time decision. As mentioned by
Vladimir, better would be to provide alias xen_hypervisor and
xen_module in multiboot for platforms supporting Xen (is MIPS really
supported?) and use it consistently.

>  # Extra indentation to add to menu entries in a submenu. We're not in a 
> submenu
>  # yet, so it's empty. In a submenu it will be equal to '\t' (one tab).
>  submenu_indentation=""


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


Re: [Xen-devel] [PATCH v2 2/3] util/grub.d/20_linux_xen.in: Add arm64 support

2015-07-14 Thread Ian Campbell
On Tue, 2015-07-14 at 06:53 +0300, Andrei Borzenkov wrote:
> > +if [ "x$machine" != xaarch64 ]; then
> > +   multiboot_cmd="multiboot"
> > +   module_cmd="module"
> > +else
> > +   multiboot_cmd="xen_hypervisor"
> > +   module_cmd="xen_module"
> > +fi
> > +
> 
> Strictly speaking, this is boot-time decision. As mentioned by
> Vladimir, better would be to provide alias xen_hypervisor and
> xen_module in multiboot for platforms supporting Xen (is MIPS really
> supported?) and use it consistently.

I had been thinking of this the other way around, e.g. on platforms
which support Xen but not multiboot1 "multiboot" would be added as an
alias for xen_hypervisor.

However so long as grub-mkconfig (via 20_linux_xen) work for everyone
and that peoples existing hand-crafted x86/multiboot/Xen grub.cfg's
continue to work then I think having the alias go either way would be
fine.

BTW I had been going to suggest a function at the grub.cfg level which
dispatched to the correct command, but I suppose an actual alias is
better.

Ian.


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


Re: [Xen-devel] [PATCH v2 2/3] util/grub.d/20_linux_xen.in: Add arm64 support

2015-07-14 Thread Fu Wei
Hi Andrei,

Great thanks for your review.

So Are you suggesting this:
(1) in util/grub.d/20_linux_xen.in, we only use xen_hypervisor/xen_module.
(2) in xen_boot.c, we only register command xen_hypervisor/xen_module.
(3) in grub-core/loader/i386/xen.c, we *add*
---
  cmd_xen_hypervisort = grub_register_command ("xen_hypervisor", grub_cmd_xen,
0, N_("Load Linux."));
  cmd_xen_module = grub_register_command ("xen_module", grub_cmd_module,
 0, N_("Load module."));
---
(4)in grub-core/loader/multiboot.c, we *add*
---
#if defined (__i386__) || defined (__aarch64__)
  cmd_xen_hypervisort =
grub_register_command ("xen_hypervisor", grub_cmd_multiboot,
  0, N_("Load a multiboot kernel."));
  cmd_xen_module =
grub_register_command ("xen_module", grub_cmd_module,
  0, N_("Load a multiboot module."));
#endif
---

BTW, from the source code, MIPS isn't supported by multiboot,  IS
supported by multiboot2.

Please correct me. If I misunderstand your suggestion. :-)

Thanks again!


On 14 July 2015 at 11:53, Andrei Borzenkov  wrote:
> В Mon, 13 Jul 2015 16:53:59 +0800
> fu@linaro.org пишет:
>
>> From: Fu Wei 
>>
>> This patch adds the support of boot command on arm64 for XEN:
>> xen_hypervisor
>> xen_module
>>
>> Signed-off-by: Fu Wei 
>> ---
>>  util/grub.d/20_linux_xen.in | 14 +++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
>> index f532fb9..b52c50d 100644
>> --- a/util/grub.d/20_linux_xen.in
>> +++ b/util/grub.d/20_linux_xen.in
>> @@ -120,16 +120,16 @@ linux_entry ()
>>  else
>>  xen_rm_opts="no-real-mode edd=off"
>>  fi
>> - multiboot   ${rel_xen_dirname}/${xen_basename} placeholder 
>> ${xen_args} \${xen_rm_opts}
>> + ${multiboot_cmd}${rel_xen_dirname}/${xen_basename} placeholder 
>> ${xen_args} \${xen_rm_opts}
>>   echo'$(echo "$lmessage" | grub_quote)'
>> - module  ${rel_dirname}/${basename} placeholder 
>> root=${linux_root_device_thisversion} ro ${args}
>> + ${module_cmd}   ${rel_dirname}/${basename} placeholder 
>> root=${linux_root_device_thisversion} ro ${args}
>>  EOF
>>if test -n "${initrd}" ; then
>>  # TRANSLATORS: ramdisk isn't identifier. Should be translated.
>>  message="$(gettext_printf "Loading initial ramdisk ...")"
>>  sed "s/^/$submenu_indentation/" << EOF
>>   echo'$(echo "$message" | grub_quote)'
>> - module  --nounzip   ${rel_dirname}/${initrd}
>> + ${module_cmd}   --nounzip   ${rel_dirname}/${initrd}
>>  EOF
>>fi
>>sed "s/^/$submenu_indentation/" << EOF
>> @@ -185,6 +185,14 @@ case "$machine" in
>>  *) GENKERNEL_ARCH="$machine" ;;
>>  esac
>>
>> +if [ "x$machine" != xaarch64 ]; then
>> + multiboot_cmd="multiboot"
>> + module_cmd="module"
>> +else
>> + multiboot_cmd="xen_hypervisor"
>> + module_cmd="xen_module"
>> +fi
>> +
>
> Strictly speaking, this is boot-time decision. As mentioned by
> Vladimir, better would be to provide alias xen_hypervisor and
> xen_module in multiboot for platforms supporting Xen (is MIPS really
> supported?) and use it consistently.
>
>>  # Extra indentation to add to menu entries in a submenu. We're not in a 
>> submenu
>>  # yet, so it's empty. In a submenu it will be equal to '\t' (one tab).
>>  submenu_indentation=""
>



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021

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


Re: [Xen-devel] [PATCH v2 2/3] util/grub.d/20_linux_xen.in: Add arm64 support

2015-07-14 Thread Konrad Rzeszutek Wilk
On Tue, Jul 14, 2015 at 10:41:28AM +0100, Ian Campbell wrote:
> On Tue, 2015-07-14 at 06:53 +0300, Andrei Borzenkov wrote:
> > > +if [ "x$machine" != xaarch64 ]; then
> > > + multiboot_cmd="multiboot"
> > > + module_cmd="module"

And we should use the grub-file --is-multiboot2 to figure out if the
Xen binary can also do that - and use multiboot2 protocol.

But that patch I can cobble after this one is done.

> > > +else
> > > + multiboot_cmd="xen_hypervisor"
> > > + module_cmd="xen_module"
> > > +fi
> > > +
> > 
> > Strictly speaking, this is boot-time decision. As mentioned by
> > Vladimir, better would be to provide alias xen_hypervisor and
> > xen_module in multiboot for platforms supporting Xen (is MIPS really
> > supported?) and use it consistently.
> 
> I had been thinking of this the other way around, e.g. on platforms
> which support Xen but not multiboot1 "multiboot" would be added as an
> alias for xen_hypervisor.
> 
> However so long as grub-mkconfig (via 20_linux_xen) work for everyone
> and that peoples existing hand-crafted x86/multiboot/Xen grub.cfg's
> continue to work then I think having the alias go either way would be
> fine.
> 
> BTW I had been going to suggest a function at the grub.cfg level which
> dispatched to the correct command, but I suppose an actual alias is
> better.
> 
> Ian.
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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


Re: [Xen-devel] [PATCH v2 2/3] util/grub.d/20_linux_xen.in: Add arm64 support

2015-07-15 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 14.07.2015 15:09, Fu Wei wrote:
> Hi Andrei,
> 
> Great thanks for your review.
> 
> So Are you suggesting this:
> (1) in util/grub.d/20_linux_xen.in, we only use xen_hypervisor/xen_module.
> (2) in xen_boot.c, we only register command xen_hypervisor/xen_module.
> (3) in grub-core/loader/i386/xen.c, we *add*
> ---
>   cmd_xen_hypervisort = grub_register_command ("xen_hypervisor", grub_cmd_xen,
> 0, N_("Load Linux."));
>   cmd_xen_module = grub_register_command ("xen_module", grub_cmd_module,
>  0, N_("Load module."));
No. This is for pvgrub2.
> ---
> (4)in grub-core/loader/multiboot.c, we *add*
> ---
> #if defined (__i386__) || defined (__aarch64__)
>   cmd_xen_hypervisort =
> grub_register_command ("xen_hypervisor", grub_cmd_multiboot,
>   0, N_("Load a multiboot kernel."));
>   cmd_xen_module =
> grub_register_command ("xen_module", grub_cmd_module,
>   0, N_("Load a multiboot module."));
> #endif
No. Don't mix arm64 xen with multiboot. Neither in command names nor in
the descriptions.
> ---
> 
> BTW, from the source code, MIPS isn't supported by multiboot,  IS
> supported by multiboot2.
> 
> Please correct me. If I misunderstand your suggestion. :-)
> 
> Thanks again!
> 
> 
> On 14 July 2015 at 11:53, Andrei Borzenkov  wrote:
>> В Mon, 13 Jul 2015 16:53:59 +0800
>> fu@linaro.org пишет:
>>
>>> From: Fu Wei 
>>>
>>> This patch adds the support of boot command on arm64 for XEN:
>>> xen_hypervisor
>>> xen_module
>>>
>>> Signed-off-by: Fu Wei 
>>> ---
>>>  util/grub.d/20_linux_xen.in | 14 +++---
>>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
>>> index f532fb9..b52c50d 100644
>>> --- a/util/grub.d/20_linux_xen.in
>>> +++ b/util/grub.d/20_linux_xen.in
>>> @@ -120,16 +120,16 @@ linux_entry ()
>>>  else
>>>  xen_rm_opts="no-real-mode edd=off"
>>>  fi
>>> - multiboot   ${rel_xen_dirname}/${xen_basename} placeholder 
>>> ${xen_args} \${xen_rm_opts}
>>> + ${multiboot_cmd}${rel_xen_dirname}/${xen_basename} 
>>> placeholder ${xen_args} \${xen_rm_opts}
>>>   echo'$(echo "$lmessage" | grub_quote)'
>>> - module  ${rel_dirname}/${basename} placeholder 
>>> root=${linux_root_device_thisversion} ro ${args}
>>> + ${module_cmd}   ${rel_dirname}/${basename} placeholder 
>>> root=${linux_root_device_thisversion} ro ${args}
>>>  EOF
>>>if test -n "${initrd}" ; then
>>>  # TRANSLATORS: ramdisk isn't identifier. Should be translated.
>>>  message="$(gettext_printf "Loading initial ramdisk ...")"
>>>  sed "s/^/$submenu_indentation/" << EOF
>>>   echo'$(echo "$message" | grub_quote)'
>>> - module  --nounzip   ${rel_dirname}/${initrd}
>>> + ${module_cmd}   --nounzip   ${rel_dirname}/${initrd}
>>>  EOF
>>>fi
>>>sed "s/^/$submenu_indentation/" << EOF
>>> @@ -185,6 +185,14 @@ case "$machine" in
>>>  *) GENKERNEL_ARCH="$machine" ;;
>>>  esac
>>>
>>> +if [ "x$machine" != xaarch64 ]; then
>>> + multiboot_cmd="multiboot"
>>> + module_cmd="module"
>>> +else
>>> + multiboot_cmd="xen_hypervisor"
>>> + module_cmd="xen_module"
>>> +fi
>>> +
>>
>> Strictly speaking, this is boot-time decision. As mentioned by
>> Vladimir, better would be to provide alias xen_hypervisor and
>> xen_module in multiboot for platforms supporting Xen (is MIPS really
>> supported?) and use it consistently.
>>
>>>  # Extra indentation to add to menu entries in a submenu. We're not in a 
>>> submenu
>>>  # yet, so it's empty. In a submenu it will be equal to '\t' (one tab).
>>>  submenu_indentation=""
>>
> 
> 
> 




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel