All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] xen/arm64: allow to make aarch32 support optional
@ 2025-08-06  9:49 Grygorii Strashko
  2025-08-06  9:49 ` [PATCH v2 1/4] xen/arm: split set_domain_type() between arm64/arm32 Grygorii Strashko
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Grygorii Strashko @ 2025-08-06  9:49 UTC (permalink / raw)
  To: xen-devel@lists.xenproject.org
  Cc: Grygorii Strashko, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
	Anthony PERARD, Jan Beulich, Roger Pau Monné

From: Grygorii Strashko <grygorii_strashko@epam.com>

Hi,

During review of v1 [1] of this series Julien Grall raised concern that 
"If the desire is to make 32-bit domain optional on Arm64. 
Then I think it would be better to pass the domain type when the domain is
created (IOW add an extra flags to XEN_DOMCTL_createdomain)." for which
I've sent patches attempting to start solving the problem [2] and try to
probe guest kernels before creating domains. While proposed changes [2] is
under review and hence there are definitely more work is required I'd
appreciated if current series can be considered as it's Arm specific only
and working (and tested) with current Xen in its current state.

Now Arm64 AArch32 guest support is always enabled and built-in while not
all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this
support might not be needed (Arm64 AArch32 is used quite rarely in embedded
systems). More over, when focusing on safety certification, AArch32 related
code in Xen leaves a gap in terms of coverage that cannot really be
justified in words. This leaves two options: either support it (lots of
additional testing, requirements and documents would be needed) or compile
it out.

Patches 1-2 Prerequisite patches
Patch 3 - allows to make aarch32 support optional by introducing Kconfig option
          CONFIG_ARM64_AARCH32
Patch 4 - enables build-time optimization of AArch32 specific code by redefining some
          is_32/64bit_domain() macro as constants

Tested (CONFIG_ARM64_AARCH32=y/n):
- dom0 with AArch32/64 kernel
- toolstack create domain with AArch32/64 kernel
- dom0less domains with AArch32/64 kernel
- creating domain with AArch64 kernel and AArch32 EL0 rootfs

Changes in v2:
- dropped patches
  - (licensing issue) "xen/arm: move vcpu_switch_to_aarch64_mode() in arch_vcpu_create()"
  - (problematic change) "xen/arm: move vcpu_switch_to_aarch64_mode() in arm64"
  - constifying is_32/64bit_domain() macro gives most of results in terms of coverage,
    drop regs changes for now (can be added latter):
    "xen/arm: regs.h split subarch definitions between arm64/arm32"
    "xen/arm64: constify regs_mode_is_32bit macro for CONFIG_ARM64_AARCH32=n"
- use Arm64 "cpu_has_el1_32" in all places to check if HW has AArch32 support
- rework Arm64 XEN_DOMCTL_set_address_size hypercall handling to work with any initial domain type
  set (32bit or 64 bit)
- fix comments related to macro parameters evaluation issues
- do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
  AArch32 is disabled

Link on v1:
[1] https://lore.kernel.org/xen-devel/20250723075835.3993182-1-grygorii_strashko@epam.com/

[2] https://patchwork.kernel.org/project/xen-devel/cover/20250731094234.996684-1-grygorii_strashko@epam.com/

Grygorii Strashko (4):
  xen/arm: split set_domain_type() between arm64/arm32
  xen/arm: split is_32bit/64bit_domain() between arm64/arm32
  xen/arm64: allow to make aarch32 support optional
  xen/arm64: constify is_32/64bit_domain() macro for
    CONFIG_ARM64_AARCH32=n

 xen/arch/arm/Kconfig                    |  7 +++
 xen/arch/arm/arm32/Makefile             |  1 +
 xen/arch/arm/arm32/domain-build.c       | 22 +++++++++
 xen/arch/arm/arm64/Makefile             |  1 +
 xen/arch/arm/arm64/domain-build.c       | 66 +++++++++++++++++++++++++
 xen/arch/arm/arm64/domctl.c             | 16 +++---
 xen/arch/arm/arm64/vsysreg.c            |  9 ++++
 xen/arch/arm/dom0less-build.c           | 14 ------
 xen/arch/arm/domain.c                   |  9 ++++
 xen/arch/arm/domain_build.c             | 21 ++------
 xen/arch/arm/include/asm/arm32/domain.h | 32 ++++++++++++
 xen/arch/arm/include/asm/arm64/domain.h | 54 ++++++++++++++++++++
 xen/arch/arm/include/asm/domain.h       |  7 ++-
 xen/arch/arm/setup.c                    |  2 +-
 xen/include/xen/dom0less-build.h        |  8 +++
 15 files changed, 227 insertions(+), 42 deletions(-)
 create mode 100644 xen/arch/arm/arm32/domain-build.c
 create mode 100644 xen/arch/arm/arm64/domain-build.c
 create mode 100644 xen/arch/arm/include/asm/arm32/domain.h
 create mode 100644 xen/arch/arm/include/asm/arm64/domain.h

-- 
2.34.1


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

* [PATCH v2 2/4] xen/arm: split is_32bit/64bit_domain() between arm64/arm32
  2025-08-06  9:49 [PATCH v2 0/4] xen/arm64: allow to make aarch32 support optional Grygorii Strashko
  2025-08-06  9:49 ` [PATCH v2 1/4] xen/arm: split set_domain_type() between arm64/arm32 Grygorii Strashko
@ 2025-08-06  9:49 ` Grygorii Strashko
  2025-08-27  0:23   ` Volodymyr Babchuk
  2025-08-06  9:49 ` [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional Grygorii Strashko
  2025-08-06  9:49 ` [PATCH v2 4/4] xen/arm64: constify is_32/64bit_domain() macro for CONFIG_ARM64_AARCH32=n Grygorii Strashko
  3 siblings, 1 reply; 19+ messages in thread
From: Grygorii Strashko @ 2025-08-06  9:49 UTC (permalink / raw)
  To: xen-devel@lists.xenproject.org
  Cc: Grygorii Strashko, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Volodymyr Babchuk

From: Grygorii Strashko <grygorii_strashko@epam.com>

Split is_32bit/64bit_domain() macro implementations between arm64/arm32.

Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
v2:
- fix comments related to macro parameters evaluation issues

 xen/arch/arm/include/asm/arm32/domain.h | 20 +++++++++++++++++
 xen/arch/arm/include/asm/arm64/domain.h | 29 +++++++++++++++++++++++++
 xen/arch/arm/include/asm/domain.h       |  7 +++---
 3 files changed, 52 insertions(+), 4 deletions(-)
 create mode 100644 xen/arch/arm/include/asm/arm32/domain.h
 create mode 100644 xen/arch/arm/include/asm/arm64/domain.h

diff --git a/xen/arch/arm/include/asm/arm32/domain.h b/xen/arch/arm/include/asm/arm32/domain.h
new file mode 100644
index 000000000000..1bd0735c9194
--- /dev/null
+++ b/xen/arch/arm/include/asm/arm32/domain.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef ARM_ARM32_DOMAIN_H
+#define ARM_ARM32_DOMAIN_H
+
+/* Arm32 always runs guests in AArch32 mode */
+
+#define is_32bit_domain(d) ((void)(d), 1)
+#define is_64bit_domain(d) ((void)(d), 0)
+
+#endif /* ARM_ARM32_DOMAIN_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/include/asm/arm64/domain.h b/xen/arch/arm/include/asm/arm64/domain.h
new file mode 100644
index 000000000000..1a2d73950bf0
--- /dev/null
+++ b/xen/arch/arm/include/asm/arm64/domain.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef ARM_ARM64_DOMAIN_H
+#define ARM_ARM64_DOMAIN_H
+
+/*
+ * Returns true if guest execution state is AArch32
+ *
+ * @d: pointer to the domain structure
+ */
+#define is_32bit_domain(d) ((d)->arch.type == DOMAIN_32BIT)
+
+/*
+ * Returns true if guest execution state is AArch64
+ *
+ * @d: pointer to the domain structure
+ */
+#define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
+
+#endif /* ARM_ARM64_DOMAIN_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index a3487ca71303..dde48178b857 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -22,11 +22,10 @@ enum domain_type {
     DOMAIN_32BIT,
     DOMAIN_64BIT,
 };
-#define is_32bit_domain(d) ((d)->arch.type == DOMAIN_32BIT)
-#define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
+
+# include <asm/arm64/domain.h>
 #else
-#define is_32bit_domain(d) (1)
-#define is_64bit_domain(d) (0)
+# include <asm/arm32/domain.h>
 #endif
 
 /*
-- 
2.34.1


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

* [PATCH v2 1/4] xen/arm: split set_domain_type() between arm64/arm32
  2025-08-06  9:49 [PATCH v2 0/4] xen/arm64: allow to make aarch32 support optional Grygorii Strashko
@ 2025-08-06  9:49 ` Grygorii Strashko
  2025-08-27  0:22   ` Volodymyr Babchuk
  2025-08-06  9:49 ` [PATCH v2 2/4] xen/arm: split is_32bit/64bit_domain() " Grygorii Strashko
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Grygorii Strashko @ 2025-08-06  9:49 UTC (permalink / raw)
  To: xen-devel@lists.xenproject.org
  Cc: Grygorii Strashko, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
	Anthony PERARD, Jan Beulich, Roger Pau Monné

From: Grygorii Strashko <grygorii_strashko@epam.com>

Split set_domain_type() between Arm64/Arm32 sub-arches as
set_domain_type() implementation is going to be extended for Arm64.

Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
v2:
- no changes, rebase

 xen/arch/arm/arm32/Makefile       |  1 +
 xen/arch/arm/arm32/domain-build.c | 22 ++++++++++++++++++++++
 xen/arch/arm/arm64/Makefile       |  1 +
 xen/arch/arm/arm64/domain-build.c | 24 ++++++++++++++++++++++++
 xen/arch/arm/dom0less-build.c     | 14 --------------
 xen/include/xen/dom0less-build.h  |  8 ++++++++
 6 files changed, 56 insertions(+), 14 deletions(-)
 create mode 100644 xen/arch/arm/arm32/domain-build.c
 create mode 100644 xen/arch/arm/arm64/domain-build.c

diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 531168f58a0a..0fd3f5272361 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -6,6 +6,7 @@ obj-y += cache.o
 obj-$(CONFIG_EARLY_PRINTK) += debug.o
 obj-y += domctl.o
 obj-y += domain.o
+obj-y += domain-build.o
 obj-y += entry.o
 obj-y += head.o
 obj-y += insn.o
diff --git a/xen/arch/arm/arm32/domain-build.c b/xen/arch/arm/arm32/domain-build.c
new file mode 100644
index 000000000000..e34261e4a2ad
--- /dev/null
+++ b/xen/arch/arm/arm32/domain-build.c
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/fdt-kernel.h>
+#include <xen/sched.h>
+
+#include <asm/domain.h>
+
+#ifdef CONFIG_DOM0LESS_BOOT
+void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
+{
+    /* Nothing to do */
+}
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 6491c5350b2e..3272fe7e4ca2 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_HARDEN_BRANCH_PREDICTOR) += bpi.o
 obj-$(CONFIG_EARLY_PRINTK) += debug.o
 obj-y += domctl.o
 obj-y += domain.o
+obj-y += domain-build.o
 obj-y += entry.o
 obj-y += head.o
 obj-y += insn.o
diff --git a/xen/arch/arm/arm64/domain-build.c b/xen/arch/arm/arm64/domain-build.c
new file mode 100644
index 000000000000..3a89ee46b8c6
--- /dev/null
+++ b/xen/arch/arm/arm64/domain-build.c
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/fdt-kernel.h>
+#include <xen/sched.h>
+
+#include <asm/domain.h>
+
+#ifdef CONFIG_DOM0LESS_BOOT
+/* TODO: make arch.type generic ? */
+void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
+{
+    /* type must be set before allocate memory */
+    d->arch.type = kinfo->arch.type;
+}
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index c8d07213e247..58f77628df1f 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -236,20 +236,6 @@ int __init make_arch_nodes(struct kernel_info *kinfo)
     return 0;
 }
 
-/* TODO: make arch.type generic ? */
-#ifdef CONFIG_ARM_64
-void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
-{
-    /* type must be set before allocate memory */
-    d->arch.type = kinfo->arch.type;
-}
-#else
-void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
-{
-    /* Nothing to do */
-}
-#endif
-
 int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
                       const struct dt_device_node *node)
 {
diff --git a/xen/include/xen/dom0less-build.h b/xen/include/xen/dom0less-build.h
index 408859e3255a..3e81d8ba3a47 100644
--- a/xen/include/xen/dom0less-build.h
+++ b/xen/include/xen/dom0less-build.h
@@ -57,6 +57,14 @@ int init_vuart(struct domain *d, struct kernel_info *kinfo,
 int make_intc_domU_node(struct kernel_info *kinfo);
 int make_arch_nodes(struct kernel_info *kinfo);
 
+/*
+ * Set domain type from struct kernel_info which defines guest Execution
+ * State 32-bit/64-bit (for Arm AArch32/AArch64).
+ * The domain type must be set before allocate_memory.
+ *
+ * @d: pointer to the domain structure.
+ * @kinfo: pointer to the kinfo structure.
+ */
 void set_domain_type(struct domain *d, struct kernel_info *kinfo);
 
 int init_intc_phandle(struct kernel_info *kinfo, const char *name,
-- 
2.34.1


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

* [PATCH v2 4/4] xen/arm64: constify is_32/64bit_domain() macro for CONFIG_ARM64_AARCH32=n
  2025-08-06  9:49 [PATCH v2 0/4] xen/arm64: allow to make aarch32 support optional Grygorii Strashko
                   ` (2 preceding siblings ...)
  2025-08-06  9:49 ` [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional Grygorii Strashko
@ 2025-08-06  9:49 ` Grygorii Strashko
  2025-08-27  0:19   ` Volodymyr Babchuk
  3 siblings, 1 reply; 19+ messages in thread
From: Grygorii Strashko @ 2025-08-06  9:49 UTC (permalink / raw)
  To: xen-devel@lists.xenproject.org
  Cc: Grygorii Strashko, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Volodymyr Babchuk

From: Grygorii Strashko <grygorii_strashko@epam.com>

Constify is_32/64bit_domain() macro for the case CONFIG_ARM64_AARCH32=n and
so allow compiler to opt out Aarch32 specific code.

Before (CONFIG_ARM64_AARCH32=y):
   text	   data	    bss	    dec	    hex	filename
 859212	 322404	 270880	1452496	 1629d0	xen-syms-before

After (CONFIG_ARM64_AARCH32=n):
   text	   data	    bss	    dec	    hex	filename
 851256	 322404	 270880	1444540	 160abc	xen-syms-after

Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
v2:
- use IS_ENABLED(CONFIG_ARM64_AARCH32) instead of ifdefs

 xen/arch/arm/include/asm/arm64/domain.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/include/asm/arm64/domain.h b/xen/arch/arm/include/asm/arm64/domain.h
index bebcbc582f97..70dfbeac7443 100644
--- a/xen/arch/arm/include/asm/arm64/domain.h
+++ b/xen/arch/arm/include/asm/arm64/domain.h
@@ -12,14 +12,16 @@ struct kernel_info;
  *
  * @d: pointer to the domain structure
  */
-#define is_32bit_domain(d) ((d)->arch.type == DOMAIN_32BIT)
+#define is_32bit_domain(d)                                                     \
+        (IS_ENABLED(CONFIG_ARM64_AARCH32) && (d)->arch.type == DOMAIN_32BIT)
 
 /*
  * Returns true if guest execution state is AArch64
  *
  * @d: pointer to the domain structure
  */
-#define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
+#define is_64bit_domain(d)                                                     \
+        (!IS_ENABLED(CONFIG_ARM64_AARCH32) || (d)->arch.type == DOMAIN_64BIT)
 
 /*
  * Arm64 declares AArch32 (32bit) Execution State support in the
-- 
2.34.1


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

* [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional
  2025-08-06  9:49 [PATCH v2 0/4] xen/arm64: allow to make aarch32 support optional Grygorii Strashko
  2025-08-06  9:49 ` [PATCH v2 1/4] xen/arm: split set_domain_type() between arm64/arm32 Grygorii Strashko
  2025-08-06  9:49 ` [PATCH v2 2/4] xen/arm: split is_32bit/64bit_domain() " Grygorii Strashko
@ 2025-08-06  9:49 ` Grygorii Strashko
  2025-08-27  0:16   ` Volodymyr Babchuk
                     ` (2 more replies)
  2025-08-06  9:49 ` [PATCH v2 4/4] xen/arm64: constify is_32/64bit_domain() macro for CONFIG_ARM64_AARCH32=n Grygorii Strashko
  3 siblings, 3 replies; 19+ messages in thread
From: Grygorii Strashko @ 2025-08-06  9:49 UTC (permalink / raw)
  To: xen-devel@lists.xenproject.org
  Cc: Grygorii Strashko, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Volodymyr Babchuk

From: Grygorii Strashko <grygorii_strashko@epam.com>

Now Arm64 AArch32 guest support is always enabled and built-in while not
all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this
support might not be needed (Arm64 AArch32 is used quite rarely in embedded
systems). More over, when focusing on safety certification, AArch32 related
code in Xen leaves a gap in terms of coverage that cannot really be
justified in words. This leaves two options: either support it (lots of
additional testing, requirements and documents would be needed) or compile
it out.

Hence, this patch introduces basic support to allow make Arm64
AArch32 guest support optional. The following changes are introduced:

- Introduce Kconfig option CONFIG_ARM64_AARCH32 to allow enable/disable
  Arm64 AArch32 guest support (default y)

- Introduce is_aarch32_enabled() helper which accounts Arm64 HW capability
  and CONFIG_ARM64_AARCH32 setting

- Introduce arm64_set_domain_type() to configure Arm64 domain type in
  unified way instead of open coding (d)->arch.type, and account
  CONFIG_ARM64_AARCH32 configuration.

- toolstack: do not advertise "xen-3.0-armv7l " capability if AArch32 is
  disabled.

- do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
  AArch32 is disabled.

- Set Arm64 domain type to DOMAIN_64BIT by default.
  - the Arm Xen boot code is handling this case properly already;
  - for toolstack case the XEN_DOMCTL_set_address_size hypercall handling
    updated to forcibly configure domain type regardless of current domain
    type configuration, so toolstack behavior is unchanged.

With CONFIG_ARM64_AARCH32=n the Xen will reject AArch32 guests (kernels) if
configured by user in the following way:
- Xen boot will fail with panic during dom0 or dom0less domains creation
- toolstack domain creation will be rejected due to xc_dom_compat_check()
  failure.

Making Arm64 AArch32 guest support open further possibilities for build
optimizations of Arm64 AArch32 guest support code.

Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
changes in v2:
- use Arm64 "cpu_has_el1_32" in all places to check if HW has AArch32 support
- rework Arm64 XEN_DOMCTL_set_address_size hypercall handling to work with any
  initial domain type set (32bit or 64 bit)
- fix comments related to macro parameters evaluation issues
- do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
  AArch32 is disabled

 xen/arch/arm/Kconfig                    |  7 ++++
 xen/arch/arm/arm64/domain-build.c       | 46 +++++++++++++++++++++++--
 xen/arch/arm/arm64/domctl.c             | 16 +++++----
 xen/arch/arm/arm64/vsysreg.c            |  9 +++++
 xen/arch/arm/domain.c                   |  9 +++++
 xen/arch/arm/domain_build.c             | 21 +++--------
 xen/arch/arm/include/asm/arm32/domain.h | 12 +++++++
 xen/arch/arm/include/asm/arm64/domain.h | 23 +++++++++++++
 xen/arch/arm/setup.c                    |  2 +-
 9 files changed, 119 insertions(+), 26 deletions(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index a0c816047427..bf6dd73caf73 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -266,6 +266,13 @@ config PCI_PASSTHROUGH
 	help
 	  This option enables PCI device passthrough
 
+config ARM64_AARCH32
+	bool "AArch32 Guests support on on ARM64 (UNSUPPORTED)" if UNSUPPORTED
+	depends on ARM_64
+	default y
+	help
+	  This option enables AArch32 Guests on ARM64.
+
 endmenu
 
 menu "ARM errata workaround via the alternative framework"
diff --git a/xen/arch/arm/arm64/domain-build.c b/xen/arch/arm/arm64/domain-build.c
index 3a89ee46b8c6..5951f002e3af 100644
--- a/xen/arch/arm/arm64/domain-build.c
+++ b/xen/arch/arm/arm64/domain-build.c
@@ -4,13 +4,55 @@
 #include <xen/sched.h>
 
 #include <asm/domain.h>
+#include <asm/arm64/sve.h>
+
+int __init arm64_set_domain_type(struct kernel_info *kinfo)
+{
+    struct domain *d = kinfo->bd.d;
+    enum domain_type type;
+
+    ASSERT(d);
+    ASSERT(kinfo);
+
+    type = kinfo->arch.type;
+
+    if ( !is_aarch32_enabled() )
+    {
+        ASSERT(d->arch.type == DOMAIN_64BIT);
+
+        if ( type == DOMAIN_32BIT )
+        {
+            const char *str = "not available";
+
+            if ( !IS_ENABLED(CONFIG_ARM64_AARCH32) )
+                str = "disabled";
+            printk("aarch32 guests support is %s\n", str);
+            return -EINVAL;
+        }
+
+        return 0;
+    }
+
+    if ( is_sve_domain(d) && type == DOMAIN_32BIT )
+    {
+        printk("SVE is not available for 32-bit domain\n");
+        return -EINVAL;
+    }
+
+    d->arch.type = type;
+
+    return 0;
+}
 
 #ifdef CONFIG_DOM0LESS_BOOT
 /* TODO: make arch.type generic ? */
 void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
 {
-    /* type must be set before allocate memory */
-    d->arch.type = kinfo->arch.type;
+    int rc;
+
+    rc = arm64_set_domain_type(kinfo);
+    if ( rc < 0 )
+        panic("Unsupported guest type\n");
 }
 #endif
 
diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
index 8720d126c97d..4f0f143daef8 100644
--- a/xen/arch/arm/arm64/domctl.c
+++ b/xen/arch/arm/arm64/domctl.c
@@ -13,6 +13,11 @@
 #include <asm/arm64/sve.h>
 #include <asm/cpufeature.h>
 
+static void vcpu_switch_to_aarch32_mode(struct vcpu *v)
+{
+    v->arch.hcr_el2 &= ~HCR_RW;
+}
+
 static long switch_mode(struct domain *d, enum domain_type type)
 {
     struct vcpu *v;
@@ -21,14 +26,14 @@ static long switch_mode(struct domain *d, enum domain_type type)
         return -EINVAL;
     if ( domain_tot_pages(d) != 0 )
         return -EBUSY;
-    if ( d->arch.type == type )
-        return 0;
 
     d->arch.type = type;
 
-    if ( is_64bit_domain(d) )
-        for_each_vcpu(d, v)
+    for_each_vcpu(d, v)
+        if ( is_64bit_domain(d) )
             vcpu_switch_to_aarch64_mode(v);
+        else
+            vcpu_switch_to_aarch32_mode(v);
 
     return 0;
 }
@@ -38,7 +43,7 @@ static long set_address_size(struct domain *d, uint32_t address_size)
     switch ( address_size )
     {
     case 32:
-        if ( !cpu_has_el1_32 )
+        if ( !is_aarch32_enabled() )
             return -EINVAL;
         /* SVE is not supported for 32 bit domain */
         if ( is_sve_domain(d) )
@@ -58,7 +63,6 @@ long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
     {
     case XEN_DOMCTL_set_address_size:
         return set_address_size(d, domctl->u.address_size.size);
-
     default:
         return -ENOSYS;
     }
diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
index d14258290ff0..9f7e735c9b74 100644
--- a/xen/arch/arm/arm64/vsysreg.c
+++ b/xen/arch/arm/arm64/vsysreg.c
@@ -330,6 +330,15 @@ void do_sysreg(struct cpu_user_regs *regs,
     {
         register_t guest_reg_value = domain_cpuinfo.pfr64.bits[0];
 
+        if ( !is_aarch32_enabled() )
+        {
+            /* do not expose EL1 AArch32 support if disabled */
+            register_t mask = GENMASK(ID_AA64PFR0_EL1_SHIFT + 4 - 1,
+                                      ID_AA64PFR0_EL1_SHIFT);
+            guest_reg_value &= ~mask;
+            guest_reg_value |= (1 << ID_AA64PFR0_EL1_SHIFT) & mask;
+        }
+
         if ( is_sve_domain(v->domain) )
         {
             /* 4 is the SVE field width in id_aa64pfr0_el1 */
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 310c5789096d..544d1422a793 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -791,6 +791,15 @@ int arch_domain_create(struct domain *d,
     d->arch.sve_vl = config->arch.sve_vl;
 #endif
 
+#ifdef CONFIG_ARM_64
+    /*
+     * Set default Execution State to AArch64 (64bit)
+     * - Xen will reconfigure it properly at boot time
+     * - toolstack will reconfigure it properly by XEN_DOMCTL_set_address_size
+     */
+    d->arch.type = DOMAIN_64BIT;
+#endif
+
     return 0;
 
 fail:
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 47ba920fc6b0..c616127e8da5 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1873,19 +1873,6 @@ int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
     BUG_ON(v->is_initialised);
 
 #ifdef CONFIG_ARM_64
-    /* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
-    if ( !(cpu_has_el1_32) && kinfo->arch.type == DOMAIN_32BIT )
-    {
-        printk("Platform does not support 32-bit domain\n");
-        return -EINVAL;
-    }
-
-    if ( is_sve_domain(d) && (kinfo->arch.type == DOMAIN_32BIT) )
-    {
-        printk("SVE is not available for 32-bit domain\n");
-        return -EINVAL;
-    }
-
     if ( is_64bit_domain(d) )
         vcpu_switch_to_aarch64_mode(v);
 
@@ -1983,6 +1970,10 @@ static int __init construct_dom0(struct domain *d)
     if ( rc < 0 )
         return rc;
 
+    rc = arm64_set_domain_type(&kinfo);
+    if ( rc < 0 )
+        return rc;
+
     return construct_hwdom(&kinfo, NULL);
 }
 
@@ -1994,10 +1985,6 @@ int __init construct_hwdom(struct kernel_info *kinfo,
 
     iommu_hwdom_init(d);
 
-#ifdef CONFIG_ARM_64
-    /* type must be set before allocate_memory */
-    d->arch.type = kinfo->arch.type;
-#endif
     find_gnttab_region(d, kinfo);
     if ( is_domain_direct_mapped(d) )
         allocate_memory_11(d, kinfo);
diff --git a/xen/arch/arm/include/asm/arm32/domain.h b/xen/arch/arm/include/asm/arm32/domain.h
index 1bd0735c9194..30e8818ff2ae 100644
--- a/xen/arch/arm/include/asm/arm32/domain.h
+++ b/xen/arch/arm/include/asm/arm32/domain.h
@@ -3,11 +3,23 @@
 #ifndef ARM_ARM32_DOMAIN_H
 #define ARM_ARM32_DOMAIN_H
 
+struct kernel_info;
+
 /* Arm32 always runs guests in AArch32 mode */
 
 #define is_32bit_domain(d) ((void)(d), 1)
 #define is_64bit_domain(d) ((void)(d), 0)
 
+static inline bool is_aarch32_enabled(void)
+{
+    return true;
+}
+
+static inline int arm64_set_domain_type(struct kernel_info *kinfo)
+{
+    return 0;
+}
+
 #endif /* ARM_ARM32_DOMAIN_H */
 
 /*
diff --git a/xen/arch/arm/include/asm/arm64/domain.h b/xen/arch/arm/include/asm/arm64/domain.h
index 1a2d73950bf0..bebcbc582f97 100644
--- a/xen/arch/arm/include/asm/arm64/domain.h
+++ b/xen/arch/arm/include/asm/arm64/domain.h
@@ -3,6 +3,10 @@
 #ifndef ARM_ARM64_DOMAIN_H
 #define ARM_ARM64_DOMAIN_H
 
+#include <asm/cpufeature.h>
+
+struct kernel_info;
+
 /*
  * Returns true if guest execution state is AArch32
  *
@@ -17,6 +21,25 @@
  */
 #define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
 
+/*
+ * Arm64 declares AArch32 (32bit) Execution State support in the
+ * Processor Feature Registers (PFR0), but also can be disabled manually.
+ */
+static inline bool is_aarch32_enabled(void)
+{
+    return IS_ENABLED(CONFIG_ARM64_AARCH32) && cpu_has_el1_32;
+}
+
+/*
+ * Set domain type from struct kernel_info which defines guest Execution
+ * State AArch32/AArch64 during regular dom0 or predefined (dom0less)
+ * domains creation .
+ * Type must be set before allocate_memory or create vcpus.
+ *
+ * @kinfo: pointer to the kinfo structure.
+ */
+int arm64_set_domain_type(struct kernel_info *kinfo);
+
 #endif /* ARM_ARM64_DOMAIN_H */
 
 /*
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index bb35afe56cec..c29573f0ffec 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -530,7 +530,7 @@ static int __init init_xen_cap_info(void)
 #ifdef CONFIG_ARM_64
     safe_strcat(xen_cap_info, "xen-3.0-aarch64 ");
 #endif
-    if ( cpu_has_aarch32 )
+    if ( is_aarch32_enabled() )
         safe_strcat(xen_cap_info, "xen-3.0-armv7l ");
 
     return 0;
-- 
2.34.1


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

* Re: [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional
  2025-08-06  9:49 ` [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional Grygorii Strashko
@ 2025-08-27  0:16   ` Volodymyr Babchuk
  2025-09-04 20:09     ` Grygorii Strashko
  2025-09-05  3:43   ` Demi Marie Obenour
  2025-09-05  7:04   ` Julien Grall
  2 siblings, 1 reply; 19+ messages in thread
From: Volodymyr Babchuk @ 2025-08-27  0:16 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel

Hi Grygorii,

Grygorii Strashko <grygorii_strashko@epam.com> writes:

> From: Grygorii Strashko <grygorii_strashko@epam.com>
>
> Now Arm64 AArch32 guest support is always enabled and built-in while not
> all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this
> support might not be needed (Arm64 AArch32 is used quite rarely in embedded
> systems). More over, when focusing on safety certification, AArch32 related
> code in Xen leaves a gap in terms of coverage that cannot really be
> justified in words. This leaves two options: either support it (lots of
> additional testing, requirements and documents would be needed) or compile
> it out.
>
> Hence, this patch introduces basic support to allow make Arm64
> AArch32 guest support optional. The following changes are introduced:
>
> - Introduce Kconfig option CONFIG_ARM64_AARCH32 to allow enable/disable
>   Arm64 AArch32 guest support (default y)
>
> - Introduce is_aarch32_enabled() helper which accounts Arm64 HW capability
>   and CONFIG_ARM64_AARCH32 setting
>
> - Introduce arm64_set_domain_type() to configure Arm64 domain type in
>   unified way instead of open coding (d)->arch.type, and account
>   CONFIG_ARM64_AARCH32 configuration.
>
> - toolstack: do not advertise "xen-3.0-armv7l " capability if AArch32 is
>   disabled.
>
> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>   AArch32 is disabled.
>
> - Set Arm64 domain type to DOMAIN_64BIT by default.
>   - the Arm Xen boot code is handling this case properly already;
>   - for toolstack case the XEN_DOMCTL_set_address_size hypercall handling
>     updated to forcibly configure domain type regardless of current domain
>     type configuration, so toolstack behavior is unchanged.
>
> With CONFIG_ARM64_AARCH32=n the Xen will reject AArch32 guests (kernels) if
> configured by user in the following way:
> - Xen boot will fail with panic during dom0 or dom0less domains creation
> - toolstack domain creation will be rejected due to xc_dom_compat_check()
>   failure.
>
> Making Arm64 AArch32 guest support open further possibilities for build
> optimizations of Arm64 AArch32 guest support code.
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
> ---
> changes in v2:
> - use Arm64 "cpu_has_el1_32" in all places to check if HW has AArch32 support
> - rework Arm64 XEN_DOMCTL_set_address_size hypercall handling to work with any
>   initial domain type set (32bit or 64 bit)
> - fix comments related to macro parameters evaluation issues
> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>   AArch32 is disabled
>
>  xen/arch/arm/Kconfig                    |  7 ++++
>  xen/arch/arm/arm64/domain-build.c       | 46 +++++++++++++++++++++++--
>  xen/arch/arm/arm64/domctl.c             | 16 +++++----
>  xen/arch/arm/arm64/vsysreg.c            |  9 +++++
>  xen/arch/arm/domain.c                   |  9 +++++
>  xen/arch/arm/domain_build.c             | 21 +++--------
>  xen/arch/arm/include/asm/arm32/domain.h | 12 +++++++
>  xen/arch/arm/include/asm/arm64/domain.h | 23 +++++++++++++
>  xen/arch/arm/setup.c                    |  2 +-
>  9 files changed, 119 insertions(+), 26 deletions(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index a0c816047427..bf6dd73caf73 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -266,6 +266,13 @@ config PCI_PASSTHROUGH
>  	help
>  	  This option enables PCI device passthrough
>  
> +config ARM64_AARCH32
> +	bool "AArch32 Guests support on on ARM64 (UNSUPPORTED)" if UNSUPPORTED

But aarch32 guests are supported... I understand that you wanted to say
"Disabling aarch32 support is unsupported". But currently this entry
reads backwards. I think it should be reworded better. But I have no
idea - how to do this.

> +	depends on ARM_64
> +	default y
> +	help
> +	  This option enables AArch32 Guests on ARM64.
> +
>  endmenu
>  
>  menu "ARM errata workaround via the alternative framework"
> diff --git a/xen/arch/arm/arm64/domain-build.c b/xen/arch/arm/arm64/domain-build.c
> index 3a89ee46b8c6..5951f002e3af 100644
> --- a/xen/arch/arm/arm64/domain-build.c
> +++ b/xen/arch/arm/arm64/domain-build.c
> @@ -4,13 +4,55 @@
>  #include <xen/sched.h>
>  
>  #include <asm/domain.h>
> +#include <asm/arm64/sve.h>
> +
> +int __init arm64_set_domain_type(struct kernel_info *kinfo)
> +{
> +    struct domain *d = kinfo->bd.d;
> +    enum domain_type type;
> +
> +    ASSERT(d);
> +    ASSERT(kinfo);

I don't think there is a sense in asserting that kinfo != NULL after you
dereferenced it retrieve "d"

> +
> +    type = kinfo->arch.type;
> +
> +    if ( !is_aarch32_enabled() )
> +    {
> +        ASSERT(d->arch.type == DOMAIN_64BIT);
> +
> +        if ( type == DOMAIN_32BIT )
> +        {
> +            const char *str = "not available";
> +
> +            if ( !IS_ENABLED(CONFIG_ARM64_AARCH32) )
> +                str = "disabled";
> +            printk("aarch32 guests support is %s\n", str);

XENLOG_ERROR?

> +            return -EINVAL;
> +        }
> +
> +        return 0;
> +    }
> +
> +    if ( is_sve_domain(d) && type == DOMAIN_32BIT )
> +    {
> +        printk("SVE is not available for 32-bit domain\n");

XENLOG_ERROR?

> +        return -EINVAL;
> +    }
> +
> +    d->arch.type = type;
> +
> +    return 0;
> +}
>  
>  #ifdef CONFIG_DOM0LESS_BOOT
>  /* TODO: make arch.type generic ? */
>  void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
>  {
> -    /* type must be set before allocate memory */
> -    d->arch.type = kinfo->arch.type;
> +    int rc;
> +
> +    rc = arm64_set_domain_type(kinfo);
> +    if ( rc < 0 )
> +        panic("Unsupported guest type\n");
>  }
>  #endif
>  
> diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
> index 8720d126c97d..4f0f143daef8 100644
> --- a/xen/arch/arm/arm64/domctl.c
> +++ b/xen/arch/arm/arm64/domctl.c
> @@ -13,6 +13,11 @@
>  #include <asm/arm64/sve.h>
>  #include <asm/cpufeature.h>
>  
> +static void vcpu_switch_to_aarch32_mode(struct vcpu *v)
> +{
> +    v->arch.hcr_el2 &= ~HCR_RW;
> +}
> +
>  static long switch_mode(struct domain *d, enum domain_type type)
>  {
>      struct vcpu *v;
> @@ -21,14 +26,14 @@ static long switch_mode(struct domain *d, enum domain_type type)
>          return -EINVAL;
>      if ( domain_tot_pages(d) != 0 )
>          return -EBUSY;
> -    if ( d->arch.type == type )
> -        return 0;
>  
>      d->arch.type = type;
>  
> -    if ( is_64bit_domain(d) )
> -        for_each_vcpu(d, v)
> +    for_each_vcpu(d, v)
> +        if ( is_64bit_domain(d) )

Do you really need to check domain type for every vCPU? I think that
original variant was better.

>              vcpu_switch_to_aarch64_mode(v);
> +        else
> +            vcpu_switch_to_aarch32_mode(v);
>  
>      return 0;
>  }
> @@ -38,7 +43,7 @@ static long set_address_size(struct domain *d, uint32_t address_size)
>      switch ( address_size )
>      {
>      case 32:
> -        if ( !cpu_has_el1_32 )
> +        if ( !is_aarch32_enabled() )
>              return -EINVAL;
>          /* SVE is not supported for 32 bit domain */
>          if ( is_sve_domain(d) )
> @@ -58,7 +63,6 @@ long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
>      {
>      case XEN_DOMCTL_set_address_size:
>          return set_address_size(d, domctl->u.address_size.size);
> -

Stray change?

>      default:
>          return -ENOSYS;
>      }
> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
> index d14258290ff0..9f7e735c9b74 100644
> --- a/xen/arch/arm/arm64/vsysreg.c
> +++ b/xen/arch/arm/arm64/vsysreg.c
> @@ -330,6 +330,15 @@ void do_sysreg(struct cpu_user_regs *regs,
>      {
>          register_t guest_reg_value = domain_cpuinfo.pfr64.bits[0];
>  
> +        if ( !is_aarch32_enabled() )
> +        {
> +            /* do not expose EL1 AArch32 support if disabled */
> +            register_t mask = GENMASK(ID_AA64PFR0_EL1_SHIFT + 4 - 1,
> +                                      ID_AA64PFR0_EL1_SHIFT);
> +            guest_reg_value &= ~mask;
> +            guest_reg_value |= (1 << ID_AA64PFR0_EL1_SHIFT) & mask;

Why do you need to apply mask here?

> +        }
> +
>          if ( is_sve_domain(v->domain) )
>          {
>              /* 4 is the SVE field width in id_aa64pfr0_el1 */
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 310c5789096d..544d1422a793 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -791,6 +791,15 @@ int arch_domain_create(struct domain *d,
>      d->arch.sve_vl = config->arch.sve_vl;
>  #endif
>  
> +#ifdef CONFIG_ARM_64
> +    /*
> +     * Set default Execution State to AArch64 (64bit)
> +     * - Xen will reconfigure it properly at boot time
> +     * - toolstack will reconfigure it properly by XEN_DOMCTL_set_address_size
> +     */
> +    d->arch.type = DOMAIN_64BIT;
> +#endif
> +
>      return 0;
>  
>  fail:
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 47ba920fc6b0..c616127e8da5 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1873,19 +1873,6 @@ int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
>      BUG_ON(v->is_initialised);
>  
>  #ifdef CONFIG_ARM_64
> -    /* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
> -    if ( !(cpu_has_el1_32) && kinfo->arch.type == DOMAIN_32BIT )
> -    {
> -        printk("Platform does not support 32-bit domain\n");
> -        return -EINVAL;
> -    }
> -
> -    if ( is_sve_domain(d) && (kinfo->arch.type == DOMAIN_32BIT) )
> -    {
> -        printk("SVE is not available for 32-bit domain\n");
> -        return -EINVAL;
> -    }
> -
>      if ( is_64bit_domain(d) )
>          vcpu_switch_to_aarch64_mode(v);
>  
> @@ -1983,6 +1970,10 @@ static int __init construct_dom0(struct domain *d)
>      if ( rc < 0 )
>          return rc;
>  
> +    rc = arm64_set_domain_type(&kinfo);
> +    if ( rc < 0 )
> +        return rc;
> +
>      return construct_hwdom(&kinfo, NULL);
>  }
>  
> @@ -1994,10 +1985,6 @@ int __init construct_hwdom(struct kernel_info *kinfo,
>  
>      iommu_hwdom_init(d);
>  
> -#ifdef CONFIG_ARM_64
> -    /* type must be set before allocate_memory */
> -    d->arch.type = kinfo->arch.type;
> -#endif
>      find_gnttab_region(d, kinfo);
>      if ( is_domain_direct_mapped(d) )
>          allocate_memory_11(d, kinfo);
> diff --git a/xen/arch/arm/include/asm/arm32/domain.h b/xen/arch/arm/include/asm/arm32/domain.h
> index 1bd0735c9194..30e8818ff2ae 100644
> --- a/xen/arch/arm/include/asm/arm32/domain.h
> +++ b/xen/arch/arm/include/asm/arm32/domain.h
> @@ -3,11 +3,23 @@
>  #ifndef ARM_ARM32_DOMAIN_H
>  #define ARM_ARM32_DOMAIN_H
>  
> +struct kernel_info;
> +
>  /* Arm32 always runs guests in AArch32 mode */
>  
>  #define is_32bit_domain(d) ((void)(d), 1)
>  #define is_64bit_domain(d) ((void)(d), 0)
>  
> +static inline bool is_aarch32_enabled(void)
> +{
> +    return true;
> +}
> +
> +static inline int arm64_set_domain_type(struct kernel_info *kinfo)
> +{
> +    return 0;
> +}
> +
>  #endif /* ARM_ARM32_DOMAIN_H */
>  
>  /*
> diff --git a/xen/arch/arm/include/asm/arm64/domain.h b/xen/arch/arm/include/asm/arm64/domain.h
> index 1a2d73950bf0..bebcbc582f97 100644
> --- a/xen/arch/arm/include/asm/arm64/domain.h
> +++ b/xen/arch/arm/include/asm/arm64/domain.h
> @@ -3,6 +3,10 @@
>  #ifndef ARM_ARM64_DOMAIN_H
>  #define ARM_ARM64_DOMAIN_H
>  
> +#include <asm/cpufeature.h>
> +
> +struct kernel_info;
> +
>  /*
>   * Returns true if guest execution state is AArch32
>   *
> @@ -17,6 +21,25 @@
>   */
>  #define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
>  
> +/*
> + * Arm64 declares AArch32 (32bit) Execution State support in the
> + * Processor Feature Registers (PFR0), but also can be disabled manually.
> + */
> +static inline bool is_aarch32_enabled(void)
> +{
> +    return IS_ENABLED(CONFIG_ARM64_AARCH32) && cpu_has_el1_32;
> +}
> +
> +/*
> + * Set domain type from struct kernel_info which defines guest Execution
> + * State AArch32/AArch64 during regular dom0 or predefined (dom0less)
> + * domains creation .

Extra space before full stop

> + * Type must be set before allocate_memory or create vcpus.
> + *
> + * @kinfo: pointer to the kinfo structure.
> + */
> +int arm64_set_domain_type(struct kernel_info *kinfo);
> +
>  #endif /* ARM_ARM64_DOMAIN_H */
>  
>  /*
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index bb35afe56cec..c29573f0ffec 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -530,7 +530,7 @@ static int __init init_xen_cap_info(void)
>  #ifdef CONFIG_ARM_64
>      safe_strcat(xen_cap_info, "xen-3.0-aarch64 ");
>  #endif
> -    if ( cpu_has_aarch32 )
> +    if ( is_aarch32_enabled() )
>          safe_strcat(xen_cap_info, "xen-3.0-armv7l ");
>  
>      return 0;

-- 
WBR, Volodymyr

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

* Re: [PATCH v2 4/4] xen/arm64: constify is_32/64bit_domain() macro for CONFIG_ARM64_AARCH32=n
  2025-08-06  9:49 ` [PATCH v2 4/4] xen/arm64: constify is_32/64bit_domain() macro for CONFIG_ARM64_AARCH32=n Grygorii Strashko
@ 2025-08-27  0:19   ` Volodymyr Babchuk
  0 siblings, 0 replies; 19+ messages in thread
From: Volodymyr Babchuk @ 2025-08-27  0:19 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel



Hi,

Grygorii Strashko <grygorii_strashko@epam.com> writes:

> From: Grygorii Strashko <grygorii_strashko@epam.com>
>
> Constify is_32/64bit_domain() macro for the case CONFIG_ARM64_AARCH32=n and
> so allow compiler to opt out Aarch32 specific code.
>
> Before (CONFIG_ARM64_AARCH32=y):
>    text	   data	    bss	    dec	    hex	filename
>  859212	 322404	 270880	1452496	 1629d0	xen-syms-before
>
> After (CONFIG_ARM64_AARCH32=n):
>    text	   data	    bss	    dec	    hex	filename
>  851256	 322404	 270880	1444540	 160abc	xen-syms-after
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

> ---
> v2:
> - use IS_ENABLED(CONFIG_ARM64_AARCH32) instead of ifdefs
>
>  xen/arch/arm/include/asm/arm64/domain.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/arm64/domain.h b/xen/arch/arm/include/asm/arm64/domain.h
> index bebcbc582f97..70dfbeac7443 100644
> --- a/xen/arch/arm/include/asm/arm64/domain.h
> +++ b/xen/arch/arm/include/asm/arm64/domain.h
> @@ -12,14 +12,16 @@ struct kernel_info;
>   *
>   * @d: pointer to the domain structure
>   */
> -#define is_32bit_domain(d) ((d)->arch.type == DOMAIN_32BIT)
> +#define is_32bit_domain(d)                                                     \
> +        (IS_ENABLED(CONFIG_ARM64_AARCH32) && (d)->arch.type == DOMAIN_32BIT)
>  
>  /*
>   * Returns true if guest execution state is AArch64
>   *
>   * @d: pointer to the domain structure
>   */
> -#define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
> +#define is_64bit_domain(d)                                                     \
> +        (!IS_ENABLED(CONFIG_ARM64_AARCH32) || (d)->arch.type == DOMAIN_64BIT)
>  
>  /*
>   * Arm64 declares AArch32 (32bit) Execution State support in the

-- 
WBR, Volodymyr

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

* Re: [PATCH v2 1/4] xen/arm: split set_domain_type() between arm64/arm32
  2025-08-06  9:49 ` [PATCH v2 1/4] xen/arm: split set_domain_type() between arm64/arm32 Grygorii Strashko
@ 2025-08-27  0:22   ` Volodymyr Babchuk
  2025-09-04 20:09     ` Grygorii Strashko
  0 siblings, 1 reply; 19+ messages in thread
From: Volodymyr Babchuk @ 2025-08-27  0:22 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Andrew Cooper, Anthony PERARD,
	Jan Beulich, Roger Pau Monné

Hi,

Grygorii Strashko <grygorii_strashko@epam.com> writes:

> From: Grygorii Strashko <grygorii_strashko@epam.com>
>
> Split set_domain_type() between Arm64/Arm32 sub-arches as
> set_domain_type() implementation is going to be extended for Arm64.
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
> ---
> v2:
> - no changes, rebase
>
>  xen/arch/arm/arm32/Makefile       |  1 +
>  xen/arch/arm/arm32/domain-build.c | 22 ++++++++++++++++++++++
>  xen/arch/arm/arm64/Makefile       |  1 +
>  xen/arch/arm/arm64/domain-build.c | 24 ++++++++++++++++++++++++
>  xen/arch/arm/dom0less-build.c     | 14 --------------
>  xen/include/xen/dom0less-build.h  |  8 ++++++++
>  6 files changed, 56 insertions(+), 14 deletions(-)
>  create mode 100644 xen/arch/arm/arm32/domain-build.c
>  create mode 100644 xen/arch/arm/arm64/domain-build.c

Is it really worth to create two more source files just for one
function? Maybe it is better to use already existing
xen/arch/arm/arm*/domain.c ?

>
> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
> index 531168f58a0a..0fd3f5272361 100644
> --- a/xen/arch/arm/arm32/Makefile
> +++ b/xen/arch/arm/arm32/Makefile
> @@ -6,6 +6,7 @@ obj-y += cache.o
>  obj-$(CONFIG_EARLY_PRINTK) += debug.o
>  obj-y += domctl.o
>  obj-y += domain.o
> +obj-y += domain-build.o
>  obj-y += entry.o
>  obj-y += head.o
>  obj-y += insn.o
> diff --git a/xen/arch/arm/arm32/domain-build.c b/xen/arch/arm/arm32/domain-build.c
> new file mode 100644
> index 000000000000..e34261e4a2ad
> --- /dev/null
> +++ b/xen/arch/arm/arm32/domain-build.c
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/fdt-kernel.h>
> +#include <xen/sched.h>
> +
> +#include <asm/domain.h>
> +
> +#ifdef CONFIG_DOM0LESS_BOOT
> +void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
> +{
> +    /* Nothing to do */
> +}
> +#endif
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> index 6491c5350b2e..3272fe7e4ca2 100644
> --- a/xen/arch/arm/arm64/Makefile
> +++ b/xen/arch/arm/arm64/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_HARDEN_BRANCH_PREDICTOR) += bpi.o
>  obj-$(CONFIG_EARLY_PRINTK) += debug.o
>  obj-y += domctl.o
>  obj-y += domain.o
> +obj-y += domain-build.o
>  obj-y += entry.o
>  obj-y += head.o
>  obj-y += insn.o
> diff --git a/xen/arch/arm/arm64/domain-build.c b/xen/arch/arm/arm64/domain-build.c
> new file mode 100644
> index 000000000000..3a89ee46b8c6
> --- /dev/null
> +++ b/xen/arch/arm/arm64/domain-build.c
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/fdt-kernel.h>
> +#include <xen/sched.h>
> +
> +#include <asm/domain.h>
> +
> +#ifdef CONFIG_DOM0LESS_BOOT
> +/* TODO: make arch.type generic ? */
> +void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
> +{
> +    /* type must be set before allocate memory */
> +    d->arch.type = kinfo->arch.type;
> +}
> +#endif
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index c8d07213e247..58f77628df1f 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -236,20 +236,6 @@ int __init make_arch_nodes(struct kernel_info *kinfo)
>      return 0;
>  }
>  
> -/* TODO: make arch.type generic ? */
> -#ifdef CONFIG_ARM_64
> -void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
> -{
> -    /* type must be set before allocate memory */
> -    d->arch.type = kinfo->arch.type;
> -}
> -#else
> -void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
> -{
> -    /* Nothing to do */
> -}
> -#endif
> -
>  int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
>                        const struct dt_device_node *node)
>  {
> diff --git a/xen/include/xen/dom0less-build.h b/xen/include/xen/dom0less-build.h
> index 408859e3255a..3e81d8ba3a47 100644
> --- a/xen/include/xen/dom0less-build.h
> +++ b/xen/include/xen/dom0less-build.h
> @@ -57,6 +57,14 @@ int init_vuart(struct domain *d, struct kernel_info *kinfo,
>  int make_intc_domU_node(struct kernel_info *kinfo);
>  int make_arch_nodes(struct kernel_info *kinfo);
>  
> +/*
> + * Set domain type from struct kernel_info which defines guest Execution
> + * State 32-bit/64-bit (for Arm AArch32/AArch64).
> + * The domain type must be set before allocate_memory.
> + *
> + * @d: pointer to the domain structure.
> + * @kinfo: pointer to the kinfo structure.
> + */
>  void set_domain_type(struct domain *d, struct kernel_info *kinfo);
>  
>  int init_intc_phandle(struct kernel_info *kinfo, const char *name,

-- 
WBR, Volodymyr

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

* Re: [PATCH v2 2/4] xen/arm: split is_32bit/64bit_domain() between arm64/arm32
  2025-08-06  9:49 ` [PATCH v2 2/4] xen/arm: split is_32bit/64bit_domain() " Grygorii Strashko
@ 2025-08-27  0:23   ` Volodymyr Babchuk
  0 siblings, 0 replies; 19+ messages in thread
From: Volodymyr Babchuk @ 2025-08-27  0:23 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel

Grygorii Strashko <grygorii_strashko@epam.com> writes:

> From: Grygorii Strashko <grygorii_strashko@epam.com>
>
> Split is_32bit/64bit_domain() macro implementations between arm64/arm32.
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

> ---
> v2:
> - fix comments related to macro parameters evaluation issues
>
>  xen/arch/arm/include/asm/arm32/domain.h | 20 +++++++++++++++++
>  xen/arch/arm/include/asm/arm64/domain.h | 29 +++++++++++++++++++++++++
>  xen/arch/arm/include/asm/domain.h       |  7 +++---
>  3 files changed, 52 insertions(+), 4 deletions(-)
>  create mode 100644 xen/arch/arm/include/asm/arm32/domain.h
>  create mode 100644 xen/arch/arm/include/asm/arm64/domain.h
>
> diff --git a/xen/arch/arm/include/asm/arm32/domain.h b/xen/arch/arm/include/asm/arm32/domain.h
> new file mode 100644
> index 000000000000..1bd0735c9194
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/arm32/domain.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef ARM_ARM32_DOMAIN_H
> +#define ARM_ARM32_DOMAIN_H
> +
> +/* Arm32 always runs guests in AArch32 mode */
> +
> +#define is_32bit_domain(d) ((void)(d), 1)
> +#define is_64bit_domain(d) ((void)(d), 0)
> +
> +#endif /* ARM_ARM32_DOMAIN_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/include/asm/arm64/domain.h b/xen/arch/arm/include/asm/arm64/domain.h
> new file mode 100644
> index 000000000000..1a2d73950bf0
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/arm64/domain.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef ARM_ARM64_DOMAIN_H
> +#define ARM_ARM64_DOMAIN_H
> +
> +/*
> + * Returns true if guest execution state is AArch32
> + *
> + * @d: pointer to the domain structure
> + */
> +#define is_32bit_domain(d) ((d)->arch.type == DOMAIN_32BIT)
> +
> +/*
> + * Returns true if guest execution state is AArch64
> + *
> + * @d: pointer to the domain structure
> + */
> +#define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
> +
> +#endif /* ARM_ARM64_DOMAIN_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
> index a3487ca71303..dde48178b857 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -22,11 +22,10 @@ enum domain_type {
>      DOMAIN_32BIT,
>      DOMAIN_64BIT,
>  };
> -#define is_32bit_domain(d) ((d)->arch.type == DOMAIN_32BIT)
> -#define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
> +
> +# include <asm/arm64/domain.h>
>  #else
> -#define is_32bit_domain(d) (1)
> -#define is_64bit_domain(d) (0)
> +# include <asm/arm32/domain.h>
>  #endif
>  
>  /*

-- 
WBR, Volodymyr

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

* Re: [PATCH v2 1/4] xen/arm: split set_domain_type() between arm64/arm32
  2025-08-27  0:22   ` Volodymyr Babchuk
@ 2025-09-04 20:09     ` Grygorii Strashko
  2025-09-05 12:10       ` Volodymyr Babchuk
  0 siblings, 1 reply; 19+ messages in thread
From: Grygorii Strashko @ 2025-09-04 20:09 UTC (permalink / raw)
  To: Volodymyr Babchuk
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Andrew Cooper, Anthony PERARD,
	Jan Beulich, Roger Pau Monné



On 27.08.25 03:22, Volodymyr Babchuk wrote:
> Hi,
> 
> Grygorii Strashko <grygorii_strashko@epam.com> writes:
> 
>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>
>> Split set_domain_type() between Arm64/Arm32 sub-arches as
>> set_domain_type() implementation is going to be extended for Arm64.
>>
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>> ---
>> v2:
>> - no changes, rebase
>>
>>   xen/arch/arm/arm32/Makefile       |  1 +
>>   xen/arch/arm/arm32/domain-build.c | 22 ++++++++++++++++++++++
>>   xen/arch/arm/arm64/Makefile       |  1 +
>>   xen/arch/arm/arm64/domain-build.c | 24 ++++++++++++++++++++++++
>>   xen/arch/arm/dom0less-build.c     | 14 --------------
>>   xen/include/xen/dom0less-build.h  |  8 ++++++++
>>   6 files changed, 56 insertions(+), 14 deletions(-)
>>   create mode 100644 xen/arch/arm/arm32/domain-build.c
>>   create mode 100644 xen/arch/arm/arm64/domain-build.c
> 
> Is it really worth to create two more source files just for one
> function? Maybe it is better to use already existing
> xen/arch/arm/arm*/domain.c ?

It seems a common approach used for splitting ARM subarch code.
code from arch/arm/A.c goes in
  -> arch/arm/arm32/A.c
  -> arch/arm/arm64/A.c
(just "-" is used vs "_")

This approach also simplifies any further code split (there ~250 CONFIG_ARM_64/32 ifdefs).

One additional thing, I probably missed, is that "x-build.c" should be placeholder for "__init" code only and
so added as "domain-build.init.o" in Makefile.

> 
>>
>> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
>> index 531168f58a0a..0fd3f5272361 100644
>> --- a/xen/arch/arm/arm32/Makefile
>> +++ b/xen/arch/arm/arm32/Makefile
>> @@ -6,6 +6,7 @@ obj-y += cache.o
>>   obj-$(CONFIG_EARLY_PRINTK) += debug.o
>>   obj-y += domctl.o
>>   obj-y += domain.o
>> +obj-y += domain-build.o
>>   obj-y += entry.o
>>   obj-y += head.o
>>   obj-y += insn.o
>> diff --git a/xen/arch/arm/arm32/domain-build.c b/xen/arch/arm/arm32/domain-build.c
>> new file mode 100644
>> index 000000000000..e34261e4a2ad
>> --- /dev/null
>> +++ b/xen/arch/arm/arm32/domain-build.c
>> @@ -0,0 +1,22 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#include <xen/fdt-kernel.h>
>> +#include <xen/sched.h>
>> +
>> +#include <asm/domain.h>
>> +
>> +#ifdef CONFIG_DOM0LESS_BOOT
>> +void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
>> +{
>> +    /* Nothing to do */
>> +}
>> +#endif
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
>> index 6491c5350b2e..3272fe7e4ca2 100644
>> --- a/xen/arch/arm/arm64/Makefile
>> +++ b/xen/arch/arm/arm64/Makefile
>> @@ -8,6 +8,7 @@ obj-$(CONFIG_HARDEN_BRANCH_PREDICTOR) += bpi.o
>>   obj-$(CONFIG_EARLY_PRINTK) += debug.o
>>   obj-y += domctl.o
>>   obj-y += domain.o
>> +obj-y += domain-build.o
>>   obj-y += entry.o
>>   obj-y += head.o
>>   obj-y += insn.o
>> diff --git a/xen/arch/arm/arm64/domain-build.c b/xen/arch/arm/arm64/domain-build.c
>> new file mode 100644
>> index 000000000000..3a89ee46b8c6
>> --- /dev/null
>> +++ b/xen/arch/arm/arm64/domain-build.c
>> @@ -0,0 +1,24 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#include <xen/fdt-kernel.h>
>> +#include <xen/sched.h>
>> +
>> +#include <asm/domain.h>
>> +
>> +#ifdef CONFIG_DOM0LESS_BOOT
>> +/* TODO: make arch.type generic ? */
>> +void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
>> +{
>> +    /* type must be set before allocate memory */
>> +    d->arch.type = kinfo->arch.type;
>> +}
>> +#endif
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
>> index c8d07213e247..58f77628df1f 100644
>> --- a/xen/arch/arm/dom0less-build.c
>> +++ b/xen/arch/arm/dom0less-build.c
>> @@ -236,20 +236,6 @@ int __init make_arch_nodes(struct kernel_info *kinfo)
>>       return 0;
>>   }
>>   
>> -/* TODO: make arch.type generic ? */
>> -#ifdef CONFIG_ARM_64
>> -void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
>> -{
>> -    /* type must be set before allocate memory */
>> -    d->arch.type = kinfo->arch.type;
>> -}
>> -#else
>> -void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
>> -{
>> -    /* Nothing to do */
>> -}
>> -#endif
>> -
>>   int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
>>                         const struct dt_device_node *node)
>>   {
>> diff --git a/xen/include/xen/dom0less-build.h b/xen/include/xen/dom0less-build.h
>> index 408859e3255a..3e81d8ba3a47 100644
>> --- a/xen/include/xen/dom0less-build.h
>> +++ b/xen/include/xen/dom0less-build.h
>> @@ -57,6 +57,14 @@ int init_vuart(struct domain *d, struct kernel_info *kinfo,
>>   int make_intc_domU_node(struct kernel_info *kinfo);
>>   int make_arch_nodes(struct kernel_info *kinfo);
>>   
>> +/*
>> + * Set domain type from struct kernel_info which defines guest Execution
>> + * State 32-bit/64-bit (for Arm AArch32/AArch64).
>> + * The domain type must be set before allocate_memory.
>> + *
>> + * @d: pointer to the domain structure.
>> + * @kinfo: pointer to the kinfo structure.
>> + */
>>   void set_domain_type(struct domain *d, struct kernel_info *kinfo);
>>   
>>   int init_intc_phandle(struct kernel_info *kinfo, const char *name,
> 

-- 
Best regards,
-grygorii



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

* Re: [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional
  2025-08-27  0:16   ` Volodymyr Babchuk
@ 2025-09-04 20:09     ` Grygorii Strashko
  2025-09-05 12:15       ` Volodymyr Babchuk
  0 siblings, 1 reply; 19+ messages in thread
From: Grygorii Strashko @ 2025-09-04 20:09 UTC (permalink / raw)
  To: Volodymyr Babchuk
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel



On 27.08.25 03:16, Volodymyr Babchuk wrote:
> Hi Grygorii,
> 
> Grygorii Strashko <grygorii_strashko@epam.com> writes:
> 
>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>
>> Now Arm64 AArch32 guest support is always enabled and built-in while not
>> all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this
>> support might not be needed (Arm64 AArch32 is used quite rarely in embedded
>> systems). More over, when focusing on safety certification, AArch32 related
>> code in Xen leaves a gap in terms of coverage that cannot really be
>> justified in words. This leaves two options: either support it (lots of
>> additional testing, requirements and documents would be needed) or compile
>> it out.
>>
>> Hence, this patch introduces basic support to allow make Arm64
>> AArch32 guest support optional. The following changes are introduced:
>>
>> - Introduce Kconfig option CONFIG_ARM64_AARCH32 to allow enable/disable
>>    Arm64 AArch32 guest support (default y)
>>
>> - Introduce is_aarch32_enabled() helper which accounts Arm64 HW capability
>>    and CONFIG_ARM64_AARCH32 setting
>>
>> - Introduce arm64_set_domain_type() to configure Arm64 domain type in
>>    unified way instead of open coding (d)->arch.type, and account
>>    CONFIG_ARM64_AARCH32 configuration.
>>
>> - toolstack: do not advertise "xen-3.0-armv7l " capability if AArch32 is
>>    disabled.
>>
>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>    AArch32 is disabled.
>>
>> - Set Arm64 domain type to DOMAIN_64BIT by default.
>>    - the Arm Xen boot code is handling this case properly already;
>>    - for toolstack case the XEN_DOMCTL_set_address_size hypercall handling
>>      updated to forcibly configure domain type regardless of current domain
>>      type configuration, so toolstack behavior is unchanged.
>>
>> With CONFIG_ARM64_AARCH32=n the Xen will reject AArch32 guests (kernels) if
>> configured by user in the following way:
>> - Xen boot will fail with panic during dom0 or dom0less domains creation
>> - toolstack domain creation will be rejected due to xc_dom_compat_check()
>>    failure.
>>
>> Making Arm64 AArch32 guest support open further possibilities for build
>> optimizations of Arm64 AArch32 guest support code.
>>
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>> ---
>> changes in v2:
>> - use Arm64 "cpu_has_el1_32" in all places to check if HW has AArch32 support
>> - rework Arm64 XEN_DOMCTL_set_address_size hypercall handling to work with any
>>    initial domain type set (32bit or 64 bit)
>> - fix comments related to macro parameters evaluation issues
>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>    AArch32 is disabled
>>
>>   xen/arch/arm/Kconfig                    |  7 ++++
>>   xen/arch/arm/arm64/domain-build.c       | 46 +++++++++++++++++++++++--
>>   xen/arch/arm/arm64/domctl.c             | 16 +++++----
>>   xen/arch/arm/arm64/vsysreg.c            |  9 +++++
>>   xen/arch/arm/domain.c                   |  9 +++++
>>   xen/arch/arm/domain_build.c             | 21 +++--------
>>   xen/arch/arm/include/asm/arm32/domain.h | 12 +++++++
>>   xen/arch/arm/include/asm/arm64/domain.h | 23 +++++++++++++
>>   xen/arch/arm/setup.c                    |  2 +-
>>   9 files changed, 119 insertions(+), 26 deletions(-)
>>
>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>> index a0c816047427..bf6dd73caf73 100644
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -266,6 +266,13 @@ config PCI_PASSTHROUGH
>>   	help
>>   	  This option enables PCI device passthrough
>>   
>> +config ARM64_AARCH32
>> +	bool "AArch32 Guests support on on ARM64 (UNSUPPORTED)" if UNSUPPORTED
> 
> But aarch32 guests are supported... I understand that you wanted to say
> "Disabling aarch32 support is unsupported". But currently this entry
> reads backwards. I think it should be reworded better. But I have no
> idea - how to do this.

I think "(UNSUPPORTED)" can be just dropped. Is it ok?

> 
>> +	depends on ARM_64
>> +	default y
>> +	help
>> +	  This option enables AArch32 Guests on ARM64.
>> +
>>   endmenu
>>   
>>   menu "ARM errata workaround via the alternative framework"
>> diff --git a/xen/arch/arm/arm64/domain-build.c b/xen/arch/arm/arm64/domain-build.c
>> index 3a89ee46b8c6..5951f002e3af 100644
>> --- a/xen/arch/arm/arm64/domain-build.c
>> +++ b/xen/arch/arm/arm64/domain-build.c
>> @@ -4,13 +4,55 @@
>>   #include <xen/sched.h>
>>   
>>   #include <asm/domain.h>
>> +#include <asm/arm64/sve.h>
>> +
>> +int __init arm64_set_domain_type(struct kernel_info *kinfo)
>> +{
>> +    struct domain *d = kinfo->bd.d;
>> +    enum domain_type type;
>> +
>> +    ASSERT(d);
>> +    ASSERT(kinfo);
> 
> I don't think there is a sense in asserting that kinfo != NULL after you
> dereferenced it retrieve "d"

right

> 
>> +
>> +    type = kinfo->arch.type;
>> +
>> +    if ( !is_aarch32_enabled() )
>> +    {
>> +        ASSERT(d->arch.type == DOMAIN_64BIT);
>> +
>> +        if ( type == DOMAIN_32BIT )
>> +        {
>> +            const char *str = "not available";
>> +
>> +            if ( !IS_ENABLED(CONFIG_ARM64_AARCH32) )
>> +                str = "disabled";
>> +            printk("aarch32 guests support is %s\n", str);
> 
> XENLOG_ERROR?

ok

> 
>> +            return -EINVAL;
>> +        }
>> +
>> +        return 0;
>> +    }
>> +
>> +    if ( is_sve_domain(d) && type == DOMAIN_32BIT )
>> +    {
>> +        printk("SVE is not available for 32-bit domain\n");
> 
> XENLOG_ERROR?
> 
>> +        return -EINVAL;
>> +    }
>> +
>> +    d->arch.type = type;
>> +
>> +    return 0;
>> +}
>>   
>>   #ifdef CONFIG_DOM0LESS_BOOT
>>   /* TODO: make arch.type generic ? */
>>   void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
>>   {
>> -    /* type must be set before allocate memory */
>> -    d->arch.type = kinfo->arch.type;
>> +    int rc;
>> +
>> +    rc = arm64_set_domain_type(kinfo);
>> +    if ( rc < 0 )
>> +        panic("Unsupported guest type\n");
>>   }
>>   #endif
>>   
>> diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
>> index 8720d126c97d..4f0f143daef8 100644
>> --- a/xen/arch/arm/arm64/domctl.c
>> +++ b/xen/arch/arm/arm64/domctl.c
>> @@ -13,6 +13,11 @@
>>   #include <asm/arm64/sve.h>
>>   #include <asm/cpufeature.h>
>>   
>> +static void vcpu_switch_to_aarch32_mode(struct vcpu *v)
>> +{
>> +    v->arch.hcr_el2 &= ~HCR_RW;
>> +}
>> +
>>   static long switch_mode(struct domain *d, enum domain_type type)
>>   {
>>       struct vcpu *v;
>> @@ -21,14 +26,14 @@ static long switch_mode(struct domain *d, enum domain_type type)
>>           return -EINVAL;
>>       if ( domain_tot_pages(d) != 0 )
>>           return -EBUSY;
>> -    if ( d->arch.type == type )
>> -        return 0;
>>   
>>       d->arch.type = type;
>>   
>> -    if ( is_64bit_domain(d) )
>> -        for_each_vcpu(d, v)
>> +    for_each_vcpu(d, v)
>> +        if ( is_64bit_domain(d) )
> 
> Do you really need to check domain type for every vCPU? I think that
> original variant was better.
> 
>>               vcpu_switch_to_aarch64_mode(v);
>> +        else
>> +            vcpu_switch_to_aarch32_mode(v);

Do you mean like below?

     if ( is_64bit_domain(d) )
         for_each_vcpu(d, v)
             vcpu_switch_to_aarch64_mode(v);
     else
         for_each_vcpu(d, v)
             vcpu_switch_to_aarch32_mode(v);


>>   
>>       return 0;
>>   }
>> @@ -38,7 +43,7 @@ static long set_address_size(struct domain *d, uint32_t address_size)
>>       switch ( address_size )
>>       {
>>       case 32:
>> -        if ( !cpu_has_el1_32 )
>> +        if ( !is_aarch32_enabled() )
>>               return -EINVAL;
>>           /* SVE is not supported for 32 bit domain */
>>           if ( is_sve_domain(d) )
>> @@ -58,7 +63,6 @@ long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
>>       {
>>       case XEN_DOMCTL_set_address_size:
>>           return set_address_size(d, domctl->u.address_size.size);
>> -
> 
> Stray change?

ok

> 
>>       default:
>>           return -ENOSYS;
>>       }
>> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
>> index d14258290ff0..9f7e735c9b74 100644
>> --- a/xen/arch/arm/arm64/vsysreg.c
>> +++ b/xen/arch/arm/arm64/vsysreg.c
>> @@ -330,6 +330,15 @@ void do_sysreg(struct cpu_user_regs *regs,
>>       {
>>           register_t guest_reg_value = domain_cpuinfo.pfr64.bits[0];
>>   
>> +        if ( !is_aarch32_enabled() )
>> +        {
>> +            /* do not expose EL1 AArch32 support if disabled */
>> +            register_t mask = GENMASK(ID_AA64PFR0_EL1_SHIFT + 4 - 1,
>> +                                      ID_AA64PFR0_EL1_SHIFT);
>> +            guest_reg_value &= ~mask;
>> +            guest_reg_value |= (1 << ID_AA64PFR0_EL1_SHIFT) & mask;
> 
> Why do you need to apply mask here?

will drop

> 
>> +        }
>> +
>>           if ( is_sve_domain(v->domain) )
>>           {
>>               /* 4 is the SVE field width in id_aa64pfr0_el1 */
>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index 310c5789096d..544d1422a793 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -791,6 +791,15 @@ int arch_domain_create(struct domain *d,
>>       d->arch.sve_vl = config->arch.sve_vl;
>>   #endif
>>   
>> +#ifdef CONFIG_ARM_64
>> +    /*
>> +     * Set default Execution State to AArch64 (64bit)
>> +     * - Xen will reconfigure it properly at boot time
>> +     * - toolstack will reconfigure it properly by XEN_DOMCTL_set_address_size
>> +     */
>> +    d->arch.type = DOMAIN_64BIT;
>> +#endif
>> +
>>       return 0;
>>   
>>   fail:
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 47ba920fc6b0..c616127e8da5 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -1873,19 +1873,6 @@ int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
>>       BUG_ON(v->is_initialised);
>>   
>>   #ifdef CONFIG_ARM_64
>> -    /* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
>> -    if ( !(cpu_has_el1_32) && kinfo->arch.type == DOMAIN_32BIT )
>> -    {
>> -        printk("Platform does not support 32-bit domain\n");
>> -        return -EINVAL;
>> -    }
>> -
>> -    if ( is_sve_domain(d) && (kinfo->arch.type == DOMAIN_32BIT) )
>> -    {
>> -        printk("SVE is not available for 32-bit domain\n");
>> -        return -EINVAL;
>> -    }
>> -
>>       if ( is_64bit_domain(d) )
>>           vcpu_switch_to_aarch64_mode(v);
>>   
>> @@ -1983,6 +1970,10 @@ static int __init construct_dom0(struct domain *d)
>>       if ( rc < 0 )
>>           return rc;
>>   
>> +    rc = arm64_set_domain_type(&kinfo);
>> +    if ( rc < 0 )
>> +        return rc;
>> +
>>       return construct_hwdom(&kinfo, NULL);
>>   }
>>   
>> @@ -1994,10 +1985,6 @@ int __init construct_hwdom(struct kernel_info *kinfo,
>>   
>>       iommu_hwdom_init(d);
>>   
>> -#ifdef CONFIG_ARM_64
>> -    /* type must be set before allocate_memory */
>> -    d->arch.type = kinfo->arch.type;
>> -#endif
>>       find_gnttab_region(d, kinfo);
>>       if ( is_domain_direct_mapped(d) )
>>           allocate_memory_11(d, kinfo);
>> diff --git a/xen/arch/arm/include/asm/arm32/domain.h b/xen/arch/arm/include/asm/arm32/domain.h
>> index 1bd0735c9194..30e8818ff2ae 100644
>> --- a/xen/arch/arm/include/asm/arm32/domain.h
>> +++ b/xen/arch/arm/include/asm/arm32/domain.h
>> @@ -3,11 +3,23 @@
>>   #ifndef ARM_ARM32_DOMAIN_H
>>   #define ARM_ARM32_DOMAIN_H
>>   
>> +struct kernel_info;
>> +
>>   /* Arm32 always runs guests in AArch32 mode */
>>   
>>   #define is_32bit_domain(d) ((void)(d), 1)
>>   #define is_64bit_domain(d) ((void)(d), 0)
>>   
>> +static inline bool is_aarch32_enabled(void)
>> +{
>> +    return true;
>> +}
>> +
>> +static inline int arm64_set_domain_type(struct kernel_info *kinfo)
>> +{
>> +    return 0;
>> +}
>> +
>>   #endif /* ARM_ARM32_DOMAIN_H */
>>   
>>   /*
>> diff --git a/xen/arch/arm/include/asm/arm64/domain.h b/xen/arch/arm/include/asm/arm64/domain.h
>> index 1a2d73950bf0..bebcbc582f97 100644
>> --- a/xen/arch/arm/include/asm/arm64/domain.h
>> +++ b/xen/arch/arm/include/asm/arm64/domain.h
>> @@ -3,6 +3,10 @@
>>   #ifndef ARM_ARM64_DOMAIN_H
>>   #define ARM_ARM64_DOMAIN_H
>>   
>> +#include <asm/cpufeature.h>
>> +
>> +struct kernel_info;
>> +
>>   /*
>>    * Returns true if guest execution state is AArch32
>>    *
>> @@ -17,6 +21,25 @@
>>    */
>>   #define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
>>   
>> +/*
>> + * Arm64 declares AArch32 (32bit) Execution State support in the
>> + * Processor Feature Registers (PFR0), but also can be disabled manually.
>> + */
>> +static inline bool is_aarch32_enabled(void)
>> +{
>> +    return IS_ENABLED(CONFIG_ARM64_AARCH32) && cpu_has_el1_32;
>> +}
>> +
>> +/*
>> + * Set domain type from struct kernel_info which defines guest Execution
>> + * State AArch32/AArch64 during regular dom0 or predefined (dom0less)
>> + * domains creation .
> 
> Extra space before full stop

ok

> 
>> + * Type must be set before allocate_memory or create vcpus.
>> + *
>> + * @kinfo: pointer to the kinfo structure.
>> + */
>> +int arm64_set_domain_type(struct kernel_info *kinfo);
>> +
>>   #endif /* ARM_ARM64_DOMAIN_H */
>>   
>>   /*
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index bb35afe56cec..c29573f0ffec 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -530,7 +530,7 @@ static int __init init_xen_cap_info(void)
>>   #ifdef CONFIG_ARM_64
>>       safe_strcat(xen_cap_info, "xen-3.0-aarch64 ");
>>   #endif
>> -    if ( cpu_has_aarch32 )
>> +    if ( is_aarch32_enabled() )
>>           safe_strcat(xen_cap_info, "xen-3.0-armv7l ");
>>   
>>       return 0;
> 

-- 
Best regards,
-grygorii



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

* Re: [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional
  2025-08-06  9:49 ` [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional Grygorii Strashko
  2025-08-27  0:16   ` Volodymyr Babchuk
@ 2025-09-05  3:43   ` Demi Marie Obenour
  2025-09-05  9:35     ` Grygorii Strashko
  2025-09-05  7:04   ` Julien Grall
  2 siblings, 1 reply; 19+ messages in thread
From: Demi Marie Obenour @ 2025-09-05  3:43 UTC (permalink / raw)
  To: Grygorii Strashko, xen-devel@lists.xenproject.org
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk


[-- Attachment #1.1.1: Type: text/plain, Size: 3896 bytes --]

On 8/6/25 05:49, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@epam.com>
> 
> Now Arm64 AArch32 guest support is always enabled and built-in while not
> all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this
> support might not be needed (Arm64 AArch32 is used quite rarely in embedded
> systems). More over, when focusing on safety certification, AArch32 related
> code in Xen leaves a gap in terms of coverage that cannot really be
> justified in words. This leaves two options: either support it (lots of
> additional testing, requirements and documents would be needed) or compile
> it out.
> 
> Hence, this patch introduces basic support to allow make Arm64
> AArch32 guest support optional. The following changes are introduced:
> 
> - Introduce Kconfig option CONFIG_ARM64_AARCH32 to allow enable/disable
>   Arm64 AArch32 guest support (default y)
> 
> - Introduce is_aarch32_enabled() helper which accounts Arm64 HW capability
>   and CONFIG_ARM64_AARCH32 setting
> 
> - Introduce arm64_set_domain_type() to configure Arm64 domain type in
>   unified way instead of open coding (d)->arch.type, and account
>   CONFIG_ARM64_AARCH32 configuration.
> 
> - toolstack: do not advertise "xen-3.0-armv7l " capability if AArch32 is
>   disabled.
> 
> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>   AArch32 is disabled.
> 
> - Set Arm64 domain type to DOMAIN_64BIT by default.
>   - the Arm Xen boot code is handling this case properly already;
>   - for toolstack case the XEN_DOMCTL_set_address_size hypercall handling
>     updated to forcibly configure domain type regardless of current domain
>     type configuration, so toolstack behavior is unchanged.
> 
> With CONFIG_ARM64_AARCH32=n the Xen will reject AArch32 guests (kernels) if
> configured by user in the following way:
> - Xen boot will fail with panic during dom0 or dom0less domains creation
> - toolstack domain creation will be rejected due to xc_dom_compat_check()
>   failure.
> 
> Making Arm64 AArch32 guest support open further possibilities for build
> optimizations of Arm64 AArch32 guest support code.
> 
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
> ---
> changes in v2:
> - use Arm64 "cpu_has_el1_32" in all places to check if HW has AArch32 support
> - rework Arm64 XEN_DOMCTL_set_address_size hypercall handling to work with any
>   initial domain type set (32bit or 64 bit)
> - fix comments related to macro parameters evaluation issues
> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>   AArch32 is disabled
> 
>  xen/arch/arm/Kconfig                    |  7 ++++
>  xen/arch/arm/arm64/domain-build.c       | 46 +++++++++++++++++++++++--
>  xen/arch/arm/arm64/domctl.c             | 16 +++++----
>  xen/arch/arm/arm64/vsysreg.c            |  9 +++++
>  xen/arch/arm/domain.c                   |  9 +++++
>  xen/arch/arm/domain_build.c             | 21 +++--------
>  xen/arch/arm/include/asm/arm32/domain.h | 12 +++++++
>  xen/arch/arm/include/asm/arm64/domain.h | 23 +++++++++++++
>  xen/arch/arm/setup.c                    |  2 +-
>  9 files changed, 119 insertions(+), 26 deletions(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index a0c816047427..bf6dd73caf73 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -266,6 +266,13 @@ config PCI_PASSTHROUGH
>  	help
>  	  This option enables PCI device passthrough
>  
> +config ARM64_AARCH32
> +	bool "AArch32 Guests support on on ARM64 (UNSUPPORTED)" if UNSUPPORTED
> +	depends on ARM_64
> +	default y
> +	help
> +	  This option enables AArch32 Guests on ARM64.
> +
>  endmenu
Why UNSUPPORTED?  A safety-certified build of Xen should only be using
security-supported features.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional
  2025-08-06  9:49 ` [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional Grygorii Strashko
  2025-08-27  0:16   ` Volodymyr Babchuk
  2025-09-05  3:43   ` Demi Marie Obenour
@ 2025-09-05  7:04   ` Julien Grall
  2025-09-05  9:34     ` Grygorii Strashko
  2 siblings, 1 reply; 19+ messages in thread
From: Julien Grall @ 2025-09-05  7:04 UTC (permalink / raw)
  To: Grygorii Strashko, xen-devel@lists.xenproject.org
  Cc: Stefano Stabellini, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk

Hi Grygorii,

On 06/08/2025 10:49, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@epam.com>
> 
> Now Arm64 AArch32 guest support is always enabled and built-in while not
> all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this

typo s/exmaple/example/

> support might not be needed (Arm64 AArch32 is used quite rarely in embedded
> systems). 


 > More over, when focusing on safety certification, AArch32 related> 
code in Xen leaves a gap in terms of coverage that cannot really be
> justified in words. This leaves two options: either support it (lots of
> additional testing, requirements and documents would be needed) or compile
> it out.

To clarify my understanding, what you are removing is support for 32-bit 
EL1. But 32-bit EL0 will still be supported. Is that correct?

[...]

> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>    AArch32 is disabled.

A guest is not allowed to switch bitness. So I am not sure why we need 
to hide EL1. Depending on the answer above, you might need to hide EL0 
and have more code (TBC) to prevent 32-bit EL0 running.

Cheers,

-- 
Julien Grall



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

* Re: [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional
  2025-09-05  7:04   ` Julien Grall
@ 2025-09-05  9:34     ` Grygorii Strashko
  0 siblings, 0 replies; 19+ messages in thread
From: Grygorii Strashko @ 2025-09-05  9:34 UTC (permalink / raw)
  To: Julien Grall, xen-devel@lists.xenproject.org
  Cc: Stefano Stabellini, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk

Hi Julien,

On 05.09.25 10:04, Julien Grall wrote:
> Hi Grygorii,
> 
> On 06/08/2025 10:49, Grygorii Strashko wrote:
>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>
>> Now Arm64 AArch32 guest support is always enabled and built-in while not
>> all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this
> 
> typo s/exmaple/example/

ok

> 
>> support might not be needed (Arm64 AArch32 is used quite rarely in embedded
>> systems). 
> 
> 
>> More over, when focusing on safety certification, AArch32 related
>> code in Xen leaves a gap in terms of coverage that cannot really be
>> justified in words. This leaves two options: either support it (lots of
>> additional testing, requirements and documents would be needed) or compile
>> it out.
> 
> To clarify my understanding, what you are removing is support for 32-bit EL1. But 32-bit EL0 will still be supported. Is that correct?

Yes. The 32-bit EL0 is still supported
(and was tested by creating domain with AArch64 kernel and AArch32 EL0 rootfs).

I will update Kconfig description.

> 
> [...]
> 
>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>    AArch32 is disabled.
> 
> A guest is not allowed to switch bitness. So I am not sure why we need to hide EL1.

I can drop code chunk which changes ID_AA64PFR0_EL1. Right?

> Depending on the answer above, you might need to hide EL0 and have more code (TBC) to prevent 32-bit EL0 running.

EL0 is supported.

Thank you for you comments.

-- 
Best regards,
-grygorii



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

* Re: [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional
  2025-09-05  3:43   ` Demi Marie Obenour
@ 2025-09-05  9:35     ` Grygorii Strashko
  0 siblings, 0 replies; 19+ messages in thread
From: Grygorii Strashko @ 2025-09-05  9:35 UTC (permalink / raw)
  To: Demi Marie Obenour, xen-devel@lists.xenproject.org
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk

Hi Demi,

On 05.09.25 06:43, Demi Marie Obenour wrote:
> On 8/6/25 05:49, Grygorii Strashko wrote:
>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>
>> Now Arm64 AArch32 guest support is always enabled and built-in while not
>> all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this
>> support might not be needed (Arm64 AArch32 is used quite rarely in embedded
>> systems). More over, when focusing on safety certification, AArch32 related
>> code in Xen leaves a gap in terms of coverage that cannot really be
>> justified in words. This leaves two options: either support it (lots of
>> additional testing, requirements and documents would be needed) or compile
>> it out.
>>
>> Hence, this patch introduces basic support to allow make Arm64
>> AArch32 guest support optional. The following changes are introduced:
>>
>> - Introduce Kconfig option CONFIG_ARM64_AARCH32 to allow enable/disable
>>    Arm64 AArch32 guest support (default y)
>>
>> - Introduce is_aarch32_enabled() helper which accounts Arm64 HW capability
>>    and CONFIG_ARM64_AARCH32 setting
>>
>> - Introduce arm64_set_domain_type() to configure Arm64 domain type in
>>    unified way instead of open coding (d)->arch.type, and account
>>    CONFIG_ARM64_AARCH32 configuration.
>>
>> - toolstack: do not advertise "xen-3.0-armv7l " capability if AArch32 is
>>    disabled.
>>
>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>    AArch32 is disabled.
>>
>> - Set Arm64 domain type to DOMAIN_64BIT by default.
>>    - the Arm Xen boot code is handling this case properly already;
>>    - for toolstack case the XEN_DOMCTL_set_address_size hypercall handling
>>      updated to forcibly configure domain type regardless of current domain
>>      type configuration, so toolstack behavior is unchanged.
>>
>> With CONFIG_ARM64_AARCH32=n the Xen will reject AArch32 guests (kernels) if
>> configured by user in the following way:
>> - Xen boot will fail with panic during dom0 or dom0less domains creation
>> - toolstack domain creation will be rejected due to xc_dom_compat_check()
>>    failure.
>>
>> Making Arm64 AArch32 guest support open further possibilities for build
>> optimizations of Arm64 AArch32 guest support code.
>>
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>> ---
>> changes in v2:
>> - use Arm64 "cpu_has_el1_32" in all places to check if HW has AArch32 support
>> - rework Arm64 XEN_DOMCTL_set_address_size hypercall handling to work with any
>>    initial domain type set (32bit or 64 bit)
>> - fix comments related to macro parameters evaluation issues
>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>    AArch32 is disabled
>>
>>   xen/arch/arm/Kconfig                    |  7 ++++
>>   xen/arch/arm/arm64/domain-build.c       | 46 +++++++++++++++++++++++--
>>   xen/arch/arm/arm64/domctl.c             | 16 +++++----
>>   xen/arch/arm/arm64/vsysreg.c            |  9 +++++
>>   xen/arch/arm/domain.c                   |  9 +++++
>>   xen/arch/arm/domain_build.c             | 21 +++--------
>>   xen/arch/arm/include/asm/arm32/domain.h | 12 +++++++
>>   xen/arch/arm/include/asm/arm64/domain.h | 23 +++++++++++++
>>   xen/arch/arm/setup.c                    |  2 +-
>>   9 files changed, 119 insertions(+), 26 deletions(-)
>>
>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>> index a0c816047427..bf6dd73caf73 100644
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -266,6 +266,13 @@ config PCI_PASSTHROUGH
>>   	help
>>   	  This option enables PCI device passthrough
>>   
>> +config ARM64_AARCH32
>> +	bool "AArch32 Guests support on on ARM64 (UNSUPPORTED)" if UNSUPPORTED
>> +	depends on ARM_64
>> +	default y
>> +	help
>> +	  This option enables AArch32 Guests on ARM64.
>> +
>>   endmenu
> Why UNSUPPORTED?  A safety-certified build of Xen should only be using
> security-supported features.

I'd probably introduced a confusion here due to my limited experience with
Xen upstream, sorry for that.

What I've tried to note with "UNSUPPORTED" is that it's new option, which allows to
change default Xen cfg.

After re-checking existing Kconfig files - I think correct way will be:
- change dependency to EXPERT
- remove "UNSUPPORTED"

Will it be ok?


Thank you for you comments.

-- 
Best regards,
-grygorii



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

* Re: [PATCH v2 1/4] xen/arm: split set_domain_type() between arm64/arm32
  2025-09-04 20:09     ` Grygorii Strashko
@ 2025-09-05 12:10       ` Volodymyr Babchuk
  0 siblings, 0 replies; 19+ messages in thread
From: Volodymyr Babchuk @ 2025-09-05 12:10 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Andrew Cooper, Anthony PERARD,
	Jan Beulich, Roger Pau Monné


Hi Grygorii,

Grygorii Strashko <grygorii_strashko@epam.com> writes:

> On 27.08.25 03:22, Volodymyr Babchuk wrote:
>> Hi,
>> Grygorii Strashko <grygorii_strashko@epam.com> writes:
>> 
>>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>>
>>> Split set_domain_type() between Arm64/Arm32 sub-arches as
>>> set_domain_type() implementation is going to be extended for Arm64.
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>>> ---
>>> v2:
>>> - no changes, rebase
>>>
>>>   xen/arch/arm/arm32/Makefile       |  1 +
>>>   xen/arch/arm/arm32/domain-build.c | 22 ++++++++++++++++++++++
>>>   xen/arch/arm/arm64/Makefile       |  1 +
>>>   xen/arch/arm/arm64/domain-build.c | 24 ++++++++++++++++++++++++
>>>   xen/arch/arm/dom0less-build.c     | 14 --------------
>>>   xen/include/xen/dom0less-build.h  |  8 ++++++++
>>>   6 files changed, 56 insertions(+), 14 deletions(-)
>>>   create mode 100644 xen/arch/arm/arm32/domain-build.c
>>>   create mode 100644 xen/arch/arm/arm64/domain-build.c
>> Is it really worth to create two more source files just for one
>> function? Maybe it is better to use already existing
>> xen/arch/arm/arm*/domain.c ?
>
> It seems a common approach used for splitting ARM subarch code.
> code from arch/arm/A.c goes in
>  -> arch/arm/arm32/A.c
>  -> arch/arm/arm64/A.c
> (just "-" is used vs "_")

Yeah, my point was that both arch/arm/arm32/domain.c and
arch/arm/arm64/domain.c already exists, so you don't have to create a
new files. But this is up to you, actually. I'll be fine with either
approach, just wanted to mentioned that there is another way.

[...]

-- 
WBR, Volodymyr

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

* Re: [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional
  2025-09-04 20:09     ` Grygorii Strashko
@ 2025-09-05 12:15       ` Volodymyr Babchuk
  2025-09-05 13:07         ` Julien Grall
  0 siblings, 1 reply; 19+ messages in thread
From: Volodymyr Babchuk @ 2025-09-05 12:15 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel

Hi,

Grygorii Strashko <grygorii_strashko@epam.com> writes:

> On 27.08.25 03:16, Volodymyr Babchuk wrote:
>> Hi Grygorii,
>> Grygorii Strashko <grygorii_strashko@epam.com> writes:
>> 
>>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>>
>>> Now Arm64 AArch32 guest support is always enabled and built-in while not
>>> all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this
>>> support might not be needed (Arm64 AArch32 is used quite rarely in embedded
>>> systems). More over, when focusing on safety certification, AArch32 related
>>> code in Xen leaves a gap in terms of coverage that cannot really be
>>> justified in words. This leaves two options: either support it (lots of
>>> additional testing, requirements and documents would be needed) or compile
>>> it out.
>>>
>>> Hence, this patch introduces basic support to allow make Arm64
>>> AArch32 guest support optional. The following changes are introduced:
>>>
>>> - Introduce Kconfig option CONFIG_ARM64_AARCH32 to allow enable/disable
>>>    Arm64 AArch32 guest support (default y)
>>>
>>> - Introduce is_aarch32_enabled() helper which accounts Arm64 HW capability
>>>    and CONFIG_ARM64_AARCH32 setting
>>>
>>> - Introduce arm64_set_domain_type() to configure Arm64 domain type in
>>>    unified way instead of open coding (d)->arch.type, and account
>>>    CONFIG_ARM64_AARCH32 configuration.
>>>
>>> - toolstack: do not advertise "xen-3.0-armv7l " capability if AArch32 is
>>>    disabled.
>>>
>>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>>    AArch32 is disabled.
>>>
>>> - Set Arm64 domain type to DOMAIN_64BIT by default.
>>>    - the Arm Xen boot code is handling this case properly already;
>>>    - for toolstack case the XEN_DOMCTL_set_address_size hypercall handling
>>>      updated to forcibly configure domain type regardless of current domain
>>>      type configuration, so toolstack behavior is unchanged.
>>>
>>> With CONFIG_ARM64_AARCH32=n the Xen will reject AArch32 guests (kernels) if
>>> configured by user in the following way:
>>> - Xen boot will fail with panic during dom0 or dom0less domains creation
>>> - toolstack domain creation will be rejected due to xc_dom_compat_check()
>>>    failure.
>>>
>>> Making Arm64 AArch32 guest support open further possibilities for build
>>> optimizations of Arm64 AArch32 guest support code.
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>>> ---
>>> changes in v2:
>>> - use Arm64 "cpu_has_el1_32" in all places to check if HW has AArch32 support
>>> - rework Arm64 XEN_DOMCTL_set_address_size hypercall handling to work with any
>>>    initial domain type set (32bit or 64 bit)
>>> - fix comments related to macro parameters evaluation issues
>>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>>    AArch32 is disabled
>>>
>>>   xen/arch/arm/Kconfig                    |  7 ++++
>>>   xen/arch/arm/arm64/domain-build.c       | 46 +++++++++++++++++++++++--
>>>   xen/arch/arm/arm64/domctl.c             | 16 +++++----
>>>   xen/arch/arm/arm64/vsysreg.c            |  9 +++++
>>>   xen/arch/arm/domain.c                   |  9 +++++
>>>   xen/arch/arm/domain_build.c             | 21 +++--------
>>>   xen/arch/arm/include/asm/arm32/domain.h | 12 +++++++
>>>   xen/arch/arm/include/asm/arm64/domain.h | 23 +++++++++++++
>>>   xen/arch/arm/setup.c                    |  2 +-
>>>   9 files changed, 119 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>> index a0c816047427..bf6dd73caf73 100644
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -266,6 +266,13 @@ config PCI_PASSTHROUGH
>>>   	help
>>>   	  This option enables PCI device passthrough
>>>   +config ARM64_AARCH32
>>> +	bool "AArch32 Guests support on on ARM64 (UNSUPPORTED)" if UNSUPPORTED
>> But aarch32 guests are supported... I understand that you wanted to
>> say
>> "Disabling aarch32 support is unsupported". But currently this entry
>> reads backwards. I think it should be reworded better. But I have no
>> idea - how to do this.
>
> I think "(UNSUPPORTED)" can be just dropped. Is it ok?

As I understand, If you want this feature to be eventually certified, it
should not be UNSUPPORTED nor EXPERIMENTAL.


[...]

>>> @@ -21,14 +26,14 @@ static long switch_mode(struct domain *d, enum domain_type type)
>>>           return -EINVAL;
>>>       if ( domain_tot_pages(d) != 0 )
>>>           return -EBUSY;
>>> -    if ( d->arch.type == type )
>>> -        return 0;
>>>         d->arch.type = type;
>>>   -    if ( is_64bit_domain(d) )
>>> -        for_each_vcpu(d, v)
>>> +    for_each_vcpu(d, v)
>>> +        if ( is_64bit_domain(d) )
>> Do you really need to check domain type for every vCPU? I think that
>> original variant was better.
>> 
>>>               vcpu_switch_to_aarch64_mode(v);
>>> +        else
>>> +            vcpu_switch_to_aarch32_mode(v);
>
> Do you mean like below?
>
>     if ( is_64bit_domain(d) )
>         for_each_vcpu(d, v)
>             vcpu_switch_to_aarch64_mode(v);
>     else
>         for_each_vcpu(d, v)
>             vcpu_switch_to_aarch32_mode(v);
>
>

Yep, something like that.

[...]


-- 
WBR, Volodymyr

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

* Re: [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional
  2025-09-05 12:15       ` Volodymyr Babchuk
@ 2025-09-05 13:07         ` Julien Grall
  2025-09-05 18:30           ` Grygorii Strashko
  0 siblings, 1 reply; 19+ messages in thread
From: Julien Grall @ 2025-09-05 13:07 UTC (permalink / raw)
  To: Volodymyr Babchuk, Grygorii Strashko
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini,
	Bertrand Marquis, Michal Orzel



On 05/09/2025 13:15, Volodymyr Babchuk wrote:
> Hi,
> 
> Grygorii Strashko <grygorii_strashko@epam.com> writes:
> 
>> On 27.08.25 03:16, Volodymyr Babchuk wrote:
>>> Hi Grygorii,
>>> Grygorii Strashko <grygorii_strashko@epam.com> writes:
>>>
>>>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>>>
>>>> Now Arm64 AArch32 guest support is always enabled and built-in while not
>>>> all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this
>>>> support might not be needed (Arm64 AArch32 is used quite rarely in embedded
>>>> systems). More over, when focusing on safety certification, AArch32 related
>>>> code in Xen leaves a gap in terms of coverage that cannot really be
>>>> justified in words. This leaves two options: either support it (lots of
>>>> additional testing, requirements and documents would be needed) or compile
>>>> it out.
>>>>
>>>> Hence, this patch introduces basic support to allow make Arm64
>>>> AArch32 guest support optional. The following changes are introduced:
>>>>
>>>> - Introduce Kconfig option CONFIG_ARM64_AARCH32 to allow enable/disable
>>>>     Arm64 AArch32 guest support (default y)
>>>>
>>>> - Introduce is_aarch32_enabled() helper which accounts Arm64 HW capability
>>>>     and CONFIG_ARM64_AARCH32 setting
>>>>
>>>> - Introduce arm64_set_domain_type() to configure Arm64 domain type in
>>>>     unified way instead of open coding (d)->arch.type, and account
>>>>     CONFIG_ARM64_AARCH32 configuration.
>>>>
>>>> - toolstack: do not advertise "xen-3.0-armv7l " capability if AArch32 is
>>>>     disabled.
>>>>
>>>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>>>     AArch32 is disabled.
>>>>
>>>> - Set Arm64 domain type to DOMAIN_64BIT by default.
>>>>     - the Arm Xen boot code is handling this case properly already;
>>>>     - for toolstack case the XEN_DOMCTL_set_address_size hypercall handling
>>>>       updated to forcibly configure domain type regardless of current domain
>>>>       type configuration, so toolstack behavior is unchanged.
>>>>
>>>> With CONFIG_ARM64_AARCH32=n the Xen will reject AArch32 guests (kernels) if
>>>> configured by user in the following way:
>>>> - Xen boot will fail with panic during dom0 or dom0less domains creation
>>>> - toolstack domain creation will be rejected due to xc_dom_compat_check()
>>>>     failure.
>>>>
>>>> Making Arm64 AArch32 guest support open further possibilities for build
>>>> optimizations of Arm64 AArch32 guest support code.
>>>>
>>>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>>>> ---
>>>> changes in v2:
>>>> - use Arm64 "cpu_has_el1_32" in all places to check if HW has AArch32 support
>>>> - rework Arm64 XEN_DOMCTL_set_address_size hypercall handling to work with any
>>>>     initial domain type set (32bit or 64 bit)
>>>> - fix comments related to macro parameters evaluation issues
>>>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>>>     AArch32 is disabled
>>>>
>>>>    xen/arch/arm/Kconfig                    |  7 ++++
>>>>    xen/arch/arm/arm64/domain-build.c       | 46 +++++++++++++++++++++++--
>>>>    xen/arch/arm/arm64/domctl.c             | 16 +++++----
>>>>    xen/arch/arm/arm64/vsysreg.c            |  9 +++++
>>>>    xen/arch/arm/domain.c                   |  9 +++++
>>>>    xen/arch/arm/domain_build.c             | 21 +++--------
>>>>    xen/arch/arm/include/asm/arm32/domain.h | 12 +++++++
>>>>    xen/arch/arm/include/asm/arm64/domain.h | 23 +++++++++++++
>>>>    xen/arch/arm/setup.c                    |  2 +-
>>>>    9 files changed, 119 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>>> index a0c816047427..bf6dd73caf73 100644
>>>> --- a/xen/arch/arm/Kconfig
>>>> +++ b/xen/arch/arm/Kconfig
>>>> @@ -266,6 +266,13 @@ config PCI_PASSTHROUGH
>>>>    	help
>>>>    	  This option enables PCI device passthrough
>>>>    +config ARM64_AARCH32
>>>> +	bool "AArch32 Guests support on on ARM64 (UNSUPPORTED)" if UNSUPPORTED
>>> But aarch32 guests are supported... I understand that you wanted to
>>> say
>>> "Disabling aarch32 support is unsupported". But currently this entry
>>> reads backwards. I think it should be reworded better. But I have no
>>> idea - how to do this.
>>
>> I think "(UNSUPPORTED)" can be just dropped. Is it ok?
> 
> As I understand, If you want this feature to be eventually certified, it
> should not be UNSUPPORTED nor EXPERIMENTAL.

The certification is somewhat irrelevant to the decision of the state of 
the feature. Instead, the decision should be based on the criteria based 
in SUPPORT.MD (see "Status"). If it is experimental/unsupported, then 
what's missing to make it supported?

In addition to that, there is the "EXPERT" mode. This was introduced 
mainly to allow the user to tailor the Kconfig but also limit to what we 
security support. This is to reduce the amount of workload on the 
security team when it comes to decide on whether we need to issue an XSA 
(the more possibility, the more difficult it becomes).

There has been discussion on providing a small set of config (one could 
be for certification purpose) that would be security supported. But I 
don't think we come to a conclusion yet.

Cheers,

-- 
Julien Grall



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

* Re: [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional
  2025-09-05 13:07         ` Julien Grall
@ 2025-09-05 18:30           ` Grygorii Strashko
  0 siblings, 0 replies; 19+ messages in thread
From: Grygorii Strashko @ 2025-09-05 18:30 UTC (permalink / raw)
  To: Julien Grall, Volodymyr Babchuk
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini,
	Bertrand Marquis, Michal Orzel, Oleksandr Tyshchenko

Hi

On 05.09.25 16:07, Julien Grall wrote:
> 
> 
> On 05/09/2025 13:15, Volodymyr Babchuk wrote:
>> Hi,
>>
>> Grygorii Strashko <grygorii_strashko@epam.com> writes:
>>
>>> On 27.08.25 03:16, Volodymyr Babchuk wrote:
>>>> Hi Grygorii,
>>>> Grygorii Strashko <grygorii_strashko@epam.com> writes:
>>>>
>>>>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>>>>
>>>>> Now Arm64 AArch32 guest support is always enabled and built-in while not
>>>>> all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this
>>>>> support might not be needed (Arm64 AArch32 is used quite rarely in embedded
>>>>> systems). More over, when focusing on safety certification, AArch32 related
>>>>> code in Xen leaves a gap in terms of coverage that cannot really be
>>>>> justified in words. This leaves two options: either support it (lots of
>>>>> additional testing, requirements and documents would be needed) or compile
>>>>> it out.
>>>>>
>>>>> Hence, this patch introduces basic support to allow make Arm64
>>>>> AArch32 guest support optional. The following changes are introduced:
>>>>>
>>>>> - Introduce Kconfig option CONFIG_ARM64_AARCH32 to allow enable/disable
>>>>>     Arm64 AArch32 guest support (default y)
>>>>>
>>>>> - Introduce is_aarch32_enabled() helper which accounts Arm64 HW capability
>>>>>     and CONFIG_ARM64_AARCH32 setting
>>>>>
>>>>> - Introduce arm64_set_domain_type() to configure Arm64 domain type in
>>>>>     unified way instead of open coding (d)->arch.type, and account
>>>>>     CONFIG_ARM64_AARCH32 configuration.
>>>>>
>>>>> - toolstack: do not advertise "xen-3.0-armv7l " capability if AArch32 is
>>>>>     disabled.
>>>>>
>>>>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>>>>     AArch32 is disabled.
>>>>>
>>>>> - Set Arm64 domain type to DOMAIN_64BIT by default.
>>>>>     - the Arm Xen boot code is handling this case properly already;
>>>>>     - for toolstack case the XEN_DOMCTL_set_address_size hypercall handling
>>>>>       updated to forcibly configure domain type regardless of current domain
>>>>>       type configuration, so toolstack behavior is unchanged.
>>>>>
>>>>> With CONFIG_ARM64_AARCH32=n the Xen will reject AArch32 guests (kernels) if
>>>>> configured by user in the following way:
>>>>> - Xen boot will fail with panic during dom0 or dom0less domains creation
>>>>> - toolstack domain creation will be rejected due to xc_dom_compat_check()
>>>>>     failure.
>>>>>
>>>>> Making Arm64 AArch32 guest support open further possibilities for build
>>>>> optimizations of Arm64 AArch32 guest support code.
>>>>>
>>>>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>>>>> ---
>>>>> changes in v2:
>>>>> - use Arm64 "cpu_has_el1_32" in all places to check if HW has AArch32 support
>>>>> - rework Arm64 XEN_DOMCTL_set_address_size hypercall handling to work with any
>>>>>     initial domain type set (32bit or 64 bit)
>>>>> - fix comments related to macro parameters evaluation issues
>>>>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>>>>     AArch32 is disabled
>>>>>
>>>>>    xen/arch/arm/Kconfig                    |  7 ++++
>>>>>    xen/arch/arm/arm64/domain-build.c       | 46 +++++++++++++++++++++++--
>>>>>    xen/arch/arm/arm64/domctl.c             | 16 +++++----
>>>>>    xen/arch/arm/arm64/vsysreg.c            |  9 +++++
>>>>>    xen/arch/arm/domain.c                   |  9 +++++
>>>>>    xen/arch/arm/domain_build.c             | 21 +++--------
>>>>>    xen/arch/arm/include/asm/arm32/domain.h | 12 +++++++
>>>>>    xen/arch/arm/include/asm/arm64/domain.h | 23 +++++++++++++
>>>>>    xen/arch/arm/setup.c                    |  2 +-
>>>>>    9 files changed, 119 insertions(+), 26 deletions(-)
>>>>>
>>>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>>>> index a0c816047427..bf6dd73caf73 100644
>>>>> --- a/xen/arch/arm/Kconfig
>>>>> +++ b/xen/arch/arm/Kconfig
>>>>> @@ -266,6 +266,13 @@ config PCI_PASSTHROUGH
>>>>>        help
>>>>>          This option enables PCI device passthrough
>>>>>    +config ARM64_AARCH32
>>>>> +    bool "AArch32 Guests support on on ARM64 (UNSUPPORTED)" if UNSUPPORTED
>>>> But aarch32 guests are supported... I understand that you wanted to
>>>> say
>>>> "Disabling aarch32 support is unsupported". But currently this entry
>>>> reads backwards. I think it should be reworded better. But I have no
>>>> idea - how to do this.
>>>
>>> I think "(UNSUPPORTED)" can be just dropped. Is it ok?
>>
>> As I understand, If you want this feature to be eventually certified, it
>> should not be UNSUPPORTED nor EXPERIMENTAL.
> 
> The certification is somewhat irrelevant to the decision of the state of the feature. 
> Instead, the decision should be based on the criteria based in SUPPORT.MD (see "Status"). 
> If it is experimental/unsupported, then what's missing to make it supported?
> 
> In addition to that, there is the "EXPERT" mode. This was introduced mainly to allow the user
> to tailor the Kconfig but also limit to what we security support. This is to reduce the amount
> of workload on the security team when it comes to decide on whether we need to issue an XSA
> (the more possibility, the more difficult it becomes).

If I understood comments about Kconfig option correctly (which I might not be):
- Not sure if it can be called a "feature" it rather optimization (enabled optionally)
- From my point of view it's basically completed
- The "Arm64 AArch32 guest support" is not mentioned SUPPORT.MD

I'm not sure what support level can be considered for "Arm64 AArch32 guest support"
hence there was no related Kconfig option before.

if treat "Arm64 AArch32 guest support" as : "Supported"
(a) by default ARM64_AARCH32=y, so no changes in Xen default behavior or interfaces
(b) switching to ARM64_AARCH32=n might change status and seems fall under
    "EXPERT and DEBUG Kconfig options are not security supported."

if treat status other than "Supported" - probably no need to depend from EXPERT.

I'm going to change dependency to EXPERT in the next version.

> 
> There has been discussion on providing a small set of config (one could be for certification purpose)
> that would be security supported. But I don't think we come to a conclusion yet.

Thanks a lot for your comments.

-- 
Best regards,
-grygorii



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

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

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-06  9:49 [PATCH v2 0/4] xen/arm64: allow to make aarch32 support optional Grygorii Strashko
2025-08-06  9:49 ` [PATCH v2 1/4] xen/arm: split set_domain_type() between arm64/arm32 Grygorii Strashko
2025-08-27  0:22   ` Volodymyr Babchuk
2025-09-04 20:09     ` Grygorii Strashko
2025-09-05 12:10       ` Volodymyr Babchuk
2025-08-06  9:49 ` [PATCH v2 2/4] xen/arm: split is_32bit/64bit_domain() " Grygorii Strashko
2025-08-27  0:23   ` Volodymyr Babchuk
2025-08-06  9:49 ` [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional Grygorii Strashko
2025-08-27  0:16   ` Volodymyr Babchuk
2025-09-04 20:09     ` Grygorii Strashko
2025-09-05 12:15       ` Volodymyr Babchuk
2025-09-05 13:07         ` Julien Grall
2025-09-05 18:30           ` Grygorii Strashko
2025-09-05  3:43   ` Demi Marie Obenour
2025-09-05  9:35     ` Grygorii Strashko
2025-09-05  7:04   ` Julien Grall
2025-09-05  9:34     ` Grygorii Strashko
2025-08-06  9:49 ` [PATCH v2 4/4] xen/arm64: constify is_32/64bit_domain() macro for CONFIG_ARM64_AARCH32=n Grygorii Strashko
2025-08-27  0:19   ` Volodymyr Babchuk

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.