[Xen-devel] [PATCH v2 2/3] util/grub.d/20_linux_xen.in: Add arm64 support
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
В 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
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
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
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
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