2.02~beta3 release

2016-02-28 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Hello, all. I've just released 2.02~beta3. The goal of this release is
to chase bugs so that we can move to 2.02 release. So from now on if you
think that your patch should be included in 2.02 rather than in 2.03,
please add [2.02] to the beginning of your email subject

The tarball is available at
http://alpha.gnu.org/gnu/grub/grub-2.02~beta3.tar.xz
and signature at
http://alpha.gnu.org/gnu/grub/grub-2.02~beta3.tar.xz.sig

Signed with following fingerprint:
 E53D 497F 3FA4 2AD8 C9B4  D1E8 35A9 3B74 E82E 4209

It's also available as a signed tag grub-2.02-beta3 in official git
repository.

If you don't have xz support alternatively you may consider files
http://alpha.gnu.org/gnu/grub/grub-2.02~beta3.tar.xz
and signature at
http://alpha.gnu.org/gnu/grub/grub-2.02~beta3.tar.xz.sig






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


Re: Grub translation

2016-02-28 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 28.02.2016 14:32, Baadur Jobava wrote:
> Hello, is there some resource out there on how to get into Grub
> localization?
> 
> I'd like to look at translating Grub to Romanian.
> 
It's managed by Translation project.
https://translationproject.org/html/welcome.html.
> Thanks!
> 
> Jobava
> 
> 
> 
> ___
> 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] efidisk: Respect block_io_protocol buffer alignment

2016-02-28 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 18.02.2016 19:05, Leif Lindholm wrote:
> Returned from the OpenProtocol operation, the grub_efi_block_io_media
> structure contains the io_align field, specifying the minimum alignment
> required for buffers used in any data transfers with the device.
> 
> Make grub_efidisk_readwrite() allocate a temporary buffer, aligned to
> this boundary, if the buffer passed to it does not already meet the
> requirements.
> 
> Also sanity check the io_align field in grub_efidisk_open() for
> power-of-two-ness and bail if invalid.
> 
I have committed this variant to make tests pass, so that I can release
beta3. Optimisations can go in separately after this
> Reported-by: Jeremy Linton 
> ---
> 
> v4 - Improved alignment checking as suggested by Vladimir, and moved
>  the error exit as requested by Andrei.
> 
>  grub-core/disk/efi/efidisk.c | 48 
> +---
>  1 file changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> index 1c00e3e..e4f4c25 100644
> --- a/grub-core/disk/efi/efidisk.c
> +++ b/grub-core/disk/efi/efidisk.c
> @@ -493,8 +493,15 @@ grub_efidisk_open (const char *name, struct grub_disk 
> *disk)
>m = d->block_io->media;
>/* FIXME: Probably it is better to store the block size in the disk,
>   and total sectors should be replaced with total blocks.  */
> -  grub_dprintf ("efidisk", "m = %p, last block = %llx, block size = %x\n",
> - m, (unsigned long long) m->last_block, m->block_size);
> +  grub_dprintf ("efidisk",
> + "m = %p, last block = %llx, block size = %x, io align = %x\n",
> + m, (unsigned long long) m->last_block, m->block_size,
> + m->io_align);
> +
> +  /* Ensure required buffer alignment is a power of two (or is zero). */
> +  if (m->io_align & (m->io_align - 1))
> +return grub_error (GRUB_ERR_IO, "invalid buffer alignment %d", 
> m->io_align);
> +
>disk->total_sectors = m->last_block + 1;
>/* Don't increase this value due to bug in some EFI.  */
>disk->max_agglomerate = 0xa >> (GRUB_DISK_CACHE_BITS + 
> GRUB_DISK_SECTOR_BITS);
> @@ -524,15 +531,42 @@ grub_efidisk_readwrite (struct grub_disk *disk, 
> grub_disk_addr_t sector,
>  {
>struct grub_efidisk_data *d;
>grub_efi_block_io_t *bio;
> +  grub_efi_status_t status;
> +  grub_size_t io_align, num_bytes;
> +  char *aligned_buf;
>  
>d = disk->data;
>bio = d->block_io;
>  
> -  return efi_call_5 ((wr ? bio->write_blocks : bio->read_blocks), bio,
> -  bio->media->media_id,
> -  (grub_efi_uint64_t) sector,
> -  (grub_efi_uintn_t) size << disk->log_sector_size,
> -  buf);
> +  /* Set alignment to 1 if 0 specified */
> +  io_align = bio->media->io_align ? bio->media->io_align : 1;
> +  num_bytes = size << disk->log_sector_size;
> +
> +  if ((grub_addr_t) buf & (io_align - 1))
> +{
> +  aligned_buf = grub_memalign (io_align, num_bytes);
> +  if (! aligned_buf)
> + return GRUB_EFI_OUT_OF_RESOURCES;
> +  if (wr)
> + grub_memcpy (aligned_buf, buf, num_bytes);
> +}
> +  else
> +{
> +  aligned_buf = buf;
> +}
> +
> +  status =  efi_call_5 ((wr ? bio->write_blocks : bio->read_blocks), bio,
> + bio->media->media_id, (grub_efi_uint64_t) sector,
> + (grub_efi_uintn_t) num_bytes, aligned_buf);
> +
> +  if ((grub_addr_t) buf & (io_align - 1))
> +{
> +  if (!wr)
> + grub_memcpy (buf, aligned_buf, num_bytes);
> +  grub_free (aligned_buf);
> +}
> +
> +  return status;
>  }
>  
>  static grub_err_t
> 




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


Re: How to chainload grub on different platforms?

2016-02-26 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 24.02.2016 18:14, Andrei Borzenkov wrote:
> On x86 we can use multiboot (on in exceptional case chainload boot block).
> 
> On EFI we can use chainloader to launch EFI binary.
> 
> What can we do on PowerPC? SPARC? ARM? MIPS?
> 
> I think we really need some generic platform-independent interface to
> load another grub binary.
> 
Currently we have:
pc: multiboot
*-efi: chainloader
i386-multiboot: multiboot
*-coreboot: chainloader
*-xen: linux
mips-loongson: linux
arm-uboot: linux
Nothing:
*-ieee1275
mips(el)-arc

Nothing, flash-only targets:
i386-qemu
mips-qemu_mips


> ___
> 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 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

2016-02-26 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 26.02.2016 12:15, Fu Wei wrote:
> Hi Andrei,
> 
> On 26 February 2016 at 18:50, Andrei Borzenkov  wrote:
>> On Fri, Feb 26, 2016 at 8:59 AM, Fu Wei  wrote:
>>> +@subsection xen_module
>>>
>>> -@deffn Command xen_linux file [arguments]
>>> -Load a dom0 kernel image for xen hypervisor at the booting process of 
>>> xen.
>>> +@deffn Command xen_module [--nounzip] file [arguments]
>>> +Load a module for xen hypervisor at the booting process of xen.
>>> The rest of the line is passed verbatim as the module command line.
>>> +Each module will be identified by the order in which the modules are 
>>> added.
>>> +The 1st module: dom0 kernel image
>>> +The 2nd module: dom0 ramdisk
>>> +All subsequent modules: UNKNOW
>>> @end deffn
>>
>> Hmm ... from previous discussion I gathered that Xen can detect module
>> type. What if there is no initrd for dom0? How can subsequent modules be
>
> Now , Xen detect module type by the order. (at least on ARM64).
> I think i386 is using Multiboot(2) protocol, so maybe this order is
> nothing to do with i386.
>

 Then we have obvious problem with your XSM patch 
 (http://savannah.gnu.org/bugs/?43420) - XSM may land as the first module. 
 That's actually something to solve on Xen side I think. It's just that so 
 far we had just kernel and initrd, so that was non issue.
>>>
>>> Oh, did you mean Wei Liu's patch?
>>>
>>> I guess XSM may land as the third module (or the module after linux
>>> kernel, if you don't have initrd)
>>>
>>> Yes, agree. (That's actually something to solve on Xen side)
>>>
>>> I guess xen can get xsm from a special initrd. so for now there is not
>>> big problem on xsm.
>>>
>>> Please correct me if I misunderstand something. :-)
>>>
>>> Thanks!
>>>
>>> Back to this patch, is that OK for you, or any suggestion?  Thanks !
>>>
>>
>> Yes, as this is dedicated Xen loader we should document this mandatory
>> order - first module must be kernel image, second module must be
>> initrd. I do not think we need to mention possibility to load more
>> than two modules until there is clear understanding how it can be done
>> without initrd.
> 
> Great thanks for your review, I have updated and sent the v3 patchset,
> Hope I understood your suggestion correctly, Please check.  :-)
> 
Your patches look fine. Let's wait for Andrei's opinion but I'm leaning
to commit them
> 
> 




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


Re: OBP available region patch fails with clang

2016-02-24 Thread Vladimir 'φ-coder/phcoder' Serbinenko

On 24.02.2016 18:35, Andrei Borzenkov wrote:
> kern/ieee1275/mmap.c:52:22: error: comparison of integers of different
> signs: 'grub_ssize_t'
>   (aka 'int') and 'unsigned int' [-Werror,-Wsign-compare]
>   if (available_size > sizeof (available))
>   ~~ ^ ~~
> 1 error generated.
> 
> While we can of course cast to (grub_ssize_t) sizeof (available) -
> should not we check for available_size < 0 as well?
> 
I have fixed it locally but forgot to push
> 
> ___
> 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: aarch64 clang build failure

2016-02-24 Thread Vladimir 'φ-coder/phcoder' Serbinenko

On 24.02.2016 19:25, Andrei Borzenkov wrote:
> TARGET_OBJ2ELF= sh genmod.sh moddep.lst disk.module
> build-grub-module-verifier disk.mod
> build-grub-module-verifier: error: unsupported relocation 0x10d.
> 
https://llvm.org/bugs/show_bug.cgi?id=26030



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


[PATCH] U-Boot self-relocatable images

2016-02-23 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Currently to compile arm-uboot port for a different platform you need to
relink it at new address with modifying source code. This set of patches
changes to U-boot in-place loading and images that relocate themselves.
I'd be interested in opinion of ARM guys
From 80f8cfe29ace49167c1ad6439ddd15519b0a5455 Mon Sep 17 00:00:00 2001
From: Vladimir Serbinenko 
Date: Thu, 18 Feb 2016 20:26:44 +0100
Subject: [PATCH 1/6] mkimage.c: Split into separate files.

util/grub-mkimagexx.c is included in a special way into mkimage.c.
Interoperation between defines makes this very tricky. Instead
just have a clean interface and compile util/grub-mkimage*.c separately
from mkimage.c
---
 Makefile.util.def   |  10 ++
 include/grub/util/mkimage.h | 170 +++
 util/grub-mkimage32.c   |  22 
 util/grub-mkimage64.c   |  22 
 util/grub-mkimagexx.c   | 246 +++
 util/mkimage.c  | 273 ++--
 6 files changed, 405 insertions(+), 338 deletions(-)
 create mode 100644 include/grub/util/mkimage.h
 create mode 100644 util/grub-mkimage32.c
 create mode 100644 util/grub-mkimage64.c

diff --git a/Makefile.util.def b/Makefile.util.def
index db7e8ec..ed9b4c6 100644
--- a/Makefile.util.def
+++ b/Makefile.util.def
@@ -172,6 +172,8 @@ program = {
 
   common = util/grub-mkimage.c;
   common = util/mkimage.c;
+  common = util/grub-mkimage32.c;
+  common = util/grub-mkimage64.c;
   common = util/resolve.c;
   common = grub-core/kern/emu/argp_common.c;
   common = grub-core/osdep/init.c;
@@ -510,6 +512,8 @@ program = {
   common = util/render-label.c;
   common = util/glue-efi.c;
   common = util/mkimage.c;
+  common = util/grub-mkimage32.c;
+  common = util/grub-mkimage64.c;
   common = util/grub-install-common.c;
   common = util/setup_bios.c;
   common = util/setup_sparc.c;
@@ -552,6 +556,8 @@ program = {
   common = util/render-label.c;
   common = util/glue-efi.c;
   common = util/mkimage.c;
+  common = util/grub-mkimage32.c;
+  common = util/grub-mkimage64.c;
   common = util/grub-install-common.c;
   common = util/setup_bios.c;
   common = util/setup_sparc.c;
@@ -595,6 +601,8 @@ program = {
   common = util/grub-install.c;
   common = util/probe.c;
   common = util/mkimage.c;
+  common = util/grub-mkimage32.c;
+  common = util/grub-mkimage64.c;
   common = util/grub-install-common.c;
   common = util/setup_bios.c;
   common = util/setup_sparc.c;
@@ -632,6 +640,8 @@ program = {
   common = util/grub-mknetdir.c;
 
   common = util/mkimage.c;
+  common = util/grub-mkimage32.c;
+  common = util/grub-mkimage64.c;
   common = util/grub-install-common.c;
   common = util/setup_bios.c;
   common = util/setup_sparc.c;
diff --git a/include/grub/util/mkimage.h b/include/grub/util/mkimage.h
new file mode 100644
index 000..564adbc
--- /dev/null
+++ b/include/grub/util/mkimage.h
@@ -0,0 +1,170 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2013  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 .
+ */
+
+#ifndef GRUB_UTIL_MKIMAGE_HEADER
+#define GRUB_UTIL_MKIMAGE_HEADER	1
+
+/* Private header. Use only in mkimage-related sources.  */
+char *
+grub_mkimage_load_image32 (const char *kernel_path, size_t *exec_size, 
+			   size_t *kernel_sz, size_t *bss_size,
+			   size_t total_module_size, grub_uint64_t *start,
+			   void **reloc_section, size_t *reloc_size,
+			   size_t *align,
+			   const struct grub_install_image_target_desc *image_target);
+char *
+grub_mkimage_load_image64 (const char *kernel_path, size_t *exec_size, 
+			   size_t *kernel_sz, size_t *bss_size,
+			   size_t total_module_size, grub_uint64_t *start,
+			   void **reloc_section, size_t *reloc_size,
+			   size_t *align,
+			   const struct grub_install_image_target_desc *image_target);
+void
+grub_mkimage_generate_elf32 (const struct grub_install_image_target_desc *image_target,
+			 int note, char **core_img, size_t *core_size,
+			 Elf32_Addr target_addr, grub_size_t align,
+			 size_t kernel_size, size_t bss_size);
+void
+grub_mkimage_generate_elf64 (const struct grub_install_image_target_desc *image_target,
+			 int note, char **core_img, size_t *core_size,
+			 Elf64_Addr target_addr, grub_size_t align,
+			 size_t kernel_size, size_t bss_size);
+
+struct 

Re: [PATCH] efidisk: Respect block_io_protocol buffer alignment

2016-02-18 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 18.02.2016 16:00, Leif Lindholm wrote:
> Returned from the OpenProtocol operation, the grub_efi_block_io_media
> structure contains the io_align field, specifying the minimum alignment
> required for buffers used in any data transfers with the device.
> 
> Make grub_efidisk_readwrite() allocate a temporary buffer, aligned to
> this boundary, if the buffer passed to it does not already meet the
> requirements.
> 
> Also sanity check the io_align field in grub_efidisk_open() for
> power-of-two-ness and bail if invalid.
> 
> Reported-by: Jeremy Linton 
> ---
> 
> v3 - Fixed up types and added a check on the io_align field, as per
>  Andrei's comments.
> 
>  grub-core/disk/efi/efidisk.c | 53 
> ++--
>  1 file changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> index 1c00e3e..c1afbe4 100644
> --- a/grub-core/disk/efi/efidisk.c
> +++ b/grub-core/disk/efi/efidisk.c
> @@ -463,6 +463,7 @@ grub_efidisk_open (const char *name, struct grub_disk 
> *disk)
>int num;
>struct grub_efidisk_data *d = 0;
>grub_efi_block_io_media_t *m;
> +  unsigned long i = 0;
>  
>grub_dprintf ("efidisk", "opening %s\n", name);
>  
> @@ -491,10 +492,21 @@ grub_efidisk_open (const char *name, struct grub_disk 
> *disk)
>  
>disk->id = ((num << GRUB_CHAR_BIT) | name[0]);
>m = d->block_io->media;
> +  /* Ensure required buffer alignment is a power of two (or is zero). */
> +  do
> +{
> +  if ((m->io_align & (1UL << i)) && (m->io_align & ~(1UL << i)))
> + return grub_error (GRUB_ERR_IO, "invalid buffer alignment %d",
> +m->io_align);
> +}
> +  while (++i < (sizeof (grub_addr_t) * 8));
> +
if (m->io_align & (m->io_align - 1)) { ...error handling... }

>/* FIXME: Probably it is better to store the block size in the disk,
>   and total sectors should be replaced with total blocks.  */
> -  grub_dprintf ("efidisk", "m = %p, last block = %llx, block size = %x\n",
> - m, (unsigned long long) m->last_block, m->block_size);
> +  grub_dprintf ("efidisk",
> + "m = %p, last block = %llx, block size = %x, io align = %x\n",
> + m, (unsigned long long) m->last_block, m->block_size,
> + m->io_align);
>disk->total_sectors = m->last_block + 1;
>/* Don't increase this value due to bug in some EFI.  */
>disk->max_agglomerate = 0xa >> (GRUB_DISK_CACHE_BITS + 
> GRUB_DISK_SECTOR_BITS);
> @@ -524,15 +536,42 @@ grub_efidisk_readwrite (struct grub_disk *disk, 
> grub_disk_addr_t sector,
>  {
>struct grub_efidisk_data *d;
>grub_efi_block_io_t *bio;
> +  grub_efi_status_t status;
> +  grub_size_t io_align, num_bytes;
> +  char *aligned_buf;
>  
>d = disk->data;
>bio = d->block_io;
>  
> -  return efi_call_5 ((wr ? bio->write_blocks : bio->read_blocks), bio,
> -  bio->media->media_id,
> -  (grub_efi_uint64_t) sector,
> -  (grub_efi_uintn_t) size << disk->log_sector_size,
> -  buf);
> +  /* Set alignment to 1 if 0 specified */
> +  io_align = bio->media->io_align ? bio->media->io_align : 1;
> +  num_bytes = size << disk->log_sector_size;
> +
> +  if ((grub_addr_t) buf & (io_align - 1))
> +{
> +  aligned_buf = grub_memalign (io_align, num_bytes);
> +  if (! aligned_buf)
> + return GRUB_EFI_OUT_OF_RESOURCES;
Rather than constantly allocating and deallocating perhaps we should
allocate maximum size and then always use the same buffer? See
max_agglomerate. We already limit to at most 640KiB per read on EFI due
to EFI bugs.
> +  if (wr)
> + grub_memcpy (aligned_buf, buf, num_bytes);
> +}
> +  else
> +{
> +  aligned_buf = buf;
> +}
> +
> +  status =  efi_call_5 ((wr ? bio->write_blocks : bio->read_blocks), bio,
> + bio->media->media_id, (grub_efi_uint64_t) sector,
> + (grub_efi_uintn_t) num_bytes, aligned_buf);
> +
> +  if ((grub_addr_t) buf & (io_align - 1))
> +{
> +  if (!wr)
> + grub_memcpy (buf, aligned_buf, num_bytes);
> +  grub_free (aligned_buf);
> +}
> +
> +  return status;
>  }
>  
>  static grub_err_t
> 




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


Re: Respect EFI block-io buffer alignment

2016-02-17 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 17.02.2016 18:00, Andrei Borzenkov wrote:
> 17.02.2016 19:23, Vladimir 'φ-coder/phcoder' Serbinenko пишет:
>> On 17.02.2016 16:48, Leif Lindholm wrote:
>>> This resolves a complete failure to access devices connected to the
>>> SATA port on the ARM ltd. Juno platform (apart from a violation of the
>>> UEFI block io protocol).
>>>
>>> The below is a bit of a hack, but I'd like some feedback on preferred
>>> solution before over(or under)engineering something.
>>>
>>> As far as I can tell, a struct_disk is only ever allocated in
>>> kern/disk.c, using grub_zalloc(). So the only reason for the horrid
>>> ifdefs is that there is no grub_memalign for EMU.
>>>
>>> Do I:
>>> - Keep the ifdefs?
>>> - Implement grub_memalign() for EMU?
>> You could insipire by grub_osdep_dl_memalign
>>> - Something else?
>>>
>> The code as-is will not work. Buf is passed from external call to
>> grub_disk_read and grub_disk_read tries to read in-place whenever
>> possible. There are 2 cases in current codebase when we need a special
> 
> It can be changed to read into cache and copy in buf instead of read
> into buf and copy in cache.
> 
This is often not very good idea as you lose buffering as you split
transactions. This was a reason GRUB2 was slower than GRUB Legacy on
some systems until it was rewritten to current code.
> 
> 
> 
> ___
> 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: Respect EFI block-io buffer alignment

2016-02-17 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 17.02.2016 16:48, Leif Lindholm wrote:
> This resolves a complete failure to access devices connected to the
> SATA port on the ARM ltd. Juno platform (apart from a violation of the
> UEFI block io protocol).
> 
> The below is a bit of a hack, but I'd like some feedback on preferred
> solution before over(or under)engineering something.
> 
> As far as I can tell, a struct_disk is only ever allocated in
> kern/disk.c, using grub_zalloc(). So the only reason for the horrid
> ifdefs is that there is no grub_memalign for EMU.
> 
> Do I:
> - Keep the ifdefs?
> - Implement grub_memalign() for EMU?
You could insipire by grub_osdep_dl_memalign
> - Something else?
> 
The code as-is will not work. Buf is passed from external call to
grub_disk_read and grub_disk_read tries to read in-place whenever
possible. There are 2 cases in current codebase when we need a special
buffer: usbms and ahci. In both cases corresponding disk routines
allocate corresponding temporary buffer. While it would be nicer to
avoid such a buffer, it will result in messy code and I doubt there is
any speed benefit. So feel free to create temporary buffer or prove me
wrong about speed difference.
> /
> Leif
> 
>>From 2d8b7ae2dd4c639252517e9a1df783ed0564c112 Mon Sep 17 00:00:00 2001
> From: Leif Lindholm 
> Date: Wed, 17 Feb 2016 15:33:47 +
> Subject: [PATCH] efidisk: Respect block_io_protocol buffer alignment
> 
> Returned from the OpenProtocol operation, the grub_efi_block_io_media
> structure contains the io_align field, specifying the minimum alignment
> required for buffers used in any data transfers with the device.
> 
> Add an io_align field to struct grub_disk, and use it in kern/disk.c
> when GRUB_MACHINE_EFI is defined.
> 
> Reported-by: Jeremy Linton 
> ---
>  grub-core/disk/efi/efidisk.c | 1 +
>  grub-core/kern/disk.c| 9 +
>  include/grub/disk.h  | 3 +++
>  3 files changed, 13 insertions(+)
> 
> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> index 1c00e3e..424ff92 100644
> --- a/grub-core/disk/efi/efidisk.c
> +++ b/grub-core/disk/efi/efidisk.c
> @@ -495,6 +495,7 @@ grub_efidisk_open (const char *name, struct grub_disk 
> *disk)
>   and total sectors should be replaced with total blocks.  */
>grub_dprintf ("efidisk", "m = %p, last block = %llx, block size = %x\n",
>   m, (unsigned long long) m->last_block, m->block_size);
> +  disk->io_align = m->io_align;
>disk->total_sectors = m->last_block + 1;
>/* Don't increase this value due to bug in some EFI.  */
>disk->max_agglomerate = 0xa >> (GRUB_DISK_CACHE_BITS + 
> GRUB_DISK_SECTOR_BITS);
> diff --git a/grub-core/kern/disk.c b/grub-core/kern/disk.c
> index 789f8c0..27bef10 100644
> --- a/grub-core/kern/disk.c
> +++ b/grub-core/kern/disk.c
> @@ -331,7 +331,12 @@ grub_disk_read_small_real (grub_disk_t disk, 
> grub_disk_addr_t sector,
>  }
>  
>/* Allocate a temporary buffer.  */
> +#ifdef GRUB_MACHINE_EFI
> +  tmp_buf = grub_memalign (disk->io_align,
> +GRUB_DISK_SECTOR_SIZE << GRUB_DISK_CACHE_BITS);
> +#else
>tmp_buf = grub_malloc (GRUB_DISK_SECTOR_SIZE << GRUB_DISK_CACHE_BITS);
> +#endif
>if (! tmp_buf)
>  return grub_errno;
>  
> @@ -373,7 +378,11 @@ grub_disk_read_small_real (grub_disk_t disk, 
> grub_disk_addr_t sector,
>  num = ((size + offset + (1ULL << (disk->log_sector_size))
>   - 1) >> (disk->log_sector_size));
>  
> +#ifdef GRUB_MACHINE_EFI
> +tmp_buf = grub_memalign (disk->io_align, num << disk->log_sector_size);
> +#else
>  tmp_buf = grub_malloc (num << disk->log_sector_size);
> +#endif
>  if (!tmp_buf)
>return grub_errno;
>  
> diff --git a/include/grub/disk.h b/include/grub/disk.h
> index b385af8..b063bf6 100644
> --- a/include/grub/disk.h
> +++ b/include/grub/disk.h
> @@ -126,6 +126,9 @@ struct grub_disk
>/* Logarithm of sector size.  */
>unsigned int log_sector_size;
>  
> +  /* Minimum read/write buffer alignment */
> +  unsigned int io_align;
> +
>/* Maximum number of sectors read divided by GRUB_DISK_CACHE_SIZE.  */
>unsigned int max_agglomerate;
>  
> 




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

2016-02-12 Thread 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, ,
>>>  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
___

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

2016-02-12 Thread 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

2016-02-12 Thread 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


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

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 12.02.2016 15:38, Andrei Borzenkov wrote:
> On Fri, Feb 12, 2016 at 5:29 PM, Vladimir 'φ-coder/phcoder' Serbinenko
> <phco...@gmail.com> wrote:
>> 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 <andreas.l...@gmx.net>
>>>
>> I feel like we should be fixing diskfilter. Consider another case: dead
>> disk dangling on cable and returning mostly I/O errors
> 
> Could you explain what do you mean? Removable media detection remains
> valid case and cannot be solved without low level driver cooperation
> anyway. If you mean some ratelimiting, this probably has to go into
> core, not in diskfilter, but it looks orthogonal to this patch.
> 
I mean what if we have a legitimately bad disk unrelated to any
diskfilter VGs. If diskfilter is unable to read from it, it should still
be able to assemble VGs and skip failed disk
> ___
> 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 2/6] relocator: Do not use memory region if its starta is smaller than size

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Applied, thanks
On 20.07.2015 16:35, 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)
> {
>   target = starta - size;
>   if (target > end - size)
> 




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


Re: Plain dm-crypt

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 27.10.2015 12:15, Andrei Borzenkov wrote:
> On Tue, Oct 27, 2015 at 2:10 PM, Vladimir 'phcoder' Serbinenko
>  wrote:
>> There are patches for it but they will not be integrated as plain dm-crypt
>> has no advantages compared to LUKS and cannot be configured reliably when
>> device names change as they have no UUID
>>
> 
> Still there is real demand. If these patches can be made as
> independent module, providing them as part of grub-extra makes sense.
> 
I think it makes sense to include them in main repository but it should
be a separate module and not pollute cryptomount command.
>> Le 27 oct. 2015 8:20 AM,  a écrit :
>>>
>>> Hello;
>>> I apologize if this question has already been asked. A web search didn't
>>> turn anything up. Are there any plans to include plain dm-crypt support in a
>>> future version of grub?
>>>
>>> Thank you.
>>> Chris
>>>
>>> ___
>>> 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 4/5] Cryptomount support plain dm-crypt

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 29.06.2015 16:30, John Lane wrote:
> From: John Lane 
> 
> ---
>  grub-core/disk/cryptodisk.c | 298 
> +++-
>  grub-core/disk/luks.c   | 195 +
>  include/grub/cryptodisk.h   |   8 ++
>  3 files changed, 310 insertions(+), 191 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index a27e70c..cd5cfc9 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -44,6 +44,12 @@ static const struct grub_arg_option options[] =
>  {"keyfile", 'k', 0, N_("Key file"), 0, ARG_TYPE_STRING},
>  {"keyfile-offset", 'O', 0, N_("Key file offset (bytes)"), 0, 
> ARG_TYPE_INT},
>  {"keyfile-size", 'S', 0, N_("Key file data size (bytes)"), 0, 
> ARG_TYPE_INT},
> +{"plain", 'p', 0, N_("Plain (no LUKS header)"), 0, ARG_TYPE_NONE},
> +{"cipher", 'c', 0, N_("Plain mode cipher"), 0, ARG_TYPE_STRING},
> +{"digest", 'd', 0, N_("Plain mode passphrase digest (hash)"), 0, 
> ARG_TYPE_STRING},
> +{"offset", 'o', 0, N_("Plain mode data sector offset"), 0, ARG_TYPE_INT},
> +{"size", 's', 0, N_("Size of raw device (sectors, defaults to whole 
> device)"), 0, ARG_TYPE_INT},
> +{"key-size", 'K', 0, N_("Set key size (bits)"), 0, ARG_TYPE_INT},
>  {0, 0, 0, 0, 0, 0}
>};
>  
This is hairy, doesn't fit into cryptomount syntax and not maintainable.
Can we have a separate command for this?
> @@ -927,6 +933,48 @@ grub_cryptodisk_scan_device (const char *name,
>return have_it && search_uuid ? 1 : 0;
>  }
>  
> +/* Hashes a passphrase into a key and stores it with cipher. */
> +static gcry_err_code_t
> +set_passphrase (grub_cryptodisk_t dev, grub_size_t keysize, const char 
> *passphrase)
> +{
> +  grub_uint8_t derived_hash[GRUB_CRYPTODISK_MAX_KEYLEN * 2], *dh = 
> derived_hash;
> +  char *p;
> +  unsigned int round, i;
> +  unsigned int len, size;
> +
> +  /* Need no passphrase if there's no key */
> +  if (keysize == 0)
> +return GPG_ERR_INV_KEYLEN;
> +
> +  /* Hack to support the "none" hash */
> +  if (dev->hash)
> +len = dev->hash->mdlen;
> +  else
> +len = grub_strlen (passphrase);
> +
> +  if (keysize > GRUB_CRYPTODISK_MAX_KEYLEN || len > 
> GRUB_CRYPTODISK_MAX_KEYLEN)
> +return GPG_ERR_INV_KEYLEN;
> +
> +  p = grub_malloc (grub_strlen (passphrase) + 2 + keysize / len);
> +  if (!p)
> +return grub_errno;
> +
> +  for (round = 0, size = keysize; size; round++, dh += len, size -= len)
> +{
> +  for (i = 0; i < round; i++)
> + p[i] = 'A';
> +
> +  grub_strcpy (p + i, passphrase);
> +
> +  if (len > size)
> + len = size;
> +
> +  grub_crypto_hash (dev->hash, dh, p, grub_strlen (p));
> +}
> +
> +  return grub_cryptodisk_setkey (dev, derived_hash, keysize);
> +}
> +
>  static grub_err_t
>  grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
>  {
> @@ -1046,7 +1094,63 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int 
> argc, char **args)
> return GRUB_ERR_NONE;
>   }
>  
> -  err = grub_cryptodisk_scan_device_real (args[0], disk);
> +  if (state[7].set) /* Plain mode */
> +{
> +  char *cipher;
> +  char *mode;
> +  char *digest;
> +  int offset, size, key_size;
> +
> +  cipher = grub_strdup (state[8].set ? state[8].arg : 
> GRUB_CRYPTODISK_PLAIN_CIPHER);
> +  digest = grub_strdup (state[9].set ? state[9].arg : 
> GRUB_CRYPTODISK_PLAIN_DIGEST);
> +  offset = state[10].set ? grub_strtoul (state[10].arg, 0, 0) : 0;
> +  size   = state[11].set ? grub_strtoul (state[11].arg, 0, 0) : 0;
> +  key_size = ( state[12].set ? grub_strtoul (state[12].arg, 0, 0) \
> +  : GRUB_CRYPTODISK_PLAIN_KEYSIZE ) / 8;
> +
> +  /* no strtok, do it manually */
> +  mode = grub_strchr(cipher,'-');
> +  if (!mode)
> +return GRUB_ERR_BAD_ARGUMENT;
> +  else
> +*mode++ = 0;
> +
> +  dev = grub_cryptodisk_create (disk, NULL, cipher, mode, digest);
> +
> +  dev->offset = offset;
> +   if (size) dev->total_length = size;
> +
> +  if (key)
> + {
> +  err = grub_cryptodisk_setkey (dev, key, key_size);
> +  if (err)
> +return err;
> + }
> +   else
> + {
> +  char passphrase[GRUB_CRYPTODISK_MAX_PASSPHRASE] = "";
> +
> +  grub_printf_ (N_("Enter passphrase for %s: "), diskname);
> +  if (!grub_password_get (passphrase, 
> GRUB_CRYPTODISK_MAX_PASSPHRASE))
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not 
> supplied");
> +
> +  err = set_passphrase (dev, key_size, passphrase);
> +  if (err)
> +{
> +  grub_crypto_cipher_close (dev->cipher);
> +  return err;
> +}
> + }
> +
> 

Re: Dead link in TODO file

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 12.02.2016 09:28, Thomas Huth wrote:
> 
>  Hi,
> 
> The TODO file in the grub repository refers to Grub Wiki at
> http://grub.enbug.org/ ... however, that URL seems to be invalid
> nowadays. Is there a new Wiki somewhere else?
> 
Unfortunately not unless we find a volunteer to host it
>  Thomas
> 
> ___
> 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] grub-install: Include all decompressor modules in pvxen core image.

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 07.12.2014 18:26, Andrei Borzenkov wrote:
> В Sun, 07 Dec 2014 18:18:47 +0100
> Vladimir 'φ-coder/phcoder' Serbinenko <phco...@gmail.com> пишет:
> 
>> On 30.11.2014 14:34, Ian Campbell wrote:
>>> On Sun, 2014-11-30 at 11:51 +, Ian Campbell wrote:
>>>> On Sun, 2014-11-30 at 14:31 +0300, Andrei Borzenkov wrote:
>>>>> if [ x$grub_platform = xxen ]; then
>>>>>   insmod xzio
>>>>> fi
>>>>
>>>> I think that could work.
>>>
>>> Indeed it does, so how about this instead of the patch at the start of
>>> the thread?
>>>
>> This should probably have been autoloaded.
> 
> File filters are not autoloaded and I do not see how it can really be
> done. OTOH unconditionally loading them may have unwanted side effect
> where we assumed files were not decompressed.
> 
> In this specific case extending grub-file to detect compression type
> used by kernel is probably OK.
Please test attached file.
> 
>>Can you send me privately a
>> kernel you use, so I can reproduce your tests?
>>> >From f4199776eca80dfad4e9378a01ddb5866face3d7 Mon Sep 17 00:00:00 2001
>>> From: Ian Campbell <i...@debian.org>
>>> Date: Sun, 30 Nov 2014 12:12:52 +
>>> Subject: [PATCH] Arrange to insmod xzio when booting a kernel as a Xen guest
>>>
>>> This is needed in case the Linux kernel is compiled with CONFIG_KERNEL_XZ
>>> rather than CONFIG_KERNEL_GZ (gzio is already loaded by grub.cfg).
>>>
>>> Signed-off-by: Ian Campbell <i...@debian.org>
>>>
>>> Patch-Name: insmod-xzio-on-xen.patch
>>> ---
>>>  util/grub.d/10_linux.in | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
>>> index 79fa03a..86e35f2 100644
>>> --- a/util/grub.d/10_linux.in
>>> +++ b/util/grub.d/10_linux.in
>>> @@ -150,6 +150,7 @@ linux_entry ()
>>>fi
>>>  
>>>echo "   insmod gzio" | sed "s/^/$submenu_indentation/"
>>> +  echo "   if [ x\$grub_platform = xxen ]; then insmod xzio; insmod 
>>> lzopio; fi" | sed "s/^/$submenu_indentation/"
>>>  
>>>if [ x$dirname = x/ ]; then
>>>  if [ -z "${prepare_root_cache}" ]; then
>>>
>>
>>
> 
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 

diff --git a/grub-core/loader/i386/xen_file.c b/grub-core/loader/i386/xen_file.c
index 5836218..90e0a6e 100644
--- a/grub-core/loader/i386/xen_file.c
+++ b/grub-core/loader/i386/xen_file.c
@@ -20,12 +20,99 @@
 #include 
 #include 
 
+#define LZOP_MAGIC "\x89\x4c\x5a\x4f\x00\x0d\x0a\x1a\x0a"
+#define LZOP_MAGIC_SIZE 9
+#define XZ_MAGIC "\3757zXZ\0"
+#define XZ_MAGIC_SIZE 6
+#define ELF_MAGIC "\177ELF"
+#define ELF_MAGIC_SIZE 4
+#define GZ_MAGIC "\x1F\x8B"
+#define GZ_MAGIC_SIZE 2
+#define GZ_OLD_MAGIC "\x1F\x9E"
+#define GZ_OLD_MAGIC_SIZE 2
+
+#ifndef GRUB_UTIL
+
+#include 
+
+static grub_uint8_t
+mod_31 (grub_uint16_t v)
+{
+  /* At most 2 iterations for any number that
+ we can get here.
+ In any case faster than real division.  */
+  while (v > 0x1f)
+v = (v & 0x1f) + (v >> 5);
+  if (v == 0x1f)
+return 0;
+  return v;
+}
+
+static int
+is_zlib (grub_uint8_t *head)
+{
+  grub_uint8_t cmf, flg;
+ 
+  cmf = head[0];
+  flg = head[1];
+
+  if ((cmf & 0xf) != 8)
+return 0;
+
+  if (mod_31 (cmf + flg * 4) != 0)
+return 0;
+
+  if (flg & 0x20)
+return 0;
+  
+  return 1;
+}
+
+static void
+autoload_filters (grub_file_t file, grub_uint32_t payload_offset)
+{
+  grub_uint8_t payload_header[LZOP_MAGIC_SIZE];
+
+  grub_file_seek (file, payload_offset);
+
+  if (grub_file_read (file, _header, sizeof (payload_header)) != sizeof (payload_header))
+{
+  grub_print_error ();
+  return;
+}
+
+  if (grub_memcmp (payload_header, ELF_MAGIC, ELF_MAGIC_SIZE) == 0)
+{
+  /* Uncompressed.  */
+}
+  else if (grub_memcmp (payload_header, XZ_MAGIC, XZ_MAGIC_SIZE) == 0)
+{
+  grub_dl_load("xzio");
+  grub_print_error ();
+}
+  else if (grub_memcmp (payload_header, LZOP_MAGIC, LZOP_MAGIC_SIZE) == 0)
+{
+  grub_dl_load("lzopio");
+  grub_print_error ();
+}
+  else if (grub_memcmp (payload_header, GZ_MAGIC, GZ_MAGIC_SIZE) == 0
+	   || grub_memcmp (payload_header, GZ_OLD_MAGIC, GZ_OLD_MAGIC_SIZE) == 0
+	   || is_zlib (payload_header))
+{
+  grub_dl_load("gzio");
+ 

Re: [PATCH v3 1/3] Add helper functions to interact with android bootimg disks.

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 08.02.2016 21:47, Shea Levy wrote:
> Currently, the kernel, command line, and ramdisk can be extracted.
> ---
>  grub-core/Makefile.core.def|   1 +
>  grub-core/loader/android_bootimg.c | 253 
> +
>  include/grub/android_bootimg.h |  12 ++
>  3 files changed, 266 insertions(+)
>  create mode 100644 grub-core/loader/android_bootimg.c
>  create mode 100644 include/grub/android_bootimg.h
> 
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 0cc40bb..991adad 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1669,6 +1669,7 @@ module = {
>arm64 = loader/arm64/linux.c;
>common = loader/linux.c;
>common = lib/cmdline.c;
> +  common = loader/android_bootimg.c;
>enable = noemu;
>  };
>  
> diff --git a/grub-core/loader/android_bootimg.c 
> b/grub-core/loader/android_bootimg.c
> new file mode 100644
> index 000..bbee915
> --- /dev/null
> +++ b/grub-core/loader/android_bootimg.c
> @@ -0,0 +1,253 @@
> +/* android_bootimg.c - Helpers for interacting with the android bootimg 
> format. */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2016 Free Software Foundation, Inc.
> + *
> + *  This program 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.
> + *
> + *  This program 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 this program.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +/* from 
> https://android.googlesource.com/platform/system/core/+/506d233e7ac8ca4efa80768153d842c296477f99/mkbootimg/bootimg.h
>  */
Parts of the file cannot be effectively under different licenses. Please
put this into separate header file.
> +/* From here until the end of struct boot_img_hdr available under the 
> following terms:
> + *
> + * Copyright 2007, The Android Open Source Project
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +#define BOOT_MAGIC   "ANDROID!"
> +#define BOOT_MAGIC_SIZE  8
> +#define BOOT_NAME_SIZE   16
> +#define BOOT_EXTRA_ARGS_SIZE 1024
> +
> +struct boot_img_hdr
> +{
> +  grub_uint8_t magic[BOOT_MAGIC_SIZE];
> +
> +  grub_uint32_t kernel_size;
> +  grub_uint32_t kernel_addr;
> +
> +  grub_uint32_t ramdisk_size;
> +  grub_uint32_t ramdisk_addr;
> +
> +  grub_uint32_t second_size;
> +  grub_uint32_t second_addr;
> +
> +  grub_uint32_t tags_addr;
> +  grub_uint32_t page_size;
> +  grub_uint32_t unused[2];
> +
> +  grub_uint8_t name[BOOT_NAME_SIZE];
> +
> +  grub_uint8_t cmdline[BOOT_ARGS_SIZE];
> +
> +  grub_uint32_t id[8];
> +
> +  grub_uint8_t extra_cmdline[BOOT_EXTRA_ARGS_SIZE];
> +} GRUB_PACKED;
> +
> +static grub_err_t
> +read_hdr (grub_disk_t disk, struct boot_img_hdr *hd)
> +{
> +  if (grub_disk_read (disk, 0, 0, sizeof *hd, hd))
> +goto fail;
> +
> +  if (grub_memcmp (hd->magic, BOOT_MAGIC, BOOT_MAGIC_SIZE))
> +goto fail;
> +
> +  if (hd->unused[0] || hd->unused[1])
> +goto fail;
> +
> +  hd->kernel_size = grub_le_to_cpu32 (hd->kernel_size);
> +  hd->kernel_addr = grub_le_to_cpu32 (hd->kernel_addr);
> +  hd->ramdisk_size = grub_le_to_cpu32 (hd->ramdisk_size);
> +  hd->ramdisk_addr = grub_le_to_cpu32 (hd->ramdisk_addr);
> +  hd->second_size = grub_le_to_cpu32 (hd->second_size);
> +  hd->second_addr = grub_le_to_cpu32 (hd->second_addr);
> +  hd->tags_addr = grub_le_to_cpu32 (hd->tags_addr);
> +  hd->page_size = grub_le_to_cpu32 (hd->page_size);
> +
> +  grub_size_t i;
> +  for (i = 0; i < sizeof hd->id / sizeof hd->id[0]; ++i)
> +{
> +  hd->id[i] = grub_le_to_cpu32 (hd->id[i]);
> +}
Why do you need this byteswap? Why not byteswap when it's used?
Also in loaders you can avoid byteswap usually as you know that you load
the file for the same arch as you currently run. Exceptions are arm-be,
ppc-le and fat binary headers.
> +
> +  return GRUB_ERR_NONE;
> +
> +fail:
> +  return grub_error (GRUB_ERR_BAD_FS, N_("%s not an android bootimg"),
> +

Re: [PATCH v3 2/3] i386: Add support for loading from android bootimg

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 08.02.2016 21:47, Shea Levy wrote:
> ---
>  grub-core/loader/i386/linux.c | 27 +--
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
> index fddcc46..6ab8d3c 100644
> --- a/grub-core/loader/i386/linux.c
> +++ b/grub-core/loader/i386/linux.c
> @@ -35,6 +35,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  GRUB_MOD_LICENSE ("GPLv3+");
>  
> @@ -695,7 +696,13 @@ grub_cmd_linux (grub_command_t cmd __attribute__ 
> ((unused)),
>goto fail;
>  }
>  
> -  file = grub_file_open (argv[0]);
> +  char android_cmdline[BOOT_ARGS_SIZE];
> +  android_cmdline[0] = '\0';
> +  if (grub_memcmp (argv[0], "android_bootimg:", sizeof "android_bootimg:" - 
> 1) == 0)
> +grub_android_bootimg_load_kernel (argv[0] + sizeof "android_bootimg:" - 
> 1,
> +  , android_cmdline);
> +  else
> +  file = grub_file_open (argv[0]);
I hoped more for autodetection. This gets a bit hairy and proper
separation is better. Sorry for confusion. I think it's simpler with
commands like
android_bootimg [--no-cmdline] [--no-initrd] IMAGE [EXTRA_ARGUMENTS]
by default it will load both IMAGE, with cmdline and initrd. With
--no-initrd you can use initrd for custom initrd.
>if (! file)
>  goto fail;
>  
> @@ -1008,12 +1015,20 @@ grub_cmd_linux (grub_command_t cmd __attribute__ 
> ((unused)),
>linux_cmdline = grub_zalloc (maximal_cmdline_size + 1);
>if (!linux_cmdline)
>  goto fail;
> -  grub_memcpy (linux_cmdline, LINUX_IMAGE, sizeof (LINUX_IMAGE));
> +  grub_size_t cmdline_offset = 0;
> +  if (android_cmdline[0])
> +{
> +  cmdline_offset = grub_strlen (android_cmdline) + 1;
> +  grub_memcpy (linux_cmdline, android_cmdline, cmdline_offset - 1);
> +  linux_cmdline[cmdline_offset - 1] = ' ';
> +}
> +  grub_memcpy (linux_cmdline + cmdline_offset, LINUX_IMAGE, sizeof 
> (LINUX_IMAGE));
> +  cmdline_offset += sizeof LINUX_IMAGE - 1;
LINUX_IMAGE must be at the beginning. don't forget brackets around sizeof.
>grub_create_loader_cmdline (argc, argv,
> -   linux_cmdline
> -   + sizeof (LINUX_IMAGE) - 1,
> -   maximal_cmdline_size
> -   - (sizeof (LINUX_IMAGE) - 1));
> +  linux_cmdline
> +  + cmdline_offset,
> +  maximal_cmdline_size
> +  - cmdline_offset);
>  
>len = prot_file_size;
>if (grub_file_read (file, prot_mode_mem, len) != len && !grub_errno)
> 




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


Re: [PATCH] core/partmap: Add El Torito boot catalog parsing

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 20.06.2015 12:16, Ross Lagerwall wrote:
> GRUB calculates a boot device based on the device path given by the
> firmware so:
Please reread what Andrei said. GRUB combines stored prefix and
information from firmware. Syntax ()/boot/grub will always use the full disk



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] Add helper functions to interact with android bootimg disks.

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 12.02.2016 18:42, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 08.02.2016 21:47, Shea Levy wrote:
>> Currently, the kernel, command line, and ramdisk can be extracted.
>> ---
>>  grub-core/Makefile.core.def|   1 +
>>  grub-core/loader/android_bootimg.c | 253 
>> +
>>  include/grub/android_bootimg.h |  12 ++
>>  3 files changed, 266 insertions(+)
>>  create mode 100644 grub-core/loader/android_bootimg.c
>>  create mode 100644 include/grub/android_bootimg.h
>>
>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>> index 0cc40bb..991adad 100644
>> --- a/grub-core/Makefile.core.def
>> +++ b/grub-core/Makefile.core.def
>> @@ -1669,6 +1669,7 @@ module = {
>>arm64 = loader/arm64/linux.c;
>>common = loader/linux.c;
>>common = lib/cmdline.c;
>> +  common = loader/android_bootimg.c;
>>enable = noemu;
>>  };
>>  
>> diff --git a/grub-core/loader/android_bootimg.c 
>> b/grub-core/loader/android_bootimg.c
>> new file mode 100644
>> index 000..bbee915
>> --- /dev/null
>> +++ b/grub-core/loader/android_bootimg.c
>> @@ -0,0 +1,253 @@
>> +/* android_bootimg.c - Helpers for interacting with the android bootimg 
>> format. */
>> +/*
>> + *  GRUB  --  GRand Unified Bootloader
>> + *  Copyright (C) 2016 Free Software Foundation, Inc.
>> + *
>> + *  This program 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.
>> + *
>> + *  This program 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 this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/* from 
>> https://android.googlesource.com/platform/system/core/+/506d233e7ac8ca4efa80768153d842c296477f99/mkbootimg/bootimg.h
>>  */
> Parts of the file cannot be effectively under different licenses. Please
> put this into separate header file.
>> +/* From here until the end of struct boot_img_hdr available under the 
>> following terms:
>> + *
>> + * Copyright 2007, The Android Open Source Project
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at
>> + *
>> + * http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +#define BOOT_MAGIC  "ANDROID!"
>> +#define BOOT_MAGIC_SIZE 8
>> +#define BOOT_NAME_SIZE  16
>> +#define BOOT_EXTRA_ARGS_SIZE1024
>> +
>> +struct boot_img_hdr
>> +{
>> +  grub_uint8_t magic[BOOT_MAGIC_SIZE];
>> +
>> +  grub_uint32_t kernel_size;
>> +  grub_uint32_t kernel_addr;
>> +
>> +  grub_uint32_t ramdisk_size;
>> +  grub_uint32_t ramdisk_addr;
>> +
>> +  grub_uint32_t second_size;
>> +  grub_uint32_t second_addr;
>> +
>> +  grub_uint32_t tags_addr;
>> +  grub_uint32_t page_size;
>> +  grub_uint32_t unused[2];
>> +
>> +  grub_uint8_t name[BOOT_NAME_SIZE];
>> +
>> +  grub_uint8_t cmdline[BOOT_ARGS_SIZE];
>> +
>> +  grub_uint32_t id[8];
>> +
>> +  grub_uint8_t extra_cmdline[BOOT_EXTRA_ARGS_SIZE];
>> +} GRUB_PACKED;
>> +
>> +static grub_err_t
>> +read_hdr (grub_disk_t disk, struct boot_img_hdr *hd)
>> +{
>> +  if (grub_disk_read (disk, 0, 0, sizeof *hd, hd))
>> +goto fail;
>> +
>> +  if (grub_memcmp (hd->magic, BOOT_MAGIC, BOOT_MAGIC_SIZE))
>> +goto fail;
>> +
>> +  if (hd->unused[0] || hd->unused[1])
>> +goto fail;
>> +
>> +  hd->kernel_size = grub_le_to_cpu32

Re: CMOS_ADDRESS of ThinkVantage

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 02.08.2015 17:28, Andreas Freimuth wrote:
> Hi,
> 
> i just wonted to contribute the correct CMOS address for the
> ThinkVantage-Button for:
> 
> LENOVO ThinkPad T410s (2912W1C)
> 
> it is:
> 
> GRUB_BUTTON_CMOS_ADDRESS=101:3
> 
Added to docs, thank you
> Regards
> 
> 
> 
> ___
> 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: [RFC] grub-install: allow none or multiple install devices on PC BIOS

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 08.05.2015 20:53, Andrei Borzenkov wrote:
> There are two main applications.
> 
> 1. Omit install device to create generic image intended for chainloading
> from other master loader. Such image can be put on any device (or file
> system) and will still be able to find its $root. Currently even with
> --no-bootsector grub-install optimizes image by skipping UUID search if
> possible.
> 
> 2. Redundant installation on multi-device filesystem, RAID or similar.
> This allows both optimizing image w.r.t. to using --prefix vs. load.cfg
> as well as creating image just once.
> 
> Patch allows transparently use none or multiple installation devices,
> similar to
> 
> grub_devices="/dev/sda /dev/sda1 /dev/sdb"
> grub-install $grub_devices
> 
> where grub_devices can be empty and still do the right thing.
> 
> This is work in progress, although it is functionally complete and just
> needs some cleanups.
> 
> Comments?
I like the idea and it was on my "nice to have" list for some time. Do
you want to clean it up first or should I review this version?
> 
> ---
>  grub-core/kern/disk.c |   5 +-
>  include/grub/disk.h   |   2 +
>  util/grub-install.c   | 217 
> +++---
>  3 files changed, 143 insertions(+), 81 deletions(-)
> 
> diff --git a/grub-core/kern/disk.c b/grub-core/kern/disk.c
> index 789f8c0..56f16b4 100644
> --- a/grub-core/kern/disk.c
> +++ b/grub-core/kern/disk.c
> @@ -168,7 +168,10 @@ grub_disk_dev_unregister (grub_disk_dev_t dev)
>  
>  /* Return the location of the first ',', if any, which is not
> escaped by a '\'.  */
> -static const char *
> +#if !defined (GRUB_UTIL)
> +static
> +#endif
> +const char *
>  find_part_sep (const char *name)
>  {
>const char *p = name;
> diff --git a/include/grub/disk.h b/include/grub/disk.h
> index b385af8..b2081eb 100644
> --- a/include/grub/disk.h
> +++ b/include/grub/disk.h
> @@ -256,6 +256,8 @@ void grub_ldm_fini (void);
>  void grub_mdraid09_fini (void);
>  void grub_mdraid1x_fini (void);
>  void grub_diskfilter_fini (void);
> +extern const char *find_part_sep (const char *);
> +
>  #endif
>  
>  #endif /* ! GRUB_DISK_HEADER */
> diff --git a/util/grub-install.c b/util/grub-install.c
> index 7b394c9..bb6532c 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -57,7 +57,10 @@ static char *target;
>  static int removable = 0;
>  static int recheck = 0;
>  static int update_nvram = 1;
> -static char *install_device = NULL;
> +static char **install_devices = NULL;
> +static char **install_drives = NULL;
> +static int n_install_devices;
> +static int n_allocated_devices;
>  static char *debug_image = NULL;
>  static char *rootdir = NULL;
>  static char *bootdir = NULL;
> @@ -234,9 +237,12 @@ argp_parser (int key, char *arg, struct argp_state 
> *state)
>return 0;
>  
>  case ARGP_KEY_ARG:
> -  if (install_device)
> - grub_util_error ("%s", _("More than one install device?"));
> -  install_device = xstrdup (arg);
> +  if (n_install_devices >= n_allocated_devices)
> + {
> +   n_allocated_devices += 16;
> +   install_devices = xrealloc (install_devices, n_allocated_devices);
> + }
> +  install_devices[n_install_devices++] = xstrdup (arg);
>return 0;
>  
>  default:
> @@ -534,25 +540,55 @@ probe_cryptodisk_uuid (grub_disk_t disk)
>  }
>  
>  static int
> -is_same_disk (const char *a, const char *b)
> +same_disks (char **root_devs)
>  {
> -  while (1)
> +  int i;
> +
> +  for (i = 0; i < n_install_devices; i++)
>  {
> -  if ((*a == ',' || *a == '\0') && (*b == ',' || *b == '\0'))
> - return 1;
> -  if (*a != *b)
> - return 0;
> -  if (*a == '\\')
> +  char **d;
> +  const char *p1 = find_part_sep (install_drives[i]);
> +  size_t len1 = p1 ? p1 - install_drives[i] : strlen (install_drives[i]);
> +
> +  for (d = root_devs; *d; d++)
>   {
> -   if (a[1] != b[1])
> - return 0;
> -   a += 2;
> -   b += 2;
> -   continue;
> +   const char *p2 = find_part_sep (*d);
> +   size_t len2 = p2 ? p2 - *d : strlen (*d);
> +
> +   if (len1 == len2 &&
> +   strncmp (install_drives[i], *d, len1) == 0)
> + break;
>   }
> -  a++;
> -  b++;
> +  if (!*d)
> + return 0;
> +}
> +
> +  return 1;
> +}
> +
> +static int
> +same_partitions (char **root_devs)
> +{
> +  const char *first_part;
> +  char **p;
> +
> +  if (!root_devs[1])
> +return 1;
> +
> +  first_part = find_part_sep (root_devs[0]);
> +  for (p = root_devs + 1; *p; p++)
> +{
> +  const char *part = find_part_sep (*p);
> +
> +  if ((first_part == NULL) ^ (part == NULL))
> + return 0;
> +  if (!first_part && !part)
> + continue;
> +  if (strcmp (first_part, part))
> + return 0;
>  }
> +
> +  return 1;
>  }
>  
>  static char *
> @@ -835,6 +871,8 @@ main (int argc, char *argv[])
>int efidir_is_mac = 0;
>int is_prep = 0;
>const char *pkgdatadir;

Re: [PATCH v3 2/3] i386: Add support for loading from android bootimg

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 12.02.2016 20:34, Shea Levy wrote:
> OK. Do you have any thoughts on how best to extract the "load the kernel
> and set the command line to a specific string" functionality out of the
> linux command, then?
> 
At first I wanted to write a short skeleton patch to show what I meant
but once skeleton is done, there is little actual code involved. Please
try attached patch
> On 2016-02-12 14:22, Vladimir 'phcoder' Serbinenko wrote:
>> Separate command is better as it keeps interface tidy and unpoluted,
>> decreasing maintenance cost. Correct me if I'm wrong but it should be
>> clear from context of file is Android image or usual linux one?
>>
>> Le ven. 12 févr. 2016 20:19, Shea Levy <s...@shealevy.com> a écrit :
>>
>>> On 2016-02-12 12:50, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>>> > On 08.02.2016 21:47, Shea Levy wrote:
>>> >> ---
>>> >>  grub-core/loader/i386/linux.c | 27 +--
>>> >>  1 file changed, 21 insertions(+), 6 deletions(-)
>>> >>
>>> >> diff --git a/grub-core/loader/i386/linux.c
>>> >> b/grub-core/loader/i386/linux.c
>>> >> index fddcc46..6ab8d3c 100644
>>> >> --- a/grub-core/loader/i386/linux.c
>>> >> +++ b/grub-core/loader/i386/linux.c
>>> >> @@ -35,6 +35,7 @@
>>> >>  #include 
>>> >>  #include 
>>> >>  #include 
>>> >> +#include 
>>> >>
>>> >>  GRUB_MOD_LICENSE ("GPLv3+");
>>> >>
>>> >> @@ -695,7 +696,13 @@ grub_cmd_linux (grub_command_t cmd
>>> >> __attribute__ ((unused)),
>>> >>goto fail;
>>> >>  }
>>> >>
>>> >> -  file = grub_file_open (argv[0]);
>>> >> +  char android_cmdline[BOOT_ARGS_SIZE];
>>> >> +  android_cmdline[0] = '';
>>> >> +  if (grub_memcmp (argv[0], "android_bootimg:", sizeof
>>> >> "android_bootimg:" - 1) == 0)
>>> >> +grub_android_bootimg_load_kernel (argv[0] + sizeof
>>> >> "android_bootimg:" - 1,
>>> >> +  , android_cmdline);
>>> >> +  else
>>> >> +  file = grub_file_open (argv[0]);
>>> > I hoped more for autodetection. This gets a bit hairy and proper
>>> > separation is better. Sorry for confusion. I think it's simpler with
>>> > commands like
>>> > android_bootimg [--no-cmdline] [--no-initrd] IMAGE [EXTRA_ARGUMENTS]
>>> > by default it will load both IMAGE, with cmdline and initrd. With
>>> > --no-initrd you can use initrd for custom initrd.
>>>
>>> Autodetection would be possible actually, I didn't think of that. If
>>> grub_file_open fails, we can try grub_android_bootimg_load_kernel on the
>>> same file. Would that be preferable or do we still want a separate
>>> command?
>>>
>>> >
>>> >>if (! file)
>>> >>  goto fail;
>>> >>
>>> >> @@ -1008,12 +1015,20 @@ grub_cmd_linux (grub_command_t cmd
>>> >> __attribute__ ((unused)),
>>> >>linux_cmdline = grub_zalloc (maximal_cmdline_size + 1);
>>> >>if (!linux_cmdline)
>>> >>  goto fail;
>>> >> -  grub_memcpy (linux_cmdline, LINUX_IMAGE, sizeof (LINUX_IMAGE));
>>> >> +  grub_size_t cmdline_offset = 0;
>>> >> +  if (android_cmdline[0])
>>> >> +{
>>> >> +  cmdline_offset = grub_strlen (android_cmdline) + 1;
>>> >> +  grub_memcpy (linux_cmdline, android_cmdline, cmdline_offset -
>>> >> 1);
>>> >> +  linux_cmdline[cmdline_offset - 1] = ' ';
>>> >> +}
>>> >> +  grub_memcpy (linux_cmdline + cmdline_offset, LINUX_IMAGE, sizeof
>>> >> (LINUX_IMAGE));
>>> >> +  cmdline_offset += sizeof LINUX_IMAGE - 1;
>>> > LINUX_IMAGE must be at the beginning. don't forget brackets around
>>> > sizeof.
>>> >>grub_create_loader_cmdline (argc, argv,
>>> >> -  linux_cmdline
>>> >> -  + sizeof (LINUX_IMAGE) - 1,
>>> >> -  maximal_cmdline_size
>>> >> -  - (sizeof (LINUX_IMAGE) - 1));
>>> >> +  linux_cmdline
>>> >> +  + cmdl

Re: [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Committed, thanks
On 12.02.2016 22:35, Eric Snowberg wrote:
> OBP available region contains grub. Start at grub_phys_end.
> 
> This prevents a problem where grub was being overwritten since
> grub_phys_start does not start at a zero offset within the memory
> map.
> 
> Signed-off-by: Eric Snowberg 
> ---
>  grub-core/loader/sparc64/ieee1275/linux.c |   16 
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/grub-core/loader/sparc64/ieee1275/linux.c 
> b/grub-core/loader/sparc64/ieee1275/linux.c
> index d44d7a1..67ef048 100644
> --- a/grub-core/loader/sparc64/ieee1275/linux.c
> +++ b/grub-core/loader/sparc64/ieee1275/linux.c
> @@ -203,20 +203,20 @@ alloc_phys_choose (grub_uint64_t addr, grub_uint64_t 
> len,
>if (addr + ctx->size >= end)
>  return 0;
>  
> -  if (addr >= grub_phys_start && addr < grub_phys_end)
> -{
> -  addr = ALIGN_UP (grub_phys_end, FOUR_MB);
> -  if (addr + ctx->size >= end)
> - return 0;
> -}
> -  if ((addr + ctx->size) >= grub_phys_start
> -  && (addr + ctx->size) < grub_phys_end)
> +  /* OBP available region contains grub. Start at grub_phys_end. */
> +  /* grub_phys_start does not start at the beginning of the memory region */
> +  if ((grub_phys_start >= addr && grub_phys_end < end) ||
> +  (addr > grub_phys_start && addr < grub_phys_end))
>  {
>addr = ALIGN_UP (grub_phys_end, FOUR_MB);
>if (addr + ctx->size >= end)
>   return 0;
>  }
>  
> +  grub_dprintf("loader",
> +"addr = 0x%lx grub_phys_start = 0x%lx grub_phys_end = 0x%lx\n",
> +addr, grub_phys_start, grub_phys_end);
> +
>if (loaded)
>  {
>grub_addr_t linux_end = ALIGN_UP (linux_paddr + linux_size, FOUR_MB);
> 




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

2016-01-22 Thread 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

2016-01-22 Thread 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: [Xen-devel] Uniform commands for booting xen

2016-01-22 Thread 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: PV-Grub2 works here but doesnt work their...

2016-01-22 Thread 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

2016-01-22 Thread 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 (, 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 (>gw, source, sizeof 

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

2016-01-22 Thread 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] = 000200251023 04a1
>>>> (XEN) L1

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

2016-01-22 Thread 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 

Re: test command (in-)compatibility

2016-01-22 Thread 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] Uniform commands for booting xen

2016-01-11 Thread 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: enable_progress_indicator

2016-01-07 Thread 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


Re: ARM*-EFI timers

2016-01-07 Thread 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


ARM*-EFI timers

2016-01-07 Thread 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: util/bin2h.c?

2016-01-01 Thread 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: [PATCH] arm64: build with -mcmodel=large

2015-12-31 Thread 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

2015-12-31 Thread 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: [PATCH] 30_os-prober: derive --class from os-prober generated label

2015-12-30 Thread 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: Each grub-mkrescue run leaves a file in /tmp/

2015-12-29 Thread 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

2015-12-29 Thread 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

2015-12-29 Thread 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 v3 2/3] mkrescue: add argument --fixed-time to get reproducible uuids

2015-12-14 Thread 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, , 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 ();
>  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: [PATCH v3 3/3] Makefile/coreboot use SOURCE_DATE_EPOCH as time source if set

2015-12-14 Thread 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 1/3] mkstandalone: add argument --fixed-time to override mtime of files

2015-12-14 Thread 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 1/3] mkstandalone: add argument --fixed-time to override mtime of files

2015-12-04 Thread 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

2015-12-04 Thread 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: grub causing NVDIMMs to be treated as normal memory

2015-11-27 Thread 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

2015-11-27 Thread 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 get and set efi variables

2015-11-27 Thread 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: grub causing NVDIMMs to be treated as normal memory

2015-11-27 Thread 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: [PATCH v3 3/4] * util/grub.d/20_linux_xen.in: Add support of the XEN boot on aarch64

2015-11-12 Thread 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

2015-11-12 Thread 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 

Uniform commands for booting xen

2015-11-12 Thread 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 5/6] multiboot2: Add support for relocatable images

2015-11-12 Thread 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" <daniel.ki...@oracle.com> 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 <daniel.ki...@oracle.com>
>>>> 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


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

2015-11-12 Thread 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

2015-11-12 Thread 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: [PATCH v4] ofdisk: add sas disks to the device list

2015-11-11 Thread 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] Use UEFI Time Service to calibrate TSC

2015-11-09 Thread 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, _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

2015-11-09 Thread 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 <mch...@suse.com> 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 =
-	_efi_system_table->configuration_table[i].vendor_guid;
-
-  if (! grub_memcmp (guid, _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 =
-	_efi_system_table->configuration_table[i].vendor_guid;
-
-  if (! grub_memcmp (guid, _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 will be useful,
+ *  but WITHOUT ANY WARR

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

2015-11-09 Thread 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 v2 2/6] relocator: Do not use memory region if its starta is smaller than size

2015-11-09 Thread 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


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

2015-11-09 Thread 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 1/6] gitignore: Ignore *.orig, *.rej and *.swp files

2015-11-09 Thread 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" <ian.campb...@citrix.com> 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 <daniel.ki...@oracle.com>
>>>>>
>>>>> 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


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

2015-11-09 Thread 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" > 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 >
---
 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 (_mmap_size,
tag->efi_mmap, NULL,
-  _desc_size,
_desc_version);
-else
   {
-   if (grub_efi_get_memory_map (_mmap_size, (void *)
tag->efi_mmap,
-NULL,
-_desc_size,
_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 (_mmap_size,
tag->efi_mmap, NULL,
+_desc_size,
_desc_version);
+
+   if (err)
+ return err;
+
+   tag->descr_size = efi_desc_size;
+   tag->descr_vers = efi_desc_version;
+   tag->size = sizeof 

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

2015-11-09 Thread 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"
> 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" > 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 >
---
 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 (_mmap_size,
tag->efi_mmap, NULL,
-  _desc_size,
_desc_version);
-else
   {
-   if (grub_efi_get_memory_map (_mmap_size, (void *)
tag->efi_mmap,
-

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

2015-11-09 Thread 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)  (_relocator##x##_end - 
> _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, , 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), 
> _relocator64_efi_start,
> + RELOCATOR_SIZEOF (64_efi));
> +
> +  err = grub_relocator_prepare_relocs (rel, get_physical_target_address (ch),
> +, 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 

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

2015-11-09 Thread 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 4/6] multiboot2: Add tags used to pass ImageHandle to loaded image

2015-11-09 Thread 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];
>  };
> 
> 

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

2015-11-09 Thread 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: Anonymous commit (empty Author and Committer)

2015-10-30 Thread 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 v3 3/4] * util/grub.d/20_linux_xen.in: Add support of the XEN boot on aarch64

2015-10-30 Thread 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
> <phco...@gmail.com> 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)

2015-10-30 Thread 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)

2015-10-30 Thread 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

2015-10-30 Thread 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: [PATCH v3 1/4] arm64: Add and export some accessor functions for xen boot

2015-10-29 Thread 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: [edk2] [grub PATCH] efinet: disable MNP background polling

2015-10-29 Thread 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: [PATCH v3 3/4] * util/grub.d/20_linux_xen.in: Add support of the XEN boot on aarch64

2015-10-29 Thread 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 2/4] arm64: Add xen_boot module file

2015-10-29 Thread 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 

Re: Be File System identified as bfs

2015-10-29 Thread 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 v2] Suport for bi-endianess in elf file

2015-10-26 Thread 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 

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

2015-10-26 Thread 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, _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 (_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 (_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

2015-10-26 Thread 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

2015-10-26 Thread 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, _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] sparc64: boot performance improvements

2015-10-25 Thread 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
>>> +{
>>> +  

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

2015-10-25 Thread 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, );
> -
> -  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: Add check when store disk cache

2015-10-13 Thread 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

2015-10-13 Thread 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: GNU GRUB maintenance

2015-10-09 Thread 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 <arvidj...@gmail.com> 
> wrote:
>> On Thu, Oct 8, 2015 at 12:14 AM, Vladimir 'φ-coder/phcoder' Serbinenko
>> <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.
>>
>> 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: [PATCH v2] Ensure that MIPS target code is compiled for the O32 ABI.

2015-10-09 Thread 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

2015-10-09 Thread 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
> <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.
> 
> 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: [PATCH v2] Ensure that MIPS target code is compiled for the O32 ABI.

2015-10-09 Thread 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] sparc64: boot performance improvements

2015-10-09 Thread 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 (, "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 (_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 (, "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: GNU GRUB maintenance

2015-10-07 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 07.10.2015 23:36, SevenBits wrote:
> 
> 
> On Wednesday, October 7, 2015, Vladimir 'φ-coder/phcoder' Serbinenko
> <phco...@gmail.com <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


GNU GRUB maintenance

2015-10-07 Thread 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


  1   2   3   4   5   6   7   8   9   10   >