Re: [Xen-devel] [PATCH v2 10/18] xen: setup hypercall page for PVH

2018-10-19 Thread Andrew Cooper
On 19/10/18 13:30, Daniel Kiper wrote:
> On Tue, Oct 09, 2018 at 01:03:09PM +0200, Juergen Gross wrote:
>> +
>> +  __arg0 = a0;
>> +  __arg1 = a1;
>> +  __arg2 = a2;
>> +  __arg3 = a3;
>> +  __arg4 = a4;
>> +  asm volatile ("call *%[callno]"
>> +: "=r" (__res), "+r" (__arg0), "+r" (__arg1), "+r" (__arg2),
>> +  "+r" (__arg3), "+r" (__arg4)
>> +: [callno] "a" (&hypercall_page[callno])
>> +: "memory");

call hypercall_page + %c[offset]

passing [offset] "i" (callno * 32)

which gives you a direct call, rather than an indirect one.

~Andrew

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


Re: [Xen-devel] [PATCH 01/18] x86/boot/reloc: mask out MBI_BOOTDEV from mbi flags

2015-01-30 Thread Andrew Cooper
On 30/01/15 17:54, Daniel Kiper wrote:
> ..because it is ignored by Xen.
>
> Signed-off-by: Daniel Kiper 

Reviewed-by: Andrew Cooper 

> ---
>  xen/arch/x86/boot/reloc.c |1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
> index f971920..63045c0 100644
> --- a/xen/arch/x86/boot/reloc.c
> +++ b/xen/arch/x86/boot/reloc.c
> @@ -90,7 +90,6 @@ multiboot_info_t *reloc(multiboot_info_t *mbi_old)
>  
>  /* Mask features we don't understand or don't relocate. */
>  mbi->flags &= (MBI_MEMLIMITS |
> -   MBI_BOOTDEV |
> MBI_CMDLINE |
> MBI_MODULES |
> MBI_MEMMAP |


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


Re: [PATCH 02/18] x86/boot/reloc: create generic alloc and copy functions

2015-01-30 Thread Andrew Cooper
On 30/01/15 17:54, Daniel Kiper wrote:
> Create generic alloc and copy functions. We need
> separate tools for memory allocation and copy to
> provide multiboot2 protocol support.
>
> Signed-off-by: Daniel Kiper 

Reviewed-by: Andrew Cooper 

> ---
>  xen/arch/x86/boot/reloc.c |   52 
> -
>  1 file changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
> index 63045c0..f964cda 100644
> --- a/xen/arch/x86/boot/reloc.c
> +++ b/xen/arch/x86/boot/reloc.c
> @@ -33,9 +33,10 @@ asm (
>  typedef unsigned int u32;
>  #include "../../../include/xen/multiboot.h"
>  
> -static void *reloc_mbi_struct(void *old, unsigned int bytes)
> +static u32 alloc_struct(u32 bytes)
>  {
> -void *new;
> +u32 s;
> +
>  asm(
>  "call 1f  \n"
>  "1:  pop  %%edx   \n"
> @@ -43,50 +44,63 @@ static void *reloc_mbi_struct(void *old, unsigned int 
> bytes)
>  "sub  %1,%0   \n"
>  "and  $~15,%0 \n"
>  "mov  %0,alloc-1b(%%edx)  \n"
> -"mov  %0,%%edi\n"
> -"rep  movsb   \n"
> -   : "=&r" (new), "+c" (bytes), "+S" (old)
> - : : "edx", "edi", "memory");
> -return new;
> +   : "=&r" (s) : "r" (bytes) : "edx", "memory");
> +
> +return s;
> +}
> +
> +static u32 copy_struct(u32 src, u32 bytes)
> +{
> +u32 dst, dst_asm;
> +
> +dst = alloc_struct(bytes);
> +dst_asm = dst;
> +
> +asm volatile("rep movsb" : "+S" (src), "+D" (dst_asm), "+c" (bytes) : : 
> "memory");
> +
> +return dst;
>  }
>  
> -static char *reloc_mbi_string(char *old)
> +static u32 copy_string(u32 src)
>  {
>  char *p;
> -for ( p = old; *p != '\0'; p++ )
> +
> +if ( src == 0 )
> +return 0;
> +
> +for ( p = (char *)src; *p != '\0'; p++ )
>  continue;
> -return reloc_mbi_struct(old, p - old + 1);
> +
> +return copy_struct(src, p - (char *)src + 1);
>  }
>  
>  multiboot_info_t *reloc(multiboot_info_t *mbi_old)
>  {
> -multiboot_info_t *mbi = reloc_mbi_struct(mbi_old, sizeof(*mbi));
> +multiboot_info_t *mbi = (multiboot_info_t *)copy_struct((u32)mbi_old, 
> sizeof(*mbi));
>  int i;
>  
>  if ( mbi->flags & MBI_CMDLINE )
> -mbi->cmdline = (u32)reloc_mbi_string((char *)mbi->cmdline);
> +mbi->cmdline = copy_string(mbi->cmdline);
>  
>  if ( mbi->flags & MBI_MODULES )
>  {
> -module_t *mods = reloc_mbi_struct(
> -(module_t *)mbi->mods_addr, mbi->mods_count * sizeof(module_t));
> +module_t *mods = (module_t *)copy_struct(
> +mbi->mods_addr, mbi->mods_count * sizeof(module_t));
>  
>  mbi->mods_addr = (u32)mods;
>  
>  for ( i = 0; i < mbi->mods_count; i++ )
>  {
>  if ( mods[i].string )
> -mods[i].string = (u32)reloc_mbi_string((char 
> *)mods[i].string);
> +mods[i].string = copy_string(mods[i].string);
>  }
>  }
>  
>  if ( mbi->flags & MBI_MEMMAP )
> -mbi->mmap_addr = (u32)reloc_mbi_struct(
> -(memory_map_t *)mbi->mmap_addr, mbi->mmap_length);
> +mbi->mmap_addr = copy_struct(mbi->mmap_addr, mbi->mmap_length);
>  
>  if ( mbi->flags & MBI_LOADERNAME )
> -mbi->boot_loader_name = (u32)reloc_mbi_string(
> -(char *)mbi->boot_loader_name);
> +mbi->boot_loader_name = copy_string(mbi->boot_loader_name);
>  
>  /* Mask features we don't understand or don't relocate. */
>  mbi->flags &= (MBI_MEMLIMITS |


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


Re: [PATCH 04/18] xen/x86: add multiboot2 protocol support

2015-01-30 Thread Andrew Cooper
On 30/01/15 17:54, Daniel Kiper wrote:
> Add multiboot2 protocol support. Alter min memory limit handling as we
> now may not find it from either multiboot (v1) or multiboot2.
>
> This way we are laying the foundation for EFI + GRUB2 + Xen development.
>
> Signed-off-by: Daniel Kiper 

I have not reviewed the multiboot2 protocol in detail, but it appears
sane and I presume it is suitably tested and works.

As far as the mb1/mb2 interaction goes, this is looking far better.

Reviewed-by: Andrew Cooper 

> ---
>  xen/arch/x86/boot/Makefile|3 +-
>  xen/arch/x86/boot/head.S  |  104 --
>  xen/arch/x86/boot/reloc.c |  174 
> -
>  xen/arch/x86/x86_64/asm-offsets.c |6 ++
>  xen/include/xen/multiboot2.h  |  169 +++
>  5 files changed, 429 insertions(+), 27 deletions(-)
>  create mode 100644 xen/include/xen/multiboot2.h
>
> diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
> index 5fdb5ae..0efa7d2 100644
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -1,6 +1,7 @@
>  obj-bin-y += head.o
>  
> -RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h 
> $(BASEDIR)/include/xen/multiboot.h
> +RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h 
> $(BASEDIR)/include/xen/compiler.h \
> +  $(BASEDIR)/include/xen/multiboot.h 
> $(BASEDIR)/include/xen/multiboot2.h
>  
>  head.o: reloc.S
>  
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index 6180783..7861057 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -1,5 +1,6 @@
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -32,6 +33,59 @@ ENTRY(start)
>  .long   MULTIBOOT_HEADER_FLAGS
>  /* Checksum: must be the negated sum of the first two fields. */
>  .long   -(MULTIBOOT_HEADER_MAGIC + MULTIBOOT_HEADER_FLAGS)
> + 
> +/*** MULTIBOOT2 HEADER /
> +/* Some ideas are taken from grub-2.00/grub-core/tests/boot/kernel-i386.S 
> file. */
> +.align  MULTIBOOT2_HEADER_ALIGN
> +
> +.Lmultiboot2_header:
> +/* Magic number indicating a Multiboot2 header. */
> +.long   MULTIBOOT2_HEADER_MAGIC
> +/* Architecture: i386. */
> +.long   MULTIBOOT2_ARCHITECTURE_I386
> +/* Multiboot2 header length. */
> +.long   .Lmultiboot2_header_end - .Lmultiboot2_header
> +/* Multiboot2 header checksum. */
> +.long   -(MULTIBOOT2_HEADER_MAGIC + MULTIBOOT2_ARCHITECTURE_I386 + \
> +(.Lmultiboot2_header_end - .Lmultiboot2_header))
> +
> +/* Multiboot2 tags... */
> +.Lmultiboot2_info_req:
> +/* Multiboot2 information request tag. */
> +.short  MULTIBOOT2_HEADER_TAG_INFORMATION_REQUEST
> +.short  MULTIBOOT2_HEADER_TAG_REQUIRED
> +.long   .Lmultiboot2_info_req_end - .Lmultiboot2_info_req
> +.long   MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO
> +.long   MULTIBOOT2_TAG_TYPE_MMAP
> +.Lmultiboot2_info_req_end:
> +
> +.align  MULTIBOOT2_TAG_ALIGN
> +.short  MULTIBOOT2_HEADER_TAG_MODULE_ALIGN
> +.short  MULTIBOOT2_HEADER_TAG_REQUIRED
> +.long   8 /* Tag size. */
> +
> +/* Console flags tag. */
> +.align  MULTIBOOT2_TAG_ALIGN
> +.short  MULTIBOOT2_HEADER_TAG_CONSOLE_FLAGS
> +.short  MULTIBOOT2_HEADER_TAG_OPTIONAL
> +.long   12 /* Tag size. */
> +.long   MULTIBOOT2_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED
> +
> +/* Framebuffer tag. */
> +.align  MULTIBOOT2_TAG_ALIGN
> +.short  MULTIBOOT2_HEADER_TAG_FRAMEBUFFER
> +.short  MULTIBOOT2_HEADER_TAG_OPTIONAL
> +.long   20 /* Tag size. */
> +.long   0 /* Number of the columns - no preference. */
> +.long   0 /* Number of the lines - no preference. */
> +.long   0 /* Number of bits per pixel - no preference. */
> +
> +/* Multiboot2 header end tag. */
> +.align  MULTIBOOT2_TAG_ALIGN
> +.short  MULTIBOOT2_HEADER_TAG_END
> +.short  0
> +.long   8 /* Tag size. */
> +.Lmultiboot2_header_end:
>  
>  .section .init.rodata, "a", @progbits
>  .align 4
> @@ -81,10 +135,51 @@ __start:
>  mov %ecx,%es
>  mov %ecx,%ss
>  
> +/* Bootloaders may set multiboot[12].mem_lower to a nonzero value */
> +xor %edx,%edx
> +
>  /* Check for Multiboot bootloader */
>  cmp $MULTIBOOT_BOOTLOADER_MAGIC,%eax
> -jne not_multiboot
> +je  multiboot1_proto
>  
> +/

Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms

2015-01-30 Thread Andrew Cooper
On 30/01/15 17:54, Daniel Kiper wrote:
> Signed-off-by: Daniel Kiper 
> ---
>  xen/arch/x86/boot/head.S  |  174 
> +++--
>  xen/arch/x86/efi/efi-boot.h   |   29 +++
>  xen/arch/x86/setup.c  |   23 ++---
>  xen/arch/x86/x86_64/asm-offsets.c |2 +
>  xen/common/efi/boot.c |   11 +++
>  5 files changed, 222 insertions(+), 17 deletions(-)
>
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index 7861057..89f5aa7 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -8,6 +8,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  .text
>  .code32
> @@ -57,6 +58,9 @@ ENTRY(start)
>  .long   .Lmultiboot2_info_req_end - .Lmultiboot2_info_req
>  .long   MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO
>  .long   MULTIBOOT2_TAG_TYPE_MMAP
> +.long   MULTIBOOT2_TAG_TYPE_EFI_BS
> +.long   MULTIBOOT2_TAG_TYPE_EFI64
> +.long   MULTIBOOT2_TAG_TYPE_EFI64_IH
>  .Lmultiboot2_info_req_end:
>  
>  .align  MULTIBOOT2_TAG_ALIGN
> @@ -80,6 +84,19 @@ ENTRY(start)
>  .long   0 /* Number of the lines - no preference. */
>  .long   0 /* Number of bits per pixel - no preference. */
>  
> +/* Do not disable EFI boot services. */
> +.align  MULTIBOOT2_TAG_ALIGN
> +.short  MULTIBOOT2_HEADER_TAG_EFI_BS
> +.short  MULTIBOOT2_HEADER_TAG_OPTIONAL
> +.long   8 /* Tag size. */
> +
> +/* EFI64 entry point. */
> +.align  MULTIBOOT2_TAG_ALIGN
> +.short  MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS_EFI64
> +.short  MULTIBOOT2_HEADER_TAG_OPTIONAL
> +.long   12 /* Tag size. */
> +.long   sym_phys(__efi64_start)
> +
>  /* Multiboot2 header end tag. */
>  .align  MULTIBOOT2_TAG_ALIGN
>  .short  MULTIBOOT2_HEADER_TAG_END
> @@ -94,6 +111,17 @@ ENTRY(start)
>  gdt_boot_descr:
>  .word   6*8-1
>  .long   sym_phys(trampoline_gdt)
> +.long   0 /* Needed for 64-bit lgdt */
> +
> +cs32_switch_addr:
> +.long   sym_phys(cs32_switch)
> +.long   BOOT_CS32

.word

ljmpl refers to an m32:16 not an m32:32

> +
> +efi_st:
> +.quad   0
> +
> +efi_ih:
> +.quad   0
>  
>  .Lbad_cpu_msg: .asciz "ERR: Not a 64-bit CPU!"
>  .Lbad_ldr_msg: .asciz "ERR: Not a Multiboot bootloader!"
> @@ -124,6 +152,133 @@ print_err:
>  .Lhalt: hlt
>  jmp .Lhalt
>  
> +.code64
> +
> +__efi64_start:
> +cld
> +
> +/* Bootloaders may set multiboot[12].mem_lower to a nonzero value */
> +xor %edx,%edx
> +
> +/* Check for Multiboot2 bootloader */
> +cmp $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
> +je  efi_multiboot2_proto
> +
> +jmp not_multiboot

not_multiboot is 32bit code.  You must drop out of 64bit before using
it, or make a 64bit variant.

> +
> +efi_multiboot2_proto:
> +/* Skip Multiboot2 information fixed part */
> +lea MB2_fixed_sizeof(%ebx),%ecx
> +
> +0:
> +/* Get mem_lower from Multiboot2 information */
> +cmpl$MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,(%ecx)
> +jne 1f
> +
> +mov MB2_mem_lower(%ecx),%edx
> +jmp 4f
> +
> +1:
> +/* Get EFI SystemTable address from Multiboot2 information */
> +cmpl$MULTIBOOT2_TAG_TYPE_EFI64,(%ecx)
> +jne 2f
> +
> +lea MB2_efi64_st(%ecx),%esi
> +lea efi_st(%rip),%edi
> +movsq

This is complete overkill for copying a 64bit variable out of the tag
and into a local variable.  Just use a plain 64bit load and store.

> +
> +/* Do not go into real mode on EFI platform */
> +movb$1,skip_realmode(%rip)
> +
> +jmp 4f
> +
> +2:
> +/* Get EFI ImageHandle address from Multiboot2 information */
> +cmpl$MULTIBOOT2_TAG_TYPE_EFI64_IH,(%ecx)
> +jne 3f
> +
> +lea MB2_efi64_ih(%ecx),%esi
> +lea efi_ih(%rip),%edi
> +movsq

And here.

> +jmp 4f
> +
> +3:
> +/* Is it the end of Multiboot2 information? */
> +cmpl$MULTIBOOT2_TAG_TYPE_END,(%ecx)
> +je  run_bs
> +
> +4:
> +/* Go to next Multiboot2 information tag */
> +add MB2_tag_size(%ecx),%ecx
> +add $(MULTIBOOT2_TAG_ALIGN-1),%ecx
> +and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
> +jmp 0b
> +
> +run_bs:
> +push%rax
> +push%rdx

Does the EFI spec guarantee that we have a good stack to use at this point?

> +
> +/* Initialize BSS (no nasty surprises!) */
> +lea __bss_start(%rip),%rdi
> +lea _end(%rip),%rcx
> +sub %rdi,%rcx
> +xor %rax,%rax

xor %eax,%eax is shorter.

> +rep stosb

It would be more efficient to make sure that the linker aligns
__bss_start and _end on 8 byte boundaries, and use stos

Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms

2015-01-30 Thread Andrew Cooper
On 30/01/2015 23:43, Daniel Kiper wrote:
> On Fri, Jan 30, 2015 at 07:06:53PM +0000, Andrew Cooper wrote:
>> On 30/01/15 17:54, Daniel Kiper wrote:
>>> +
>>> +efi_multiboot2_proto:
>>> +/* Skip Multiboot2 information fixed part */
>>> +lea MB2_fixed_sizeof(%ebx),%ecx
>>> +
>>> +0:
>>> +/* Get mem_lower from Multiboot2 information */
>>> +cmpl$MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,(%ecx)
>>> +jne 1f
>>> +
>>> +mov MB2_mem_lower(%ecx),%edx
>>> +jmp 4f
>>> +
>>> +1:
>>> +/* Get EFI SystemTable address from Multiboot2 information */
>>> +cmpl$MULTIBOOT2_TAG_TYPE_EFI64,(%ecx)
>>> +jne 2f
>>> +
>>> +lea MB2_efi64_st(%ecx),%esi
>>> +lea efi_st(%rip),%edi
>>> +movsq
>> This is complete overkill for copying a 64bit variable out of the tag
>> and into a local variable.  Just use a plain 64bit load and store.
> I am not sure what do you mean by "64bit load and store" but I have
> just realized that we do not need these variables. They are remnants
> from early developments when I thought that we need ImageHandle
> and SystemTable here and later somewhere else.

mov MB2_efi64_st(%rcx), %rdi
mov %rdi, efi_st(%rip)

But if they are not needed, drop the code completely.

>>> +jmp 4f
>>> +
>>> +3:
>>> +/* Is it the end of Multiboot2 information? */
>>> +cmpl$MULTIBOOT2_TAG_TYPE_END,(%ecx)
>>> +je  run_bs
>>> +
>>> +4:
>>> +/* Go to next Multiboot2 information tag */
>>> +add MB2_tag_size(%ecx),%ecx
>>> +add $(MULTIBOOT2_TAG_ALIGN-1),%ecx
>>> +and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
>>> +jmp 0b
>>> +
>>> +run_bs:
>>> +push%rax
>>> +push%rdx
>> Does the EFI spec guarantee that we have a good stack to use at this point?
> Unified Extensible Firmware Interface Specification, Version 2.4 Errata B,
> section 2.3.4, x64 Platforms says: During boot services time the processor
> is in the following execution mode: ..., 128 KiB, or more, of available
> stack space. GRUB2 uses this stack too and do not move it to different
> memory region. So, I think that here we are on safe side.

Sounds ok then.

>
>>> +/* Initialize BSS (no nasty surprises!) */
>>> +lea __bss_start(%rip),%rdi
>>> +lea _end(%rip),%rcx
>>> +sub %rdi,%rcx
>>> +xor %rax,%rax
>> xor %eax,%eax is shorter.
>>
>>> +rep stosb
>> It would be more efficient to make sure that the linker aligns
>> __bss_start and _end on 8 byte boundaries, and use stosq instead.
> Right but just for this. Is it pays? We do this only once.

The BSS in Xen is 300k.  It is absolutely better to clear it 8 bytes at
a time rather than 1.

> However, if you wish...
>
>>> +mov efi_ih(%rip),%rdi   /* EFI ImageHandle */
>>> +mov efi_st(%rip),%rsi   /* EFI SystemTable */
>>> +callefi_multiboot2
>>> +
>>> +pop %rcx
>>> +pop %rax
>>> +
>>> +shl $10-4,%rcx  /* Convert multiboot2.mem_lower to 
>>> bytes/16 */
>>> +
>>> +cli
>> This looks suspiciously out of place.  Surely the EFI spec doesn't
>> permit entry with interrupts enabled?
> Unified Extensible Firmware Interface Specification, Version 2.4 Errata B,
> section 2.3.4, x64 Platforms says: During boot services time the processor
> is in the following execution mode: ..., Interrupts are enabled–though no
> interrupt services are supported other than the UEFI boot services timer
> functions (All loaded device drivers are serviced synchronously by 
> “polling.”).
> So, I think that we should use BS with interrupts enabled and disable
> them after ExitBootServices(). Hmmm... Now I think that we should use
> cli immediately after efi_multiboot2() call.

I presume then that the firmware has set up a valid idt somewhere and is
actually serving any interrupts we get.

> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index f8be3dd..c5725ca 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -75,6 +75,17 @@ static size_t wstrlen(const CHAR16 * s);
>  static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
>  static bool_t match_guid(const EFI_GUID *guid1, const EFI_GUID *

Re: [PATCH 00/18] x86: multiboot2 protocol support

2015-02-04 Thread Andrew Cooper
On 03/02/2015 17:14, Daniel Kiper wrote:
> On Mon, Feb 02, 2015 at 09:28:49AM +, Jan Beulich wrote:
> On 30.01.15 at 18:54,  wrote:
>>>   - xen.efi build will not so strongly depend
>>> on a given GCC and binutils version.
>> While I can see the possibility of making the binutils version
>> dependency go away (by manually creating the PE header), I can't
>> see how you'd overcome the gcc one: The MS calling convention
>> support is still going to be needed (not having looked at the patches
> Right, I forgot about that one.
>
>> themselves yet, I can't see myself accepting the introduction of
>> stubs to convert between calling conventions).

How about __attribute__((ms_abi)) ?  It would appear to exist for this
purpose.

~Andrew

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


Re: [PATCH 00/18] x86: multiboot2 protocol support

2015-02-05 Thread Andrew Cooper
On 04/02/15 09:51, Jan Beulich wrote:
 On 04.02.15 at 10:04,  wrote:
>> On 03/02/2015 17:14, Daniel Kiper wrote:
>>> On Mon, Feb 02, 2015 at 09:28:49AM +, Jan Beulich wrote:
>>> On 30.01.15 at 18:54,  wrote:
>   - xen.efi build will not so strongly depend
> on a given GCC and binutils version.
 While I can see the possibility of making the binutils version
 dependency go away (by manually creating the PE header), I can't
 see how you'd overcome the gcc one: The MS calling convention
 support is still going to be needed (not having looked at the patches
>>> Right, I forgot about that one.
>>>
 themselves yet, I can't see myself accepting the introduction of
 stubs to convert between calling conventions).
>> How about __attribute__((ms_abi)) ?  It would appear to exist for this
>> purpose.
> But that's the point: Older compilers don't support it. And with
> compilers supporting it we need no stubs.

If the use of __attribute__((ms_abi)) was suitably contained within a
#ifdef CONFIG_EFI, I don't see an problem.  CONFIG_EFI could derive
primarily from a compiler version check if we don't wish to force a
minimum newer version of gcc.

One way or another, a newer set of tools is needed to build EFI support,
and a binary capable of both legacy and efi boot is quite desirable to have.

~Andrew

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


Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms

2015-02-10 Thread Andrew Cooper
On 10/02/2015 21:27, Daniel Kiper wrote:
> On Fri, Jan 30, 2015 at 06:54:22PM +0100, Daniel Kiper wrote:
>> Signed-off-by: Daniel Kiper 
>> ---
>>  xen/arch/x86/boot/head.S  |  174 
>> +++--
>>  xen/arch/x86/efi/efi-boot.h   |   29 +++
>>  xen/arch/x86/setup.c  |   23 ++---
>>  xen/arch/x86/x86_64/asm-offsets.c |2 +
>>  xen/common/efi/boot.c |   11 +++
>>  5 files changed, 222 insertions(+), 17 deletions(-)
> After some testing we have found at least one machine on which this thing
> does not work. It is Dell PowerEdge R820 with latest firmware. Machine
> crashes/stops because early 32-bit code is not relocatable and must live
> under 0x10 address. (side note: I am surprised how it worked without
> any issue until now; Multiboot protocol, any version, does not guarantee
> that OS image will be loaded at specified/requested address; So, it looks
> that there are not any legacy BIOS machines with e.g. 1 MiB - 2 MiB region
> reserved in the wild or they are not very common; Am I missing something?).
> Sadly, this region is used by BS, so, GRUB2 loads Xen higher (at least
> above 64 MiB). Please look at memory map on this machine:
>
> Type   StartEnd  # Pages  Attributes
> BS_Data0001-0009 0090 000F
> BS_Data0010-03FF 3F00 000F
> Available  0400-0FFFEFFF BFFF 000F
> BS_Code0000-1006CFFF 006E 000F
> Available  1006D000-B3E73FFF 000A3E07 000F
>
> [...]
>
> Additionally, early Xen boot code maps only first 16 MiB of memory. Hence,
> it means that jump into __high_start fails immediately.
>
> Now I see two solutions for these issues:
>
> 1) We can make early 32-bit code relocatable. We may use something similar
>to xen/arch/x86/boot/trampoline.S:bootsym_rel(). Additionally, I think
>that early code should not blindly map first 16 MiB of memory. It should
>map first 1 MiB of memory and then 16 MiB of memory starting from
>xen_phys_start. This way we also fix long standing bug in early code
>which I described earlier.
>
> 2) We can jump from EFI x86-64 mode directly into "Xen x86-64 mode" like
>it is done in case of EFI loader. However, then we must duplicate 
> multiboot2
>protocol implementation in x86-64 mode (if we wish that multiboot2 protocol
>can be used on legacy BIOS and EFI platforms; I think that we should 
> support
>this protocol on both for users convenience). Additionally, we must use
>a workaround to relocate trampoline if boot services uses memory below 1 
> MiB
>(please check commit c1f2dfe8f6a559bc28935f24e31bb33d17d9713d, x86/EFI: 
> make
>trampoline allocation more flexible, for more details).
>
> I prefer #1 because this way we do not duplicate multiboot2 protocol 
> implementation
> (one for legacy BIOS and EFI) and we avoid issues with trampoline relocation 
> when
> low memory is occupied by boot services and/or 1:1 EFI page tables.
>
> Daniel
>
> PS I have just realized that commit c1f2dfe8f6a559bc28935f24e31bb33d17d9713d
>will not work if trampoline code will overwrite some of EFI 1:1 page 
> tables.
>Dell PowerEdge R820 store part of 1:1 page tables below 1 MiB. Xen loaded
>by native EFI loader boots but it is only lucky coincidence that it does
>not overwrite used entries. So, I tend to go and choose #1 even more.

I have to admit to also being surprised at the fragility of the Xen, and
how it cannot function if _start isn't loaded on the 1MB boundary.

Given that there is provably one system in the wild which causes us to
fall over this limitation, it clearly needs fixing.

I would also agree that making the entry point relocatable is the
correct way forwards.  However, you cannot use something like
bootsym_rel() as that requires infrastructure to patch the references
(observe the loop over __trampoline_rel_{start,end} just before the call
to cmdline_parse_early())

This can probably be achieved by having a small bit of hand-written PIC
which puts an appropriate offset into %ds.

~Andrew

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


Re: [PATCH 08/18] efi: build xen.gz with EFI code

2015-03-27 Thread Andrew Cooper

On 27/03/15 11:46, Jan Beulich wrote:

On 27.03.15 at 12:14,  wrote:

IIRC, MS ABI is supported starting from GCC v4.0.

Where did you find that? From all I know __attribute__((__ms_abi__))
is being supported only by 4.5 and newer. The mere support of the
MS ABI via command line option doesn't help us, as we need to be
able to mix ABIs within a single source file.

Jan



As I have indicated elsewhere (but can't seem to locate - it was a while 
ago), I think it is perfectly reasonable to have a CONFIG_EFI which is 
only enabled if the makefile detects GCC >= 4.5


That way, older build environments don't get EFI support, while newer 
ones do, and newer systems can safely use __attribute__((__ms_abi__)) to 
make a mixed abi binary.


~Andrew

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


Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader

2015-03-27 Thread Andrew Cooper

On 27/03/15 13:43, Jan Beulich wrote:

On 27.03.15 at 14:32,  wrote:

On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote:

On 30.01.15 at 18:54,  wrote:

We need more fine grained knowledge about EFI environment and check
for EFI platform and EFI loader separately to properly support
multiboot2 protocol.

... because of ... (i.e. I can't see from the description what the
separation is good for). Looking at the comments you placed
aside the variables doesn't help me either.


In general Xen loaded by this protocol uses
memory mappings and loaded modules in simliar way to Xen loaded
by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform
and efi_loader.

And if I'm guessing things right, then introducing efi_loader but
leaving efi_enabled alone (only converting where needed) would

efi_enabled is not fortunate name for new usage. Currently it means
that Xen binary have or does not have EFI support build in. However,
if we build in multiboot2 protocol into xen.gz then it means that
it can ran on legacy BIOS or EFI platform. So, I think that we
should rename efi_enabled to efi_platform because it will mean
that Xen runs on EFI platform or not.

I disagree here.


efi_loader is used to differentiate between EFI native loader
and multiboot2 protocol.

And I agree here.


I suppose "built with efi support" is known because of CONFIG_EFI or 
not, and doesn't need a variable.


However, "booted legacy" vs "booted EFI" does need distinguishing at 
runtime, as either is possible.


~Andrew

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


Re: [Xen-devel] [PATCH v2 08/23] x86: add multiboot2 protocol support

2015-08-14 Thread Andrew Cooper



On 14/08/15 11:03, Jan Beulich wrote:

On 13.08.15 at 21:22,  wrote:

On Mon, Aug 10, 2015 at 03:17:48PM -0400, Konrad Rzeszutek Wilk wrote:

On Mon, Jul 20, 2015 at 04:29:03PM +0200, Daniel Kiper wrote:

@@ -34,6 +57,42 @@ multiboot1_header_start:   /*** MULTIBOOT1 HEADER /
  .long   -(MULTIBOOT_HEADER_MAGIC + MULTIBOOT_HEADER_FLAGS)
  multiboot1_header_end:

+/*** MULTIBOOT2 HEADER /
+/* Some ideas are taken from grub-2.00/grub-core/tests/boot/kernel-i386.S 
file. */
+.align  MULTIBOOT2_HEADER_ALIGN
+
+.Lmultiboot2_header:

How come you use .L? It makes this hidden while the multiboot1 headers
are visible? Makes it a bit harder to see the contents of this under
an debugger.

Good point. IIRC, Jan asked about that. I will remove .L if he does not
object.

For this particular one I think it's okay to drop the .L, but generally
I'd like to see .L used more widely for any auxiliary labels (i.e. only
"main" labels - function entry points and data objects - should have
their labels present in the final symbol table).


In general I would agree.

However, the multiboot 1 and 2 headers are special.  They are binary 
data included in the .text section, so having non-local lables makes the 
disassembly easier to read.


~Andrew

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


Re: [PATCH v2 10/23] efi: build xen.gz with EFI code

2015-08-25 Thread Andrew Cooper
On 24/08/15 21:54, Daniel Kiper wrote:
>
> Currently, PE file contains many sections which are not "linear" (one
> after another without any holes) or even do not have representation
> in a file (e.g. BSS). In theory there is a chance that we could build
> proper PE file using current build system. However, it means that
 What is "improper" about the currently built PE file? And if there is
 anything improper, did you inform the binutils maintainers of the
 problem?
>>> From PE loader point of view everything is OK. However, current Xen PE
>>> image (at least build on my machines) is not usable by multiboot (v1)
>>> or multiboot2 protocol compatible loader because it is not linear (one
>>> section does not live immediately after another without any voids).
>> Again - either I'm missing something (and then your explanation is
>> not good enough) or this is (as said above) a pointless adjustment.
> Let's focus on multiboot2 protocol (multiboot (v1) is similar to multiboot2
> in discussed case). In general multiboot2 is able to load any file which has:
>   1. proper multiboot2 header in first 32 KiB of a given file,
>   2. "the text and data segments must be consecutive in the OS image"
>  (The Multiboot Specification version 1.6).

What is the reasoning for the restriction in 2. ?

~Andrew

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


Re: [PATCH v2 22/23] x86: make Xen early boot code relocatable

2015-08-27 Thread Andrew Cooper
On 27/08/15 16:29, Jan Beulich wrote:
 On 27.08.15 at 17:10,  wrote:
>> On Thu, Aug 27, 2015 at 07:12:38AM -0600, Jan Beulich wrote:
>> On 20.07.15 at 16:29,  wrote:
  /* Copy bootstrap trampoline to low memory, below 1MB. */
 -mov $sym_phys(trampoline_start),%esi
 +lea sym_offset(trampoline_start)(%ebp),%esi
  mov $trampoline_end - trampoline_start,%ecx
  rep movsb

>>> The latest at this final hunk I'm getting tired (and upset). I'd much
>>> rather not touch all this code in these fragile ways, and instead
>>> require Xen to be booted without grub2 on badly written firmware
>>> like the one on the machine you quote in the description.
>> Let's start discussion from here. My further work on this patch series
>> is pointless until we do not agree details in this case.
>>
>> So, I agree that any software (including firmware) should not use
>> precious resources (i.e. low memory in that case) if it is not strictly
>> required. And I do not think so that latest UEFI implementations
>> on new hardware really need this low memory to survive (at least page
>> tables could be put anywhere above low memory). However, in case of
>> UEFI it is a wish of smart people not requirement set by spec. I was
>> checking UEFI docs a few times but I was not able to find anything
>> which says that e.g. "...developer MUST allocate memory outside of low
>> memory ...". Even I have not found any suggestion about that. However,
>> I must admit that I do not know whole UEFI spec, so, if you know something
>> which is similar to above please tell me where it is (title, revision,
>> page, paragraph, etc.). Hence, if there is not strict requirement in
>> regards to memory allocation in specs we are lost. Of course we can
>> encourage people to not do nasty things but I do not believe that many
>> will listen. So, sadly, I suppose that we will see more and more machines
>> in the wild which are "broken" (well, let's say do not align to good 
>> practices).
>>
>> On the other hand I think that we should not assume that a given memory
>> region (in our case starting at 1 MiB) is (or will be) available in every
>> machine. I have not seen anything which says that it is true. We should
>> stop doing that even if it works right now. I think that it works by
>> chance and there is a chance that it will stop working one day because
>> somebody will discover that he or she can put there e.g. device or hole.
>>
>> Last but not least. I suppose that Xen users and especially distros will
>> complain when they are not able to use GRUB2 with Xen on every platform
>> just because somebody (i.e. platform designers, developers, or whatever)
>> do not accept our beliefs or we require that platform must obey rules
>> (i.e. memory map requirements) which are specified nowhere.
> You're right, there's no such requirement on memory use in the spec.
> But you're missing the point. Supporting grub2 on UEFI is already a
> hack (ignoring all intentions EFI had from its first days). And now
> you've found an environment where that hack needs another hack
> (in Xen) to actually work. That's too much hackery for my taste, the
> more that things on this system can (afaict) work quite okay (without
> grub2, or with using its chainloader mechanism).
>
> So no, I'm still not convinced of the need for this patch.

I disagree.  There are many things a grub environment gives us which
plain EFI doesn't, most notably a familiar booting environment and
recovery facilities for users (which vastly trumps how EFI expects to
run things).

Furthermore, it offers common management of /boot in the dom0 filesystem
between legacy and EFI boots.

Therefore I am very much +1 get grub working.

~Andrew

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


Re: [PATCH v2 22/23] x86: make Xen early boot code relocatable

2015-08-28 Thread Andrew Cooper
On 28/08/15 07:54, Jan Beulich wrote:
>
>> Therefore I am very much +1 get grub working.
> Then you kind of misunderstood: I'm not against getting grub2
> working (i.e. patches prior to this one are fine in principle). What
> I'm against is hacking around firmware+grub2 combinations not
> suitable for Xen's purposes (where one could argue which of the
> three is really at fault, and I'm far from convinced it's Xen).

Irrespective of the EFI angle, there are other good reasons for making
Xens boot code position-independent and able to be located elsewhere.

The region of memory between 0x1 and 0x10fffe is at increased risk
of clobbering from buggy firmwares.  I have submitted bugfixes to Xen
before, and there are further investigations which didn't result in
anything upstream.

Moving the Xen text section out of this high risk region would make even
legacy boot more reliable.  Currently IMO it is a Xen bug that it
strictly must be placed at the 1MB boundary to boot.

~Andrew

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


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

2015-11-12 Thread Andrew Cooper
On 12/11/15 15:44, Jan Beulich wrote:
 On 12.11.15 at 14:41,  wrote:
>> 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 ???
> xen_ucode (and we might add more going forward). I don't see
> why the multiboot mechanism (kernel plus any number of modules)
> can't be used, without adding any Xen-specific directives.

I also don't see why multiple directives are necessary or useful.

All that matters is that the xen binary gets started at any of its
optional entry points, and that there are one or more modules loaded.

~Andrew

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


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

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

~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[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

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

2016-01-22 Thread Andrew Cooper
On 22/01/16 13:08, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> 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.

This is from my microkernel framework, and is probably the most succinct
code implementation:

http://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen-test-framework.git;a=blob;f=arch/x86/pv/traps.c;h=7f9a1908d260659c10f5cbb1d2d234c9fea1edb5;hb=HEAD#l31

The hypercall ABI documentation is:

http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/include/public/arch-x86/xen.h;h=cdd93c1c6446a92e89188c6a5132538188825d27;hb=refs/heads/staging#l126

~Andrew

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


Re: [MULTIBOOT2 DOC PATCH 01/10] multiboot2: Remove redundant if

2016-06-09 Thread Andrew Cooper
On 09/06/2016 21:30, Daniel Kiper wrote:
> Signed-off-by: Daniel Kiper 
> ---
>  doc/multiboot.texi |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> index 4b92918..27e5a2f 100644
> --- a/doc/multiboot.texi
> +++ b/doc/multiboot.texi
> @@ -425,7 +425,7 @@ u32 | size  |
>  
>  @samp{type} is divided into 2 parts. Lower contains an identifier of 
> contents of the rest of the tag.
>  @samp{size} contains the size of tag including header fields.
> -If bit @samp{0} of @samp{flags} (also known as @samp{optional}) is set if 
> bootloader may ignore this tag if it 
> +If bit @samp{0} of @samp{flags} (also known as @samp{optional}) is set 
> bootloader may ignore this tag if it 

As a native English speaker, I think you want to s/if/, the/ in this
case.  Neither option currently parses.

~Andrew

>  lacks relevant support.
>  Tags are terminated by a tag of type @samp{0} and size @samp{8}.
>  


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


Re: [Xen-devel] [MULTIBOOT2 DOC PATCH 02/10] multiboot2: Clarify meaning of information request header tag

2016-06-09 Thread Andrew Cooper
On 09/06/2016 21:30, Daniel Kiper wrote:
> Signed-off-by: Daniel Kiper 
> ---
>  doc/multiboot.texi |   20 
>  1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> index 27e5a2f..a7e3584 100644
> --- a/doc/multiboot.texi
> +++ b/doc/multiboot.texi
> @@ -443,15 +443,19 @@ u32[n]  | mbi_tag_types |
>  @end group
>  @end example
>  
> -@samp{mbi_tag_types} is an array of u32 each one representing an information
> -request
> -If this tag is present and @samp{optional} is set to @samp{0} information
> -conveyed by requested tag types must be present. If bootloader is unable
> -to supply this information it must fail with an error
> +@samp{mbi_tag_types} is an array of u32 each one representing an information 
> request.

"u32's, each"

>  
> -Note: it doesn't garantee that any tags of type @samp{mbi_tag_types} will
> -actually be present. E.g. on a videoless system even if you requested tag
> -@samp{8} no tags of type @samp{8} will be present in mbi.
> +If this tag is present and @samp{optional} is set to @samp{0} bootloader must

", the bootloader"

> +support (understand meaning of) requested tag(s) and be able to provide 
> relevant

"the requested".  I don't think you need to explain what supported
means, so I would just drop the brackets entirely.

> +information to image if it is available. If bootloader do not understand 
> meaning

"the image".  "the bootloader does not". "the meaning".

> +of requested tag(s) it must fail with an error. However, if it support a 
> given

"the requested".  "supports".

> +tag(s) but information conveyed by it/them is not available bootloader can 
> do not

"a given tag(s)" is an odd way of phrasing this.  I would recommend just
"supports a given tag, but the information requested by it is".

"available, the bootloader can't provide the requested".

> +provide requested tag(s) in Multiboot information structure and proceed 
> further.

"in the Multiboot".  What do you mean by "and proceed further" in this
case?  It also doesn't parse.  I presume you mean that it is legal for
the bootloader to not provide the requested information, but may
continue booting.

> +
> +Note: Above means that there is not guarantee that any tags of type 
> @samp{mbi_tag_types}

"The above", "there is no guarentee"

> +will actually be present. E.g. on a videoless system even if you requested 
> tag @samp{8}
> +and bootloader support it no tags of type @samp{8} will be present in 
> Multiboot

"the bootloader supports it, no".  "The Multiboot".

~Andrew

> +information structure.
>  
>  
>  @node Address header tag


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


Re: [MULTIBOOT2 DOC PATCH 04/10] multiboot2: Add description of support for EFI boot services

2016-06-09 Thread Andrew Cooper
On 09/06/2016 21:30, Daniel Kiper wrote:
> Signed-off-by: Daniel Kiper 
> ---
>  doc/multiboot.texi |  108 
> +++-
>  doc/multiboot2.h   |2 +
>  2 files changed, 108 insertions(+), 2 deletions(-)
>
> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> index e43df93..1583b76 100644
> --- a/doc/multiboot.texi
> +++ b/doc/multiboot.texi
> @@ -534,6 +534,62 @@ The physical address to which the boot loader should 
> jump in order to
>  start running the operating system.
>  @end table
>  
> +@subsection EFI i386 entry address tag of Multiboot header
> +
> +@example
> +@group
> ++---+
> +u16 | type = 8  |
> +u16 | flags |
> +u32 | size  |
> +u_virt  | entry_addr|
> ++---+
> +@end group
> +@end example
> +
> +All of the address fields in this tag are physical addresses.
> +The meaning of each is as follows:
> +
> +@table @code
> +
> +@item entry_addr
> +The physical address to which the boot loader should jump in order to
> +start running EFI i386 compatible operating system code.
> +@end table
> +
> +This tag is taken into account only on EFI i386 platforms
> +when Multiboot image header contains EFI boot services tag.
> +Then entry point specified in ELF header and the entry address
> +tag of Multiboot header are ignored.
> +
> +@subsection EFI amd64 entry address tag of Multiboot header
> +
> +@example
> +@group
> ++---+
> +u16 | type = 9  |
> +u16 | flags |
> +u32 | size  |
> +u_virt  | entry_addr|
> ++---+
> +@end group
> +@end example
> +
> +All of the address fields in this tag are physical addresses.
> +The meaning of each is as follows:

A 64bit entry cannot possibly be a physical, as paging is mandatory.

This is clearly supposed to be the entry virtual address, but it should
not be conflated with the position of the image in RAM.  The chances of
the two being the same is very small.

> +
> +@table @code
> +
> +@item entry_addr
> +The physical address to which the boot loader should jump in order to
> +start running EFI amd64 compatible operating system code.
> +@end table
> +
> +This tag is taken into account only on EFI amd64 platforms
> +when Multiboot image header contains EFI boot services tag.
> +Then entry point specified in ELF header and the entry address
> +tag of Multiboot header are ignored.
> +
>  @node Console header tags
>  @subsection Flags tag
>  
> @@ -714,12 +770,60 @@ The OS image must leave interrupts disabled until it 
> sets up its own
>  
>  On EFI system boot services must be terminated.
>  
> +@section EFI i386 machine state with boot services enabled
> +
> +When the boot loader invokes the 32-bit operating system on EFI i386
> +platform and EFI boot services tag together with EFI i386 entry address
> +tag are present in the image Multiboot header, the machine must have the
> +following state:
> +
> +@table @samp
> +@item EAX
> +Must contain the magic value @samp{0x36d76289}; the presence of this
> +value indicates to the operating system that it was loaded by a
> +Multiboot-compliant boot loader (e.g. as opposed to another type of

Multiboot 2, not multiboot.

> +boot loader that the operating system can also be loaded from).
> +
> +@item EBX
> +Must contain the 32-bit physical address of the Multiboot
> +information structure provided by the boot loader (@pxref{Boot
> +information format}).
> +@end table
> +
> +All other processor registers, flag bits and state are set accordingly
> +to Unified Extensible Firmware Interface Specification, Version 2.6,
> +section 2.3.2, IA-32 Platforms, boot services.
> +
> +@section EFI amd64 machine state with boot services enabled
> +
> +When the boot loader invokes the 64-bit operating system on EFI amd64
> +platform and EFI boot services tag together with EFI amd64 entry address
> +tag are present in the image Multiboot header, the machine must have the
> +following state:
> +
> +@table @samp
> +@item RAX
> +Must contain the magic value @samp{0x36d76289}; the presence of this
> +value indicates to the operating system that it was loaded by a
> +Multiboot-compliant boot loader (e.g. as opposed to another type of

Again, Multiboot 2.

> +boot loader that the operating system can also be loaded from).
> +
> +@item RBX
> +Must contain the 64-bit physical address of the Multiboot

This is surely a virtual address, unless you expect a multiboot payload
to edit its own pagetables before it can read the info structure.

> +information structure provided by the boot loader (@pxref{Boot
> +information format}).
> +@end table
> +
> +All other processor registers, flag bits and state are set accordingly
> +to Unified Extensible Firmware Interface Specification, Version 2.6,
> +section 2.3.4, x64 Platforms, boot services.
> +
>  @node Boot information format
>  @section Boot information
>  @subsection

Re: [Xen-devel] [MULTIBOOT2 DOC PATCH 06/10] multiboot2: Add description of support for relocatable images

2016-06-09 Thread Andrew Cooper
On 09/06/2016 21:30, Daniel Kiper wrote:
> Signed-off-by: Daniel Kiper 
> ---
>  doc/multiboot.texi |   56 
> 
>  doc/multiboot2.h   |   24 ++
>  2 files changed, 80 insertions(+)
>
> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> index 130176a..f1e0e09 100644
> --- a/doc/multiboot.texi
> +++ b/doc/multiboot.texi
> @@ -359,6 +359,7 @@ executable header.
>  * Console header tags::
>  * Module alignment tag::
>  * EFI boot services tag::
> +* Relocatable header tag::
>  
>  @end menu
>  
> @@ -681,6 +682,47 @@ u32 | size = 8  |
>  This tag indicates that payload supports starting without
>  terminating boot services.
>  
> +@node Relocatable header tag
> +@subsection Relocatable header tag
> +
> +@example
> +@group
> ++---+
> +u16 | type = 10 |
> +u16 | flags |
> +u32 | size = 24 |
> +u32 | min_addr  |
> +u32 | max_addr  |
> +u32 | align |
> +u32 | preference|
> ++---+
> +@end group
> +@end example
> +
> +This tag indicates that image is relocatable.
> +
> +The meaning of each field is as follows:
> +
> +@table @code
> +@item min_addr
> +Lowest possible physical address at which image should be loaded.
> +Boot loader cannot load any part of image below this address.

"The bootloader".

> +
> +@item max_addr
> +Highest possible physical address at which loaded image should end.
> +Boot loader cannot load any part of image above this address.

"The bootloader".

> +
> +@item align
> +Image alignment in memory, e.g. 4096 (know on many machines as page).

I would drop the qualification in brackets.  It isn't helpful to the
intended audience.

> +
> +@item preference
> +It contains load address placement suggestion for boot loader.
> +Boot loader should follow it. @samp{0} means none, @samp{1} means
> +load image at lowest possible address but not lower than min_addr
> +and @samp{2} means load image at highest possible address but not
> +higher than max_addr.
> +@end table
> +
>  @node Machine state
>  @section MIPS machine state
>  
> @@ -1309,6 +1351,20 @@ u64 | pointer   |
>  This tag contains pointer to EFI amd64 image handle.
>  Usually it is boot loader image handle.
>  
> +@subsection Image load base physical address
> +@example
> +@group
> ++---+
> +u32 | type = 21 |
> +u32 | size = 12 |
> +u32 | load_base_addr|
> ++---+
> +@end group
> +@end example
> +
> +This tag contains image load base physical address. It is
> +provided only if image has relocatable header tag.
> +
>  @node Examples
>  @chapter Examples
>  
> diff --git a/doc/multiboot2.h b/doc/multiboot2.h
> index b85cb13..b181607 100644
> --- a/doc/multiboot2.h
> +++ b/doc/multiboot2.h
> @@ -62,6 +62,7 @@
>  #define MULTIBOOT_TAG_TYPE_EFI_BS18
>  #define MULTIBOOT_TAG_TYPE_EFI32_IH  19
>  #define MULTIBOOT_TAG_TYPE_EFI64_IH  20
> +#define MULTIBOOT_TAG_TYPE_LOAD_BASE_ADDR21
>  
>  #define MULTIBOOT_HEADER_TAG_END  0
>  #define MULTIBOOT_HEADER_TAG_INFORMATION_REQUEST  1
> @@ -73,11 +74,16 @@
>  #define MULTIBOOT_HEADER_TAG_EFI_BS7
>  #define MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI32  8
>  #define MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64  9
> +#define MULTIBOOT_HEADER_TAG_RELOCATABLE  10
>  
>  #define MULTIBOOT_ARCHITECTURE_I386  0
>  #define MULTIBOOT_ARCHITECTURE_MIPS32  4
>  #define MULTIBOOT_HEADER_TAG_OPTIONAL 1
>  
> +#define MULTIBOOT_LOAD_PREFERENCE_NONE 0
> +#define MULTIBOOT_LOAD_PREFERENCE_LOW 1
> +#define MULTIBOOT_LOAD_PREFERENCE_HIGH 2
> +
>  #define MULTIBOOT_CONSOLE_FLAGS_CONSOLE_REQUIRED 1
>  #define MULTIBOOT_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED 2
>  
> @@ -162,6 +168,17 @@ struct multiboot_header_tag_module_align
>multiboot_uint32_t size;
>  };
>  
> +struct multiboot_header_tag_relocatable
> +{
> +  multiboot_uint16_t type;
> +  multiboot_uint16_t flags;
> +  multiboot_uint32_t size;
> +  multiboot_uint32_t min_addr;
> +  multiboot_uint32_t max_addr;

64bit multiboot2 payloads could reasonably expect to be able to have
themselves relocated about the 4G boundary.

~Andrew

> +  multiboot_uint32_t align;
> +  multiboot_uint32_t preference;
> +};
> +
>  struct multiboot_color
>  {
>multiboot_uint8_t red;
> @@ -388,6 +405,13 @@ struct multiboot_tag_efi64_ih
>multiboot_uint64_t pointer;
>  };
>  
> +struct multiboot_tag_load_base_addr
> +{
> +  multiboot_uint32_t type;
> +  multiboot_uint32_t size;
> +  multiboot_uint32_t load_base_addr;
> +};
> +
>  #endif /* ! ASM_FILE */
>  
>  #endif /* ! MULTIBOOT_HEADER */


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


Re: [MULTIBOOT2 DOC PATCH 07/10] multiboot2: Say that memory maps may not be available on EFI platforms

2016-06-09 Thread Andrew Cooper
On 09/06/2016 21:30, Daniel Kiper wrote:
> Signed-off-by: Daniel Kiper 
> ---
>  doc/multiboot.texi |   11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> index f1e0e09..c81b2ea 100644
> --- a/doc/multiboot.texi
> +++ b/doc/multiboot.texi
> @@ -927,6 +927,10 @@ possible value for lower memory is 640 kilobytes. The 
> value returned for
>  upper memory is maximally the address of the first upper memory hole
>  minus 1 megabyte. It is not guaranteed to be this value.
>  
> +This tag may not be provided by some boot loaders on EFI platforms if EFI
> +boot services are enabled and available for loaded image (EFI boot services

"for the loaded".  And below.

~Andrew

> +not terminated tag exists in Multiboot information structure).
> +
>  @subsection BIOS Boot device
>  @example
>  @group
> @@ -1078,6 +1082,10 @@ indicated a reserved area.
>  The map provided is guaranteed to list all standard @sc{ram} that should
>  be available for normal use. This type however includes the regions occupied 
> by kernel, mbi, segments and modules. Kernel must take care not to overwrite 
> these regions.
>  
> +This tag may not be provided by some boot loaders on EFI platforms if EFI
> +boot services are enabled and available for loaded image (EFI boot services
> +not terminated tag exists in Multiboot information structure).
> +
>  @subsection Boot loader name
>  @example
>  @group
> @@ -1310,6 +1318,9 @@ u32 | descriptor version|
>  
>  This tag contains EFI memory map as per EFI specification.
>  
> +This tag may not be provided by some boot loaders on EFI platforms if EFI
> +boot services are enabled and available for loaded image (EFI boot services
> +not terminated tag exists in Multiboot information structure).
>  
>  @subsection EFI boot services not terminated
>  @example


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


Re: [MULTIBOOT2 DOC PATCH 08/10] multiboot2: Add C structure alignment and padding consideration section

2016-06-09 Thread Andrew Cooper
On 09/06/2016 21:30, Daniel Kiper wrote:
> Signed-off-by: Daniel Kiper 
> ---
>  doc/multiboot.texi |   17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> index c81b2ea..bf02a1b 100644
> --- a/doc/multiboot.texi
> +++ b/doc/multiboot.texi
> @@ -1384,6 +1384,7 @@ document, but are included for prospective operating 
> system and boot
>  loader writers.
>  
>  @menu
> +* C structure alignment and padding consideration::
>  * Notes on PC:: 
>  * BIOS device mapping techniques::  
>  * Example OS code:: 
> @@ -1391,6 +1392,22 @@ loader writers.
>  @end menu
>  
>  
> +@node C structure alignment and padding consideration
> +@section C structure alignment and padding consideration
> +
> +Many C compilers try to optimize memory accesses aligning structure

"by aligning"

> +members properly. Usually they reach the goal by adding some padding.

What does "properly" mean here?  The default padding will be specified
by the default ABI the compiler conforms to.

> +This is very useful thing in general. However, if you try to mix assembler
> +with C or use C to implement structure low level access this behavior
> +may lead, at least, to quite surprising results. Hence, compiler should
> +be instructed to not optimize such accesses. Usually it is done by special
> +attribute added to structure definition, e.g. GCC compatible sources use
> +@samp{__attribute__ ((__packed__))} for this purpose. However, this is not
> +required if it is known that its members are properly aligned and compiler
> +does not do any optimization. Very good example of this is shown below in
> +@file{multiboot2.h} file.

I am not sure what you are trying to say.

~Andrew

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


Re: [Xen-devel] [PATCH v7 2/3] * util/grub.d/20_linux_xen.in: Add xen_boot command support for aarch64

2017-05-03 Thread Andrew Cooper
On 02/05/17 08:06, fu@linaro.org wrote:
> From: Fu Wei 
>
> This patch adds the support of xen_boot command for aarch64:
> xen_hypervisor
> xen_module
> These two commands are only for aarch64, since it has its own protocol and
> commands to boot xen hypervisor and Dom0, but not multiboot.
>
> For other architectures, they are still using multiboot and module
> commands.
>
> Signed-off-by: Fu Wei 

Sorry if I am jumping in late and asking awkward questions, but what is
special about Xen/aarch64 here?  Why is it using a non-standard entry
mechanism?

~Andrew

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


Re: [Xen-devel] [PATCH v7 2/3] * util/grub.d/20_linux_xen.in: Add xen_boot command support for aarch64

2017-05-05 Thread Andrew Cooper
On 05/05/17 09:57, Fu Wei wrote:
> Hi Stefano,
>
> On 4 May 2017 at 04:53, Stefano Stabellini  wrote:
>> On Wed, 3 May 2017, Andrew Cooper wrote:
>>> On 02/05/17 08:06, fu@linaro.org wrote:
>>>> From: Fu Wei 
>>>>
>>>> This patch adds the support of xen_boot command for aarch64:
>>>> xen_hypervisor
>>>> xen_module
>>>> These two commands are only for aarch64, since it has its own protocol and
>>>> commands to boot xen hypervisor and Dom0, but not multiboot.
>>>>
>>>> For other architectures, they are still using multiboot and module
>>>> commands.
>>>>
>>>> Signed-off-by: Fu Wei 
>>> Sorry if I am jumping in late and asking awkward questions, but what is
>>> special about Xen/aarch64 here?  Why is it using a non-standard entry
>>> mechanism?
>> Multiboot is not available on ARM. The boot protocol we have on ARM is
>> FDT based:
>>
>> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/arm/device-tree/booting.txt
>>
>> We often refer to it as "multiboot for arm" but in fact it is not
>> related to the x86 multiboot in any way.
>>
>> In Grub, the two protocols (multiboot and
>> docs/misc/arm/device-tree/booting.txt) are kept clearly distinct
>> (55a687e5.4070...@gmail.com). Hence, the need for this patch.
> Exactly, great thanks for your explanation! :-)

So it really is an entirely custom Xen booting protocol.

It is unfortunate that this wasn't reviewed sensibly at the time (and
implemented in a project-neutral way), but it looks like the time to fix
that properly has long since passed.

~Andrew

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


Re: [Xen-devel] Grant table version

2013-10-28 Thread Andrew Cooper
On 28/10/13 18:06, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> Hello, all. I'm working on pvgrub2 and my problem is following:
> When you first use grant tables you commit yourself to a particular
> grant table version. GRUB has to read from disk and hence use grant
> tables. By doing so it commits anything that it loads to the same version.
> Would it be possible to have a hypercall to reset grant table version?
> Of course, before doing such a call one would need to revoke all grants
> or this call would discard all current grants.

You can change back and forth with grant table versions using the
GNTTABOP_set_version hypercall, so long as you have no active grants.

It is not possible to revoke a grant, as the grant ABI guarantees that a
successful grant stays mapped until explicitly unmapped.  Otherwise, a
reset would cause unexpected pagefaults in the mapper domain.

For compatibility reasons it is not strictly required, but it is
certainly expected that a new kernel explicitly chooses a gnttab version
using the hypercall.  There is certainly an area for problems if pvgrub2
uses grant v2, then leaves v2 active and hands off to an older kernel
who expects v1 and doesn't explicitly set the version.

>From this point of view, the safe course of action is to use whichever
type of grants you want, then tear down all the front/back pairs, and
reset the version to v1 before handing off.

~Andrew

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

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


Re: [Xen-devel] pvgrub2 is merged

2013-11-26 Thread Andrew Cooper
On 26/11/13 18:12, Andrey Borzenkov wrote:
> В Tue, 26 Nov 2013 18:58:47 +0100
> Fabio Fantoni  пишет:
>
>> I have also another question:
>> Is possible specify multiple path where search the grub.cfg for support 
>> all mainly distributions and add a custom cfg path support taking it 
>> from arguments?
>>
> You can do something like
>
> if search --set root --file /boot/grub2/grub.cfg ; then
>   configfile /boot/grub2/grub.cfg
> elif search --set root --file /boot/grub/grub.cfg ; then
>   configfile /boot/grub/grub.cfg
> elif ...
>   ...
> fi
>
> If xen provides way to pass arguments to kernel, it sure could be
> implemented as arguments to grub. Actually someone asked for a way to
> pass arguments to grub on EFI, so this could share implementation.

The way PV guests get a command line from the toolstack is via the
start_info.cmd_line, which is up to 1024 bytes.

This will be available to anything which gets its hand on the start info
page.

~Andrew

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


Re: multiboot2 and module2 boot issues via GRUB2

2021-03-30 Thread Andrew Cooper via Grub-devel
On 30/03/2021 19:28, Roman Shaposhnik wrote:
> Hi!
>
> seems like I've run into an issue with multiboot2 and module2
> commands that I can't quite explain. Since it may be something
> super simply and silly -- I wanted to reach out here before I do
> a GRUB/Xen/LK source deepdive.
>
> So here's the deal: whenever I boot straight up Linux kernel
> I can do the following sequence of commands:
>    linux /kernel
>    initrd foo.cpio.gz bar.cpio.gz
> and have linux kernel effectively stack content of bar.cpio.gz
> on top of foo.cpio.gz and present a unified initramfs that way.
>
> I'm trying to replicate it with Xen, but:
>      multiboot2 /boot/xen.gz
>      module2 /kernel
>      module2 foo.cpio.gz
>      module2 bar.cpio.gz
> only seems to be picking up foo.cpio.gz
>
> Has anyone run into this issue before?

I can explain why that happens.  Not sure if it counts as a feature, bug
or mis-expectation, but CC'ing grub-devel for their input.

The initrd command is presumably concatenating those two files together
in memory, and presenting Linux a single initrd pointer.

For the module2 example, you're putting 3 distinct files in memory, and
giving Xen a list 3 modules.

Xen is capable of taking various things via modules, such as an
XSM/Flask policy, or microcode, so has logic to identify these if
present and separate them from "other stuff".  However, there is a
hardcoded expectation that the first module is the dom0 kernel, and the
next unrecognised module, if present, is *the* initrd.

I expect that Xen isn't handing bar.cpio.gz on to dom0, but I'm not sure
whether passing two distinct initrd-like-things to Linux is even possible.

What you presumably want is some `initrd` side effect in Grub so you can
write `module2 foo.cpio.gz bar.cpio.gz` and have it concatenate things
together in memory and present one MB2 module, but I suspect that exact
syntax might be ambiguous with command line handling.  I have no idea
whether such a command currently exists.

~Andrew


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


Re: multiboot2 and module2 boot issues via GRUB2

2021-04-01 Thread Andrew Cooper via Grub-devel
On 01/04/2021 09:44, Roger Pau Monné wrote:
> On Thu, Apr 01, 2021 at 09:31:07AM +0200, Jan Beulich wrote:
>> On 01.04.2021 03:06, Roman Shaposhnik wrote:
>>> And the obvious next question: is my EVE usecase esoteric enough that
>>> I should just go ahead and do a custom GRUB patch or is there a more
>>> general interest in this?
>> Not sure if it ought to be a grub patch - the issue could as well
>> be dealt with in Xen, by concatenating modules to form a monolithic
>> initrd.
> I would rather have it done in the loader than Xen, mostly because
> it's a Linux boot specific format, and hence I don't think Xen should
> have any knowledge about it.
>
> If it turns out to be impossible to implement on the loader side we
> should consider doing it in Xen, but that's not my first option.

Concatenating random things which may or may not be initrds is
absolutely not something Xen should do.  We don't have enough context to
do it safely/sensibly.

Honestly, I like the idea of supporting something like this generally in
grub.  Linux already commonly has initrd preparation prepending an
uncompressed microcode CPIO archive, and I can see a usability advantage
from maintaining the initrd fragments separately.

Looking at the grub manual, this behaviour of the `initrd` command isn't
even documented.  Perhaps that should be fixed first, and then maybe
`module2_multi` added too?

~Andrew


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


Re: multiboot2 and module2 boot issues via GRUB2

2021-04-06 Thread Andrew Cooper via Grub-devel
On 06/04/2021 09:19, Jan Beulich wrote:
> On 01.04.2021 21:43, Andrew Cooper wrote:
>> On 01/04/2021 09:44, Roger Pau Monné wrote:
>>> On Thu, Apr 01, 2021 at 09:31:07AM +0200, Jan Beulich wrote:
>>>> On 01.04.2021 03:06, Roman Shaposhnik wrote:
>>>>> And the obvious next question: is my EVE usecase esoteric enough that
>>>>> I should just go ahead and do a custom GRUB patch or is there a more
>>>>> general interest in this?
>>>> Not sure if it ought to be a grub patch - the issue could as well
>>>> be dealt with in Xen, by concatenating modules to form a monolithic
>>>> initrd.
>>> I would rather have it done in the loader than Xen, mostly because
>>> it's a Linux boot specific format, and hence I don't think Xen should
>>> have any knowledge about it.
>>>
>>> If it turns out to be impossible to implement on the loader side we
>>> should consider doing it in Xen, but that's not my first option.
>> Concatenating random things which may or may not be initrds is
>> absolutely not something Xen should do.  We don't have enough context to
>> do it safely/sensibly.
> Well, I wasn't suggesting anywhere to concatenate random things.
> Instead I was envisioning a command line option giving us the
> context we need (e.g. "initrd=3+5").

That's a massive layering violation, and incredibly fragile to the order
of lines in the boot stanza.

Either fix it by using a single monolithic initrd, which has worked
perfectly well for the past 2 decades, or add a feature to grub to get
initrd-like behaviour for MB2 too.  I will object very strongly to any
Xen patches trying to do this.

~Andrew

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


Re: multiboot2 and module2 boot issues via GRUB2

2021-04-06 Thread Andrew Cooper via Grub-devel
On 06/04/2021 19:03, Roman Shaposhnik wrote:
> On Tue, Apr 6, 2021 at 10:51 AM Andrew Cooper
> mailto:andrew.coop...@citrix.com>> wrote:
>
> On 06/04/2021 09:19, Jan Beulich wrote:
> > On 01.04.2021 21:43, Andrew Cooper wrote:
> >> On 01/04/2021 09:44, Roger Pau Monné wrote:
> >>> On Thu, Apr 01, 2021 at 09:31:07AM +0200, Jan Beulich wrote:
> >>>> On 01.04.2021 03:06, Roman Shaposhnik wrote:
> >>>>> And the obvious next question: is my EVE usecase esoteric
> enough that
> >>>>> I should just go ahead and do a custom GRUB patch or is
> there a more
> >>>>> general interest in this?
> >>>> Not sure if it ought to be a grub patch - the issue could as well
> >>>> be dealt with in Xen, by concatenating modules to form a
> monolithic
> >>>> initrd.
> >>> I would rather have it done in the loader than Xen, mostly because
> >>> it's a Linux boot specific format, and hence I don't think Xen
> should
> >>> have any knowledge about it.
> >>>
> >>> If it turns out to be impossible to implement on the loader
> side we
> >>> should consider doing it in Xen, but that's not my first option.
> >> Concatenating random things which may or may not be initrds is
> >> absolutely not something Xen should do.  We don't have enough
> context to
> >> do it safely/sensibly.
> > Well, I wasn't suggesting anywhere to concatenate random things.
> > Instead I was envisioning a command line option giving us the
> > context we need (e.g. "initrd=3+5").
>
> That's a massive layering violation, and incredibly fragile to the
> order
> of lines in the boot stanza.
>
>
> Don't have enough karma to argue Xen architectural side of things, but...
>  
>
> Either fix it by using a single monolithic initrd, which has worked
> perfectly well for the past 2 decades
>
>
> ...just to point out the obvious here:  even Debian who can HARDLY be
> blamed for tracking the bleeding edge has been recommending this
> for quite some time:
>  
>   
> https://wiki.debian.org/DebianInstaller/NetbootFirmware#The_Solution:_Add_Firmware_to_Initramfs
> <https://wiki.debian.org/DebianInstaller/NetbootFirmware#The_Solution:_Add_Firmware_to_Initramfs>
> Obviously there's no way to do that with Xen today out of the box.

?

Those instructions do work out of the box with Xen.

It seems that pxelinux gained support for multiple initrd fragments in
3.05, but the sum total of documentation I can find is in the
changelog.  iPXE might have this ability, but it's documentation is
self-contradictory on the matter.

>
> Now, you can turn around and say "well, how hard could it be to
> concatenate?". That's fair. But it is also fair to point out that
> everytime
> we do that we portray Xen as "not quite as user friendly as X" (and
> this is, of course, pure perception -- but if we want users to stick
> perception matters a lot).

It's distinctly not trivial to do correctly at the Xen level, and fairly
invasive in at least two areas of logic.  Specific firmware layouts and
multiple fragments might even be impossible to concatenate, and its
better for the bootloaders to bail if they can't make the memory layout
work, than to leave Xen holding the the pieces and unable to boot. 
Either way, it would appear that common bootloaders already have logic
for this, which gets you a lot further than starting from scratch in Xen.


Furthermore, I don't think it fair to claim that this is a usability bug
in Xen, when what the Linux people have done is upstream Linux-specific
hacks to bootloaders.  Fixing the bootloaders to not have useful
features be Linux specific benefits everyone using Multiboot, not just
the Xen ecosystem.  If you're looking for general betterness to all OSS,
then fixing the bootloaders is clearly the better approach.

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