All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio
@ 2025-05-27 19:56 Edgar E. Iglesias
  2025-05-27 19:56 ` [PATCH v1 1/3] xen/arm: Add a way to disable traps on unmapped MMIO Edgar E. Iglesias
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Edgar E. Iglesias @ 2025-05-27 19:56 UTC (permalink / raw)
  To: xen-devel
  Cc: sstabellini, julien, bertrand.marquis, michal.orzel,
	Volodymyr_Babchuk, edgar.iglesias

From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

This follows up on the virtio-pci discussion and adds a per-domain
option to select the behaviour of accesses to unmapped mmio ranges.
The new option is trap-unmapped-mmio. For dom0less I negated it to
be able to use a boolean prop and keep existing behaviour, i.e
trap-unmapped-mmio-disabled.

I'm happy with any name though so if you have better ideas please
suggest them!

For the domain config i followed the example of x86 flags
and XEN_X86_MSR_RELXED, creating a flags field for ARM.

Thanks,
Edgar

Edgar E. Iglesias (3):
  xen/arm: Add a way to disable traps on unmapped MMIO
  xen/arm: dom0less: Add trap-unmapped-mmio-disabled
  tools/arm: Add the trap_unmapped_mmio xl config option

 docs/man/xl.cfg.5.pod.in              |  6 +++++
 docs/misc/arm/device-tree/booting.txt |  7 ++++++
 tools/golang/xenlight/helpers.gen.go  |  3 ++-
 tools/golang/xenlight/types.gen.go    |  1 +
 tools/libs/light/libxl_arm.c          |  7 ++++++
 tools/libs/light/libxl_types.idl      |  1 +
 tools/xl/xl_parse.c                   |  3 +++
 xen/arch/arm/dom0less-build.c         |  4 ++++
 xen/arch/arm/domain.c                 |  2 ++
 xen/arch/arm/domain_build.c           |  3 +++
 xen/arch/arm/include/asm/domain.h     |  2 ++
 xen/arch/arm/io.c                     | 33 +++++++++++++++++++++++++--
 xen/include/public/arch-arm.h         |  9 ++++++++
 13 files changed, 78 insertions(+), 3 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 1/3] xen/arm: Add a way to disable traps on unmapped MMIO
  2025-05-27 19:56 [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio Edgar E. Iglesias
@ 2025-05-27 19:56 ` Edgar E. Iglesias
  2025-05-29  0:34   ` Stefano Stabellini
  2025-05-27 19:56 ` [PATCH v1 2/3] xen/arm: dom0less: Add trap-unmapped-mmio-disabled Edgar E. Iglesias
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Edgar E. Iglesias @ 2025-05-27 19:56 UTC (permalink / raw)
  To: xen-devel
  Cc: sstabellini, julien, bertrand.marquis, michal.orzel,
	Volodymyr_Babchuk, edgar.iglesias, Anthony PERARD, Juergen Gross

From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

Add a per-domain way to optionally disable traps on unmapped MMIO.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 tools/libs/light/libxl_arm.c      |  3 +++
 xen/arch/arm/dom0less-build.c     |  3 +++
 xen/arch/arm/domain.c             |  2 ++
 xen/arch/arm/domain_build.c       |  3 +++
 xen/arch/arm/include/asm/domain.h |  2 ++
 xen/arch/arm/io.c                 | 33 +++++++++++++++++++++++++++++--
 xen/include/public/arch-arm.h     |  9 +++++++++
 7 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 75c811053c..40cd005619 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 MMIO. */
+    config->arch.flags = XEN_ARM_TRAP_UNMAPPED_MMIO;
+
     return 0;
 }
 
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index a49764f0ad..e5e13e07d0 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 MMIO. */
+    d_cfg->arch.flags = XEN_ARM_TRAP_UNMAPPED_MMIO;
 }
 
 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..54c6ae7678 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -714,6 +714,8 @@ int arch_domain_create(struct domain *d,
     ioreq_domain_init(d);
 #endif
 
+    d->arch.trap_unmapped_mmio = config->arch.flags & XEN_ARM_TRAP_UNMAPPED_MMIO;
+
     /* p2m_init relies on some value initialized by the IOMMU subsystem */
     if ( (rc = iommu_domain_init(d, config->iommu_opts)) != 0 )
         goto fail;
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index b189a7cfae..c3c8212260 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2018,6 +2018,9 @@ void __init create_dom0(void)
     dom0_cfg.arch.tee_type = tee_get_type();
     dom0_cfg.max_vcpus = dom0_max_vcpus();
 
+    /* Dom0 always traps on unmapped MMIO.  */
+    dom0_cfg.arch.flags |= XEN_ARM_TRAP_UNMAPPED_MMIO;
+
     if ( iommu_enabled )
         dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
 
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index a3487ca713..4d1a180ce2 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -121,6 +121,8 @@ struct arch_domain
     void *tee;
 #endif
 
+    bool trap_unmapped_mmio;
+
 }  __cacheline_aligned;
 
 struct arch_vcpu
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index 5a4b0e8f25..11ffa48969 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -21,6 +21,32 @@
 
 #include "decode.h"
 
+/* Handler for unmapped ranges. Writes ignored, reads return all ones.  */
+static int unmapped_read(struct vcpu *v, mmio_info_t *info, register_t *r,
+                         void *priv)
+{
+    uint64_t mask = GENMASK_ULL((1U << info->dabt.size) * 8 - 1, 0);
+
+    /* Mask off upper bits.  */
+    *r = UINT64_MAX & mask;
+    return 1;
+}
+
+static int unmapped_write(struct vcpu *v, mmio_info_t *info, register_t r,
+                          void *priv)
+{
+    return 1;
+}
+
+static const struct mmio_handler_ops unmapped_ops = {
+    .read = unmapped_read,
+    .write = unmapped_write
+};
+
+static const struct mmio_handler unmapped_handler = {
+    .ops = &unmapped_ops
+};
+
 static enum io_state handle_read(const struct mmio_handler *handler,
                                  struct vcpu *v,
                                  mmio_info_t *info)
@@ -178,8 +204,11 @@ enum io_state try_handle_mmio(struct cpu_user_regs *regs,
         rc = try_fwd_ioserv(regs, v, info);
         if ( rc == IO_HANDLED )
             return handle_ioserv(regs, v);
-
-        return rc;
+        else if ( rc == IO_UNHANDLED && !v->domain->arch.trap_unmapped_mmio ) {
+            /* Fallback to the unmapped handler. */
+            handler = &unmapped_handler;
+        } else
+            return rc;
     }
 
     /*
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index e2412a1747..32b023504d 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -350,6 +350,15 @@ struct xen_arch_domainconfig {
      *
      */
     uint32_t clock_frequency;
+    /*
+     * IN
+     *
+     * XEN_ARM_TRAP_UNMAPPED_MMIO enables trapping of memory accesses
+     * into unmapped ranges. When disabled, Xen will handle the access
+     * by reading 0xFFFFFFFF and ignoring writes.
+     */
+#define XEN_ARM_TRAP_UNMAPPED_MMIO (1U << 0)
+    uint32_t flags;
 };
 #endif /* __XEN__ || __XEN_TOOLS__ */
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 2/3] xen/arm: dom0less: Add trap-unmapped-mmio-disabled
  2025-05-27 19:56 [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio Edgar E. Iglesias
  2025-05-27 19:56 ` [PATCH v1 1/3] xen/arm: Add a way to disable traps on unmapped MMIO Edgar E. Iglesias
@ 2025-05-27 19:56 ` Edgar E. Iglesias
  2025-05-29  0:41   ` Stefano Stabellini
  2025-05-27 19:56 ` [PATCH v1 3/3] tools/arm: Add the trap_unmapped_mmio xl config option Edgar E. Iglesias
  2025-05-27 20:03 ` [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio Andrew Cooper
  3 siblings, 1 reply; 9+ messages in thread
From: Edgar E. Iglesias @ 2025-05-27 19:56 UTC (permalink / raw)
  To: xen-devel
  Cc: sstabellini, julien, bertrand.marquis, michal.orzel,
	Volodymyr_Babchuk, edgar.iglesias

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.
+
 - 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



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 3/3] tools/arm: Add the trap_unmapped_mmio xl config option
  2025-05-27 19:56 [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio Edgar E. Iglesias
  2025-05-27 19:56 ` [PATCH v1 1/3] xen/arm: Add a way to disable traps on unmapped MMIO Edgar E. Iglesias
  2025-05-27 19:56 ` [PATCH v1 2/3] xen/arm: dom0less: Add trap-unmapped-mmio-disabled Edgar E. Iglesias
@ 2025-05-27 19:56 ` Edgar E. Iglesias
  2025-05-27 20:03 ` [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio Andrew Cooper
  3 siblings, 0 replies; 9+ messages in thread
From: Edgar E. Iglesias @ 2025-05-27 19:56 UTC (permalink / raw)
  To: xen-devel
  Cc: sstabellini, julien, bertrand.marquis, michal.orzel,
	Volodymyr_Babchuk, edgar.iglesias, Anthony PERARD, Nick Rosbrook,
	George Dunlap, Juergen Gross

From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 docs/man/xl.cfg.5.pod.in             | 6 ++++++
 tools/golang/xenlight/helpers.gen.go | 3 ++-
 tools/golang/xenlight/types.gen.go   | 1 +
 tools/libs/light/libxl_arm.c         | 8 ++++++--
 tools/libs/light/libxl_types.idl     | 1 +
 tools/xl/xl_parse.c                  | 3 +++
 6 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 7339c44efd..6dd0a05482 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -3089,6 +3089,12 @@ will be used for the domain. Otherwise, the value specified by the `nr_spis`
 parameter will be used. The number of SPIs should match the highest interrupt
 ID that will be assigned to the domain.
 
+=item B<trap_unmapped_mmio=BOOLEAN>
+
+An Optional boolean parameter that configures handling of accesses to unmapped
+MMIO ranges. If enabled, guest accesses will trap. If disabled, guest accesses
+will read 0xFFFFFFFF and writes ignored.
+
 =back
 
 =head3 x86
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index 90846ea8e8..b04561929c 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1163,6 +1163,7 @@ x.ArchArm.GicVersion = GicVersion(xc.arch_arm.gic_version)
 x.ArchArm.Vuart = VuartType(xc.arch_arm.vuart)
 x.ArchArm.SveVl = SveType(xc.arch_arm.sve_vl)
 x.ArchArm.NrSpis = uint32(xc.arch_arm.nr_spis)
+x.ArchArm.TrapUnmappedMmio = uint32(xc.arch_arm.trap_unmapped_mmio)
 if err := x.ArchX86.MsrRelaxed.fromC(&xc.arch_x86.msr_relaxed);err != nil {
 return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
 }
@@ -1687,7 +1688,7 @@ return fmt.Errorf("invalid union key '%v'", x.Type)}
 xc.arch_arm.gic_version = C.libxl_gic_version(x.ArchArm.GicVersion)
 xc.arch_arm.vuart = C.libxl_vuart_type(x.ArchArm.Vuart)
 xc.arch_arm.sve_vl = C.libxl_sve_type(x.ArchArm.SveVl)
-xc.arch_arm.nr_spis = C.uint32_t(x.ArchArm.NrSpis)
+xc.arch_arm.trap_unmapped_mmio = C.uint32_t(x.ArchArm.TrapUnmappedMmio)
 if err := x.ArchX86.MsrRelaxed.toC(&xc.arch_x86.msr_relaxed); err != nil {
 return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
 }
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index e7667f1ce3..89cc976bdc 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -599,6 +599,7 @@ GicVersion GicVersion
 Vuart VuartType
 SveVl SveType
 NrSpis uint32
+TrapUnmappedMmio Defbool
 }
 ArchX86 struct {
 MsrRelaxed Defbool
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 40cd005619..cce3fc4684 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -233,8 +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 MMIO. */
-    config->arch.flags = XEN_ARM_TRAP_UNMAPPED_MMIO;
+    config->arch.flags = 0;
+    if (libxl_defbool_val(d_config->b_info.arch_arm.trap_unmapped_mmio))
+        config->arch.flags |= XEN_ARM_TRAP_UNMAPPED_MMIO;
 
     return 0;
 }
@@ -1714,6 +1715,9 @@ int libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
     /* ACPI is disabled by default */
     libxl_defbool_setdefault(&b_info->acpi, false);
 
+    /* Trapping of unmapped MMIO access enabled by default.  */
+    libxl_defbool_setdefault(&b_info->arch_arm.trap_unmapped_mmio, true);
+
     /* Sanitise SVE parameter */
     if (b_info->arch_arm.sve_vl) {
         unsigned int max_sve_vl =
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 9bb2969931..bd5425fe50 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -724,6 +724,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                ("vuart", libxl_vuart_type),
                                ("sve_vl", libxl_sve_type),
                                ("nr_spis", uint32, {'init_val': 'LIBXL_NR_SPIS_DEFAULT'}),
+                               ("trap_unmapped_mmio", libxl_defbool),
                               ])),
     ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
                               ])),
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 089a88935a..3099086198 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2975,6 +2975,9 @@ skip_usbdev:
     if (!xlu_cfg_get_long (config, "nr_spis", &l, 0))
         b_info->arch_arm.nr_spis = l;
 
+    xlu_cfg_get_defbool(config, "trap_unmapped_mmio",
+                        &b_info->arch_arm.trap_unmapped_mmio, 0);
+
     parse_vkb_list(config, d_config);
 
     d_config->virtios = NULL;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio
  2025-05-27 19:56 [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio Edgar E. Iglesias
                   ` (2 preceding siblings ...)
  2025-05-27 19:56 ` [PATCH v1 3/3] tools/arm: Add the trap_unmapped_mmio xl config option Edgar E. Iglesias
@ 2025-05-27 20:03 ` Andrew Cooper
  2025-05-28 20:38   ` Edgar E. Iglesias
  3 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2025-05-27 20:03 UTC (permalink / raw)
  To: Edgar E. Iglesias, xen-devel
  Cc: sstabellini, julien, bertrand.marquis, michal.orzel,
	Volodymyr_Babchuk, edgar.iglesias

On 27/05/2025 8:56 pm, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
>
> This follows up on the virtio-pci discussion and adds a per-domain
> option to select the behaviour of accesses to unmapped mmio ranges.
> The new option is trap-unmapped-mmio. For dom0less I negated it to
> be able to use a boolean prop and keep existing behaviour, i.e
> trap-unmapped-mmio-disabled.
>
> I'm happy with any name though so if you have better ideas please
> suggest them!
>
> For the domain config i followed the example of x86 flags
> and XEN_X86_MSR_RELXED, creating a flags field for ARM.
>
> Thanks,
> Edgar

I think this should be common, rather than ARM specific.

Traditionally on x86, access to unimplemented address space was ignored
(write discard, read ~0), but these days you do get a machine check on
certain ranges, which is for all intents and purposes the same as a data
abort.

So even if x86 requires it to be false in the short term, I think the
control ought to be common, so x86 and others can opt in at a later point.

I don't have a good suggestion for the name, but it's not really about
MMIO space; it's about address space generally.

~Andrew


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio
  2025-05-27 20:03 ` [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio Andrew Cooper
@ 2025-05-28 20:38   ` Edgar E. Iglesias
  0 siblings, 0 replies; 9+ messages in thread
From: Edgar E. Iglesias @ 2025-05-28 20:38 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: xen-devel, sstabellini, julien, bertrand.marquis, michal.orzel,
	Volodymyr_Babchuk, edgar.iglesias

On Tue, May 27, 2025 at 09:03:11PM +0100, Andrew Cooper wrote:
> On 27/05/2025 8:56 pm, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> >
> > This follows up on the virtio-pci discussion and adds a per-domain
> > option to select the behaviour of accesses to unmapped mmio ranges.
> > The new option is trap-unmapped-mmio. For dom0less I negated it to
> > be able to use a boolean prop and keep existing behaviour, i.e
> > trap-unmapped-mmio-disabled.
> >
> > I'm happy with any name though so if you have better ideas please
> > suggest them!
> >
> > For the domain config i followed the example of x86 flags
> > and XEN_X86_MSR_RELXED, creating a flags field for ARM.
> >
> > Thanks,
> > Edgar
> 
> I think this should be common, rather than ARM specific.
> 
> Traditionally on x86, access to unimplemented address space was ignored
> (write discard, read ~0), but these days you do get a machine check on
> certain ranges, which is for all intents and purposes the same as a data
> abort.
> 
> So even if x86 requires it to be false in the short term, I think the
> control ought to be common, so x86 and others can opt in at a later point.

We can do that, we'd need to have different default values though since
x86 wants default false and ARM default true.

> 
> I don't have a good suggestion for the name, but it's not really about
> MMIO space; it's about address space generally.

What about trap-unmapped-access ?

Cheers,
Edgar


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 1/3] xen/arm: Add a way to disable traps on unmapped MMIO
  2025-05-27 19:56 ` [PATCH v1 1/3] xen/arm: Add a way to disable traps on unmapped MMIO Edgar E. Iglesias
@ 2025-05-29  0:34   ` Stefano Stabellini
  0 siblings, 0 replies; 9+ messages in thread
From: Stefano Stabellini @ 2025-05-29  0:34 UTC (permalink / raw)
  To: Edgar E. Iglesias
  Cc: xen-devel, sstabellini, julien, bertrand.marquis, michal.orzel,
	Volodymyr_Babchuk, edgar.iglesias, Anthony PERARD, Juergen Gross

On Tue, 27 May 2025, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> 
> Add a per-domain way to optionally disable traps on unmapped MMIO.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>

The ARM changes look OK; I'll ack the next version when the option
becomes arch common as Andrew suggested


> ---
>  tools/libs/light/libxl_arm.c      |  3 +++
>  xen/arch/arm/dom0less-build.c     |  3 +++
>  xen/arch/arm/domain.c             |  2 ++
>  xen/arch/arm/domain_build.c       |  3 +++
>  xen/arch/arm/include/asm/domain.h |  2 ++
>  xen/arch/arm/io.c                 | 33 +++++++++++++++++++++++++++++--
>  xen/include/public/arch-arm.h     |  9 +++++++++
>  7 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 75c811053c..40cd005619 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 MMIO. */
> +    config->arch.flags = XEN_ARM_TRAP_UNMAPPED_MMIO;
> +
>      return 0;
>  }
>  
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index a49764f0ad..e5e13e07d0 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 MMIO. */
> +    d_cfg->arch.flags = XEN_ARM_TRAP_UNMAPPED_MMIO;
>  }
>  
>  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..54c6ae7678 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -714,6 +714,8 @@ int arch_domain_create(struct domain *d,
>      ioreq_domain_init(d);
>  #endif
>  
> +    d->arch.trap_unmapped_mmio = config->arch.flags & XEN_ARM_TRAP_UNMAPPED_MMIO;
> +
>      /* p2m_init relies on some value initialized by the IOMMU subsystem */
>      if ( (rc = iommu_domain_init(d, config->iommu_opts)) != 0 )
>          goto fail;
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index b189a7cfae..c3c8212260 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2018,6 +2018,9 @@ void __init create_dom0(void)
>      dom0_cfg.arch.tee_type = tee_get_type();
>      dom0_cfg.max_vcpus = dom0_max_vcpus();
>  
> +    /* Dom0 always traps on unmapped MMIO.  */
> +    dom0_cfg.arch.flags |= XEN_ARM_TRAP_UNMAPPED_MMIO;
> +
>      if ( iommu_enabled )
>          dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>  
> diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
> index a3487ca713..4d1a180ce2 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -121,6 +121,8 @@ struct arch_domain
>      void *tee;
>  #endif
>  
> +    bool trap_unmapped_mmio;
> +
>  }  __cacheline_aligned;
>  
>  struct arch_vcpu
> diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
> index 5a4b0e8f25..11ffa48969 100644
> --- a/xen/arch/arm/io.c
> +++ b/xen/arch/arm/io.c
> @@ -21,6 +21,32 @@
>  
>  #include "decode.h"
>  
> +/* Handler for unmapped ranges. Writes ignored, reads return all ones.  */
> +static int unmapped_read(struct vcpu *v, mmio_info_t *info, register_t *r,
> +                         void *priv)
> +{
> +    uint64_t mask = GENMASK_ULL((1U << info->dabt.size) * 8 - 1, 0);
> +
> +    /* Mask off upper bits.  */
> +    *r = UINT64_MAX & mask;
> +    return 1;
> +}
> +
> +static int unmapped_write(struct vcpu *v, mmio_info_t *info, register_t r,
> +                          void *priv)
> +{
> +    return 1;
> +}
> +
> +static const struct mmio_handler_ops unmapped_ops = {
> +    .read = unmapped_read,
> +    .write = unmapped_write
> +};
> +
> +static const struct mmio_handler unmapped_handler = {
> +    .ops = &unmapped_ops
> +};
> +
>  static enum io_state handle_read(const struct mmio_handler *handler,
>                                   struct vcpu *v,
>                                   mmio_info_t *info)
> @@ -178,8 +204,11 @@ enum io_state try_handle_mmio(struct cpu_user_regs *regs,
>          rc = try_fwd_ioserv(regs, v, info);
>          if ( rc == IO_HANDLED )
>              return handle_ioserv(regs, v);
> -
> -        return rc;
> +        else if ( rc == IO_UNHANDLED && !v->domain->arch.trap_unmapped_mmio ) {
> +            /* Fallback to the unmapped handler. */
> +            handler = &unmapped_handler;
> +        } else
> +            return rc;
>      }
>  
>      /*
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index e2412a1747..32b023504d 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -350,6 +350,15 @@ struct xen_arch_domainconfig {
>       *
>       */
>      uint32_t clock_frequency;
> +    /*
> +     * IN
> +     *
> +     * XEN_ARM_TRAP_UNMAPPED_MMIO enables trapping of memory accesses
> +     * into unmapped ranges. When disabled, Xen will handle the access
> +     * by reading 0xFFFFFFFF and ignoring writes.
> +     */
> +#define XEN_ARM_TRAP_UNMAPPED_MMIO (1U << 0)
> +    uint32_t flags;
>  };
>  #endif /* __XEN__ || __XEN_TOOLS__ */
>  
> -- 
> 2.43.0
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/3] xen/arm: dom0less: Add trap-unmapped-mmio-disabled
  2025-05-27 19:56 ` [PATCH v1 2/3] xen/arm: dom0less: Add trap-unmapped-mmio-disabled Edgar E. Iglesias
@ 2025-05-29  0:41   ` Stefano Stabellini
  2025-05-29 15:30     ` Edgar E. Iglesias
  0 siblings, 1 reply; 9+ messages in thread
From: Stefano Stabellini @ 2025-05-29  0:41 UTC (permalink / raw)
  To: Edgar E. Iglesias
  Cc: xen-devel, sstabellini, julien, bertrand.marquis, michal.orzel,
	Volodymyr_Babchuk, edgar.iglesias

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.


>  - 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
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/3] xen/arm: dom0less: Add trap-unmapped-mmio-disabled
  2025-05-29  0:41   ` Stefano Stabellini
@ 2025-05-29 15:30     ` Edgar E. Iglesias
  0 siblings, 0 replies; 9+ messages in thread
From: Edgar E. Iglesias @ 2025-05-29 15:30 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, julien, bertrand.marquis, michal.orzel,
	Volodymyr_Babchuk, edgar.iglesias

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
> > 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-05-29 15:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-27 19:56 [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio Edgar E. Iglesias
2025-05-27 19:56 ` [PATCH v1 1/3] xen/arm: Add a way to disable traps on unmapped MMIO Edgar E. Iglesias
2025-05-29  0:34   ` Stefano Stabellini
2025-05-27 19:56 ` [PATCH v1 2/3] xen/arm: dom0less: Add trap-unmapped-mmio-disabled Edgar E. Iglesias
2025-05-29  0:41   ` Stefano Stabellini
2025-05-29 15:30     ` Edgar E. Iglesias
2025-05-27 19:56 ` [PATCH v1 3/3] tools/arm: Add the trap_unmapped_mmio xl config option Edgar E. Iglesias
2025-05-27 20:03 ` [PATCH v1 0/3] xen/arm: Add option to optionally disable trapping on unmapped mmio Andrew Cooper
2025-05-28 20:38   ` Edgar E. Iglesias

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.