[PATCH v5 08/10] xen: Split HAS_DEVICE_TREE in two

Alejandro Vallejo posted 10 patches 4 months ago
There is a newer version of this series
[PATCH v5 08/10] xen: Split HAS_DEVICE_TREE in two
Posted by Alejandro Vallejo 4 months ago
Moving forward the idea is for there to be:

  1. Basic DT support: used by dom0less/hyperlaunch.
  2. Full DT support: used for device discovery and HW setup.

Rename HAS_DEVICE_TREE to HAS_DEVICE_TREE_DISCOVERY to describe (2) and
create a new DEVICE_TREE_PARSE to describe (1).

Have DEVICE_TREE_PARSE selected by both HAS_DEVICE_TREE_DISCOVERY and
DOM0LESS_BOOT.

Add a dependency on STATIC_MEMORY for discovery, as it relies on
the memory map itself being described on the DTB.

Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
v5:
  * Replaces the old "xen: Rename CONFIG_HAS_DEVICE_TREE
    to CONFIG_HAS_DEVICE_TREE_DISCOVERY", though it renames
    largely a rename.

One could imagine STATIC_MEMORY being adapted for x86, but it's
no small amount of work, so I shan't.

I didn't add Stefano's R-by because the patch seems functionally
different even if its diff is very similar.
---
 xen/Kconfig.debug                |  2 +-
 xen/arch/arm/Kconfig             |  2 +-
 xen/arch/ppc/Kconfig             |  2 +-
 xen/arch/riscv/Kconfig           |  2 +-
 xen/common/Kconfig               | 13 +++++++++----
 xen/common/Makefile              |  4 ++--
 xen/common/device.c              |  4 ++--
 xen/common/efi/boot.c            |  2 +-
 xen/common/sched/Kconfig         |  2 +-
 xen/drivers/char/ns16550.c       |  6 +++---
 xen/drivers/passthrough/Makefile |  2 +-
 xen/drivers/passthrough/iommu.c  |  2 +-
 xen/include/asm-generic/device.h | 10 +++++-----
 xen/include/xen/iommu.h          |  8 ++++----
 xen/include/xsm/dummy.h          |  4 ++--
 xen/include/xsm/xsm.h            | 12 ++++++------
 xen/xsm/dummy.c                  |  2 +-
 xen/xsm/flask/hooks.c            |  6 +++---
 xen/xsm/xsm_core.c               |  4 ++--
 xen/xsm/xsm_policy.c             |  4 ++--
 20 files changed, 49 insertions(+), 44 deletions(-)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 38a134fa3b..d900d926c5 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -92,7 +92,7 @@ config VERBOSE_DEBUG
 
 config DEVICE_TREE_DEBUG
 	bool "Device tree debug messages"
-	depends on HAS_DEVICE_TREE
+	depends on DEVICE_TREE_PARSE
 	help
 	  Device tree parsing and DOM0 device tree building messages are
 	  logged in the Xen ring buffer.
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 3f25da3ca5..74b16f7167 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -15,7 +15,7 @@ config ARM
 	select FUNCTION_ALIGNMENT_4B
 	select GENERIC_UART_INIT
 	select HAS_ALTERNATIVE if HAS_VMAP
-	select HAS_DEVICE_TREE
+	select HAS_DEVICE_TREE_DISCOVERY
 	select HAS_DOM0LESS
 	select HAS_GRANT_CACHE_FLUSH if GRANT_TABLE
 	select HAS_STACK_PROTECTOR
diff --git a/xen/arch/ppc/Kconfig b/xen/arch/ppc/Kconfig
index 917f5d53a6..5bedf6055e 100644
--- a/xen/arch/ppc/Kconfig
+++ b/xen/arch/ppc/Kconfig
@@ -1,7 +1,7 @@
 config PPC
 	def_bool y
 	select FUNCTION_ALIGNMENT_4B
-	select HAS_DEVICE_TREE
+	select HAS_DEVICE_TREE_DISCOVERY
 	select HAS_UBSAN
 	select HAS_VMAP
 
diff --git a/xen/arch/riscv/Kconfig b/xen/arch/riscv/Kconfig
index 62c5b7ba34..8fd3c314ea 100644
--- a/xen/arch/riscv/Kconfig
+++ b/xen/arch/riscv/Kconfig
@@ -2,7 +2,7 @@ config RISCV
 	def_bool y
 	select FUNCTION_ALIGNMENT_16B
 	select GENERIC_BUG_FRAME
-	select HAS_DEVICE_TREE
+	select HAS_DEVICE_TREE_DISCOVERY
 	select HAS_PMAP
 	select HAS_UBSAN
 	select HAS_VMAP
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 6d784da839..98f6ea8764 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -14,7 +14,8 @@ config CORE_PARKING
 
 config DOM0LESS_BOOT
 	bool "Dom0less boot support" if EXPERT
-	depends on HAS_DOM0LESS && HAS_DEVICE_TREE && DOMAIN_BUILD_HELPERS
+	depends on HAS_DOM0LESS && DOMAIN_BUILD_HELPERS
+	select DEVICE_TREE_PARSE
 	default y
 	help
 	  Dom0less boot support enables Xen to create and start domU guests during
@@ -85,7 +86,11 @@ config HAS_ALTERNATIVE
 config HAS_COMPAT
 	bool
 
-config HAS_DEVICE_TREE
+config HAS_DEVICE_TREE_DISCOVERY
+	bool
+	select DEVICE_TREE_PARSE
+
+config DEVICE_TREE_PARSE
 	bool
 	select LIBFDT
 
@@ -154,7 +159,7 @@ config NUMA
 
 config STATIC_MEMORY
 	bool "Static Allocation Support (UNSUPPORTED)" if UNSUPPORTED
-	depends on DOM0LESS_BOOT
+	depends on DOM0LESS_BOOT && HAS_DEVICE_TREE_DISCOVERY
 	help
 	  Static Allocation refers to system or sub-system(domains) for
 	  which memory areas are pre-defined by configuration using physical
@@ -553,7 +558,7 @@ config DOM0_MEM
 
 config DTB_FILE
 	string "Absolute path to device tree blob"
-	depends on HAS_DEVICE_TREE
+	depends on HAS_DEVICE_TREE_DISCOVERY
 	help
 	  When using a bootloader that has no device tree support or when there
 	  is no bootloader at all, use this option to specify the absolute path
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 98f0873056..d541fbcf49 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -6,9 +6,9 @@ obj-$(CONFIG_HYPFS_CONFIG) += config_data.o
 obj-$(CONFIG_CORE_PARKING) += core_parking.o
 obj-y += cpu.o
 obj-$(CONFIG_DEBUG_TRACE) += debugtrace.o
-obj-$(CONFIG_HAS_DEVICE_TREE) += device.o
+obj-$(CONFIG_HAS_DEVICE_TREE_DISCOVERY) += device.o
 obj-$(filter-out $(CONFIG_X86),$(CONFIG_ACPI)) += device.o
-obj-$(CONFIG_HAS_DEVICE_TREE) += device-tree/
+obj-$(CONFIG_HAS_DEVICE_TREE_DISCOVERY) += device-tree/
 obj-$(CONFIG_IOREQ_SERVER) += dm.o
 obj-y += domain.o
 obj-y += event_2l.o
diff --git a/xen/common/device.c b/xen/common/device.c
index 33e0d58f2f..0c0afad49f 100644
--- a/xen/common/device.c
+++ b/xen/common/device.c
@@ -11,7 +11,7 @@
 
 #include <asm/device.h>
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 
 extern const struct device_desc _sdevice[], _edevice[];
 
@@ -56,7 +56,7 @@ enum device_class device_get_class(const struct dt_device_node *dev)
     return DEVICE_UNKNOWN;
 }
 
-#endif
+#endif /* CONFIG_HAS_DEVICE_TREE_DISCOVERY */
 
 #ifdef CONFIG_ACPI
 
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 9306dc8953..31b4039049 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -623,7 +623,7 @@ static int __init __maybe_unused set_color(uint32_t mask, int bpp,
    return max(*pos + *sz, bpp);
 }
 
-#ifndef CONFIG_HAS_DEVICE_TREE
+#ifndef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 static int __init efi_check_dt_boot(const EFI_LOADED_IMAGE *loaded_image)
 {
     return 0;
diff --git a/xen/common/sched/Kconfig b/xen/common/sched/Kconfig
index 18ca1ce7ab..1fb622e6cf 100644
--- a/xen/common/sched/Kconfig
+++ b/xen/common/sched/Kconfig
@@ -67,7 +67,7 @@ endmenu
 
 config BOOT_TIME_CPUPOOLS
 	bool "Create cpupools at boot time"
-	depends on HAS_DEVICE_TREE
+	depends on HAS_DEVICE_TREE_DISCOVERY
 	help
 	  Creates cpupools during boot time and assigns cpus to them. Cpupools
 	  options can be specified in the device tree.
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 6b4fb4ad31..c1c08b235e 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -34,7 +34,7 @@
 #include <xen/8250-uart.h>
 #include <xen/vmap.h>
 #include <asm/io.h>
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 #include <asm/device.h>
 #endif
 #ifdef CONFIG_X86
@@ -1766,7 +1766,7 @@ void __init ns16550_init(int index, struct ns16550_defaults *defaults)
 
 #endif /* CONFIG_X86 */
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
                                        const void *data)
 {
@@ -1845,7 +1845,7 @@ DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
         .init = ns16550_uart_dt_init,
 DT_DEVICE_END
 
-#endif /* HAS_DEVICE_TREE */
+#endif /* HAS_DEVICE_TREE_DISCOVERY */
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_ARM)
 #include <xen/acpi.h>
diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
index a1621540b7..eb4aeafb42 100644
--- a/xen/drivers/passthrough/Makefile
+++ b/xen/drivers/passthrough/Makefile
@@ -5,6 +5,6 @@ obj-$(CONFIG_ARM) += arm/
 
 obj-y += iommu.o
 obj-$(CONFIG_HAS_PCI) += pci.o
-obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
+obj-$(CONFIG_HAS_DEVICE_TREE_DISCOVERY) += device_tree.o
 obj-$(CONFIG_HAS_PCI) += ats.o
 obj-$(CONFIG_HAS_PCI_MSI) += msi.o
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 16aad86973..c9425d6971 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -638,7 +638,7 @@ int iommu_do_domctl(
     ret = iommu_do_pci_domctl(domctl, d, u_domctl);
 #endif
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
     if ( ret == -ENODEV )
         ret = iommu_do_dt_domctl(domctl, d, u_domctl);
 #endif
diff --git a/xen/include/asm-generic/device.h b/xen/include/asm-generic/device.h
index 1acd1ba1d8..3bd97e33c5 100644
--- a/xen/include/asm-generic/device.h
+++ b/xen/include/asm-generic/device.h
@@ -6,7 +6,7 @@
 
 enum device_type
 {
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
     DEV_DT,
 #endif
     DEV_PCI
@@ -26,7 +26,7 @@ enum device_class
 struct device
 {
     enum device_type type;
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
     struct dt_device_node *of_node; /* Used by drivers imported from Linux */
 #endif
 #ifdef CONFIG_HAS_PASSTHROUGH
@@ -37,7 +37,7 @@ struct device
 
 typedef struct device device_t;
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 
 #include <xen/device_tree.h>
 
@@ -87,9 +87,9 @@ struct device_desc {
     int (*init)(struct dt_device_node *dev, const void *data);
 };
 
-#else /* !CONFIG_HAS_DEVICE_TREE */
+#else /* !CONFIG_HAS_DEVICE_TREE_DISCOVERY */
 #define dev_is_dt(dev) ((void)(dev), false)
-#endif /* CONFIG_HAS_DEVICE_TREE */
+#endif /* CONFIG_HAS_DEVICE_TREE_DISCOVERY */
 
 #define dev_is_pci(dev) ((dev)->type == DEV_PCI)
 
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 832775754b..5483840645 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -230,7 +230,7 @@ struct msi_msg;
 #define PT_IRQ_TIME_OUT MILLISECS(8)
 #endif /* HAS_PCI */
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 #include <xen/device_tree.h>
 
 #ifdef CONFIG_HAS_PASSTHROUGH
@@ -288,7 +288,7 @@ static inline int iommu_release_dt_devices(struct domain *d)
 
 #endif /* HAS_PASSTHROUGH */
 
-#endif /* HAS_DEVICE_TREE */
+#endif /* HAS_DEVICE_TREE_DISCOVERY */
 
 struct page_info;
 
@@ -355,7 +355,7 @@ struct iommu_ops {
     int (*get_reserved_device_memory)(iommu_grdm_t *func, void *ctxt);
     void (*dump_page_tables)(struct domain *d);
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
     /*
      * All IOMMU drivers which support generic IOMMU DT bindings should use
      * this callback. This is a way for the framework to provide the driver
@@ -403,7 +403,7 @@ struct domain_iommu {
     /* iommu_ops */
     const struct iommu_ops *platform_ops;
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
     /* List of DT devices assigned to this domain */
     struct list_head dt_devices;
 #endif
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 9227205fcd..12792c3a43 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -423,7 +423,7 @@ static XSM_INLINE int cf_check xsm_deassign_device(
 
 #endif /* HAS_PASSTHROUGH && HAS_PCI */
 
-#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
+#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE_DISCOVERY)
 static XSM_INLINE int cf_check xsm_assign_dtdevice(
     XSM_DEFAULT_ARG struct domain *d, const char *dtpath)
 {
@@ -438,7 +438,7 @@ static XSM_INLINE int cf_check xsm_deassign_dtdevice(
     return xsm_default_action(action, current->domain, d);
 }
 
-#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE */
+#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE_DISCOVERY */
 
 static XSM_INLINE int cf_check xsm_resource_plug_core(XSM_DEFAULT_VOID)
 {
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 24acc16125..abeb4b04ad 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -125,7 +125,7 @@ struct xsm_ops {
     int (*deassign_device)(struct domain *d, uint32_t machine_bdf);
 #endif
 
-#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
+#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE_DISCOVERY)
     int (*assign_dtdevice)(struct domain *d, const char *dtpath);
     int (*deassign_dtdevice)(struct domain *d, const char *dtpath);
 #endif
@@ -535,7 +535,7 @@ static inline int xsm_deassign_device(
 }
 #endif /* HAS_PASSTHROUGH && HAS_PCI) */
 
-#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
+#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE_DISCOVERY)
 static inline int xsm_assign_dtdevice(
     xsm_default_t def, struct domain *d, const char *dtpath)
 {
@@ -548,7 +548,7 @@ static inline int xsm_deassign_dtdevice(
     return alternative_call(xsm_ops.deassign_dtdevice, d, dtpath);
 }
 
-#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE */
+#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE_DISCOVERY */
 
 static inline int xsm_resource_plug_pci(xsm_default_t def, uint32_t machine_bdf)
 {
@@ -789,7 +789,7 @@ int xsm_multiboot_policy_init(
     struct boot_info *bi, void **policy_buffer, size_t *policy_size);
 #endif
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 /*
  * Initialize XSM
  *
@@ -839,7 +839,7 @@ static inline int xsm_multiboot_init(struct boot_info *bi)
 }
 #endif
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 static inline int xsm_dt_init(void)
 {
     return 0;
@@ -849,7 +849,7 @@ static inline bool has_xsm_magic(paddr_t start)
 {
     return false;
 }
-#endif /* CONFIG_HAS_DEVICE_TREE */
+#endif /* CONFIG_HAS_DEVICE_TREE_DISCOVERY */
 
 #endif /* CONFIG_XSM */
 
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 93fbfc43cc..7f67683839 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -81,7 +81,7 @@ static const struct xsm_ops __initconst_cf_clobber dummy_ops = {
     .deassign_device               = xsm_deassign_device,
 #endif
 
-#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
+#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE_DISCOVERY)
     .assign_dtdevice               = xsm_assign_dtdevice,
     .deassign_dtdevice             = xsm_deassign_dtdevice,
 #endif
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 6a53487ea4..78bad6e56b 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1456,7 +1456,7 @@ static int cf_check flask_deassign_device(
 }
 #endif /* HAS_PASSTHROUGH && HAS_PCI */
 
-#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
+#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE_DISCOVERY)
 static int flask_test_assign_dtdevice(const char *dtpath)
 {
     uint32_t rsid;
@@ -1517,7 +1517,7 @@ static int cf_check flask_deassign_dtdevice(
     return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__REMOVE_DEVICE,
                                 NULL);
 }
-#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE */
+#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE_DISCOVERY */
 
 static int cf_check flask_platform_op(uint32_t op)
 {
@@ -1981,7 +1981,7 @@ static const struct xsm_ops __initconst_cf_clobber flask_ops = {
     .deassign_device = flask_deassign_device,
 #endif
 
-#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
+#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE_DISCOVERY)
     .assign_dtdevice = flask_assign_dtdevice,
     .deassign_dtdevice = flask_deassign_dtdevice,
 #endif
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index f255fb63bf..b7e864a874 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -25,7 +25,7 @@
 #include <asm/setup.h>
 #endif
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 #include <asm/setup.h>
 #endif
 
@@ -166,7 +166,7 @@ int __init xsm_multiboot_init(struct boot_info *bi)
 }
 #endif
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 int __init xsm_dt_init(void)
 {
     int ret = 0;
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index 1b4030edb4..3f04375347 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -24,7 +24,7 @@
 #include <asm/setup.h>
 #endif
 #include <xen/bitops.h>
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 # include <asm/setup.h>
 # include <xen/device_tree.h>
 #endif
@@ -65,7 +65,7 @@ int __init xsm_multiboot_policy_init(
 }
 #endif
 
-#ifdef CONFIG_HAS_DEVICE_TREE
+#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
 int __init xsm_dt_policy_init(void **policy_buffer, size_t *policy_size)
 {
     struct boot_module *mod = boot_module_find_by_kind(BOOTMOD_XSM);
-- 
2.43.0
Re: [PATCH v5 08/10] xen: Split HAS_DEVICE_TREE in two
Posted by Jan Beulich 4 months ago
On 01.07.2025 12:57, Alejandro Vallejo wrote:
> @@ -85,7 +86,11 @@ config HAS_ALTERNATIVE
>  config HAS_COMPAT
>  	bool
>  
> -config HAS_DEVICE_TREE
> +config HAS_DEVICE_TREE_DISCOVERY
> +	bool
> +	select DEVICE_TREE_PARSE
> +
> +config DEVICE_TREE_PARSE
>  	bool
>  	select LIBFDT
>  

We're in the middle of various ([almost] alphabetically sorted) HAS_* here.
Thus DEVICE_TREE_PARSE wants to move elsewhere. Or we want to move back to
naming it HAS_DEVICE_TREE_PARSE, but I think there was a reason why we didn't
want it like that? Just that I don't recall what that reason was ...

> --- a/xen/common/sched/Kconfig
> +++ b/xen/common/sched/Kconfig
> @@ -67,7 +67,7 @@ endmenu
>  
>  config BOOT_TIME_CPUPOOLS
>  	bool "Create cpupools at boot time"
> -	depends on HAS_DEVICE_TREE
> +	depends on HAS_DEVICE_TREE_DISCOVERY

Is this correct? CPU pool creation isn't driven by DT discovery, I thought,
but is a software-only thing much like dom0less?

> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -423,7 +423,7 @@ static XSM_INLINE int cf_check xsm_deassign_device(
>  
>  #endif /* HAS_PASSTHROUGH && HAS_PCI */
>  
> -#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
> +#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE_DISCOVERY)
>  static XSM_INLINE int cf_check xsm_assign_dtdevice(
>      XSM_DEFAULT_ARG struct domain *d, const char *dtpath)
>  {
> @@ -438,7 +438,7 @@ static XSM_INLINE int cf_check xsm_deassign_dtdevice(
>      return xsm_default_action(action, current->domain, d);
>  }
>  
> -#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE */
> +#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE_DISCOVERY */

Here I'm similarly unsure: Pass-through again isn't a platform thing, is it?

> @@ -789,7 +789,7 @@ int xsm_multiboot_policy_init(
>      struct boot_info *bi, void **policy_buffer, size_t *policy_size);
>  #endif
>  
> -#ifdef CONFIG_HAS_DEVICE_TREE
> +#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
>  /*
>   * Initialize XSM
>   *
> @@ -839,7 +839,7 @@ static inline int xsm_multiboot_init(struct boot_info *bi)
>  }
>  #endif
>  
> -#ifdef CONFIG_HAS_DEVICE_TREE
> +#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
>  static inline int xsm_dt_init(void)
>  {
>      return 0;
> @@ -849,7 +849,7 @@ static inline bool has_xsm_magic(paddr_t start)
>  {
>      return false;
>  }
> -#endif /* CONFIG_HAS_DEVICE_TREE */
> +#endif /* CONFIG_HAS_DEVICE_TREE_DISCOVERY */

The situation is yet less clear to me here.

Jan
Re: [PATCH v5 08/10] xen: Split HAS_DEVICE_TREE in two
Posted by Alejandro Vallejo 4 months ago
On Wed Jul 2, 2025 at 3:30 PM CEST, Jan Beulich wrote:
> On 01.07.2025 12:57, Alejandro Vallejo wrote:
>> @@ -85,7 +86,11 @@ config HAS_ALTERNATIVE
>>  config HAS_COMPAT
>>  	bool
>>  
>> -config HAS_DEVICE_TREE
>> +config HAS_DEVICE_TREE_DISCOVERY
>> +	bool
>> +	select DEVICE_TREE_PARSE
>> +
>> +config DEVICE_TREE_PARSE
>>  	bool
>>  	select LIBFDT
>>  
>
> We're in the middle of various ([almost] alphabetically sorted) HAS_* here.
> Thus DEVICE_TREE_PARSE wants to move elsewhere. Or we want to move back to
> naming it HAS_DEVICE_TREE_PARSE, but I think there was a reason why we didn't
> want it like that? Just that I don't recall what that reason was ...

AIUI, HAS_X are options selected by your architecture. Things that tell you
whether X is supported in your arch or not. In this case, HAS_DEVICE_TREE_PARSE
would be supported everywhere, while DEVICE_TREE_PARSE is not an arch property,
but an option selected by DOM0LESS_BOOT (which appears in menuconfig) and
HAS_DEVICE_TREE_DISCOVERY.

I can move it elsewhere, but it's unfortunate to separate two so closely
related options.

>
>> --- a/xen/common/sched/Kconfig
>> +++ b/xen/common/sched/Kconfig
>> @@ -67,7 +67,7 @@ endmenu
>>  
>>  config BOOT_TIME_CPUPOOLS
>>  	bool "Create cpupools at boot time"
>> -	depends on HAS_DEVICE_TREE
>> +	depends on HAS_DEVICE_TREE_DISCOVERY
>
> Is this correct? CPU pool creation isn't driven by DT discovery, I thought,
> but is a software-only thing much like dom0less?

In principle it would be possible. But I haven't tested the configuration, so
I'd rather err on the side of caution and not enable features prematurely.

In time, this should become DOM0LESS_BOOT || HAS_DEVICE_TREE_DISCOVERY.

>
>> --- a/xen/include/xsm/dummy.h
>> +++ b/xen/include/xsm/dummy.h
>> @@ -423,7 +423,7 @@ static XSM_INLINE int cf_check xsm_deassign_device(
>>  
>>  #endif /* HAS_PASSTHROUGH && HAS_PCI */
>>  
>> -#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
>> +#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE_DISCOVERY)
>>  static XSM_INLINE int cf_check xsm_assign_dtdevice(
>>      XSM_DEFAULT_ARG struct domain *d, const char *dtpath)
>>  {
>> @@ -438,7 +438,7 @@ static XSM_INLINE int cf_check xsm_deassign_dtdevice(
>>      return xsm_default_action(action, current->domain, d);
>>  }
>>  
>> -#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE */
>> +#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE_DISCOVERY */
>
> Here I'm similarly unsure: Pass-through again isn't a platform thing, is it?

This has to do strictly with passthrough of devices discovered via DT.

>
>> @@ -789,7 +789,7 @@ int xsm_multiboot_policy_init(
>>      struct boot_info *bi, void **policy_buffer, size_t *policy_size);
>>  #endif
>>  
>> -#ifdef CONFIG_HAS_DEVICE_TREE
>> +#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
>>  /*
>>   * Initialize XSM
>>   *
>> @@ -839,7 +839,7 @@ static inline int xsm_multiboot_init(struct boot_info *bi)
>>  }
>>  #endif
>>  
>> -#ifdef CONFIG_HAS_DEVICE_TREE
>> +#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
>>  static inline int xsm_dt_init(void)
>>  {
>>      return 0;
>> @@ -849,7 +849,7 @@ static inline bool has_xsm_magic(paddr_t start)
>>  {
>>      return false;
>>  }
>> -#endif /* CONFIG_HAS_DEVICE_TREE */
>> +#endif /* CONFIG_HAS_DEVICE_TREE_DISCOVERY */
>
> The situation is yet less clear to me here

XSM segregates multibooting and DT-booting, this refers strictly to the latter.

By DT-booting I mean platforms where the DT is given by the platform rather
than the user as a module.

Cheers,
Alejandro
Re: [PATCH v5 08/10] xen: Split HAS_DEVICE_TREE in two
Posted by Jan Beulich 3 months, 4 weeks ago
On 02.07.2025 17:28, Alejandro Vallejo wrote:
> On Wed Jul 2, 2025 at 3:30 PM CEST, Jan Beulich wrote:
>> On 01.07.2025 12:57, Alejandro Vallejo wrote:
>>> @@ -85,7 +86,11 @@ config HAS_ALTERNATIVE
>>>  config HAS_COMPAT
>>>  	bool
>>>  
>>> -config HAS_DEVICE_TREE
>>> +config HAS_DEVICE_TREE_DISCOVERY
>>> +	bool
>>> +	select DEVICE_TREE_PARSE
>>> +
>>> +config DEVICE_TREE_PARSE
>>>  	bool
>>>  	select LIBFDT
>>>  
>>
>> We're in the middle of various ([almost] alphabetically sorted) HAS_* here.
>> Thus DEVICE_TREE_PARSE wants to move elsewhere. Or we want to move back to
>> naming it HAS_DEVICE_TREE_PARSE, but I think there was a reason why we didn't
>> want it like that? Just that I don't recall what that reason was ...
> 
> AIUI, HAS_X are options selected by your architecture. Things that tell you
> whether X is supported in your arch or not. In this case, HAS_DEVICE_TREE_PARSE
> would be supported everywhere, while DEVICE_TREE_PARSE is not an arch property,
> but an option selected by DOM0LESS_BOOT (which appears in menuconfig) and
> HAS_DEVICE_TREE_DISCOVERY.

It's on the edge here, I agree. Yet we have other cases where one HAS_* selects
another HAS_*, and I think we're in the process of gaining more.

> I can move it elsewhere, but it's unfortunate to separate two so closely
> related options.

Imo there's only one of two options - move it or rename it.

>>> --- a/xen/common/sched/Kconfig
>>> +++ b/xen/common/sched/Kconfig
>>> @@ -67,7 +67,7 @@ endmenu
>>>  
>>>  config BOOT_TIME_CPUPOOLS
>>>  	bool "Create cpupools at boot time"
>>> -	depends on HAS_DEVICE_TREE
>>> +	depends on HAS_DEVICE_TREE_DISCOVERY
>>
>> Is this correct? CPU pool creation isn't driven by DT discovery, I thought,
>> but is a software-only thing much like dom0less?
> 
> In principle it would be possible. But I haven't tested the configuration, so
> I'd rather err on the side of caution and not enable features prematurely.
> 
> In time, this should become DOM0LESS_BOOT || HAS_DEVICE_TREE_DISCOVERY.

DEVICE_TREE_PARSE, that is?

>>> --- a/xen/include/xsm/dummy.h
>>> +++ b/xen/include/xsm/dummy.h
>>> @@ -423,7 +423,7 @@ static XSM_INLINE int cf_check xsm_deassign_device(
>>>  
>>>  #endif /* HAS_PASSTHROUGH && HAS_PCI */
>>>  
>>> -#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
>>> +#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE_DISCOVERY)
>>>  static XSM_INLINE int cf_check xsm_assign_dtdevice(
>>>      XSM_DEFAULT_ARG struct domain *d, const char *dtpath)
>>>  {
>>> @@ -438,7 +438,7 @@ static XSM_INLINE int cf_check xsm_deassign_dtdevice(
>>>      return xsm_default_action(action, current->domain, d);
>>>  }
>>>  
>>> -#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE */
>>> +#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE_DISCOVERY */
>>
>> Here I'm similarly unsure: Pass-through again isn't a platform thing, is it?
> 
> This has to do strictly with passthrough of devices discovered via DT.

Wait, no. You discover devices via DT, but you don't "discover" what domain
to pass them through. For the latter, DT is again only a vehicle.

>>> @@ -789,7 +789,7 @@ int xsm_multiboot_policy_init(
>>>      struct boot_info *bi, void **policy_buffer, size_t *policy_size);
>>>  #endif
>>>  
>>> -#ifdef CONFIG_HAS_DEVICE_TREE
>>> +#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
>>>  /*
>>>   * Initialize XSM
>>>   *
>>> @@ -839,7 +839,7 @@ static inline int xsm_multiboot_init(struct boot_info *bi)
>>>  }
>>>  #endif
>>>  
>>> -#ifdef CONFIG_HAS_DEVICE_TREE
>>> +#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
>>>  static inline int xsm_dt_init(void)
>>>  {
>>>      return 0;
>>> @@ -849,7 +849,7 @@ static inline bool has_xsm_magic(paddr_t start)
>>>  {
>>>      return false;
>>>  }
>>> -#endif /* CONFIG_HAS_DEVICE_TREE */
>>> +#endif /* CONFIG_HAS_DEVICE_TREE_DISCOVERY */
>>
>> The situation is yet less clear to me here
> 
> XSM segregates multibooting and DT-booting, this refers strictly to the latter.
> 
> By DT-booting I mean platforms where the DT is given by the platform rather
> than the user as a module.

Yet the platform as such hardly even knows of XSM (or in fact any of the software
that's going to run there).

I think we need DT maintainer input here.

Jan
Re: [PATCH v5 08/10] xen: Split HAS_DEVICE_TREE in two
Posted by Alejandro Vallejo 3 months, 4 weeks ago
On Thu Jul 3, 2025 at 7:55 AM CEST, Jan Beulich wrote:
> On 02.07.2025 17:28, Alejandro Vallejo wrote:
>> On Wed Jul 2, 2025 at 3:30 PM CEST, Jan Beulich wrote:
>>> On 01.07.2025 12:57, Alejandro Vallejo wrote:
>>>> @@ -85,7 +86,11 @@ config HAS_ALTERNATIVE
>>>>  config HAS_COMPAT
>>>>  	bool
>>>>  
>>>> -config HAS_DEVICE_TREE
>>>> +config HAS_DEVICE_TREE_DISCOVERY
>>>> +	bool
>>>> +	select DEVICE_TREE_PARSE
>>>> +
>>>> +config DEVICE_TREE_PARSE
>>>>  	bool
>>>>  	select LIBFDT
>>>>  
>>>
>>> We're in the middle of various ([almost] alphabetically sorted) HAS_* here.
>>> Thus DEVICE_TREE_PARSE wants to move elsewhere. Or we want to move back to
>>> naming it HAS_DEVICE_TREE_PARSE, but I think there was a reason why we didn't
>>> want it like that? Just that I don't recall what that reason was ...
>> 
>> AIUI, HAS_X are options selected by your architecture. Things that tell you
>> whether X is supported in your arch or not. In this case, HAS_DEVICE_TREE_PARSE
>> would be supported everywhere, while DEVICE_TREE_PARSE is not an arch property,
>> but an option selected by DOM0LESS_BOOT (which appears in menuconfig) and
>> HAS_DEVICE_TREE_DISCOVERY.
>
> It's on the edge here, I agree. Yet we have other cases where one HAS_* selects
> another HAS_*, and I think we're in the process of gaining more.

HAS_* selecting another HAS_* makes sense to me, but that's not what would be
going on here. On x86:

    HAS_DOM0LESS => DOM0LESS_BOOT[y] => HAS_DEVICE_TREE_PARSE

thus leading to HAS_DEVICE_TREE_PARSE being indirectly selectable via
DOM0LESS_BOOT, which is hardly a HAS_* relationship.

>
>> I can move it elsewhere, but it's unfortunate to separate two so closely
>> related options.
>
> Imo there's only one of two options - move it or rename it.

I'll just move it elsewhere then.

>
>>>> --- a/xen/common/sched/Kconfig
>>>> +++ b/xen/common/sched/Kconfig
>>>> @@ -67,7 +67,7 @@ endmenu
>>>>  
>>>>  config BOOT_TIME_CPUPOOLS
>>>>  	bool "Create cpupools at boot time"
>>>> -	depends on HAS_DEVICE_TREE
>>>> +	depends on HAS_DEVICE_TREE_DISCOVERY
>>>
>>> Is this correct? CPU pool creation isn't driven by DT discovery, I thought,
>>> but is a software-only thing much like dom0less?
>> 
>> In principle it would be possible. But I haven't tested the configuration, so
>> I'd rather err on the side of caution and not enable features prematurely.
>> 
>> In time, this should become DOM0LESS_BOOT || HAS_DEVICE_TREE_DISCOVERY.
>
> DEVICE_TREE_PARSE, that is?

Yes :)

Cheers,
Alejandro