[PATCH v2 0/3] xen/arm: Add option to optionally disable trapping on unmapped acceses

[PATCH v2 0/3] xen/arm: Add option to optionally disable trapping on unmapped acceses

[Edgar E. 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 address ranges.
The new option is trap_unmapped_accesses, it's general but for now
only implemented for ARM.

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

Thanks,
Edgar

ChangeLog:

v1 -> v2:
* Rename trap_unmapped_mmio to trap_unmapped_accesses
* Generalize to allow other archs to later support this option
* Change dom0less DT binding from boolean to integer
* Remove changes to autogenerated go bindings

Edgar E. Iglesias (3):
  xen/arm: Add way to disable traps on accesses to unmapped addresses
  xen/arm: dom0less: Add trap-unmapped-accesses
  tools/arm: Add the trap_unmapped_accesses xl config option

 docs/man/xl.cfg.5.pod.in              |  8 ++++++
 docs/misc/arm/device-tree/booting.txt |  9 +++++++
 tools/libs/light/libxl_arm.c          |  3 +++
 tools/libs/light/libxl_create.c       |  3 +++
 tools/libs/light/libxl_types.idl      |  1 +
 tools/libs/light/libxl_x86.c          |  6 +++++
 tools/xl/xl_parse.c                   |  3 +++
 xen/arch/arm/dom0less-build.c         | 10 ++++++++
 xen/arch/arm/domain.c                 |  3 ++-
 xen/arch/arm/domain_build.c           |  3 ++-
 xen/arch/arm/io.c                     | 36 +++++++++++++++++++++++++--
 xen/common/domain.c                   |  3 ++-
 xen/include/public/domctl.h           |  4 ++-
 13 files changed, 86 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] xen/arm: Add way to disable traps on accesses to unmapped addresses

[Edgar E. Iglesias]

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

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@amd.com>
---
 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             | 36 +++++++++++++++++++++++++++++++++--
 xen/common/domain.c           |  3 ++-
 xen/include/public/domctl.h   |  4 +++-
 7 files changed, 49 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 );
     unsigned int sve_vl_bits = sve_decode_vl(config->arch.sve_vl);
 
     if ( (config->flags & ~flags_optional) != flags_required )
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index b189a7cfae..7ff9c1b584 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2003,7 +2003,8 @@ void __init create_dom0(void)
 {
     struct domain *dom0;
     struct xen_domctl_createdomain dom0_cfg = {
-        .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
+        .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
+                 XEN_DOMCTL_CDF_trap_unmapped_accesses,
         .max_evtchn_port = -1,
         .max_grant_frames = gnttab_dom0_frames(),
         .max_maptrack_frames = -1,
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index 5a4b0e8f25..adfc822e7e 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)
@@ -175,11 +201,17 @@ enum io_state try_handle_mmio(struct cpu_user_regs *regs,
     handler = find_mmio_handler(v->domain, info->gpa);
     if ( !handler )
     {
+        bool trap_unmapped = v->domain->options &
+                                         XEN_DOMCTL_CDF_trap_unmapped_accesses;
         rc = try_fwd_ioserv(regs, v, info);
         if ( rc == IO_HANDLED )
             return handle_ioserv(regs, v);
-
-        return rc;
+        else if ( rc == IO_UNHANDLED && !trap_unmapped )
+        {
+            /* Fallback to the unmapped handler. */
+            handler = &unmapped_handler;
+        } else
+            return rc;
     }
 
     /*
diff --git a/xen/common/domain.c b/xen/common/domain.c
index abf1969e60..ac4f58f638 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -721,7 +721,8 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
          ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
            XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
            XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
-           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu) )
+           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
+           XEN_DOMCTL_CDF_trap_unmapped_accesses) )
     {
         dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
         return -EINVAL;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 5b2063eed9..be19ab5e26 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -66,9 +66,11 @@ struct xen_domctl_createdomain {
 #define XEN_DOMCTL_CDF_nested_virt    (1U << _XEN_DOMCTL_CDF_nested_virt)
 /* Should we expose the vPMU to the guest? */
 #define XEN_DOMCTL_CDF_vpmu           (1U << 7)
+/* Should we trap guest accesses to unmapped addresses? */
+#define XEN_DOMCTL_CDF_trap_unmapped_accesses  (1U << 8)
 
 /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
-#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpmu
+#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_trap_unmapped_accesses
 
     uint32_t flags;
 
-- 
2.43.0

[PATCH v2 2/3] xen/arm: dom0less: Add trap-unmapped-accesses

[Edgar E. Iglesias]

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

Add the trap-unmapped-accesses per-domain fdt property.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 docs/misc/arm/device-tree/booting.txt | 9 +++++++++
 xen/arch/arm/dom0less-build.c         | 9 ++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
index 59fa96a82e..8a5c40ddf3 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -225,6 +225,15 @@ with the following properties:
     option is provided with a non zero value, but the platform doesn't support
     SVE.
 
+- trap-unmapped-accesses
+
+    Optional. An integer that configures handling of accesses to unmapped
+    address ranges.
+    If set to 0, guest accesses will read 0xFFFFFFFF and writes will be ignored.
+    If set to 1, guest accesses will trap.
+
+    This option is only implemented for ARM where the default is 1.
+
 - 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 a4e0a33632..69324aa597 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -344,8 +344,15 @@ void __init arch_create_domUs(struct dt_device_node *node,
 #endif
     }
 
-    /* Trap accesses to unmapped areas. */
+    /* Trap unmapped accesses by default. */
     d_cfg->flags |= XEN_DOMCTL_CDF_trap_unmapped_accesses;
+    if ( dt_property_read_u32(node, "trap-unmapped-accesses", &val) )
+    {
+        if ( val > 1 )
+            panic("trap-unmapped-accesses: supported values are 0 or 1");
+        if ( !val )
+            d_cfg->flags &= ~XEN_DOMCTL_CDF_trap_unmapped_accesses;
+    }
 }
 
 int __init init_intc_phandle(struct kernel_info *kinfo, const char *name,
-- 
2.43.0

[PATCH v2 3/3] tools/arm: Add the trap_unmapped_accesses xl config option

[Edgar E. Iglesias]

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         | 8 ++++++++
 tools/libs/light/libxl_arm.c     | 6 +++---
 tools/libs/light/libxl_create.c  | 3 +++
 tools/libs/light/libxl_types.idl | 1 +
 tools/libs/light/libxl_x86.c     | 6 ++++++
 tools/xl/xl_parse.c              | 3 +++
 6 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 7339c44efd..55e9cb5bd9 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -3089,6 +3089,14 @@ 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_accesses=BOOLEAN>
+
+An Optional boolean parameter that configures handling of accesses to unmapped
+address ranges. If enabled, guest accesses will trap. If disabled, guest
+accesses will read 0xFFFFFFFF and writes will be ignored.
+
+This option is only implemented for ARM where the default is enabled.
+
 =back
 
 =head3 x86
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 9530996e72..afc62a5299 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -233,9 +233,6 @@ 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;
 }
 
@@ -1714,6 +1711,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 accesses enabled by default.  */
+    libxl_defbool_setdefault(&b_info->trap_unmapped_accesses, true);
+
     /* Sanitise SVE parameter */
     if (b_info->arch_arm.sve_vl) {
         unsigned int max_sve_vl =
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index e03599ea99..38770eea5b 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -667,6 +667,9 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
         if (libxl_defbool_val(b_info->vpmu))
             create.flags |= XEN_DOMCTL_CDF_vpmu;
 
+        if (libxl_defbool_val(b_info->trap_unmapped_accesses))
+            create.flags |= XEN_DOMCTL_CDF_trap_unmapped_accesses;
+
         assert(info->passthrough != LIBXL_PASSTHROUGH_DEFAULT);
         LOG(DETAIL, "passthrough: %s",
             libxl_passthrough_to_string(info->passthrough));
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 9bb2969931..e33785c661 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -736,6 +736,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("vmtrace_buf_kb", integer),
 
     ("vpmu", libxl_defbool),
+    ("trap_unmapped_accesses", libxl_defbool),
 
     ], dir=DIR_IN,
        copy_deprecated_fn="libxl__domain_build_info_copy_deprecated",
diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index 0b1c2d3a96..a9d470c9f6 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -26,6 +26,11 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
     if (libxl_defbool_val(d_config->b_info.arch_x86.msr_relaxed))
         config->arch.misc_flags |= XEN_X86_MSR_RELAXED;
 
+    if (libxl_defbool_val(d_config->b_info.trap_unmapped_accesses)) {
+            LOG(ERROR, "trap_unmapped_accesses is not supported on x86\n");
+            return ERROR_FAIL;
+    }
+
     return 0;
 }
 
@@ -813,6 +818,7 @@ int libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
 {
     libxl_defbool_setdefault(&b_info->acpi, true);
     libxl_defbool_setdefault(&b_info->arch_x86.msr_relaxed, false);
+    libxl_defbool_setdefault(&b_info->trap_unmapped_accesses, false);
 
     /*
      * The config parameter "altp2m" replaces the parameter "altp2mhvm".
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 089a88935a..40da75ef74 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_accesses",
+                        &b_info->trap_unmapped_accesses, 0);
+
     parse_vkb_list(config, d_config);
 
     d_config->virtios = NULL;
-- 
2.43.0

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

[Julien Grall]

Hi Edgar,

On 29/05/2025 16:50, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> 
> 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@amd.com>
> ---
>   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             | 36 +++++++++++++++++++++++++++++++++--
>   xen/common/domain.c           |  3 ++-
>   xen/include/public/domctl.h   |  4 +++-

Looking at the changelog, I saw you removed the go bindings (although, 
they were in patch 3). But I don't quite understand why.

Also, I think you need to update the OCaml bindings.


>   7 files changed, 49 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 );
>       unsigned int sve_vl_bits = sve_decode_vl(config->arch.sve_vl);
>   
>       if ( (config->flags & ~flags_optional) != flags_required )
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index b189a7cfae..7ff9c1b584 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2003,7 +2003,8 @@ void __init create_dom0(void)
>   {
>       struct domain *dom0;
>       struct xen_domctl_createdomain dom0_cfg = {
> -        .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
> +        .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
> +                 XEN_DOMCTL_CDF_trap_unmapped_accesses,
>           .max_evtchn_port = -1,
>           .max_grant_frames = gnttab_dom0_frames(),
>           .max_maptrack_frames = -1,
> diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
> index 5a4b0e8f25..adfc822e7e 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);

NIT: Looking at the other part of io.c, we are using GENMASK. Any reason 
to not use the same?

> +
> +    /* 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)
> @@ -175,11 +201,17 @@ enum io_state try_handle_mmio(struct cpu_user_regs *regs,
>       handler = find_mmio_handler(v->domain, info->gpa);
>       if ( !handler )
>       {
> +        bool trap_unmapped = v->domain->options &
> +                                         XEN_DOMCTL_CDF_trap_unmapped_accesses;
>           rc = try_fwd_ioserv(regs, v, info);
>           if ( rc == IO_HANDLED )
>               return handle_ioserv(regs, v);
> -
> -        return rc;
> +        else if ( rc == IO_UNHANDLED && !trap_unmapped )
> +        {
> +            /* Fallback to the unmapped handler. */
> +            handler = &unmapped_handler;
> +        } else

Style:

else if ( ... )
{
}
else
{
}

> +            return rc;
>       }
>   
>       /*
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index abf1969e60..ac4f58f638 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -721,7 +721,8 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>            ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
>              XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
>              XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
> -           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu) )
> +           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
> +           XEN_DOMCTL_CDF_trap_unmapped_accesses) )
>       {
>           dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
>           return -EINVAL;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 5b2063eed9..be19ab5e26 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -66,9 +66,11 @@ struct xen_domctl_createdomain {
>   #define XEN_DOMCTL_CDF_nested_virt    (1U << _XEN_DOMCTL_CDF_nested_virt)
>   /* Should we expose the vPMU to the guest? */
>   #define XEN_DOMCTL_CDF_vpmu           (1U << 7)
> +/* Should we trap guest accesses to unmapped addresses? */
> +#define XEN_DOMCTL_CDF_trap_unmapped_accesses  (1U << 8)
>   
>   /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
> -#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpmu
> +#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_trap_unmapped_accesses
>   
>       uint32_t flags;
>   

-- 
Julien Grall

Re: [PATCH v2 2/3] xen/arm: dom0less: Add trap-unmapped-accesses

[Julien Grall]

Hi Edgar,

On 29/05/2025 16:50, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> 
> Add the trap-unmapped-accesses per-domain fdt property.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>   docs/misc/arm/device-tree/booting.txt | 9 +++++++++
>   xen/arch/arm/dom0less-build.c         | 9 ++++++++-
>   2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
> index 59fa96a82e..8a5c40ddf3 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -225,6 +225,15 @@ with the following properties:
>       option is provided with a non zero value, but the platform doesn't support
>       SVE.
>   
> +- trap-unmapped-accesses
> +
> +    Optional. An integer that configures handling of accesses to unmapped
> +    address ranges.
> +    If set to 0, guest accesses will read 0xFFFFFFFF and writes will be ignored.

Looking at the code, if I am not mistaken, it will only return this 
value for 32-bit. For 64-bit there will be a few Fs more and for less 
there will be less. So I think this needs to be reworded.

The rest looks good to me.

> +
> +    This option is only implemented for ARM where the default is 1.
> +
>   - 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 a4e0a33632..69324aa597 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -344,8 +344,15 @@ void __init arch_create_domUs(struct dt_device_node *node,
>   #endif
>       }
>   
> -    /* Trap accesses to unmapped areas. */
> +    /* Trap unmapped accesses by default. */
>       d_cfg->flags |= XEN_DOMCTL_CDF_trap_unmapped_accesses;
> +    if ( dt_property_read_u32(node, "trap-unmapped-accesses", &val) )
> +    {
> +        if ( val > 1 )
> +            panic("trap-unmapped-accesses: supported values are 0 or 1");
> +        if ( !val )
> +            d_cfg->flags &= ~XEN_DOMCTL_CDF_trap_unmapped_accesses;
> +    }
>   }
>   
>   int __init init_intc_phandle(struct kernel_info *kinfo, const char *name,

Cheers,

-- 
Julien Grall

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

[Edgar E. Iglesias]

On Thu, May 29, 2025 at 04:59:21PM +0100, Julien Grall wrote:
> Hi Edgar,

Hi Julien,


> 
> On 29/05/2025 16:50, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> > 
> > 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@amd.com>
> > ---
> >   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             | 36 +++++++++++++++++++++++++++++++++--
> >   xen/common/domain.c           |  3 ++-
> >   xen/include/public/domctl.h   |  4 +++-
> 
> Looking at the changelog, I saw you removed the go bindings (although, they
> were in patch 3). But I don't quite understand why.

I got a little confused. The file tools/golang/xenlight/helpers.gen.go
has the following at the top:
// Code generated by gengotypes.py. DO NOT EDIT.
// source: libxl_types.idl


So I got the impression that we shouldn't be editing it.
Should I edit it manually? Or should I try to rerun gengotypes.py
to generate these bindings?


> 
> Also, I think you need to update the OCaml bindings.

I see, I'll have a look.

> 
> 
> >   7 files changed, 49 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 );
> >       unsigned int sve_vl_bits = sve_decode_vl(config->arch.sve_vl);
> >       if ( (config->flags & ~flags_optional) != flags_required )
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index b189a7cfae..7ff9c1b584 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -2003,7 +2003,8 @@ void __init create_dom0(void)
> >   {
> >       struct domain *dom0;
> >       struct xen_domctl_createdomain dom0_cfg = {
> > -        .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
> > +        .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
> > +                 XEN_DOMCTL_CDF_trap_unmapped_accesses,
> >           .max_evtchn_port = -1,
> >           .max_grant_frames = gnttab_dom0_frames(),
> >           .max_maptrack_frames = -1,
> > diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
> > index 5a4b0e8f25..adfc822e7e 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);
> 
> NIT: Looking at the other part of io.c, we are using GENMASK. Any reason to
> not use the same?

Looking closer at it now and no, there's no good reason. I'll change
to GENMASK in v3.


> 
> > +
> > +    /* 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)
> > @@ -175,11 +201,17 @@ enum io_state try_handle_mmio(struct cpu_user_regs *regs,
> >       handler = find_mmio_handler(v->domain, info->gpa);
> >       if ( !handler )
> >       {
> > +        bool trap_unmapped = v->domain->options &
> > +                                         XEN_DOMCTL_CDF_trap_unmapped_accesses;
> >           rc = try_fwd_ioserv(regs, v, info);
> >           if ( rc == IO_HANDLED )
> >               return handle_ioserv(regs, v);
> > -
> > -        return rc;
> > +        else if ( rc == IO_UNHANDLED && !trap_unmapped )
> > +        {
> > +            /* Fallback to the unmapped handler. */
> > +            handler = &unmapped_handler;
> > +        } else
> 
> Style:
> 
> else if ( ... )
> {
> }
> else
> {
> }

Will fix for v3.

Thanks,
Edgar


> 
> > +            return rc;
> >       }
> >       /*
> > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > index abf1969e60..ac4f58f638 100644
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -721,7 +721,8 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
> >            ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
> >              XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
> >              XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
> > -           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu) )
> > +           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
> > +           XEN_DOMCTL_CDF_trap_unmapped_accesses) )
> >       {
> >           dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
> >           return -EINVAL;
> > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > index 5b2063eed9..be19ab5e26 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -66,9 +66,11 @@ struct xen_domctl_createdomain {
> >   #define XEN_DOMCTL_CDF_nested_virt    (1U << _XEN_DOMCTL_CDF_nested_virt)
> >   /* Should we expose the vPMU to the guest? */
> >   #define XEN_DOMCTL_CDF_vpmu           (1U << 7)
> > +/* Should we trap guest accesses to unmapped addresses? */
> > +#define XEN_DOMCTL_CDF_trap_unmapped_accesses  (1U << 8)
> >   /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
> > -#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpmu
> > +#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_trap_unmapped_accesses
> >       uint32_t flags;
> 
> -- 
> Julien Grall
> 

Re: [PATCH v2 2/3] xen/arm: dom0less: Add trap-unmapped-accesses

[Edgar E. Iglesias]

On Thu, May 29, 2025 at 05:03:12PM +0100, Julien Grall wrote:
> Hi Edgar,
> 
> On 29/05/2025 16:50, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> > 
> > Add the trap-unmapped-accesses per-domain fdt property.
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > ---
> >   docs/misc/arm/device-tree/booting.txt | 9 +++++++++
> >   xen/arch/arm/dom0less-build.c         | 9 ++++++++-
> >   2 files changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
> > index 59fa96a82e..8a5c40ddf3 100644
> > --- a/docs/misc/arm/device-tree/booting.txt
> > +++ b/docs/misc/arm/device-tree/booting.txt
> > @@ -225,6 +225,15 @@ with the following properties:
> >       option is provided with a non zero value, but the platform doesn't support
> >       SVE.
> > +- trap-unmapped-accesses
> > +
> > +    Optional. An integer that configures handling of accesses to unmapped
> > +    address ranges.
> > +    If set to 0, guest accesses will read 0xFFFFFFFF and writes will be ignored.
> 
> Looking at the code, if I am not mistaken, it will only return this value
> for 32-bit. For 64-bit there will be a few Fs more and for less there will
> be less. So I think this needs to be reworded.
> 
> The rest looks good to me.

Thanks, in v3 I'll change it to:
    If set to 0, guest accesses will read all bits as ones, e.g 0xFFFFFFFF
    for a 32bit access and writes will be ignored.

Cheers,
Edgar

> 
> > +
> > +    This option is only implemented for ARM where the default is 1.
> > +
> >   - 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 a4e0a33632..69324aa597 100644
> > --- a/xen/arch/arm/dom0less-build.c
> > +++ b/xen/arch/arm/dom0less-build.c
> > @@ -344,8 +344,15 @@ void __init arch_create_domUs(struct dt_device_node *node,
> >   #endif
> >       }
> > -    /* Trap accesses to unmapped areas. */
> > +    /* Trap unmapped accesses by default. */
> >       d_cfg->flags |= XEN_DOMCTL_CDF_trap_unmapped_accesses;
> > +    if ( dt_property_read_u32(node, "trap-unmapped-accesses", &val) )
> > +    {
> > +        if ( val > 1 )
> > +            panic("trap-unmapped-accesses: supported values are 0 or 1");
> > +        if ( !val )
> > +            d_cfg->flags &= ~XEN_DOMCTL_CDF_trap_unmapped_accesses;
> > +    }
> >   }
> >   int __init init_intc_phandle(struct kernel_info *kinfo, const char *name,
> 
> Cheers,
> 
> -- 
> Julien Grall
> 

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

[Julien Grall]

On 29/05/2025 17:23, Edgar E. Iglesias wrote:
> On Thu, May 29, 2025 at 04:59:21PM +0100, Julien Grall wrote:
>> Hi Edgar,
> 
> Hi Julien,
> 
> 
>>
>> On 29/05/2025 16:50, Edgar E. Iglesias wrote:
>>> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
>>>
>>> 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@amd.com>
>>> ---
>>>    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             | 36 +++++++++++++++++++++++++++++++++--
>>>    xen/common/domain.c           |  3 ++-
>>>    xen/include/public/domctl.h   |  4 +++-
>>
>> Looking at the changelog, I saw you removed the go bindings (although, they
>> were in patch 3). But I don't quite understand why.
> 
> I got a little confused. The file tools/golang/xenlight/helpers.gen.go
> has the following at the top:
> // Code generated by gengotypes.py. DO NOT EDIT.
> // source: libxl_types.idl
> 
> 
> So I got the impression that we shouldn't be editing it.
> Should I edit it manually? Or should I try to rerun gengotypes.py
> to generate these bindings?

As the file is checked in, I think we expect the developper to rerun 
gengotypes.py. Anthony, can you confirm?

Cheers,

-- 
Julien Grall

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

[Edgar E. Iglesias]

On Fri, May 30, 2025 at 10:01:23AM +0100, Julien Grall wrote:
> 
> 
> On 29/05/2025 17:23, Edgar E. Iglesias wrote:
> > On Thu, May 29, 2025 at 04:59:21PM +0100, Julien Grall wrote:
> > > Hi Edgar,
> > 
> > Hi Julien,
> > 
> > 
> > > 
> > > On 29/05/2025 16:50, Edgar E. Iglesias wrote:
> > > > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> > > > 
> > > > 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@amd.com>
> > > > ---
> > > >    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             | 36 +++++++++++++++++++++++++++++++++--
> > > >    xen/common/domain.c           |  3 ++-
> > > >    xen/include/public/domctl.h   |  4 +++-
> > > 
> > > Looking at the changelog, I saw you removed the go bindings (although, they
> > > were in patch 3). But I don't quite understand why.
> > 
> > I got a little confused. The file tools/golang/xenlight/helpers.gen.go
> > has the following at the top:
> > // Code generated by gengotypes.py. DO NOT EDIT.
> > // source: libxl_types.idl
> > 
> > 
> > So I got the impression that we shouldn't be editing it.
> > Should I edit it manually? Or should I try to rerun gengotypes.py
> > to generate these bindings?
> 
> As the file is checked in, I think we expect the developper to rerun
> gengotypes.py. Anthony, can you confirm?
>

Thanks, I've included a regeneration of the bindings in v3. If it for
some reason is not needed we can drop it.

Cheers,
Edgar