Re: [Xen-devel] [PATCH v5 04/12] x86: make paddr_bits available earlier

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jan 29, 2020 at 08:20:26PM +0000, Wei Liu wrote:
> Move early_cpu_init before init_e820, such that paddr_bits can be used
> by e820 code.
> 
> This will reduce code repetition and prepare for further adjustment when
> L0 hypervisor comes into play.
> 
> Signed-off-by: Wei Liu <liuwe@xxxxxxxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

One typo below.

> ---
>  xen/arch/x86/e820.c  | 14 ++++----------
>  xen/arch/x86/setup.c |  5 +++--
>  2 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
> index 082f9928a1..3892c9cfb7 100644
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -420,7 +420,7 @@ static uint64_t __init mtrr_top_of_ram(void)
>  {
>      uint32_t eax, ebx, ecx, edx;
>      uint64_t mtrr_cap, mtrr_def, addr_mask, base, mask, top;
> -    unsigned int i, phys_bits = 36;
> +    unsigned int i;
>  
>      /* By default we check only Intel systems. */
>      if ( e820_mtrr_clip == -1 )
> @@ -445,15 +445,9 @@ static uint64_t __init mtrr_top_of_ram(void)
>      if ( !test_bit(X86_FEATURE_MTRR & 31, &edx) )
>           return 0;
>  
> -    /* Find the physical address size for this CPU. */
> -    eax = cpuid_eax(0x80000000);
> -    if ( (eax >> 16) == 0x8000 && eax >= 0x80000008 )
> -    {
> -        phys_bits = (uint8_t)cpuid_eax(0x80000008);
> -        if ( phys_bits > PADDR_BITS )
> -            phys_bits = PADDR_BITS;
> -    }
> -    addr_mask = ((1ull << phys_bits) - 1) & ~((1ull << 12) - 1);
> +    /* paddr_bits must have been set at this point */
> +    ASSERT(paddr_bits);
> +    addr_mask = ((1ull << paddr_bits) - 1) & PAGE_MASK;
>  
>      rdmsrl(MSR_MTRRcap, mtrr_cap);
>      rdmsrl(MSR_MTRRdefType, mtrr_def);
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index d858883404..89fe49149f 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -954,6 +954,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>      else
>          panic("Bootloader provided no memory information\n");
>  
> +    /* This must come before e820 code becuause it sets paddr_bits. */
                                          ^ because

Thanks, Roger.

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