On 02/07/2025 16:14, Hari Limaye wrote:
> From: Luca Fancellu <luca.fance...@arm.com>
>
> Implement the function early_fdt_map(), which is responsible for mapping
> the Device Tree Blob in the early stages of the boot process, for MPU
> systems.
>
> We make use of the map_pages_to_xen() and destroy_xen_mappings() APIs.
> In particular the latter function is necessary in the case that the
> initial mapping of the fdt_header is insufficient to cover the entire
> DTB, as we must destroy and then remap the region due to the APIs no
> providing support for extending the size of an existing region.
>
> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
> Signed-off-by: Hari Limaye <hari.lim...@arm.com>
> Reviewed-by: Ayan Kumar Halder <ayan.kumar.hal...@amd.com>
> ---
> Changes from v1:
> - Add Ayan's R-b
> ---
> xen/arch/arm/mpu/setup.c | 74 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 72 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/mpu/setup.c b/xen/arch/arm/mpu/setup.c
> index b4da77003f..ab00cb944b 100644
> --- a/xen/arch/arm/mpu/setup.c
> +++ b/xen/arch/arm/mpu/setup.c
> @@ -1,17 +1,87 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
>
> +#include <xen/bootfdt.h>
> #include <xen/bug.h>
> #include <xen/init.h>
> +#include <xen/libfdt/libfdt.h>
> #include <xen/mm.h>
> +#include <xen/pfn.h>
> #include <xen/types.h>
> #include <asm/setup.h>
>
> +static paddr_t __initdata mapped_fdt_paddr = INVALID_PADDR;
I think it would be better to name it mapped_fdt_base, given that in MPU
everything refers to base,limit.
> +static paddr_t __initdata mapped_fdt_limit = INVALID_PADDR;
> +
> void __init setup_pagetables(void) {}
>
> void * __init early_fdt_map(paddr_t fdt_paddr)
> {
> - BUG_ON("unimplemented");
> - return NULL;
> + /* Map at least a page containing the DTB address, exclusive range */
> + paddr_t base = round_pgdown(fdt_paddr);
> + paddr_t limit = round_pgup(fdt_paddr + sizeof(struct fdt_header));
> + unsigned int flags = PAGE_HYPERVISOR_RO;
> + void *fdt_virt = (void *)fdt_paddr; /* virt == paddr for MPU */
> + int rc;
> + unsigned long nr_mfns;
> +
> + /*
> + * Check whether the physical FDT address is set and meets the minimum
> + * alignment requirement. Since we are relying on MIN_FDT_ALIGN to be at
> + * least 8 bytes so that we always access the magic and size fields
> + * of the FDT header after mapping the first chunk, double check if
> + * that is indeed the case.
> + */
> + BUILD_BUG_ON(MIN_FDT_ALIGN < 8);
> + if ( !fdt_paddr || fdt_paddr % MIN_FDT_ALIGN )
> + return NULL;
> +
> + /* DTB starting at this address has already been mapped. */
When can this happen?
> + if ( mapped_fdt_paddr == fdt_paddr )
> + return fdt_virt;
> +
> + /*
> + * DTB starting at a different address has been mapped, so destroy this
> + * before continuing.
> + */
> + if ( mapped_fdt_paddr != INVALID_PADDR )
> + {
> + rc = destroy_xen_mappings(round_pgdown(mapped_fdt_paddr),
> + mapped_fdt_limit);
> + if ( rc )
> + panic("Unable to unmap existing device-tree.\n");
NIT: no need for a dot at the end. It only takes space and is really not needed.
> + }
> +
> + nr_mfns = (limit - base) >> PAGE_SHIFT;
> +
> + rc = map_pages_to_xen(base, maddr_to_mfn(base), nr_mfns, flags);
> + if ( rc )
> + panic("Unable to map the device-tree.\n");
> +
> + mapped_fdt_paddr = fdt_paddr;
> + mapped_fdt_limit = limit;
> +
> + if ( fdt_magic(fdt_virt) != FDT_MAGIC )
> + return NULL;
> +
> + limit = round_pgup(fdt_paddr + fdt_totalsize(fdt_virt));
You're missing a sanity check for MAX_FDT_SIZE
> +
> + /* If the mapped range is not enough, map the rest of the DTB. */
> + if ( limit > mapped_fdt_limit )
> + {
> + rc = destroy_xen_mappings(base, mapped_fdt_limit);
> + if ( rc )
> + panic("Unable to unmap the device-tree header.\n");
> +
> + nr_mfns = (limit - base) >> PAGE_SHIFT;
> +
> + rc = map_pages_to_xen(base, maddr_to_mfn(base), nr_mfns, flags);
> + if ( rc )
> + panic("Unable to map the device-tree.\n");
> +
> + mapped_fdt_limit = limit;
> + }
> +
> + return fdt_virt;
> }
>
> /*
~Michal
