Re: [Xen-devel] [PATCH v2 09/18] xen: add PVH boot entry code

[Juergen Gross]

On 19/10/2018 14:17, Daniel Kiper wrote:
> On Tue, Oct 09, 2018 at 01:03:08PM +0200, Juergen Gross wrote:
>> Add the code for the Xen PVH mode boot entry.
>>
>> Signed-off-by: Juergen Gross <address@hidden>
>> ---
>>  grub-core/kern/i386/xen/startup_pvh.S | 50 
>> +++++++++++++++++++++++++++++++++++
>>  1 file changed, 50 insertions(+)
>>
>> diff --git a/grub-core/kern/i386/xen/startup_pvh.S 
>> b/grub-core/kern/i386/xen/startup_pvh.S
>> index e18ee5b31..0ddb63b31 100644
>> --- a/grub-core/kern/i386/xen/startup_pvh.S
>> +++ b/grub-core/kern/i386/xen/startup_pvh.S
>> @@ -19,11 +19,61 @@
>>
>>  #include <config.h>
>>  #include <grub/symbol.h>
>> +#include <grub/machine/memory.h>
>>
>>      .file   "startup_pvh.S"
>>      .text
>> +    .globl  start, _start
>> +    .code32
>>
>> +start:
>> +_start:
>> +    cld
>> +    lgdt    gdtdesc
>> +    ljmp    $GRUB_MEMORY_MACHINE_PROT_MODE_CSEG, $1f
>> +1:
>> +    movl    $GRUB_MEMORY_MACHINE_PROT_MODE_DSEG, %eax
>> +    mov     %eax, %ds
>> +    mov     %eax, %es
>> +    mov     %eax, %ss
> 
> Should not you load null descriptor into %fs and %gs?
> Just in case...

Hmm, if you want I can do that, sure.

> 
>> +    leal    LOCAL(stack_end), %esp
>> +
>> +    /* Save address of start info structure. */
>> +    mov     %ebx, pvh_start_info
>> +    call    EXT_C(grub_main)
>> +    /* Doesn't return. */
>> +
>> +    .p2align        3
>> +gdt:
>> +    .word   0, 0
>> +    .byte   0, 0, 0, 0
>> +
>> +    /* -- code segment --
>> +     * base = 0x00000000, limit = 0xFFFFF (4 KiB Granularity), present
>> +     * type = 32bit code execute/read, DPL = 0
>> +     */
>> +    .word   0xFFFF, 0
>> +    .byte   0, 0x9A, 0xCF, 0
>> +
>> +    /* -- data segment --
>> +     * base = 0x00000000, limit 0xFFFFF (4 KiB Granularity), present
>> +     * type = 32 bit data read/write, DPL = 0
>> +     */
>> +    .word   0xFFFF, 0
>> +    .byte   0, 0x92, 0xCF, 0
>> +
>> +    .p2align        3
>> +/* this is the GDT descriptor */
>> +gdtdesc:
>> +    .word   0x17            /* limit */
>> +    .long   gdt             /* addr */
>> +
>> +    .p2align        2
>>  /* Saved pointer to start info structure. */
>>      .globl  pvh_start_info
>>  pvh_start_info:
>>      .long   0
>> +
>> +    .bss
>> +    .space  (1 << 22)
> 
> Hmmm... Why do we need 4 MiB here? If this is really needed then it begs for
> a comment. And I would like to see a constant instead of plain number here.

This is just copied from xen/startup.S

I can reduce it to something near GRUB_MEMORY_MACHINE_PROT_STACK_SIZE
(about 64kB).


Juergen