Re: [PATCH v3 1/5] xen/arm: Add way to disable traps on accesses to unmapped addresses

["Edgar E. Iglesias" <edgar.iglesias@xxxxxxx>]

On Tue, Jun 03, 2025 at 10:36:40AM +0100, Julien Grall wrote:
> Hi Edgar,

Hi Julien,

> 
> On 30/05/2025 14:45, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxx>
> > 
> > Add a per-domain way to optionally disable traps for accesses
> > to unmapped addresses.
> > 
> > The domain flag is general but it's only implemented for ARM for now.
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxx>
> > ---
> >   tools/libs/light/libxl_arm.c  |  3 +++
> >   xen/arch/arm/dom0less-build.c |  3 +++
> >   xen/arch/arm/domain.c         |  3 ++-
> >   xen/arch/arm/domain_build.c   |  3 ++-
> >   xen/arch/arm/io.c             | 37 +++++++++++++++++++++++++++++++++--
> >   xen/common/domain.c           |  3 ++-
> >   xen/include/public/domctl.h   |  4 +++-
> >   7 files changed, 50 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> > index 75c811053c..9530996e72 100644
> > --- a/tools/libs/light/libxl_arm.c
> > +++ b/tools/libs/light/libxl_arm.c
> > @@ -233,6 +233,9 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> >           config->arch.sve_vl = d_config->b_info.arch_arm.sve_vl / 128U;
> >       }
> > +    /* Trap accesses to unmapped areas. */
> > +    config->flags |= XEN_DOMCTL_CDF_trap_unmapped_accesses;
> > +
> >       return 0;
> >   }
> > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> > index a49764f0ad..a4e0a33632 100644
> > --- a/xen/arch/arm/dom0less-build.c
> > +++ b/xen/arch/arm/dom0less-build.c
> > @@ -343,6 +343,9 @@ void __init arch_create_domUs(struct dt_device_node 
> > *node,
> >           panic("'sve' property found, but CONFIG_ARM64_SVE not 
> > selected\n");
> >   #endif
> >       }
> > +
> > +    /* Trap accesses to unmapped areas. */
> > +    d_cfg->flags |= XEN_DOMCTL_CDF_trap_unmapped_accesses;
> >   }
> >   int __init init_intc_phandle(struct kernel_info *kinfo, const char *name,
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index 45aeb8bddc..be58a23dd7 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -612,7 +612,8 @@ int arch_sanitise_domain_config(struct 
> > xen_domctl_createdomain *config)
> >       unsigned int max_vcpus;
> >       unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | 
> > XEN_DOMCTL_CDF_hap);
> >       unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | 
> > XEN_DOMCTL_CDF_vpmu |
> > -                                   XEN_DOMCTL_CDF_xs_domain );
> > +                                   XEN_DOMCTL_CDF_xs_domain |
> > +                                   XEN_DOMCTL_CDF_trap_unmapped_accesses );
> 
> Just to double check, doesn't this mean the flag will be allowed on x86? If
> so, shouldn't we reject it in an arch?

Yes, I had initially thought I could block the flag for x86 in xl but I
didn't consider go/ocaml bindings nor Xen internal missconfig. In v4,
I'm adding a check in x86's arch_sanitise_domain_config().

Cheers,
Edgar


> 
> Cheers,
> 
> -- 
> Julien Grall
>