Re: [PATCH v1 2/3] xen/arm: dom0less: Add trap-unmapped-mmio-disabled

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

On Wed, May 28, 2025 at 05:41:34PM -0700, Stefano Stabellini wrote:
> On Tue, 27 May 2025, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> > 
> > Add the trap-unmapped-mmio-disabled per-domain fdt property.
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > ---
> >  docs/misc/arm/device-tree/booting.txt | 7 +++++++
> >  xen/arch/arm/dom0less-build.c         | 3 ++-
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
> > index 59fa96a82e..75fbb245d1 100644
> > --- a/docs/misc/arm/device-tree/booting.txt
> > +++ b/docs/misc/arm/device-tree/booting.txt
> > @@ -225,6 +225,13 @@ with the following properties:
> >      option is provided with a non zero value, but the platform doesn't support
> >      SVE.
> >  
> > +- trap-unmapped-mmio-disabled
> > +
> > +    Optional. A boolean property that configures handling of accesses to
> > +    unmapped MMIO ranges.
> > +    If set, guest accesses will read 0xFFFFFFFF and writes ignored.
> > +    If not set, guest accesses will trap.
> 
> I would prefer that we are consistent with the name of the parameter we
> use in libxl and elsewhere so I would stick with trap-unmapped-mmio
> without -disabled.
> 
> We can still default the property to "enabled" when absent. Although
> this is not a common pattern for device tree, it happens and for
> instance the property "status" works that way as it is implied to be
> "enabled" when absent.


Sounds good Stefano,

Boolean DT props have no values so we can't have a default of true since
there wouldn't be a way of setting it to false.
But we can make trap-unmapped-acceses an integer. E.g:

trap-unmapped-acceses = <0>; // Disabled
trap-unmapped-acceses = <1>; // Enabled
// trap-unmapped-acceses not present defaults to Enabled.

I've done this latter for v2, avoiding the -disable suffix.

Cheers,
Edgar


> 
> 
> >  - xen,enhanced
> >  
> >      A string property. Possible property values are:
> > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> > index e5e13e07d0..cd1ef05d89 100644
> > --- a/xen/arch/arm/dom0less-build.c
> > +++ b/xen/arch/arm/dom0less-build.c
> > @@ -344,8 +344,9 @@ void __init arch_create_domUs(struct dt_device_node *node,
> >  #endif
> >      }
> >  
> > -    /* Trap accesses to unmapped MMIO. */
> >      d_cfg->arch.flags = XEN_ARM_TRAP_UNMAPPED_MMIO;
> > +    if ( dt_property_read_bool(node, "trap-unmapped-mmio-disabled") )
> > +        d_cfg->arch.flags &= ~XEN_ARM_TRAP_UNMAPPED_MMIO;
> >  }
> >  
> >  int __init init_intc_phandle(struct kernel_info *kinfo, const char *name,
> > -- 
> > 2.43.0
> >