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

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

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




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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






signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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




signature.asc
Description: OpenPGP digital signature
___

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

2016-02-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Applied, thanks
On 20.07.2015 16:35, Daniel Kiper wrote:
> malloc_in_range() should not use memory region if its starta is smaller
> than size. Otherwise target wraps around and points to region which is
> usually not a RAM, e.g.:
> 
> loader/multiboot.c:93: segment 0: paddr=0x80, memsz=0x3f80, 
> vaddr=0x80
> lib/relocator.c:1241: min_addr = 0x0, max_addr = 0x, target = 
> 0x80
> lib/relocator.c:434: trying to allocate in 0x80-0x 
> aligned 0x1 size 0x3f80
> lib/relocator.c:434: trying to allocate in 0x0-0x80 aligned 0x1 size 
> 0x3f80
> lib/relocator.c:434: trying to allocate in 0x0-0x aligned 0x1 
> size 0x3f80
> lib/relocator.c:1188: allocated: 0xc07f+0x3f80
> lib/relocator.c:1277: allocated 0xc07f/0x80
> 
> Signed-off-by: Daniel Kiper 
> ---
>  grub-core/lib/relocator.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/grub-core/lib/relocator.c b/grub-core/lib/relocator.c
> index f759c7f..4eee0c5 100644
> --- a/grub-core/lib/relocator.c
> +++ b/grub-core/lib/relocator.c
> @@ -748,7 +748,7 @@ malloc_in_range (struct grub_relocator *rel,
> /* Found an usable address.  */
> goto found;
> }
> - if (isinsidebefore && !isinsideafter && !from_low_priv)
> + if (isinsidebefore && !isinsideafter && !from_low_priv && starta >= 
> size)
> {
>   target = starta - size;
>   if (target > end - size)
> 




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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

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

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

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

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



xen.diff
Description: application/ext-patch


signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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

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




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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

[Xen-devel] Uniform commands for booting xen

2015-11-12 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Hello, all. I'd like to have set of commands that would boot xen on all
platforms. I thought of following set:

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

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



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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




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




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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

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

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

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




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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




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




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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

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

2015-07-15 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 30.05.2015 07:25, Andrei Borzenkov wrote:
 В 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.
 
My suggestion would be to pass avoid_boot_services = 1 in multiboot2 iff
we don't terminate the services. Any problems with this approach?



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

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

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

2015-07-15 Thread Vladimir 'φ-coder/phcoder' Serbinenko
On 14.07.2015 15:09, Fu Wei wrote:
 Hi Andrei,
 
 Great thanks for your review.
 
 So Are you suggesting this:
 (1) in util/grub.d/20_linux_xen.in, we only use xen_hypervisor/xen_module.
 (2) in xen_boot.c, we only register command xen_hypervisor/xen_module.
 (3) in grub-core/loader/i386/xen.c, we *add*
 ---
   cmd_xen_hypervisort = grub_register_command (xen_hypervisor, grub_cmd_xen,
 0, N_(Load Linux.));
   cmd_xen_module = grub_register_command (xen_module, grub_cmd_module,
  0, N_(Load module.));
No. This is for pvgrub2.
 ---
 (4)in grub-core/loader/multiboot.c, we *add*
 ---
 #if defined (__i386__) || defined (__aarch64__)
   cmd_xen_hypervisort =
 grub_register_command (xen_hypervisor, grub_cmd_multiboot,
   0, N_(Load a multiboot kernel.));
   cmd_xen_module =
 grub_register_command (xen_module, grub_cmd_module,
   0, N_(Load a multiboot module.));
 #endif
No. Don't mix arm64 xen with multiboot. Neither in command names nor in
the descriptions.
 ---
 
 BTW, from the source code, MIPS isn't supported by multiboot,  IS
 supported by multiboot2.
 
 Please correct me. If I misunderstand your suggestion. :-)
 
 Thanks again!
 
 
 On 14 July 2015 at 11:53, Andrei Borzenkov arvidj...@gmail.com wrote:
 В 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=

 
 
 




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 2/5] i386/relocator: Remove unused extern grub_relocator64_rip_addr

2015-05-07 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Committed, thanks.



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel