[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [v2 1/4] xen/arm64: head: Don't map too much in boot_third


  • To: Julien Grall <julien@xxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Tue, 4 Jul 2023 14:09:06 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=EogklM6MvVYcQsFwT0exf4GD0ezono7El60mI0r+JDE=; b=MJ+eNXG0BfuR3zGgPcq2z1Y9BfrzT6fWT3Q11HMo5udYRGAi0sCBfXPlW1+TFvNu30bg/YHSarv2PMwlt0ypdfsC1H56kpQlWr9x1FZoHTFJQd+knIq4rTiOoLJfmS6OPggbT6y+CsVCxzqcBt9TIU60gmY+1pkkhqKtOyXM0UQgN0SityBYn0DI5YeN7bz5yDaVMbGD90U3UKQM0aqTTTueF3pa5eWaPPHzpqMvD/mr0OeFJlfy6/NE3tN1N+L6C//yjPTiA8SvrARN0jSa9fkHp+ach+teJS4aydPFrtV9Lgd0mVipmDNxG8ABtjosIaUGZgCGPVgkwDsveR+s8g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=T12WbI7oLNSnLx176fcj1mfQyoGBFMgxKus+GNwhyarZ7TYm/6Qc+WOEDwQeTXri9W173dpMvHT98l1GudmDg6JhHXak2Cdfd/rM9xvS62D//MLJZ5Z0HW5CEk7F3gqmAYWJ5j6Vh9w87nWlYCvl4HPjxuxHFu+7x/COK9bvbRWyAnxVzTjKH4Cg1ocBErrkqXL1xJFxqq4UNWP9CqHy+vP3rxO9IQpHimhbc5TAOFG5/4g9VXxLmTULexQ43qBaeldGbIgLeBaQbF2aEVc9dEORr6qnnUgZLk19hfk+pEBXdu8pQTpZVQIG+dJ8PMmasoLPTx9TgqD5ZfpvT+mbsA==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Luca Fancellu <Luca.Fancellu@xxxxxxx>, "michal.orzel@xxxxxxx" <michal.orzel@xxxxxxx>, Henry Wang <Henry.Wang@xxxxxxx>, Julien Grall <jgrall@xxxxxxxxxx>
  • Delivery-date: Tue, 04 Jul 2023 14:09:23 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHZqsX4yAVem92w702jqx8j/I6WWK+prJaA
  • Thread-topic: [v2 1/4] xen/arm64: head: Don't map too much in boot_third

Hi Julien,

> On 29 Jun 2023, at 22:11, Julien Grall <julien@xxxxxxx> wrote:
> 
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> At the moment, we are mapping the size of the reserved area for Xen
> (i.e. 2MB) even if the binary is smaller. We don't exactly know what's
> after Xen, so it is not a good idea to map more than necessary for a
> couple of reasons:
>    * We would need to use break-before-make if the extra PTE needs to
>      be updated to point to another region
>    * The extra area mapped may be mapped again by Xen with different
>      memory attribute. This would result to attribute mismatch.
> 
> Therefore, rework the logic in create_page_tables() to map only what's
> necessary. To simplify the logic, we also want to make sure _end
> is page-aligned. So align the symbol in the linker and add an assert
> to catch any change.
> 
> Lastly, take the opportunity to confirm that _start is equal to
> XEN_VIRT_START as the assembly is using both interchangeably.
> 
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>

Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand

> 
> ---
> 
>    Changes in v2:
>        - Fix typo and coding style
>        - Check _start == XEN_VIRT_START
> ---
> xen/arch/arm/arm64/head.S | 15 ++++++++++++++-
> xen/arch/arm/xen.lds.S    |  9 +++++++++
> 2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index c0e03755bb10..5e9562a22240 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -572,6 +572,19 @@ create_page_tables:
>         create_table_entry boot_first, boot_second, x0, 1, x1, x2, x3
>         create_table_entry boot_second, boot_third, x0, 2, x1, x2, x3
> 
> +        /*
> +         * Find the size of Xen in pages and multiply by the size of a
> +         * PTE. This will then be compared in the mapping loop below.
> +         *
> +         * Note the multiplication is just to avoid using an extra
> +         * register/instruction per iteration.
> +         */
> +        ldr   x0, =_start            /* x0 := vaddr(_start) */
> +        ldr   x1, =_end              /* x1 := vaddr(_end) */
> +        sub   x0, x1, x0             /* x0 := effective size of Xen */
> +        lsr   x0, x0, #PAGE_SHIFT    /* x0 := Number of pages for Xen */
> +        lsl   x0, x0, #3             /* x0 := Number of pages * PTE size */
> +
>         /* Map Xen */
>         adr_l x4, boot_third
> 
> @@ -585,7 +598,7 @@ create_page_tables:
> 1:      str   x2, [x4, x1]           /* Map vaddr(start) */
>         add   x2, x2, #PAGE_SIZE     /* Next page */
>         add   x1, x1, #8             /* Next slot */
> -        cmp   x1, #(XEN_PT_LPAE_ENTRIES<<3) /* 512 entries per page */
> +        cmp   x1, x0                 /* Loop until we map all of Xen */
>         b.lt  1b
> 
>         /*
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index d36b67708ab1..a3c90ca82316 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -212,6 +212,7 @@ SECTIONS
>        . = ALIGN(POINTER_ALIGN);
>        __bss_end = .;
>   } :text
> +  . = ALIGN(PAGE_SIZE);
>   _end = . ;
> 
>   /* Section for the device tree blob (if any). */
> @@ -226,6 +227,12 @@ SECTIONS
>   ELF_DETAILS_SECTIONS
> }
> 
> +/*
> + * The assembly code use _start and XEN_VIRT_START interchangeably to
> + * match the context.
> + */
> +ASSERT(_start == XEN_VIRT_START, "_start != XEN_VIRT_START")
> +
> /*
>  * We require that Xen is loaded at a page boundary, so this ensures that any
>  * code running on the identity map cannot cross a section boundary.
> @@ -241,4 +248,6 @@ ASSERT(IS_ALIGNED(__init_begin,     4), "__init_begin is 
> misaligned")
> ASSERT(IS_ALIGNED(__init_end,       4), "__init_end is misaligned")
> ASSERT(IS_ALIGNED(__bss_start,      POINTER_ALIGN), "__bss_start is 
> misaligned")
> ASSERT(IS_ALIGNED(__bss_end,        POINTER_ALIGN), "__bss_end is misaligned")
> +/* To simplify the logic in head.S, we want to _end to be page aligned */
> +ASSERT(IS_ALIGNED(_end,             PAGE_SIZE), "_end is not page aligned")
> ASSERT((_end - _start) <= XEN_VIRT_SIZE, "Xen is too big")
> -- 
> 2.40.1
> 
> 




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.