Re: [Minios-devel] [UNIKRAFT PATCH 1/4] plat/kvm: readibility fixes

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

Hi Florian,

this patch looks good to me.

Reviewed-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>

BR, Yuri.

Florian Schmidt <florian.schmidt@xxxxxxxxx> writes:

> * Remove magic numbers from early boot code, replacing it with labels
>   and defines.
> * Add and rectify comments.
>
> Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
> ---
>  plat/kvm/include/kvm-x86/traps.h |  7 ++++---
>  plat/kvm/x86/entry64.S           | 19 +++++++++++++------
>  2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/plat/kvm/include/kvm-x86/traps.h 
> b/plat/kvm/include/kvm-x86/traps.h
> index 8210613b..c457bda5 100644
> --- a/plat/kvm/include/kvm-x86/traps.h
> +++ b/plat/kvm/include/kvm-x86/traps.h
> @@ -24,12 +24,12 @@
>  /*
>   * GDT layout
>   *
> - * This should be kept consistent with the layout used by the ukvm target (as
> - * defined in ukvm/ukvm_cpu_x86_64.h.
> + * This must be kept consistent with the layout used by the kvm entry code 
> (as
> + * defined in plat/kvm/x86/entry64.S)
>   */
>  #define GDT_DESC_NULL           0
>  #define GDT_DESC_CODE           1
> -#define GDT_DESC_CODE32         2 /* Used by boot.S on virtio targets */
> +#define GDT_DESC_CODE32         2 /* Used by _libkvmplat_start32 */
>  #define GDT_DESC_DATA           3
>  #define GDT_DESC_TSS_LO         4
>  #define GDT_DESC_TSS_HI         5
> @@ -39,6 +39,7 @@
>  #define GDT_NUM_ENTRIES         6
>  
>  #define GDT_DESC_CODE_VAL       0x00af99000000ffff
> +#define GDT_DESC_CODE32_VAL     0x00cf9b000000ffff
>  #define GDT_DESC_DATA_VAL       0x00cf93000000ffff
>  
>  
> diff --git a/plat/kvm/x86/entry64.S b/plat/kvm/x86/entry64.S
> index 1ec81364..e15d41e2 100644
> --- a/plat/kvm/x86/entry64.S
> +++ b/plat/kvm/x86/entry64.S
> @@ -79,12 +79,14 @@ ENTRY(_libkvmplat_start32)
>       jne nomultiboot
>  
>       lgdt (gdt64_ptr)
> -     pushl $0x0
> -     pushw $0x10
> +     /* let lret jump just one instruction ahead, but set %cs
> +      * to the correct GDT entry while doing that.
> +      */
> +     pushl $(gdt64_32cs-gdt64)
>       pushl $1f
>       lret
>  
> -1:   movl $0x18, %eax
> +1:   movl $(gdt64_ds-gdt64), %eax
>       movl %eax, %ds
>       movl %eax, %es
>       movl %eax, %ss
> @@ -117,8 +119,10 @@ ENTRY(_libkvmplat_start32)
>       orl $X86_CR0_PG, %eax
>       movl %eax, %cr0
>  
> -     /* 5: poetically longjump to longmode */
> -     pushw $0x08
> +     /* 5: poetically longjump to longmode,
> +      *    letting lret set %cs to the correct GDT entry on the way
> +      */
> +     pushl $(gdt64_64cs-gdt64)
>       pushl $_libkvmplat_start64
>       lret
>  
> @@ -145,8 +149,11 @@ END(_libkvmplat_start32)
>  .align 64
>  gdt64:
>       .quad 0x0000000000000000
> +gdt64_64cs:
>       .quad GDT_DESC_CODE_VAL         /* 64bit CS             */
> -     .quad 0x00cf9b000000ffff        /* 32bit CS             */
> +gdt64_32cs:
> +     .quad GDT_DESC_CODE32_VAL       /* 32bit CS             */
> +gdt64_ds:
>       .quad GDT_DESC_DATA_VAL         /* DS                   */
>       .quad 0x0000000000000000        /* TSS part 1 (via C)   */
>       .quad 0x0000000000000000        /* TSS part 2 (via C)   */
> -- 
> 2.21.0
>

-- 
Yuri Volchkov
Software Specialist

NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel