Re: [PATCH 6/8] xen/arm: dump debug message in MPU system

["Halder, Ayan Kumar" <ayankuma@xxxxxxx>]

Hi Luca,

On 20/04/2026 15:25, Luca Fancellu wrote:
> From: Penny Zheng <Penny.Zheng@xxxxxxx>
>
> A set of helpers dump_xxx and show_registers are responsible for
> dumping memory mapping info and register info when debugging.
> In this commit, we implement them all in MPU system too.
>
> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
>   xen/arch/arm/mpu/mm.c  | 12 +++++++++++-
>   xen/arch/arm/mpu/p2m.c | 40 +++++++++++++++++++++++++++++++++++++++-
>   xen/arch/arm/traps.c   |  8 ++++++++
>   3 files changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 5ed77355a5f9..459fb952d125 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -554,7 +554,17 @@ int modify_xen_mappings(unsigned long s, unsigned long e, 
> unsigned int nf)
>   
>   void dump_hyp_walk(vaddr_t addr)
>   {
> -    BUG_ON("unimplemented");
> +    uint8_t i = 0;
> +    pr_t region;
> +
> +    for ( i = 0; i < max_mpu_regions; i++ )
> +    {
> +        read_protection_region(&region, i);
> +        if ( region_is_valid(&region) )
> +            printk(XENLOG_INFO
> +                   "Walking hypervisor MPU memory region [%u]: 
> 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
> +                   i, pr_get_base(&region), pr_get_limit(&region));
> +    }
>   }
>   
>   /* Release all __init and __initdata ranges to be reused */
> diff --git a/xen/arch/arm/mpu/p2m.c b/xen/arch/arm/mpu/p2m.c
> index bf87c65c106c..06c92a3ef41b 100644
> --- a/xen/arch/arm/mpu/p2m.c
> +++ b/xen/arch/arm/mpu/p2m.c
> @@ -215,9 +215,47 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
>       return p2m_get_mpu_region(p2m, gfn, 1, t, valid);
>   }
>   
> +static void dump_mpu_walk(pr_t *table, uint8_t nr_regions)
> +{
> +    uint8_t i = 0;
> +
> +    for ( ; i < nr_regions; i++ )
NIT: for ( uint8_t i = 0; ... ).
> +    {
> +        paddr_t base, limit;
> +
> +        if ( region_is_valid(&table[i]) )
> +        {
> +            base = pr_get_base(&table[i]);
> +            limit = pr_get_limit(&table[i]);
> +
> +            printk(XENLOG_INFO
> +                   "Walking MPU memory mapping table: Region[%u]: 
> 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
> +                   i, base, limit);
> +        }
> +    }
> +}
> +
> +void dump_p2m_lookup(struct domain *d, paddr_t addr)
> +{
> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +
> +    printk("dom%d IPA 0x%"PRIpaddr"\n", d->domain_id, addr);
Shouldn't we take a lock here like we do for the next function ?

p2m_read_lock(p2m);

> +
> +    printk("P2M @ %p mfn:%#"PRI_mfn"\n",
> +           p2m->root, mfn_x(page_to_mfn(p2m->root)));
> +
> +    dump_mpu_walk((pr_t *)page_to_virt(p2m->root), p2m->nr_regions);
> +}
> +
>   void p2m_dump_info(struct domain *d)
>   {
> -    BUG_ON("unimplemented");
> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +
> +    p2m_read_lock(p2m);
> +    printk("p2m mappings for domain %d (vmid %d):\n",
> +           d->domain_id, p2m->vmid);
> +    printk("  Number of P2M Memory Region: %u \n", p2m->nr_regions);
Trailing whitespace before the newline.
> +    p2m_read_unlock(p2m);
>   }
>   
>   static int p2m_alloc_table(struct domain *d)
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 081bd2e51979..fba7d6c00e37 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -718,6 +718,8 @@ struct reg_ctxt {
>   #ifdef CONFIG_MMU
>       /* Hypervisor-side state */
>       uint64_t vttbr_el2;
> +#else
> +    uint64_t vsctlr_el2;
>   #endif
>   };
>   
> @@ -918,6 +920,8 @@ static void _show_registers(const struct cpu_user_regs *regs,
>       printk("  VTCR_EL2: %"PRIregister"\n", READ_SYSREG(VTCR_EL2));
>   #ifdef CONFIG_MMU
>       printk(" VTTBR_EL2: %016"PRIx64"\n", ctxt->vttbr_el2);
> +#else
> +    printk(" VSCTLR_EL2: %016"PRIx64"\n", ctxt->vsctlr_el2);
>   #endif
>       printk("\n");
>   
> @@ -959,6 +963,8 @@ void show_registers(const struct cpu_user_regs *regs)
>   #endif
>   #ifdef CONFIG_MMU
>       ctxt.vttbr_el2 = READ_SYSREG64(VTTBR_EL2);
> +#else
> +    ctxt.vsctlr_el2 = READ_SYSREG(VSCTLR_EL2);
>   #endif
>   
>       _show_registers(regs, &ctxt, guest_mode(regs), current);
> @@ -985,6 +991,8 @@ void vcpu_show_registers(struct vcpu *v)
>   
>   #ifdef CONFIG_MMU
>       ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr;
> +#else
> +    ctxt.vsctlr_el2 = v->domain->arch.p2m.vsctlr;
>   #endif
>   
>       _show_registers(&v->arch.cpu_info->guest_cpu_user_regs, &ctxt, 1, v);
- Ayan