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

2015-09-01 Thread Jan Beulich 
>>> On 31.08.15 at 21:49,  wrote:
> On Fri, Aug 28, 2015 at 08:16:05AM -0600, Jan Beulich wrote:
>> >>> On 28.08.15 at 15:42,  wrote:
>> > Now that said - do you have suggestions on how to make this work
>> > with GRUB in the picture?
>>
>> ... I don't think I'm the one to make suggestions on how to make
>> things work with grub in the picture when I continue to be of the
>> opinion that it shouldn't have been brought into the picture in the
>> first place.
> 
> Could you be more specific what is wrong with this patch or at least last
> hunk which you reviewed? What is real technical reason that it could not
> be accepted? If idea is wrong in general please tell me where and why.
> Otherwise I am not able to work out other better solution.

The patch may not be wrong technically (and I never said so), it is
just that the way you carry things out is too intrusive for my taste.
Since Andrew is happy with the change in principle, I think I wouldn't
veto it going in with his R-b.

> By the way, once I have put 3 (IIRC) proposals for this problem on the 
> table.
> Even we discussed this issue in Shanghai. You and Andrew approved more or
> less this one. So, I am a bit disappointed that you withdraw your approval
> (yes, partial but still the approval) at this stage with just vague 
> explanations.

I've never approved of anything here. At the hackathon we've only
hashed out possible options.

>> But for the purely technical (patch) aspect: Anything (e.g.
>> macroization such that at least some sym_phys() uses can remain
>> untouched) allowing to limit the impact of said patch on the source
>> code (thus helping review and perhaps also long term
>> maintainability) would be a step towards talking me into
>> withdrawing my objection.
> 
> Ditto. This is too vague. So, I will be very grateful if you review this
> patch until the end or at least tell me what (if you add why it will be
> nice) exactly should be fixed.

How is this to vague? I gave a possible direction (macroization) as
well as the criteria (less impact on existing code; to be slightly more
precise I'd specifically like to see most of the open coded %fs: uses
gone).

Again - if Andrew thinks this is the right thing to do, I'll defer to him
for reviewing these final few patches of the series, since as said
before to me this is a workaround for a misguided design, and as
such I'm not willing to accept as intrusive a patch as this one is. (And
no, I don't really buy his argument of Xen boot code needing to be
relocatable anyway - the legacy boot path doesn't really need to
fear there not being free memory right above 1Mb. At least I have
seen no proof of there being any system [supporting legacy boot]
where Xen can't boot that way. And even if there was, it would
still need to be determined whether this is legitimately so, or again
due to poorly written firmware.)

Jan


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

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

2015-08-31 Thread Daniel Kiper 
On Fri, Aug 28, 2015 at 08:16:05AM -0600, Jan Beulich wrote:
> >>> On 28.08.15 at 15:42,  wrote:
> > And I am not comfortable to say 'GRUB2+Xen cannot run on this hardware
> > because your firmware vendor is not following the EFI spec in spirit.'
>
> Well, not the least since I don't really agree with this (albeit I can
> see where you're coming from) ...
>
> > Now that said - do you have suggestions on how to make this work
> > with GRUB in the picture?
>
> ... I don't think I'm the one to make suggestions on how to make
> things work with grub in the picture when I continue to be of the
> opinion that it shouldn't have been brought into the picture in the
> first place.

Could you be more specific what is wrong with this patch or at least last
hunk which you reviewed? What is real technical reason that it could not
be accepted? If idea is wrong in general please tell me where and why.
Otherwise I am not able to work out other better solution.

By the way, once I have put 3 (IIRC) proposals for this problem on the table.
Even we discussed this issue in Shanghai. You and Andrew approved more or
less this one. So, I am a bit disappointed that you withdraw your approval
(yes, partial but still the approval) at this stage with just vague 
explanations.
Though I am not trying to argue that this patch is fully correct. For sure
it could be improved but I do not think that this invalidates idea as whole.

> But for the purely technical (patch) aspect: Anything (e.g.
> macroization such that at least some sym_phys() uses can remain
> untouched) allowing to limit the impact of said patch on the source
> code (thus helping review and perhaps also long term
> maintainability) would be a step towards talking me into
> withdrawing my objection.

Ditto. This is too vague. So, I will be very grateful if you review this
patch until the end or at least tell me what (if you add why it will be
nice) exactly should be fixed.

Daniel

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

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

2015-08-28 Thread Jan Beulich 
 On 27.08.15 at 20:04, andrew.coop...@citrix.com wrote:
 On 27/08/15 16:29, Jan Beulich wrote:
 On 27.08.15 at 17:10, daniel.ki...@oracle.com wrote:
 On Thu, Aug 27, 2015 at 07:12:38AM -0600, Jan Beulich wrote:
 On 20.07.15 at 16:29, daniel.ki...@oracle.com wrote:
  /* Copy bootstrap trampoline to low memory, below 1MB. */
 -mov $sym_phys(trampoline_start),%esi
 +lea sym_offset(trampoline_start)(%ebp),%esi
  mov $trampoline_end - trampoline_start,%ecx
  rep movsb

 The latest at this final hunk I'm getting tired (and upset). I'd much
 rather not touch all this code in these fragile ways, and instead
 require Xen to be booted without grub2 on badly written firmware
 like the one on the machine you quote in the description.
 Let's start discussion from here. My further work on this patch series
 is pointless until we do not agree details in this case.

 So, I agree that any software (including firmware) should not use
 precious resources (i.e. low memory in that case) if it is not strictly
 required. And I do not think so that latest UEFI implementations
 on new hardware really need this low memory to survive (at least page
 tables could be put anywhere above low memory). However, in case of
 UEFI it is a wish of smart people not requirement set by spec. I was
 checking UEFI docs a few times but I was not able to find anything
 which says that e.g. ...developer MUST allocate memory outside of low
 memory  Even I have not found any suggestion about that. However,
 I must admit that I do not know whole UEFI spec, so, if you know something
 which is similar to above please tell me where it is (title, revision,
 page, paragraph, etc.). Hence, if there is not strict requirement in
 regards to memory allocation in specs we are lost. Of course we can
 encourage people to not do nasty things but I do not believe that many
 will listen. So, sadly, I suppose that we will see more and more machines
 in the wild which are broken (well, let's say do not align to good 
 practices).

 On the other hand I think that we should not assume that a given memory
 region (in our case starting at 1 MiB) is (or will be) available in every
 machine. I have not seen anything which says that it is true. We should
 stop doing that even if it works right now. I think that it works by
 chance and there is a chance that it will stop working one day because
 somebody will discover that he or she can put there e.g. device or hole.

 Last but not least. I suppose that Xen users and especially distros will
 complain when they are not able to use GRUB2 with Xen on every platform
 just because somebody (i.e. platform designers, developers, or whatever)
 do not accept our beliefs or we require that platform must obey rules
 (i.e. memory map requirements) which are specified nowhere.
 You're right, there's no such requirement on memory use in the spec.
 But you're missing the point. Supporting grub2 on UEFI is already a
 hack (ignoring all intentions EFI had from its first days). And now
 you've found an environment where that hack needs another hack
 (in Xen) to actually work. That's too much hackery for my taste, the
 more that things on this system can (afaict) work quite okay (without
 grub2, or with using its chainloader mechanism).

 So no, I'm still not convinced of the need for this patch.
 
 I disagree.  There are many things a grub environment gives us which
 plain EFI doesn't, most notably a familiar booting environment and
 recovery facilities for users (which vastly trumps how EFI expects to
 run things).

Familiar booting environment? I knew of EFI's much earlier than
I saw grub for the first time, so to me EFI's is more familiar if you
so will.

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

Which is an argument for porting suitable file system drivers to EFI,
not an argument to layer multiple boot loaders. Plus booting _is_
(naturally) different between platforms. Plus if EFI was ported to
architectures of interest (see ARM), the difference between
platforms would decrease too (which btw was one of the original
goals of EFI). Are you btw saying that ARM should use grub2 too?
I didn't hear that mentioned as a goal anywhere so far...

 Therefore I am very much +1 get grub working.

Then you kind of misunderstood: I'm not against getting grub2
working (i.e. patches prior to this one are fine in principle). What
I'm against is hacking around firmware+grub2 combinations not
suitable for Xen's purposes (where one could argue which of the
three is really at fault, and I'm far from convinced it's Xen).

Jan

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

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

2015-08-28 Thread Jan Beulich 
 On 27.08.15 at 19:56, 426...@gmail.com wrote:
 If you advocate direct booting ( no boot loader)  on production machines I
 wont argue much, as long as there is good recovery tools to deal with
 failed boots (grub does this very well, I am not aware of anything
 comparable that is pure efi). however the other use case which care more
 about is dual booting. I am a novice when it comes to Xen, although
 otherwise competent. The test machines I have for playing with Xen are also
 used for other tests, some of which require raw hardware support, so I want
 the ability to use a boot menu.

All EFI systems that I've seen so far have a boot manager. Are you
saying there are vendors who remove that?

Jan


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

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

2015-08-28 Thread Andrew Cooper 
On 28/08/15 07:54, Jan Beulich wrote:

 Therefore I am very much +1 get grub working.
 Then you kind of misunderstood: I'm not against getting grub2
 working (i.e. patches prior to this one are fine in principle). What
 I'm against is hacking around firmware+grub2 combinations not
 suitable for Xen's purposes (where one could argue which of the
 three is really at fault, and I'm far from convinced it's Xen).

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

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

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

~Andrew

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

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

2015-08-28 Thread Konrad Rzeszutek Wilk 
On Fri, Aug 28, 2015 at 02:22:46AM -0600, Jan Beulich wrote:
  On 27.08.15 at 19:56, 426...@gmail.com wrote:
  If you advocate direct booting ( no boot loader)  on production machines I
  wont argue much, as long as there is good recovery tools to deal with
  failed boots (grub does this very well, I am not aware of anything
  comparable that is pure efi). however the other use case which care more
  about is dual booting. I am a novice when it comes to Xen, although
  otherwise competent. The test machines I have for playing with Xen are also
  used for other tests, some of which require raw hardware support, so I want
  the ability to use a boot menu.
 
 All EFI systems that I've seen so far have a boot manager. Are you
 saying there are vendors who remove that?

Worst. There are some where the boot managers coughcertain families of 
AMI based BIOSes/cough which delete all but the last boot entry to
only have _one_ boot entry per ESP. No DualBoot for you!

The 'boot manager' in question is an boot entry (F8 or F12), which does not
allow any changes (change the parameters, etc) - just select the entry.

Furthermore my recollection is Microsoft has a 'boot configuration manager'
after EFI boots - you hit F8 and you can select which options you want to
boot Windows with.

We need that functionality and we can't depend on EFI getting it right - 
hence need GRUB. It would be fantastic if everybody fixed their EFI firmware
but since some of these for SMI have 'O.E.M vendor should
put information here' (or whatever it says) - it makes me think they
don't care the sligthest after the product has been released.

And I am not comfortable to say 'GRUB2+Xen cannot run on this hardware
because your firmware vendor is not following the EFI spec in spirit.'

Now that said - do you have suggestions on how to make this work
with GRUB in the picture?

Thank you.

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

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

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

2015-08-28 Thread Jan Beulich 
 On 27.08.15 at 17:29, jbeul...@suse.com wrote:
 You're right, there's no such requirement on memory use in the spec.
 But you're missing the point. Supporting grub2 on UEFI is already a
 hack (ignoring all intentions EFI had from its first days). And now
 you've found an environment where that hack needs another hack
 (in Xen) to actually work. That's too much hackery for my taste, the
 more that things on this system can (afaict) work quite okay (without
 grub2, or with using its chainloader mechanism).

It has been brought to my attention that the use of the work hack
above could have been understood as an offense. It certainly
wasn't meant so, and I'd like to apologize if it came over that way. I
simply used it not finding a better term while writing; perhaps I could
have used misguided and workaround respectively, but I'm not
sure that would have been received much better.

In any event - I didn't mean to insult you or anyone else.

Jan


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

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

2015-08-28 Thread Jan Beulich 
 On 28.08.15 at 15:42, konrad.w...@oracle.com wrote:
 And I am not comfortable to say 'GRUB2+Xen cannot run on this hardware
 because your firmware vendor is not following the EFI spec in spirit.'

Well, not the least since I don't really agree with this (albeit I can
see where you're coming from) ...

 Now that said - do you have suggestions on how to make this work
 with GRUB in the picture?

... I don't think I'm the one to make suggestions on how to make
things work with grub in the picture when I continue to be of the
opinion that it shouldn't have been brought into the picture in the
first place.

But for the purely technical (patch) aspect: Anything (e.g.
macroization such that at least some sym_phys() uses can remain
untouched) allowing to limit the impact of said patch on the source
code (thus helping review and perhaps also long term
maintainability) would be a step towards talking me into
withdrawing my objection.

Jan


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

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

2015-08-27 Thread Jan Beulich 
 On 20.07.15 at 16:29, daniel.ki...@oracle.com wrote:
   - %fs register is filled with segment descriptor which describes memory 
 region
 with Xen image (it could be relocated or not);

This is too fuzzy. Please be very precise which region it is that %fs
is supposed to point to (so that reviewers have a chance to validate
uses).

 --- a/xen/arch/x86/Makefile
 +++ b/xen/arch/x86/Makefile
 @@ -72,8 +72,10 @@ efi-$(x86_64) := $(shell if [ ! -r 
 $(BASEDIR)/include/xen/compile.h -o \
   echo '$(TARGET).efi'; fi)
  
  $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
 - ./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x10 \
 - `$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`
 +#THIS IS UGLY HACK! PLEASE DO NOT COMPLAIN. I WILL FIX IT IN NEXT 
 RELEASE.
 + ./boot/mkelf32 $(TARGET)-syms $(TARGET) $(XEN_IMG_PHYS_START) 
 0x82d08100
 +#./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x10 \
 +#`$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`

I do complain nevertheless: Such a patch should be tagged RFC. And
with such a hack in place I'm not even sure it's worth reviewing.

 --- a/xen/arch/x86/boot/head.S
 +++ b/xen/arch/x86/boot/head.S
 @@ -12,13 +12,15 @@
  .text
  .code32
  
 -#define sym_phys(sym) ((sym) - __XEN_VIRT_START)
 +#define sym_phys(sym) ((sym) - __XEN_VIRT_START + XEN_IMG_PHYS_START - 
 XEN_IMG_OFFSET)
 +#define sym_offset(sym)   ((sym) - __XEN_VIRT_START)

With XEN_IMG_PHYS_START == XEN_IMG_OFFSET - what's the point?
(If there is a point, then there's obviously a comment missing here
explaining things, including when to use which.)

 @@ -105,12 +107,13 @@ multiboot1_header_end:
  
  .word   0
  gdt_boot_descr:
 -.word   6*8-1
 -.long   sym_phys(trampoline_gdt)
 +.word   7*8-1
 +gdt_boot_descr_addr:

gdt_boot_base would seem a better name; with C in mind what
you use currently could be easily mistaken for a variable holding
gdt_boot_descr.

 @@ -263,12 +271,8 @@ __start:
  cld
  cli
  
 -/* Initialise GDT and basic data segments. */
 -lgdt%cs:sym_phys(gdt_boot_descr)
 -mov $BOOT_DS,%ecx
 -mov %ecx,%ds
 -mov %ecx,%es
 -mov %ecx,%ss
 +/* Load default Xen image base address. */
 +mov $sym_phys(__image_base__),%ebp

Isn't this always zero? I.e. wouldn't you better use xor %ebp,%ebp?

  /* Save the Multiboot info struct (after relocation) for later use. 
 */
 -mov $sym_phys(cpu0_stack)+1024,%esp
 +lea (sym_offset(cpu0_stack)+1024)(%ebp),%esp

Please avoid obfuscating the code by adding pointless parentheses.

 @@ -390,72 +432,111 @@ trampoline_setup:
  
  /* Stash TSC to calculate a good approximation of time-since-boot */
  rdtsc
 -mov %eax,sym_phys(boot_tsc_stamp)
 -mov %edx,sym_phys(boot_tsc_stamp+4)
 +mov %eax,%fs:sym_offset(boot_tsc_stamp)
 +mov %edx,%fs:sym_offset(boot_tsc_stamp+4)
  
 -/* Initialise L2 boot-map page table entries (16MB). */
 -mov $sym_phys(l2_bootmap),%edx
 -mov $PAGE_HYPERVISOR|_PAGE_PSE,%eax
 -mov $8,%ecx
 +/* Update frame addreses in page tables. */
 +lea sym_offset(__page_tables_start)(%ebp),%edx
 +mov $((__page_tables_end-__page_tables_start)/8),%ecx
 +1:  testl   $_PAGE_PRESENT,(%edx)
 +jz  2f
 +add %ebp,(%edx)
 +2:  add $8,%edx
 +loop1b
 +
 +/* Initialise L2 boot-map page table entries (14MB). */
 +lea sym_offset(l2_bootmap)(%ebp),%edx
 +lea sym_offset(start)(%ebp),%eax
 +and $~((1L2_PAGETABLE_SHIFT)-1),%eax
 +mov %eax,%ebx
 +shr $(L2_PAGETABLE_SHIFT-3),%ebx
 +and $(L2_PAGETABLE_ENTRIES*4*8-1),%ebx
 +add %ebx,%edx
 +add $(PAGE_HYPERVISOR|_PAGE_PSE),%eax
 +mov $7,%ecx
  1:  mov %eax,(%edx)
  add $8,%edx
  add $(1L2_PAGETABLE_SHIFT),%eax
  loop1b
 +
  /* Initialise L3 boot-map page directory entry. */
 -mov $sym_phys(l2_bootmap)+__PAGE_HYPERVISOR,%eax
 -mov %eax,sym_phys(l3_bootmap) + 0*8
 +lea (sym_offset(l2_bootmap)+__PAGE_HYPERVISOR)(%ebp),%eax
 +lea sym_offset(l3_bootmap)(%ebp),%ebx
 +mov $4,%ecx
 +1:  mov %eax,(%ebx)
 +add $8,%ebx
 +add $(L2_PAGETABLE_ENTRIES*8),%eax
 +loop1b
 +
 +/* Initialise L2 direct map page table entries (14MB). */
 +lea sym_offset(l2_identmap)(%ebp),%edx
 +lea sym_offset(start)(%ebp),%eax
 +and $~((1L2_PAGETABLE_SHIFT)-1),%eax
 +mov %eax,%ebx
 +shr $(L2_PAGETABLE_SHIFT-3),%ebx
 +and $(L2_PAGETABLE_ENTRIES*4*8-1),%ebx
 +add

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

2015-08-27 Thread Ben Hildred 
On Thu, Aug 27, 2015 at 9:29 AM, Jan Beulich jbeul...@suse.com wrote:

  On 27.08.15 at 17:10, daniel.ki...@oracle.com wrote:
  On Thu, Aug 27, 2015 at 07:12:38AM -0600, Jan Beulich wrote:
   On 20.07.15 at 16:29, daniel.ki...@oracle.com wrote:
/* Copy bootstrap trampoline to low memory, below 1MB. */
   -mov $sym_phys(trampoline_start),%esi
   +lea sym_offset(trampoline_start)(%ebp),%esi
mov $trampoline_end - trampoline_start,%ecx
rep movsb
  
 
  The latest at this final hunk I'm getting tired (and upset). I'd much
  rather not touch all this code in these fragile ways, and instead
  require Xen to be booted without grub2 on badly written firmware
  like the one on the machine you quote in the description.
 
  Let's start discussion from here. My further work on this patch series
  is pointless until we do not agree details in this case.
 
  So, I agree that any software (including firmware) should not use
  precious resources (i.e. low memory in that case) if it is not strictly
  required. And I do not think so that latest UEFI implementations
  on new hardware really need this low memory to survive (at least page
  tables could be put anywhere above low memory). However, in case of
  UEFI it is a wish of smart people not requirement set by spec. I was
  checking UEFI docs a few times but I was not able to find anything
  which says that e.g. ...developer MUST allocate memory outside of low
  memory  Even I have not found any suggestion about that. However,
  I must admit that I do not know whole UEFI spec, so, if you know
 something
  which is similar to above please tell me where it is (title, revision,
  page, paragraph, etc.). Hence, if there is not strict requirement in
  regards to memory allocation in specs we are lost. Of course we can
  encourage people to not do nasty things but I do not believe that many
  will listen. So, sadly, I suppose that we will see more and more machines
  in the wild which are broken (well, let's say do not align to good
  practices).
 
  On the other hand I think that we should not assume that a given memory
  region (in our case starting at 1 MiB) is (or will be) available in every
  machine. I have not seen anything which says that it is true. We should
  stop doing that even if it works right now. I think that it works by
  chance and there is a chance that it will stop working one day because
  somebody will discover that he or she can put there e.g. device or hole.
 
  Last but not least. I suppose that Xen users and especially distros will
  complain when they are not able to use GRUB2 with Xen on every platform
  just because somebody (i.e. platform designers, developers, or whatever)
  do not accept our beliefs or we require that platform must obey rules
  (i.e. memory map requirements) which are specified nowhere.

 You're right, there's no such requirement on memory use in the spec.
 But you're missing the point. Supporting grub2 on UEFI is already a
 hack (ignoring all intentions EFI had from its first days). And now
 you've found an environment where that hack needs another hack
 (in Xen) to actually work. That's too much hackery for my taste, the
 more that things on this system can (afaict) work quite okay (without
 grub2, or with using its chainloader mechanism).

 If you advocate direct booting ( no boot loader)  on production machines I
wont argue much, as long as there is good recovery tools to deal with
failed boots (grub does this very well, I am not aware of anything
comparable that is pure efi). however the other use case which care more
about is dual booting. I am a novice when it comes to Xen, although
otherwise competent. The test machines I have for playing with Xen are also
used for other tests, some of which require raw hardware support, so I want
the ability to use a boot menu. I am aware of refit, but it does not have
serial support which makes automating testing more difficult. and I have
not yet got ipxe to successfully boot Xen (although this is purely because
I have not yet devoted enough time to that project and I am not yet
familiar with how Xen boots).

So no, I'm still not convinced of the need for this patch.


I am at a loss for alternatives. Yes grub on efi violates the spirit of
efi. Propose a better way forward rather than deriding those who have found
a successful, portable way around the limitations of efi implementations.


 Jan


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




-- 
--
Ben Hildred
Automation Support Services
303 815 6721
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

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

 The latest at this final hunk I'm getting tired (and upset). I'd much
 rather not touch all this code in these fragile ways, and instead
 require Xen to be booted without grub2 on badly written firmware
 like the one on the machine you quote in the description.
 Let's start discussion from here. My further work on this patch series
 is pointless until we do not agree details in this case.

 So, I agree that any software (including firmware) should not use
 precious resources (i.e. low memory in that case) if it is not strictly
 required. And I do not think so that latest UEFI implementations
 on new hardware really need this low memory to survive (at least page
 tables could be put anywhere above low memory). However, in case of
 UEFI it is a wish of smart people not requirement set by spec. I was
 checking UEFI docs a few times but I was not able to find anything
 which says that e.g. ...developer MUST allocate memory outside of low
 memory  Even I have not found any suggestion about that. However,
 I must admit that I do not know whole UEFI spec, so, if you know something
 which is similar to above please tell me where it is (title, revision,
 page, paragraph, etc.). Hence, if there is not strict requirement in
 regards to memory allocation in specs we are lost. Of course we can
 encourage people to not do nasty things but I do not believe that many
 will listen. So, sadly, I suppose that we will see more and more machines
 in the wild which are broken (well, let's say do not align to good 
 practices).

 On the other hand I think that we should not assume that a given memory
 region (in our case starting at 1 MiB) is (or will be) available in every
 machine. I have not seen anything which says that it is true. We should
 stop doing that even if it works right now. I think that it works by
 chance and there is a chance that it will stop working one day because
 somebody will discover that he or she can put there e.g. device or hole.

 Last but not least. I suppose that Xen users and especially distros will
 complain when they are not able to use GRUB2 with Xen on every platform
 just because somebody (i.e. platform designers, developers, or whatever)
 do not accept our beliefs or we require that platform must obey rules
 (i.e. memory map requirements) which are specified nowhere.
 You're right, there's no such requirement on memory use in the spec.
 But you're missing the point. Supporting grub2 on UEFI is already a
 hack (ignoring all intentions EFI had from its first days). And now
 you've found an environment where that hack needs another hack
 (in Xen) to actually work. That's too much hackery for my taste, the
 more that things on this system can (afaict) work quite okay (without
 grub2, or with using its chainloader mechanism).

 So no, I'm still not convinced of the need for this patch.

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

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

Therefore I am very much +1 get grub working.

~Andrew

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

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

2015-08-27 Thread Daniel Kiper 
On Thu, Aug 27, 2015 at 07:12:38AM -0600, Jan Beulich wrote:
  On 20.07.15 at 16:29, daniel.ki...@oracle.com wrote:

[...]

   /* Copy bootstrap trampoline to low memory, below 1MB. */
  -mov $sym_phys(trampoline_start),%esi
  +lea sym_offset(trampoline_start)(%ebp),%esi
   mov $trampoline_end - trampoline_start,%ecx
   rep movsb
 

 The latest at this final hunk I'm getting tired (and upset). I'd much
 rather not touch all this code in these fragile ways, and instead
 require Xen to be booted without grub2 on badly written firmware
 like the one on the machine you quote in the description.

Let's start discussion from here. My further work on this patch series
is pointless until we do not agree details in this case.

So, I agree that any software (including firmware) should not use
precious resources (i.e. low memory in that case) if it is not strictly
required. And I do not think so that latest UEFI implementations
on new hardware really need this low memory to survive (at least page
tables could be put anywhere above low memory). However, in case of
UEFI it is a wish of smart people not requirement set by spec. I was
checking UEFI docs a few times but I was not able to find anything
which says that e.g. ...developer MUST allocate memory outside of low
memory  Even I have not found any suggestion about that. However,
I must admit that I do not know whole UEFI spec, so, if you know something
which is similar to above please tell me where it is (title, revision,
page, paragraph, etc.). Hence, if there is not strict requirement in
regards to memory allocation in specs we are lost. Of course we can
encourage people to not do nasty things but I do not believe that many
will listen. So, sadly, I suppose that we will see more and more machines
in the wild which are broken (well, let's say do not align to good practices).

On the other hand I think that we should not assume that a given memory
region (in our case starting at 1 MiB) is (or will be) available in every
machine. I have not seen anything which says that it is true. We should
stop doing that even if it works right now. I think that it works by
chance and there is a chance that it will stop working one day because
somebody will discover that he or she can put there e.g. device or hole.

Last but not least. I suppose that Xen users and especially distros will
complain when they are not able to use GRUB2 with Xen on every platform
just because somebody (i.e. platform designers, developers, or whatever)
do not accept our beliefs or we require that platform must obey rules
(i.e. memory map requirements) which are specified nowhere.

So, I think that early Xen boot code should be relocatable. However, I agree
that there is a chance that may solution is not perfect. So, I suppose,
taking into account above, we should discuss how to improve it instead
of discussing whether we should do it.

Daniel

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

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

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

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

You're right, there's no such requirement on memory use in the spec.
But you're missing the point. Supporting grub2 on UEFI is already a
hack (ignoring all intentions EFI had from its first days). And now
you've found an environment where that hack needs another hack
(in Xen) to actually work. That's too much hackery for my taste, the
more that things on this system can (afaict) work quite okay (without
grub2, or with using its chainloader mechanism).

So no, I'm still not convinced of the need for this patch.

Jan


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

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

2015-08-14 Thread Daniel Kiper 
On Fri, Aug 14, 2015 at 06:49:18AM -0600, Jan Beulich wrote:
  On 14.08.15 at 13:52, daniel.ki...@oracle.com wrote:
  On Tue, Aug 11, 2015 at 12:48:06PM -0400, Konrad Rzeszutek Wilk wrote:
  On Mon, Jul 20, 2015 at 04:29:17PM +0200, Daniel Kiper wrote:
   diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
   index 87b3341..27481ac 100644
   --- a/xen/include/asm-x86/page.h
   +++ b/xen/include/asm-x86/page.h
   @@ -283,7 +283,7 @@ extern root_pgentry_t 
   idle_pg_table[ROOT_PAGETABLE_ENTRIES];
extern l2_pgentry_t  *compat_idle_pg_table_l2;
extern unsigned int   m2p_compat_vstart;
extern l2_pgentry_t l2_xenmap[L2_PAGETABLE_ENTRIES],
   -l2_bootmap[L2_PAGETABLE_ENTRIES];
   +l2_bootmap[4*L2_PAGETABLE_ENTRIES];
 
  ? Why do we need to expand this to be 16kB?
 
  TBH, we need 8 KiB in the worst case. The worst case is when
  next GiB starts (e.g. 1 GiB ends and 2 GiB starts) in the middle
  of Xen image. In this situation we must hook up lower l2_bootmap
  table with lower l3_bootmap entry, higher l2_bootmap table with
  higher l3_bootmap entry and finally fill l2_bootmap relevant
  tables in proper way. Sadly, this method requires more calculations.
  To avoid that I have added 3 l2_bootmap tables and simply hook up
  one after another with relevant l3_bootmap entries. However, if
  you wish we can reduce number of l2_bootmap tables to two. This
  way code will be more complicated but we will save about 8 KiB.

 Wouldn't it be better (simpler) to enforce, say, 16Mb alignment
 in the PE32+ header (which the EFI loader would then honor)?

Good idea but then we must enforce this for multiboot protocol (v1 and v2) too.
multiboot2 with my patches supports that solution. However, multiboot (v1) could
be a bit problematic because it means that we must set load address to 16 MiB.
Are we sure that this region is available on all machines like region starting
at 1 MiB?

Daniel

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

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

2015-08-14 Thread Jan Beulich 
 On 14.08.15 at 15:59, daniel.ki...@oracle.com wrote:
 On Fri, Aug 14, 2015 at 06:49:18AM -0600, Jan Beulich wrote:
  On 14.08.15 at 13:52, daniel.ki...@oracle.com wrote:
  On Tue, Aug 11, 2015 at 12:48:06PM -0400, Konrad Rzeszutek Wilk wrote:
  On Mon, Jul 20, 2015 at 04:29:17PM +0200, Daniel Kiper wrote:
   diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
   index 87b3341..27481ac 100644
   --- a/xen/include/asm-x86/page.h
   +++ b/xen/include/asm-x86/page.h
   @@ -283,7 +283,7 @@ extern root_pgentry_t 
 idle_pg_table[ROOT_PAGETABLE_ENTRIES];
extern l2_pgentry_t  *compat_idle_pg_table_l2;
extern unsigned int   m2p_compat_vstart;
extern l2_pgentry_t l2_xenmap[L2_PAGETABLE_ENTRIES],
   -l2_bootmap[L2_PAGETABLE_ENTRIES];
   +l2_bootmap[4*L2_PAGETABLE_ENTRIES];
 
  ? Why do we need to expand this to be 16kB?
 
  TBH, we need 8 KiB in the worst case. The worst case is when
  next GiB starts (e.g. 1 GiB ends and 2 GiB starts) in the middle
  of Xen image. In this situation we must hook up lower l2_bootmap
  table with lower l3_bootmap entry, higher l2_bootmap table with
  higher l3_bootmap entry and finally fill l2_bootmap relevant
  tables in proper way. Sadly, this method requires more calculations.
  To avoid that I have added 3 l2_bootmap tables and simply hook up
  one after another with relevant l3_bootmap entries. However, if
  you wish we can reduce number of l2_bootmap tables to two. This
  way code will be more complicated but we will save about 8 KiB.

 Wouldn't it be better (simpler) to enforce, say, 16Mb alignment
 in the PE32+ header (which the EFI loader would then honor)?
 
 Good idea but then we must enforce this for multiboot protocol (v1 and v2) 
 too.
 multiboot2 with my patches supports that solution. However, multiboot (v1) 
 could
 be a bit problematic because it means that we must set load address to 16 
 MiB.
 Are we sure that this region is available on all machines like region 
 starting
 at 1 MiB?

This region being which one?

Jan


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

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

2015-08-14 Thread Daniel Kiper 
On Fri, Aug 14, 2015 at 08:32:05AM -0600, Jan Beulich wrote:
  On 14.08.15 at 15:59, daniel.ki...@oracle.com wrote:
  On Fri, Aug 14, 2015 at 06:49:18AM -0600, Jan Beulich wrote:
   On 14.08.15 at 13:52, daniel.ki...@oracle.com wrote:
   On Tue, Aug 11, 2015 at 12:48:06PM -0400, Konrad Rzeszutek Wilk wrote:
   On Mon, Jul 20, 2015 at 04:29:17PM +0200, Daniel Kiper wrote:
diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
index 87b3341..27481ac 100644
--- a/xen/include/asm-x86/page.h
+++ b/xen/include/asm-x86/page.h
@@ -283,7 +283,7 @@ extern root_pgentry_t
  idle_pg_table[ROOT_PAGETABLE_ENTRIES];
 extern l2_pgentry_t  *compat_idle_pg_table_l2;
 extern unsigned int   m2p_compat_vstart;
 extern l2_pgentry_t l2_xenmap[L2_PAGETABLE_ENTRIES],
-l2_bootmap[L2_PAGETABLE_ENTRIES];
+l2_bootmap[4*L2_PAGETABLE_ENTRIES];
  
   ? Why do we need to expand this to be 16kB?
  
   TBH, we need 8 KiB in the worst case. The worst case is when
   next GiB starts (e.g. 1 GiB ends and 2 GiB starts) in the middle
   of Xen image. In this situation we must hook up lower l2_bootmap
   table with lower l3_bootmap entry, higher l2_bootmap table with
   higher l3_bootmap entry and finally fill l2_bootmap relevant
   tables in proper way. Sadly, this method requires more calculations.
   To avoid that I have added 3 l2_bootmap tables and simply hook up
   one after another with relevant l3_bootmap entries. However, if
   you wish we can reduce number of l2_bootmap tables to two. This
   way code will be more complicated but we will save about 8 KiB.
 
  Wouldn't it be better (simpler) to enforce, say, 16Mb alignment
  in the PE32+ header (which the EFI loader would then honor)?
 
  Good idea but then we must enforce this for multiboot protocol (v1 and v2)
  too.
  multiboot2 with my patches supports that solution. However, multiboot (v1)
  could
  be a bit problematic because it means that we must set load address to 16
  MiB.
  Are we sure that this region is available on all machines like region
  starting
  at 1 MiB?

 This region being which one?

16 MiB - 32 MiB.

Daniel

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

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

2015-08-14 Thread Jan Beulich 
 On 14.08.15 at 16:37, daniel.ki...@oracle.com wrote:
 On Fri, Aug 14, 2015 at 08:32:05AM -0600, Jan Beulich wrote:
  On 14.08.15 at 15:59, daniel.ki...@oracle.com wrote:
  On Fri, Aug 14, 2015 at 06:49:18AM -0600, Jan Beulich wrote:
   On 14.08.15 at 13:52, daniel.ki...@oracle.com wrote:
   On Tue, Aug 11, 2015 at 12:48:06PM -0400, Konrad Rzeszutek Wilk wrote:
   On Mon, Jul 20, 2015 at 04:29:17PM +0200, Daniel Kiper wrote:
diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
index 87b3341..27481ac 100644
--- a/xen/include/asm-x86/page.h
+++ b/xen/include/asm-x86/page.h
@@ -283,7 +283,7 @@ extern root_pgentry_t
  idle_pg_table[ROOT_PAGETABLE_ENTRIES];
 extern l2_pgentry_t  *compat_idle_pg_table_l2;
 extern unsigned int   m2p_compat_vstart;
 extern l2_pgentry_t l2_xenmap[L2_PAGETABLE_ENTRIES],
-l2_bootmap[L2_PAGETABLE_ENTRIES];
+l2_bootmap[4*L2_PAGETABLE_ENTRIES];
  
   ? Why do we need to expand this to be 16kB?
  
   TBH, we need 8 KiB in the worst case. The worst case is when
   next GiB starts (e.g. 1 GiB ends and 2 GiB starts) in the middle
   of Xen image. In this situation we must hook up lower l2_bootmap
   table with lower l3_bootmap entry, higher l2_bootmap table with
   higher l3_bootmap entry and finally fill l2_bootmap relevant
   tables in proper way. Sadly, this method requires more calculations.
   To avoid that I have added 3 l2_bootmap tables and simply hook up
   one after another with relevant l3_bootmap entries. However, if
   you wish we can reduce number of l2_bootmap tables to two. This
   way code will be more complicated but we will save about 8 KiB.
 
  Wouldn't it be better (simpler) to enforce, say, 16Mb alignment
  in the PE32+ header (which the EFI loader would then honor)?
 
  Good idea but then we must enforce this for multiboot protocol (v1 and v2)
  too.
  multiboot2 with my patches supports that solution. However, multiboot (v1)
  could
  be a bit problematic because it means that we must set load address to 16
  MiB.
  Are we sure that this region is available on all machines like region
  starting
  at 1 MiB?

 This region being which one?
 
 16 MiB - 32 MiB.

While I think all systems where Xen can reasonably run on would
have memory in that range, I'd really prefer not to touch the MB1
case (i.e. find a way for it to continue to load at 1Mb). Perhaps
the 16Mb alignment could be specified only in the PE32+ header,
but not enforced in the ELF one?

Jan


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

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

2015-08-14 Thread Konrad Rzeszutek Wilk 
trampoline_bios_setup:
   +mov %ebp,%esi
   +
   +/* Initialise GDT and basic data segments. */
   +add %ebp,sym_offset(gdt_boot_descr_addr)(%esi)
   +lgdtsym_offset(gdt_boot_descr)(%esi)
   +
   +mov $BOOT_DS,%ecx
   +mov %ecx,%ds
   +mov %ecx,%es
   +mov %ecx,%fs
   +mov %ecx,%gs
   +mov %ecx,%ss
   +
 
 
  The non-EFI boot path is now:
 
  start
   \- __start
   \- multiboot2_proto
   |jmp trampoline_bios_setup
   |
   \-and if not MB2: jmp trampoline_bios_setup.
 
 
  In here you tweak the GDT and reload the %ds - but during
  this call chain we do touch the %ds - via:
 
  __start+27:testb  $0x1,(%rbx)
  __start+30:cmovne 0x4(%rbx),%edx
 
  which is OK (as MB1 says that the %ds has to cover up to 4GB).
  But I wonder why the __start code had the segments reloaded so early?
  Was the bootloader not setting the proper segments?
 
 This is very good question. I was asking myself about that thing at
 least once. Sadly, I cannot find real explanation.
 
  Let me double-check what SYSLINUX's mboot.c32 does. Perhaps
  it had done something odd in the past.
 
 Good idea!

Nope, the mboot.c32 COMBOOT images are booted with:

  %ds,%es,%fs,%gs : 32-bit data segment with zero base and 4GB limit  

So that is OK. Perhaps this code used to be shared with trampoline
startup and got copied over.

Anyhow not worried about it.

.. snip..
   +/* Initialize %fs and later use it to access Xen data if 
   possible. */
   +mov $BOOT_FS,%edx
   +mov %edx,%fs
   +
 
  We just modified the GDT. Should we reload it (lgdt?)?
 
 I do not think it is needed.
 
 Intel 64 and IA-32 Architectures Software Developer’s Manual,
 Volume 2 (2A, 2B  2C): Instruction Set Reference, A-Z says:
 
 LGDT ... Loads the values in the source operand into the global
 descriptor table register (GDTR)...
 
 ...and ...
 
 MOV ... If the destination operand is a segment register (DS, ES,
 FS, GS, or SS), the source operand must be a valid segment selector.
 In protected mode, moving a segment selector into a segment register
 automatically causes the segment descriptor information associated
 with that segment selector to be loaded into the hidden (shadow) part
 of the segment register. While loading this information, the segment

.. snip..
 shadow register.) When a segment selector is loaded into the visible
 part of a segment register, the processor also loads the hidden part
 of the segment register with the base address, segment limit, and access
 control information from the segment descriptor pointed to by the segment
 selector. The information cached in the segment register (visible and

Excellent!

 hidden) allows the processor to translate addresses without taking
 extra bus cycles to read the base address and limit from the segment
 descriptor. In systems in which multiple processors have access to the
 same descriptor tables, it is the responsibility of software to reload
 the segment registers when the descriptor tables are modified (side note:
 GDTR reload is not required! probably the same applies to UP systems
 and if CPU update own active GDT). If this is not done, an old segment
 descriptor cached in a segment register might be used after its
 memory-resident version has been modified.
 
 AIUI, only GDT address and limit are loaded into GDTR when lgdt is executed.
 Segment descriptor is loaded directly from memory into segment register
 (hiden part) only when relevant load instruction is used (e.g. mov %edx,%fs).

Yeey!

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

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

2015-08-14 Thread Jan Beulich 
 On 14.08.15 at 13:52, daniel.ki...@oracle.com wrote:
 On Tue, Aug 11, 2015 at 12:48:06PM -0400, Konrad Rzeszutek Wilk wrote:
 On Mon, Jul 20, 2015 at 04:29:17PM +0200, Daniel Kiper wrote:
  diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
  index 87b3341..27481ac 100644
  --- a/xen/include/asm-x86/page.h
  +++ b/xen/include/asm-x86/page.h
  @@ -283,7 +283,7 @@ extern root_pgentry_t 
  idle_pg_table[ROOT_PAGETABLE_ENTRIES];
   extern l2_pgentry_t  *compat_idle_pg_table_l2;
   extern unsigned int   m2p_compat_vstart;
   extern l2_pgentry_t l2_xenmap[L2_PAGETABLE_ENTRIES],
  -l2_bootmap[L2_PAGETABLE_ENTRIES];
  +l2_bootmap[4*L2_PAGETABLE_ENTRIES];

 ? Why do we need to expand this to be 16kB?
 
 TBH, we need 8 KiB in the worst case. The worst case is when
 next GiB starts (e.g. 1 GiB ends and 2 GiB starts) in the middle
 of Xen image. In this situation we must hook up lower l2_bootmap
 table with lower l3_bootmap entry, higher l2_bootmap table with
 higher l3_bootmap entry and finally fill l2_bootmap relevant
 tables in proper way. Sadly, this method requires more calculations.
 To avoid that I have added 3 l2_bootmap tables and simply hook up
 one after another with relevant l3_bootmap entries. However, if
 you wish we can reduce number of l2_bootmap tables to two. This
 way code will be more complicated but we will save about 8 KiB.

Wouldn't it be better (simpler) to enforce, say, 16Mb alignment
in the PE32+ header (which the EFI loader would then honor)?

Jan


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

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

2015-08-11 Thread Konrad Rzeszutek Wilk 
On Mon, Jul 20, 2015 at 04:29:17PM +0200, Daniel Kiper wrote:
 Every multiboot protocol (regardless of version) compatible image must
 specify its load address (in ELF or multiboot header). Multiboot protocol
 compatible loader have to load image at specified address. However, there
 is no guarantee that the requested memory region (in case of Xen it starts
 at 1 MiB and ends at 17 MiB) where image should be loaded initially is a RAM
 and it is free (legacy BIOS platforms are merciful for Xen but I found at
 least one EFI platform on which Xen load address conflicts with EFI boot
 services; it is Dell PowerEdge R820 with latest firmware). To cope with
 that problem we must make Xen early boot code relocatable. This patch does
 that. However, it does not add multiboot2 protocol interface which is done
 in next patch.

s/next patch/x86: add multiboot2 protocol support for relocatable image.
 
 This patch changes following things:
   - default load address is changed from 1 MiB to 2 MiB; I did that because
 initial page tables are using 2 MiB huge pages and this way required
 updates for them are quite easy; it means that e.g. we avoid spacial
 cases for beginning and end of required memory region if it live at
 address not aligned to 2 MiB,
   - %ebp register is used as a storage for Xen image base address; this way
 we can get this value very quickly if it is needed; however, %ebp register
 is not used directly to access a given memory region,
   - %fs register is filled with segment descriptor which describes memory 
 region
 with Xen image (it could be relocated or not); it is used to access some 
 of

'memory region with Xen image' ? Not sure I follow?

Perhaps:
segment descriptor which starts (0) at Xen image base (_start).


 Xen data in early boot code; potentially we can use above mentioned 
 segment
 descriptor to access data using %ds:%esi and/or %es:%esi (e.g. movs*); 
 however,
 I think that it could unnecessarily obfuscate code (e.g. we need at least
 to operations to reload a given segment descriptor) and current solution

s/to/two/ ?
 looks quite optimal.
 
 Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
 ---
  xen/arch/x86/Makefile  |6 +-
  xen/arch/x86/Rules.mk  |4 +
  xen/arch/x86/boot/head.S   |  165 
 ++--
  xen/arch/x86/boot/trampoline.S |   11 ++-
  xen/arch/x86/boot/wakeup.S |6 +-
  xen/arch/x86/boot/x86_64.S |   34 -
  xen/arch/x86/setup.c   |   33 
  xen/arch/x86/x86_64/mm.c   |2 +-
  xen/arch/x86/xen.lds.S |2 +-
  xen/include/asm-x86/config.h   |3 +
  xen/include/asm-x86/page.h |2 +-
  11 files changed, 182 insertions(+), 86 deletions(-)
 
 diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
 index 82c5a93..93069a8 100644
 --- a/xen/arch/x86/Makefile
 +++ b/xen/arch/x86/Makefile
 @@ -72,8 +72,10 @@ efi-$(x86_64) := $(shell if [ ! -r 
 $(BASEDIR)/include/xen/compile.h -o \
   echo '$(TARGET).efi'; fi)
  
  $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
 - ./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x10 \
 - `$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`
 +#THIS IS UGLY HACK! PLEASE DO NOT COMPLAIN. I WILL FIX IT IN NEXT 
 RELEASE.

OK :-)

 + ./boot/mkelf32 $(TARGET)-syms $(TARGET) $(XEN_IMG_PHYS_START) 
 0x82d08100
 +#./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x10 \
 +#`$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`
  
  
  ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o 
 $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
 diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
 index 4a04a8a..7ccb8a0 100644
 --- a/xen/arch/x86/Rules.mk
 +++ b/xen/arch/x86/Rules.mk
 @@ -15,6 +15,10 @@ HAS_GDBSX := y
  HAS_PDX := y
  xenoprof := y
  
 +XEN_IMG_PHYS_START = 0x20
 +
 +CFLAGS += -DXEN_IMG_PHYS_START=$(XEN_IMG_PHYS_START)
 +
  CFLAGS += -I$(BASEDIR)/include 
  CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
  CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
 diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
 index 3f1054d..d484f68 100644
 --- a/xen/arch/x86/boot/head.S
 +++ b/xen/arch/x86/boot/head.S
 @@ -12,13 +12,15 @@
  .text
  .code32
  
 -#define sym_phys(sym) ((sym) - __XEN_VIRT_START)
 +#define sym_phys(sym) ((sym) - __XEN_VIRT_START + XEN_IMG_PHYS_START - 
 XEN_IMG_OFFSET)
 +#define sym_offset(sym)   ((sym) - __XEN_VIRT_START)
  
  #define BOOT_CS320x0008
  #define BOOT_CS640x0010
  #define BOOT_DS  0x0018
  #define BOOT_PSEUDORM_CS 0x0020
  #define BOOT_PSEUDORM_DS 0x0028
 +#define BOOT_FS  0x0030
  
  #define MB2_HT(name)  (MULTIBOOT2_HEADER_TAG_##name)
  #define MB2_TT(name)  (MULTIBOOT2_TAG_TYPE_##name)
 @@ -105,12 +107,13 @@ multiboot1_header_end:
  
  .word   0
  gdt_boot_descr:
 -

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

2015-07-20 Thread Daniel Kiper 
Every multiboot protocol (regardless of version) compatible image must
specify its load address (in ELF or multiboot header). Multiboot protocol
compatible loader have to load image at specified address. However, there
is no guarantee that the requested memory region (in case of Xen it starts
at 1 MiB and ends at 17 MiB) where image should be loaded initially is a RAM
and it is free (legacy BIOS platforms are merciful for Xen but I found at
least one EFI platform on which Xen load address conflicts with EFI boot
services; it is Dell PowerEdge R820 with latest firmware). To cope with
that problem we must make Xen early boot code relocatable. This patch does
that. However, it does not add multiboot2 protocol interface which is done
in next patch.

This patch changes following things:
  - default load address is changed from 1 MiB to 2 MiB; I did that because
initial page tables are using 2 MiB huge pages and this way required
updates for them are quite easy; it means that e.g. we avoid spacial
cases for beginning and end of required memory region if it live at
address not aligned to 2 MiB,
  - %ebp register is used as a storage for Xen image base address; this way
we can get this value very quickly if it is needed; however, %ebp register
is not used directly to access a given memory region,
  - %fs register is filled with segment descriptor which describes memory region
with Xen image (it could be relocated or not); it is used to access some of
Xen data in early boot code; potentially we can use above mentioned segment
descriptor to access data using %ds:%esi and/or %es:%esi (e.g. movs*); 
however,
I think that it could unnecessarily obfuscate code (e.g. we need at least
to operations to reload a given segment descriptor) and current solution
looks quite optimal.

Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
---
 xen/arch/x86/Makefile  |6 +-
 xen/arch/x86/Rules.mk  |4 +
 xen/arch/x86/boot/head.S   |  165 ++--
 xen/arch/x86/boot/trampoline.S |   11 ++-
 xen/arch/x86/boot/wakeup.S |6 +-
 xen/arch/x86/boot/x86_64.S |   34 -
 xen/arch/x86/setup.c   |   33 
 xen/arch/x86/x86_64/mm.c   |2 +-
 xen/arch/x86/xen.lds.S |2 +-
 xen/include/asm-x86/config.h   |3 +
 xen/include/asm-x86/page.h |2 +-
 11 files changed, 182 insertions(+), 86 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 82c5a93..93069a8 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -72,8 +72,10 @@ efi-$(x86_64) := $(shell if [ ! -r 
$(BASEDIR)/include/xen/compile.h -o \
  echo '$(TARGET).efi'; fi)
 
 $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
-   ./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x10 \
-   `$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`
+#  THIS IS UGLY HACK! PLEASE DO NOT COMPLAIN. I WILL FIX IT IN NEXT 
RELEASE.
+   ./boot/mkelf32 $(TARGET)-syms $(TARGET) $(XEN_IMG_PHYS_START) 
0x82d08100
+#  ./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x10 \
+#  `$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`
 
 
 ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o 
$(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 4a04a8a..7ccb8a0 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -15,6 +15,10 @@ HAS_GDBSX := y
 HAS_PDX := y
 xenoprof := y
 
+XEN_IMG_PHYS_START = 0x20
+
+CFLAGS += -DXEN_IMG_PHYS_START=$(XEN_IMG_PHYS_START)
+
 CFLAGS += -I$(BASEDIR)/include 
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 3f1054d..d484f68 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -12,13 +12,15 @@
 .text
 .code32
 
-#define sym_phys(sym) ((sym) - __XEN_VIRT_START)
+#define sym_phys(sym) ((sym) - __XEN_VIRT_START + XEN_IMG_PHYS_START - 
XEN_IMG_OFFSET)
+#define sym_offset(sym)   ((sym) - __XEN_VIRT_START)
 
 #define BOOT_CS320x0008
 #define BOOT_CS640x0010
 #define BOOT_DS  0x0018
 #define BOOT_PSEUDORM_CS 0x0020
 #define BOOT_PSEUDORM_DS 0x0028
+#define BOOT_FS  0x0030
 
 #define MB2_HT(name)  (MULTIBOOT2_HEADER_TAG_##name)
 #define MB2_TT(name)  (MULTIBOOT2_TAG_TYPE_##name)
@@ -105,12 +107,13 @@ multiboot1_header_end:
 
 .word   0
 gdt_boot_descr:
-.word   6*8-1
-.long   sym_phys(trampoline_gdt)
+.word   7*8-1
+gdt_boot_descr_addr:
+.long   sym_offset(trampoline_gdt)
 .long   0 /* Needed for 64-bit lgdt */
 
 cs32_switch_addr:
-.long   sym_phys(cs32_switch)
+.long   sym_offset(cs32_switch)
 .word   BOOT_CS32
 
 .Lbad_cpu_msg: .asciz ERR: Not a 64-bit CPU!
@@ -120,13