[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/9] xen/arm64: head: Don't map too much in boot_third
On 28/06/2023 20:21, Julien Grall wrote: > > > Hi, > > On 26/06/2023 12:28, Michal Orzel wrote: >> On 25/06/2023 22:49, Julien Grall 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 attribue mismatch. >> s/attribue/attribute >> >>> >>> 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. >>> >>> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx> >>> --- >>> xen/arch/arm/arm64/head.S | 15 ++++++++++++++- >>> xen/arch/arm/xen.lds.S | 3 +++ >>> 2 files changed, 17 insertions(+), 1 deletion(-) >>> >>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >>> index f37133cf7ccd..66bc85d4c39e 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) */ >> x0 is already set to vaddr of _start by the first instruction of >> create_page_tables >> and is preserved by create_table_entry. You could just reuse it instead of >> re-loading. > > I agree that the load is technically redundant. However, I find this > approach easier to read (you don't have to remember that _start equals > XEN_VIRT_START). Ok. As a side note not related to this patch, would it make sense to add an assert in linker script to make sure _start equals XEN_VIRT_START, since we use them interchangeably? ~Michal
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |