Re: [PATCH] zfs: fix compilation failure with clang due to alignment

[Vladimir 'φ-coder/phcoder' Serbinenko]

Go ahead
On 07.07.2015 19:17, Leif Lindholm wrote:
> On Fri, Jul 03, 2015 at 10:05:47PM +0300, Andrei Borzenkov wrote:
>> I do not claim I understand why clang complains, but this patch does
>> fix it.
>>
>> fs/xfs.c:452:25: error: cast from 'struct grub_xfs_btree_node *' to
>>   'grub_uint64_t *' (aka 'unsigned long long *') increases required
>>   alignment from 1 to 8 [-Werror,-Wcast-align]
>>   grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>> ^~~
>> 1 error generated.
>>
>> ---
>>
>> Jan, do you have any idea what's wrong and whether this is proper fix?
>> Or should I raise it with clang?
> 
> Well, the problem is that struct grub_xfs_btree_node is defined with
> GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the
> 8-byte requirement that would naturally be enforced by the struct
> contents. And apparently clang objects to this, whereas gcc thinks
> everything is fine ... even though -Wcast-align is explicitly used.
> 
> Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(),
> where it is immediately stuffed back into another 8-byte aligned
> pointer. And then the alignment is immediately discarded again by
> casting it to a (char *) for an arithmetic operation.
> 
> If the alignment is indeed not required, it may be worth explicitly
> marking that pointer as one to a potentially unaligned location.
> But we don't currently appear to have a GRUB_UNALIGNED macro, to match
> the GRUB_PACKED for structs. Should we?
> 
> If so, something like the following could be added to your patch for a
> more complete fix:
> --- a/grub-core/fs/xfs.c
> +++ b/grub-core/fs/xfs.c
> @@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node,
> grub_disk_addr_t fileblock)
>if (node->inode.format == XFS_INODE_FORMAT_BTREE)
>  {
>struct grub_xfs_btree_root *root;
> -  const grub_uint64_t *keys;
> +  const grub_uint64_t *keys GRUB_UNALIGNED;
>int recoffset;
>  
>leaf = grub_malloc (node->data->bsize);
> diff --git a/include/grub/types.h b/include/grub/types.h
> index e732efb..720e236 100644
> --- a/include/grub/types.h
> +++ b/include/grub/types.h
> @@ -30,6 +30,8 @@
>  #define GRUB_PACKED __attribute__ ((packed))
>  #endif
>  
> +#define GRUB_UNALIGNED __attribute__ ((aligned (1)))
> +
>  #ifdef GRUB_BUILD
>  # define GRUB_CPU_SIZEOF_VOID_PBUILD_SIZEOF_VOID_P
>  # define GRUB_CPU_SIZEOF_LONG  BUILD_SIZEOF_LONG
> 
> /
> Leif
> 
>>  grub-core/fs/xfs.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
>> index 7249291..ea8cf7e 100644
>> --- a/grub-core/fs/xfs.c
>> +++ b/grub-core/fs/xfs.c
>> @@ -445,14 +445,14 @@ grub_xfs_next_de(struct grub_xfs_data *data, struct 
>> grub_xfs_dir2_entry *de)
>>return (struct grub_xfs_dir2_entry *)(((char *)de) + ALIGN_UP(size, 8));
>>  }
>>  
>> -static grub_uint64_t *
>> +static void *
>>  grub_xfs_btree_keys(struct grub_xfs_data *data,
>>  struct grub_xfs_btree_node *leaf)
>>  {
>> -  grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>> +  char *keys = (char *)leaf + sizeof (*leaf);
>>  
>>if (data->hascrc)
>> -keys += 6;  /* skip crc, uuid, ... */
>> +keys += 6 * sizeof (grub_uint64_t); /* skip crc, uuid, ... */
>>return keys;
>>  }
>>  
>> -- 
>> tg: (7a21030..) u/xfs-clang-align (depends on: master)
>>
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] zfs: fix compilation failure with clang due to alignment

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 03.07.2015 21:05, Andrei Borzenkov wrote:
> I do not claim I understand why clang complains, but this patch does
> fix it.
> 
> fs/xfs.c:452:25: error: cast from 'struct grub_xfs_btree_node *' to
>   'grub_uint64_t *' (aka 'unsigned long long *') increases required
>   alignment from 1 to 8 [-Werror,-Wcast-align]
>   grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
> ^~~
> 1 error generated.
> 
> ---
> 
> Jan, do you have any idea what's wrong and whether this is proper fix?
> Or should I raise it with clang?
> 
>  grub-core/fs/xfs.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
> index 7249291..ea8cf7e 100644
> --- a/grub-core/fs/xfs.c
> +++ b/grub-core/fs/xfs.c
> @@ -445,14 +445,14 @@ grub_xfs_next_de(struct grub_xfs_data *data, struct 
> grub_xfs_dir2_entry *de)
>return (struct grub_xfs_dir2_entry *)(((char *)de) + ALIGN_UP(size, 8));
>  }
>  
> -static grub_uint64_t *
> +static void *
>  grub_xfs_btree_keys(struct grub_xfs_data *data,
>   struct grub_xfs_btree_node *leaf)
>  {
> -  grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
> +  char *keys = (char *)leaf + sizeof (*leaf);
>  
>if (data->hascrc)
> -keys += 6;   /* skip crc, uuid, ... */
> +keys += 6 * sizeof (grub_uint64_t);  /* skip crc, uuid, ... */
>return keys;
>  }
>  
This would only hide the problem behind void*. Leif's patch solves the
problem. Another possibility is to analyze if packed is really required
but most likely it is.
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 0/3] arm64: Add multiboot support (via fdt) for Xen boot

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 13.07.2015 10:53, fu@linaro.org wrote:
> From: Fu Wei 
> 
>   - This adds support for the Xen boot on ARM specification for arm64.
> 
>   - The implementation for Xen is following  :
> http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/Multiboot
> and xen/docs/misc/arm/device-tree/booting.txt in Xen source code.
> 
>   - The multiboot/module commands have existed, so we use 
> xen_hypervisor/xen_module instead.
> 
>   - This Xen boot support is built into linux module for aarch64,
> and can not be used alone.
> 
>   - Adding this functionality to the existing "linux" module is for
> reusing the existing code of devicetree.
> 
This is a misguided decision. Modules can depend on other modules.
Ideally shared functionality should be in a separate module but having
xen depend on linux is an OK stopgap solution. Putting everything in one
module is bad.
>   - Add the support of xen_hypervisor/xen_module commands in 
> util/grub.d/20_linux_xen.in
> 
>   - Add the introduction of xen_hypervisor/xen_module commands in 
> docs/grub.texi
> 
>   - The example of this support is  Foundation FVP model>
> 
> https://wiki.linaro.org/LEG/Engineering/Grub2/Xen_booting_on_Foundation_FVP_model_by_GRUB
> 
> Changelog:
> v2: remove the patches which have been accepted.
> according to Vladimir's suggestion, change the command manes
> and relevant code:
> multiboot-->xen_hypervisor
> module-->xen_module
> improve the option parsing support for xen_hypervisor/xen_module commands.
> add a patch for adding xen_hypervisor/xen_module support
> in util/grub.d/20_linux_xen.in.
> update docs/grub.texi patch for the new command names.
> 
> v1: The first version upstream patchset to grub-devel mailing list
> 
> 
> Fu Wei (3):
>   arm64: Add Xen boot support file
>   * util/grub.d/20_linux_xen.in: Add support of the XEN boot on aarch64
>   arm64: Add the introduction of xen_hypervisor/xen_module command in
> docs/grub.texi
> 
>  docs/grub.texi|  27 ++
>  grub-core/Makefile.core.def   |   1 +
>  grub-core/loader/arm64/linux.c|   6 +
>  grub-core/loader/arm64/xen_boot.c | 615 
> ++
>  include/grub/arm64/xen_boot.h | 115 +++
>  util/grub.d/20_linux_xen.in   |  14 +-
>  6 files changed, 775 insertions(+), 3 deletions(-)
>  create mode 100644 grub-core/loader/arm64/xen_boot.c
>  create mode 100644 include/grub/arm64/xen_boot.h
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 1/3] arm64: Add Xen boot support file

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 13.07.2015 10:53, fu@linaro.org wrote:
> From: Fu Wei 
> 
> This patch adds Xen boot support file:
> grub-core/loader/arm64/xen_boot.c
> include/grub/arm64/xen_boot.h
> 
> This patch also adds commands register code and hearder file into
> grub-core/loader/arm64/linux.c
> 
>   - This adds support for the Xen boot on ARM specification for arm64.
>   - The implementation for Xen is following  :
>   
> http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/Multiboot
Please don't refer to this protocol as multiboot anywhere in grub or
around because it's NOT multiboot and we don't want to confuse those 2
protocols.
> and xen/docs/misc/arm/device-tree/booting.txt in Xen source code.
>   - The multiboot/module commands have existed,
> so we use xen_hypervisor/xen_module instead.
>   - This Xen boot support is built into linux module for aarch64.
>   - Adding this functionality to the existing "linux" module is for
> reusing the existing code of devicetree.
> 
Please create separate module. Modules are dynamically linked.
> Signed-off-by: Fu Wei 
> ---
>  grub-core/Makefile.core.def   |   1 +
>  grub-core/loader/arm64/linux.c|   6 +
>  grub-core/loader/arm64/xen_boot.c | 615 
> ++
>  include/grub/arm64/xen_boot.h | 115 +++
>  4 files changed, 737 insertions(+)
>  create mode 100644 grub-core/loader/arm64/xen_boot.c
>  create mode 100644 include/grub/arm64/xen_boot.h
> 
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index a6101de..01f8261 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1659,6 +1659,7 @@ module = {
>ia64_efi = loader/ia64/efi/linux.c;
>arm = loader/arm/linux.c;
>arm64 = loader/arm64/linux.c;
> +  arm64 = loader/arm64/xen_boot.c;
>fdt = lib/fdt.c;
>common = loader/linux.c;
>common = lib/cmdline.c;
> diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> index 987f5b9..7ae9bde 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -477,6 +478,9 @@ GRUB_MOD_INIT (linux)
>cmd_devicetree =
>  grub_register_command ("devicetree", grub_cmd_devicetree, 0,
>  N_("Load DTB file."));
> +
> +  grub_arm64_linux_register_xen_boot_command (mod, &loaded);
> +
>my_mod = mod;
>  }
>  
> @@ -485,4 +489,6 @@ GRUB_MOD_FINI (linux)
>grub_unregister_command (cmd_linux);
>grub_unregister_command (cmd_initrd);
>grub_unregister_command (cmd_devicetree);
> +
> +  grub_arm64_linux_unregister_xen_boot_command ();
>  }
Not needed with separate module.
> diff --git a/grub-core/loader/arm64/xen_boot.c 
> b/grub-core/loader/arm64/xen_boot.c
> new file mode 100644
> index 000..23bd00e
> --- /dev/null
> +++ b/grub-core/loader/arm64/xen_boot.c
> @@ -0,0 +1,615 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2014  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static grub_dl_t linux_mod;
> +static int *loaded;
> +
> +static struct xen_boot_binary *xen_hypervisor;
> +static struct xen_boot_binary *module_head;
> +static const grub_size_t module_default_align[] = {
> +  MODULE_IMAGE_MIN_ALIGN,
> +  MODULE_INITRD_MIN_ALIGN,
> +  MODULE_OTHER_MIN_ALIGN,
> +  MODULE_CUSTOM_MIN_ALIGN
> +};
> +
> +static void *xen_boot_fdt;
> +static const compat_string_struct_t default_compat_string[] = {
> +  FDT_COMPATIBLE (MODULE_IMAGE_COMPATIBLE),
> +  FDT_COMPATIBLE (MODULE_INITRD_COMPATIBLE),
> +  FDT_COMPATIBLE (MODULE_OTHER_COMPATIBLE)
> +};
> +
> +
> +/* Parse all the options of xen_module command. For now, we support
> +   (1) --type 
> +   (2) --nounzip
> +   We also set up the type of module in this function.
> +   If there are some "--type" options in the command line,
> +   we make a custom compatible stream in this function. */
> +static grub_err_t
> +set_module_type (struct xen_boot_binary *module, int argc, char *argv[],
> + 

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

[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
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: CentOS 7 - gfxterm does not work during Kernel boot

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 02.07.2015 16:55, Abdelghani Ouchabane wrote:
> Hallo,
>   my system is installed with CentOS 7 (7.1.1503), and it has:
> 
> 1. grub2-2.02-0.16.el7.centos.x86_64
> 2. kernel-3.10.0-229.1.2.el7.x86_64
> 3. xorg-x11-drv-catalyst-14.4-1.eldel7.2.3r.1.x86_64
> 
> I want to configure grub2 to use GRUB graphical terminal for both grub
> and kernel boot. For grub gfxterm works fine but during kernel boot it
> does not work, the system switches to console mode.
> 
kernel never uses GRUB's gfxterm. It uses own graphical terminal and
it's outside of scope of this ML
> I do not see ""loading kernel.." message + background image during
> kernel boot
> 
> I used to have gfxterm works fine for both grub and kernel boot with
> fedora 19 which has:
> 
> grub2-2.00-15.fc18.i686
> kernel-3.10.0-123.13.1.fc18.i686
> 
> Please check :  http://bugs.centos.org/view.php?id=8924
> 
> Thanks
> Ghani
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/2] Add powerpc little-endian (ppc64le) flags

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 30.06.2015 20:28, Andrei Borzenkov wrote:
> В Tue, 30 Jun 2015 15:05:46 -0300
> Paulo Flabiano Smorigo  пишет:
> 
>> On 2015-06-30 11:33, Vladimir 'phcoder' Serbinenko wrote:
>>> What about clang?
>>
>> Not good news about clang support. This is what the toolchain team said 
>> about it:
>>
>> The -mbig-endian option was added around April 10th, 2014. 
>> Unfortunately, those who implemented it only implemented it for ARM and 
>> one other architecture.
>>
>> The option is currently accepted on Power systems, but does not have any 
>> affect on the code generation for Power.
>>
>>
>> So, what can we do here? Maybe add a constrain in the configure file 
>> saying that it's not possible to build GRUB in a LE environment using clang.
> 
> If I understand it correctly, it is possible to build for big-endian
> PowerPC using
> 
> clang -target=powerpc
> 
@pfsmorigo. Please try attached patch
> but then we depend on clang being built with BE target support; and
> e.g. openSUSE builds it with
> 
> cmake -G "Ninja" \
>   -DBUILD_SHARED_LIBS=OFF \
>   -DCMAKE_BUILD_TYPE=Release \
>   -DLLVM_ENABLE_ASSERTIONS=OFF \
>   -DLLVM_TARGETS_TO_BUILD=host \
>   -DCMAKE_C_FLAGS="-O0" \
>   -DCMAKE_CXX_FLAGS="-O0" \
>   -DLLVM_HOST_TRIPLE=%{host_triple} \
> 
> so as I understand on ppc64le it does not build support for any other
> target. Actually openSUSE does not even build clang on ppc at all.
> 
> 
> 
>>>
>>> Le 30 juin 2015 16:30, "Andrei Borzenkov" >> > a écrit :
>>>
>>> On Tue, Jun 30, 2015 at 5:03 PM, Vladimir 'phcoder' Serbinenko
>>> mailto:phco...@gmail.com>> wrote:
>>>  > Which compilers support these flags? I'm concerned of breaking
>>> old compilers
>>>  >
>>>
>>> -static and -mbig-endian are listed in gcc 2.95.3 documentation for PPC.
>>>
>>>  > Le 30 juin 2015 14:57, "Paulo Flabiano Smorigo"
>>>  > >> > a écrit :
>>>  >>
>>>  >> libgcc dependency was removed *just* for this target because
>>>  >> the distros that use ppc64el doesn't have 32-bit support on it.
>>>  >>
>>>  >> * configure.ac : Add targets for
>>> powerpc64el and skip libgcc.
>>>  >> * Makefile.am: Likewise.
>>>  >> ---
>>>  >> configure.ac  | 8 
>>>  >>  1 file changed, 8 insertions(+)
>>>  >>
>>>  >> diff --git a/configure.ac  b/configure.ac
>>> 
>>>  >> index fd8a62e..0a79fad 100644
>>>  >> --- a/configure.ac 
>>>  >> +++ b/configure.ac 
>>>  >> @@ -116,6 +116,7 @@ if test "x$with_platform" = x; then
>>>  >>  x86_64-*) platform=pc ;;
>>>  >>  powerpc-*) platform=ieee1275 ;;
>>>  >>  powerpc64-*) platform=ieee1275 ;;
>>>  >> +powerpc64le-*) platform=ieee1275 ;;
>>>  >>  sparc64-*) platform=ieee1275 ;;
>>>  >>  mipsel-*) platform=loongson ;;
>>>  >>  mips-*) platform=arc ;;
>>>  >> @@ -138,6 +139,7 @@ case "$target_cpu"-"$platform" in
>>>  >>x86_64-none) ;;
>>>  >>x86_64-*) target_cpu=i386 ;;
>>>  >>powerpc64-ieee1275) target_cpu=powerpc ;;
>>>  >> +  powerpc64le-ieee1275) target_cpu=powerpc ;;
>>>  >>  esac
>>>  >>
>>>  >>  # Check if the platform is supported, make final adjustments.
>>>  >> @@ -601,6 +603,12 @@ if test "x$target_cpu" = xi386 && test
>>> "x$platform"
>>>  >> != xemu; then
>>>  >>TARGET_CFLAGS="$TARGET_CFLAGS -march=i386"
>>>  >>  fi
>>>  >>
>>>  >> +if test x$target_cpu = xpowerpc; then
>>>  >> +  TARGET_CFLAGS="$TARGET_CFLAGS -mbig-endian"
>>>  >> +  TARGET_CCASFLAGS="$TARGET_CCASFLAGS -mbig-endian"
>>>  >> +  TARGET_LDFLAGS="$TARGET_LDFLAGS -static -mbig-endian"
>>>  >> +fi
>>>  >> +
>>>  >>  if test "x$target_m32" = x1; then
>>>  >># Force 32-bit mode.
>>>  >>TARGET_CFLAGS="$TARGET_CFLAGS -m32"
>>>  >> --
>>>  >> 2.1.0
>>>  >>
>>>  >>
>>>  >> ___
>>>  >> Grub-devel mailing list
>>>  >> Grub-devel@gnu.org 
>>>  >> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>  >
>>>  >
>>>  > ___
>>>  > Grub-devel mailing list
>>>  > Grub-devel@gnu.org 
>>>  > https://lists.gnu.org/mailman/listinfo/grub-devel
>>>  >
>>>
>>> ___
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org 
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>>
>>>
>>> ___
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
> 
> 
> _

Re: [PATCH 1/2] Add powerpc little-endian (ppc64le) flags

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 04.07.2015 08:46, Andrei Borzenkov wrote:
> В Tue, 30 Jun 2015 21:34:10 +0200
> "Vladimir 'phcoder' Serbinenko"  пишет:
> 
>> I think those flags disable only runtime libraries, not the code generation
> 
> Yes, you are right. Clang seems to be built for the whole family, i.e.
> PowerPC, which seems to automatically include all three versions (ppc,
> ppc64, ppc64le)
> 
> See below for details.
> 
>> Le 30 juin 2015 20:29, "Andrei Borzenkov"  a écrit :
>>
>>> В Tue, 30 Jun 2015 15:05:46 -0300
>>> Paulo Flabiano Smorigo  пишет:
>>>
 On 2015-06-30 11:33, Vladimir 'phcoder' Serbinenko wrote:
> What about clang?

 Not good news about clang support. This is what the toolchain team said
 about it:

 The -mbig-endian option was added around April 10th, 2014.
 Unfortunately, those who implemented it only implemented it for ARM and
 one other architecture.

 The option is currently accepted on Power systems, but does not have any
 affect on the code generation for Power.

> 
> Support for it looks rather trivial; see attached patch. Anyone has good
> connection to clang development community?
> 
Just send it to their ML. They usually have good response. If this patch
is accepted we can probably simplify it in our code
> Using this patch the
> 
> clang --target=powerpc64le -mbig-endian -m32 produces code for PPC32
> BE, so just works:
> 
> bor@opensuse:~/build/clang-ppc/bin> ./clang  --target=powerpc64le   -c 
> -mbig-endian -m32 /tmp/foo.c
> /tmp/foo.c:1:1: warning: type specifier missing, defaults to 'int'
>   [-Wimplicit-int]
> foo()
> ^
> 1 warning generated.
> bor@opensuse:~/build/clang-ppc/bin> file foo.o
> foo.o: ELF 32-bit MSB relocatable, PowerPC or cisco 4500, version 1 (SYSV), 
> not stripped
> 
> The problem is that in this case we do not really have any way to test
> if it works in configure, so no way to fail gracefully.
> 
Just check the defines.

 So, what can we do here? Maybe add a constrain in the configure file
 saying that it's not possible to build GRUB in a LE environment using
>>> clang.
>>>
>>> If I understand it correctly, it is possible to build for big-endian
>>> PowerPC using
>>>
>>> clang -target=powerpc
>>>
>>> but then we depend on clang being built with BE target support; and
>>> e.g. openSUSE builds it with
>>>
> 
> Yes, that works too. But I'm not sure how we can test for it. Brute
> force would of course be
> 
> $TARGET_CC --version | grep clang && TARGET_CFLAGS=--target=powerpc
> 
> Comments? I'm inclined to use this workaround. This still may have
> issues when using external assembler, but here we can simply mandate
> support for clang 3.6 at the minimum, which should use integrated
> assembler on PPC by default.
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/2] Add powerpc little-endian (ppc64le) flags

[Vladimir 'φ-coder/phcoder' Serbinenko]

> It is possible to compile using "--target=powerpc -Wl,-EB" - this works
> on all three ppc, ppc64 and ppc64le, but this means hardcoding GNU ld
> dependency.
> 
Thanks for this info.
See attached patch
> So I'd rather try to produce patch for proper support of
> -mbig-endian/-mlittle-endian (including passing it onto gcc linker) and
> until then declared clang on ppc64le having limited support (i.e.
> support for automatic detection by configure).
> 
I agree that this is a better approach in long term. But if we can get
some support for current tools with little work (see patch) and without
risks of breaking other tools I see no reason not to go for it in short term
> 

diff --git a/configure.ac b/configure.ac
index fd8a62e..113e7c7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -116,6 +116,7 @@ if test "x$with_platform" = x; then
 x86_64-*) platform=pc ;;
 powerpc-*) platform=ieee1275 ;;
 powerpc64-*) platform=ieee1275 ;;
+powerpc64le-*) platform=ieee1275 ;;
 sparc64-*) platform=ieee1275 ;;
 mipsel-*) platform=loongson ;;
 mips-*) platform=arc ;;
@@ -138,6 +139,7 @@ case "$target_cpu"-"$platform" in
   x86_64-none) ;;
   x86_64-*) target_cpu=i386 ;;
   powerpc64-ieee1275) target_cpu=powerpc ;;
+  powerpc64le-ieee1275) target_cpu=powerpc ;;
 esac
 
 # Check if the platform is supported, make final adjustments.
@@ -601,6 +603,37 @@ if test "x$target_cpu" = xi386 && test "x$platform" != xemu; then
   TARGET_CFLAGS="$TARGET_CFLAGS -march=i386"
 fi
 
+if test x$target_cpu = xpowerpc; then
+  AC_CACHE_CHECK([for options to get big-endian compilation], grub_cv_target_cc_big_endian, [
+grub_cv_target_cc_big_endian=no
+for cand in "-target powerpc -Wl,-EB" "-target powerpc" \
+		"-mbig-endian"; do
+  if test x"$grub_cv_target_cc_big_endian" != xno ; then
+break
+  fi
+  CFLAGS="$TARGET_CFLAGS $cand -Werror"
+  AC_LINK_IFELSE([AC_LANG_PROGRAM([[
+#if defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && (__ORDER_BIG_ENDIAN__ != __BYTE_ORDER__)
+#error still little endian
+#endif
+asm (".globl start; start:");
+void __main (void);
+void __main (void) {}
+int main (void);
+]], [[]])],
+		[grub_cv_target_cc_big_endian="$cand"], [])
+done
+  ])
+
+  if test x"$grub_cv_target_cc_big_endian" = xno ; then
+AC_MSG_ERROR([could not force big-endian])
+  fi
+
+  TARGET_CFLAGS="$TARGET_CFLAGS $grub_cv_target_cc_big_endian"
+  TARGET_CCASFLAGS="$TARGET_CCASFLAGS $grub_cv_target_cc_big_endian"
+  TARGET_LDFLAGS="$TARGET_LDFLAGS -static $grub_cv_target_cc_big_endian"
+fi
+
 if test "x$target_m32" = x1; then
   # Force 32-bit mode.
   TARGET_CFLAGS="$TARGET_CFLAGS -m32"


signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] zfs: fix compilation failure with clang due to alignment

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 15.07.2015 18:52, Leif Lindholm wrote:
> On Wed, Jul 15, 2015 at 05:47:50PM +0200, Vladimir 'φ-coder/phcoder' 
> Serbinenko wrote:
>> Go ahead
> 
> The below was more of an RFC than something committable - are you OK
> with me splitting the types.h changes out as a separate patch?
> 
Yes
>> On 07.07.2015 19:17, Leif Lindholm wrote:
>>> On Fri, Jul 03, 2015 at 10:05:47PM +0300, Andrei Borzenkov wrote:
>>>> I do not claim I understand why clang complains, but this patch does
>>>> fix it.
>>>>
>>>> fs/xfs.c:452:25: error: cast from 'struct grub_xfs_btree_node *' to
>>>>   'grub_uint64_t *' (aka 'unsigned long long *') increases required
>>>>   alignment from 1 to 8 [-Werror,-Wcast-align]
>>>>   grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>>>> ^~~
>>>> 1 error generated.
>>>>
>>>> ---
>>>>
>>>> Jan, do you have any idea what's wrong and whether this is proper fix?
>>>> Or should I raise it with clang?
>>>
>>> Well, the problem is that struct grub_xfs_btree_node is defined with
>>> GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the
>>> 8-byte requirement that would naturally be enforced by the struct
>>> contents. And apparently clang objects to this, whereas gcc thinks
>>> everything is fine ... even though -Wcast-align is explicitly used.
>>>
>>> Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(),
>>> where it is immediately stuffed back into another 8-byte aligned
>>> pointer. And then the alignment is immediately discarded again by
>>> casting it to a (char *) for an arithmetic operation.
>>>
>>> If the alignment is indeed not required, it may be worth explicitly
>>> marking that pointer as one to a potentially unaligned location.
>>> But we don't currently appear to have a GRUB_UNALIGNED macro, to match
>>> the GRUB_PACKED for structs. Should we?
>>>
>>> If so, something like the following could be added to your patch for a
>>> more complete fix:
>>> --- a/grub-core/fs/xfs.c
>>> +++ b/grub-core/fs/xfs.c
>>> @@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node,
>>> grub_disk_addr_t fileblock)
>>>if (node->inode.format == XFS_INODE_FORMAT_BTREE)
>>>  {
>>>struct grub_xfs_btree_root *root;
>>> -  const grub_uint64_t *keys;
>>> +  const grub_uint64_t *keys GRUB_UNALIGNED;
>>>int recoffset;
>>>  
>>>leaf = grub_malloc (node->data->bsize);
>>> diff --git a/include/grub/types.h b/include/grub/types.h
>>> index e732efb..720e236 100644
>>> --- a/include/grub/types.h
>>> +++ b/include/grub/types.h
>>> @@ -30,6 +30,8 @@
>>>  #define GRUB_PACKED __attribute__ ((packed))
>>>  #endif
>>>  
>>> +#define GRUB_UNALIGNED __attribute__ ((aligned (1)))
>>> +
>>>  #ifdef GRUB_BUILD
>>>  # define GRUB_CPU_SIZEOF_VOID_PBUILD_SIZEOF_VOID_P
>>>  # define GRUB_CPU_SIZEOF_LONG  BUILD_SIZEOF_LONG
>>>
>>> /
>>> Leif
>>>
>>>>  grub-core/fs/xfs.c | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
>>>> index 7249291..ea8cf7e 100644
>>>> --- a/grub-core/fs/xfs.c
>>>> +++ b/grub-core/fs/xfs.c
>>>> @@ -445,14 +445,14 @@ grub_xfs_next_de(struct grub_xfs_data *data, struct 
>>>> grub_xfs_dir2_entry *de)
>>>>return (struct grub_xfs_dir2_entry *)(((char *)de) + ALIGN_UP(size, 8));
>>>>  }
>>>>  
>>>> -static grub_uint64_t *
>>>> +static void *
>>>>  grub_xfs_btree_keys(struct grub_xfs_data *data,
>>>>struct grub_xfs_btree_node *leaf)
>>>>  {
>>>> -  grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>>>> +  char *keys = (char *)leaf + sizeof (*leaf);
>>>>  
>>>>if (data->hascrc)
>>>> -keys += 6;/* skip crc, uuid, ... */
>>>> +keys += 6 * sizeof (grub_uint64_t);   /* skip crc, uuid, ... */
>>>>return keys;
>>>>  }
>>>>  
>>>> -- 
>>>> tg: (7a21030..) u/xfs-clang-align (depends on: master)
>>>>
>>>> ___
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>> ___
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
> 
> 
> 
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 3/5] i386/relocator: Remove unused avoid_efi_bootservices argument

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 30.05.2015 07:25, Andrei Borzenkov wrote:
> В Fri, 29 May 2015 22:58:43 +0200
> Daniel Kiper  пишет:
> 
>> Another questions is why grub_relocator_alloc_chunk_addr() does not consult 
>> EFI
>> memory map if grub_relocator_alloc_chunk_align() does. Should not we fix it?
> 
> My best guess is that grub_relocator_alloc_chunk_addr() gets target
> from elsewhere so there is nothing to consult (it is caller
> responsibility); while grub_relocator_alloc_chunk_align() needs to
> actually search for suitable memory region.
> 
My suggestion would be to pass avoid_boot_services = 1 in multiboot2 iff
we don't terminate the services. Any problems with this approach?



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFD] diskfilter stale RAID member detection vs. lazy scanning

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 28.06.2015 20:06, Andrei Borzenkov wrote:
> I was looking at implementing detection of outdated RAID members.
> Unfortunately it appears to be fundamentally incompatible with lazy
> scanning as implemented currently by GRUB. We simply cannot stop
> scanning for other copies of metadata once "enough" was seen. Because
> any other disk may contain more actual copy which invalidates
> everything seen up to this point.
> 
> So basically either we officially admit that GRUB is not able to detect
> stale members or we drop lazy scanning.
> 
> Comments, ideas?
> 
We don't need to see all disks to decide that there is no staleness. If
you have an array with N devices and you can lose at most K of them,
then you can check for staleness after you have seen max(K+1, N-K)
drives. Why?

Let those disks have generation numbers g_0,...,g_{N-1}. Our goal is to
find the largest number G s.t. number of indices with
g_i >= G is at least N-K.
In most common case when you have seen K+1 disks all of them will have
the same generation number
g_0=g_1=...=g_{K}
Then we know that
G<=g_0
Suppose not then all of 0,...,K are stale and we have lost K+1 drives
which contradicts our goal.
On the other hand when we have seen N-K devices we know that
G>=min(g_0,...,g_{N-K-1})
as with G=min(g_0,...,g_{N-K-1}) we already have N-K disks.

In cases other than mirror usually K+1<=N-K and so we don't even need to
scan for more disks to detect staleness.
The code will be slightly tricky as it has to handle tolerating
staleness if there are too little disks but it's totally feasible. Let
me figure out the rest of math and write a prototype.
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFD] diskfilter stale RAID member detection vs. lazy scanning

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 15.07.2015 20:05, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 28.06.2015 20:06, Andrei Borzenkov wrote:
>> I was looking at implementing detection of outdated RAID members.
>> Unfortunately it appears to be fundamentally incompatible with lazy
>> scanning as implemented currently by GRUB. We simply cannot stop
>> scanning for other copies of metadata once "enough" was seen. Because
>> any other disk may contain more actual copy which invalidates
>> everything seen up to this point.
>>
>> So basically either we officially admit that GRUB is not able to detect
>> stale members or we drop lazy scanning.
>>
>> Comments, ideas?
>>
> We don't need to see all disks to decide that there is no staleness. If
> you have an array with N devices and you can lose at most K of them,
> then you can check for staleness after you have seen max(K+1, N-K)
> drives. Why?
> 
> Let those disks have generation numbers g_0,...,g_{N-1}. Our goal is to
> find the largest number G s.t. number of indices with
> g_i >= G is at least N-K.
> In most common case when you have seen K+1 disks all of them will have
> the same generation number
> g_0=g_1=...=g_{K}
> Then we know that
> G<=g_0
> Suppose not then all of 0,...,K are stale and we have lost K+1 drives
> which contradicts our goal.
> On the other hand when we have seen N-K devices we know that
> G>=min(g_0,...,g_{N-K-1})
> as with G=min(g_0,...,g_{N-K-1}) we already have N-K disks.
> 
> In cases other than mirror usually K+1<=N-K and so we don't even need to
> scan for more disks to detect staleness.
> The code will be slightly tricky as it has to handle tolerating
> staleness if there are too little disks but it's totally feasible. Let
> me figure out the rest of math and write a prototype.
Untested patch implementing these ideas, just to illustrate
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
> 
> 

diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index c4f6678..850046e 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -41,77 +41,201 @@ static int lv_num = 0;
 
 static struct grub_diskfilter_lv *
 find_lv (const char *name);
-static int is_lv_readable (struct grub_diskfilter_lv *lv, int easily);
+static void
+recompute_lv_readable (struct grub_diskfilter_lv *lv);
 
 
 
-static grub_err_t
-is_node_readable (const struct grub_diskfilter_node *node, int easily)
+static int
+get_number_needed_nodes(struct grub_diskfilter_segment *seg, int easily)
+{
+  int need = seg->node_count;
+
+  switch (seg->type)
+{
+case GRUB_DISKFILTER_RAID6:
+  if (!easily)
+	need--;
+  /* Fallthrough.  */
+case GRUB_DISKFILTER_RAID4:
+case GRUB_DISKFILTER_RAID5:
+  if (!easily)
+	need--;
+  /* Fallthrough.  */
+case GRUB_DISKFILTER_STRIPED:
+  break;
+  
+case GRUB_DISKFILTER_MIRROR:
+  need = 1;
+  break;
+
+case GRUB_DISKFILTER_RAID10:
+  {
+	unsigned int n;
+	n = seg->layout & 0xFF;
+	if (n == 1)
+	  n = (seg->layout >> 8) & 0xFF;
+	need = seg->node_count - n + 1;
+  }
+  break;
+}
+  return need;
+}
+
+/* Return whether node is readable and if so update node->generation.  */
+static void
+recompute_node_readable (struct grub_diskfilter_node *node)
 {
   /* Check whether we actually know the physical volume we want to
  read from.  */
   if (node->pv)
-return !!(node->pv->disk);
+{
+  node->readability.is_readable = !!(node->pv->disk);
+  node->readability.is_easily_readable = !!(node->pv->disk);
+  node->readability.generation = node->pv->generation;
+  return;
+}
   if (node->lv)
-return is_lv_readable (node->lv, easily);
+{
+  recompute_lv_readable (node->lv);
+  node->readability = node->lv->readability;
+  return;
+}
+  node->readability.is_readable = 0;
+  node->readability.is_easily_readable = 0;
+  node->readability.generation = 0;
+  return;
+}
+
+static grub_uint64_t
+compute_generation (struct grub_diskfilter_segment *seg, int need)
+{
+  unsigned i;
+  for (i = 0; i < seg->node_count; i++)
+if (seg->nodes[i].readability.is_readable) {
+  unsigned j;
+  grub_uint64_t candidate = seg->nodes[i].readability.generation;
+  int higher = 0, strictly_higher = 0;
+  for (j = 0; j < seg->node_count; j++)
+	if (seg->nodes[j].readability.is_readable) {
+	  strictly_higher += (seg->nodes[j].readability.generation > candidate);
+	  higher += (seg->nodes[j].readability.generation >= candidate);
+	}
+  if (higher >= need &&a

Re: [RFD] diskfilter stale RAID member detection vs. lazy scanning

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 16.07.2015 05:42, Andrei Borzenkov wrote:
> В Wed, 15 Jul 2015 20:05:56 +0200
> Vladimir 'φ-coder/phcoder' Serbinenko  пишет:
> 
>> On 28.06.2015 20:06, Andrei Borzenkov wrote:
>>> I was looking at implementing detection of outdated RAID members.
>>> Unfortunately it appears to be fundamentally incompatible with lazy
>>> scanning as implemented currently by GRUB. We simply cannot stop
>>> scanning for other copies of metadata once "enough" was seen. Because
>>> any other disk may contain more actual copy which invalidates
>>> everything seen up to this point.
>>>
>>> So basically either we officially admit that GRUB is not able to detect
>>> stale members or we drop lazy scanning.
>>>
>>> Comments, ideas?
>>>
>> We don't need to see all disks to decide that there is no staleness. If
>> you have an array with N devices and you can lose at most K of them,
>> then you can check for staleness after you have seen max(K+1, N-K)
>> drives. Why?
>>
> 
> It's not the problem. The problem is what to do if you see disk with
> generation N+1 after you assembled array with generation N. This can
> mean that what we see is old copy and we should through it away and
> start collecting new one. If I read Linux MD code correctly, that is
> what it actually does. And this means we cannot stop scanning even
> after array is complete.
> 
While it's true that it's possible that all the members we have seen are
stale, it shouldn't be common and it's not the biggest problem. Biggest
problem is inconsistency.
We can never guarantee of having seen all the disks as they may not be
eeven visible through firmware but it shouldn't stop us from fixing the
inconsistency problem.
> Extreme example is three-pieces mirror where each piece is actually
> perfectly valid and usable by itself so losing two of them still means
> we can continue to work with remaining one.
> 
Mirrors get completely assembled in my patch.



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] zfs: fix compilation failure with clang due to alignment

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 16.07.2015 05:46, Andrei Borzenkov wrote:
> В Wed, 15 Jul 2015 17:49:30 +0200
> Vladimir 'φ-coder/phcoder' Serbinenko  пишет:
> 
>>
>> On 03.07.2015 21:05, Andrei Borzenkov wrote:
>>> I do not claim I understand why clang complains, but this patch does
>>> fix it.
>>>
>>> fs/xfs.c:452:25: error: cast from 'struct grub_xfs_btree_node *' to
>>>   'grub_uint64_t *' (aka 'unsigned long long *') increases required
>>>   alignment from 1 to 8 [-Werror,-Wcast-align]
>>>   grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>>> ^~~
>>> 1 error generated.
>>>
>>> ---
>>>
>>> Jan, do you have any idea what's wrong and whether this is proper fix?
>>> Or should I raise it with clang?
>>>
>>>  grub-core/fs/xfs.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
>>> index 7249291..ea8cf7e 100644
>>> --- a/grub-core/fs/xfs.c
>>> +++ b/grub-core/fs/xfs.c
>>> @@ -445,14 +445,14 @@ grub_xfs_next_de(struct grub_xfs_data *data, struct 
>>> grub_xfs_dir2_entry *de)
>>>return (struct grub_xfs_dir2_entry *)(((char *)de) + ALIGN_UP(size, 8));
>>>  }
>>>  
>>> -static grub_uint64_t *
>>> +static void *
>>>  grub_xfs_btree_keys(struct grub_xfs_data *data,
>>> struct grub_xfs_btree_node *leaf)
>>>  {
>>> -  grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>>> +  char *keys = (char *)leaf + sizeof (*leaf);
>>>  
>>>if (data->hascrc)
>>> -keys += 6; /* skip crc, uuid, ... */
>>> +keys += 6 * sizeof (grub_uint64_t);/* skip crc, uuid, ... */
>>>return keys;
>>>  }
>>>  
>> This would only hide the problem behind void*. Leif's patch solves the
>> problem. Another possibility is to analyze if packed is really required
>> but most likely it is.
>>>
>>
>>
> 
> grub_uint64_t is not really required at all. The whole code is used to
> compute bite offset. So in reality this should simply be replaced by
> char *. I would rather avoid making code even more complicated.
> 
> Jan, do I miss something?
> 
We have exactly the same problem at line 500:
  keys = &root->keys[0];
and root is in packed struct. I think it's better to solve both in the
same way.



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] zfs: fix compilation failure with clang due to alignment

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 15.07.2015 18:52, Leif Lindholm wrote:
> On Wed, Jul 15, 2015 at 05:47:50PM +0200, Vladimir 'φ-coder/phcoder' 
> Serbinenko wrote:
>> Go ahead
> 
> The below was more of an RFC than something committable - are you OK
> with me splitting the types.h changes out as a separate patch?
> 
Actually I think this patch doesn't work as __attribute__((aligned(X)))
can only increase alignment, not decrease it. I'm going to fix it by
using char * and grub_get_unaligned64
>> On 07.07.2015 19:17, Leif Lindholm wrote:
>>> On Fri, Jul 03, 2015 at 10:05:47PM +0300, Andrei Borzenkov wrote:
>>>> I do not claim I understand why clang complains, but this patch does
>>>> fix it.
>>>>
>>>> fs/xfs.c:452:25: error: cast from 'struct grub_xfs_btree_node *' to
>>>>   'grub_uint64_t *' (aka 'unsigned long long *') increases required
>>>>   alignment from 1 to 8 [-Werror,-Wcast-align]
>>>>   grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>>>> ^~~
>>>> 1 error generated.
>>>>
>>>> ---
>>>>
>>>> Jan, do you have any idea what's wrong and whether this is proper fix?
>>>> Or should I raise it with clang?
>>>
>>> Well, the problem is that struct grub_xfs_btree_node is defined with
>>> GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the
>>> 8-byte requirement that would naturally be enforced by the struct
>>> contents. And apparently clang objects to this, whereas gcc thinks
>>> everything is fine ... even though -Wcast-align is explicitly used.
>>>
>>> Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(),
>>> where it is immediately stuffed back into another 8-byte aligned
>>> pointer. And then the alignment is immediately discarded again by
>>> casting it to a (char *) for an arithmetic operation.
>>>
>>> If the alignment is indeed not required, it may be worth explicitly
>>> marking that pointer as one to a potentially unaligned location.
>>> But we don't currently appear to have a GRUB_UNALIGNED macro, to match
>>> the GRUB_PACKED for structs. Should we?
>>>
>>> If so, something like the following could be added to your patch for a
>>> more complete fix:
>>> --- a/grub-core/fs/xfs.c
>>> +++ b/grub-core/fs/xfs.c
>>> @@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node,
>>> grub_disk_addr_t fileblock)
>>>if (node->inode.format == XFS_INODE_FORMAT_BTREE)
>>>  {
>>>struct grub_xfs_btree_root *root;
>>> -  const grub_uint64_t *keys;
>>> +  const grub_uint64_t *keys GRUB_UNALIGNED;
>>>int recoffset;
>>>  
>>>leaf = grub_malloc (node->data->bsize);
>>> diff --git a/include/grub/types.h b/include/grub/types.h
>>> index e732efb..720e236 100644
>>> --- a/include/grub/types.h
>>> +++ b/include/grub/types.h
>>> @@ -30,6 +30,8 @@
>>>  #define GRUB_PACKED __attribute__ ((packed))
>>>  #endif
>>>  
>>> +#define GRUB_UNALIGNED __attribute__ ((aligned (1)))
>>> +
>>>  #ifdef GRUB_BUILD
>>>  # define GRUB_CPU_SIZEOF_VOID_PBUILD_SIZEOF_VOID_P
>>>  # define GRUB_CPU_SIZEOF_LONG  BUILD_SIZEOF_LONG
>>>
>>> /
>>> Leif
>>>
>>>>  grub-core/fs/xfs.c | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
>>>> index 7249291..ea8cf7e 100644
>>>> --- a/grub-core/fs/xfs.c
>>>> +++ b/grub-core/fs/xfs.c
>>>> @@ -445,14 +445,14 @@ grub_xfs_next_de(struct grub_xfs_data *data, struct 
>>>> grub_xfs_dir2_entry *de)
>>>>return (struct grub_xfs_dir2_entry *)(((char *)de) + ALIGN_UP(size, 8));
>>>>  }
>>>>  
>>>> -static grub_uint64_t *
>>>> +static void *
>>>>  grub_xfs_btree_keys(struct grub_xfs_data *data,
>>>>struct grub_xfs_btree_node *leaf)
>>>>  {
>>>> -  grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>>>> +  char *keys = (char *)leaf + sizeof (*leaf);
>>>>  
>>>>if (data->hascrc)
>>>> -keys += 6;/* skip crc, uuid, ... */
>>>> +keys += 6 * sizeof (grub_uint64_t);   /* skip crc, uuid, ... */
>>>>return keys;
>>>>  }
>>>>  
>>>> -- 
>>>> tg: (7a21030..) u/xfs-clang-align (depends on: master)
>>>>
>>>> ___
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>> ___
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
> 
> 
> 
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] zfs: fix compilation failure with clang due to alignment

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 15.07.2015 18:52, Leif Lindholm wrote:
> On Wed, Jul 15, 2015 at 05:47:50PM +0200, Vladimir 'φ-coder/phcoder' 
> Serbinenko wrote:
>> Go ahead
> 
> The below was more of an RFC than something committable - are you OK
> with me splitting the types.h changes out as a separate patch?
> 
I've fixed this and several other bad alignments in xfs.c
>> On 07.07.2015 19:17, Leif Lindholm wrote:
>>> On Fri, Jul 03, 2015 at 10:05:47PM +0300, Andrei Borzenkov wrote:
>>>> I do not claim I understand why clang complains, but this patch does
>>>> fix it.
>>>>
>>>> fs/xfs.c:452:25: error: cast from 'struct grub_xfs_btree_node *' to
>>>>   'grub_uint64_t *' (aka 'unsigned long long *') increases required
>>>>   alignment from 1 to 8 [-Werror,-Wcast-align]
>>>>   grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>>>> ^~~
>>>> 1 error generated.
>>>>
>>>> ---
>>>>
>>>> Jan, do you have any idea what's wrong and whether this is proper fix?
>>>> Or should I raise it with clang?
>>>
>>> Well, the problem is that struct grub_xfs_btree_node is defined with
>>> GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the
>>> 8-byte requirement that would naturally be enforced by the struct
>>> contents. And apparently clang objects to this, whereas gcc thinks
>>> everything is fine ... even though -Wcast-align is explicitly used.
>>>
>>> Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(),
>>> where it is immediately stuffed back into another 8-byte aligned
>>> pointer. And then the alignment is immediately discarded again by
>>> casting it to a (char *) for an arithmetic operation.
>>>
>>> If the alignment is indeed not required, it may be worth explicitly
>>> marking that pointer as one to a potentially unaligned location.
>>> But we don't currently appear to have a GRUB_UNALIGNED macro, to match
>>> the GRUB_PACKED for structs. Should we?
>>>
>>> If so, something like the following could be added to your patch for a
>>> more complete fix:
>>> --- a/grub-core/fs/xfs.c
>>> +++ b/grub-core/fs/xfs.c
>>> @@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node,
>>> grub_disk_addr_t fileblock)
>>>if (node->inode.format == XFS_INODE_FORMAT_BTREE)
>>>  {
>>>struct grub_xfs_btree_root *root;
>>> -  const grub_uint64_t *keys;
>>> +  const grub_uint64_t *keys GRUB_UNALIGNED;
>>>int recoffset;
>>>  
>>>leaf = grub_malloc (node->data->bsize);
>>> diff --git a/include/grub/types.h b/include/grub/types.h
>>> index e732efb..720e236 100644
>>> --- a/include/grub/types.h
>>> +++ b/include/grub/types.h
>>> @@ -30,6 +30,8 @@
>>>  #define GRUB_PACKED __attribute__ ((packed))
>>>  #endif
>>>  
>>> +#define GRUB_UNALIGNED __attribute__ ((aligned (1)))
>>> +
>>>  #ifdef GRUB_BUILD
>>>  # define GRUB_CPU_SIZEOF_VOID_PBUILD_SIZEOF_VOID_P
>>>  # define GRUB_CPU_SIZEOF_LONG  BUILD_SIZEOF_LONG
>>>
>>> /
>>> Leif
>>>
>>>>  grub-core/fs/xfs.c | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
>>>> index 7249291..ea8cf7e 100644
>>>> --- a/grub-core/fs/xfs.c
>>>> +++ b/grub-core/fs/xfs.c
>>>> @@ -445,14 +445,14 @@ grub_xfs_next_de(struct grub_xfs_data *data, struct 
>>>> grub_xfs_dir2_entry *de)
>>>>return (struct grub_xfs_dir2_entry *)(((char *)de) + ALIGN_UP(size, 8));
>>>>  }
>>>>  
>>>> -static grub_uint64_t *
>>>> +static void *
>>>>  grub_xfs_btree_keys(struct grub_xfs_data *data,
>>>>struct grub_xfs_btree_node *leaf)
>>>>  {
>>>> -  grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>>>> +  char *keys = (char *)leaf + sizeof (*leaf);
>>>>  
>>>>if (data->hascrc)
>>>> -keys += 6;/* skip crc, uuid, ... */
>>>> +keys += 6 * sizeof (grub_uint64_t);   /* skip crc, uuid, ... */
>>>>return keys;
>>>>  }
>>>>  
>>>> -- 
>>>> tg: (7a21030..) u/xfs-clang-align (depends on: master)
>>>>
>>>> ___
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>> ___
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
> 
> 
> 
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/2] Add powerpc little-endian (ppc64le) flags

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 16.07.2015 20:55, Andrei Borzenkov wrote:
> В Wed, 15 Jul 2015 19:42:39 +0200
> Vladimir 'φ-coder/phcoder' Serbinenko  пишет:
> 
>>
>>> It is possible to compile using "--target=powerpc -Wl,-EB" - this works
>>> on all three ppc, ppc64 and ppc64le, but this means hardcoding GNU ld
>>> dependency.
>>>
>> Thanks for this info.
>> See attached patch
> 
> Test has to come before asm tests (so that we are sure to compile
> for the right target).
> 
> But real problem is that -Wl,-EB has to be added to TARGET_LDFLAGS only
> 
Yes, discovered it already and updated the patch
>  [  106s] configure:24950: clang -c -v -Wall -W -Wshadow
>  -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment 
> -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal 
> -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit 
> -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces 
> -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type 
> -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs 
> -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label 
> -Wunused-parameter -Wunused-value  -Wunused-variable -Wwrite-strings 
> -Wnested-externs -Wstrict-prototypes -g -Wredundant-decls 
> -Wmissing-prototypes -Wmissing-declarations -Wcast-align  -Wextra 
> -Wattributes -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch 
> -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast 
> -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign 
> -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2 
> -target powerpc -Wl,-EB -m32 -msoft-float -W
error  -Wall -W  -DGRUB_MACHINE_IEEE1275=1 -DGRUB_MACHINE=POWERPC_IEEE1275 
conftest.c >&5 [  106s] clang version 3.6.1 (tags/RELEASE_361/final 238133)
> [  106s] Target: powerpc
> [  106s] Thread model: posix
> [  106s] clang-3.6: error: -Wl,-EB: 'linker' input unused
> 
> 
>>> So I'd rather try to produce patch for proper support of
>>> -mbig-endian/-mlittle-endian (including passing it onto gcc linker) and
>>> until then declared clang on ppc64le having limited support (i.e.
>>> support for automatic detection by configure).
>>>
>> I agree that this is a better approach in long term. But if we can get
>> some support for current tools with little work (see patch) and without
>> risks of breaking other tools I see no reason not to go for it in short term
>>>
>>
> 

diff --git a/configure.ac b/configure.ac
index fd8a62e..e54b9df 100644
--- a/configure.ac
+++ b/configure.ac
@@ -116,6 +116,7 @@ if test "x$with_platform" = x; then
 x86_64-*) platform=pc ;;
 powerpc-*) platform=ieee1275 ;;
 powerpc64-*) platform=ieee1275 ;;
+powerpc64le-*) platform=ieee1275 ;;
 sparc64-*) platform=ieee1275 ;;
 mipsel-*) platform=loongson ;;
 mips-*) platform=arc ;;
@@ -138,6 +139,7 @@ case "$target_cpu"-"$platform" in
   x86_64-none) ;;
   x86_64-*) target_cpu=i386 ;;
   powerpc64-ieee1275) target_cpu=powerpc ;;
+  powerpc64le-ieee1275) target_cpu=powerpc ;;
 esac
 
 # Check if the platform is supported, make final adjustments.
@@ -560,6 +562,41 @@ AC_COMPILE_IFELSE(
 ]])],
 [grub_cv_cc_target_clang=no], [grub_cv_cc_target_clang=yes])])
 
+if test x$target_cpu = xpowerpc; then
+  AC_CACHE_CHECK([for options to get big-endian compilation], grub_cv_target_cc_big_endian, [
+grub_cv_target_cc_big_endian=no
+for cand in "-target powerpc -Wl,-EB" "-target powerpc" \
+		"-target powerpc-linux-gnu -Wl,-EB" "-target powerpc-linux-gnu" \
+		"-mbig-endian"; do
+  if test x"$grub_cv_target_cc_big_endian" != xno ; then
+break
+  fi
+  CFLAGS="$TARGET_CFLAGS $cand -Werror"
+  AC_LINK_IFELSE([AC_LANG_PROGRAM([[
+#if defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && (__ORDER_BIG_ENDIAN__ != __BYTE_ORDER__)
+#error still little endian
+#endif
+asm (".globl start; start:");
+void __main (void);
+void __main (void) {}
+int main (void);
+]], [[]])],
+		[grub_cv_target_cc_big_endian="$cand"], [])
+done
+  ])
+
+  if test x"$grub_cv_target_cc_big_endian" = xno ; then
+AC_MSG_ERROR([could not force big-endian])
+  fi
+
+  skip_linkflags="$(echo "$grub_cv_target_cc_big_endian"|sed 's@-Wl,-EB@@')"
+
+  TARGET_CFLAGS="$TARGET_CFLAGS $skip_linkflags"
+  TARGET_CPPFLAGS="$TARGET_CPPFLAGS $skip_linkflags"
+  TARGET_CCASFLAGS="$TARGET_CCASFLAGS $skip_linkflags"
+  TARGET_LDFLAGS="$TARGET_LDFLAGS $grub_cv_target_cc_big_endian"
+fi
+
 AC_CACHE_CHECK([for options to compile assembly], [grub_cv_cc_target_asm_compile], [
 test_program=
 case "x$target_cpu-$platform" in


signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: On MIPS clang default to different arch than GNU

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 19.07.2015 09:33, Andrei Borzenkov wrote:
> clang -cc1 version 3.8.0 based upon LLVM 3.8.0svn default target 
> x86_64-unknown-linux-gnu
> ignoring nonexistent directory "/include"
> #include "..." search starts here:
> #include <...> search starts here:
>  /usr/local/include
>  /home/bor/build/clang/bin/../lib/clang/3.8.0/include
>  /usr/include
> End of search list.
>  "/usr/bin/mips-suse-linux-as" -march mips32r2 -mabi 32 -call_nonpic -EB 
> -msoft-float -KPIC -o conftest /tmp/foo-3da0d2.s
> bor@opensuse:~/src/llvm/tools/clang> file conftest 
> conftest: ELF 32-bit MSB relocatable, MIPS, MIPS32 rel2 version 1 (SYSV), not 
> stripped
> 
> While for the same file GAS creates
> 
> bor@opensuse:~/build/grub> mips-suse-linux-as -v asm-tests/mips.S 
> GNU assembler version 2.24.0 (mips-suse-linux) using BFD version (GNU 
> Binutils; openSUSE 13.2) 2.24.0.20140403-6.1
> bor@opensuse:~/build/grub> file a.out 
> a.out: ELF 32-bit MSB relocatable, MIPS, MIPS-I version 1 (SYSV), not stripped
> bor@opensuse:~/build/grub> 
> 
> (using -integrated-as does not change it - arch is still the same).
> 
> Not sure if it matters.
> 
> 
I think it doesn't. It's just on the level of flags. I don't think that
it really generates any non-mips1 instructions unless specifically asked
for. Just it fails to reflect this in flags in resulting binary.
Unlike in the case of x86, on mips there is no need to enable
instruction subsets, so if compiler generates instructions not supported
by target, then it's misconfigured and it's likely bigger problem than
just GRUB.
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: GRUB release schedule?

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 20.07.2015 21:25, Vladimir 'phcoder' Serbinenko wrote:
> I'll do next beta tomorrow and will assess current open bugs to see how
> far we're from release
> 
Fixing tests takes longer than expected. I'll continue tomorrow.
> Le 20 juil. 2015 20:23, "Peter Jones"  > a écrit :
> 
> Hi everyone,
> Is there a plan for when upcoming GNU GRUB releases will happen?
> 
> As far as I can tell, the last official release on
> ftp://ftp.gnu.org/gnu/grub/ was 2.00 on 28-Jun-2012, and the last beta
> on http://alpha.gnu.org/pub/gnu/grub/ for the next version was
> 2.02~beta2 on 24-Dec-2013 .  There are (give or take) 471 patches
> committed since that beta 18 months ago.
> 
> In the mean time, nearly every Linux distro is shipping a package
> derived from the 2.02~beta2 release plus some number of patches,
> some from the upstream repo and some not, and it's cumbersome to rectify
> which ones aren't upstream vs which ones have been fixed upstream with
> /nearly/ the same patch, etc., with all the noise of so many patches
> since the release.
> 
> I suspect this would be better for a lot of GRUB users if releases
> happened on a regular schedule, or if, relatively often (say once or
> twice per year), a release schedule that spans several weeks and
> organized some kind of alpha->beta->release progression were decided
> upon and followed.
> 
> So, can we make a release process that happens according to some regular
> cadence?  What needs to be done to make regular releases happen?  Going
> for years with the patch volume GRUB sees without doing a release is
> really not good for anybody.
> 
> --
> Peter
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org 
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: ELF check: GRUB_ERR_BAD_FILE_TYPE vs. GRUB_ERR_BAD_OS

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 24.07.2015 20:50, Andrei Borzenkov wrote:
> There are two places using GRUB_ERR_BAD_FILE_TYPE and other are using
> GRUB_ERR_BAD_OS when EFL class check fails. I am not sure about exact
> semantics of either but probably they should be the same?
> 
BAD_FILE_TYPE is only for errors like EISDIR and ENODIR. BAD_OS should
be used for invalid ELF contents or BAD_MODULE if elf code is primarily
meant to load GRUB-shipped ELFs.
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: cryptodisk: teach grub_cryptodisk_insert() about partitions (bug #45889)

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 06.09.2015 21:10, TJ wrote:
> https://savannah.gnu.org/bugs/index.php?45889
> 
> Boot disk with 3 LUKS/dm-crypt GPT partitions
> 
> (hd0,gpt3) (hd0,gpt4) (hd0,gpt5)
> 
> grub is in (hd0,gpt3). The others have a LVM VG each.
> 
> Using GRUB_ENABLE_CRYPTODISK=y I deliberately fail the first pass-phrase
> entry to get the rescue environment. I then
> 
> cryptomount hd0,gpt3
> 
> (crypto0) device is now present and prefix/root are set correctly. I
> insmod some other modules (exploring available functions) and
> 
> set debug=cryptodisk
> 
> I try to
> 
> cryptomount hd0,gpt4
> cryptomount hd0,gpt4
> 
> and see the message
> 
> disk/cryptodisk.c:978: already mounted as crypto0
> 
> But ls shows only (crypto0)
> 
> With the attached patch the mounts now work:
> 
> Attempting to decrypt master key...
> Enter passphrase for hd0,gpt3 ( ...UUID...)
> Slot 0 opened
>  next line comes from temporary grub_dprintf() not included in patch
>
> disk/cryptodisk.c:718: insert 0, source 'hd0,gpt3', id 128, dev_id 0
> grub rescue> ls
> (hd0) (hd0,gpt5) (hd0,gpt4) (hd0,gpt3) (hd0,gpt2) (hd0,gpt1) (crypto0)
> (proc)
> grub rescue> cryptomount hd0,gpt4
> Attempting to decrypt master key...
> Enter passphrase for hd0,gpt4 (...UUID...)
> Slot 0 opened
> disk/cryptodisk.c:718: insert 1, source 'hd0,gpt4', id 128, dev_id 0
> grub rescue> cryptomount hd0,gpt5
> Attempting to decrypt master key...
> Enter passphrase for hd0,gpt5 (...UUID...)
> Slot 0 opened
> disk/cryptodisk.c:718: insert 2, source 'hd0,gpt4', id 128, dev_id 0
> grub rescue> insmod lvm
> grub rescue> ls
> (lvm/VG_OS-x86_64.usr_local) (lvm/VG_OS-ubuntu_15.10_var)
> (lvm/VG_OS-ubuntu_15.10_rootfs) (lvm/VG_DATA-home) (hd0) (hd0,gpt5)
> (hd0,gpt4) (hd0,gpt3) (hd0,gpt2) (hd0,gpt1) (crypto2) (crypto1)
> (crypto0) (proc)
> 
> ---
>  grub-core/disk/cryptodisk.c | 7 ++-
>  include/grub/cryptodisk.h   | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 82a3dcb..0e6bc3f 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #ifdef GRUB_UTIL
>  #include 
> @@ -718,6 +719,7 @@ grub_cryptodisk_insert (grub_cryptodisk_t newdev,
> const char *name,
>newdev->id = last_cryptodisk_id++;
>newdev->source_id = source->id;
>newdev->source_dev_id = source->dev->id;
> +  newdev->partition_number = source->partition ?
> source->partition->number : 0;
>newdev->next = cryptodisk_list;
>cryptodisk_list = newdev;
> 
> @@ -740,7 +742,9 @@ grub_cryptodisk_get_by_source_disk (grub_disk_t disk)
>grub_cryptodisk_t dev;
>for (dev = cryptodisk_list; dev != NULL; dev = dev->next)
>  if (dev->source_id == disk->id && dev->source_dev_id == disk->dev->id)
> -  return dev;
> +  if ((disk->partition && disk->partition->number ==
> dev->partition_number) ||
> +  (!disk->partition && dev->partition_number == 0))
> +return dev;
Please store and compare partition start, not parition number as the
same partition can be available several times through different partiton
schemes under different numbers. Additionally this allows to use
get_partition_start which already has the logic of handling empty partitions
>return NULL;
>  }
> 
> @@ -761,6 +765,7 @@ grub_cryptodisk_cheat_insert (grub_cryptodisk_t
> newdev, const char *name,
>newdev->cheat_fd = GRUB_UTIL_FD_INVALID;
>newdev->source_id = source->id;
>newdev->source_dev_id = source->dev->id;
> +  newdev->partition_number = source->partition ?
> source->partition->number : 0;
>newdev->id = last_cryptodisk_id++;
>newdev->next = cryptodisk_list;
>cryptodisk_list = newdev;
> diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> index f2ad2a7..b638f2e 100644
> --- a/include/grub/cryptodisk.h
> +++ b/include/grub/cryptodisk.h
> @@ -97,6 +97,7 @@ struct grub_cryptodisk
>grub_uint8_t rekey_key[64];
>grub_uint64_t last_rekey;
>int rekey_derived_size;
> +  int partition_number;
>  };
>  typedef struct grub_cryptodisk *grub_cryptodisk_t;
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Remove unconditional disablement of the watchdog

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 01.09.2015 07:51, Arthur Mesh wrote:
> Starting with d9a0c9413e81d3c0affc6383693bdd28dc863a5c, GRUB unconditionally
> disables watchdog on EFI platforms. This opens up a window (starting at GRUB's
> grub_efi_init(), until OS re-enables it) when EFI system operates w/o 
> watchdog.
> If an EFI system gets stuck in that window, the chipset will never reset the
> system.
> 
> Remove the unconditional disablement of the watchdog, and create a command 
> line
> interface to enable/disable watchdog:
> 
This patch will break any current config on systems with watchdog. You
need to ensure that:
a) watchdog timeout is plenty to load config file even on very slow
netboot systems.
b) disable watchdog if the config didn't opt-in to have the watchdog.
>  efi-watchdog (enable|disable) 
> ---
>  docs/grub.texi|8 ++
>  grub-core/kern/efi/init.c |   60 
> ++---
>  2 files changed, 65 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index b9f41a7..4cfd50c 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3784,6 +3784,7 @@ you forget a command, you can run the command 
> @command{help}
>  * distrust::Remove a pubkey from trusted keys
>  * drivemap::Map a drive to another
>  * echo::Display a line of text
> +* efi-watchdog::Manipulate EFI watchdog
>  * eval::Evaluate agruments as GRUB commands
>  * export::  Export an environment variable
>  * false::   Do nothing, unsuccessfully
> @@ -4192,6 +4193,13 @@ When interpreting backslash escapes, backslash 
> followed by any other
>  character will print that character.
>  @end deffn
>  
> +@node efi-watchdog
> +@subsection efi-watchdog
> +
> +@deffn Command efi-watchdog enable|disable 
> +Enable or disable the system's watchdog timer. Only available in EFI targeted
> +GRUB.
> +@end deffn
>  
>  @node eval
>  @subsection eval
> diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
> index e9c85de..f8a79eb 100644
> --- a/grub-core/kern/efi/init.c
> +++ b/grub-core/kern/efi/init.c
> @@ -25,9 +25,62 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  grub_addr_t grub_modbase;
>  
> +static grub_command_t cmd_list;
> +
> +static grub_err_t
> +grub_cmd_efi_watchdog (grub_command_t cmd  __attribute__ ((unused)),
> +int argc, char **args)
> +{
> +long input;
> +grub_efi_status_t status;
> +grub_efi_uintn_t timeout;
> +
> +if (argc < 1)
> + return grub_error (GRUB_ERR_BAD_ARGUMENT,
> + N_("efi-watchdog (enable|disable) "));
> +
> +if (grub_strcasecmp (args[0], "enable") == 0) {
> +
> + if (argc != 2)
> + return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +N_("efi-watchdog enable "));
> +
> + input = grub_strtol (args[1], 0, 0);
> +
> + if (input >= 0) {
> + timeout = input;
> + } else {
> + return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +N_(" must be non-negative"));
> + }
> +
> +} else if (grub_strcasecmp (args[0], "disable") == 0) {
> +
> + if (argc != 1)
> + return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +N_("efi-watchdog disable"));
> + timeout = 0;
> +
> +} else {
> + return grub_error (GRUB_ERR_BAD_ARGUMENT,
> + N_("efi-watchdog (enable|disable) "));
> +}
> +
> +status = efi_call_4 
> (grub_efi_system_table->boot_services->set_watchdog_timer,
> +  timeout, 0, sizeof(L"GRUB"), L"GRUB");
> +
> +if (status != GRUB_EFI_SUCCESS)
> + return grub_error (GRUB_ERR_BUG,
> +N_("EFI SetWatchdogTimer() bug"));
> +else
> + return GRUB_ERR_NONE;
> +}
> +
>  void
>  grub_efi_init (void)
>  {
> @@ -39,10 +92,10 @@ grub_efi_init (void)
>/* Initialize the memory management system.  */
>grub_efi_mm_init ();
>  
> -  efi_call_4 (grub_efi_system_table->boot_services->set_watchdog_timer,
> -   0, 0, 0, NULL);
> -
>grub_efidisk_init ();
> +
> +  cmd_list = grub_register_command ("efi-watchdog", grub_cmd_efi_watchdog, 0,
> + N_("Enable/Disable system's watchdog 
> timer."));
>  }
>  
>  void (*grub_efi_net_config) (grub_efi_handle_t hnd, 
> @@ -77,4 +130,5 @@ grub_efi_fini (void)
>  {
>grub_efidisk_fini ();
>grub_console_fini ();
> +  grub_unregister_command (cmd_list);
>  }
> 
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] Ensure that MIPS target code is compiled for the O32 ABI.

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 23.08.2015 23:50, Mark H Weaver wrote:
> Include -mabi=32 in CFLAGS_PLATFORM and CCASFLAGS_PLATFORM to compile
> code for the O32 ABI when targetting MIPS, since the MIPS assembly code
> in GRUB assumes this.
Could you be more precise where we assume this? Why not fix the assembly
instead?
>  This flag is also needed when compiling
> asm-tests/mips.S from configure, because GNU as rejects MIPS register
> names such as $t2 unless the O32 ABI is selected.
> 
> This is needed when using a compiler that defaults to one of the newer
> MIPS ABIs such as N32 or N64, e.g. when natively compiling on a system
> that uses one of these newer MIPS ABIs.
> ---
>  conf/Makefile.common | 4 
>  configure.ac | 7 +--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/conf/Makefile.common b/conf/Makefile.common
> index fcb8d2e..bd125da 100644
> --- a/conf/Makefile.common
> +++ b/conf/Makefile.common
> @@ -20,6 +20,10 @@ endif
>  if COND_powerpc_ieee1275
>CFLAGS_PLATFORM += -mcpu=powerpc
>  endif
> +if COND_mips
> +  CFLAGS_PLATFORM += -mabi=32
> +  CCASFLAGS_PLATFORM = -mabi=32
> +endif
>  
>  #FIXME: discover and check XEN headers
>  CPPFLAGS_XEN = -I/usr/include
> diff --git a/configure.ac b/configure.ac
> index c864311..1f5e8a2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2,7 +2,8 @@
>  
>  # Process this file with autoconf to produce a configure script.
>  
> -# Copyright (C) 2002,2003,2004,2005,2006,2007,2008,2009,2010  Free Software 
> Foundation, Inc.
> +# Copyright (C) 2002,2003,2004,2005,2006,2007,2008,2009,2010,2011,
> +#   2012,2013,2014,2015  Free Software Foundation, Inc.
>  #
>  # This configure.ac is free software; the author
>  # gives unlimited permission to copy and/or distribute it,
> @@ -599,9 +600,11 @@ fi
>  
>  AC_CACHE_CHECK([for options to compile assembly], 
> [grub_cv_cc_target_asm_compile], [
>  test_program=
> +test_ccasflags=
>  case "x$target_cpu-$platform" in
>   xmips-* | xmipsel-*)
>  test_program=mips
> +test_ccasflags=-mabi=32
>   ;;
>   xi386-pc)
> test_program=i386-pc
> @@ -618,7 +621,7 @@ if test x"$test_program" = x ; then
>  else
>found=no
>for arg in "" "-no-integrated-as"; do
> -cmdline="$TARGET_CC -c -o /dev/null $TARGET_CCASFLAGS $arg 
> $TARGET_CPPFLAGS $srcdir/asm-tests/$test_program.S"
> +cmdline="$TARGET_CC -c -o /dev/null $TARGET_CCASFLAGS $test_ccasflags 
> $arg $TARGET_CPPFLAGS $srcdir/asm-tests/$test_program.S"
>  echo "Running $cmdline" >&AS_MESSAGE_LOG_FD
>  if $cmdline >&AS_MESSAGE_LOG_FD 2>&AS_MESSAGE_LOG_FD; then
>grub_cv_cc_target_asm_compile="$arg"
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

GNU GRUB maintenance

[Vladimir 'φ-coder/phcoder' Serbinenko]

Hello, all. I'm sorry for not being available to do enough maintenance
for GRUB in last time but I was overbooked. Yet there is a good news. At
Google there is a 20% project and GRUB has been approved as 20% project
for me. The goal is to have 2.02 released before the end of this year.
Other than the raw lack of time there is another issue which makes
maintenance difficult: inefficient VCS. It requires me or someone with
privileges manually copy the patch. What other systems would be ok? It
obviously has to be a free software and hosted on free software-friendly
hosting. It also has to have an efficient 1-click merge (so that someone
with privileges can get any patch submitted to the system merged in
couple of clicks).



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: GNU GRUB maintenance

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 07.10.2015 23:36, SevenBits wrote:
> 
> 
> On Wednesday, October 7, 2015, Vladimir 'φ-coder/phcoder' Serbinenko
> mailto:phco...@gmail.com>> wrote:
> 
> Hello, all. I'm sorry for not being available to do enough maintenance
> for GRUB in last time but I was overbooked. Yet there is a good news. At
> Google there is a 20% project and GRUB has been approved as 20% project
> for me. The goal is to have 2.02 released before the end of this year.
> Other than the raw lack of time there is another issue which makes
> maintenance difficult: inefficient VCS. It requires me or someone with
> privileges manually copy the patch. What other systems would be ok? It
> obviously has to be a free software and hosted on free software-friendly
> hosting. It also has to have an efficient 1-click merge (so that someone
> with privileges can get any patch submitted to the system merged in
> couple of clicks).
> 
> 
> 
> I know that git is pretty popular... 
> 
Git integration is a plus. Like gerrit: git based with a review system
on top of it.
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] sparc64: boot performance improvements

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 06.10.2015 19:39, Eric Snowberg wrote:
> Keep of devices open.  This can save 6 - 7 seconds per open call and
> can decrease boot times from over 10 minutes to 29 seconds on
> larger SPARC systems.  The open/close calls with some vendors'
> SAS controllers cause the entire card to be reinitialized after
> each close.
> 
Unfortunately on some systems (AFAIR old sparcs) hard-locks if the same
device is opened twice. We tried to detect when 2 devices are named
differently but it's not possible to do reliably on IEEE1275. Our only
solution was to ensure that only 1 disk is open at a time. Do you use
some kind of RAID? Can you try keeping last_handle open in ofdisk.c
rather than closing it?
> Signed-off-by: Eric Snowberg 
> ---
>  grub-core/kern/ieee1275/ieee1275.c |   39 
> 
>  1 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/grub-core/kern/ieee1275/ieee1275.c 
> b/grub-core/kern/ieee1275/ieee1275.c
> index 9821702..30f973b 100644
> --- a/grub-core/kern/ieee1275/ieee1275.c
> +++ b/grub-core/kern/ieee1275/ieee1275.c
> @@ -19,11 +19,24 @@
>  
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  
>  #define IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_cell_t) -1)
>  #define IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_cell_t) 0)
>  #define IEEE1275_CELL_INVALID ((grub_ieee1275_cell_t) -1)
>  
> +struct grub_of_opened_device
> +{
> +  struct grub_of_opened_device *next;
> +  struct grub_of_opened_device **prev;
> +  grub_ieee1275_ihandle_t ihandle;
> +  char *path;
> +};
> +
> +static struct grub_of_opened_device *grub_of_opened_devices;
> +
>  
>  
>  int
> @@ -452,6 +465,18 @@ grub_ieee1275_open (const char *path, 
> grub_ieee1275_ihandle_t *result)
>}
>args;
>  
> +  struct grub_of_opened_device *dev;
> +
> +  FOR_LIST_ELEMENTS(dev, grub_of_opened_devices)
> +if (grub_strcmp(dev->path, path) == 0)
> +  break;
> +
> +  if (dev)
> +  {
> +*result = dev->ihandle;
> +return 0;
> +  }
> +
>INIT_IEEE1275_COMMON (&args.common, "open", 1, 1);
>args.path = (grub_ieee1275_cell_t) path;
>  
> @@ -460,6 +485,11 @@ grub_ieee1275_open (const char *path, 
> grub_ieee1275_ihandle_t *result)
>*result = args.result;
>if (args.result == IEEE1275_IHANDLE_INVALID)
>  return -1;
> +
> +  dev = grub_zalloc(sizeof(struct grub_of_opened_device));
> +  dev->path = grub_strdup(path);
> +  dev->ihandle = args.result;
> +  grub_list_push(GRUB_AS_LIST_P (&grub_of_opened_devices), GRUB_AS_LIST 
> (dev));
>return 0;
>  }
>  
> @@ -473,6 +503,15 @@ grub_ieee1275_close (grub_ieee1275_ihandle_t ihandle)
>}
>args;
>  
> +  struct grub_of_opened_device *dev;
> +
> +  FOR_LIST_ELEMENTS(dev, grub_of_opened_devices)
> +if (dev->ihandle == ihandle)
> +  break;
> +
> +  if (dev)
> +return 0;
> +
>INIT_IEEE1275_COMMON (&args.common, "close", 1, 0);
>args.ihandle = ihandle;
>  
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] Ensure that MIPS target code is compiled for the O32 ABI.

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 13.09.2015 08:32, Andrei Borzenkov wrote:
> 08.09.2015 20:11, Vladimir 'φ-coder/phcoder' Serbinenko пишет:
>> On 23.08.2015 23:50, Mark H Weaver wrote:
>>> Include -mabi=32 in CFLAGS_PLATFORM and CCASFLAGS_PLATFORM to compile
>>> code for the O32 ABI when targetting MIPS, since the MIPS assembly code
>>> in GRUB assumes this.
>> Could you be more precise where we assume this? Why not fix the assembly
>> instead?
> 
> If I understand it correctly, this is not only about assembly - ABIs
> differ in sizes of types as well,
Only in obscure types long double and uint128_t. We use neither.
> so we'd need to define whole new CPU
> in grub.
Nope
> Not sure if it's worth it. We can consider ourselves lucky it
> was caught that early.
The differences are:
* Use of rela instead of rel section. (I've prepared a patch)
* Register names: There is no $t5 but there is $a5. I prepared a draft,
cleaning up
* Different parameter/result passing in presence of either
  a) >4 parameters
  b) floats
  c) 64-bit integers
  d) structs
 Neither happens between C and assembly. In all we have only following
interfaces between C and assembly:
  a) Relocator call. No arguments void (*) (void);
  b) void grub_main(void);
  c) void
grub_decompress_core (void *src, void *dst, unsigned long srcsize,
  unsigned long dstsize);
  d) void EXPORT_FUNC(grub_arch_sync_caches) (void *address, grub_size_t
len);
  e) void grub_arch_sync_dma_caches (void *address, grub_size_t len);
  f) int grub_setjmp (grub_jmp_buf env) // grub_jmp_buf is an array, so
pointer
  g) int grub_longjmp (grub_jmp_buf env, int val)
* $gp is caller vs callee-saved. We don't use $gp in assembly except in
setjmp which is already appropriate for n32.
So, we need just few cleanups to make it work. Whether we want to
continuously maintain this compatibility is another question. I'm going
to commit the fixes





signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] Ensure that MIPS target code is compiled for the O32 ABI.

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 09.10.2015 23:14, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 13.09.2015 08:32, Andrei Borzenkov wrote:
>> 08.09.2015 20:11, Vladimir 'φ-coder/phcoder' Serbinenko пишет:
>>> On 23.08.2015 23:50, Mark H Weaver wrote:
>>>> Include -mabi=32 in CFLAGS_PLATFORM and CCASFLAGS_PLATFORM to compile
>>>> code for the O32 ABI when targetting MIPS, since the MIPS assembly code
>>>> in GRUB assumes this.
>>> Could you be more precise where we assume this? Why not fix the assembly
>>> instead?
>>
>> If I understand it correctly, this is not only about assembly - ABIs
>> differ in sizes of types as well,
> Only in obscure types long double and uint128_t. We use neither.
Correction: I was comparing o32 to n32. We still need to ensure that
either o32 or n32 is used. It's easier to just ensure that o32 is used,
so that we have less variability in the code but can be relaxed later if
need be.
>   f) int grub_setjmp (grub_jmp_buf env) // grub_jmp_buf is an array, so
> pointer
>   g) int grub_longjmp (grub_jmp_buf env, int val)
Correction: setjmp/longjmp are not adapted to n32 but we don't use them
anyway. Probably it's time to delete the dead code after double checking
that extras don't use it either.




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: GNU GRUB maintenance

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 08.10.2015 21:34, Konrad Rzeszutek Wilk wrote:
> On October 8, 2015 10:52:25 AM EDT, Andrei Borzenkov  
> wrote:
>> On Thu, Oct 8, 2015 at 12:14 AM, Vladimir 'φ-coder/phcoder' Serbinenko
>>  wrote:
>>> Hello, all. I'm sorry for not being available to do enough
>> maintenance
>>> for GRUB in last time but I was overbooked. Yet there is a good news.
>> At
>>> Google there is a 20% project and GRUB has been approved as 20%
>> project
>>> for me. The goal is to have 2.02 released before the end of this
>> year.
>>> Other than the raw lack of time there is another issue which makes
>>> maintenance difficult: inefficient VCS.
>>
>> VCS is actually OK. The project of size Linux kernel seems to work
>> well using pull request e-mails. The disadvantages are
>>
>> - contributors must have repository available via Internet
> 
> 
> That is quite easy nowadays. And you can always ask for signed tags if you 
> are worried about repos being subverted.
> 
>> - contributors are trusted to actually submit pull request for branch
>> that was reviewed
> 
> 
> 
> 
> It is a disadvantage to trust people!?
> 
> 
>> - it needs to be done locally and pushed
> 
> 
> Or you can have different maintainers pushing the patches in if they are 
> Acked or Reviewed.
> 
> Meaning the committee does not have to be the same person who reviews/acks it.
> 
>>
>>>   It requires me
>> or someone with
>>> privileges manually copy the patch. What other systems would be ok?
>> It
>>> obviously has to be a free software and hosted on free
>> software-friendly
>>> hosting. It also has to have an efficient 1-click merge (so that
>> someone
>>> with privileges can get any patch submitted to the system merged in
>>> couple of clicks).
>>>
>>>
> 
> Clicks? That sounds like a GUI thing. And it sounds like you need to have an 
> admin to set it up, patch it occasionally, deal with spammers, etc.
> 
> What is wrong with the old mechanism of emails.
> 
It takes too much effort to:
a) Track if there are any unresolved issues
b) It takes non-trivial amount of effort to commit once it's reviewed:
you need to copy patch from mail client to git, do commit, copy
description and so on
c) No integration with continous testing systems



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: GNU GRUB maintenance

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 08.10.2015 16:52, Andrei Borzenkov wrote:
> On Thu, Oct 8, 2015 at 12:14 AM, Vladimir 'φ-coder/phcoder' Serbinenko
>  wrote:
>> Hello, all. I'm sorry for not being available to do enough maintenance
>> for GRUB in last time but I was overbooked. Yet there is a good news. At
>> Google there is a 20% project and GRUB has been approved as 20% project
>> for me. The goal is to have 2.02 released before the end of this year.
>> Other than the raw lack of time there is another issue which makes
>> maintenance difficult: inefficient VCS.
> 
> VCS is actually OK. The project of size Linux kernel seems to work
> well using pull request e-mails. The disadvantages are
> 
> - contributors must have repository available via Internet
> - contributors are trusted to actually submit pull request for branch
> that was reviewed
> - it needs to be done locally and pushed
> 
>>   It requires me or 
>> someone with
>> privileges manually copy the patch. What other systems would be ok? It
>> obviously has to be a free software and hosted on free software-friendly
>> hosting. It also has to have an efficient 1-click merge (so that someone
>> with privileges can get any patch submitted to the system merged in
>> couple of clicks).
>>
>>
> 
> It does not like like we have much choice. If we speak about free
> external hosting, this is probably github, gerrithub, gitlab. I do not
> know if any of them is considered friendly enough by FSF.
> 
> If we speak about self hosting, then it is probably gerrit and
> reviewboard (I wish we could join KDE reviewboard, but grub hardly can
> be called KDE application ... :) )
> 
> I am not thrilled by github workflows. From what I could gather
> gerrithub looks more appealing, but would love to hear from someone
> who actually used both.
> 
I spoke with Stefan Reinauer and he proposed to host us at
review.coreboot.org if we don't generate too much traffic. I had
positive experiences with their gerrit except that some functions are
broken on mobile. I'd like to be able to review from phone but it's not
a hard requirement.
> One problem is that none of them apparently allows reviewing by
> E-Mail. This worked (and probably works, just I'm no more involved)
> quite well in KDE reviewboard. This means all review must be done via
> web. For me it is rather disadvantage.
That's a disadvantage but I believe that being able to get changes
merged quickly outweights this disadvantage.
> Also merged requests are
> removed, which means history and past discussions are no more present.
They're kept on review.coreboot.org case
> Which again is better using e-mail review.
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Add check when store disk cache

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 19.09.2015 09:00, Andrei Borzenkov wrote:
> 18.09.2015 12:07, Arch Stack пишет:
>> I want to use the part of the filesystem codes in GRUB to read different
>> filesystems on Windows. I have almost completed it and I will release
>> it in
>> a few days.
>> But it crash sometimes because of the write of zero pointer.I debug it
>> and
>> find why it crashed.
> 
> Please show stack trace of crash. GRUB does not run multithreaded so
> something different must happen.
> 
Most likely he has just run the grub code multithreaded in his fuse-like
driver. It's probably a bad idea since GRUB is not designed for this and
in fact this lock is more for the cases when one disk code calls another
disk code. I'm not sure it actually can happen i
n current code. Moreover current way of handling ->lock isn't thread-safe
as we first check it and then set rather than using a lock primitive. If you
want to go into making parts of GRUB multi-threaded I suggest going with
giant lock
first and then breaking it down progressively. The locks should fully
disappear when compiling for real GRUB. This will probably require
preprocessor magic which makes variables disappear when GRUB_UTIL is not
defined.


>> When I apply this patch, it won't crash because of this
>> reason.
>>
>> On Fri, Sep 18, 2015 at 12:03 PM, Andrei Borzenkov 
>> wrote:
>>
>>> 18.09.2015 03:15, Arch Stack пишет:
>>>
 I found that the function *grub_disk_cache_store* didn't check for
 *cache->lock* before free *cache->data*, and didn't set *cache->lock*
 before memcpy something to *cache->data*. If multi thread handle
 with the
 same cache at the same time, it will cause a fault.

>>>
>>> Do you actually observe a problem or it is pure hypothesis? GRUB does
>>> not
>>> run multi-threaded and probably never will.
>>>
>>> I have created a patch
 for it.



 ___
 Grub-devel mailing list
 Grub-devel@gnu.org
 https://lists.gnu.org/mailman/listinfo/grub-devel


>>>
>>> ___
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
>>
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Keyboard not working with QEMU and coreboot and GRUB payload

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 18.08.2015 14:11, Andrei Borzenkov wrote:
> On Tue, Aug 18, 2015 at 12:41 AM, Paul Menzel
>  wrote:
>> Dear GRUB folks,
>>
>>
>> just a note that using QEMU with coreboot and GRUB payload, I am unable
>> to enter anything.
>>
>> Using libpayload based payloads, the keyboard works, so I think it’s
>> GRUB related.
>>
> 
> Does it work with any earlier GRUB version?
> 
Checked with latest GRUB HEAD and couple days old coreboot HEAD. It works.
QEMU:
QEMU emulator version 2.4.0 (Debian 1:2.4+dfsg-4), Copyright (c)
2003-2008 Fabrice Bellard

Please retest and provide qemu details if it still fails
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] sparc64: boot performance improvements

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 11.10.2015 18:49, Eric Snowberg wrote:
> 
>> On Oct 10, 2015, at 1:35 AM, Vladimir 'phcoder' Serbinenko 
>>  wrote:
>>
>>
>> Le 10 oct. 2015 3:31 AM, "Eric Snowberg"  a écrit :
>>>
>>>
 On Oct 9, 2015, at 12:50 AM, Andrei Borzenkov  wrote:

 On Thu, Oct 8, 2015 at 2:20 AM, Eric Snowberg  
 wrote:
>
>> On Oct 7, 2015, at 2:36 AM, Andrei Borzenkov  wrote:
>>
>> On Tue, Oct 6, 2015 at 8:39 PM, Eric Snowberg  
>> wrote:
>>> Keep of devices open.  This can save 6 - 7 seconds per open call and
>>> can decrease boot times from over 10 minutes to 29 seconds on
>>> larger SPARC systems.  The open/close calls with some vendors'
>>> SAS controllers cause the entire card to be reinitialized after
>>> each close.
>>>
>>
>> Is it necessary to close these handles before launching kernel?
>
> On SPARC hardware it is not necessary.  I would be interested to hear if 
> it is necessary on other IEEE1275 platforms.
>
>> It sounds like it can accumulate quite a lot of them - are there any
>> memory limits/restrictions? Also your patch is rather generic and so
>> applies to any IEEE1275 platform, I think some of them may have less
>> resources. Just trying to understand what run-time cost is.
>
> The most I have seen are two entries in the linked list.  So the 
> additional memory requirements are very small.  I have tried this on a 
> T2000, T4, and T5.

 I thought you were speaking about *larger* SPARC servers :)

 Anyway I'd still prefer if this would be explicitly restricted to
 Oracle servers. Please look at
 grub-core/kern/ieee1275/cmain.c:grub_ieee1275_find_options() if new
 quirk can be added to detect your platform(s).

>>>
>>> That makes sense, I’ll restrict it to all sun4v equipment.
>>>
>> Not good enough. Some of sun4v probably have the bug I told about in the 
>> other e-mail
> 
> I can get access to every sun4v platform.  Could you provide any additional 
> information on which sun4v systems had this failure.  If so, I’ll try to 
> submit a fix for that problem as well.  It seems like there could be a better 
> way to handle this than resetting the SAS or SATA HBA before each read 
> operation.
> 
>
See http://permalink.gmane.org/gmane.comp.boot-loaders.grub.devel/10533
for holding open handle.
Do you readily have a scenario when you need multiple handles open?
Can you try following naive optimisation:
diff --git a/grub-core/disk/ieee1275/ofdisk.c
b/grub-core/disk/ieee1275/ofdisk.c
index 331769b..34237f9 100644
--- a/grub-core/disk/ieee1275/ofdisk.c
+++ b/grub-core/disk/ieee1275/ofdisk.c
@@ -448,13 +448,6 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
 static void
 grub_ofdisk_close (grub_disk_t disk)
 {
-  if (disk->data == last_devpath)
-{
-  if (last_ihandle)
-   grub_ieee1275_close (last_ihandle);
-  last_ihandle = 0;
-  last_devpath = NULL;
-}
   disk->data = 0;
 }


> There are many problems with the upstream grub2 implementation for sun4v.  I 
> will be submitting additional follow on patches, since it currently will not 
> even boot to the grub menu.  My intention is to get grub2 working on every 
> sun4v platform.
> 
Could you start with
> 

>
> The path name sent into the grub_ieee1275_open function is not consistent 
> throughout the code, even though it is referencing the same device.  
> Originally I tried having a global containing the path sent into 
> grub_ieee1275_open, but this failed due to the various ways of 
> referencing the same device name.  That is why I added the linked list 
> just to be safe.  Caching the grub_ieee1275_ihandle_t this way saves a 
> significant amount of time, since OF calls are expensive.  We were seeing 
> the same device opened 50+ times in the process of displaying the grub 
> menu and then launching the kernel.
>
>>
>>> Signed-off-by: Eric Snowberg 
>>> ---
>>> grub-core/kern/ieee1275/ieee1275.c |   39 
>>> 
>>> 1 files changed, 39 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/grub-core/kern/ieee1275/ieee1275.c 
>>> b/grub-core/kern/ieee1275/ieee1275.c
>>> index 9821702..30f973b 100644
>>> --- a/grub-core/kern/ieee1275/ieee1275.c
>>> +++ b/grub-core/kern/ieee1275/ieee1275.c
>>> @@ -19,11 +19,24 @@
>>>
>>> #include 
>>> #include 
>>> +#include 
>>> +#include 
>>> +#include 
>>>
>>> #define IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_cell_t) -1)
>>> #define IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_cell_t) 0)
>>> #define IEEE1275_CELL_INVALID ((grub_ieee1275_cell_t) -1)
>>>
>>> +struct grub_of_opened_device
>>> +{
>>> +  struct grub_of_opened_device *next;
>>> +  struct grub_of_opened_device **prev;
>>> +  grub_ieee1275_ihandle_t ihandle;
>>> +  char *path;
>>> +};
>>> 

Re: [PATCH] Make CTRL and ALT keys work as expected on EFI systems (version 5).

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 25.02.2014 23:12, Peter Jones wrote:
> This is version 4.
> 
> Changes from version 1:
> - handles SHIFT as a modifier
> - handles F11 and F12 keys
> - uses the handle provided by the system table to find our _EX protocol.
> 
> Changes from version 2:
> - eliminate duplicate keycode translation.
> 
> Changes from version 3:
> - Do not add the shift modifier for any ascii character between space
>   (0x20) and DEL (0x7f); the combination of the modifier and many of the
>   keys causes it not to be recognized at all.  Specifically, if we
>   include the modifier on any querty punctuation character, i.e.
>   anything the string "~!@#$%^&*()_+{}|:\"<>?" represents in C, it stops
>   being recognized whatsoever.
> 
> Changes from version 4:
> - Always initialize term->data from locate protocol (i.e. make it
>   unconditional.)
> 
> Signed-off-by: Peter Jones 
> ---
>  grub-core/term/efi/console.c | 118 
> +++
>  include/grub/efi/api.h   |  65 +++-
>  2 files changed, 161 insertions(+), 22 deletions(-)
> 
> diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
> index a37eb84..677eab5 100644
> --- a/grub-core/term/efi/console.c
> +++ b/grub-core/term/efi/console.c
> @@ -104,26 +104,12 @@ const unsigned efi_codes[] =
>  GRUB_TERM_KEY_DC, GRUB_TERM_KEY_PPAGE, GRUB_TERM_KEY_NPAGE, 
> GRUB_TERM_KEY_F1,
>  GRUB_TERM_KEY_F2, GRUB_TERM_KEY_F3, GRUB_TERM_KEY_F4, GRUB_TERM_KEY_F5,
>  GRUB_TERM_KEY_F6, GRUB_TERM_KEY_F7, GRUB_TERM_KEY_F8, GRUB_TERM_KEY_F9,
> -GRUB_TERM_KEY_F10, 0, 0, '\e'
> +GRUB_TERM_KEY_F10, GRUB_TERM_KEY_F10, GRUB_TERM_KEY_F11, '\e'
>};
>  
Yikes
> -
>  static int
> -grub_console_getkey (struct grub_term_input *term __attribute__ ((unused)))
> +grub_efi_translate_key (grub_efi_input_key_t key)
>  {
> -  grub_efi_simple_input_interface_t *i;
> -  grub_efi_input_key_t key;
> -  grub_efi_status_t status;
> -
> -  if (grub_efi_is_finished)
> -return 0;
> -
> -  i = grub_efi_system_table->con_in;
> -  status = efi_call_2 (i->read_key_stroke, i, &key);
> -
> -  if (status != GRUB_EFI_SUCCESS)
> -return GRUB_TERM_NO_KEY;
> -
>if (key.scan_code == 0)
>  {
>/* Some firmware implementations use VT100-style codes against the 
> spec.
> @@ -139,9 +125,98 @@ grub_console_getkey (struct grub_term_input *term 
> __attribute__ ((unused)))
>else if (key.scan_code < ARRAY_SIZE (efi_codes))
>  return efi_codes[key.scan_code];
>  
> +  if (key.unicode_char >= 0x20 && key.unicode_char <= 0x7f)
> +return key.unicode_char;
> +
This ignores enter, tab and so on.

I fixed 2 above issues and committed.



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] Suport for bi-endianess in elf file

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 23.07.2015 16:10, Paulo Flabiano Smorigo wrote:
> Updated version with the suggestions from Andrei Borzenkov.
> 
> * grub-core/kern/elf.c: check and switch endianess with grub_{be,le}_to
> cpu functions.
> * grub-core/kern/elfXX.c: Likewise.
Applied after fixing following problems:
* Since you already use WORDS_BIGENDIAN you might as well use swap_bytes
and get some benefits of it like smaller code (we usually prefer
le_to_cpu but their point is exactly to avoid to check for WORDS_BIGENDIAN)
* phdr iterator assumed that phentsize == sizeof (*ElfXX_Phdr).
* __powerpc__ && IEEE1275 condition was repeated way too often.
Encapsulated.
* Confusing function names
* file-local functions non-static
* Missing error check
> 
> Also-by: Tomohiro B Berry 
> ---
>  grub-core/kern/elf.c   | 51 ++---
>  grub-core/kern/elfXX.c | 76 
> ++
>  2 files changed, 123 insertions(+), 4 deletions(-)
> 
> diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
> index 5f99c43..1be9c1c 100644
> --- a/grub-core/kern/elf.c
> +++ b/grub-core/kern/elf.c
> @@ -28,6 +28,11 @@
>  
>  GRUB_MOD_LICENSE ("GPLv3+");
>  
> +#if defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275)
> +void grub_elf32_check_endianess (grub_elf_t elf);
> +void grub_elf64_check_endianess (grub_elf_t elf);
> +#endif /* defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275) */
> +
>  /* Check if EHDR is a valid ELF header.  */
>  static grub_err_t
>  grub_elf_check_header (grub_elf_t elf)
> @@ -38,8 +43,19 @@ grub_elf_check_header (grub_elf_t elf)
>|| e->e_ident[EI_MAG1] != ELFMAG1
>|| e->e_ident[EI_MAG2] != ELFMAG2
>|| e->e_ident[EI_MAG3] != ELFMAG3
> -  || e->e_ident[EI_VERSION] != EV_CURRENT
> -  || e->e_version != EV_CURRENT)
> +  || e->e_ident[EI_VERSION] != EV_CURRENT)
> +return grub_error (GRUB_ERR_BAD_OS, N_("invalid arch-independent ELF 
> magic"));
> +
> +#if defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275)
> +  if (grub_elf_is_elf32 (elf))
> +  grub_elf32_check_endianess (elf);
> +  else if (grub_elf_is_elf64 (elf))
> +  grub_elf64_check_endianess (elf);
> +  else
> +return grub_error (GRUB_ERR_BAD_OS, N_("Uknown ELF class"));
> +#endif /* defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275) */
> +
> +  if (e->e_version != EV_CURRENT)
>  return grub_error (GRUB_ERR_BAD_OS, N_("invalid arch-independent ELF 
> magic"));
>  
>return GRUB_ERR_NONE;
> @@ -116,7 +132,11 @@ grub_elf_open (const char *name)
>return elf;
>  }
>  
> -
> +#define grub_be_to_halfXX grub_be_to_cpu16
> +#define grub_le_to_halfXX grub_le_to_cpu16
> +#define grub_be_to_wordXX grub_be_to_cpu32
> +#define grub_le_to_wordXX grub_le_to_cpu32
> +
>  /* 32-bit */
>  #define ehdrXX ehdr32
>  #define ELFCLASSXX ELFCLASS32
> @@ -127,7 +147,15 @@ grub_elf_open (const char *name)
>  #define grub_elf_is_elfXX grub_elf_is_elf32
>  #define grub_elfXX_load_phdrs grub_elf32_load_phdrs
>  #define ElfXX_Phdr Elf32_Phdr
> +#define ElfXX_Ehdr Elf32_Ehdr
>  #define grub_uintXX_t grub_uint32_t
> +#define grub_be_to_addrXX grub_be_to_cpu32
> +#define grub_le_to_addrXX grub_le_to_cpu32
> +#define grub_be_to_offXX grub_be_to_cpu32
> +#define grub_le_to_offXX grub_le_to_cpu32
> +#define grub_be_to_XwordXX grub_be_to_cpu32
> +#define grub_le_to_XwordXX grub_le_to_cpu32
> +#define grub_elfXX_check_endianess grub_elf32_check_endianess
>  
>  #include "elfXX.c"
>  
> @@ -140,9 +168,16 @@ grub_elf_open (const char *name)
>  #undef grub_elf_is_elfXX
>  #undef grub_elfXX_load_phdrs
>  #undef ElfXX_Phdr
> +#undef ElfXX_Ehdr
>  #undef grub_uintXX_t
> +#undef grub_be_to_addrXX
> +#undef grub_le_to_addrXX
> +#undef grub_be_to_offXX
> +#undef grub_le_to_offXX
> +#undef grub_be_to_XwordXX
> +#undef grub_le_to_XwordXX
> +#undef grub_elfXX_check_endianess
>  
> -
>  /* 64-bit */
>  #define ehdrXX ehdr64
>  #define ELFCLASSXX ELFCLASS64
> @@ -153,6 +188,14 @@ grub_elf_open (const char *name)
>  #define grub_elf_is_elfXX grub_elf_is_elf64
>  #define grub_elfXX_load_phdrs grub_elf64_load_phdrs
>  #define ElfXX_Phdr Elf64_Phdr
> +#define ElfXX_Ehdr Elf64_Ehdr
>  #define grub_uintXX_t grub_uint64_t
> +#define grub_be_to_addrXX grub_be_to_cpu64
> +#define grub_le_to_addrXX grub_le_to_cpu64
> +#define grub_be_to_offXX grub_be_to_cpu64
> +#define grub_le_to_offXX grub_le_to_cpu64
> +#define grub_be_to_XwordXX grub_be_to_cpu64
> +#define grub_le_to_XwordXX grub_le_to_cpu64
> +#define grub_elfXX_check_endianess grub_elf64_check_endianess
>  
>  #include "elfXX.c"
> diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
> index 1d09971..c14e071 100644
> --- a/grub-core/kern/elfXX.c
> +++ b/grub-core/kern/elfXX.c
> @@ -31,6 +31,39 @@ grub_elfXX_load_phdrs (grub_elf_t elf)
>return grub_errno;
>  }
>  
> +#if defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275)
> +  ElfXX_Phdr *phdr;
> +  for (phdr = elf->phdrs;
> +   phdr && phdr < (ElfXX_Phdr *) elf->phdrs + elf->e

Re: [PATCH 3/3] ieee1275: ofdisk - don't continue to query block-size after we have it

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 26.10.2015 22:43, Eric Snowberg wrote:
> Within commit: 87ec3b7fa9061f470616ed927fc140e995831c00 - "Don't continue
> to query block-size if disk doesn't have it.”  Disks that returned 0 to the
> block-size query, still get queried every time.
> 
> Fix logic in grub_ofdisk_get_block_size so the block size is not requested
> upon each open since it will not change.
> 
Is it true for removable disks as well? What about someone unplugging
USB stick and plugging-in 4K USB HDD?
> Signed-off-by: Eric Snowberg 
> ---
>  grub-core/disk/ieee1275/ofdisk.c |   30 +++---
>  1 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/grub-core/disk/ieee1275/ofdisk.c 
> b/grub-core/disk/ieee1275/ofdisk.c
> index 297f058..a75ea51 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -35,7 +35,8 @@ struct ofdisk_hash_ent
>char *grub_devpath;
>int is_boot;
>int is_removable;
> -  int block_size_fails;
> +  int block_size_retries;
> +  grub_uint32_t block_size;
>/* Pointer to shortest available name on nodes representing canonical 
> names,
>   otherwise NULL.  */
>const char *shortest;
> @@ -446,7 +447,17 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
>disk->log_sector_size = 9;
>}
>  
> +  if (last_ihandle)
> +grub_ieee1275_close (last_ihandle);
> +
> +  last_ihandle = 0;
> +  last_devpath = NULL;
> +  grub_ieee1275_open (devpath, &last_ihandle);
>grub_free (devpath);
> +
> +  if (! last_ihandle)
> +return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "can't open device");
> +
>return 0;
>  }
>  
> @@ -619,6 +630,12 @@ grub_ofdisk_get_block_size (const char *device, 
> grub_uint32_t *block_size,
>grub_ieee1275_cell_t size2;
>  } args_ieee1275;
>  
> +  if ((op->block_size_retries >= 2) || (op->block_size > 0))
> +{
> +  *block_size = op->block_size;
> +  return GRUB_ERR_NONE;
> +}
> +
>if (last_ihandle)
>  grub_ieee1275_close (last_ihandle);
>  
> @@ -630,9 +647,7 @@ grub_ofdisk_get_block_size (const char *device, 
> grub_uint32_t *block_size,
>  return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "can't open device");
>  
>*block_size = 0;
> -
> -  if (op->block_size_fails >= 2)
> -return GRUB_ERR_NONE;
> +  op->block_size_retries++;
>  
>INIT_IEEE1275_COMMON (&args_ieee1275.common, "call-method", 2, 2);
>args_ieee1275.method = (grub_ieee1275_cell_t) "block-size";
> @@ -642,21 +657,22 @@ grub_ofdisk_get_block_size (const char *device, 
> grub_uint32_t *block_size,
>if (IEEE1275_CALL_ENTRY_FN (&args_ieee1275) == -1)
>  {
>grub_dprintf ("disk", "can't get block size: failed call-method\n");
> -  op->block_size_fails++;
>  }
>else if (args_ieee1275.result)
>  {
>grub_dprintf ("disk", "can't get block size: %lld\n",
>   (long long) args_ieee1275.result);
> -  op->block_size_fails++;
>  }
>else if (args_ieee1275.size1
>  && !(args_ieee1275.size1 & (args_ieee1275.size1 - 1))
>  && args_ieee1275.size1 >= 512 && args_ieee1275.size1 <= 16384)
>  {
> -  op->block_size_fails = 0;
> +  op->block_size = args_ieee1275.size1;
>*block_size = args_ieee1275.size1;
>  }
>  
> +  grub_ieee1275_close (last_ihandle);
> +  last_ihandle = 0;
> +
>return 0;
>  }
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/3] ieee1275: ofdisk dangling pointer

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 26.10.2015 22:43, Eric Snowberg wrote:
> Within commit: 87ec3b7fa9061f470616ed927fc140e995831c00 -
> "Don't continue to query block-size if disk doesn't have it.”
> a dangling pointer was introduced.
> 
> Fix dangling pointer issue in grub_ofdisk_open where devpath is freed
> and then used again within the call to grub_ofdisk_get_block_size. This
> solves many memory corruption issues we were seeing.
> 
Committed, thanks
> Signed-off-by: Eric Snowberg 
> ---
>  grub-core/disk/ieee1275/ofdisk.c |7 ---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/grub-core/disk/ieee1275/ofdisk.c 
> b/grub-core/disk/ieee1275/ofdisk.c
> index 331769b..4a5632c 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -422,10 +422,11 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
>  op = ofdisk_hash_find (devpath);
>  if (!op)
>op = ofdisk_hash_add (devpath, NULL);
> -else
> -  grub_free (devpath);
>  if (!op)
> -  return grub_errno;
> +  {
> +grub_free (devpath);
> +return grub_errno;
> +  }
>  disk->id = (unsigned long) op;
>  disk->data = op->open_path;
>  
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 2/3] ieee1275: ofdisk memory leak

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 26.10.2015 22:43, Eric Snowberg wrote:
> Fix memory leak added within commit:
> 87ec3b7fa9061f470616ed927fc140e995831c00 - "Don't continue to
> query block-size if disk doesn't have it.”
> 
Committed, thanks
> Signed-off-by: Eric Snowberg 
> ---
>  grub-core/disk/ieee1275/ofdisk.c |6 +-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/grub-core/disk/ieee1275/ofdisk.c 
> b/grub-core/disk/ieee1275/ofdisk.c
> index 4a5632c..297f058 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -432,7 +432,10 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
>  
>  err = grub_ofdisk_get_block_size (devpath, &block_size, op);
>  if (err)
> -  return err;
> +  {
> +grub_free (devpath);
> +return err;
> +  }
>  if (block_size != 0)
>{
>   for (disk->log_sector_size = 0;
> @@ -443,6 +446,7 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
>disk->log_sector_size = 9;
>}
>  
> +  grub_free (devpath);
>return 0;
>  }
>  
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 1/4] arm64: Add and export some accessor functions for xen boot

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 23.07.2015 07:16, fu@linaro.org wrote:
> From: Fu Wei 
> 
> Add accessor functions of "loaded" flag in
> grub-core/loader/arm64/linux.c.
> 
> Export accessor functions of "loaded" flag and
> grub_linux_get_fdt function in include/grub/arm64/linux.h.
> 
> Purpose: Reuse the existing code of devicetree in linux module.
> 
> Signed-off-by: Fu Wei 
> ---
>  grub-core/loader/arm64/linux.c | 13 +
>  include/grub/arm64/linux.h |  6 +-
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> index 987f5b9..cf6026e 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -48,6 +48,19 @@ static grub_addr_t initrd_end;
>  static void *loaded_fdt;
>  static void *fdt;
>  
> +/* The accessor functions for "loaded" flag */
> +int
> +grub_linux_get_loaded (void)
> +{
> +  return loaded;
> +}
> +
> +void
> +grub_linux_set_loaded (int loaded_flag)
> +{
> +  loaded = loaded_flag;
> +}
> +
Accessor functions are usually useless in GRUB. We have no public API to
respect. So it only adds clutter. Also "loaded" flag is static for а
good reason: it's specific to linux.c. I'm going to move fdt part to
fdt.c and have uniform interface for both linux and xen.
>  static void *
>  get_firmware_fdt (void)
>  {
> diff --git a/include/grub/arm64/linux.h b/include/grub/arm64/linux.h
> index 65796d9..20058f3 100644
> --- a/include/grub/arm64/linux.h
> +++ b/include/grub/arm64/linux.h
> @@ -43,10 +43,14 @@ struct grub_arm64_linux_kernel_header
>  };
>  
>  /* Declare the functions for getting dtb and checking/booting image */
> -void *grub_linux_get_fdt (void);
>  grub_err_t grub_arm64_uefi_check_image (struct grub_arm64_linux_kernel_header
>  *lh);
>  grub_err_t grub_arm64_uefi_boot_image (grub_addr_t addr, grub_size_t size,
> char *args);
>  
> +/* Export the accessor functions for gettin dtb and "loaded" flag */
> +void EXPORT_FUNC (*grub_linux_get_fdt) (void);
> +int EXPORT_FUNC (grub_linux_get_loaded) (void);
> +void EXPORT_FUNC (grub_linux_set_loaded) (int loaded_flag);
> +
EXPORT_* are necessary only for core. Not for modules.
>  #endif /* ! GRUB_LINUX_CPU_HEADER */
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 2/4] arm64: Add xen_boot module file

[Vladimir 'φ-coder/phcoder' Serbinenko]

Committed without the xen_module command. Its argument parsing was
non-trivial and I don't quite get what its intent is. Can you resubmit?
On 23.07.2015 07:16, fu@linaro.org wrote:
> From: Fu Wei 
> 
> grub-core/loader/arm64/xen_boot.c
> 
>   - This adds support for the Xen boot on ARM specification for arm64.
>   - Introduce xen_hypervisor, xen_linux, xen_initrd and xen_xsm
> to load different binaries for xen boot;
> Introduce xen_module to load common or custom module for xen boot.
>   - This Xen boot support is a separated  module for aarch64,
> but reuse the existing code of devicetree in linux module.
> 
> Signed-off-by: Fu Wei 
> ---
>  grub-core/Makefile.core.def   |   7 +
>  grub-core/loader/arm64/xen_boot.c | 685 
> ++
>  2 files changed, 692 insertions(+)
> 
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index a6101de..796f7e9 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1648,6 +1648,13 @@ module = {
>  };
>  
>  module = {
> +  name = xen_boot;
> +  common = lib/cmdline.c;
> +  arm64 = loader/arm64/xen_boot.c;
> +  enable = arm64;
> +};
> +
> +module = {
>name = linux;
>x86 = loader/i386/linux.c;
>xen = loader/i386/xen.c;
> diff --git a/grub-core/loader/arm64/xen_boot.c 
> b/grub-core/loader/arm64/xen_boot.c
> new file mode 100644
> index 000..33a65dd
> --- /dev/null
> +++ b/grub-core/loader/arm64/xen_boot.c
> @@ -0,0 +1,685 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2014  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include/* required by struct xen_hypervisor_header */
> +#include 
> +#include 
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +#define XEN_HYPERVISOR_NAME  "xen_hypervisor"
> +
> +#define MODULE_DEFAULT_ALIGN  (0x0)
> +#define MODULE_IMAGE_MIN_ALIGN  MODULE_DEFAULT_ALIGN
> +#define MODULE_INITRD_MIN_ALIGN  MODULE_DEFAULT_ALIGN
> +#define MODULE_XSM_MIN_ALIGN  MODULE_DEFAULT_ALIGN
> +#define MODULE_CUSTOM_MIN_ALIGN  MODULE_DEFAULT_ALIGN
> +
> +/* #define MODULE_IMAGE_COMPATIBLE  "xen,linux-image\0xen,module"
> +#define MODULE_INITRD_COMPATIBLE  "xen,linux-image\0xen,module"
> +#define MODULE_XSM_COMPATIBLE  "xen,xsm-policy\0xen,module"
> +#define MODULE_CUSTOM_COMPATIBLE  "xen,module" */
> +#define MODULE_IMAGE_COMPATIBLE  "multiboot,kernel\0multiboot,module"
> +#define MODULE_INITRD_COMPATIBLE  "multiboot,ramdisk\0multiboot,module"
> +#define MODULE_XSM_COMPATIBLE  "xen,xsm-policy\0multiboot,module"
> +#define MODULE_CUSTOM_COMPATIBLE  "multiboot,module"
> +
> +/* This maximum size is defined in Power.org ePAPR V1.1
> + * https://www.power.org/documentation/epapr-version-1-1/
> + * 2.2.1.1 Node Name Requirements
> + * node-name@unit-address
> + * 31 + 1(@) + 16(64bit address in hex format) + 1(\0) = 49
> + */
> +#define FDT_NODE_NAME_MAX_SIZE  (49)
> +
> +#define ARG_SHIFT(argc, argv) \
> +  do { \
> +(argc)--; \
> +(argv)++; \
> +  } while (0)
> +
> +struct compat_string_struct
> +{
> +  grub_size_t size;
> +  const char *compat_string;
> +};
> +typedef struct compat_string_struct compat_string_struct_t;
> +#define FDT_COMPATIBLE(x) {.size = sizeof(x), .compat_string = (x)}
> +
> +enum module_type
> +{
> +  MODULE_IMAGE,
> +  MODULE_INITRD,
> +  MODULE_XSM,
> +  MODULE_CUSTOM
> +};
> +typedef enum module_type module_type_t;
> +
> +struct fdt_node_info
> +{
> +  module_type_t type;
> +
> +  const char *compat_string;
> +  grub_size_t compat_string_size;
> +};
> +
> +struct xen_hypervisor_header
> +{
> +  struct grub_arm64_linux_kernel_header efi_head;
> +
> +  /* This is always PE\0\0.  */
> +  grub_uint8_t signature[GRUB_PE32_SIGNATURE_SIZE];
> +  /* The COFF file header.  */
> +  struct grub_pe32_coff_header coff_header;
> +  /* The Optional header.  */
> +  struct grub_pe64_optional_header optional_header;
> +};
> +
> +struct xen_boot_binary
> +{
> +  struct xen_boot_binary *next;
> +  struct xen_boot_binary **prev;
> +  const char *name;
> +
> +  grub_addr_t start;
> +  grub_size_t size;
> +  grub_size_t align;
> +
> +  char *cmdline;
> +  int cmdline_size;
> +
> 

Re: [edk2] [grub PATCH] efinet: disable MNP background polling

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 14.10.2015 17:39, Seth Goldberg wrote:
> 
> 
>> On Oct 14, 2015, at 4:08 AM, Daniel Kiper  wrote:
>>
>>> On Wed, Oct 14, 2015 at 05:19:32AM +, Ye, Ting wrote:
>>> Hi all,
>>>
>>> If I understand the issue correctly, I don't quite agree that UEFI
>>> spec is imprecise about SNP constraints described as following.
>>> The "constraint" described here is that the grub should use attribute
>>> "EXCLUSIVE" to open SNP protocol to gain exclusive access. This usage
>>> is clearly described in page 184, chapter 6.3 
>>> EFI_BOOT_SERVICES.OpenProtocol().
>>>
>>> EXCLUSIVEUsed by applications to gain exclusive access to a 
>>> protocol interface.
>>>If any drivers have the protocol interface opened with an 
>>> attribute of BY_DRIVER,
>>>then an attempt will be made to remove them by calling the 
>>> driver's Stop() function.
>>>
>>> The grub code should not assume that the SNP is not occupied by other
>>> drivers, instead, it should use EXCLUSIVE to open SNP protocol, or to
>>> be more precise, use OpenProtocolInformation() to check whether SNP is
>>> already opened by other driver, then decide whether need to use EXCLUSIVE
>>> to disconnect the other drivers. This is the typical usage for all UEFI
>>> protocol, not particular constraints to SNP protocol.
>>
>> Looks good! Great! However, it looks that we still have a problem if somebody
>> opens SNP in EXCLUSIVE mode. Then GRUB2 SNP open will fail according to UEFI 
>> spec.
>> Sadly we do not have a control on other stuff and one day our approach may 
>> fail
>> because somebody decided to open SNP in EXCLUSIVE mode in e.g. a driver. Does
>> it mean that migration to MNP is one sensible solution for our problems? As 
>> I know
>> this is huge overhaul, so, we should think twice before choosing that way.
> 
> 
>Then it is fortunate that when I wrote the MNP implementation that we ship 
> with Oracle Solaris 11.2, that I tested it on many thousands of systems as 
> well as on new UEFI implementations at the UEFI Plugfest ;).
> 
Can you please point to the patch you used?
I think the only sane solution judging from what I have read so far is
to use MNP as far as possible and fallback to current code if MNP fails
>   --S
> 
> 
> 
>>
>> Daniel
>>
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> .
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Be File System identified as bfs

[Vladimir 'φ-coder/phcoder' Serbinenko]

> As you can see, it thinks /dev/sda6 a 'bfs' partition, causing script
> 83haiku to fail.
> 
12  befs|befs_be) debug "$partition is a BeFS partition" ;;
Looks like this is a simple one-line change. befs|befs_be is a name used
by old driver which had to be removed




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 3/4] * util/grub.d/20_linux_xen.in: Add support of the XEN boot on aarch64

[Vladimir 'φ-coder/phcoder' Serbinenko]

> +if [ "x$machine" != xaarch64 ]; then
> + multiboot_cmd="multiboot"
> + module_linux_cmd="module"
> + module_initrd_cmd="module --nounzip"
> +else
> + multiboot_cmd="xen_hypervisor"
> + module_linux_cmd="xen_linux"
> + module_initrd_cmd="xen_initrd"
> +fi
> +
Please do not hardcode an assumption that grub-mkconfig is executed on
the same machine as GRUB is booted. I know that we have instances of
such assumption in some cases but we'd like to eliminate them. Alternatives:
- Check arch on boot time
- Check that new xen commands are supported (define a new feature)
Please add xen_* aliases on x86 as well




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 3/4] * util/grub.d/20_linux_xen.in: Add support of the XEN boot on aarch64

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 30.10.2015 09:44, Fu Wei wrote:
> Hi Vladimir,
> 
> Great thanks for your suggestion! :-)
> 
> On 29 October 2015 at 23:25, Vladimir 'φ-coder/phcoder' Serbinenko
>  wrote:
>>> +if [ "x$machine" != xaarch64 ]; then
>>> + multiboot_cmd="multiboot"
>>> + module_linux_cmd="module"
>>> + module_initrd_cmd="module --nounzip"
>>> +else
>>> + multiboot_cmd="xen_hypervisor"
>>> + module_linux_cmd="xen_linux"
>>> + module_initrd_cmd="xen_initrd"
>>> +fi
>>> +
>> Please do not hardcode an assumption that grub-mkconfig is executed on
>> the same machine as GRUB is booted. I know that we have instances of
>> such assumption in some cases but we'd like to eliminate them. Alternatives:
>> - Check arch on boot time
> 
> 
>> - Check that new xen commands are supported (define a new feature)
>> Please add xen_* aliases on x86 as well
> I would like to go this way, but could you provide some help or a
> little example for :
> (1) How to check the new xen commands(or xen_boot module)
> (2)add xen_* aliases on x86, is that like something below?
> 
see grub-core/normal/main.c the features array.
> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> index c4d9689..b88d51b 100644
> --- a/grub-core/loader/i386/xen.c
> +++ b/grub-core/loader/i386/xen.c
> @@ -696,10 +696,14 @@ GRUB_MOD_INIT (xen)
>0, N_("Load Linux."));
>cmd_multiboot = grub_register_command ("multiboot", grub_cmd_xen,
>  0, N_("Load Linux."));
> +  cmd_multiboot = grub_register_command ("xen_hypervisor", grub_cmd_xen,
> +0, N_("Load Linux."));
>cmd_initrd = grub_register_command ("initrd", grub_cmd_initrd,
>   0, N_("Load initrd."));
>cmd_module = grub_register_command ("module", grub_cmd_module,
>   0, N_("Load module."));
> +  cmd_module = grub_register_command ("xen_linux", grub_cmd_module,
> + 0, N_("Load module."));
>my_mod = mod;
>  }
> 
> But how to deal with xen_initrd ?
> Could you help me ?
> 
Just another alias to module. Possibly you might want to add a code to
xen_initrd tto check that xen_linux was already run
> Great thanks !!
> 
>>
>>
> 
> 
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Anonymous commit (empty Author and Committer)

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 30.10.2015 15:26, Andrei Borzenkov wrote:
> See
> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
> 
> 
> I was not even aware that this is possible. Is there anything on server
> side that can prevent it?
> 
> Would be good if commit were amended and force pushed to fix it.
> 
It is a bug in SGit. I'll investigate how it happened
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> .
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Anonymous commit (empty Author and Committer)

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 30.10.2015 21:06, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 30.10.2015 15:26, Andrei Borzenkov wrote:
>> See
>> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
>>
>>
>> I was not even aware that this is possible. Is there anything on server
>> side that can prevent it?
>>
>> Would be good if commit were amended and force pushed to fix it.
>>
> It is a bug in SGit. I'll investigate how it happened
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>> .
>>
> 
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] mkimage: zero fill alignment space

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 30.10.2015 16:49, Andrei Borzenkov wrote:
> This did not cause real problem but is good for reproducible builds. I hit
> it with recent bootinfoscript that displays embedded config; I was puzzled
> by random garbage at the end.
> 
> Also remove redundant zeroing code where we fill in the whole memory block
> anyway.
> 
Could we just zero-out the whole block when we allocate it to have an
easier code and avoid it happening again?
> ---
>  util/mkimage.c | 29 +
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/util/mkimage.c b/util/mkimage.c
> index 35df998..1c522fe 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -992,7 +992,7 @@ grub_install_generate_image (const char *dir, const char 
> *prefix,
>  {
>char *kernel_img, *core_img;
>size_t kernel_size, total_module_size, core_size, exec_size;
> -  size_t memdisk_size = 0, config_size = 0, config_size_pure = 0;
> +  size_t memdisk_size = 0, memdisk_size_pure = 0, config_size = 0, 
> config_size_pure = 0;
>size_t prefix_size = 0;
>char *kernel_path;
>size_t offset;
> @@ -1035,7 +1035,8 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>  
>if (memdisk_path)
>  {
> -  memdisk_size = ALIGN_UP(grub_util_get_image_size (memdisk_path), 512);
> +  memdisk_size_pure = grub_util_get_image_size (memdisk_path);
> +  memdisk_size = ALIGN_UP(memdisk_size_pure, 512);
>grub_util_info ("the size of memory disk is 0x%" 
> GRUB_HOST_PRIxLONG_LONG,
> (unsigned long long) memdisk_size);
>total_module_size += memdisk_size + sizeof (struct grub_module_header);
> @@ -1043,8 +1044,8 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>  
>if (config_path)
>  {
> -  config_size_pure = grub_util_get_image_size (config_path) + 1;
> -  config_size = ALIGN_ADDR (config_size_pure);
> +  config_size_pure = grub_util_get_image_size (config_path);
> +  config_size = ALIGN_ADDR (config_size_pure + 1);
>grub_util_info ("the size of config file is 0x%" 
> GRUB_HOST_PRIxLONG_LONG,
> (unsigned long long) config_size);
>total_module_size += config_size + sizeof (struct grub_module_header);
> @@ -1140,19 +1141,21 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>  size_t i;
>  for (i = 0; i < npubkeys; i++)
>{
> - size_t curs;
>   struct grub_module_header *header;
> + size_t key_size, orig_size;
>  
> - curs = grub_util_get_image_size (pubkey_paths[i]);
> + orig_size = grub_util_get_image_size (pubkey_paths[i]);
> + key_size = ALIGN_ADDR (orig_size);
>  
>   header = (struct grub_module_header *) (kernel_img + offset);
>   memset (header, 0, sizeof (struct grub_module_header));
>   header->type = grub_host_to_target32 (OBJ_TYPE_PUBKEY);
> - header->size = grub_host_to_target32 (curs + sizeof (*header));
> + header->size = grub_host_to_target32 (key_size + sizeof (*header));
>   offset += sizeof (*header);
>  
>   grub_util_load_image (pubkey_paths[i], kernel_img + offset);
> - offset += ALIGN_ADDR (curs);
> + memset (kernel_img + offset + orig_size, 0, key_size - orig_size);
> + offset += key_size;
>}
>}
>  
> @@ -1167,6 +1170,7 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>offset += sizeof (*header);
>  
>grub_util_load_image (memdisk_path, kernel_img + offset);
> +  memset (kernel_img + offset + memdisk_size_pure, 0, memdisk_size - 
> memdisk_size_pure);
>offset += memdisk_size;
>  }
>  
> @@ -1181,7 +1185,7 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>offset += sizeof (*header);
>  
>grub_util_load_image (config_path, kernel_img + offset);
> -  *(kernel_img + offset + config_size_pure - 1) = 0;
> +  memset (kernel_img + offset + config_size_pure, 0, config_size - 
> config_size_pure);
>offset += config_size;
>  }
>  
> @@ -1267,17 +1271,10 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
> = grub_host_to_target_addr (image_target->link_addr);
>   }
>full_size = core_size + decompress_size;
> -
>full_img = xmalloc (full_size);
> -  memset (full_img, 0, full_size); 
> -
>memcpy (full_img, decompress_img, decompress_size);
> -
>memcpy (full_img + decompress_size, core_img, core_size);
>  
> -  memset (full_img + decompress_size + core_size, 0,
> -   full_size - (decompress_size + core_size));
> -
>free (core_img);
>core_img = full_img;
>core_size = full_size;
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Anonymous commit (empty Author and Committer)

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 30.10.2015 21:09, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 30.10.2015 21:06, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> On 30.10.2015 15:26, Andrei Borzenkov wrote:
>>> See
>>> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
>>>
>>>
>>> I was not even aware that this is possible. Is there anything on server
>>> side that can prevent it?
>>>
>>> Would be good if commit were amended and force pushed to fix it.
>>>
>> It is a bug in SGit. I'll investigate how it happened

I don't have non-fast-forward rights. Does someone from savannah-users
have them? Could he just delete this commit?




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] ofdisk: allocate space for vscsi table

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 09.11.2015 02:33, Paulo Flabiano Smorigo wrote:
> +  /* 64 entries should be enough */
> +  table_size = sizeof (grub_uint64_t) * 64;
Can we do something better than assuming a particular upper limit? You
don't even pass this limit to the function in any way. You basically get
a buffer overflow. Should we perhaps pass the limit in nentries? Or do
something similar?



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Use UEFI Time Service to calibrate TSC

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 09.11.2015 09:03, Michael Chang wrote:
> On Mon, Nov 09, 2015 at 10:29:55AM +0300, Andrei Borzenkov wrote:
>> On Mon, Nov 9, 2015 at 10:07 AM, Michael Chang  wrote:
>>> This patch tries to detect PIT timer is broken and use UEFI Time Service
>>> to calibrate TSC.
>>
>> Second try :)
>>
>> https://lists.gnu.org/archive/html/grub-devel/2014-11/msg00079.html
>>
>> Although I think that this one is acceptable - it is used as fallback
>> only. Will it also catch the case when PIT is not present at all?
> 
> I think yes, actually this patch was tested and can fix the timeout
> problem on hyper-v Generation 2 VMs with UEFI, which is known to ship
> without PIT.
> 
> In addition to that, the linux kernel also calibrates the tsc via PIT as
> default on x86.
> 
> http://lxr.free-electrons.com/source/arch/x86/kernel/tsc.c#L441
> 
> But with some sanity checks to detect the SMI storm interfering the
> result and will fallback to other timer sources if the sanity check
> fails. This patch is inspired by that one of that check with much more
> restricted loopmin to "1" that's basically a insane condition. 
> 
I like some aspects of this patch, i.a. that it's unlikely to break
compatibility. ut I feel that we can do a bit better:
1) Returning through pointed variable is expensive in terms of binary
size. Just plain return is better.
2) More modern calibrations can calibrate in 1ms, not 55ms. This is one
that slows down coreboot boot (I think only USB init is slower).
3) We could have a cascade of methods. E.g.
on EFI: PIT -> PM -> Stall -> hardcoded 1GHz (putting PM as first would
be slightly more risk
on coreboot: PM -> PIT -> hardcoded 1GHz
rest: PIT -> hardcoded 1GHz (need to keep size down)

I'm going to prepare proof-of-concept patch
> Thanks,
> Michael
> 
>>
>>> ---
>>>  grub-core/kern/i386/tsc.c |   33 +
>>>  1 files changed, 29 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/grub-core/kern/i386/tsc.c b/grub-core/kern/i386/tsc.c
>>> index bc441d0..bd24cea 100644
>>> --- a/grub-core/kern/i386/tsc.c
>>> +++ b/grub-core/kern/i386/tsc.c
>>> @@ -29,6 +29,10 @@
>>>  #include 
>>>  #else
>>>  #include 
>>> +#ifdef GRUB_MACHINE_EFI
>>> +#include 
>>> +#include 
>>> +#endif
>>>  #endif
>>>  #include 
>>>
>>> @@ -72,7 +76,7 @@ grub_cpu_is_tsc_supported (void)
>>>  }
>>>
>>>  static void
>>> -grub_pit_wait (grub_uint16_t tics)
>>> +grub_pit_wait (grub_uint16_t tics, int *is_started)
>>>  {
>>>/* Disable timer2 gate and speaker.  */
>>>grub_outb (grub_inb (GRUB_PIT_SPEAKER_PORT)
>>> @@ -90,8 +94,17 @@ grub_pit_wait (grub_uint16_t tics)
>>>  | GRUB_PIT_SPK_TMR2,
>>>   GRUB_PIT_SPEAKER_PORT);
>>>
>>> -  /* Wait.  */
>>> -  while ((grub_inb (GRUB_PIT_SPEAKER_PORT) & GRUB_PIT_SPK_TMR2_LATCH) == 
>>> 0x00);
>>> +  if ((grub_inb (GRUB_PIT_SPEAKER_PORT) & GRUB_PIT_SPK_TMR2_LATCH))
>>> +{
>>> +  /* The ticks have expired too fast to know the counting really 
>>> started or not */
>>> +  *is_started = 0;
>>> +}
>>> +  else
>>> +{
>>> +  *is_started = 1;
>>> +  /* Wait.  */
>>> +  while ((grub_inb (GRUB_PIT_SPEAKER_PORT) & GRUB_PIT_SPK_TMR2_LATCH) 
>>> == 0x00);
>>> +}
>>>
>>>/* Disable timer2 gate and speaker.  */
>>>grub_outb (grub_inb (GRUB_PIT_SPEAKER_PORT)
>>> @@ -117,11 +130,23 @@ calibrate_tsc (void)
>>>  {
>>>/* First calibrate the TSC rate (relative, not absolute time). */
>>>grub_uint64_t end_tsc;
>>> +  int is_started;
>>>
>>>tsc_boot_time = grub_get_tsc ();
>>> -  grub_pit_wait (0x);
>>> +  grub_pit_wait (0x, &is_started);
>>>end_tsc = grub_get_tsc ();
>>>
>>> +#ifdef GRUB_MACHINE_EFI
>>> +  /* The PIT is broken as 55ms is too sufficent to any cpu to catch it */
>>> +  if (!is_started)
>>> +{
>>> +  /* Use EFI Time Service to calibrate TSC */
>>> +  tsc_boot_time = grub_get_tsc ();
>>> +  efi_call_1 (grub_efi_system_table->boot_services->stall, 54925);
>>> +  end_tsc = grub_get_tsc ();
>>> +}
>>> +#endif
>>> +
>>>grub_tsc_rate = 0;
>>>if (end_tsc > tsc_boot_time)
>>>  grub_tsc_rate = grub_divmod64 ((55ULL << 32), end_tsc - tsc_boot_time, 
>>> 0);
>>> --
>>> 1.7.3.4
>>>
>>>
>>> ___
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Use UEFI Time Service to calibrate TSC

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 09.11.2015 14:23, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 09.11.2015 09:03, Michael Chang wrote:
>> On Mon, Nov 09, 2015 at 10:29:55AM +0300, Andrei Borzenkov wrote:
>>> On Mon, Nov 9, 2015 at 10:07 AM, Michael Chang  wrote:
>>>> This patch tries to detect PIT timer is broken and use UEFI Time Service
>>>> to calibrate TSC.
>>>
>>> Second try :)
>>>
>>> https://lists.gnu.org/archive/html/grub-devel/2014-11/msg00079.html
>>>
>>> Although I think that this one is acceptable - it is used as fallback
>>> only. Will it also catch the case when PIT is not present at all?
>>
>> I think yes, actually this patch was tested and can fix the timeout
>> problem on hyper-v Generation 2 VMs with UEFI, which is known to ship
>> without PIT.
>>
>> In addition to that, the linux kernel also calibrates the tsc via PIT as
>> default on x86.
>>
>> http://lxr.free-electrons.com/source/arch/x86/kernel/tsc.c#L441
>>
>> But with some sanity checks to detect the SMI storm interfering the
>> result and will fallback to other timer sources if the sanity check
>> fails. This patch is inspired by that one of that check with much more
>> restricted loopmin to "1" that's basically a insane condition. 
>>
> I like some aspects of this patch, i.a. that it's unlikely to break
> compatibility. ut I feel that we can do a bit better:
> 1) Returning through pointed variable is expensive in terms of binary
> size. Just plain return is better.
> 2) More modern calibrations can calibrate in 1ms, not 55ms. This is one
> that slows down coreboot boot (I think only USB init is slower).
> 3) We could have a cascade of methods. E.g.
> on EFI: PIT -> PM -> Stall -> hardcoded 1GHz (putting PM as first would
> be slightly more risk
> on coreboot: PM -> PIT -> hardcoded 1GHz
> rest: PIT -> hardcoded 1GHz (need to keep size down)
> 
> I'm going to prepare proof-of-concept patch
Attached proof-of-concept. It's all in one file but different methods
should probably go in separate files.

diff --git a/grub-core/commands/efi/acpi.c b/grub-core/commands/efi/acpi.c
deleted file mode 100644
index 74f8cd1..000
--- a/grub-core/commands/efi/acpi.c
+++ /dev/null
@@ -1,59 +0,0 @@
-/* acpi.c - get acpi tables. */
-/*
- *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2009  Free Software Foundation, Inc.
- *
- *  GRUB is free software: you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation, either version 3 of the License, or
- *  (at your option) any later version.
- *
- *  GRUB is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include 
-#include 
-#include 
-#include 
-
-struct grub_acpi_rsdp_v10 *
-grub_machine_acpi_get_rsdpv1 (void)
-{
-  unsigned i;
-  static grub_efi_packed_guid_t acpi_guid = GRUB_EFI_ACPI_TABLE_GUID;
-
-  for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
-{
-  grub_efi_packed_guid_t *guid =
-	&grub_efi_system_table->configuration_table[i].vendor_guid;
-
-  if (! grub_memcmp (guid, &acpi_guid, sizeof (grub_efi_packed_guid_t)))
-	return (struct grub_acpi_rsdp_v10 *)
-	  grub_efi_system_table->configuration_table[i].vendor_table;
-}
-  return 0;
-}
-
-struct grub_acpi_rsdp_v20 *
-grub_machine_acpi_get_rsdpv2 (void)
-{
-  unsigned i;
-  static grub_efi_packed_guid_t acpi20_guid = GRUB_EFI_ACPI_20_TABLE_GUID;
-
-  for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
-{
-  grub_efi_packed_guid_t *guid =
-	&grub_efi_system_table->configuration_table[i].vendor_guid;
-
-  if (! grub_memcmp (guid, &acpi20_guid, sizeof (grub_efi_packed_guid_t)))
-	return (struct grub_acpi_rsdp_v20 *)
-	  grub_efi_system_table->configuration_table[i].vendor_table;
-}
-  return 0;
-}
diff --git a/grub-core/kern/acpi.c b/grub-core/kern/acpi.c
new file mode 100644
index 000..5292597
--- /dev/null
+++ b/grub-core/kern/acpi.c
@@ -0,0 +1,119 @@
+/* 
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2012  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it w

Re: [PATCH v2 1/6] gitignore: Ignore *.orig, *.rej and *.swp files

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 09.11.2015 16:29, Daniel Kiper wrote:
> On Wed, Nov 04, 2015 at 01:03:56PM +0100, Vladimir 'phcoder' Serbinenko wrote:
>> Le 12 août 2015 11:04 AM, "Ian Campbell"  a écrit :
>>>
>>>
>>> (Having written the below I see too late that this is a grub patch not a
>>> Xen one, a tag in the subject for such cross posted patches would be
>> useful
>>> please. Anyway, my opinion counts for very little in this context but I
>>> leave it below since already I wrote it. I notice that xen.git#.gitignore
>>> _does_ list *.rej, which I think is wrong...)
>>>
>>> On Mon, 2015-07-20 at 16:35 +0200, Daniel Kiper wrote:
 Signed-off-by: Daniel Kiper 
>>>
>>> At least *.rej and perhaps *.orig are indicative of a failed patch
>>> application, I think I want them to appear in "git status".
>>>
>>> By way of comparison Linux's .gitignore includes *.orig but not *.rej and
>>> Qemu's includes neither.
>>>
>>> So nack to the addition of *.rej from me. I'm more or less ambivalent
>> about
>>> *.orig.
>>>
>> I have to agree. You should clean up *.rej *.orig after fixing conflicts
> 
> Thanks for comment on this. Could you review rest of this patchset?
> I am working on v3 and it will be nice to take your (and others if
> possible) comments into it.
> 
I will go through them today
> Daniel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 2/6] relocator: Do not use memory region if its starta is smaller than size

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 21.07.2015 08:42, Andrei Borzenkov wrote:
> On Mon, Jul 20, 2015 at 5:35 PM, Daniel Kiper  wrote:
>> malloc_in_range() should not use memory region if its starta is smaller
>> than size. Otherwise target wraps around and points to region which is
>> usually not a RAM, e.g.:
>>
>> loader/multiboot.c:93: segment 0: paddr=0x80, memsz=0x3f80, 
>> vaddr=0x80
>> lib/relocator.c:1241: min_addr = 0x0, max_addr = 0x, target 
>> = 0x80
>> lib/relocator.c:434: trying to allocate in 0x80-0x 
>> aligned 0x1 size 0x3f80
>> lib/relocator.c:434: trying to allocate in 0x0-0x80 aligned 0x1 size 
>> 0x3f80
>> lib/relocator.c:434: trying to allocate in 0x0-0x aligned 
>> 0x1 size 0x3f80
>> lib/relocator.c:1188: allocated: 0xc07f+0x3f80
>> lib/relocator.c:1277: allocated 0xc07f/0x80
>>
>> Signed-off-by: Daniel Kiper 
>> ---
>>  grub-core/lib/relocator.c |2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/grub-core/lib/relocator.c b/grub-core/lib/relocator.c
>> index f759c7f..4eee0c5 100644
>> --- a/grub-core/lib/relocator.c
>> +++ b/grub-core/lib/relocator.c
>> @@ -748,7 +748,7 @@ malloc_in_range (struct grub_relocator *rel,
>>   /* Found an usable address.  */
>>   goto found;
>>   }
>> -   if (isinsidebefore && !isinsideafter && !from_low_priv)
>> +   if (isinsidebefore && !isinsideafter && !from_low_priv && starta >= 
>> size)
> 
> That's too late, we need to check end of region on previous iteration.
> Consider region of 128 bytes, requested size 129 and alignment 256.
> Than starta still ends up high in memory.
> 
Agreed, we need a check earlier. It makes sense to split this block with
an if (from_low_priv) as both flows are completely separate and
splitting them will make it more readable
>>   {
>> target = starta - size;
>> if (target > end - size)
>> --
>> 1.7.10.4
>>
>>
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Fwd: Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to image if EFI boot services are enabled

[Vladimir 'φ-coder/phcoder' Serbinenko]

 Forwarded Message 
Subject:Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to
image if EFI boot services are enabled
Date:   Mon, 9 Nov 2015 20:07:23 +0100
From:   Vladimir 'phcoder' Serbinenko 
To: Daniel Kiper 



This one is good to go. But the main reason for not providing the map is
because it will likely be invalid. We do few allocations after filling
the map I.a. git relocator. Usually we don't care as we would already
finish boot services but if we keep them, it's easier not to provide
them, at least until somebody needs then. Please adjust the description
and add or to the comment

Le 20 juil. 2015 4:37 PM, "Daniel Kiper" mailto:daniel.ki...@oracle.com>> a écrit :

Do not pass memory maps to image if it asked for EFI boot services.
Maps are
usually invalid in that case and they can confuse potential user.
Image should
get memory map itself just before ExitBootServices() call.

Signed-off-by: Daniel Kiper mailto:daniel.ki...@oracle.com>>
---
 grub-core/loader/multiboot_mbi2.c |   71
++---
 1 file changed, 35 insertions(+), 36 deletions(-)

diff --git a/grub-core/loader/multiboot_mbi2.c
b/grub-core/loader/multiboot_mbi2.c
index 7ac64ec..26e955c 100644
--- a/grub-core/loader/multiboot_mbi2.c
+++ b/grub-core/loader/multiboot_mbi2.c
@@ -431,7 +431,7 @@ static grub_size_t
 grub_multiboot_get_mbi_size (void)
 {
 #ifdef GRUB_MACHINE_EFI
-  if (!efi_mmap_size)
+  if (!keep_bs && !efi_mmap_size)
 find_efi_mmap_size ();
 #endif
   return 2 * sizeof (grub_uint32_t) + sizeof (struct multiboot_tag)
@@ -805,12 +805,13 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
   }
   }

-  {
-struct multiboot_tag_mmap *tag = (struct multiboot_tag_mmap *)
ptrorig;
-grub_fill_multiboot_mmap (tag);
-ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
-  / sizeof (grub_properly_aligned_t);
-  }
+  if (!keep_bs)
+{
+  struct multiboot_tag_mmap *tag = (struct multiboot_tag_mmap
*) ptrorig;
+  grub_fill_multiboot_mmap (tag);
+  ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
+   / sizeof (grub_properly_aligned_t);
+}

   {
 struct multiboot_tag_elf_sections *tag
@@ -826,18 +827,19 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
   / sizeof (grub_properly_aligned_t);
   }

-  {
-struct multiboot_tag_basic_meminfo *tag
-  = (struct multiboot_tag_basic_meminfo *) ptrorig;
-tag->type = MULTIBOOT_TAG_TYPE_BASIC_MEMINFO;
-tag->size = sizeof (struct multiboot_tag_basic_meminfo);
+  if (!keep_bs)
+{
+  struct multiboot_tag_basic_meminfo *tag
+   = (struct multiboot_tag_basic_meminfo *) ptrorig;
+  tag->type = MULTIBOOT_TAG_TYPE_BASIC_MEMINFO;
+  tag->size = sizeof (struct multiboot_tag_basic_meminfo);

-/* Convert from bytes to kilobytes.  */
-tag->mem_lower = grub_mmap_get_lower () / 1024;
-tag->mem_upper = grub_mmap_get_upper () / 1024;
-ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
-   / sizeof (grub_properly_aligned_t);
-  }
+  /* Convert from bytes to kilobytes.  */
+  tag->mem_lower = grub_mmap_get_lower () / 1024;
+  tag->mem_upper = grub_mmap_get_upper () / 1024;
+  ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
+   / sizeof (grub_properly_aligned_t);
+}

   {
 struct grub_net_network_level_interface *net;
@@ -936,27 +938,24 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
 grub_efi_uintn_t efi_desc_size;
 grub_efi_uint32_t efi_desc_version;

-tag->type = MULTIBOOT_TAG_TYPE_EFI_MMAP;
-tag->size = sizeof (*tag) + efi_mmap_size;
-
 if (!keep_bs)
-  err = grub_efi_finish_boot_services (&efi_mmap_size,
tag->efi_mmap, NULL,
-  &efi_desc_size,
&efi_desc_version);
-else
   {
-   if (grub_efi_get_memory_map (&efi_mmap_size, (void *)
tag->efi_mmap,
-NULL,
-&efi_desc_size,
&efi_desc_version) <= 0)
- err = grub_error (GRUB_ERR_IO, "couldn't retrieve memory
map");
+   tag->type = MULTIBOOT_TAG_TYPE_EFI_MMAP;
+   tag->size = sizeof (*tag) + efi_mmap_size;
+
+   err = grub_efi_finish_boot_services (&efi_mmap_size,
tag->efi_mmap, NULL,
+&efi_desc_size,
&efi_desc_version);
+
+   if (err)
+ return err;
+
+   tag->descr_size = efi_desc_size;
+   tag->descr_vers = efi_desc_version;
+   tag->size = sizeof (*tag) + efi_mmap_size;
+
+   ptrorig += ALIGN_UP (t

Fwd: Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to image if EFI boot services are enabled

[Vladimir 'φ-coder/phcoder' Serbinenko]

 Forwarded Message 
Subject:Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to
image if EFI boot services are enabled
Date:   Mon, 9 Nov 2015 20:08:38 +0100
From:   Vladimir 'phcoder' Serbinenko 
To: Daniel Kiper 



And you also need to adjust loader code to reject images which request
memory tags and bs at the same time

Le 9 nov. 2015 8:07 PM, "Vladimir 'phcoder' Serbinenko"
mailto:phco...@gmail.com>> a écrit :

This one is good to go. But the main reason for not providing the
map is because it will likely be invalid. We do few allocations
after filling the map I.a. git relocator. Usually we don't care as
we would already finish boot services but if we keep them, it's
easier not to provide them, at least until somebody needs then.
Please adjust the description and add or to the comment

Le 20 juil. 2015 4:37 PM, "Daniel Kiper" mailto:daniel.ki...@oracle.com>> a écrit :

Do not pass memory maps to image if it asked for EFI boot
services. Maps are
usually invalid in that case and they can confuse potential
user. Image should
get memory map itself just before ExitBootServices() call.

Signed-off-by: Daniel Kiper mailto:daniel.ki...@oracle.com>>
---
 grub-core/loader/multiboot_mbi2.c |   71
++---
 1 file changed, 35 insertions(+), 36 deletions(-)

diff --git a/grub-core/loader/multiboot_mbi2.c
b/grub-core/loader/multiboot_mbi2.c
index 7ac64ec..26e955c 100644
--- a/grub-core/loader/multiboot_mbi2.c
+++ b/grub-core/loader/multiboot_mbi2.c
@@ -431,7 +431,7 @@ static grub_size_t
 grub_multiboot_get_mbi_size (void)
 {
 #ifdef GRUB_MACHINE_EFI
-  if (!efi_mmap_size)
+  if (!keep_bs && !efi_mmap_size)
 find_efi_mmap_size ();
 #endif
   return 2 * sizeof (grub_uint32_t) + sizeof (struct multiboot_tag)
@@ -805,12 +805,13 @@ grub_multiboot_make_mbi (grub_uint32_t
*target)
   }
   }

-  {
-struct multiboot_tag_mmap *tag = (struct multiboot_tag_mmap
*) ptrorig;
-grub_fill_multiboot_mmap (tag);
-ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
-  / sizeof (grub_properly_aligned_t);
-  }
+  if (!keep_bs)
+{
+  struct multiboot_tag_mmap *tag = (struct
multiboot_tag_mmap *) ptrorig;
+  grub_fill_multiboot_mmap (tag);
+  ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
+   / sizeof (grub_properly_aligned_t);
+}

   {
 struct multiboot_tag_elf_sections *tag
@@ -826,18 +827,19 @@ grub_multiboot_make_mbi (grub_uint32_t
*target)
   / sizeof (grub_properly_aligned_t);
   }

-  {
-struct multiboot_tag_basic_meminfo *tag
-  = (struct multiboot_tag_basic_meminfo *) ptrorig;
-tag->type = MULTIBOOT_TAG_TYPE_BASIC_MEMINFO;
-tag->size = sizeof (struct multiboot_tag_basic_meminfo);
+  if (!keep_bs)
+{
+  struct multiboot_tag_basic_meminfo *tag
+   = (struct multiboot_tag_basic_meminfo *) ptrorig;
+  tag->type = MULTIBOOT_TAG_TYPE_BASIC_MEMINFO;
+  tag->size = sizeof (struct multiboot_tag_basic_meminfo);

-/* Convert from bytes to kilobytes.  */
-tag->mem_lower = grub_mmap_get_lower () / 1024;
-tag->mem_upper = grub_mmap_get_upper () / 1024;
-ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
-   / sizeof (grub_properly_aligned_t);
-  }
+  /* Convert from bytes to kilobytes.  */
+  tag->mem_lower = grub_mmap_get_lower () / 1024;
+  tag->mem_upper = grub_mmap_get_upper () / 1024;
+  ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
+   / sizeof (grub_properly_aligned_t);
+}

   {
 struct grub_net_network_level_interface *net;
@@ -936,27 +938,24 @@ grub_multiboot_make_mbi (grub_uint32_t
*target)
 grub_efi_uintn_t efi_desc_size;
 grub_efi_uint32_t efi_desc_version;

-tag->type = MULTIBOOT_TAG_TYPE_EFI_MMAP;
-tag->size = sizeof (*tag) + efi_mmap_size;
-
 if (!keep_bs)
-  err = grub_efi_finish_boot_services (&efi_mmap_size,
tag->efi_mmap, NULL,
-  &efi_desc_size,
&efi_desc_version);
-else
   {
-   if (grub_efi_get_memory_map (&efi_mmap_size, (void *)
tag->efi_mmap,
-NULL,
-&efi_desc_size,
&efi_desc_version) <= 0)

Re: [PATCH v2 3/6] i386/relocator: Add grub_relocator64_efi relocator

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 20.07.2015 16:35, Daniel Kiper wrote:
> Add grub_relocator64_efi relocator. It will be used on EFI 64-bit platforms
> when multiboot2 compatible image requests MULTIBOOT_TAG_TYPE_EFI_BS. Relocator
> will set lower parts of %rax and %rbx accordingly to multiboot2 specification.
> On the other hand processor mode, just before jumping into loaded image, will
> be set accordingly to Unified Extensible Firmware Interface Specification,
> Version 2.4 Errata B, section 2.3.4, x64 Platforms, boot services. This way
> loaded image will be able to use EFI boot services without any issues.
> 
> If idea is accepted I will prepare grub_relocator32_efi relocator too.
> 
> Signed-off-by: Daniel Kiper 
> ---
>  grub-core/Makefile.core.def  |1 +
>  grub-core/lib/i386/relocator.c   |   53 +++
>  grub-core/lib/i386/relocator64_efi.S |   77 
> ++
>  grub-core/loader/multiboot.c |   29 +++--
>  grub-core/loader/multiboot_mbi2.c|   19 +++--
>  include/grub/i386/multiboot.h|   11 +
>  include/grub/i386/relocator.h|   21 ++
>  include/multiboot2.h |9 
>  8 files changed, 213 insertions(+), 7 deletions(-)
>  create mode 100644 grub-core/lib/i386/relocator64_efi.S
> 
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index a6101de..d583549 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1519,6 +1519,7 @@ module = {
>x86 = lib/i386/relocator_common_c.c;
>ieee1275 = lib/ieee1275/relocator.c;
>efi = lib/efi/relocator.c;
> +  x86_64_efi = lib/i386/relocator64_efi.S;
>mips = lib/mips/relocator_asm.S;
>mips = lib/mips/relocator.c;
>powerpc = lib/powerpc/relocator_asm.S;
> diff --git a/grub-core/lib/i386/relocator.c b/grub-core/lib/i386/relocator.c
> index 71dd4f0..459027e 100644
> --- a/grub-core/lib/i386/relocator.c
> +++ b/grub-core/lib/i386/relocator.c
> @@ -69,6 +69,19 @@ extern grub_uint64_t grub_relocator64_rsi;
>  extern grub_addr_t grub_relocator64_cr3;
>  extern struct grub_i386_idt grub_relocator16_idt;
>  
> +#ifdef GRUB_MACHINE_EFI
> +#ifdef __x86_64__
> +extern grub_uint8_t grub_relocator64_efi_start;
> +extern grub_uint8_t grub_relocator64_efi_end;
> +extern grub_uint64_t grub_relocator64_efi_rax;
> +extern grub_uint64_t grub_relocator64_efi_rbx;
> +extern grub_uint64_t grub_relocator64_efi_rcx;
> +extern grub_uint64_t grub_relocator64_efi_rdx;
> +extern grub_uint64_t grub_relocator64_efi_rip;
> +extern grub_uint64_t grub_relocator64_efi_rsi;
> +#endif
> +#endif
> +
>  #define RELOCATOR_SIZEOF(x)  (&grub_relocator##x##_end - 
> &grub_relocator##x##_start)
>  
>  grub_err_t
> @@ -214,3 +227,43 @@ grub_relocator64_boot (struct grub_relocator *rel,
>/* Not reached.  */
>return GRUB_ERR_NONE;
>  }
> +
> +#ifdef GRUB_MACHINE_EFI
> +#ifdef __x86_64__
> +grub_err_t
> +grub_relocator64_efi_boot (struct grub_relocator *rel,
> +struct grub_relocator64_efi_state state)
> +{
> +  grub_err_t err;
> +  void *relst;
> +  grub_relocator_chunk_t ch;
> +
> +  err = grub_relocator_alloc_chunk_align (rel, &ch, 0,
> +   0x4000 - RELOCATOR_SIZEOF 
> (64_efi),
> +   RELOCATOR_SIZEOF (64_efi), 16,
> +   GRUB_RELOCATOR_PREFERENCE_NONE, 1);
> +  if (err)
> +return err;
> +
> +  grub_relocator64_efi_rax = state.rax;
> +  grub_relocator64_efi_rbx = state.rbx;
> +  grub_relocator64_efi_rcx = state.rcx;
> +  grub_relocator64_efi_rdx = state.rdx;
> +  grub_relocator64_efi_rip = state.rip;
> +  grub_relocator64_efi_rsi = state.rsi;
> +
> +  grub_memmove (get_virtual_current_address (ch), 
> &grub_relocator64_efi_start,
> + RELOCATOR_SIZEOF (64_efi));
> +
> +  err = grub_relocator_prepare_relocs (rel, get_physical_target_address (ch),
> +&relst, NULL);
> +  if (err)
> +return err;
> +
> +  ((void (*) (void)) relst) ();
> +
> +  /* Not reached.  */
> +  return GRUB_ERR_NONE;
> +}
> +#endif
> +#endif
> diff --git a/grub-core/lib/i386/relocator64_efi.S 
> b/grub-core/lib/i386/relocator64_efi.S
> new file mode 100644
> index 000..fcd1964
> --- /dev/null
> +++ b/grub-core/lib/i386/relocator64_efi.S
> @@ -0,0 +1,77 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2009,2010  Free Software Foundation, Inc.
> + *  Copyright (C) 2014,2015  Oracle Co.
> + *  Author: Daniel Kiper
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR

Re: [PATCH v2 4/6] multiboot2: Add tags used to pass ImageHandle to loaded image

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 09.11.2015 20:15, Vladimir 'phcoder' Serbinenko wrote:
> Lgtm
> 
> Le 20 juil. 2015 4:36 PM, "Daniel Kiper"  > a écrit :
> 
> Add tags used to pass ImageHandle to loaded image. It is used
> by at least ExitBootServices() function.
> 
> Signed-off-by: Daniel Kiper  >
> ---
>  grub-core/loader/multiboot_mbi2.c |   46
> +
>  include/multiboot2.h  |   16 +
>  2 files changed, 53 insertions(+), 9 deletions(-)
> 
> diff --git a/grub-core/loader/multiboot_mbi2.c
> b/grub-core/loader/multiboot_mbi2.c
> index 8d66e3f..dc9c709 100644
> --- a/grub-core/loader/multiboot_mbi2.c
> +++ b/grub-core/loader/multiboot_mbi2.c
> @@ -172,6 +172,8 @@ grub_multiboot_load (grub_file_t file, const
> char *filename)
>   case MULTIBOOT_TAG_TYPE_NETWORK:
>   case MULTIBOOT_TAG_TYPE_EFI_MMAP:
>   case MULTIBOOT_TAG_TYPE_EFI_BS:
> + case MULTIBOOT_TAG_TYPE_EFI32_IH:
> + case MULTIBOOT_TAG_TYPE_EFI64_IH:
> break;
> 
>   default:
> @@ -407,16 +409,18 @@ grub_multiboot_get_mbi_size (void)
>  + grub_get_multiboot_mmap_count ()
>  * sizeof (struct multiboot_mmap_entry)),
> MULTIBOOT_TAG_ALIGN)
>  + ALIGN_UP (sizeof (struct multiboot_tag_framebuffer),
> MULTIBOOT_TAG_ALIGN)
> +#ifdef GRUB_MACHINE_EFI
>  + ALIGN_UP (sizeof (struct multiboot_tag_efi32),
> MULTIBOOT_TAG_ALIGN)
>  + ALIGN_UP (sizeof (struct multiboot_tag_efi64),
> MULTIBOOT_TAG_ALIGN)
> ++ ALIGN_UP (sizeof (struct multiboot_tag_efi32_ih),
> MULTIBOOT_TAG_ALIGN)
> ++ ALIGN_UP (sizeof (struct multiboot_tag_efi64_ih),
> MULTIBOOT_TAG_ALIGN)
> ++ ALIGN_UP (sizeof (struct multiboot_tag_efi_mmap)
> +   + efi_mmap_size, MULTIBOOT_TAG_ALIGN)
> +#endif
>  + ALIGN_UP (sizeof (struct multiboot_tag_old_acpi)
> + sizeof (struct grub_acpi_rsdp_v10),
> MULTIBOOT_TAG_ALIGN)
>  + acpiv2_size ()
>  + net_size ()
> -#ifdef GRUB_MACHINE_EFI
> -+ ALIGN_UP (sizeof (struct multiboot_tag_efi_mmap)
> -   + efi_mmap_size, MULTIBOOT_TAG_ALIGN)
> -#endif
>  + sizeof (struct multiboot_tag_vbe) + MULTIBOOT_TAG_ALIGN - 1
>  + sizeof (struct multiboot_tag_apm) + MULTIBOOT_TAG_ALIGN - 1;
>  }
> @@ -906,11 +910,35 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
> 
>if (keep_bs)
>  {
> -  struct multiboot_tag *tag = (struct multiboot_tag *) ptrorig;
> -  tag->type = MULTIBOOT_TAG_TYPE_EFI_BS;
> -  tag->size = sizeof (struct multiboot_tag);
> -  ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
> -   / sizeof (grub_properly_aligned_t);
> +  {
> +   struct multiboot_tag *tag = (struct multiboot_tag *) ptrorig;
> +   tag->type = MULTIBOOT_TAG_TYPE_EFI_BS;
> +   tag->size = sizeof (struct multiboot_tag);
> +   ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
> + / sizeof (grub_properly_aligned_t);
> +  }
> +
> +#ifdef __x86_64__
> +  {
> +   struct multiboot_tag_efi64_ih *tag = (struct
> multiboot_tag_efi64_ih *) ptrorig;
> +   tag->type = MULTIBOOT_TAG_TYPE_EFI64_IH;
> +   tag->size = sizeof (*tag);
> +   tag->pointer = (grub_addr_t) grub_efi_image_handle;
> +   ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
> + / sizeof (grub_properly_aligned_t);
> +  }
> +#endif
> +
> +#ifdef __i386__
> +  {
> +   struct multiboot_tag_efi32_ih *tag = (struct
> multiboot_tag_efi32_ih *) ptrorig;
> +   tag->type = MULTIBOOT_TAG_TYPE_EFI32_IH;
> +   tag->size = sizeof (*tag);
> +   tag->pointer = (grub_addr_t) grub_efi_image_handle;
> +   ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
> + / sizeof (grub_properly_aligned_t);
> +  }
> +#endif
>  }
>  #endif
> 
> diff --git a/include/multiboot2.h b/include/multiboot2.h
> index b3977e3..9f97ddc 100644
> --- a/include/multiboot2.h
> +++ b/include/multiboot2.h
> @@ -60,6 +60,8 @@
>  #define MULTIBOOT_TAG_TYPE_NETWORK   16
>  #define MULTIBOOT_TAG_TYPE_EFI_MMAP  17
>  #define MULTIBOOT_TAG_TYPE_EFI_BS18
> +#define MULTIBOOT_TAG_TYPE_EFI32_IH  19
> +#define MULTIBOOT_TAG_TYPE_EFI64_IH  20
> 
>  #define MULTIBOOT_HEADER_TAG_END  0
>  #define MULTIBOOT_HEADER_TAG_INFORMATION_REQUEST  1
> @@ -379,6 +381,20 @@ struct multiboot_tag_efi_mmap
>multiboot_uint8_t efi_mmap[0];
>  };
> 
> +struct multiboot_tag_efi32_ih
> +{
> +  mult

Re: [PATCH v2 5/6] multiboot2: Add support for relocatable images

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 20.07.2015 16:35, Daniel Kiper wrote:
> Signed-off-by: Daniel Kiper 
What is handling the actual relocation? Why doesn't this patch include
support for ELF relocations?
> ---
>  grub-core/loader/i386/multiboot_mbi.c |6 ++--
>  grub-core/loader/multiboot.c  |   12 +--
>  grub-core/loader/multiboot_elfxx.c|   28 +++
>  grub-core/loader/multiboot_mbi2.c |   63 
> +
>  include/grub/multiboot.h  |4 ++-
>  include/multiboot2.h  |   24 +
>  6 files changed, 118 insertions(+), 19 deletions(-)
> 
> diff --git a/grub-core/loader/i386/multiboot_mbi.c 
> b/grub-core/loader/i386/multiboot_mbi.c
> index 956d0e3..abdb98b 100644
> --- a/grub-core/loader/i386/multiboot_mbi.c
> +++ b/grub-core/loader/i386/multiboot_mbi.c
> @@ -72,7 +72,8 @@ load_kernel (grub_file_t file, const char *filename,
>grub_err_t err;
>if (grub_multiboot_quirks & GRUB_MULTIBOOT_QUIRK_BAD_KLUDGE)
>  {
> -  err = grub_multiboot_load_elf (file, filename, buffer);
> +  err = grub_multiboot_load_elf (file, filename, buffer, 0, 0, 0, 0,
> +  GRUB_RELOCATOR_PREFERENCE_NONE, NULL, 0);
>if (err == GRUB_ERR_UNKNOWN_OS && (header->flags & 
> MULTIBOOT_AOUT_KLUDGE))
>   grub_errno = err = GRUB_ERR_NONE;
>  }
> @@ -118,7 +119,8 @@ load_kernel (grub_file_t file, const char *filename,
>return GRUB_ERR_NONE;
>  }
>  
> -  return grub_multiboot_load_elf (file, filename, buffer);
> +  return grub_multiboot_load_elf (file, filename, buffer, 0, 0, 0, 0,
> +   GRUB_RELOCATOR_PREFERENCE_NONE, NULL, 0);
>  }
>  
>  static struct multiboot_header *
> diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
> index ca7154f..1b1f7a9 100644
> --- a/grub-core/loader/multiboot.c
> +++ b/grub-core/loader/multiboot.c
> @@ -190,12 +190,18 @@ static grub_uint64_t highest_load;
>  /* Load ELF32 or ELF64.  */
>  grub_err_t
>  grub_multiboot_load_elf (grub_file_t file, const char *filename,
> -  void *buffer)
> +  void *buffer, int relocatable, grub_uint32_t min_addr,
> +  grub_uint32_t max_addr, grub_size_t align, 
> grub_uint32_t preference,
> +  grub_uint32_t *base_addr, int avoid_efi_boot_services)
>  {
>if (grub_multiboot_is_elf32 (buffer))
> -return grub_multiboot_load_elf32 (file, filename, buffer);
> +return grub_multiboot_load_elf32 (file, filename, buffer, relocatable,
> +   min_addr, max_addr, align, preference,
> +   base_addr, avoid_efi_boot_services);
>else if (grub_multiboot_is_elf64 (buffer))
> -return grub_multiboot_load_elf64 (file, filename, buffer);
> +return grub_multiboot_load_elf64 (file, filename, buffer, relocatable,
> +   min_addr, max_addr, align, preference,
> +   base_addr, avoid_efi_boot_services);
>  
>return grub_error (GRUB_ERR_UNKNOWN_OS, N_("invalid arch-dependent ELF 
> magic"));
>  }
> diff --git a/grub-core/loader/multiboot_elfxx.c 
> b/grub-core/loader/multiboot_elfxx.c
> index 6a220bd..4fce685 100644
> --- a/grub-core/loader/multiboot_elfxx.c
> +++ b/grub-core/loader/multiboot_elfxx.c
> @@ -51,7 +51,10 @@ CONCAT(grub_multiboot_is_elf, XX) (void *buffer)
>  }
>  
>  static grub_err_t
> -CONCAT(grub_multiboot_load_elf, XX) (grub_file_t file, const char *filename, 
> void *buffer)
> +CONCAT(grub_multiboot_load_elf, XX) (grub_file_t file, const char *filename,
> +  void *buffer, int relocatable, 
> grub_uint32_t min_addr,
> +  grub_uint32_t max_addr, grub_size_t align, 
> grub_uint32_t preference,
> +  grub_uint32_t *base_addr, int 
> avoid_efi_boot_services)
>  {
>Elf_Ehdr *ehdr = (Elf_Ehdr *) buffer;
>char *phdr_base;
> @@ -89,19 +92,30 @@ CONCAT(grub_multiboot_load_elf, XX) (grub_file_t file, 
> const char *filename, voi
> if (phdr(i)->p_paddr + phdr(i)->p_memsz > highest_load)
>   highest_load = phdr(i)->p_paddr + phdr(i)->p_memsz;
>  
> -   grub_dprintf ("multiboot_loader", "segment %d: paddr=0x%lx, 
> memsz=0x%lx, vaddr=0x%lx\n",
> - i, (long) phdr(i)->p_paddr, (long) phdr(i)->p_memsz, 
> (long) phdr(i)->p_vaddr);
> +   grub_dprintf ("multiboot_loader", "segment %d: paddr=0x%lx, 
> memsz=0x%lx, vaddr=0x%lx,"
> + "align=0x%lx, relocatable=%d, 
> avoid_efi_boot_services=%d\n", i,
> + (long) phdr(i)->p_paddr, (long) phdr(i)->p_memsz, 
> (long) phdr(i)->p_vaddr,
> + (long) align, relocatable, avoid_efi_boot_services);
>  
> {
>   grub_relocator_chunk_t ch;
> - err = grub_relocator_alloc_chunk_addr (grub_multiboot_relocator, 
> -  

Re: [PATCH v2 1/6] gitignore: Ignore *.orig, *.rej and *.swp files

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 09.11.2015 16:39, Daniel Kiper wrote:
> On Mon, Nov 09, 2015 at 04:34:20PM +0100, Vladimir 'φ-coder/phcoder' 
> Serbinenko wrote:
>> On 09.11.2015 16:29, Daniel Kiper wrote:
>>> On Wed, Nov 04, 2015 at 01:03:56PM +0100, Vladimir 'phcoder' Serbinenko 
>>> wrote:
>>>> Le 12 août 2015 11:04 AM, "Ian Campbell"  a écrit 
>>>> :
>>>>>
>>>>>
>>>>> (Having written the below I see too late that this is a grub patch not a
>>>>> Xen one, a tag in the subject for such cross posted patches would be
>>>> useful
>>>>> please. Anyway, my opinion counts for very little in this context but I
>>>>> leave it below since already I wrote it. I notice that xen.git#.gitignore
>>>>> _does_ list *.rej, which I think is wrong...)
>>>>>
>>>>> On Mon, 2015-07-20 at 16:35 +0200, Daniel Kiper wrote:
>>>>>> Signed-off-by: Daniel Kiper 
>>>>>
>>>>> At least *.rej and perhaps *.orig are indicative of a failed patch
>>>>> application, I think I want them to appear in "git status".
>>>>>
>>>>> By way of comparison Linux's .gitignore includes *.orig but not *.rej and
>>>>> Qemu's includes neither.
>>>>>
>>>>> So nack to the addition of *.rej from me. I'm more or less ambivalent
>>>> about
>>>>> *.orig.
>>>>>
>>>> I have to agree. You should clean up *.rej *.orig after fixing conflicts
>>>
>>> Thanks for comment on this. Could you review rest of this patchset?
>>> I am working on v3 and it will be nice to take your (and others if
>>> possible) comments into it.
>>>
>> I will go through them today
> 
> Thanks a lot!
> 
All reviewed. Some of them already good but they have dependencies. Feel
free to either fix concerns with dependencies or rebase in a way to get
the good ones committed first in a meaningful way
> Daniel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] broken ESC navigation if authentication is used

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 11.06.2015 05:55, Andrei Borzenkov wrote:
> В Wed, 10 Jun 2015 21:35:51 +0200
> "Vladimir 'phcoder' Serbinenko"  пишет:
> 
>> This patch may allow to escape to shell if menu was called from context
>> without menu entries. This may happen inadvertently I.a. when using
>> configfile. You need to add an additional parameter to indicate whether
>> it's OK to break from menu
> 
> Could you explain? Grub does
> 
> grub_enter_normal
>   grub_normal_execute
> grub_show_menu
>   grub_cmdline_run
> 
> if after processing config file there are no menu entries we do not
> even call grub_show_menu. And even if we do, after return from it there
> is mandatory authentication in grub_cmdline_run.
> 
Imagine something like following:
grub.cfg:
# Use another config file
configfile grub2.cfg
grub2.cfg:
superusers=root

Then pressing escape would lead you to the parent context where there is
no password protection.
Question is whether this is a misconfiguration on grub.cfg side (i.a.
should have been source, not configfile) or something to deal on code side.
> I see how it could happen in original commit when authentication was
> added, but I miss code path that cause it now. 
> 
>> Le 10 juin 2015 21:32, "Andrei Borzenkov"  a écrit :
>>
>>> В Wed, 10 Jun 2015 18:29:59 +0200
>>> Florian Kaiser  пишет:
>>>
 Hi,

 we are using grub2 with authentication enabled and multiple submenus.
 Unfortunately it is not possible to return to a previous menu with ESC
>>> without
 triggering a superuser password prompt. This is not the desired behavior
>>> in
 my opinion.
 I attached a patch to this email, which removes the password prompt when
 pressing escape.

>>>
>>> Looks OK; I'm not sure why this was needed in the first place - it does
>>> not look like it is even possible to exit primary menu.
>>>
>>> Vladimir, OK to commit?
>>>
>>> ___
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v4] ofdisk: add sas disks to the device list

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 11.11.2015 14:14, Paulo Flabiano Smorigo wrote:
> +  ptr = (grub_uint64_t *) (table + sizeof (grub_uint64_t) * i);
> +  grub_snprintf (bufptr, 32, "/disk@%" PRIxGRUB_UINT64_T, *ptr);
Please declare table as being grub_uint64_t * and then just use
table[i]. This code violates cast-align



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 3/4] * util/grub.d/20_linux_xen.in: Add support of the XEN boot on aarch64

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 10.11.2015 08:01, Andrei Borzenkov wrote:
> Not really, although this is a good observation. Actually I think that
> xen_initrd should indeed have behavior 1, because what it does is
> provide initrd image to Linux kernel, and - although not often used -
> initrd image may be constructed by concatenation.
> 
> But what I meant - initially it was intended to have xen_module with
> some options. As soon as we add option parsing we must have defined
> way to differentiate between opption for xen_module itself and option
> for module loaded by xen_module. I.e. it should be possible to
> 
> xen_module --type=XXX some-module --option1=FOO --option2=bar
Yes, everything before the filename would be an option to command
itself. We have a similar thing with multiboot command. Currently there
are no options to xen_*, so no code to handle them. But it's impossible
to get any confusion as if you currently specify any of such future
options you'll get an error of bad file, so we won't break any
compatibility. Do you see any reason to introduce flag-handling code
now? Would it be for better error message?



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH GRUB] Allow initrd concatenation on ARM64

[Vladimir 'φ-coder/phcoder' Serbinenko]

While on it also change "xen_linux" to "xen_kernel" to be vendor-neutral
Could someone test please? I only compile-tested it
diff --git a/docs/grub.texi b/docs/grub.texi
index 1df3db2..112b42b 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3859,7 +3859,7 @@ you forget a command, you can run the command @command{help}
 * videoinfo::   List available video modes
 @comment * xen_*::  Xen boot commands
 * xen_hypervisor::  Load xen hypervisor binary
-* xen_linux::   Load dom0 kernel for xen hypervisor
+* xen_kernel::  Load dom0 kernel for xen hypervisor
 * xen_initrd::  Load dom0 initrd for dom0 kernel
 * xen_xsm:: Load xen security module for xen hypervisor
 @end menu
@@ -5134,10 +5134,10 @@ verbatim as the @dfn{kernel command-line}. Any other binaries must be
 reloaded after using this command.
 @end deffn
 
-@node xen_linux
-@subsection xen_linux
+@node xen_kernel
+@subsection xen_kernel
 
-@deffn Command xen_linux file [arguments]
+@deffn Command xen_kernel file [arguments]
 Load a dom0 kernel image for xen hypervisor at the booting process of xen.
 The rest of the line is passed verbatim as the module command line.
 @end deffn
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index d9fa0e3..34f7b61 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1641,6 +1642,7 @@ module = {
 module = {
   name = xen_boot;
   common = lib/cmdline.c;
+  common = loader/linux.c;
   arm64 = loader/arm64/xen_boot.c;
   enable = arm64;
 };
diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c
index d1a2189..e4a12bc 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -33,6 +33,7 @@
 #include 	/* required by struct xen_hypervisor_header */
 #include 
 #include 
+#include 
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
@@ -137,17 +138,53 @@ xen_boot_address_align (grub_addr_t start, grub_size_t align)
 }
 
 /* set module type according to command name. */
-static grub_err_t
-set_module_type (grub_command_t cmd, struct xen_boot_binary *module)
+static struct xen_boot_binary *
+allocate_module (module_type_t type)
 {
-  if (!grub_strcmp (cmd->name, "xen_linux"))
-module->node_info.type = MODULE_IMAGE;
-  else if (!grub_strcmp (cmd->name, "xen_initrd"))
-module->node_info.type = MODULE_INITRD;
-  else if (!grub_strcmp (cmd->name, "xen_xsm"))
-module->node_info.type = MODULE_XSM;
+  struct xen_boot_binary *module;
 
-  return GRUB_ERR_NONE;
+  if (!loaded)
+{
+  grub_error (GRUB_ERR_BAD_ARGUMENT,
+		  N_("you need to load the Xen Hypervisor first"));
+  return NULL;
+}
+
+  module =
+(struct xen_boot_binary *) grub_zalloc (sizeof (struct xen_boot_binary));
+  if (!module)
+return NULL;
+
+  module->node_info.type = type;
+
+  switch (module->node_info.type)
+{
+case MODULE_IMAGE:
+case MODULE_INITRD:
+case MODULE_XSM:
+  module->node_info.compat_string =
+	default_compat_string[module->node_info.type].compat_string;
+  module->node_info.compat_string_size =
+	default_compat_string[module->node_info.type].size;
+  break;
+
+case MODULE_CUSTOM:
+  /* we have set the node_info in set_module_type */
+  break;
+
+default:
+  grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument"));
+  return NULL;
+}
+  module->name = module->node_info.compat_string;
+  module->align = module_default_align[module->node_info.type];
+
+  grub_dprintf ("xen_loader", "Init %s module and node info:\n"
+		"compatible %s\ncompat_string_size 0x%lx\n",
+		module->name, module->node_info.compat_string,
+		module->node_info.compat_string_size);
+
+  return module;
 }
 
 static grub_err_t
@@ -372,11 +409,11 @@ xen_unload (void)
   return GRUB_ERR_NONE;
 }
 
-static void
-xen_boot_binary_load (struct xen_boot_binary *binary, grub_file_t file,
-		  int argc, char *argv[])
+static grub_err_t
+xen_boot_binary_allocate (struct xen_boot_binary *binary,
+			  grub_size_t size)
 {
-  binary->size = grub_file_size (file);
+  binary->size = size;
   grub_dprintf ("xen_loader", "Xen_boot %s file size: 0x%lx\n",
 		binary->name, binary->size);
 
@@ -386,13 +423,21 @@ xen_boot_binary_load (struct xen_boot_binary *binary, grub_file_t file,
 	 (binary->size +
 	  binary->align));
   if (!binary->start)
-{
-  grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
-  return;
-}
+return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
 
   grub_dprintf ("xen_loader", "Xen_boot %s numpages: 0x%lx\n",
 		binary->name, GRUB_EFI_BYTES_TO_PAGES (binary->size + binary->align));
+  return GRUB_ERR_NONE;
+}
+
+static void
+xen_boot_binary_load (struct xen_boot_binary *binary, grub_file_t file,
+		  int argc, char *argv[])
+{
+  if (xen_boot_binary_allocate(binary, grub_file_size(file)))
+{
+  return;
+}
 
   if (grub_f

Re: [PATCH v2 5/6] multiboot2: Add support for relocatable images

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 12.11.2015 14:15, Daniel Kiper wrote:
> On Tue, Nov 10, 2015 at 04:23:46PM +0100, Vladimir 'phcoder' Serbinenko wrote:
>> Le 10 nov. 2015 3:52 PM, "Daniel Kiper"  a écrit :
>>> On Mon, Nov 09, 2015 at 09:08:35PM +0100, Vladimir 'φ-coder/phcoder'
>> Serbinenko wrote:
>>>> On 20.07.2015 16:35, Daniel Kiper wrote:
>>>>> Signed-off-by: Daniel Kiper 
>>>> What is handling the actual relocation?
>>>
>>> In Xen we get image offset from GRUB2 and using this value we calculate
>>> relative addresses. We are using %fs for it.
>>>
>>>> Why doesn't this patch include support for ELF relocations?
>>>
>>> This is another option. We considered it too. However, in our case
>>> relative addressing looks simpler then ELF relocations.
>> How is it simpler than to have a fully relocated binary when you start?
> 
> Xen ELF file does not have relocations.
> 
>> How do you pass the offset?
> 
> Via MULTIBOOT_TAG_TYPE_BASE_ADDR tag.
> 
>> Does xen have any relocation entries?
> 
> No.
> 
Can we then settle on using your interface but saying that bootloader
has to execute all ELF relocations? Since you don't have them, you
wouldn't be affected by the change but it would allow easy creation of
relocatable binaries. Feel free in the first patch just to have a check
for absence of relocation entries. x86-64 has only about 5 relocations
we care about so should be easy to implement. We can reuse code in dl.c.
>> Was such xen already released? Just looking for how it should
>> be in perspective and how to get there
> 
> We agreed (in Xen community) that we would like to have multiboot2 protocol
> with additional features set in stone at first. So, this patch series just
> propose some changes which are required by Xen but they are not used by any
> Xen release yet. Additionally, we want that these extensions are quite generic
> and could be used by other projects if they need them too.
> 
> Daniel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Uniform commands for booting xen

[Vladimir 'φ-coder/phcoder' Serbinenko]

Hello, all. I'd like to have set of commands that would boot xen on all
platforms. I thought of following set:

xen_hypervisor FILE XEN_OPTIONS
xen_kernel FILE KERNEL_OPTIONS
xen_initrd INITRD INITRD INITRD
all initrds are concatenated.
xen_xsm ???

On arm64 it would use the arm64 xen FDT protocol but on x86 should we
use multiboot2 if multiboot2 header is present and multiboot otherwise?
Or do xen devs have other preferences?



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 3/6] i386/relocator: Add grub_relocator64_efi relocator

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 10.11.2015 15:38, Daniel Kiper wrote:
>>> -  if (entry_specified)
>>> > > +  if (keep_bs && efi_entry_specified)
>>> > > +grub_multiboot_payload_eip = efi_entry;
>>> > > +  else if (entry_specified)
>>> > >  grub_multiboot_payload_eip = entry;
>>> > >
>> > This seems redundant.
> What is wrong here?
I just mean that if we use a single structure this code could go away



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Xen-devel] [PATCH GRUB] Allow initrd concatenation on ARM64

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 12.11.2015 14:27, Ian Campbell wrote:
> On Thu, 2015-11-12 at 14:08 +0100, Vladimir 'φ-coder/phcoder' Serbinenko
> wrote:
>> While on it also change "xen_linux" to "xen_kernel" to be vendor-neutral
>> Could someone test please? I only compile-tested it
> 
> I was expecting this patch to include a change
> to ./util/grub.d/20_linux_xen.in to update the naming there, but it looks
> like that aspect of the original series isn't in tree yet?
> 
We need to figure out what we should do on x86 wrt multiboot vs
multiboot2 before adapting 20_linux_xen
> BTW, you have a stray "linux" in the description of the (now) xen_kernel
> command.
> 
Fixed locally, thank you
> Ian.
> 
> .
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: grub causing NVDIMMs to be treated as normal memory

[Vladimir 'φ-coder/phcoder' Serbinenko]

What about this patch for the passing of pram?
diff --git a/grub-core/mmap/efi/mmap.c b/grub-core/mmap/efi/mmap.c
index 900a4d6..0c03c5d 100644
--- a/grub-core/mmap/efi/mmap.c
+++ b/grub-core/mmap/efi/mmap.c
@@ -118,6 +118,12 @@ grub_efi_mmap_iterate (grub_memory_hook_t hook,
void *hook_data,
GRUB_MEMORY_NVS, hook_data);
  break;

+   case GRUB_EFI_PERSISTENT_MEMORY:
+ hook (desc->physical_start, desc->num_pages * 4096,
+   GRUB_MEMORY_PRAM, hook_data);
+ break;
+
+
default:
  grub_printf ("Unknown memory type %d, considering reserved\n",
   desc->type);
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index 24a05c5..2bbfe34 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -476,6 +476,7 @@ enum grub_efi_memory_type
 GRUB_EFI_MEMORY_MAPPED_IO,
 GRUB_EFI_MEMORY_MAPPED_IO_PORT_SPACE,
 GRUB_EFI_PAL_CODE,
+GRUB_EFI_PERSISTENT_MEMORY,
 GRUB_EFI_MAX_MEMORY_TYPE
   };
 typedef enum grub_efi_memory_type grub_efi_memory_type_t;
diff --git a/include/grub/memory.h b/include/grub/memory.h
index 083cfb6..1003a9c 100644
--- a/include/grub/memory.h
+++ b/include/grub/memory.h
@@ -30,6 +30,7 @@ typedef enum grub_memory_type
 GRUB_MEMORY_ACPI = 3,
 GRUB_MEMORY_NVS = 4,
 GRUB_MEMORY_BADRAM = 5,
+GRUB_MEMORY_PRAM = 7,
 GRUB_MEMORY_COREBOOT_TABLES = 16,
 GRUB_MEMORY_CODE = 20,
 /* This one is special: it's used internally but is never reported
>>> Note (b): The internal GRUB_MEMORY_CODE (20) value is
>>> leaking through to the E820 table.
>>>
>>> That appears to be from this patch on 2013-10-14:
>>> 6de9ee86 Pass-through unknown E820 types
>>
>> If we are discussing ACPI 6.0 systems here, it explicitly says that
>> values above 12 should be treated as reserved. Does it cause
>> problems?
> 
> All undefined values are reserved for future standardization;
> the meaning they might have in the future is unpredictable.
> 
> Software compatible with ACPI 6.0 is supposed to treat them as
> reserved, but software compatible with a future version of ACPI
> might interpret them as having some different meaning that isn't
> compatible with GRUB_MEMORY_CODE.
> 
> Some companies used e820 type 12 to mean persistent memory without
> getting that assigned by the ACPI WG, so that value was
> contaminated.  We should probably mark 20 as contaminated too, 
> given this issue.
> 
I see now that we have leaked 16 (coreboot tables) as well. Could we
mark 16 as contaminated as well?
For memory code: should we just pass reserved in linux e820 or is it
better to keep doing this bug given possible reliance on it by other
software?
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: flex version

[Vladimir 'φ-coder/phcoder' Serbinenko]

Andrei, could you please commit this patch?
On 15.11.2015 07:26, Andrei Borzenkov wrote:
> 14.11.2015 19:22, Peter Cheung пишет:
>> hi
>> my flex in mac is installed using macports.
>>
>> $flex --version
>> flex 2.5.35 Apple(flex-31)
>>
>> And the line in configure fail
>>
>> version=`$LEX --version | $AWK '{ split($NF,x,"."); print
>> x[1]*1+x[2]*100+x[3]; }’`
>>
>> Better to change it to:
>>
>> version=`$LEX --version | sed 's/flex //'|sed 's/ .*//'| $AWK '{
>> split($NF,x,"."); print x[1]*1+x[2]*100+x[3]; }’`
>>
> 
> Does attached patch help?
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: grub causing NVDIMMs to be treated as normal memory

[Vladimir 'φ-coder/phcoder' Serbinenko]

New version attached

>>  GRUB_MEMORY_COREBOOT_TABLES = 16,
>>  GRUB_MEMORY_CODE = 20,
>>  /* This one is special: it's used internally but is never reported
> Note (b): The internal GRUB_MEMORY_CODE (20) value is
> leaking through to the E820 table.
>
> That appears to be from this patch on 2013-10-14:
> 6de9ee86 Pass-through unknown E820 types

 If we are discussing ACPI 6.0 systems here, it explicitly says that
 values above 12 should be treated as reserved. Does it cause
 problems?
>>>
>>> All undefined values are reserved for future standardization;
>>> the meaning they might have in the future is unpredictable.
>>>
>>> Software compatible with ACPI 6.0 is supposed to treat them as
>>> reserved, but software compatible with a future version of ACPI
>>> might interpret them as having some different meaning that isn't
>>> compatible with GRUB_MEMORY_CODE.
>>>
>>> Some companies used e820 type 12 to mean persistent memory without
>>> getting that assigned by the ACPI WG, so that value was
>>> contaminated.  We should probably mark 20 as contaminated too,
>>> given this issue.
>>>
>> I see now that we have leaked 16 (coreboot tables) as well. Could we
>> mark 16 as contaminated as well?
>> For memory code: should we just pass reserved in linux e820 or is it
>> better to keep doing this bug given possible reliance on it by other
>> software?
> 
> I think it is better to leave it as is as long as those values can be 
> reserved.
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 

diff --git a/grub-core/mmap/efi/mmap.c b/grub-core/mmap/efi/mmap.c
index 900a4d6..2beffe2 100644
--- a/grub-core/mmap/efi/mmap.c
+++ b/grub-core/mmap/efi/mmap.c
@@ -118,6 +118,11 @@ grub_efi_mmap_iterate (grub_memory_hook_t hook, void *hook_data,
 		GRUB_MEMORY_NVS, hook_data);
 	  break;
 
+	case GRUB_EFI_MEMORY_PERSISTENT:
+	  hook (desc->physical_start, desc->num_pages * 4096,
+		GRUB_MEMORY_PERSISTENT, hook_data);
+	  break;
+
 	default:
 	  grub_printf ("Unknown memory type %d, considering reserved\n",
 		   desc->type);
@@ -158,6 +163,8 @@ make_efi_memtype (int type)
 
 case GRUB_MEMORY_NVS:
   return GRUB_EFI_ACPI_MEMORY_NVS;
+case GRUB_MEMORY_PERSISTENT:
+  return GRUB_EFI_MEMORY_PERSISTENT;
 }
 
   return GRUB_EFI_UNUSABLE_MEMORY;
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index 24a05c5..d1b9799 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -476,6 +476,7 @@ enum grub_efi_memory_type
 GRUB_EFI_MEMORY_MAPPED_IO,
 GRUB_EFI_MEMORY_MAPPED_IO_PORT_SPACE,
 GRUB_EFI_PAL_CODE,
+GRUB_EFI_MEMORY_PERSISTENT,
 GRUB_EFI_MAX_MEMORY_TYPE
   };
 typedef enum grub_efi_memory_type grub_efi_memory_type_t;
diff --git a/include/grub/memory.h b/include/grub/memory.h
index 083cfb6..2e734b7 100644
--- a/include/grub/memory.h
+++ b/include/grub/memory.h
@@ -30,6 +30,7 @@ typedef enum grub_memory_type
 GRUB_MEMORY_ACPI = 3,
 GRUB_MEMORY_NVS = 4,
 GRUB_MEMORY_BADRAM = 5,
+GRUB_MEMORY_PERSISTENT = 7,
 GRUB_MEMORY_COREBOOT_TABLES = 16,
 GRUB_MEMORY_CODE = 20,
 /* This one is special: it's used internally but is never reported


signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Grub get and set efi variables

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 14.11.2015 05:03, Andrei Borzenkov wrote:
> 13.11.2015 22:42, Ignat Korchagin пишет:
> +static enum efi_var_type
> +parse_efi_var_type (const char *type)
> +{
> +  if (!grub_strncmp (type, "string", sizeof("string")))
> +return EFI_VAR_STRING;
> +


 I think this should be "ascii" or "utf8". "string" is too ambiguous
 in UEFI
 environment, it can also mean sequence of UCS-2 characters.
>>> I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
>>> when printing. Maybe, to avoid confusion, it might be better to
>>> completely remove this option. Basically, you do not want to interpret
>>> raw buffers as strings. For best compatibility "hex" mode should be
>>> promoted, I guess. What do you think?
>> Checked again the UEFI spec. For globally defined variables which are
>> strings they specify ASCII. So if we leave this option, ascii is the
>> best name.
>>
> 
> What about
> 
>   - ascii - print ASCII characters verbatim, escape non-ASCII in usual
> way (similar to "od -c")
> 
>   - raw - simply put raw variable without any interpretation.
> 
I would add:
* hex. To print and store in hex form
* utf16. Decode utf16 into utf8. utf16 is a common encoding in EFI land.
I would also skip on raw, at least until we have a real need for it as
we currently are not equiped to handle raw strings in variable contents,
including but not limited to \0 being considered a terminator
> This is better aligned with argument describing output formatting rather
> than attempting to "type" variable.
> 
> Alternative (or in addition to) ascii - dump which prints usual pretty
> hex dump of content (hexdump -C). This is handy to interactively look at
> variable content.
> 
> Or, and change name from --type to --format :)
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/3] mkstandalone: add argument --fixed-time to override mtime of files

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 04.12.2015 17:10, Alexander Couzens wrote:
> mkstandalone adds several files to an archive. Doing this it uses the
> mtime to give these files a timestamp.
> --fixed-time  overrides these timestamps with a given.
> 
> Replacing all timestamps with a specific one is required
> to get reproducible builds. See source epoch specification of
> reproducible-builds.org
Patch in general looks good. I'm unsure about which way the timestamp
should be passed and parsed. I see 3 solutions:
1) Argument and use some standard function to parse date supply argument
+
2) Essentially what you have done. It feels a bit ugly but not too much
3) Read directly from variable.
WDYT?
> +  {"fixed-time", 't', N_("TIMEEPOCH"), 0, N_("Use a fixed timestamp to 
> override mtime of all files. Time since epoch is used."), 2},
It's not worth spending a letter on this. Please keep only long version.



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 3/3] Makefile: use FIXED_TIMESTAMP for mkstandalone if set

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 04.12.2015 17:10, Alexander Couzens wrote:
> mkstandalone sets timestamps for files which can be overriden by a 
> fixed_timestamp.
> This makes it possible to build reproducible builds for coreboot.
> 
> To build a reproducible build of grub for coreboot do:
> make default_payload.elf FIXED_TIMESTAMP=1134242
Why FIXED_TIMESTAMP and not SOURCE_DATE_EPOCH ?
https://reproducible-builds.org/specs/source-date-epoch/



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 1/3] mkstandalone: add argument --fixed-time to override mtime of files

[Vladimir 'φ-coder/phcoder' Serbinenko]

> +  fixed_time = -1;
-1 is actually perfectly valid. Can we have a second boolean to avoid
special-casing -1?




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 3/3] Makefile/coreboot use SOURCE_DATE_EPOCH as time source if set

[Vladimir 'φ-coder/phcoder' Serbinenko]

Looks good
On 04.12.2015 19:32, Alexander Couzens wrote:
> mkstandalone sets timestamps for files which can be overriden by a 
> fixed_timestamp.
> This makes it possible to build reproducible builds for coreboot.
> 
> To build a reproducible build of grub for coreboot do:
> export SOURCE_DATE_EPOCH=1134242
> make default_payload.elf
> ---
>  Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 994ebbd..5c756d7 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -403,7 +403,7 @@ bootcheck: $(BOOTCHECKS)
>  
>  if COND_i386_coreboot
>  default_payload.elf: grub-mkstandalone grub-mkimage
> - pkgdatadir=. ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O 
> i386-coreboot -o $@ --modules='ahci pata ehci uhci ohci usb_keyboard usbms 
> part_msdos xfs ext2 fat at_keyboard part_gpt usbserial_usbdebug cbfs' 
> --install-modules='ls linux search configfile normal cbtime cbls memrw iorw 
> minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi chain 
> test serial multiboot cbmemc linux16 gzio echo help' --fonts= --themes= 
> --locales= -d grub-core/ /boot/grub/grub.cfg=$(srcdir)/coreboot.cfg
> + pkgdatadir=. ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O 
> i386-coreboot -o $@ --modules='ahci pata ehci uhci ohci usb_keyboard usbms 
> part_msdos xfs ext2 fat at_keyboard part_gpt usbserial_usbdebug cbfs' 
> --install-modules='ls linux search configfile normal cbtime cbls memrw iorw 
> minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi chain 
> test serial multiboot cbmemc linux16 gzio echo help' --fonts= --themes= 
> --locales= -d grub-core/ /boot/grub/grub.cfg=$(srcdir)/coreboot.cfg $(if 
> $(SOURCE_DATE_EPOCH),-t $(SOURCE_DATE_EPOCH))
>  endif
>  
>  endif
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 2/3] mkrescue: add argument --fixed-time to get reproducible uuids

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 04.12.2015 19:32, Alexander Couzens wrote:
> The uuid generation is based on the time.
> ---
>  util/grub-mkrescue.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
This breaks uniqueness assumptions for UUID and we use UUID to find the
right disk, as it's not possible to rely on passed boot disk on some
platforms (I've just documented it in grub.texi and pushed it). Also for
mkrescue we always use UUID. We need to find a way to reliably find boot
disk without depending on current time.
> diff --git a/util/grub-mkrescue.c b/util/grub-mkrescue.c
> index 4511826..1af1da2 100644
> --- a/util/grub-mkrescue.c
> +++ b/util/grub-mkrescue.c
> @@ -52,6 +52,7 @@ static int xorriso_arg_alloc;
>  static char **xorriso_argv;
>  static char *iso_uuid;
>  static char *iso9660_dir;
> +static time_t fixed_time;
>  
>  static void
>  xorriso_push (const char *val)
> @@ -110,6 +111,7 @@ static struct argp_option options[] = {
>{"product-version", OPTION_PRODUCT_VERSION, N_("STRING"), 0, N_("use 
> STRING as product version"), 2},
>{"sparc-boot", OPTION_SPARC_BOOT, 0, 0, N_("enable sparc boot. Disables 
> HFS+, APM, ARCS and boot as disk image for i386-pc"), 2},
>{"arcs-boot", OPTION_ARCS_BOOT, 0, 0, N_("enable ARCS (big-endian mips 
> machines, mostly SGI) boot. Disables HFS+, APM, sparc64 and boot as disk 
> image for i386-pc"), 2},
> +  {"fixed-time", 0, N_("TIMEEPOCH"), 0, N_("use a fixed timestamp for uuid 
> generation"), 2},
>{0, 0, 0, 0, 0, 0}
>  };
>  
> @@ -153,6 +155,8 @@ enum {
>  static error_t 
>  argp_parser (int key, char *arg, struct argp_state *state)
>  {
> +  char *b;
> +
>if (grub_install_parse (key, arg))
>  return 0;
>switch (key)
> @@ -212,6 +216,15 @@ argp_parser (int key, char *arg, struct argp_state 
> *state)
>xorriso = xstrdup (arg);
>return 0;
>  
> +case 't':
> +  fixed_time = strtoll (arg, &b, 10);
> +  if (*b !='\0') {
> +printf (_("invalid fixed time number: %s\n"), arg);
> +argp_usage (state);
> +exit (1);
> +  }
> +  return 0;
> +
>  default:
>return ARGP_ERR_UNKNOWN;
>  }
> @@ -431,6 +444,7 @@ main (int argc, char *argv[])
>  
>pkgdatadir = grub_util_get_pkgdatadir ();
>  
> +  fixed_time = -1;
>product_name = xstrdup (PACKAGE_NAME);
>product_version = xstrdup (PACKAGE_VERSION);
>xorriso = xstrdup ("xorriso");
> @@ -541,7 +555,7 @@ main (int argc, char *argv[])
>{
>  time_t tim;
>  struct tm *tmm;
> -tim = time (NULL);
> +tim = fixed_time != -1 ? fixed_time : time (NULL);
>  tmm = gmtime (&tim);
>  iso_uuid = xmalloc (55);
>  grub_snprintf (iso_uuid, 50,
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Each grub-mkrescue run leaves a file in /tmp/

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 28.12.2015 04:40, Andrei Borzenkov wrote:
> 27.12.2015 23:48, Thomas Schmitt пишет:
>> Hi,
>>
>> when running grub-mkrescue from Debian Sid, i get a
>> growing collection of files in /tmp. One per run.
>>
>> Names are like:
>>   /tmp/grub.00529W
>>
>> Content is always the same:
>>   insmod part_acorn
>>   insmod part_amiga
>>   insmod part_apple
>>   insmod part_bsd
>>   insmod part_dfly
>>   insmod part_dvh
>>   insmod part_gpt
>>   insmod part_msdos
>>   insmod part_plan
>>   insmod part_sun
>>   insmod part_sunpc
>>
> 
> That's load.cfg used to build image. I'm surprised this is the only one
> left, there should be more.
> 
>> grub-mkrescue -V
>>   grub-mkrescue (GRUB) 2.02~beta2-33
>>
>> Where to report ? Upstream or distro ?
>>
> 
> Yes, I have this hanging around. Not because of garbage in /tmp, but to
> better documentation of generated image (load.cfg is stored by
> grub-install as well).
> 
> Vladimir?
> 
Fixed. It was just omission of delete. We leave load.cfg in our
directory (boot/grub) but it makes no sense to leave anything in temp
> 
> 
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: grub causing NVDIMMs to be treated as normal memory

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 28.11.2015 07:41, Elliott, Robert (Persistent Memory) wrote:
> If you can define a standard meaning for 16 and 20, that'd be more
> useful than marking them as OEM defined.  There will always be a mix
> of software that interprets it as unusable vs. follows this new
> advice.
16 would be "RAM holding coreboot tables as defined in coreboot lbio.h"
20 would be E820 counterpart of EFI_RUNTIME_SERVICES_CODE



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] normal: fix get_logical_num_lines

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 28.12.2015 05:09, Michael Chang wrote:
> On Thu, Dec 24, 2015 at 02:48:34PM +0300, Andrei Borzenkov wrote:
>> On Wed, Dec 23, 2015 at 7:45 AM, Michael Chang  wrote:
>>> In menu editing mode, grub2 shows bogus line if the character being
>>> edited is at last column of entry. This patch fixes the problem by
>>> having the get_logical_num_lines function to calculate correct number of
>>> lines.
>>>
>>> ---
>>>  grub-core/normal/menu_entry.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/grub-core/normal/menu_entry.c b/grub-core/normal/menu_entry.c
>>> index 62c7e16..1d4b0c6 100644
>>> --- a/grub-core/normal/menu_entry.c
>>> +++ b/grub-core/normal/menu_entry.c
>>> @@ -128,7 +128,7 @@ get_logical_num_lines (struct line *linep, struct 
>>> per_term_screen *term_screen)
>>>  {
>>>return (grub_getstringwidth (linep->buf, linep->buf + linep->len,
>>>term_screen->term)
>>> - / (unsigned) term_screen->geo.entry_width) + 1;
>>> + / ((unsigned) term_screen->geo.entry_width + 1)) + 1;
>>
>> No, that's wrong. Consider entry_width = 10 and grub_getstringwidth =
>> 21. It needs 3 lines but your change gives only 2.
> 
> Alas! Indeed I am mistaken here. We should minus the numerator by one
> but not adding one to denominator. Thanks for your check. :)
> 
>>
>> It sounds like we need
>>
>> string_width = grub_getstringwidth (linep->buf, linep->buf +
>> linep->len, term_screen->term);
>> if (!string_width)
>>   return 1;
>> return (string_width + (unsigned) term_screen->geo.entry_width - 1) /
>> (unsigned) term_screen->geo.entry_width;
>>
>> Could you test if it works for you?
> 
> Yes. It works great.
@Anrei: please go ahead.
> 
> Thanks,
> Michael
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] 30_os-prober: derive --class from os-prober generated label

[Vladimir 'φ-coder/phcoder' Serbinenko]

Go ahead
On 26.12.2015 08:09, Andrey Borzenkov wrote:
> Currently only Windows gets distinguished icons, everything else is displayed
> using the same generic one. Add additional --class based on os-prober returned
> label, which usually is expected to match primary distribution name.
> 
> Also use it for Windows as well - chainloader prober nay actually return
> different strings (Windows, MS-DOS, Windows9xME).
> 
> ---
>  util/grub.d/30_os-prober.in | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/util/grub.d/30_os-prober.in b/util/grub.d/30_os-prober.in
> index 5fc4f0c..5604ce2 100644
> --- a/util/grub.d/30_os-prober.in
> +++ b/util/grub.d/30_os-prober.in
> @@ -135,6 +135,9 @@ for OS in ${OSPROBED} ; do
>  LONGNAME="${LABEL}"
>fi
>  
> +  # os-prober returns text string followed by optional counter
> +  CLASS="--class $(echo "${LABEL}" | LC_ALL=C sed 's,[[:digit:]]*$,,' | cut 
> -d' ' -f1 | tr 'A-Z' 'a-z' | LC_ALL=C sed 's,[^[:alnum:]_],_,g')"
> +
>gettext_printf "Found %s on %s\n" "${LONGNAME}" "${DEVICE}" >&2
>  
>case ${BOOT} in
> @@ -142,7 +145,7 @@ for OS in ${OSPROBED} ; do
>  
> onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
>cat << EOF
> -menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' --class windows 
> --class os \$menuentry_id_option 'osprober-chain-$(grub_get_device_id 
> "${DEVICE}")' {
> +menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' $CLASS --class os 
> \$menuentry_id_option 'osprober-chain-$(grub_get_device_id "${DEVICE}")' {
>  EOF
>save_default_entry | grub_add_tab
>prepare_grub_to_access_device ${DEVICE} | grub_add_tab
> @@ -174,7 +177,7 @@ EOF
>   DEVICE=${DEVICE%@*}
>   onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
>cat << EOF
> -menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' --class windows 
> --class os \$menuentry_id_option 'osprober-efi-$(grub_get_device_id 
> "${DEVICE}")' {
> +menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' $CLASS --class os 
> \$menuentry_id_option 'osprober-efi-$(grub_get_device_id "${DEVICE}")' {
>  EOF
>save_default_entry | sed -e "s/^/\t/"
>prepare_grub_to_access_device ${DEVICE} | sed -e "s/^/\t/"
> @@ -230,7 +233,7 @@ EOF
>  
>   if [ "x$is_top_level" = xtrue ] && [ "x${GRUB_DISABLE_SUBMENU}" != xy 
> ]; then
>  cat << EOF
> -menuentry '$(echo "$OS $onstr" | grub_quote)' --class gnu-linux --class gnu 
> --class os \$menuentry_id_option 'osprober-gnulinux-simple-$boot_device_id' {
> +menuentry '$(echo "$OS $onstr" | grub_quote)' $CLASS --class gnu-linux 
> --class gnu --class os \$menuentry_id_option 
> 'osprober-gnulinux-simple-$boot_device_id' {
>  EOF
>   save_default_entry | grub_add_tab
>   printf '%s\n' "${prepare_boot_cache}"
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] arm64: build with -mcmodel=large

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 25.12.2015 19:18, Leif Lindholm wrote:
> On Thu, Dec 24, 2015 at 04:20:29AM +, Colin Watson wrote:
>> On Thu, Dec 24, 2015 at 04:14:20AM +, Colin Watson wrote:
>>> This fixes a build failure with very current GCC versions, such as the one
>>> in Ubuntu xenial.  Leif (or anyone with suitable arm64 systems), would you
>>> mind testing that this doesn't break things?  I've tested that it builds
>>> cleanly now, but I don't have a particularly convenient way to do any
>>> run-time tests.
>>
>> Never mind, I spoke too soon and withdraw this patch, since this doesn't
>> actually fix the problem, which is:
>>
>>   $ obj/grub-efi-arm64/grub-mkimage -O arm64-efi -o test.efi -d 
>> obj/grub-efi-arm64/grub-core -p /boot/grub -v ext2
>>   obj/grub-efi-arm64/grub-mkimage: info: the total module size is 0x37e8.
> 
> *snip*
> 
>>   obj/grub-efi-arm64/grub-mkimage: info: dealing with the relocation section 
>> .rela.text for .text.
>>   obj/grub-efi-arm64/grub-mkimage: error: relocation 0x113 is not 
>> implemented yet.
>>
>> Would anyone arm64-knowledgeable mind taking a look at this?
> 
> So, it seems this toolchain generates the HI21/LO12 relocation combo:
> - R_AARCH64_ADR_PREL_PG_HI21/R_AARCH64_ADR_PREL_PG_HI21_NC
> - R_AARCH64_LDST16_ABS_LO12_NC
> - R_AARCH64_LDST32_ABS_LO12_NC
> - R_AARCH64_LDST64_ABS_LO12_NC
> - R_AARCH64_LDST128_ABS_LO12_NC
> 
> So I'll implement support for these.
> 
I'm looking forward for those patches
Unfortunately missing relocation support is common problem. I added a
verifier in build system to catch it on build time rather than runtime
> With regards to your -mcmodel=large patch - that didn't change
> anything because I already hardcoded that into
> conf/Makefile.common. Your suggested patch is probably the better way
> of doing it - so do consider pushing that anyway (dropping the
> Makefile.common stanza at the same time).
> 
> /
> Leif
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] arm64: build with -mcmodel=large

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 25.12.2015 19:18, Leif Lindholm wrote:
> On Thu, Dec 24, 2015 at 04:20:29AM +, Colin Watson wrote:
>> On Thu, Dec 24, 2015 at 04:14:20AM +, Colin Watson wrote:
>>> This fixes a build failure with very current GCC versions, such as the one
>>> in Ubuntu xenial.  Leif (or anyone with suitable arm64 systems), would you
>>> mind testing that this doesn't break things?  I've tested that it builds
>>> cleanly now, but I don't have a particularly convenient way to do any
>>> run-time tests.
>>
>> Never mind, I spoke too soon and withdraw this patch, since this doesn't
>> actually fix the problem, which is:
>>
>>   $ obj/grub-efi-arm64/grub-mkimage -O arm64-efi -o test.efi -d 
>> obj/grub-efi-arm64/grub-core -p /boot/grub -v ext2
>>   obj/grub-efi-arm64/grub-mkimage: info: the total module size is 0x37e8.
> 
> *snip*
> 
>>   obj/grub-efi-arm64/grub-mkimage: info: dealing with the relocation section 
>> .rela.text for .text.
>>   obj/grub-efi-arm64/grub-mkimage: error: relocation 0x113 is not 
>> implemented yet.
>>
>> Would anyone arm64-knowledgeable mind taking a look at this?
> 
> So, it seems this toolchain generates the HI21/LO12 relocation combo:
> - R_AARCH64_ADR_PREL_PG_HI21/R_AARCH64_ADR_PREL_PG_HI21_NC
> - R_AARCH64_LDST16_ABS_LO12_NC
> - R_AARCH64_LDST32_ABS_LO12_NC
> - R_AARCH64_LDST64_ABS_LO12_NC
> - R_AARCH64_LDST128_ABS_LO12_NC
> 
> So I'll implement support for these.
> 
relocations 264, 266, 268, 269 are also missing
> With regards to your -mcmodel=large patch - that didn't change
> anything because I already hardcoded that into
> conf/Makefile.common. Your suggested patch is probably the better way
> of doing it - so do consider pushing that anyway (dropping the
> Makefile.common stanza at the same time).
> 
> /
> Leif
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: util/bin2h.c?

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 01.01.2016 09:01, Andrei Borzenkov wrote:
> Do still need it? It is not used since 2 years.
> 
Feel free to remove it
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: enable_progress_indicator

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 06.01.2016 12:27, Andrei Borzenkov wrote:
> Do we really need yet another magic variable? Tests already set
> "grubshell", and in normal case why would one load progress but then
> disable it?
> 
> Why progress gets loaded in the first place? Can we somehow omit it?
> 
emu loads all the modules. And we use emu to generate checksums.h. So
unless we disable progress on it, all of our golden images will have it
which would be bad.
Before this variable there was no way of disabling progress once module
is loaded (I wouldn't recommend unloading any modules currently). So
it's probably good to have some way of disabling it when need be
independently of tests. We may also want to have tests specifically for
progress (we'll have to mock out the timer for this), so it doesn't seem
like grubshell is the right variable to check.
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

ARM*-EFI timers

[Vladimir 'φ-coder/phcoder' Serbinenko]

Hello, all. In my automated tests I found out that on ARM64-EFI sleep 10
actually sleeps for 100s. The culprit is that EFI doesn't call our timer
every 1ms but every 10ms. I propose time1.diff to correct: request EFI
to call us every 10ms and increment timer variable by 10.
For arm64 I propose to use CPU timer instead time2.diff. Can any of ARM
guys comment on this?


time1.diff
Description: application/ext-patch


time2.diff
Description: application/ext-patch


signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: ARM*-EFI timers

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 07.01.2016 19:59, Andrei Borzenkov wrote:
> 07.01.2016 21:08, Vladimir 'φ-coder/phcoder' Serbinenko пишет:
>> Hello, all. In my automated tests I found out that on ARM64-EFI sleep 10
>> actually sleeps for 100s. The culprit is that EFI doesn't call our timer
>> every 1ms but every 10ms. I propose time1.diff to correct: request EFI
>> to call us every 10ms and increment timer variable by 10.
> 
> openSUSE carries similar patch for quite some time. Patch is authored by
> RH. So it can be considered tested in real life.
> 
> https://build.opensuse.org/package/view_file/Base:System/grub2/grub2-arm64-Reduce-timer-event-frequency-by-10.patch?expand=1
> 
Ok, I took RH patch. Now question is about the second patch
>> For arm64 I propose to use CPU timer instead time2.diff. Can any of ARM
>> guys comment on this?
>>
>>
>>
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
> 
> 
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Xen-devel] Uniform commands for booting xen

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 13.11.2015 10:50, Ian Campbell wrote:
> On Fri, 2015-11-13 at 12:04 +0300, Andrei Borzenkov wrote:
>>> How do you express modules other than kernel+initrd in that
>>> scheme, without grub needing to be aware of any new addition we
>>> may find necessary going forward?
>>>
>>
>> Are modules used by Xen self-identifying? Is it enough to simply pass
>> Xen kernel list of binary blobs or Xen kernel must be told what these
>> binary blobs are? If they are self identifying, why arm needs to be
>> passed module type in the first place?
> 
> At first Xen/ARM required the bootloader to identify, but that was since
> identified as causing madness and fixed by having Xen/ARM do as Xen/x86
> does and figure things out for itself, but I failed to communicate this
> clearly and things got implemented on the grub side under the old
> assumptions.
> 
This changes a lot. This removes most of hurdles towards uniformity. Are
you ok with replacing xen_kernel/xen_xsm/... with just xen_module and
dropping type altogether?
Do you think that it makes sense to have xen_initrd in order to have
in-memory initrd concatenation like baremetal counterpart? In either
case we can add it later. I'd rather not have a command than to change
its meaning later.
Jan, does it address your concerns?
> I just replied in more detail about that to Jan's mail, so I won't repeat
> myself further here.
> 
> Ian.
> 



xen.diff
Description: application/ext-patch


signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Xen-devel] Uniform commands for booting xen

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 11.01.2016 15:32, Jan Beulich wrote:
 On 11.01.16 at 15:06,  wrote:
>> On 13.11.2015 10:50, Ian Campbell wrote:
>>> On Fri, 2015-11-13 at 12:04 +0300, Andrei Borzenkov wrote:
> How do you express modules other than kernel+initrd in that
> scheme, without grub needing to be aware of any new addition we
> may find necessary going forward?
>

 Are modules used by Xen self-identifying? Is it enough to simply pass
 Xen kernel list of binary blobs or Xen kernel must be told what these
 binary blobs are? If they are self identifying, why arm needs to be
 passed module type in the first place?
>>>
>>> At first Xen/ARM required the bootloader to identify, but that was since
>>> identified as causing madness and fixed by having Xen/ARM do as Xen/x86
>>> does and figure things out for itself, but I failed to communicate this
>>> clearly and things got implemented on the grub side under the old
>>> assumptions.
>>>
>> This changes a lot. This removes most of hurdles towards uniformity. Are
>> you ok with replacing xen_kernel/xen_xsm/... with just xen_module and
>> dropping type altogether?
>> Do you think that it makes sense to have xen_initrd in order to have
>> in-memory initrd concatenation like baremetal counterpart? In either
>> case we can add it later. I'd rather not have a command than to change
>> its meaning later.
>> Jan, does it address your concerns?
> 
> It improves things a bit, but I'd really like to not see any xen_
> prefixed commands at all in grub2 - after all Xen should just be
> an ordinary multiboot client.
> 
This is true for x86 but on ARM64 the protocol xen expects is quite
different and not really multiboot. How would we avoid xen_ prefixed
commands for ARM64? And when we have them it makes sense to have them on
x86 as well so that the same set of commands works on both arm64 and x86
> Jan
> 
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Xen-devel] Uniform commands for booting xen

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 18.01.2016 11:28, Ian Campbell wrote:
> On Mon, 2016-01-11 at 15:06 +0100, Vladimir 'φ-coder/phcoder' Serbinenko
> wrote:
>> On 13.11.2015 10:50, Ian Campbell wrote:
>>> On Fri, 2015-11-13 at 12:04 +0300, Andrei Borzenkov wrote:
>>>>> How do you express modules other than kernel+initrd in that
>>>>> scheme, without grub needing to be aware of any new addition we
>>>>> may find necessary going forward?
>>>>>
>>>>
>>>> Are modules used by Xen self-identifying? Is it enough to simply pass
>>>> Xen kernel list of binary blobs or Xen kernel must be told what these
>>>> binary blobs are? If they are self identifying, why arm needs to be
>>>> passed module type in the first place?
>>>
>>> At first Xen/ARM required the bootloader to identify, but that was
>>> since
>>> identified as causing madness and fixed by having Xen/ARM do as Xen/x86
>>> does and figure things out for itself, but I failed to communicate this
>>> clearly and things got implemented on the grub side under the old
>>> assumptions.
>>>
>> This changes a lot. This removes most of hurdles towards uniformity. Are
>> you ok with replacing xen_kernel/xen_xsm/... with just xen_module and
>> dropping type altogether?
> 
> So ending up with xen_hypervisor followed by one or more xen_module lines?
> That's fine with me. This bit:
> 
> @@ -203,15 +155,11 @@ prepare_xen_module_params (struct xen_boot_binary 
> *module, void *xen_boot_fdt)
>grub_fdt_add_subnode (xen_boot_fdt, chosen_node, module_name);
>  
>retval = grub_fdt_set_prop (xen_boot_fdt, module_node, "compatible",
> - module->node_info.compat_string,
> - (grub_uint32_t) module->
> - node_info.compat_string_size);
> + "deprecated", sizeof("deprecated") - 1);
> 
> Seems to be changing the compatibility string of hte node to "deprecated",
> which isn't right (or at least won't work). The nodes still need to be
> identified as being modules per http://xenbits.xen.org/docs/unstable/misc/a
> rm/device-tree/booting.txt that means "multiboot,module" (or if you insist
> "xen,multiboot-module").
> 
Changed to "multiboot,module" and committed
>> Do you think that it makes sense to have xen_initrd in order to have
>> in-memory initrd concatenation like baremetal counterpart? In either
>> case we can add it later. I'd rather not have a command than to change
>> its meaning later.
> 
> If it is useful on baremetal (and I can see that it would be) then I think
> it would be useful on Xen too.
> 
> Ian.
> 
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: test command (in-)compatibility

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 08.12.2015 17:36, Andrei Borzenkov wrote:
> `test' in GRUB implicitly assumes `and' operation between consecutive
> terms and does not enforce proper syntax like UNIX (bash) `test' does. Both
> 
> test x y z
> test x = y z = w
> 
> result in error in Linux and are silently accepted by GRUB with
> interpretation
> 
> test x -a y -a z
> test ( x = y ) -a ( z = w )
> 
> I do not have any strong opinion about it; but simply documenting it
> needs the least efforts :)
> 
Documented
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Xen-devel] [BUG] XEN domU crash when PV grub chainloads 32-bit domU grub

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 22.09.2015 10:53, Ian Campbell wrote:
> Hi Vladimir & grub-devel,
> 
> Do you have any thoughts on this issue with i386 pv-grub2?
> 
Is it still an issue? If so I'll try to replicate it. From stack dump I
see that it has jumped to NULL. GRUB has no threads so it's not a race
condition with itself but may be one with some Xen part. An altrnative
possibility is that grub forgets to flush cache at some point in boot
process.
> Thanks, Ian.
> 
> On Mon, 2015-09-21 at 22:03 +0200, Andreas Sundstrom wrote:
>> This is using Debian Jessie and grub 2.02~beta2-22 (with Debian patches
>> applied) and Xen 4.4.1
>>
>> I originally posted a bug report with Debian but got the suggestion to
>> file bugs with upstream as well.
>> Debian bug report:
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=799480
>>
>> Note that my original thought was that this bug probably is within GRUB.
>> But Ian asked me to file a bug with Xen as well, you have to live with
>> the
>> fact that it is centered around GRUB though.
>>
>> Here's the information from my original bug report:
>>
>> Using 64-bit dom0 and 32-bit domU PV (para-virtualized) grub sometimes
>> fail when chainloading the domU's grub. 64-bit domU seem to work 100%
>> of the time.
>>
>> My understanding of the process:
>>
>>  * dom0 launches domU with grub that is loaded from dom0's disk.
>>  * Grub reads config file from memdisk, and then looks for grub binary in
>> domU filesystem.
>>  * If grub is found in domU it then chainloads (multiboot) that grub
>> binary
>> and the domU grub reads grub.cfg and continue booting.
>>  * If grub is not found in domU it reads grub.cfg and continues with
>> boot.
>>
>> It fails at step 3 in my list of the boot process, but sometimes it
>> does work so it may be something like a race condition that causes the
>> problem?
>>
>> A workaround is to not install or rename /boot/xen in domU so that the
>> first grub that is loaded from dom0's disk will not find the grub
>> binary in the domU filesystem and hence continues to read grub.cfg and
>> boot. The drawback of this is of course that the two versions can't
>> differ too much as there are different setups creating grub.cfg and
>> then reading/parsing it at boot time.
>>
>> I am not sure at this point whether this is a problem in XEN or a
>> problem in grub but I compiled the legacy pvgrub that uses some minios
>> from XEN (don't really know much more about it) and when that legacy
>> pvgrub chainloads the domU grub it seems to work 100% of the time. Now
>> the legace pvgrub is not a real alternative as it's not packaged for
>> Debian though.
>>
>> When it fails "xl create vm -c" outputs this:
>> Parsing config from /etc/xen/vm
>> libxl: error: libxl_dom.c:35:libxl__domain_type: unable to get domain
>> type for domid=16
>> Unable to attach console
>> libxl: error: libxl_exec.c:118:libxl_report_child_exitstatus: console
>> child [0] exited with error status 1
>>
>> And "xl dmesg" shows errors like this:
>> (XEN) traps.c:2514:d15 Domain attempted WRMSR c0010201 from
>> 0x to 0x.
>> (XEN) d16:v0: unhandled page fault (ec=0010)
>> (XEN) Pagetable walk from :
>> (XEN) L4[0x000] = 000200256027 049c
>> (XEN) L3[0x000] = 000200255027 049d
>> (XEN) L2[0x000] = 000200251023 04a1
>> (XEN) L1[0x000] =  
>> (XEN) domain_crash_sync called from entry.S: fault at 82d08021feb0
>> compat_create_bounce_frame+0xc6/0xde
>> (XEN) Domain 16 (vcpu#0) crashed on cpu#0:
>> (XEN) [ Xen-4.4.1 x86_64 debug=n Not tainted ]
>> (XEN) CPU: 0
>> (XEN) RIP: e019:[<>]
>> (XEN) RFLAGS: 0246 EM: 1 CONTEXT: pv guest
>> (XEN) rax:  rbx:  rcx: 
>> (XEN) rdx:  rsi: 00499000 rdi: 0080
>> (XEN) rbp: 000a rsp: 005a5ff0 r8: 
>> (XEN) r9:  r10: 83023e9b9000 r11: 83023e9b9000
>> (XEN) r12: 033f3d335bfb r13: 82d080300800 r14: 82d0802ea940
>> (XEN) r15: 83005e819000 cr0: 8005003b cr4: 000506f0
>> (XEN) cr3: 000200b7a000 cr2: 
>> (XEN) ds: e021 es: e021 fs: e021 gs: e021 ss: e021 cs: e019
>> (XEN) Guest stack trace from esp=005a5ff0:
>> (XEN) 0010  0001e019 00010046 0016b38b 0016b38a 0016b389
>> 0016b388
>> (XEN) 0016b387 0016b386 0016b385 0016b384 0016b383 0016b382 0016b381
>> 0016b380
>> (XEN) 0016b37f 0016b37e 0016b37d 0016b37c 0016b37b 0016b37a 0016b379
>> 0016b378
>> (XEN) 0016b377 0016b376 0016b375 0016b374 0016b373 0016b372 0016b371
>> 0016b370
>> (XEN) 0016b36f 0016b36e 0016b36d 0016b36c 0016b36b 0016b36a 0016b369
>> 0016b368
>> (XEN) 0016b367 0016b366 0016b365 0016b364 0016b363 0016b362 0016b361
>> 0016b360
>> (XEN) 0016b35f 0016b35e 0016b35d 0016b35c 0016b35b 0016b35a 0016b359
>> 0016b358
>> (XEN) 0016b357 0016b356 0016b355 0016b354 0016b353

Re: PV-Grub2 works here but doesnt work their...

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 10.09.2015 20:46, Shaun Reitan wrote:
> I posted this to the Xen Mailing Lists too but figured i should post
> here as well. 
>  
> We are experiencing a odd issue after we built grub2 support.  The image
> we built works fine on some hosts and then just hangs on others.
>  
> I built grub2 as follows...
> -
> git clone http://git.savannah.gnu.org/cgit/grub.git
> cd  grub
> wget http://prgmr.com/~srn/grub2/xen-linux16.patch
> patch -p1 < xen-linux16.patch
> ./autogen.sh
> ./configure --target=x86_64 --with-platform=xen --prefix=/opt/grub2
> make
> make install
> export PATH=/opt/grub2/bin:/opt/grub2/sbin:$PATH
>  
> Next I built the image as follows...
> ---
> cat > grub-bootstrap.cfg << EOF
> normal (memdisk)/grub.cfg
> EOF
> cat > grub.cfg << EOF
> for grubcfg in /boot/grub2/grub.cfg /boot/grub2/grub2.cfg
> /boot/grub/grub2.cfg /boot/grub/grub.cfg /grub2/grub2.cfg /grub/grub.cfg
> /etc/grub2.cfg /etc/grub.cfg ; do
>if search -s -f $grubcfg ; then
>   echo "Reading (${root}$grubcfg"
>   configfile $grubcfg
>fi
> done
> EOF
> tar cf memdisk.tar grub.cfg
> /opt/grub2/bin/grub-mkimage -O x86_64-xen -c grub-bootstrap.cfg -m
> memdisk.tar -o grub2-x86_64 /opt/grub2/lib/grub/x86_64-xen/*.mod
>  
> This works fine on some of our hosts, but then just seams to hang on
> others.  When starting the guest i see the following...
>  
> [root@devhostxxx ~]# xm create -c /home/username/vs.config
> Using config file "/home/username/vs.config".
> Started domain username (id=34)
>  [root@devhostxxx ~]#
>  
> Then it just hangs from their.
>  
Are you able to add echo's along the way and look at xen console?
> These hosts are CentOS 6 using the CentOS-Xen RPMS. Hosts are running
> Xen version 4.4.1-8el6 and still using xend with xm commands
>  
>  
> Any idea what may be going on here? Or how i should go about debugging
> this issue?
>  
> --
> Shaun
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH V2] net: fix ipv6 routing

[Vladimir 'φ-coder/phcoder' Serbinenko]

Were Andrey's comments ever adressed? Other than his comments the patch
looks good.
On 20.09.2015 11:50, Andrei Borzenkov wrote:
> 28.08.2015 18:24, Josef Bacik пишет:
>> ipv6 routing in grub2 is broken, we cannot talk to anything outside
>> our local
>> network or anything that doesn't route in our global namespace.  This
>> patch
>> fixes this by doing a couple of things
>>
>> 1) Read the router information off of the router advertisement.  If we
>> have a
>> router lifetime we need to take the source address and create a route
>> from it.
>>
>> 2) Changes the routing stuff slightly to allow you to specify a
>> gateway _and_ an
>> interface.  Since the router advertisements come in on the link local
>> address we
>> need to associate it with the global address on the card.  So when we are
>> processing the router advertisement, either use the SLAAC interface we
>> create
>> and add the route to that interface, or loop through the global
>> addresses we
>> currently have on our interface and associate it with one of those
>> addresses.
>> We need to have a special case here for the default route so that it
>> gets used,
>> we do this by setting the masksize to 0 to mean it encompasses all
>> networks.
>> The routing code will automatically select the best route so if there
>> is a
>> closer match we will use that.
>>
> 
> I think this is OK; minor comments below. We probably want to augment
> net_route to allow both gateway and interface as well.
> 
>> With this patch I can now talk to ipv6 addresses outside of my local
>> network.
>> Thanks,
>>
>> Signed-off-by: Josef Bacik 
>> ---
>> V1->V2:
>> -reworked the route stuff to hold an interface for gateways as well.
>> -fixed the slaac stuff so the route information is separate from the
>> interface
>>   configuration
>>
>>   grub-core/net/bootp.c  |  2 +-
>>   grub-core/net/drivers/ieee1275/ofnet.c |  4 +--
>>   grub-core/net/icmp6.c  | 63
>> +-
>>   grub-core/net/net.c| 40 -
>>   include/grub/net.h | 25 +-
>>   5 files changed, 103 insertions(+), 31 deletions(-)
>>
>> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
>> index 37d1cfa..9fc47bd 100644
>> --- a/grub-core/net/bootp.c
>> +++ b/grub-core/net/bootp.c
>> @@ -83,7 +83,7 @@ parse_dhcp_vendor (const char *name, const void
>> *vend, int limit, int *mask)
>> grub_memcpy (&gw.ipv4, ptr, sizeof (gw.ipv4));
>> rname = grub_xasprintf ("%s:default", name);
>> if (rname)
>> -grub_net_add_route_gw (rname, target, gw);
>> +grub_net_add_route_gw (rname, target, gw, NULL);
>> grub_free (rname);
>>   }
>> break;
>> diff --git a/grub-core/net/drivers/ieee1275/ofnet.c
>> b/grub-core/net/drivers/ieee1275/ofnet.c
>> index eea8e71..d238628 100644
>> --- a/grub-core/net/drivers/ieee1275/ofnet.c
>> +++ b/grub-core/net/drivers/ieee1275/ofnet.c
>> @@ -151,7 +151,7 @@ grub_ieee1275_parse_bootpath (const char *devpath,
>> char *bootpath,
>> grub_net_network_level_address_t client_addr, gateway_addr,
>> subnet_mask;
>> grub_net_link_level_address_t hw_addr;
>> grub_net_interface_flags_t flags = 0;
>> -  struct grub_net_network_level_interface *inter;
>> +  struct grub_net_network_level_interface *inter = NULL;
>>
>> hw_addr.type = GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET;
>>
>> @@ -221,7 +221,7 @@ grub_ieee1275_parse_bootpath (const char *devpath,
>> char *bootpath,
>> target.ipv4.masksize = 0;
>> rname = grub_xasprintf ("%s:default", ((*card)->name));
>> if (rname)
>> -grub_net_add_route_gw (rname, target, gateway_addr);
>> +grub_net_add_route_gw (rname, target, gateway_addr, inter);
>> else
>>   return grub_errno;
>>   }
>> diff --git a/grub-core/net/icmp6.c b/grub-core/net/icmp6.c
>> index 7953e68..79a4a30 100644
>> --- a/grub-core/net/icmp6.c
>> +++ b/grub-core/net/icmp6.c
>> @@ -115,6 +115,7 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
>>   grub_uint8_t ttl)
>>   {
>> struct icmp_header *icmph;
>> +  struct grub_net_network_level_interface *orig_inf = inf;
>> grub_err_t err;
>> grub_uint16_t checksum;
>>
>> @@ -345,8 +346,25 @@ grub_net_recv_icmp6_packet (struct grub_net_buff
>> *nb,
>> {
>>   grub_uint8_t *ptr;
>>   struct option_header *ohdr;
>> +struct router_adv *radv;
>> +struct grub_net_network_level_interface *route_inf = NULL;
>> +int default_route = 0;
>>   if (icmph->code)
>> break;
>> +radv = (void *)nb->data;
>> +if (grub_be_to_cpu16 (radv->router_lifetime) > 0)
> 
> This should come after grub_netbuff_pull error check to make sure we get
> ohis struct.
> 
> 
>> +  {
>> +struct grub_net_route *route;
>> +
>> +FOR_NET_ROUTES (route)
>> +{
>> +  if (!grub_memcmp (&route->gw, source, sizeof (rout

Re: [Xen-devel] [BUG] XEN domU crash when PV grub chainloads 32-bit domU grub

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 22.01.2016 14:01, Andrew Cooper wrote:
> On 22/01/16 12:56, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> On 22.09.2015 10:53, Ian Campbell wrote:
>>> Hi Vladimir & grub-devel,
>>>
>>> Do you have any thoughts on this issue with i386 pv-grub2?
>>>
>> Is it still an issue? If so I'll try to replicate it. From stack dump I
>> see that it has jumped to NULL. GRUB has no threads so it's not a race
>> condition with itself but may be one with some Xen part. An altrnative
>> possibility is that grub forgets to flush cache at some point in boot
>> process.
> 
> Looks like GRUB doesn't have a traptable registered with Xen (the PV
> equivalent of the IDT).
> 
> First, Xen tried to inject a #GP fault and found that the entry EIP was
> at 0 (which is sadly the default if nothing is specified).  It then took
> a pagefault while attempting to inject the #GP, and crashed the domain.
> 
Do you have a link how to add one? We can put a catch-stacktrace-abort
on it.
> ~Andrew
> 
>>> Thanks, Ian.
>>>
>>> On Mon, 2015-09-21 at 22:03 +0200, Andreas Sundstrom wrote:
>>>> This is using Debian Jessie and grub 2.02~beta2-22 (with Debian patches
>>>> applied) and Xen 4.4.1
>>>>
>>>> I originally posted a bug report with Debian but got the suggestion to
>>>> file bugs with upstream as well.
>>>> Debian bug report:
>>>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=799480
>>>>
>>>> Note that my original thought was that this bug probably is within GRUB.
>>>> But Ian asked me to file a bug with Xen as well, you have to live with
>>>> the
>>>> fact that it is centered around GRUB though.
>>>>
>>>> Here's the information from my original bug report:
>>>>
>>>> Using 64-bit dom0 and 32-bit domU PV (para-virtualized) grub sometimes
>>>> fail when chainloading the domU's grub. 64-bit domU seem to work 100%
>>>> of the time.
>>>>
>>>> My understanding of the process:
>>>>
>>>>  * dom0 launches domU with grub that is loaded from dom0's disk.
>>>>  * Grub reads config file from memdisk, and then looks for grub binary in
>>>> domU filesystem.
>>>>  * If grub is found in domU it then chainloads (multiboot) that grub
>>>> binary
>>>> and the domU grub reads grub.cfg and continue booting.
>>>>  * If grub is not found in domU it reads grub.cfg and continues with
>>>> boot.
>>>>
>>>> It fails at step 3 in my list of the boot process, but sometimes it
>>>> does work so it may be something like a race condition that causes the
>>>> problem?
>>>>
>>>> A workaround is to not install or rename /boot/xen in domU so that the
>>>> first grub that is loaded from dom0's disk will not find the grub
>>>> binary in the domU filesystem and hence continues to read grub.cfg and
>>>> boot. The drawback of this is of course that the two versions can't
>>>> differ too much as there are different setups creating grub.cfg and
>>>> then reading/parsing it at boot time.
>>>>
>>>> I am not sure at this point whether this is a problem in XEN or a
>>>> problem in grub but I compiled the legacy pvgrub that uses some minios
>>>> from XEN (don't really know much more about it) and when that legacy
>>>> pvgrub chainloads the domU grub it seems to work 100% of the time. Now
>>>> the legace pvgrub is not a real alternative as it's not packaged for
>>>> Debian though.
>>>>
>>>> When it fails "xl create vm -c" outputs this:
>>>> Parsing config from /etc/xen/vm
>>>> libxl: error: libxl_dom.c:35:libxl__domain_type: unable to get domain
>>>> type for domid=16
>>>> Unable to attach console
>>>> libxl: error: libxl_exec.c:118:libxl_report_child_exitstatus: console
>>>> child [0] exited with error status 1
>>>>
>>>> And "xl dmesg" shows errors like this:
>>>> (XEN) traps.c:2514:d15 Domain attempted WRMSR c0010201 from
>>>> 0x to 0x.
>>>> (XEN) d16:v0: unhandled page fault (ec=0010)
>>>> (XEN) Pagetable walk from :
>>>> (XEN) L4[0x000] = 000200256027 049c
>>>> (XEN) L3[0x000] = 000200255027 049d
>>>> (XEN) L2[0x000] =

Re: [PATCH] arm64: build with -mcmodel=large

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 22.01.2016 03:14, Colin Watson wrote:
> On Fri, Dec 25, 2015 at 06:18:55PM +, Leif Lindholm wrote:
>> So, it seems this toolchain generates the HI21/LO12 relocation combo:
>> - R_AARCH64_ADR_PREL_PG_HI21/R_AARCH64_ADR_PREL_PG_HI21_NC
>> - R_AARCH64_LDST16_ABS_LO12_NC
>> - R_AARCH64_LDST32_ABS_LO12_NC
>> - R_AARCH64_LDST64_ABS_LO12_NC
>> - R_AARCH64_LDST128_ABS_LO12_NC
>>
>> So I'll implement support for these.
> 
> I found a temporary workaround for this via
> https://bugs.launchpad.net/bugs/1533009, which refers to
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63304; given that set of
> clues, I've confirmed that TARGET_CFLAGS='-Os
> -mpc-relative-literal-loads' fixes my build for the time being.
> 
> This isn't necessarily a good solution for upstream, because only
> certain versions of GCC support it (although perhaps we could detect it
> in configure.ac until such time as appropriate relocation support is
> added?), but I'm mentioning it here in case any other distributors have
> the same problem.
> 
I have implemented and committed the support for needed relocations



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] [RFC] Add exitcode support

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 18.08.2015 21:17, Ben Hildred wrote:
> Let's assume for a minute that I have compiled grub as a multiboot image
> and have called it from another bootloader, say iPXE.Now iPXE assumes
> that any false return is an error. What happens when grub returns with
> exit next, does iPXE get a true or false? What about exit fred where
> fred is not defined by any platform? What if I do an exit config which
> is only defined for coreboot?
Neither multiboot nor coreboot have any return semantics. The situation
with current platforms is as follows:
No return/exit semantics at all or machine shutdown:
i386_coreboot, i386_qemu, i386_multiboot, mips_qemu_mips, mips_loongson
no-args exit:
*-ieee1275, i386-pc, mips-arc
Xen semantics (crash vs poweroff):
*-xen
EFI semantics:
*-efi
Unix-like semantics:
arm-uboot, emu

Emu is of no real interest and I have no idea what Uboot does with
return code but I suppose nothing.

This leaves us only with xen and EFI semantics. Xen is enough of outlier
to handle it separately. So only EFI is remaining.

On i386-pc the default behaviour of exit is to try next boot entry. EFI
should probably do the same. What is the current behaviour of grub_exit
and what is the value in returning EFI_SUCCESS ?
Can we have returncode-aware command to be EFI-specific?






signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 3/6] xen: factor out allocation of page tables into separate function

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 11.02.2016 13:53, Juergen Gross wrote:
> On 11/02/16 13:27, Daniel Kiper wrote:
>> On Thu, Feb 11, 2016 at 08:53:23AM +0100, Juergen Gross wrote:
>>> Do the allocation of page tables in a separate function. This will
>>> allow to do the allocation at different times of the boot preparations
>>> depending on the features the kernel is supporting.
>>>
>>> Signed-off-by: Juergen Gross 
>>> ---
>>>  grub-core/loader/i386/xen.c | 82 
>>> -
>>>  1 file changed, 51 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
>>> index e48cc3f..65cec27 100644
>>> --- a/grub-core/loader/i386/xen.c
>>> +++ b/grub-core/loader/i386/xen.c
>>> @@ -56,6 +56,9 @@ static struct grub_relocator_xen_state state;
>>>  static grub_xen_mfn_t *virt_mfn_list;
>>>  static struct start_info *virt_start_info;
>>>  static grub_xen_mfn_t console_pfn;
>>> +static grub_uint64_t *virt_pgtable;
>>> +static grub_uint64_t pgtbl_start;
>>> +static grub_uint64_t pgtbl_end;
>>
>> Same as in patches #1 and #2.
> 
> Yep.
> 
>>
>>>  #define PAGE_SIZE 4096
>>>  #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
>>> @@ -106,17 +109,17 @@ get_pgtable_size (grub_uint64_t total_pages, 
>>> grub_uint64_t virt_base)
>>>
>>>  static void
>>>  generate_page_table (grub_uint64_t *where, grub_uint64_t paging_start,
>>> -grub_uint64_t total_pages, grub_uint64_t virt_base,
>>> -grub_xen_mfn_t *mfn_list)
>>> +grub_uint64_t paging_end, grub_uint64_t total_pages,
>>> +grub_uint64_t virt_base, grub_xen_mfn_t *mfn_list)
>>>  {
>>>if (!virt_base)
>>> -total_pages++;
>>> +paging_end++;
>>>
>>>grub_uint64_t lx[NUMBER_OF_LEVELS], lxs[NUMBER_OF_LEVELS];
>>>grub_uint64_t nlx, nls, sz = 0;
>>>int l;
>>>
>>> -  nlx = total_pages;
>>> +  nlx = paging_end;
>>>nls = virt_base >> PAGE_SHIFT;
>>>for (l = 0; l < NUMBER_OF_LEVELS; l++)
>>>  {
>>> @@ -160,7 +163,7 @@ generate_page_table (grub_uint64_t *where, 
>>> grub_uint64_t paging_start,
>>>if (pr)
>>>  pg += POINTERS_PER_PAGE;
>>>
>>> -  for (j = 0; j < total_pages; j++)
>>> +  for (j = 0; j < paging_end; j++)
>>>  {
>>>if (j >= paging_start && j < lp)
>>> pg[j + lxs[0]] = page2offset (mfn_list[j]) | 5;
>>> @@ -261,24 +264,12 @@ grub_xen_special_alloc (void)
>>>  }
>>>
>>>  static grub_err_t
>>> -grub_xen_boot (void)
>>> +grub_xen_pt_alloc (void)
>>>  {
>>>grub_relocator_chunk_t ch;
>>>grub_err_t err;
>>>grub_uint64_t nr_info_pages;
>>>grub_uint64_t nr_pages, nr_pt_pages, nr_need_pages;
>>> -  struct gnttab_set_version gnttab_setver;
>>> -  grub_size_t i;
>>> -
>>> -  if (grub_xen_n_allocated_shared_pages)
>>> -return grub_error (GRUB_ERR_BUG, "active grants");
>>> -
>>> -  err = grub_xen_p2m_alloc ();
>>> -  if (err)
>>> -return err;
>>> -  err = grub_xen_special_alloc ();
>>> -  if (err)
>>> -return err;
>>>
>>>next_start.pt_base = max_addr + xen_inf.virt_base;
>>>state.paging_start = max_addr >> PAGE_SHIFT;
>>> @@ -298,30 +289,59 @@ grub_xen_boot (void)
>>>nr_pages = nr_need_pages;
>>>  }
>>>
>>> -  grub_dprintf ("xen", "bootstrap domain %llx+%llx\n",
>>> -   (unsigned long long) xen_inf.virt_base,
>>> -   (unsigned long long) page2offset (nr_pages));
>>> -
>>>err = grub_relocator_alloc_chunk_addr (relocator, &ch,
>>>  max_addr, page2offset (nr_pt_pages));
>>>if (err)
>>>  return err;
>>>
>>> +  virt_pgtable = get_virtual_current_address (ch);
>>> +  pgtbl_start = max_addr >> PAGE_SHIFT;
>>> +  max_addr += page2offset (nr_pt_pages);
>>> +  state.stack = max_addr + STACK_SIZE + xen_inf.virt_base;
>>> +  state.paging_size = nr_pt_pages;
>>> +  next_start.nr_pt_frames = nr_pt_pages;
>>> +  max_addr = page2offset (nr_pages);
>>> +  pgtbl_end = nr_pages;
>>> +
>>> +  return GRUB_ERR_NONE;
>>> +}
>>> +
>>> +static grub_err_t
>>> +grub_xen_boot (void)
>>> +{
>>> +  grub_err_t err;
>>> +  grub_uint64_t nr_pages;
>>> +  struct gnttab_set_version gnttab_setver;
>>> +  grub_size_t i;
>>> +
>>> +  if (grub_xen_n_allocated_shared_pages)
>>> +return grub_error (GRUB_ERR_BUG, "active grants");
>>> +
>>> +  err = grub_xen_p2m_alloc ();
>>> +  if (err)
>>> +return err;
>>> +  err = grub_xen_special_alloc ();
>>> +  if (err)
>>> +return err;
>>> +  err = grub_xen_pt_alloc ();
>>> +  if (err)
>>> +return err;
>>
>> Should not we free memory allocated by grub_xen_p2m_alloc() and
>> grub_xen_special_alloc() if grub_xen_pt_alloc() failed?
> 
> Hmm, why? If I don't miss anything freeing memory in case of error isn't
> done anywhere (at least not in this source file).
> 
If we don't it's a bug and not an excuse to continue doing bad things
> Juergen
> 
> .
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing li

Re: [PATCH v2 4/6] xen: add capability to load initrd outside of initial mapping

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 11.02.2016 15:13, Juergen Gross wrote:
> On 11/02/16 13:33, Daniel Kiper wrote:
>> On Thu, Feb 11, 2016 at 08:53:24AM +0100, Juergen Gross wrote:
>>> Modern pvops linux kernels support an initrd not covered by the initial
>>> mapping. This capability is flagged by an elf-note.
>>>
>>> In case the elf-note is set by the kernel don't place the initrd into
>>> the initial mapping. This will allow to load larger initrds and/or
>>> support domains with larger memory, as the initial mapping is limited
>>> to 2GB and it is containing the p2m list.
>>>
>>> Signed-off-by: Juergen Gross 
>>> ---
>>>  grub-core/loader/i386/xen.c| 56
++
>>>  grub-core/loader/i386/xen_fileXX.c |  3 ++
>>>  include/grub/xen_file.h|  1 +
>>>  3 files changed, 49 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
>>> index 65cec27..0f41048 100644
>>> --- a/grub-core/loader/i386/xen.c
>>> +++ b/grub-core/loader/i386/xen.c
>>> @@ -307,15 +307,14 @@ grub_xen_pt_alloc (void)
>>>  }
>>>
>>>  static grub_err_t
>>> -grub_xen_boot (void)
>>> +grub_xen_alloc_end (void)
>>>  {
>>>grub_err_t err;
>>> -  grub_uint64_t nr_pages;
>>> -  struct gnttab_set_version gnttab_setver;
>>> -  grub_size_t i;
>>> +  static int called = 0;
>>>
>>> -  if (grub_xen_n_allocated_shared_pages)
>>> -return grub_error (GRUB_ERR_BUG, "active grants");
>>> +  if (called)
>>> +return GRUB_ERR_NONE;
>>
>> Why?
>
> Did you look at the function? grub_xen_alloc_end() (some parts of the
> original grub_xen_boot()) is new and may be called multiple times. It
> was much easier to just call it and let it do what must be done only
> at the first time called.
>
What if a boot fails and then fallback kernel is loaded?
>>> +  if (grub_xen_n_allocated_shared_pages)
>>> +return grub_error (GRUB_ERR_BUG, "active grants");
>>
>> Why?
>
> That's how it has been before. git just decided to generate the diff
> that way.
>
This is also needed to avoid passing control when some drivers are still
active
>>> +   case 16:
>>
>> Could you define this a constant and use it here?
>
> This would be the only case with a constant. All others are numeric
> as well.
>
I'm ok with not insisisting on using constants given current state but
in general constants are preferable (yes, xen code isn't always clean)






signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] efidisk: prevent errors from diskfilter scan of removable drives

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 05.02.2016 17:56, Andrei Borzenkov wrote:
> Map EFI_NO_MEDIA to GRUB_ERR_OUT_OF_RANGE that is ignored by diskfilter. This
> actually matches pretty close (we obviously attempt to read outside of media)
> and avoids adding more error codes.
> 
> This affects only internally initiated scans. If read/write from removable is
> explicitly requested, we still return an error and text explanation is more
> clear for user than generic error.
> 
> Reported and tested by Andreas Loew 
> 
I feel like we should be fixing diskfilter. Consider another case: dead
disk dangling on cable and returning mostly I/O errors
> ---
>  grub-core/disk/efi/efidisk.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> index 1c00e3e..ea75344 100644
> --- a/grub-core/disk/efi/efidisk.c
> +++ b/grub-core/disk/efi/efidisk.c
> @@ -547,7 +547,9 @@ grub_efidisk_read (struct grub_disk *disk, 
> grub_disk_addr_t sector,
>  
>status = grub_efidisk_readwrite (disk, sector, size, buf, 0);
>  
> -  if (status != GRUB_EFI_SUCCESS)
> +  if (status == GRUB_EFI_NO_MEDIA)
> +return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("no media in `%s'", 
> disk->name));
> +  else if (status != GRUB_EFI_SUCCESS)
>  return grub_error (GRUB_ERR_READ_ERROR,
>  N_("failure reading sector 0x%llx from `%s'"),
>  (unsigned long long) sector,
> @@ -568,7 +570,9 @@ grub_efidisk_write (struct grub_disk *disk, 
> grub_disk_addr_t sector,
>  
>status = grub_efidisk_readwrite (disk, sector, size, (char *) buf, 1);
>  
> -  if (status != GRUB_EFI_SUCCESS)
> +  if (status == GRUB_EFI_NO_MEDIA)
> +return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("no media in `%s'", 
> disk->name));
> +  else if (status != GRUB_EFI_SUCCESS)
>  return grub_error (GRUB_ERR_WRITE_ERROR,
>  N_("failure writing sector 0x%llx to `%s'"),
>  (unsigned long long) sector, disk->name);
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel