Re: [Xen-devel] [MULTIBOOT2 DOC PATCH v3 01/13] multiboot2: Replace u_phys with u32

2016-12-10 Thread Andrei Borzenkov 
07.12.2016 01:52, Daniel Kiper пишет:
> u_phys is used just in two places and sometimes it may confuse reader.
> Additionally, GRUB multiboot2 implementation does not use u_phys anywhere.
> So, replace it with basic well defined and used in implementation u32 type.
> 
> Signed-off-by: Daniel Kiper 
> ---
>  doc/multiboot.texi |   11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> index 4b92918..2bda9b7 100644
> --- a/doc/multiboot.texi
> +++ b/doc/multiboot.texi
> @@ -299,9 +299,6 @@ little-endian, u32 is coded in little-endian.
>  The type of unsigned 64-bit data. Because the target architecture is
>  little-endian, u64 is coded in little-endian.
>  
> -@item u_phys
> -The type of unsigned data of the same size as target architecture physical 
> address size.
> -
>  @item u_virt
>  The type of unsigned data of the same size as target architecture virtual 
> address size.
>  

So if I understand it correctly, any address used in multiboot2 is
limited to 32 bit, so anything that is relevant to boot protocol must
reside below 4G. Is my assumption correct?



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

Re: [Xen-devel] [MULTIBOOT2 DOC PATCH v2 01/11] multiboot2: Rename Multiboot to Multiboot2

2016-11-28 Thread Andrei Borzenkov 
24.11.2016 23:40, Daniel Kiper пишет:
> Multiboot2 is proper name of the boot protocol. Multiboot is name of older 
> boot
> protocol. So, rename Multiboot to Multiboot2 to not confuse the reader.
> 
> Signed-off-by: Daniel Kiper 
> ---
>  doc/multiboot.texi |  112 
> ++--

What about file name itself? It makes it impossible for multiboot and
multiboot2 documentation to coexist (or forces every downstream to
rename it anyway).


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

Re: [Xen-devel] [MULTIBOOT2 DOC PATCH v2 06/11] multiboot2: Add description of EFI image handle tags

2016-11-28 Thread Andrei Borzenkov 
24.11.2016 23:40, Daniel Kiper пишет:
> Signed-off-by: Daniel Kiper 
> ---
>  doc/multiboot.texi |   28 
>  doc/multiboot2.h   |   16 
>  2 files changed, 44 insertions(+)
> 
> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> index cc1edab..dca3e62 100644
> --- a/doc/multiboot.texi
> +++ b/doc/multiboot.texi
> @@ -1288,6 +1288,34 @@ u32 | size = 8  |
>  
>  This tag indicates ExitBootServices wasn't called
>  
> +@subsection EFI 32-bit image handle pointer
> +@example
> +@group
> ++---+
> +u32 | type = 19 |
> +u32 | size = 12 |
> +u32 | pointer   |

Why it is not u_phys or u_virt as appropriate?

> ++---+
> +@end group
> +@end example
> +
> +This tag contains pointer to EFI i386 image handle.
> +Usually it is boot loader image handle.
> +
> +@subsection EFI 64-bit image handle pointer
> +@example
> +@group
> ++---+
> +u32 | type = 20 |
> +u32 | size = 16 |
> +u64 | pointer   |

Same. Again, why there are two tags in the first place? It sounds like
perfect fit for "data of the same size as target architecture
[virtual|physical] address size".



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

Re: [Xen-devel] [MULTIBOOT2 DOC PATCH v2 05/11] multiboot2: Add description of support for EFI boot services

2016-11-28 Thread Andrei Borzenkov 
24.11.2016 23:40, Daniel Kiper пишет:
> Signed-off-by: Daniel Kiper 
> ---
> v2 - suggestions/fixes:
>- clarify physical address meaning for EFI amd64
>  mode with boot services enabled
>  (suggested by Andrew Cooper).
> ---
>  doc/multiboot.texi |  115 
> +++-
>  doc/multiboot2.h   |2 +
>  2 files changed, 115 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> index f0f167e..cc1edab 100644
> --- a/doc/multiboot.texi
> +++ b/doc/multiboot.texi
> @@ -534,6 +534,66 @@ 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 Multiboot2 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.

Should not entry_addr be u_phys then? Otherwise what is exact difference
between u_phys and u_virt? Also for type == 3?

> +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 Multiboot2 image header contains EFI boot services tag.
> +Then entry point specified in ELF header and the entry address
> +tag of Multiboot2 header are ignored.
> +
> +@subsection EFI amd64 entry address tag of Multiboot2 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 (paging
> +mode is enabled and any memory space defined by the UEFI memory map
> +is identity mapped, hence, virtual address equals physical address;

Same here; it is confusing marking field as u_virt and describing it
immediately as "physical address".

> +Unified Extensible Firmware Interface Specification, Version 2.6,
> +section 2.3.4, x64 Platforms, boot services). 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 amd64 compatible operating system code.
> +@end table
> +
> +This tag is taken into account only on EFI amd64 platforms
> +when Multiboot2 image header contains EFI boot services tag.
> +Then entry point specified in ELF header and the entry address
> +tag of Multiboot2 header are ignored.
> +

Why do we have two different tags for what is effectively the same
purpose? Is it possible for the same image to have both of them? Just
for my understanding.


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

Re: [Xen-devel] [GRUB2 PATCH v4 3/4 - FOR REVIEW ONLY] multiboot2: Do not pass memory maps to image if EFI boot services are enabled

2016-03-15 Thread Andrei Borzenkov 
15.03.2016 19:07, Vladimir 'phcoder' Serbinenko пишет:
> Looks good. Let's give a day for others to comment. Is the second email the
> version for commit?
> 
> On Tuesday, March 15, 2016, Daniel Kiper  wrote:
> 
>> Do not pass memory maps to image if it asked for EFI boot services.
>> Main reason for not providing maps is because they will likely be
>> invalid. We do a few allocations after filling them, e.g. for relocator
>> needs. Usually we do not care as we would already finish boot services.
>> If we keep boot services then it is easier to not provide maps. However,
>> if image needs memory maps and they are not provided by bootloader then
>> it should get them itself just before ExitBootServices() call.
>>

Are there any existing users of it that rely on memory map provided by
bootloader? What if image explicitly requested non-optional memory map?
According to multiboot specification it is valid and bootloader must
either provide requested information or fail load.

>> Signed-off-by: Daniel Kiper >
>> Reviewed-by: Konrad Rzeszutek Wilk >
>> ---
>> v3 - suggestions/fixes:
>>- improve commit message
>>  (suggested by Konrad Rzeszutek Wilk and Vladimir 'phcoder'
>> Serbinenko).
>> ---
>>  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 6c04402..ad1553b 100644
>> --- a/grub-core/loader/multiboot_mbi2.c
>> +++ b/grub-core/loader/multiboot_mbi2.c
>> @@ -390,7 +390,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)
>> @@ -755,12 +755,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
>> @@ -776,18 +777,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;
>> @@ -886,27 +888,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;
>> +
>> +

Re: [Xen-devel] [PATCH v3 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

2016-03-08 Thread Andrei Borzenkov 
08.03.2016 19:37, Fu Wei пишет:
> Hi Andrei,
> 
> On 8 March 2016 at 14:54, Andrei Borzenkov <arvidj...@gmail.com> wrote:
>> 07.03.2016 11:22, Fu Wei пишет:
>>> Hi Andrei,
>>>
>>> On 28 February 2016 at 00:44, Fu Wei <fu@linaro.org> wrote:
>>>> Hi Andrei
>>>>
>>>> On 28 February 2016 at 01:26, Andrei Borzenkov <arvidj...@gmail.com> wrote:
>>>>> 26.02.2016 14:13, fu@linaro.org пишет:
>>>>>> From: Fu Wei <fu@linaro.org>
>>>>>>
>>>>>> delete: xen_linux, xen_initrd, xen_xsm
>>>>>> add: xen_module
>>>>>>
>>>>>> This update bases on
>>>>>> commit 0edd750e50698854068358ea53528100a9192902
>>>>>> Author: Vladimir Serbinenko <phco...@gmail.com>
>>>>>> Date:   Fri Jan 22 10:18:47 2016 +0100
>>>>>>
>>>>>> xen_boot: Remove obsolete module type distinctions.
>>>>>>
>>>>>> Signed-off-by: Fu Wei <fu@linaro.org>
>>>>>> ---
>>>>>>  docs/grub.texi | 33 ++---
>>>>>>  1 file changed, 10 insertions(+), 23 deletions(-)
>>>>>>
>>>>>> diff --git a/docs/grub.texi b/docs/grub.texi
>>>>>> index 82f6fa4..3fbdd99 100644
>>>>>> --- a/docs/grub.texi
>>>>>> +++ b/docs/grub.texi
>>>>>> @@ -3861,9 +3861,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_initrd::  Load dom0 initrd for dom0 kernel
>>>>>> -* xen_xsm:: Load xen security module for xen 
>>>>>> hypervisor
>>>>>> +* xen_module::  Load xen modules for xen hypervisor
>>>>>>  @end menu
>>>>>>
>>>>>>
>>>>>> @@ -5141,30 +5139,19 @@ 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_module
>>>>>> +@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.
>>>>>
>>>>> ==
>>>>>> +On i386,  the modules will be identified by Multiboot(2) protocol.
>>>>>> +On arm64, each module will be identified by the order in which the
>>>>>> +modules are added.
>>>>>
>>>>> I think it is better to skip it entirely. It is not really correct -
>>>>> neither multiboot protocol provides any module identification (Xen
>>>>> probes module types), nor is i386 using multiboot2, nor can all modules
>>>>> be probed, so order still matters. To avoid confusion I'd simply
>>>>> replaced the above three lines with
>>>>>
>>>>> Modules should be loaded in the following order:
>>>>>
>>>>>> +The 1st module: dom0 kernel image
>>>>>> +The 2nd module: dom0 ramdisk (optional)
>>>>>
>>>>> This covers both supported platforms without going into too deep
>>>>> details; if you and Vladimir are OK, I'll commit with this change.
>>>>
>>>> Thank you very much!
>>>> Sorry I am not familiar with xen on i386, so maybe I misunderstand this.
>>>> So please commit with your change, Thanks for your correction :-)
>>>
>>> I just fetched the mainline GRUB, i would like to know why this
>>> patchset haven't been applied?
>>> Anything I need to do(improve it or post a new patchset according to
>>> your suggestion) for this patchset?
>>>
>>
>> Sorry for delay. It is not really about your patchset, but we need some
>> decision about loading additional modules/lack of initrd on ARM. Until
>> then I'd rather avoid committing to any high-level configuration support
>> that will require even more backward compatible hacks later.
>>
>> As it stands now either Xen needs to support autodetection or we need to
>> revert to providing module type explicitly.
> 
> So speaking of loading additional modules/lack of initrd on ARM, I thinks that
> will (only) affect loading XSM.
> For this, I have discussed of that with Julien, I think :
> (1) the first module must be kernel
> (2) the second module must be initrd, if we have initrd
> (3) Start from the 2nd module, XEN will detect that if the module is a XSM by
> the XSM binary signature. if we get XSM as the second module, that
> means we have not initrd.
> 

If that's the plan, excellent. Vladimir, is it OK to commit then?

> please correct me if I misunderstand it
> 
> :-)
> 


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

Re: [Xen-devel] [PATCH v3 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

2016-03-07 Thread Andrei Borzenkov 
07.03.2016 11:22, Fu Wei пишет:
> Hi Andrei,
> 
> On 28 February 2016 at 00:44, Fu Wei <fu@linaro.org> wrote:
>> Hi Andrei
>>
>> On 28 February 2016 at 01:26, Andrei Borzenkov <arvidj...@gmail.com> wrote:
>>> 26.02.2016 14:13, fu@linaro.org пишет:
>>>> From: Fu Wei <fu@linaro.org>
>>>>
>>>> delete: xen_linux, xen_initrd, xen_xsm
>>>> add: xen_module
>>>>
>>>> This update bases on
>>>> commit 0edd750e50698854068358ea53528100a9192902
>>>> Author: Vladimir Serbinenko <phco...@gmail.com>
>>>> Date:   Fri Jan 22 10:18:47 2016 +0100
>>>>
>>>> xen_boot: Remove obsolete module type distinctions.
>>>>
>>>> Signed-off-by: Fu Wei <fu@linaro.org>
>>>> ---
>>>>  docs/grub.texi | 33 ++---
>>>>  1 file changed, 10 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/docs/grub.texi b/docs/grub.texi
>>>> index 82f6fa4..3fbdd99 100644
>>>> --- a/docs/grub.texi
>>>> +++ b/docs/grub.texi
>>>> @@ -3861,9 +3861,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_initrd::  Load dom0 initrd for dom0 kernel
>>>> -* xen_xsm:: Load xen security module for xen 
>>>> hypervisor
>>>> +* xen_module::  Load xen modules for xen hypervisor
>>>>  @end menu
>>>>
>>>>
>>>> @@ -5141,30 +5139,19 @@ 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_module
>>>> +@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.
>>>
>>> ==
>>>> +On i386,  the modules will be identified by Multiboot(2) protocol.
>>>> +On arm64, each module will be identified by the order in which the
>>>> +modules are added.
>>>
>>> I think it is better to skip it entirely. It is not really correct -
>>> neither multiboot protocol provides any module identification (Xen
>>> probes module types), nor is i386 using multiboot2, nor can all modules
>>> be probed, so order still matters. To avoid confusion I'd simply
>>> replaced the above three lines with
>>>
>>> Modules should be loaded in the following order:
>>>
>>>> +The 1st module: dom0 kernel image
>>>> +The 2nd module: dom0 ramdisk (optional)
>>>
>>> This covers both supported platforms without going into too deep
>>> details; if you and Vladimir are OK, I'll commit with this change.
>>
>> Thank you very much!
>> Sorry I am not familiar with xen on i386, so maybe I misunderstand this.
>> So please commit with your change, Thanks for your correction :-)
> 
> I just fetched the mainline GRUB, i would like to know why this
> patchset haven't been applied?
> Anything I need to do(improve it or post a new patchset according to
> your suggestion) for this patchset?
> 

Sorry for delay. It is not really about your patchset, but we need some
decision about loading additional modules/lack of initrd on ARM. Until
then I'd rather avoid committing to any high-level configuration support
that will require even more backward compatible hacks later.

As it stands now either Xen needs to support autodetection or we need to
revert to providing module type explicitly.

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

Re: [Xen-devel] [PATCH v2 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

2016-02-29 Thread Andrei Borzenkov 
29.02.2016 17:41, Konrad Rzeszutek Wilk пишет:
> On Sun, Feb 28, 2016 at 08:10:33AM +0300, Andrei Borzenkov wrote:
>> 27.02.2016 23:33, Konrad Rzeszutek Wilk пишет:
>>> On Fri, Feb 26, 2016 at 07:15:52PM +0800, Fu Wei wrote:
>>>> Hi Andrei,
>>>>
>>>> On 26 February 2016 at 18:50, Andrei Borzenkov <arvidj...@gmail.com> wrote:
>>>>> On Fri, Feb 26, 2016 at 8:59 AM, Fu Wei <fu@linaro.org> 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.  :-)
>>>
>>> What if the initrd is catted to the kernel image (which you can
>>> do on x86)? And then the 1st module is your XSM?
>>>
>>
>> On x86 Xen can detect microcode and xsm modules; the first unknown
> 
> 'can'. If you use 'ucode=scan' on the Xen command line. Otherwise it has
> no clue.
> 

You can also pass ucode=N where N is module number ... but it goes
off-topic. The current state is - it turned out that on ARM Xen is
capable to "detect" only two types of modules - dom0 kernel and dom0
initrd. The fact is that Xen on ARM has at least one more module type
and it is not possible to reliably load it with current interface.

> It actually would be just much nicer if Multiboot2 had some tag describing
> this and Xen (or any other OS) could just parse it like that. But that is
> just hand-waving, nice-to-do.
> 

It was decided that multiboot2 will not be used, and initial interface
did pass module types from GRUB to Xen via FDT, but then it was changed
to generic xen_module with explicit type specification.

Personally I think that leaving detection of how to use module should be
left to loaded program, and that modules should include enough
information so that this can be done.

> 
>> module after that is assumed to be initrd (dom0 kernel always must be
>> the very first module provided).
> 
> That is true.
>>
>> On arm there is no detection - module type is taken from FDT; if no
>> module type is provided, the first unknown module is assumed to be
>> kernel, the second - initrd.
>>
>> See also http://lists.gnu.org/archive/html/grub-devel/2016-02/msg00333.html
>>
>>> Is this .. order dependency written somewhere in a document? In the
>>> Xen code-base that is?
>>>
>>


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

Re: [Xen-devel] [PATCH v4 10/11] xen: modify page table construction

2016-02-29 Thread Andrei Borzenkov 
29.02.2016 15:19, Juergen Gross пишет:
> On 29/02/16 10:13, Juergen Gross wrote:
>> On 25/02/16 19:33, Andrei Borzenkov wrote:
>>> 22.02.2016 16:14, Juergen Gross пишет:
>>>> On 22/02/16 13:48, Daniel Kiper wrote:
>>>>> On Mon, Feb 22, 2016 at 01:30:30PM +0100, Juergen Gross wrote:
>>>>>> On 22/02/16 13:18, Daniel Kiper wrote:
>>>>>>> On Mon, Feb 22, 2016 at 10:29:04AM +0100, Juergen Gross wrote:
>>>>>>>> On 22/02/16 10:17, Daniel Kiper wrote:
>>>>>>>>> On Mon, Feb 22, 2016 at 07:03:18AM +0100, Juergen Gross wrote:
>>>>>>>>>> diff --git a/grub-core/lib/xen/relocator.c 
>>>>>>>>>> b/grub-core/lib/xen/relocator.c
>>>>>>>>>> index 8f427d3..a05b253 100644
>>>>>>>>>> --- a/grub-core/lib/xen/relocator.c
>>>>>>>>>> +++ b/grub-core/lib/xen/relocator.c
>>>>>>>>>> @@ -29,6 +29,11 @@
>>>>>>>>>>
>>>>>>>>>>  typedef grub_addr_t grub_xen_reg_t;
>>>>>>>>>>
>>>>>>>>>> +struct grub_relocator_xen_paging_area {
>>>>>>>>>> +  grub_xen_reg_t start;
>>>>>>>>>> +  grub_xen_reg_t size;
>>>>>>>>>> +};
>>>>>>>>>> +
>>>>>>>>>
>>>>>>>>> ... this should have GRUB_PACKED because compiler may
>>>>>>>>> add padding to align size member.
>>>>>>>>
>>>>>>>> Why would the compiler add padding to a structure containing two items
>>>>>>>> of the same type? I don't think the C standard would allow this.
>>>>>>>>
>>>>>>>> grub_xen_reg_t is either unsigned (32 bit) or unsigned long (64 bit).
>>>>>>>> There is no way this could require any padding.
>>>>>>>
>>>>>>> You are right but we should add this here just in case.
>>>>>>
>>>>>> Sorry, I don't think this makes any sense. The C standard is very clear
>>>>>> in this case: a type requiring a special alignment has always a length
>>>>>> being a multiple of that alignment. Otherwise arrays wouldn't work.
>>>>>
>>>>> Sorry, I am not sure what do you mean by that.
>>>>
>>>> The size of any C type (no matter whether it is an integral type like
>>>> "int" or a structure) has always the same alignment restriction as the
>>>> type itself. So a type requiring 8 byte alignment will always have a
>>>> size of a multiple of 8 bytes. This is mandatory for arrays to work, as
>>>> otherwise either the elements wouldn't be placed consecutively in memory
>>>> or the alignment restrictions wouldn't be obeyed for all elements.
>>>>
>>>
>>> I too not follow how it is relevant to this case. We talk about internal
>>> padding between structure members, not between array elements.
>>>
>>>> For our case it means that two structure elements of the same type will
>>>> never require a padding between them, thus the annotation with "packed"
>>>> can't serve any purpose.
>>>>
>>>
>>> Well, I am not aware of any requirement. Compiler may add arbitrary
>>> padding between structure elements; it is only prohibited to add padding
>>> at the beginning. Sure, it would be unusual, but never say "never" ...
>>> also should Xen ever be ported to architecture where types are not
>>> self-aligned it will become an issue.
>>
>> So you are telling me that _all_ interfaces between e.g. Linux, grub2,
>> Xen and all wire protocols not attributed with "packed" are just wrong?
>>
>> Sorry, I don't think this is true.
> 
> Okay, just found a reference: The x86 ABI states:
> 
> Aggregates and Unions
> -
> Structures and unions assume the alignment of their most strictly
> aligned component. Each member is assigned to the lowest available
> offset with the appropriate alignment. The size of any object is always
> a multiple of the object‘s alignment.
> 
> I don't think any x86 C-compiler will violate the x86 ABI.
> 

Thank you! I was not really objecting, more thinking loud, because I
missed such explicit statemrnt. Could you post link?

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

Re: [Xen-devel] [PATCH v2 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

2016-02-27 Thread Andrei Borzenkov 
27.02.2016 23:33, Konrad Rzeszutek Wilk пишет:
> On Fri, Feb 26, 2016 at 07:15:52PM +0800, Fu Wei wrote:
>> Hi Andrei,
>>
>> On 26 February 2016 at 18:50, Andrei Borzenkov <arvidj...@gmail.com> wrote:
>>> On Fri, Feb 26, 2016 at 8:59 AM, Fu Wei <fu@linaro.org> 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.  :-)
> 
> What if the initrd is catted to the kernel image (which you can
> do on x86)? And then the 1st module is your XSM?
> 

On x86 Xen can detect microcode and xsm modules; the first unknown
module after that is assumed to be initrd (dom0 kernel always must be
the very first module provided).

On arm there is no detection - module type is taken from FDT; if no
module type is provided, the first unknown module is assumed to be
kernel, the second - initrd.

See also http://lists.gnu.org/archive/html/grub-devel/2016-02/msg00333.html

> Is this .. order dependency written somewhere in a document? In the
> Xen code-base that is?
> 


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

Re: [Xen-devel] [PATCH v3 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

2016-02-27 Thread Andrei Borzenkov 
26.02.2016 14:13, fu@linaro.org пишет:
> From: Fu Wei 
> 
> delete: xen_linux, xen_initrd, xen_xsm
> add: xen_module
> 
> This update bases on
> commit 0edd750e50698854068358ea53528100a9192902
> Author: Vladimir Serbinenko 
> Date:   Fri Jan 22 10:18:47 2016 +0100
> 
> xen_boot: Remove obsolete module type distinctions.
> 
> Signed-off-by: Fu Wei 
> ---
>  docs/grub.texi | 33 ++---
>  1 file changed, 10 insertions(+), 23 deletions(-)
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 82f6fa4..3fbdd99 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3861,9 +3861,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_initrd::  Load dom0 initrd for dom0 kernel
> -* xen_xsm:: Load xen security module for xen hypervisor
> +* xen_module::  Load xen modules for xen hypervisor
>  @end menu
>  
>  
> @@ -5141,30 +5139,19 @@ 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_module
> +@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.

==
> +On i386,  the modules will be identified by Multiboot(2) protocol.
> +On arm64, each module will be identified by the order in which the
> +modules are added.

I think it is better to skip it entirely. It is not really correct -
neither multiboot protocol provides any module identification (Xen
probes module types), nor is i386 using multiboot2, nor can all modules
be probed, so order still matters. To avoid confusion I'd simply
replaced the above three lines with

Modules should be loaded in the following order:

> +The 1st module: dom0 kernel image
> +The 2nd module: dom0 ramdisk (optional)

This covers both supported platforms without going into too deep
details; if you and Vladimir are OK, I'll commit with this change.

>  @end deffn
>  
> -@node xen_initrd
> -@subsection xen_initrd
> -
> -@deffn Command xen_initrd file
> -Load a initrd image for dom0 kernel at the booting process of xen.
> -@end deffn
> -
> -@node xen_xsm
> -@subsection xen_xsm
> -
> -@deffn Command xen_xsm file
> -Load a xen security module for xen hypervisor at the booting process of xen.
> -See @uref{http://wiki.xen.org/wiki/XSM} for more detail.
> -@end deffn
> -
> -
>  @node Networking commands
>  @section The list of networking commands
>  
> 


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

Re: [Xen-devel] [PATCH] arm64: xen_boot: Fix xen boot using Grub on AARCH64

2016-02-27 Thread Andrei Borzenkov 
23.02.2016 17:16, Fu Wei пишет:
> Hi Ian
> 
> On 23 February 2016 at 18:00, Ian Campbell  wrote:
>> On Mon, 2016-02-22 at 17:29 +0800, Fu Wei wrote:
>>> Hi Julien,
>>>
>>> On 20 February 2016 at 00:28, Julien Grall 
>>> wrote:
 Xen is currently crashing because of malformed compatible property for
 the boot module. This is because the property string is not
 null-terminated as requested by the ePAR spec.
 ---
  grub-core/loader/arm64/xen_boot.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-
 core/loader/arm64/xen_boot.c
 index a914eb8..8ae43d7 100644
 --- a/grub-core/loader/arm64/xen_boot.c
 +++ b/grub-core/loader/arm64/xen_boot.c
 @@ -156,7 +156,7 @@ 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_CUSTOM_COMPATIBLE,
 sizeof(MODULE_CUSTOM_COMPATIBLE) - 1);
 + MODULE_CUSTOM_COMPATIBLE,
 sizeof(MODULE_CUSTOM_COMPATIBLE));
if (retval)
  return grub_error (GRUB_ERR_IO, "failed to update FDT");

 --
 1.9.1
>>>
>>> I have tested it. yes, xen will crash without this patch.
>>> Tested-by: Fu Wei 
>>>
>>> BTW, since Vladimir has simplified the xen boot code , and Now Xen
>>> distinguishes modules by order.
>>> --
>>> menuentry 'Foundation Model Xen test with initramfs' {
>>> xen_hypervisor /xen -- no-bootscrub console=dtuart conswitch=x
>>> dtuart=serial0 dom0_mem=512M dom0_max_vcpus=2
>>> xen_module /dom0_kernel_Image console=hvc0 root=/dev/ram0 ro
>>> xen_module /dom0_initramfs.cpio
>>> xen_module /xsm
>>> devicetree /fvp-base-gicv2-psci.dtb
>>> }
>>> -
>>>
>>> Maybe we should update doc to  coordinate with this change.
>>>
>>> And another problem is: How does XEN identify XSM?
>>> Test with the config file above, I got "(XEN) MODULE[3]:
>>> 0008fabb4000 - 0008fabb65e7 Unknown"
>>
>> I must confess when Vlad proposed this change I thought Xen was able to
>> automatically probe for whether a module was an XSM module. Perhaps it is
>> either broken or that functionality never existed to start with?
>>
> 
> From the test result, it seems that the latest xen can NOT
> automatically probe XSM module, even we load xsm as the third module.
> 
> the test branch is master branch. Please correct me if I tested it in wrong 
> way.
> 

That's exactly what code looks like. I.e. XSM is recognized only if
bootloader provided xen,xsm-policy compatible property.

OTOH looking at xsm_dt_policy_init(), XSM module has defined magic and
multiboot version actually detects module type based on this magic. So I
think it makes sense to extend process_multiboot_node() to check for XSM
magic in case of unknown modules. This will make ARM behavior compatible
with x86. Otherwise we simply won't be able to load XSM on ARM at all,
given current implementation in GRUB.

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

Re: [Xen-devel] [PATCH v2 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

2016-02-26 Thread Andrei Borzenkov 
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.

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

Re: [Xen-devel] [PATCH v2 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

2016-02-25 Thread Andrei Borzenkov 


Отправлено с iPhone

> 26 февр. 2016 г., в 7:48, Fu Wei <fu@linaro.org> написал(а):
> 
> Hi Andrei
> 
>> On 26 February 2016 at 01:34, Andrei Borzenkov <arvidj...@gmail.com> wrote:
>> 25.02.2016 09:39, fu@linaro.org пишет:
>>> From: Fu Wei <fu@linaro.org>
>>> 
>>> delete: xen_linux, xen_initrd, xen_xsm
>>> add: xen_module
>>> 
>>> This update bases on
>>>commit 0edd750e50698854068358ea53528100a9192902
>>>Author: Vladimir Serbinenko <phco...@gmail.com>
>>>Date:   Fri Jan 22 10:18:47 2016 +0100
>>> 
>>>xen_boot: Remove obsolete module type distinctions.
>>> 
>>> Signed-off-by: Fu Wei <fu@linaro.org>
>>> ---
>>> docs/grub.texi | 32 +---
>>> 1 file changed, 9 insertions(+), 23 deletions(-)
>>> 
>>> diff --git a/docs/grub.texi b/docs/grub.texi
>>> index 82f6fa4..0f99c50 100644
>>> --- a/docs/grub.texi
>>> +++ b/docs/grub.texi
>>> @@ -3861,9 +3861,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_initrd::  Load dom0 initrd for dom0 kernel
>>> -* xen_xsm:: Load xen security module for xen hypervisor
>>> +* xen_module::  Load xen modules for xen hypervisor
>>> @end menu
>>> 
>>> 
>>> @@ -5141,30 +5139,18 @@ 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_module
>>> +@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.


> so maybe we can say:
> -
> On ARM64, 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 (optional)
> All subsequent modules: UNKNOWN
> 
> On i386,  the modules will be identified by Multiboot(2) protocol.
> -
> 
> Is that better?  please correct me if I miss something.
> 
>> loaded then?
>> 
>>> -@node xen_initrd
>>> -@subsection xen_initrd
>>> -
>>> -@deffn Command xen_initrd file
>>> -Load a initrd image for dom0 kernel at the booting process of xen.
>>> -@end deffn
>>> -
>>> -@node xen_xsm
>>> -@subsection xen_xsm
>>> -
>>> -@deffn Command xen_xsm file
>>> -Load a xen security module for xen hypervisor at the booting process of 
>>> xen.
>>> -See @uref{http://wiki.xen.org/wiki/XSM} for more detail.
>>> -@end deffn
>>> -
>>> -
>>> @node Networking commands
>>> @section The list of networking commands
> 
> 
> 
> -- 
> Best regards,
> 
> Fu Wei
> Software Engineer
> Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
> Ph: +86 21 61221326(direct)
> Ph: +86 186 2020 4684 (mobile)
> Room 1512, Regus One Corporate Avenue,Level 15,
> One Corporate Avenue,222 Hubin Road,Huangpu District,
> Shanghai,China 200021

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

Re: [Xen-devel] [PATCH v4 10/11] xen: modify page table construction

2016-02-25 Thread Andrei Borzenkov 
22.02.2016 16:14, Juergen Gross пишет:
> On 22/02/16 13:48, Daniel Kiper wrote:
>> On Mon, Feb 22, 2016 at 01:30:30PM +0100, Juergen Gross wrote:
>>> On 22/02/16 13:18, Daniel Kiper wrote:
 On Mon, Feb 22, 2016 at 10:29:04AM +0100, Juergen Gross wrote:
> On 22/02/16 10:17, Daniel Kiper wrote:
>> On Mon, Feb 22, 2016 at 07:03:18AM +0100, Juergen Gross wrote:
>>> diff --git a/grub-core/lib/xen/relocator.c 
>>> b/grub-core/lib/xen/relocator.c
>>> index 8f427d3..a05b253 100644
>>> --- a/grub-core/lib/xen/relocator.c
>>> +++ b/grub-core/lib/xen/relocator.c
>>> @@ -29,6 +29,11 @@
>>>
>>>  typedef grub_addr_t grub_xen_reg_t;
>>>
>>> +struct grub_relocator_xen_paging_area {
>>> +  grub_xen_reg_t start;
>>> +  grub_xen_reg_t size;
>>> +};
>>> +
>>
>> ... this should have GRUB_PACKED because compiler may
>> add padding to align size member.
>
> Why would the compiler add padding to a structure containing two items
> of the same type? I don't think the C standard would allow this.
>
> grub_xen_reg_t is either unsigned (32 bit) or unsigned long (64 bit).
> There is no way this could require any padding.

 You are right but we should add this here just in case.
>>>
>>> Sorry, I don't think this makes any sense. The C standard is very clear
>>> in this case: a type requiring a special alignment has always a length
>>> being a multiple of that alignment. Otherwise arrays wouldn't work.
>>
>> Sorry, I am not sure what do you mean by that.
> 
> The size of any C type (no matter whether it is an integral type like
> "int" or a structure) has always the same alignment restriction as the
> type itself. So a type requiring 8 byte alignment will always have a
> size of a multiple of 8 bytes. This is mandatory for arrays to work, as
> otherwise either the elements wouldn't be placed consecutively in memory
> or the alignment restrictions wouldn't be obeyed for all elements.
> 

I too not follow how it is relevant to this case. We talk about internal
padding between structure members, not between array elements.

> For our case it means that two structure elements of the same type will
> never require a padding between them, thus the annotation with "packed"
> can't serve any purpose.
> 

Well, I am not aware of any requirement. Compiler may add arbitrary
padding between structure elements; it is only prohibited to add padding
at the beginning. Sure, it would be unusual, but never say "never" ...
also should Xen ever be ported to architecture where types are not
self-aligned it will become an issue.

>>
>>> Adding GRUB_PACKED would make the code less clear IMO. Finding such a
>>> qualifier in some code I want to modify would make me search for the
>>> reason for it which isn't existing.
>>
>> If maintainers do not object I am not going to insist on that any longer.
> 

It seems inconsistent through the code, really. But I think in this case
it should be packed. It does not look like it is performance critical
and it ensures we always match assembler code.


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

Re: [Xen-devel] [PATCH v4 02/11] xen: avoid memleaks on error

2016-02-25 Thread Andrei Borzenkov 
22.02.2016 11:24, Daniel Kiper пишет:
> 
> Are you sure that grub_errno is always set to GRUB_ERR_NONE
> if any GRUB2 function finished successfully?

grub_errno is reset by command parser before command execution (or after
previous command finished actually). During command execution there is
no guarantee that this happens; sometimes grub_errno is explicitly reset
to suppress subsequent errors but in general errors are just passed through.

> Maybe you should
> set initialize grub_errno with GRUB_ERR_NONE at the beginning
> of parse_xen_guest()?
> 
> Daniel
> 
> ___
> Grub-devel mailing list
> grub-de...@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 


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

Re: [Xen-devel] [PATCH v2 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

2016-02-25 Thread Andrei Borzenkov 
25.02.2016 09:39, fu@linaro.org пишет:
> From: Fu Wei 
> 
> delete: xen_linux, xen_initrd, xen_xsm
> add: xen_module
> 
> This update bases on
> commit 0edd750e50698854068358ea53528100a9192902
> Author: Vladimir Serbinenko 
> Date:   Fri Jan 22 10:18:47 2016 +0100
> 
> xen_boot: Remove obsolete module type distinctions.
> 
> Signed-off-by: Fu Wei 
> ---
>  docs/grub.texi | 32 +---
>  1 file changed, 9 insertions(+), 23 deletions(-)
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 82f6fa4..0f99c50 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3861,9 +3861,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_initrd::  Load dom0 initrd for dom0 kernel
> -* xen_xsm:: Load xen security module for xen hypervisor
> +* xen_module::  Load xen modules for xen hypervisor
>  @end menu
>  
>  
> @@ -5141,30 +5139,18 @@ 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_module
> +@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
loaded then?

> -@node xen_initrd
> -@subsection xen_initrd
> -
> -@deffn Command xen_initrd file
> -Load a initrd image for dom0 kernel at the booting process of xen.
> -@end deffn
> -
> -@node xen_xsm
> -@subsection xen_xsm
> -
> -@deffn Command xen_xsm file
> -Load a xen security module for xen hypervisor at the booting process of xen.
> -See @uref{http://wiki.xen.org/wiki/XSM} for more detail.
> -@end deffn
> -
> -
>  @node Networking commands
>  @section The list of networking commands
>  
> 


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

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

2015-11-13 Thread Andrei Borzenkov 
On Fri, Nov 13, 2015 at 10:48 AM, Jan Beulich  wrote:
 On 12.11.15 at 18:09,  wrote:
>> On Thu, 2015-11-12 at 08:44 -0700, 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.
>>
>> You likely aren't aware that on ARM Xen doesn't boot via multiboot, but via
>> a protocol which involves passing modules in an fdt[0].
>>
>> I had originally hoped that this would use the same command names in the
>> grub cfg, such that things would just work, however the grub maintainers
>> didn't like that (and I appreciate why).
>>
>> Hence on grub/ARM we already have xen_{hypervisor,kernel,initrd,...}.
>>
>> The question then is what grub-mkconfig (or more precisely
>> /etc/grub.d/20_linux_xen) ought to emit so that things just work on all
>> architectures.
>>
>> The author of the grub/ARM/Xen patches initially made it generate the xen_*
>> namas for arm and the multiboot names for x86, here is Vladimir's feedback
>> on that: http://lists.gnu.org/archive/html/grub-devel/2015-10/msg00133.html
>>
>> Which I think gets us to approximately today and Vladimir's question.
>
> Now that makes the situation really ugly (and supports my
> reservations regarding grub2 as a uniform solution for everything).

Well, if you want uniform solution for everything why not simply
implement uniform boot protocol everywhere?

> 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?

> I think any architecture following a well defined, cross-arch
> protocol (like multiboot) should not require any special xen_*
> directives. If ARM64 needs Xen to be treated specially, special
> directives are maybe warranted for this particular case, but I don't
> see why all architectures supporting Xen should then automatically
> have to use those too.

So we can have common grub.cfg that does not depend on each platform
peculiarities and can be reused everywhere. Exactly so that grub could
be uniform solution for everything.

Let's face it. Bootloader obviously must be aware of any changes in
boot protocol requires for a kernel. If (Xen) boot protocol does any
addition, bootloader (GRUB) obviously must implement this addition. As
soon as it needs to implement it for at least one supported arch - it
will automatically be available on every other Xen platform. So it
does not really cost anything (and you can continue to use "module" on
multiboot complying platforms anyway).

Because we already have precedent of using incompatible Xen boot
protocol on different architectures, we try to find uniform solution
that covers everything. Give us protocol that does not require
knowledge of module type everywhere and we will use. But so far you
just shoot the messenger.

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

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

2015-11-12 Thread Andrei Borzenkov 

12.11.2015 18:44, Jan Beulich пишет:

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.



Loader commands traditionally reflected underlying protocol. Using 
multiboot command in this case would be confusing as this is not 
multiboot. But I agree that it is becoming ridiculous. Probably


xen_kernel
xen_module

would be enough.


Jan


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?





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




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

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

2015-11-12 Thread Andrei Borzenkov 

12.11.2015 20:08, Jan Beulich пишет:

On 12.11.15 at 17:58,  wrote:

12.11.2015 18:44, Jan Beulich пишет:

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.



Loader commands traditionally reflected underlying protocol. Using
multiboot command in this case would be confusing as this is not
multiboot.


Why would it not be multiboot?



It is multiboot on x86 but it is something different on arm64; or did I 
misunderstand something?


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

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

2015-10-30 Thread Andrei Borzenkov 
On Fri, Oct 30, 2015 at 12:50 PM, Vladimir 'φ-coder/phcoder'
Serbinenko  wrote:
>>
>> 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

I think we need to be careful here. We need to think about future
extenstions (in particular to other architectures) and options
handling. E.g. initrd never accepted any option and we cannot really
add one probably now; but xen_initrd did IIRC (or may be I was
mistaken with xen_module). What I mean, we should allow additional
options from the very beginning, not just making it exact alias.

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

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

2015-10-29 Thread Andrei Borzenkov 

29.10.2015 18:25, 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


This makes it even more difficult for os-prober to parse grub.cfg. We 
need some alternative before going this route.



- Check that new xen commands are supported (define a new feature)
Please add xen_* aliases on x86 as well




Yes, that's better.

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

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

2015-07-21 Thread Andrei Borzenkov 
On Mon, Jul 20, 2015 at 5:35 PM, Daniel Kiper daniel.ki...@oracle.com 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 daniel.ki...@oracle.com
 ---
  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.

   {
 target = starta - size;
 if (target  end - size)
 --
 1.7.10.4


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

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

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

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

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

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

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


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

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

2015-05-29 Thread Andrei Borzenkov 
В Fri, 29 May 2015 22:58:43 +0200
Daniel Kiper daniel.ki...@oracle.com пишет:

 Another questions is why grub_relocator_alloc_chunk_addr() does not consult 
 EFI
 memory map if grub_relocator_alloc_chunk_align() does. Should not we fix it?

My best guess is that grub_relocator_alloc_chunk_addr() gets target
from elsewhere so there is nothing to consult (it is caller
responsibility); while grub_relocator_alloc_chunk_align() needs to
actually search for suitable memory region.

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

Re: [Xen-devel] [Linaro-uefi] [PATCH 0/5]arm64: Add multiboot support (via fdt) for Xen boot

2015-05-03 Thread Andrei Borzenkov 
В Fri, 10 Apr 2015 10:32:15 +0800
Fu Wei fu@linaro.org пишет:

 Hi everybody,
 any suggestion for my patchset?
 Please let me know if there are some place need to be fixed or improved, if
 these patches look good to you, can they be merged?
 
 Any feedback or suggestion  is welcome! :-)
 Great thanks !
 

Sorry for delay. grub-devel is not overcrowded right now; I started to
review them but got distracted. I'll try to find time.

 On 26 January 2015 at 22:32, Stefano Stabellini 
 stefano.stabell...@eu.citrix.com wrote:
 
  Ping?
 
  I think it would be great to have multiboot support in grub.
  As a matter of fact without it grub cannot load xen on arm.
 
 
  On Wed, 21 Jan 2015, Fu Wei wrote:
   Hi everybody,
   any suggestion for my patchset?
   if these patches look fine, can they be merged?
  
   Any feedback is welcome! :-)
   Great thanks !
  
   On 9 January 2015 at 00:38, Ian Campbell ian.campb...@citrix.com
  wrote:
 On Fri, 2014-12-19 at 01:55 +0800, Fu Wei wrote:
  Signed-off-by: Fu Wei fu@linaro.org
  Reviewed-by: Leif Lindholm leif.lindh...@linaro.org
  
 I've tested the code at
 git://
  git.linaro.org/people/fu.wei/grub.git#multiboot_xen_support_upstream_v4.0
 which I believe is the same as this posting on an AMD Seattle
  system
 with a 40_custom containing:
  
 menuentry 'Baremetal' {
 insmod gzio
 insmod part_msdos
 insmod ext2
 set root='hd0,msdos2'
 search --no-floppy --fs-uuid --set=root
  d4178d5c-96a8-46ee-a304-0d87baa545cd
 linux /boot/vmlinuz console=ttyAMA0,115200n8
  earlycon=pl011,0xe101 root=/dev/sda2 rootwait
 }
 menuentry 'Xen' {
 insmod gzio
 insmod part_msdos
 insmod ext2
 set root='hd0,msdos2'
 search --no-floppy --fs-uuid --set=root
  d4178d5c-96a8-46ee-a304-0d87baa545cd
 multiboot /boot/xen no-bootscrub console=dtuart
  conswitch=x dtuart=/smb/serial@e101 noreboot
 sync_console dom0_mem=256M dom0_max_vcpus=1
 module /boot/vmlinuz-xen console=hvc0
  earlycon=pl011,0xe101 root=/dev/sda2 rootwait
 }
  
 (my system is too confused for 10_linux or 20_linux_xen right now,
  but I
 believe this is representative of what they would emit)
  
 The result was that I could boot both Baremetal and Xen via grub.
  So:
  
 Tested-by: Ian Campbell ian.campb...@citrix.com
  
 I've not looked at the code (although I did review several older
 iterations), would it be useful to the grub maintainers for me to
  do so?
  
 Ian.
  
  
  
  
   --
   Best regards,
  
   Fu Wei
   Software Engineer
   Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
   Ph: +86 21 61221326(direct)
   Ph: +86 186 2020 4684 (mobile)
   Room 1512, Regus One Corporate Avenue,Level 15,
   One Corporate Avenue,222 Hubin Road,Huangpu District,
   Shanghai,China 200021
  
  
 
  ___
  Linaro-uefi mailing list
  linaro-u...@lists.linaro.org
  http://lists.linaro.org/mailman/listinfo/linaro-uefi
 
 
 
 


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

Re: [Xen-devel] [PATCH 1/5] multiboot2: Fix information request tag size calculation

2015-02-14 Thread Andrei Borzenkov 
В Fri, 30 Jan 2015 18:59:24 +0100
Daniel Kiper daniel.ki...@oracle.com пишет:

 Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
 ---
  grub-core/loader/multiboot_mbi2.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/grub-core/loader/multiboot_mbi2.c 
 b/grub-core/loader/multiboot_mbi2.c
 index 6f74aee..d7c19bc 100644
 --- a/grub-core/loader/multiboot_mbi2.c
 +++ b/grub-core/loader/multiboot_mbi2.c
 @@ -150,7 +150,7 @@ grub_multiboot_load (grub_file_t file, const char 
 *filename)
   = (struct multiboot_header_tag_information_request *) tag;
 if (request_tag-flags  MULTIBOOT_HEADER_TAG_OPTIONAL)
   break;
 -   for (i = 0; i  (request_tag-size - sizeof (request_tag))
 +   for (i = 0; i  (request_tag-size - sizeof (*request_tag))
/ sizeof (request_tag-requests[0]); i++)
   switch (request_tag-requests[i])
 {

Applied. Thanks!

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

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

2015-02-14 Thread Andrei Borzenkov 
В Wed, 11 Feb 2015 08:20:04 +
Jan Beulich jbeul...@suse.com пишет:

  On 10.02.15 at 22:27, daniel.ki...@oracle.com wrote:
  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;
 
 How does it not? It's an ELF binary without relocations that's being
 loaded - I can't see how such could be validly loaded anywhere but
 at the virtual address(es) its program header states (and I don't
 know whether grub [1 or 2] would correctly process relocations if
 there were any, but I doubt it).
 

grub2 relocates own modules when loading them. It does not do
relocation when loading Xen binary, but it does not sound impossible.

  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.
 
 Between the two, 1 is certainly the preferable option.
 
  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.
 
 How awful a firmware implementation! On PC-like systems, _nothing_
 that _absolutely_ has to be below 1Mb should be placed there.
 
 Jan
 
 
 ___
 Grub-devel mailing list
 grub-de...@gnu.org
 https://lists.gnu.org/mailman/listinfo/grub-devel


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