* [PATCH v2 01/15] xen: Clean up asm-generic/device.h
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
@ 2025-06-05 19:47 ` Alejandro Vallejo
2025-06-06 6:51 ` Jan Beulich
2025-06-05 19:47 ` [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h Alejandro Vallejo
` (13 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:47 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Alistair Francis, Bob Eshleman, Connor Davis,
Oleksii Kurochko, Andrew Cooper, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Roger Pau Monné,
Stefano Stabellini, Daniel P. Smith
There's some pretense this header may be used without
CONFIG_HAS_DEVICE_TREE, but that's just wishful thinking. Only x86 lacks
that option, and it fully overrides this header, typedeffing struct
pci_dev to be device_t.
Furthermore there's an include for xen/device_tree.h halfway through the
header, but that header already includes asm/device.h, creating a cycle.
Clean up the header removing ifdef guards, merging the typedef onto the
struct definition for device_t and removing the spurious include.
The only affected file is aplic.c, in riscv, which is forced now to
include device_tree.h directly.
Not a functional change.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
v2:
* Add new comment and ifdef guard with #error
---
xen/arch/riscv/aplic.c | 3 ++-
xen/include/asm-generic/device.h | 26 ++++++++++----------------
2 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/xen/arch/riscv/aplic.c b/xen/arch/riscv/aplic.c
index 10ae81f7ac..dd7a274c52 100644
--- a/xen/arch/riscv/aplic.c
+++ b/xen/arch/riscv/aplic.c
@@ -9,13 +9,14 @@
* Copyright (c) 2024-2025 Vates
*/
+#include <xen/device_tree.h>
#include <xen/errno.h>
#include <xen/init.h>
+#include <xen/lib.h>
#include <xen/irq.h>
#include <xen/sections.h>
#include <xen/types.h>
-#include <asm/device.h>
#include <asm/intc.h>
static struct intc_info __ro_after_init aplic_info = {
diff --git a/xen/include/asm-generic/device.h b/xen/include/asm-generic/device.h
index 1acd1ba1d8..631dab046a 100644
--- a/xen/include/asm-generic/device.h
+++ b/xen/include/asm-generic/device.h
@@ -1,14 +1,20 @@
/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * This header helps DTB-based architectures abstract away where a particular
+ * device came from, be it the DTB itself or enumerated on a PCI bus.
+ */
#ifndef __ASM_GENERIC_DEVICE_H__
#define __ASM_GENERIC_DEVICE_H__
+#ifndef CONFIG_HAS_DEVICE_TREE
+#error "Header for exclusive use of DTB-based architectures"
+#endif
+
#include <xen/stdbool.h>
enum device_type
{
-#ifdef CONFIG_HAS_DEVICE_TREE
DEV_DT,
-#endif
DEV_PCI
};
@@ -23,23 +29,15 @@ enum device_class
};
/* struct device - The basic device structure */
-struct device
+typedef struct device
{
enum device_type type;
-#ifdef CONFIG_HAS_DEVICE_TREE
struct dt_device_node *of_node; /* Used by drivers imported from Linux */
-#endif
#ifdef CONFIG_HAS_PASSTHROUGH
void *iommu; /* IOMMU private data */;
struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU instance data */
#endif
-};
-
-typedef struct device device_t;
-
-#ifdef CONFIG_HAS_DEVICE_TREE
-
-#include <xen/device_tree.h>
+} device_t;
#define dev_is_dt(dev) ((dev)->type == DEV_DT)
@@ -87,10 +85,6 @@ struct device_desc {
int (*init)(struct dt_device_node *dev, const void *data);
};
-#else /* !CONFIG_HAS_DEVICE_TREE */
-#define dev_is_dt(dev) ((void)(dev), false)
-#endif /* CONFIG_HAS_DEVICE_TREE */
-
#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
#ifdef CONFIG_ACPI
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v2 01/15] xen: Clean up asm-generic/device.h
2025-06-05 19:47 ` [PATCH v2 01/15] xen: Clean up asm-generic/device.h Alejandro Vallejo
@ 2025-06-06 6:51 ` Jan Beulich
2025-06-06 9:55 ` Alejandro Vallejo
0 siblings, 1 reply; 39+ messages in thread
From: Jan Beulich @ 2025-06-06 6:51 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko,
Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Roger Pau Monné, Stefano Stabellini, Daniel P. Smith,
xen-devel
On 05.06.2025 21:47, Alejandro Vallejo wrote:
> --- a/xen/include/asm-generic/device.h
> +++ b/xen/include/asm-generic/device.h
> @@ -1,14 +1,20 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * This header helps DTB-based architectures abstract away where a particular
> + * device came from, be it the DTB itself or enumerated on a PCI bus.
> + */
> #ifndef __ASM_GENERIC_DEVICE_H__
> #define __ASM_GENERIC_DEVICE_H__
>
> +#ifndef CONFIG_HAS_DEVICE_TREE
> +#error "Header for exclusive use of DTB-based architectures"
> +#endif
> +
> #include <xen/stdbool.h>
>
> enum device_type
> {
> -#ifdef CONFIG_HAS_DEVICE_TREE
> DEV_DT,
> -#endif
> DEV_PCI
> };
My objection to these changes remains; as a generic header it ought to be what
that attribute says - generic.
Jan
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v2 01/15] xen: Clean up asm-generic/device.h
2025-06-06 6:51 ` Jan Beulich
@ 2025-06-06 9:55 ` Alejandro Vallejo
2025-06-06 10:03 ` Jan Beulich
0 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-06 9:55 UTC (permalink / raw)
To: Jan Beulich
Cc: Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko,
Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Roger Pau Monné, Stefano Stabellini, Daniel P. Smith,
xen-devel
On Fri Jun 6, 2025 at 8:51 AM CEST, Jan Beulich wrote:
> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>> --- a/xen/include/asm-generic/device.h
>> +++ b/xen/include/asm-generic/device.h
>> @@ -1,14 +1,20 @@
>> /* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * This header helps DTB-based architectures abstract away where a particular
>> + * device came from, be it the DTB itself or enumerated on a PCI bus.
>> + */
>> #ifndef __ASM_GENERIC_DEVICE_H__
>> #define __ASM_GENERIC_DEVICE_H__
>>
>> +#ifndef CONFIG_HAS_DEVICE_TREE
>> +#error "Header for exclusive use of DTB-based architectures"
>> +#endif
>> +
>> #include <xen/stdbool.h>
>>
>> enum device_type
>> {
>> -#ifdef CONFIG_HAS_DEVICE_TREE
>> DEV_DT,
>> -#endif
>> DEV_PCI
>> };
>
> My objection to these changes remains; as a generic header it ought to be what
> that attribute says - generic.
>
> Jan
It is generic for any architecture where platform DTs exist (that is, anything
but x86).
As the commit message states, these guards are useless, provide no functionality
and create the fiction that somehow this header is still relevant on an
architecture where only PCI is available. And that's just not true. x86 being
the sole architecture without DTs actively overrides it, and relies on device_t
(defined as struct device here) to be a "struct pci_dev" instead in
x86/include/asm/device.h, with dev_to_pci() and pci_to_dev() being irrelevant
because device_t* and struct pci_dev* are identical types in x86. Removing that
override header is not just a matter of performance. All the IOMMU ops are
referencing device_t, while the drivers are assuming pci_dev, so all IOMMU
code breaks immediately when x86 tries to use this.
To be perfectly clear, this patch isn't strictly required to do DT unflattening
on x86. But it's a piece of arm tech debt that Xen is better off without.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v2 01/15] xen: Clean up asm-generic/device.h
2025-06-06 9:55 ` Alejandro Vallejo
@ 2025-06-06 10:03 ` Jan Beulich
2025-06-06 10:21 ` Alejandro Vallejo
0 siblings, 1 reply; 39+ messages in thread
From: Jan Beulich @ 2025-06-06 10:03 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko,
Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Roger Pau Monné, Stefano Stabellini, Daniel P. Smith,
xen-devel
On 06.06.2025 11:55, Alejandro Vallejo wrote:
> On Fri Jun 6, 2025 at 8:51 AM CEST, Jan Beulich wrote:
>> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>>> --- a/xen/include/asm-generic/device.h
>>> +++ b/xen/include/asm-generic/device.h
>>> @@ -1,14 +1,20 @@
>>> /* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * This header helps DTB-based architectures abstract away where a particular
>>> + * device came from, be it the DTB itself or enumerated on a PCI bus.
>>> + */
>>> #ifndef __ASM_GENERIC_DEVICE_H__
>>> #define __ASM_GENERIC_DEVICE_H__
>>>
>>> +#ifndef CONFIG_HAS_DEVICE_TREE
>>> +#error "Header for exclusive use of DTB-based architectures"
>>> +#endif
>>> +
>>> #include <xen/stdbool.h>
>>>
>>> enum device_type
>>> {
>>> -#ifdef CONFIG_HAS_DEVICE_TREE
>>> DEV_DT,
>>> -#endif
>>> DEV_PCI
>>> };
>>
>> My objection to these changes remains; as a generic header it ought to be what
>> that attribute says - generic.
>
> It is generic for any architecture where platform DTs exist (that is, anything
> but x86).
Here you're limiting things to what Xen presently "knows". I'm sure there are
other architectures where DT is entirely unknown.
Furthermore isn't the work here part of the hyperlaunch effort, where DT will
be introduced to x86? Hence "anything but" isn't quite right either then.
Jan
> As the commit message states, these guards are useless, provide no functionality
> and create the fiction that somehow this header is still relevant on an
> architecture where only PCI is available. And that's just not true. x86 being
> the sole architecture without DTs actively overrides it, and relies on device_t
> (defined as struct device here) to be a "struct pci_dev" instead in
> x86/include/asm/device.h, with dev_to_pci() and pci_to_dev() being irrelevant
> because device_t* and struct pci_dev* are identical types in x86. Removing that
> override header is not just a matter of performance. All the IOMMU ops are
> referencing device_t, while the drivers are assuming pci_dev, so all IOMMU
> code breaks immediately when x86 tries to use this.
>
> To be perfectly clear, this patch isn't strictly required to do DT unflattening
> on x86. But it's a piece of arm tech debt that Xen is better off without.
>
> Cheers,
> Alejandro
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v2 01/15] xen: Clean up asm-generic/device.h
2025-06-06 10:03 ` Jan Beulich
@ 2025-06-06 10:21 ` Alejandro Vallejo
0 siblings, 0 replies; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-06 10:21 UTC (permalink / raw)
To: Jan Beulich
Cc: Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko,
Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Roger Pau Monné, Stefano Stabellini, Daniel P. Smith,
xen-devel
On Fri Jun 6, 2025 at 12:03 PM CEST, Jan Beulich wrote:
> On 06.06.2025 11:55, Alejandro Vallejo wrote:
>> On Fri Jun 6, 2025 at 8:51 AM CEST, Jan Beulich wrote:
>>> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>>>> --- a/xen/include/asm-generic/device.h
>>>> +++ b/xen/include/asm-generic/device.h
>>>> @@ -1,14 +1,20 @@
>>>> /* SPDX-License-Identifier: GPL-2.0-only */
>>>> +/*
>>>> + * This header helps DTB-based architectures abstract away where a particular
>>>> + * device came from, be it the DTB itself or enumerated on a PCI bus.
>>>> + */
>>>> #ifndef __ASM_GENERIC_DEVICE_H__
>>>> #define __ASM_GENERIC_DEVICE_H__
>>>>
>>>> +#ifndef CONFIG_HAS_DEVICE_TREE
>>>> +#error "Header for exclusive use of DTB-based architectures"
>>>> +#endif
>>>> +
>>>> #include <xen/stdbool.h>
>>>>
>>>> enum device_type
>>>> {
>>>> -#ifdef CONFIG_HAS_DEVICE_TREE
>>>> DEV_DT,
>>>> -#endif
>>>> DEV_PCI
>>>> };
>>>
>>> My objection to these changes remains; as a generic header it ought to be what
>>> that attribute says - generic.
>>
>> It is generic for any architecture where platform DTs exist (that is, anything
>> but x86).
>
> Here you're limiting things to what Xen presently "knows". I'm sure there are
> other architectures where DT is entirely unknown.
You'll struggle a lot to find one such arch unless you want to revive the
itanium port, and even then itanium would simply re-use x86's asm override.
>
> Furthermore isn't the work here part of the hyperlaunch effort, where DT will
> be introduced to x86? Hence "anything but" isn't quite right either then.
>
> Jan
Not a general DTB, no. A DTB defines the platform (in the same vein as a ACPI/
DSDT), with all platform devices enumerated there. x86 will not get any of that.
Without devices defined in the DTB, my original point still stands.
In general, if you don't have DT devices you typedef device_t to pci_dev to
avoid these wrappers altogether, and if you do you don't and the ifdef guards
stop having meaning.
If you remain unconvinced and others don't pitch in I'll just drop it from v3.
As I said, it's not essential.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
2025-06-05 19:47 ` [PATCH v2 01/15] xen: Clean up asm-generic/device.h Alejandro Vallejo
@ 2025-06-05 19:47 ` Alejandro Vallejo
2025-06-06 6:52 ` Jan Beulich
2025-06-05 19:47 ` [PATCH v2 03/15] xen/dt: Add BOOTMOD_MICROCODE Alejandro Vallejo
` (12 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:47 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
Anthony PERARD, Jan Beulich, Roger Pau Monné,
Daniel P. Smith
Not a functional change
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
v2:
* Remove both links rather than just one.
---
xen/arch/arm/include/asm/grant_table.h | 1 -
xen/common/device-tree/dom0less-build.c | 1 +
xen/common/grant_table.c | 2 ++
xen/include/xen/grant_table.h | 4 ----
4 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/include/asm/grant_table.h b/xen/arch/arm/include/asm/grant_table.h
index c5d87b60c4..c47058a3a0 100644
--- a/xen/arch/arm/include/asm/grant_table.h
+++ b/xen/arch/arm/include/asm/grant_table.h
@@ -1,7 +1,6 @@
#ifndef __ASM_GRANT_TABLE_H__
#define __ASM_GRANT_TABLE_H__
-#include <xen/grant_table.h>
#include <xen/kernel.h>
#include <xen/pfn.h>
#include <xen/sched.h>
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index 3d503c6973..d5bb1d5d35 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -26,6 +26,7 @@
#include <public/io/xs_wire.h>
#include <asm/dom0less-build.h>
+#include <asm/grant_table.h>
#include <asm/setup.h>
#include <xen/static-memory.h>
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index cf131c43a1..1e437eff50 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -42,8 +42,10 @@
#include <xen/xvmalloc.h>
#include <xen/nospec.h>
#include <xsm/xsm.h>
+
#include <asm/flushtlb.h>
#include <asm/guest_atomics.h>
+#include <asm/grant_table.h>
#ifdef CONFIG_PV_SHIM
#include <asm/guest.h>
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 297d7669e9..98c4f4136b 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -27,10 +27,6 @@
#include <xen/rwlock.h>
#include <public/grant_table.h>
-#ifdef CONFIG_GRANT_TABLE
-#include <asm/grant_table.h>
-#endif
-
struct grant_table;
/* Seed a gnttab entry for Hyperlaunch/dom0less. */
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
2025-06-05 19:47 ` [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h Alejandro Vallejo
@ 2025-06-06 6:52 ` Jan Beulich
2025-06-06 10:02 ` Alejandro Vallejo
0 siblings, 1 reply; 39+ messages in thread
From: Jan Beulich @ 2025-06-06 6:52 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, Anthony PERARD,
Roger Pau Monné, Daniel P. Smith, xen-devel
On 05.06.2025 21:47, Alejandro Vallejo wrote:
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -42,8 +42,10 @@
> #include <xen/xvmalloc.h>
> #include <xen/nospec.h>
> #include <xsm/xsm.h>
> +
> #include <asm/flushtlb.h>
> #include <asm/guest_atomics.h>
> +#include <asm/grant_table.h>
>
> #ifdef CONFIG_PV_SHIM
> #include <asm/guest.h>
> --- a/xen/include/xen/grant_table.h
> +++ b/xen/include/xen/grant_table.h
> @@ -27,10 +27,6 @@
> #include <xen/rwlock.h>
> #include <public/grant_table.h>
>
> -#ifdef CONFIG_GRANT_TABLE
> -#include <asm/grant_table.h>
> -#endif
> -
> struct grant_table;
>
> /* Seed a gnttab entry for Hyperlaunch/dom0less. */
The description doesn't make clear why these two files need changing.
Jan
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
2025-06-06 6:52 ` Jan Beulich
@ 2025-06-06 10:02 ` Alejandro Vallejo
2025-06-06 10:07 ` Jan Beulich
0 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-06 10:02 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, Anthony PERARD,
Roger Pau Monné, Daniel P. Smith, xen-devel
On Fri Jun 6, 2025 at 8:52 AM CEST, Jan Beulich wrote:
> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -42,8 +42,10 @@
>> #include <xen/xvmalloc.h>
>> #include <xen/nospec.h>
>> #include <xsm/xsm.h>
>> +
>> #include <asm/flushtlb.h>
>> #include <asm/guest_atomics.h>
>> +#include <asm/grant_table.h>
>>
>> #ifdef CONFIG_PV_SHIM
>> #include <asm/guest.h>
>> --- a/xen/include/xen/grant_table.h
>> +++ b/xen/include/xen/grant_table.h
>> @@ -27,10 +27,6 @@
>> #include <xen/rwlock.h>
>> #include <public/grant_table.h>
>>
>> -#ifdef CONFIG_GRANT_TABLE
>> -#include <asm/grant_table.h>
>> -#endif
>> -
>> struct grant_table;
>>
>> /* Seed a gnttab entry for Hyperlaunch/dom0less. */
>
> The description doesn't make clear why these two files need changing.
>
> Jan
What sort of description? I removed a conditional include and added it to one
of the few places it didn't include it already along with xen/grant_table.h.
The title does say the patch removes the crossed includes in asm/grant_table.h
and xen/grant_table.h.
It's, I hope, self-explanatory regular spring cleanup.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
2025-06-06 10:02 ` Alejandro Vallejo
@ 2025-06-06 10:07 ` Jan Beulich
2025-06-06 10:30 ` Alejandro Vallejo
0 siblings, 1 reply; 39+ messages in thread
From: Jan Beulich @ 2025-06-06 10:07 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, Anthony PERARD,
Roger Pau Monné, Daniel P. Smith, xen-devel
On 06.06.2025 12:02, Alejandro Vallejo wrote:
> On Fri Jun 6, 2025 at 8:52 AM CEST, Jan Beulich wrote:
>> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>>> --- a/xen/common/grant_table.c
>>> +++ b/xen/common/grant_table.c
>>> @@ -42,8 +42,10 @@
>>> #include <xen/xvmalloc.h>
>>> #include <xen/nospec.h>
>>> #include <xsm/xsm.h>
>>> +
>>> #include <asm/flushtlb.h>
>>> #include <asm/guest_atomics.h>
>>> +#include <asm/grant_table.h>
>>>
>>> #ifdef CONFIG_PV_SHIM
>>> #include <asm/guest.h>
>>> --- a/xen/include/xen/grant_table.h
>>> +++ b/xen/include/xen/grant_table.h
>>> @@ -27,10 +27,6 @@
>>> #include <xen/rwlock.h>
>>> #include <public/grant_table.h>
>>>
>>> -#ifdef CONFIG_GRANT_TABLE
>>> -#include <asm/grant_table.h>
>>> -#endif
>>> -
>>> struct grant_table;
>>>
>>> /* Seed a gnttab entry for Hyperlaunch/dom0less. */
>>
>> The description doesn't make clear why these two files need changing.
>
> What sort of description? I removed a conditional include and added it to one
> of the few places it didn't include it already along with xen/grant_table.h.
>
> The title does say the patch removes the crossed includes in asm/grant_table.h
> and xen/grant_table.h.
>
> It's, I hope, self-explanatory regular spring cleanup.
Then I'm sorry, to me it isn't. "Break links" has an entirely different (file
system) meaning to me, in the common case. Plus that says what is being done,
but not why. And it's the "why" that I'm seeking clarification on. From your
response to my remarks on v1 I was concluding that the issue is that in a few
places asm/grant_table.h would need including additionally. I didn't expect
any #include to (need to) go away.
Jan
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
2025-06-06 10:07 ` Jan Beulich
@ 2025-06-06 10:30 ` Alejandro Vallejo
2025-06-06 12:23 ` Jan Beulich
0 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-06 10:30 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, Anthony PERARD,
Roger Pau Monné, Daniel P. Smith, xen-devel
On Fri Jun 6, 2025 at 12:07 PM CEST, Jan Beulich wrote:
> On 06.06.2025 12:02, Alejandro Vallejo wrote:
>> On Fri Jun 6, 2025 at 8:52 AM CEST, Jan Beulich wrote:
>>> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>>>> --- a/xen/common/grant_table.c
>>>> +++ b/xen/common/grant_table.c
>>>> @@ -42,8 +42,10 @@
>>>> #include <xen/xvmalloc.h>
>>>> #include <xen/nospec.h>
>>>> #include <xsm/xsm.h>
>>>> +
>>>> #include <asm/flushtlb.h>
>>>> #include <asm/guest_atomics.h>
>>>> +#include <asm/grant_table.h>
>>>>
>>>> #ifdef CONFIG_PV_SHIM
>>>> #include <asm/guest.h>
>>>> --- a/xen/include/xen/grant_table.h
>>>> +++ b/xen/include/xen/grant_table.h
>>>> @@ -27,10 +27,6 @@
>>>> #include <xen/rwlock.h>
>>>> #include <public/grant_table.h>
>>>>
>>>> -#ifdef CONFIG_GRANT_TABLE
>>>> -#include <asm/grant_table.h>
>>>> -#endif
>>>> -
>>>> struct grant_table;
>>>>
>>>> /* Seed a gnttab entry for Hyperlaunch/dom0less. */
>>>
>>> The description doesn't make clear why these two files need changing.
>>
>> What sort of description? I removed a conditional include and added it to one
>> of the few places it didn't include it already along with xen/grant_table.h.
>>
>> The title does say the patch removes the crossed includes in asm/grant_table.h
>> and xen/grant_table.h.
>>
>> It's, I hope, self-explanatory regular spring cleanup.
>
> Then I'm sorry, to me it isn't. "Break links" has an entirely different (file
> system) meaning to me, in the common case. Plus that says what is being done,
> but not why. And it's the "why" that I'm seeking clarification on. From your
> response to my remarks on v1 I was concluding that the issue is that in a few
> places asm/grant_table.h would need including additionally. I didn't expect
> any #include to (need to) go away.
>
> Jan
Let me take a step back then. How about this commit message for this same patch?
xen/gnttab: Remove cyclic includes in xen/grant_table.h and arm's asm/grant_table.h
The way they currently include each other, with one of the includes being conditional
on CONFIG_GRANT_TABLE, makes it hard to know which contents are included when.
Seeing how nothing in either header depends on the other, let the include sites
include both if both are needed.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
2025-06-06 10:30 ` Alejandro Vallejo
@ 2025-06-06 12:23 ` Jan Beulich
0 siblings, 0 replies; 39+ messages in thread
From: Jan Beulich @ 2025-06-06 12:23 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, Anthony PERARD,
Roger Pau Monné, Daniel P. Smith, xen-devel
On 06.06.2025 12:30, Alejandro Vallejo wrote:
> On Fri Jun 6, 2025 at 12:07 PM CEST, Jan Beulich wrote:
>> On 06.06.2025 12:02, Alejandro Vallejo wrote:
>>> On Fri Jun 6, 2025 at 8:52 AM CEST, Jan Beulich wrote:
>>>> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>>>>> --- a/xen/common/grant_table.c
>>>>> +++ b/xen/common/grant_table.c
>>>>> @@ -42,8 +42,10 @@
>>>>> #include <xen/xvmalloc.h>
>>>>> #include <xen/nospec.h>
>>>>> #include <xsm/xsm.h>
>>>>> +
>>>>> #include <asm/flushtlb.h>
>>>>> #include <asm/guest_atomics.h>
>>>>> +#include <asm/grant_table.h>
>>>>>
>>>>> #ifdef CONFIG_PV_SHIM
>>>>> #include <asm/guest.h>
>>>>> --- a/xen/include/xen/grant_table.h
>>>>> +++ b/xen/include/xen/grant_table.h
>>>>> @@ -27,10 +27,6 @@
>>>>> #include <xen/rwlock.h>
>>>>> #include <public/grant_table.h>
>>>>>
>>>>> -#ifdef CONFIG_GRANT_TABLE
>>>>> -#include <asm/grant_table.h>
>>>>> -#endif
>>>>> -
>>>>> struct grant_table;
>>>>>
>>>>> /* Seed a gnttab entry for Hyperlaunch/dom0less. */
>>>>
>>>> The description doesn't make clear why these two files need changing.
>>>
>>> What sort of description? I removed a conditional include and added it to one
>>> of the few places it didn't include it already along with xen/grant_table.h.
>>>
>>> The title does say the patch removes the crossed includes in asm/grant_table.h
>>> and xen/grant_table.h.
>>>
>>> It's, I hope, self-explanatory regular spring cleanup.
>>
>> Then I'm sorry, to me it isn't. "Break links" has an entirely different (file
>> system) meaning to me, in the common case. Plus that says what is being done,
>> but not why. And it's the "why" that I'm seeking clarification on. From your
>> response to my remarks on v1 I was concluding that the issue is that in a few
>> places asm/grant_table.h would need including additionally. I didn't expect
>> any #include to (need to) go away.
>
> Let me take a step back then. How about this commit message for this same patch?
>
> xen/gnttab: Remove cyclic includes in xen/grant_table.h and arm's asm/grant_table.h
>
> The way they currently include each other, with one of the includes being conditional
> on CONFIG_GRANT_TABLE, makes it hard to know which contents are included when.
>
> Seeing how nothing in either header depends on the other, let the include sites
> include both if both are needed.
>
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
While definitely better as a description, I still don't see the point of removing
the conditional #include from xen/grant_table.h.
Jan
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 03/15] xen/dt: Add BOOTMOD_MICROCODE
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
2025-06-05 19:47 ` [PATCH v2 01/15] xen: Clean up asm-generic/device.h Alejandro Vallejo
2025-06-05 19:47 ` [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h Alejandro Vallejo
@ 2025-06-05 19:47 ` Alejandro Vallejo
2025-06-06 8:47 ` Orzel, Michal
2025-06-05 19:47 ` [PATCH v2 04/15] x86: Preinitialise all modules to be of kind BOOTMOD_UNKNOWN Alejandro Vallejo
` (11 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:47 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Daniel P. Smith
In preparation for x86 to start using bootmodule instead of boot_module
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
v2:
* Added microcode detection in early probing
---
xen/common/device-tree/bootfdt.c | 2 ++
xen/common/device-tree/bootinfo.c | 1 +
xen/include/xen/bootfdt.h | 1 +
3 files changed, 4 insertions(+)
diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c
index 9df80291b1..9daea06e57 100644
--- a/xen/common/device-tree/bootfdt.c
+++ b/xen/common/device-tree/bootfdt.c
@@ -351,6 +351,8 @@ static void __init process_multiboot_node(const void *fdt, int node,
kind = BOOTMOD_XSM;
else if ( fdt_node_check_compatible(fdt, node, "multiboot,device-tree") == 0 )
kind = BOOTMOD_GUEST_DTB;
+ else if ( fdt_node_check_compatible(fdt, node, "multiboot,microcode") == 0 )
+ kind = BOOTMOD_MICROCODE;
else
kind = BOOTMOD_UNKNOWN;
diff --git a/xen/common/device-tree/bootinfo.c b/xen/common/device-tree/bootinfo.c
index 76d652c0de..717cfa0962 100644
--- a/xen/common/device-tree/bootinfo.c
+++ b/xen/common/device-tree/bootinfo.c
@@ -31,6 +31,7 @@ const char * __init boot_module_kind_as_string(bootmodule_kind kind)
case BOOTMOD_RAMDISK: return "Ramdisk";
case BOOTMOD_XSM: return "XSM";
case BOOTMOD_GUEST_DTB: return "DTB";
+ case BOOTMOD_MICROCODE: return "Microcode";
case BOOTMOD_UNKNOWN: return "Unknown";
default: BUG();
}
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index 64db48f4fe..ff40f3078e 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -21,6 +21,7 @@ typedef enum {
BOOTMOD_RAMDISK,
BOOTMOD_XSM,
BOOTMOD_GUEST_DTB,
+ BOOTMOD_MICROCODE,
BOOTMOD_UNKNOWN
} bootmodule_kind;
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v2 04/15] x86: Preinitialise all modules to be of kind BOOTMOD_UNKNOWN
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (2 preceding siblings ...)
2025-06-05 19:47 ` [PATCH v2 03/15] xen/dt: Add BOOTMOD_MICROCODE Alejandro Vallejo
@ 2025-06-05 19:47 ` Alejandro Vallejo
2025-06-05 19:47 ` [PATCH v2 05/15] x86: Replace boot_module with bootmodule Alejandro Vallejo
` (10 subsequent siblings)
14 siblings, 0 replies; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:47 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Daniel P. Smith, Stefano Stabellini
A later patch removes boot_module and replaces its uses with bootmodule.
The equivalent field for "type" doesn't have BOOTMOD_UNKNOWN as a zero
value, so it must be explicitly set in the static xen_boot_info.
Not a functional change.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
v2:
* Add comment stating why mods initialiser isn't off by one.
---
xen/arch/x86/setup.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 1f5cb67bd0..7d3b30e1db 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -298,6 +298,12 @@ struct boot_info __initdata xen_boot_info = {
.loader = "unknown",
.cmdline = "",
.domains = { [0 ... MAX_NR_BOOTDOMS - 1] = { .domid = DOMID_INVALID } },
+ /*
+ * There's a MAX_NR_BOOTMODS-th entry in the array. It's not off by one.
+ *
+ * The extra entry exists to be able to add the Xen image as a module.
+ */
+ .mods = { [0 ... MAX_NR_BOOTMODS] = { .type = BOOTMOD_UNKNOWN } },
};
static struct boot_info *__init multiboot_fill_boot_info(
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v2 05/15] x86: Replace boot_module with bootmodule
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (3 preceding siblings ...)
2025-06-05 19:47 ` [PATCH v2 04/15] x86: Preinitialise all modules to be of kind BOOTMOD_UNKNOWN Alejandro Vallejo
@ 2025-06-05 19:47 ` Alejandro Vallejo
2025-06-05 19:47 ` [PATCH v2 06/15] xen: Refactor kernel_info to have a header like boot_domain Alejandro Vallejo
` (9 subsequent siblings)
14 siblings, 0 replies; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:47 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Daniel P. Smith
These types resemble each other very closely in layout and intent, and
with "struct bootmodule" already in common code it makes perfect sense
to merge them. In order to do so, add an arch-specific area for
x86-specific tidbits.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
--
v2:
* Remove spurious fdt_cmdline from the new arch_bootmodule
---
xen/arch/x86/cpu/microcode/core.c | 9 ++--
xen/arch/x86/hvm/dom0_build.c | 10 ++---
xen/arch/x86/include/asm/boot-domain.h | 4 +-
xen/arch/x86/include/asm/bootfdt.h | 50 ++++++++++++++++++++++
xen/arch/x86/include/asm/bootinfo.h | 58 +++-----------------------
xen/arch/x86/include/asm/setup.h | 6 +--
xen/arch/x86/pv/dom0_build.c | 8 ++--
xen/arch/x86/setup.c | 52 ++++++++++++-----------
xen/include/xen/bootfdt.h | 9 ++++
xen/xsm/xsm_policy.c | 4 +-
10 files changed, 111 insertions(+), 99 deletions(-)
create mode 100644 xen/arch/x86/include/asm/bootfdt.h
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 34a94cd25b..0111ef9156 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -760,12 +760,11 @@ static int __init early_microcode_load(struct boot_info *bi)
{
for ( idx = 0; idx < bi->nr_modules; ++idx )
{
- const struct boot_module *bm = &bi->mods[idx];
+ const struct bootmodule *bm = &bi->mods[idx];
struct cpio_data cd;
/* Search anything unclaimed or likely to be a CPIO archive. */
- if ( bm->type != BOOTMOD_UNKNOWN &&
- bm->type != BOOTMOD_RAMDISK )
+ if ( bm->kind != BOOTMOD_UNKNOWN && bm->kind != BOOTMOD_RAMDISK )
continue;
size = bm->size;
@@ -815,12 +814,12 @@ static int __init early_microcode_load(struct boot_info *bi)
return -ENODEV;
}
- if ( bi->mods[idx].type != BOOTMOD_UNKNOWN )
+ if ( bi->mods[idx].kind != BOOTMOD_UNKNOWN )
{
printk(XENLOG_WARNING "Microcode: Chosen module %d already used\n", idx);
return -ENODEV;
}
- bi->mods[idx].type = BOOTMOD_MICROCODE;
+ bi->mods[idx].kind = BOOTMOD_MICROCODE;
size = bi->mods[idx].size;
data = bootstrap_map_bm(&bi->mods[idx]);
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index a038e58c11..96410344a8 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -647,10 +647,10 @@ static int __init pvh_load_kernel(
const struct boot_domain *bd, paddr_t *entry, paddr_t *start_info_addr)
{
struct domain *d = bd->d;
- struct boot_module *image = bd->kernel;
- struct boot_module *initrd = bd->module;
+ struct bootmodule *image = bd->kernel;
+ struct bootmodule *initrd = bd->module;
void *image_base = bootstrap_map_bm(image);
- void *image_start = image_base + image->headroom;
+ void *image_start = image_base + image->arch.headroom;
unsigned long image_len = image->size;
unsigned long initrd_len = initrd ? initrd->size : 0;
size_t cmdline_len = bd->cmdline ? strlen(bd->cmdline) + 1 : 0;
@@ -721,9 +721,9 @@ static int __init pvh_load_kernel(
{
size_t initrd_space = elf_round_up(&elf, initrd_len);
- if ( initrd->cmdline_pa )
+ if ( initrd->arch.cmdline_pa )
{
- initrd_cmdline = __va(initrd->cmdline_pa);
+ initrd_cmdline = __va(initrd->arch.cmdline_pa);
if ( !*initrd_cmdline )
initrd_cmdline = NULL;
}
diff --git a/xen/arch/x86/include/asm/boot-domain.h b/xen/arch/x86/include/asm/boot-domain.h
index d7c6042e25..242e9c9c2b 100644
--- a/xen/arch/x86/include/asm/boot-domain.h
+++ b/xen/arch/x86/include/asm/boot-domain.h
@@ -13,8 +13,8 @@
struct boot_domain {
domid_t domid;
- struct boot_module *kernel;
- struct boot_module *module;
+ struct bootmodule *kernel;
+ struct bootmodule *module;
const char *cmdline;
struct domain *d;
diff --git a/xen/arch/x86/include/asm/bootfdt.h b/xen/arch/x86/include/asm/bootfdt.h
new file mode 100644
index 0000000000..2fc705a1cd
--- /dev/null
+++ b/xen/arch/x86/include/asm/bootfdt.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef X86_BOOTFDT_H
+#define X86_BOOTFDT_H
+
+#include <xen/types.h>
+
+struct arch_bootmodule
+{
+ /*
+ * Module State Flags:
+ * relocated: indicates module has been relocated in memory.
+ * released: indicates module's pages have been freed.
+ */
+ bool relocated:1;
+ bool released:1;
+
+ /*
+ * A boot module may need decompressing by Xen. Headroom is an estimate of
+ * the additional space required to decompress the module.
+ *
+ * Headroom is accounted for at the start of the module. Decompressing is
+ * done in-place with input=start, output=start-headroom, expecting the
+ * pointers to become equal (give or take some rounding) when decompression
+ * is complete.
+ *
+ * Memory layout at boot:
+ *
+ * start ----+
+ * v
+ * |<-----headroom------>|<------size------->|
+ * +-------------------+
+ * | Compressed Module |
+ * +---------------------+-------------------+
+ * | Decompressed Module |
+ * +-----------------------------------------+
+ */
+ unsigned long headroom;
+ paddr_t cmdline_pa;
+};
+
+#endif /* X86_BOOTFDT_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 3afc214c17..f3210b7d6a 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -8,6 +8,7 @@
#ifndef X86_BOOTINFO_H
#define X86_BOOTINFO_H
+#include <xen/bootfdt.h>
#include <xen/init.h>
#include <xen/multiboot.h>
#include <xen/types.h>
@@ -19,55 +20,6 @@
/* Max number of boot domains that Xen can construct */
#define MAX_NR_BOOTDOMS 1
-/* Boot module binary type / purpose */
-enum bootmod_type {
- BOOTMOD_UNKNOWN,
- BOOTMOD_XEN,
- BOOTMOD_KERNEL,
- BOOTMOD_RAMDISK,
- BOOTMOD_MICROCODE,
- BOOTMOD_XSM_POLICY,
-};
-
-struct boot_module {
- enum bootmod_type type;
-
- /*
- * Module State Flags:
- * relocated: indicates module has been relocated in memory.
- * released: indicates module's pages have been freed.
- */
- bool relocated:1;
- bool released:1;
-
- /*
- * A boot module may need decompressing by Xen. Headroom is an estimate of
- * the additional space required to decompress the module.
- *
- * Headroom is accounted for at the start of the module. Decompressing is
- * done in-place with input=start, output=start-headroom, expecting the
- * pointers to become equal (give or take some rounding) when decompression
- * is complete.
- *
- * Memory layout at boot:
- *
- * start ----+
- * v
- * |<-----headroom------>|<------size------->|
- * +-------------------+
- * | Compressed Module |
- * +---------------------+-------------------+
- * | Decompressed Module |
- * +-----------------------------------------+
- */
- unsigned long headroom;
-
- paddr_t cmdline_pa;
-
- paddr_t start;
- size_t size;
-};
-
/*
* Xen internal representation of information provided by the
* bootloader/environment, or derived from the information.
@@ -81,7 +33,7 @@ struct boot_info {
size_t memmap_length;
unsigned int nr_modules;
- struct boot_module mods[MAX_NR_BOOTMODS + 1];
+ struct bootmodule mods[MAX_NR_BOOTMODS + 1];
struct boot_domain domains[MAX_NR_BOOTDOMS];
};
@@ -94,16 +46,16 @@ struct boot_info {
* Failure - a value greater than MAX_NR_BOOTMODS
*/
static inline unsigned int __init next_boot_module_index(
- const struct boot_info *bi, enum bootmod_type t, unsigned int start)
+ const struct boot_info *bi, bootmodule_kind k, unsigned int start)
{
unsigned int i;
- if ( t == BOOTMOD_XEN )
+ if ( k == BOOTMOD_XEN )
return bi->nr_modules;
for ( i = start; i < bi->nr_modules; i++ )
{
- if ( bi->mods[i].type == t )
+ if ( bi->mods[i].kind == k )
return i;
}
diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index ac34c69855..c7deaba109 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -36,11 +36,11 @@ extern struct boot_info xen_boot_info;
unsigned long initial_images_nrpages(nodeid_t node);
void free_boot_modules(void);
-struct boot_module;
-void *bootstrap_map_bm(const struct boot_module *bm);
+struct bootmodule;
+void *bootstrap_map_bm(const struct bootmodule *bm);
void bootstrap_unmap(void);
-void release_boot_module(struct boot_module *bm);
+void release_boot_module(struct bootmodule *bm);
struct rangeset;
int remove_xen_ranges(struct rangeset *r);
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index e1b78d47c2..e6c77413f5 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -374,8 +374,8 @@ static int __init dom0_construct(const struct boot_domain *bd)
struct domain *d = bd->d;
struct vcpu *v = d->vcpu[0];
- struct boot_module *image = bd->kernel;
- struct boot_module *initrd = bd->module;
+ struct bootmodule *image = bd->kernel;
+ struct bootmodule *initrd = bd->module;
void *image_base;
unsigned long image_len;
void *image_start;
@@ -422,7 +422,7 @@ static int __init dom0_construct(const struct boot_domain *bd)
image_base = bootstrap_map_bm(image);
image_len = image->size;
- image_start = image_base + image->headroom;
+ image_start = image_base + image->arch.headroom;
d->max_pages = ~0U;
@@ -659,7 +659,7 @@ static int __init dom0_construct(const struct boot_domain *bd)
* pages. Tell the boot_module handling that we've freed it, so the
* memory is left alone.
*/
- initrd->released = true;
+ initrd->arch.released = true;
}
iommu_memory_setup(d, "initrd", mfn_to_page(_mfn(initrd_mfn)),
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 7d3b30e1db..e9a70c2c2b 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -303,7 +303,7 @@ struct boot_info __initdata xen_boot_info = {
*
* The extra entry exists to be able to add the Xen image as a module.
*/
- .mods = { [0 ... MAX_NR_BOOTMODS] = { .type = BOOTMOD_UNKNOWN } },
+ .mods = { [0 ... MAX_NR_BOOTMODS] = { .kind = BOOTMOD_UNKNOWN } },
};
static struct boot_info *__init multiboot_fill_boot_info(
@@ -338,7 +338,7 @@ static struct boot_info *__init multiboot_fill_boot_info(
*/
for ( i = 0; i < MAX_NR_BOOTMODS && i < bi->nr_modules; i++ )
{
- bi->mods[i].cmdline_pa = mods[i].string;
+ bi->mods[i].arch.cmdline_pa = mods[i].string;
if ( efi_enabled(EFI_LOADER) )
{
@@ -361,7 +361,7 @@ static struct boot_info *__init multiboot_fill_boot_info(
}
/* Variable 'i' should be one entry past the last module. */
- bi->mods[i].type = BOOTMOD_XEN;
+ bi->mods[i].kind = BOOTMOD_XEN;
return bi;
}
@@ -386,13 +386,13 @@ unsigned long __init initial_images_nrpages(nodeid_t node)
return nr;
}
-void __init release_boot_module(struct boot_module *bm)
+void __init release_boot_module(struct bootmodule *bm)
{
- ASSERT(!bm->released);
+ ASSERT(!bm->arch.released);
init_domheap_pages(bm->start, bm->start + PAGE_ALIGN(bm->size));
- bm->released = true;
+ bm->arch.released = true;
}
void __init free_boot_modules(void)
@@ -402,7 +402,7 @@ void __init free_boot_modules(void)
for ( i = 0; i < bi->nr_modules; ++i )
{
- if ( bi->mods[i].released )
+ if ( bi->mods[i].arch.released )
continue;
release_boot_module(&bi->mods[i]);
@@ -524,7 +524,7 @@ static void *__init bootstrap_map_addr(paddr_t start, paddr_t end)
return ret;
}
-void *__init bootstrap_map_bm(const struct boot_module *bm)
+void *__init bootstrap_map_bm(const struct bootmodule *bm)
{
return bootstrap_map_addr(bm->start, bm->start + bm->size);
}
@@ -694,7 +694,7 @@ static void __init noinline move_xen(void)
#undef BOOTSTRAP_MAP_LIMIT
static uint64_t __init consider_modules(
- uint64_t s, uint64_t e, uint32_t size, const struct boot_module *mods,
+ uint64_t s, uint64_t e, uint32_t size, const struct bootmodule mods[],
unsigned int nr_mods, unsigned int this_mod)
{
unsigned int i;
@@ -990,8 +990,9 @@ static size_t __init domain_cmdline_size(const struct boot_info *bi,
const struct boot_domain *bd)
{
size_t s = bi->kextra ? strlen(bi->kextra) : 0;
+ const struct arch_bootmodule *abm = &bd->kernel->arch;
- s += bd->kernel->cmdline_pa ? strlen(__va(bd->kernel->cmdline_pa)) : 0;
+ s += abm->cmdline_pa ? strlen(__va(abm->cmdline_pa)) : 0;
if ( s == 0 )
return s;
@@ -1055,9 +1056,10 @@ static struct domain *__init create_dom0(struct boot_info *bi)
if ( !(cmdline = xzalloc_array(char, cmdline_size)) )
panic("Error allocating cmdline buffer for %pd\n", d);
- if ( bd->kernel->cmdline_pa )
+ if ( bd->kernel->arch.cmdline_pa )
strlcpy(cmdline,
- cmdline_cook(__va(bd->kernel->cmdline_pa), bi->loader),
+ cmdline_cook(__va(bd->kernel->arch.cmdline_pa),
+ bi->loader),
cmdline_size);
if ( bi->kextra )
@@ -1079,7 +1081,7 @@ static struct domain *__init create_dom0(struct boot_info *bi)
strlcat(cmdline, " acpi=", cmdline_size);
strlcat(cmdline, acpi_param, cmdline_size);
}
- bd->kernel->cmdline_pa = 0;
+ bd->kernel->arch.cmdline_pa = 0;
bd->cmdline = cmdline;
}
@@ -1292,7 +1294,7 @@ void asmlinkage __init noreturn __start_xen(void)
}
/* Dom0 kernel is always first */
- bi->mods[0].type = BOOTMOD_KERNEL;
+ bi->mods[0].kind = BOOTMOD_KERNEL;
bi->domains[0].kernel = &bi->mods[0];
if ( pvh_boot )
@@ -1463,7 +1465,7 @@ void asmlinkage __init noreturn __start_xen(void)
if ( xen_phys_start )
{
- struct boot_module *xen = &bi->mods[bi->nr_modules];
+ struct bootmodule *xen = &bi->mods[bi->nr_modules];
relocated = true;
@@ -1476,7 +1478,7 @@ void asmlinkage __init noreturn __start_xen(void)
xen->size = __2M_rwdata_end - _stext;
}
- bi->mods[0].headroom =
+ bi->mods[0].arch.headroom =
bzimage_headroom(bootstrap_map_bm(&bi->mods[0]), bi->mods[0].size);
bootstrap_unmap();
@@ -1557,10 +1559,10 @@ void asmlinkage __init noreturn __start_xen(void)
/* Is the region suitable for relocating the multiboot modules? */
for ( j = bi->nr_modules - 1; j >= 0; j-- )
{
- struct boot_module *bm = &bi->mods[j];
- unsigned long size = PAGE_ALIGN(bm->headroom + bm->size);
+ struct bootmodule *bm = &bi->mods[j];
+ unsigned long size = PAGE_ALIGN(bm->arch.headroom + bm->size);
- if ( bm->relocated )
+ if ( bm->arch.relocated )
continue;
/* Don't overlap with other modules (or Xen itself). */
@@ -1570,12 +1572,12 @@ void asmlinkage __init noreturn __start_xen(void)
if ( highmem_start && end > highmem_start )
continue;
- if ( s < end && (bm->headroom || (end - size) > bm->start) )
+ if ( s < end && (bm->arch.headroom || (end - size) > bm->start) )
{
- move_memory(end - size + bm->headroom, bm->start, bm->size);
+ move_memory(end - size + bm->arch.headroom, bm->start, bm->size);
bm->start = (end - size);
- bm->size += bm->headroom;
- bm->relocated = true;
+ bm->size += bm->arch.headroom;
+ bm->arch.relocated = true;
}
}
@@ -1601,7 +1603,7 @@ void asmlinkage __init noreturn __start_xen(void)
#endif
}
- if ( bi->mods[0].headroom && !bi->mods[0].relocated )
+ if ( bi->mods[0].arch.headroom && !bi->mods[0].arch.relocated )
panic("Not enough memory to relocate the dom0 kernel image\n");
for ( i = 0; i < bi->nr_modules; ++i )
{
@@ -2159,7 +2161,7 @@ void asmlinkage __init noreturn __start_xen(void)
initrdidx = first_boot_module_index(bi, BOOTMOD_UNKNOWN);
if ( initrdidx < MAX_NR_BOOTMODS )
{
- bi->mods[initrdidx].type = BOOTMOD_RAMDISK;
+ bi->mods[initrdidx].kind = BOOTMOD_RAMDISK;
bi->domains[0].module = &bi->mods[initrdidx];
if ( first_boot_module_index(bi, BOOTMOD_UNKNOWN) < MAX_NR_BOOTMODS )
printk(XENLOG_WARNING
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index ff40f3078e..4cab5d5202 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -7,6 +7,10 @@
#include <xen/macros.h>
#include <xen/xmalloc.h>
+#if __has_include(<asm/bootfdt.h>)
+#include <asm/bootfdt.h>
+#endif
+
#define MIN_FDT_ALIGN 8
#define NR_MEM_BANKS 256
@@ -106,8 +110,13 @@ struct shared_meminfo {
struct bootmodule {
bootmodule_kind kind;
bool domU;
+
paddr_t start;
paddr_t size;
+
+#if __has_include(<asm/bootfdt.h>)
+ struct arch_bootmodule arch;
+#endif
};
/* DT_MAX_NAME is the node name max length according the DT spec */
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index 7f70d860bd..0c2cdea8ed 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -40,7 +40,7 @@ int __init xsm_multiboot_policy_init(
for_each_boot_module_by_type ( i, bi, BOOTMOD_UNKNOWN )
{
- struct boot_module *bm = &bi->mods[i];
+ struct bootmodule *bm = &bi->mods[i];
_policy_start = bootstrap_map_bm(bm);
_policy_len = bm->size;
@@ -53,7 +53,7 @@ int __init xsm_multiboot_policy_init(
printk("Policy len %#lx, start at %p.\n",
_policy_len,_policy_start);
- bm->type = BOOTMOD_XSM_POLICY;
+ bm->kind = BOOTMOD_XSM;
break;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v2 06/15] xen: Refactor kernel_info to have a header like boot_domain
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (4 preceding siblings ...)
2025-06-05 19:47 ` [PATCH v2 05/15] x86: Replace boot_module with bootmodule Alejandro Vallejo
@ 2025-06-05 19:47 ` Alejandro Vallejo
2025-06-05 19:47 ` [PATCH v2 07/15] x86: Replace boot_domain with bootdomain Alejandro Vallejo
` (8 subsequent siblings)
14 siblings, 0 replies; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:47 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
Anthony PERARD, Jan Beulich, Roger Pau Monné,
Daniel P. Smith
Create a struct header with the contents common to kernel_info and
boot_domain, and define that header in common code. This enables x86 to
use that header as-is.
Not a functional change.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
v2:
* New patch
---
xen/arch/arm/dom0less-build.c | 8 ++++----
xen/arch/arm/domain_build.c | 20 ++++++++++----------
xen/arch/arm/kernel.c | 8 ++++----
xen/common/device-tree/dom0less-build.c | 18 +++++++++---------
xen/common/device-tree/domain-build.c | 20 ++++++++++----------
xen/common/device-tree/kernel.c | 20 ++++++++++----------
xen/include/xen/bootfdt.h | 10 ++++++++++
xen/include/xen/fdt-kernel.h | 5 ++---
8 files changed, 59 insertions(+), 50 deletions(-)
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 20aabf6be5..a335551484 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -32,7 +32,7 @@ static int __init make_gicv2_domU_node(struct kernel_info *kinfo)
int res = 0;
__be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2];
__be32 *cells;
- const struct domain *d = kinfo->d;
+ const struct domain *d = kinfo->hdr.d;
res = domain_fdt_begin_node(fdt, "interrupt-controller",
vgic_dist_base(&d->arch.vgic));
@@ -85,7 +85,7 @@ static int __init make_gicv3_domU_node(struct kernel_info *kinfo)
void *fdt = kinfo->fdt;
int res = 0;
__be32 *reg, *cells;
- const struct domain *d = kinfo->d;
+ const struct domain *d = kinfo->hdr.d;
unsigned int i, len = 0;
res = domain_fdt_begin_node(fdt, "interrupt-controller",
@@ -152,7 +152,7 @@ static int __init make_gicv3_domU_node(struct kernel_info *kinfo)
int __init make_intc_domU_node(struct kernel_info *kinfo)
{
- switch ( kinfo->d->arch.vgic.version )
+ switch ( kinfo->hdr.d->arch.vgic.version )
{
#ifdef CONFIG_GICV3
case GIC_V3:
@@ -175,7 +175,7 @@ static int __init make_vpl011_uart_node(struct kernel_info *kinfo)
gic_interrupt_t intr;
__be32 reg[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS];
__be32 *cells;
- struct domain *d = kinfo->d;
+ struct domain *d = kinfo->hdr.d;
res = domain_fdt_begin_node(fdt, "sbsa-uart", d->arch.vpl011.base_addr);
if ( res )
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 590f38e520..5317665555 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -464,8 +464,8 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
int had_dom0_bootargs = 0;
struct dt_device_node *iommu_node;
- if ( kinfo->cmdline && kinfo->cmdline[0] )
- bootargs = &kinfo->cmdline[0];
+ if ( kinfo->hdr.cmdline && kinfo->hdr.cmdline[0] )
+ bootargs = &kinfo->hdr.cmdline[0];
/*
* We always skip the IOMMU device when creating DT for hwdom if there is
@@ -579,7 +579,7 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
if ( dt_node_path_is_equal(node, "/chosen") )
{
- const struct bootmodule *initrd = kinfo->initrd_bootmodule;
+ const struct bootmodule *initrd = kinfo->hdr.initrd;
if ( bootargs )
{
@@ -1386,7 +1386,7 @@ int __init make_timer_node(const struct kernel_info *kinfo)
if ( res )
return res;
- if ( !is_64bit_domain(kinfo->d) )
+ if ( !is_64bit_domain(kinfo->hdr.d) )
res = fdt_property_string(fdt, "compatible", "arm,armv7-timer");
else
res = fdt_property_string(fdt, "compatible", "arm,armv8-timer");
@@ -1398,7 +1398,7 @@ int __init make_timer_node(const struct kernel_info *kinfo)
* It always exposes an active-low level-sensitive interrupt.
*/
- if ( is_hardware_domain(kinfo->d) )
+ if ( is_hardware_domain(kinfo->hdr.d) )
{
irq[TIMER_PHYS_SECURE_PPI] = timer_get_irq(TIMER_PHYS_SECURE_PPI);
irq[TIMER_PHYS_NONSECURE_PPI] =
@@ -1447,7 +1447,7 @@ int __init make_chosen_node(const struct kernel_info *kinfo)
{
int res;
const char *bootargs = NULL;
- const struct bootmodule *initrd = kinfo->initrd_bootmodule;
+ const struct bootmodule *initrd = kinfo->hdr.initrd;
void *fdt = kinfo->fdt;
dt_dprintk("Create chosen node\n");
@@ -1455,9 +1455,9 @@ int __init make_chosen_node(const struct kernel_info *kinfo)
if ( res )
return res;
- if ( kinfo->cmdline && kinfo->cmdline[0] )
+ if ( kinfo->hdr.cmdline && kinfo->hdr.cmdline[0] )
{
- bootargs = &kinfo->cmdline[0];
+ bootargs = &kinfo->hdr.cmdline[0];
res = fdt_property(fdt, "bootargs", bootargs, strlen(bootargs) + 1);
if ( res )
return res;
@@ -1906,7 +1906,7 @@ static int __init construct_dom0(struct domain *d)
d->max_pages = dom0_mem >> PAGE_SHIFT;
kinfo.unassigned_mem = dom0_mem;
- kinfo.d = d;
+ kinfo.hdr.d = d;
rc = kernel_probe(&kinfo, NULL);
if ( rc < 0 )
@@ -1918,7 +1918,7 @@ static int __init construct_dom0(struct domain *d)
int __init construct_hwdom(struct kernel_info *kinfo,
const struct dt_device_node *node)
{
- struct domain *d = kinfo->d;
+ struct domain *d = kinfo->hdr.d;
int rc;
iommu_hwdom_init(d);
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index cb1efb19e7..0561d60d5a 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -46,7 +46,7 @@ static void __init place_modules(struct kernel_info *info,
paddr_t kernbase, paddr_t kernend)
{
/* Align DTB and initrd size to 2Mb. Linux only requires 4 byte alignment */
- const struct bootmodule *mod = info->initrd_bootmodule;
+ const struct bootmodule *mod = info->hdr.initrd;
const struct membanks *mem = kernel_info_get_mem(info);
const paddr_t initrd_len = ROUNDUP(mod ? mod->size : 0, MB(2));
const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt), MB(2));
@@ -152,12 +152,12 @@ static void __init kernel_zimage_load(struct kernel_info *info)
kernel = ioremap_wc(paddr, len);
if ( !kernel )
- panic("Unable to map the %pd kernel\n", info->d);
+ panic("Unable to map the %pd kernel\n", info->hdr.d);
- rc = copy_to_guest_phys_flush_dcache(info->d, load_addr,
+ rc = copy_to_guest_phys_flush_dcache(info->hdr.d, load_addr,
kernel, len);
if ( rc != 0 )
- panic("Unable to copy the kernel in the %pd memory\n", info->d);
+ panic("Unable to copy the kernel in the %pd memory\n", info->hdr.d);
iounmap(kernel);
}
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index d5bb1d5d35..809cb3c232 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -168,18 +168,18 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
return -EINVAL;
}
- res = iomem_permit_access(kinfo->d, paddr_to_pfn(mstart),
+ res = iomem_permit_access(kinfo->hdr.d, paddr_to_pfn(mstart),
paddr_to_pfn(PAGE_ALIGN(mstart + size - 1)));
if ( res )
{
printk(XENLOG_ERR "Unable to permit to dom%d access to"
" 0x%"PRIpaddr" - 0x%"PRIpaddr"\n",
- kinfo->d->domain_id,
+ kinfo->hdr.d->domain_id,
mstart & PAGE_MASK, PAGE_ALIGN(mstart + size) - 1);
return res;
}
- res = map_regions_p2mt(kinfo->d,
+ res = map_regions_p2mt(kinfo->hdr.d,
gaddr_to_gfn(gstart),
PFN_DOWN(size),
maddr_to_mfn(mstart),
@@ -218,7 +218,7 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
return -EINVAL;
}
- res = map_device_irqs_to_domain(kinfo->d, node, true, NULL);
+ res = map_device_irqs_to_domain(kinfo->hdr.d, node, true, NULL);
if ( res < 0 )
return res;
@@ -230,7 +230,7 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
if ( xen_force && !dt_device_is_protected(node) )
return 0;
- return iommu_assign_dt_device(kinfo->d, node);
+ return iommu_assign_dt_device(kinfo->hdr.d, node);
}
static int __init handle_prop_pfdt(struct kernel_info *kinfo,
@@ -297,14 +297,14 @@ static int __init handle_prop_pfdt(struct kernel_info *kinfo,
address_cells, size_cells);
if ( res < 0 )
{
- printk(XENLOG_ERR "Failed to assign device to %pd\n", kinfo->d);
+ printk(XENLOG_ERR "Failed to assign device to %pd\n", kinfo->hdr.d);
return res;
}
}
else if ( (xen_path && !xen_reg) || (xen_reg && !xen_path && !xen_force) )
{
printk(XENLOG_ERR "xen,reg or xen,path missing for %pd\n",
- kinfo->d);
+ kinfo->hdr.d);
return -EINVAL;
}
@@ -607,7 +607,7 @@ static int __init alloc_xenstore_page(struct domain *d)
static int __init alloc_xenstore_params(struct kernel_info *kinfo)
{
- struct domain *d = kinfo->d;
+ struct domain *d = kinfo->hdr.d;
int rc = 0;
#ifdef CONFIG_HVM
@@ -775,7 +775,7 @@ static int __init construct_domU(struct domain *d,
d->max_pages = ((paddr_t)mem * SZ_1K) >> PAGE_SHIFT;
- kinfo.d = d;
+ kinfo.hdr.d = d;
rc = kernel_probe(&kinfo, node);
if ( rc < 0 )
diff --git a/xen/common/device-tree/domain-build.c b/xen/common/device-tree/domain-build.c
index 6b8b8d7cac..e602cb91c9 100644
--- a/xen/common/device-tree/domain-build.c
+++ b/xen/common/device-tree/domain-build.c
@@ -76,7 +76,7 @@ bool __init allocate_bank_memory(struct kernel_info *kinfo, gfn_t sgfn,
paddr_t tot_size)
{
struct membanks *mem = kernel_info_get_mem(kinfo);
- struct domain *d = kinfo->d;
+ struct domain *d = kinfo->hdr.d;
struct membank *bank;
/*
@@ -170,7 +170,7 @@ int __init find_unallocated_memory(const struct kernel_info *kinfo,
unsigned int i, j;
int res;
- ASSERT(domain_use_host_layout(kinfo->d));
+ ASSERT(domain_use_host_layout(kinfo->hdr.d));
unalloc_mem = rangeset_new(NULL, NULL, 0);
if ( !unalloc_mem )
@@ -331,23 +331,23 @@ void __init dtb_load(struct kernel_info *kinfo,
unsigned long left;
printk("Loading %pd DTB to 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
- kinfo->d, kinfo->dtb_paddr,
+ kinfo->hdr.d, kinfo->dtb_paddr,
kinfo->dtb_paddr + fdt_totalsize(kinfo->fdt));
- left = copy_to_guest(kinfo->d, kinfo->dtb_paddr,
+ left = copy_to_guest(kinfo->hdr.d, kinfo->dtb_paddr,
kinfo->fdt,
fdt_totalsize(kinfo->fdt));
if ( left != 0 )
panic("Unable to copy the DTB to %pd memory (left = %lu bytes)\n",
- kinfo->d, left);
+ kinfo->hdr.d, left);
xfree(kinfo->fdt);
}
void __init initrd_load(struct kernel_info *kinfo,
copy_to_guest_phys_cb copy_to_guest)
{
- const struct bootmodule *mod = kinfo->initrd_bootmodule;
+ const struct bootmodule *mod = kinfo->hdr.initrd;
paddr_t load_addr = kinfo->initrd_paddr;
paddr_t paddr, len;
int node;
@@ -363,7 +363,7 @@ void __init initrd_load(struct kernel_info *kinfo,
len = mod->size;
printk("Loading %pd initrd from %"PRIpaddr" to 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
- kinfo->d, paddr, load_addr, load_addr + len);
+ kinfo->hdr.d, paddr, load_addr, load_addr + len);
/* Fix up linux,initrd-start and linux,initrd-end in /chosen */
node = fdt_path_offset(kinfo->fdt, "/chosen");
@@ -386,12 +386,12 @@ void __init initrd_load(struct kernel_info *kinfo,
initrd = ioremap_wc(paddr, len);
if ( !initrd )
- panic("Unable to map the %pd initrd\n", kinfo->d);
+ panic("Unable to map the %pd initrd\n", kinfo->hdr.d);
- res = copy_to_guest(kinfo->d, load_addr,
+ res = copy_to_guest(kinfo->hdr.d, load_addr,
initrd, len);
if ( res != 0 )
- panic("Unable to copy the initrd in the %pd memory\n", kinfo->d);
+ panic("Unable to copy the initrd in the %pd memory\n", kinfo->hdr.d);
iounmap(initrd);
}
diff --git a/xen/common/device-tree/kernel.c b/xen/common/device-tree/kernel.c
index cb04cd9d50..3960f951f7 100644
--- a/xen/common/device-tree/kernel.c
+++ b/xen/common/device-tree/kernel.c
@@ -136,16 +136,16 @@ int __init kernel_probe(struct kernel_info *info,
/* domain is NULL only for the hardware domain */
if ( domain == NULL )
{
- ASSERT(is_hardware_domain(info->d));
+ ASSERT(is_hardware_domain(info->hdr.d));
mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
- info->kernel_bootmodule = mod;
- info->initrd_bootmodule = boot_module_find_by_kind(BOOTMOD_RAMDISK);
+ info->hdr.kernel = mod;
+ info->hdr.initrd = boot_module_find_by_kind(BOOTMOD_RAMDISK);
cmd = boot_cmdline_find_by_kind(BOOTMOD_KERNEL);
if ( cmd )
- info->cmdline = &cmd->cmdline[0];
+ info->hdr.cmdline = &cmd->cmdline[0];
}
else
{
@@ -162,7 +162,7 @@ int __init kernel_probe(struct kernel_info *info,
dt_get_range(&val, node, &kernel_addr, &size);
mod = boot_module_find_by_addr_and_kind(
BOOTMOD_KERNEL, kernel_addr);
- info->kernel_bootmodule = mod;
+ info->hdr.kernel = mod;
}
else if ( dt_device_is_compatible(node, "multiboot,ramdisk") )
{
@@ -171,7 +171,7 @@ int __init kernel_probe(struct kernel_info *info,
val = dt_get_property(node, "reg", &len);
dt_get_range(&val, node, &initrd_addr, &size);
- info->initrd_bootmodule = boot_module_find_by_addr_and_kind(
+ info->hdr.initrd = boot_module_find_by_addr_and_kind(
BOOTMOD_RAMDISK, initrd_addr);
}
else if ( dt_device_is_compatible(node, "multiboot,device-tree") )
@@ -192,7 +192,7 @@ int __init kernel_probe(struct kernel_info *info,
name = dt_node_name(domain);
cmd = boot_cmdline_find_by_name(name);
if ( cmd )
- info->cmdline = &cmd->cmdline[0];
+ info->hdr.cmdline = &cmd->cmdline[0];
}
if ( !mod || !mod->size )
{
@@ -201,10 +201,10 @@ int __init kernel_probe(struct kernel_info *info,
}
printk("Loading %pd kernel from boot module @ %"PRIpaddr"\n",
- info->d, info->kernel_bootmodule->start);
- if ( info->initrd_bootmodule )
+ info->hdr.d, info->hdr.kernel->start);
+ if ( info->hdr.initrd )
printk("Loading ramdisk from boot module @ %"PRIpaddr"\n",
- info->initrd_bootmodule->start);
+ info->hdr.initrd->start);
/*
* uImage isn't really used nowadays thereby leave kernel_uimage_probe()
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index 4cab5d5202..e6d52a599f 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -100,6 +100,16 @@ struct shared_meminfo {
struct shmem_membank_extra extra[NR_SHMEM_BANKS];
};
+
+struct bootdomain {
+ struct domain *d;
+
+ struct bootmodule *kernel;
+ struct bootmodule *initrd;
+
+ const char* cmdline;
+};
+
/*
* The domU flag is set for kernels and ramdisks of "xen,domain" nodes.
* The purpose of the domU flag is to avoid getting confused in
diff --git a/xen/include/xen/fdt-kernel.h b/xen/include/xen/fdt-kernel.h
index 1939c3ebf7..dd009f01d0 100644
--- a/xen/include/xen/fdt-kernel.h
+++ b/xen/include/xen/fdt-kernel.h
@@ -16,7 +16,7 @@
#endif
struct kernel_info {
- struct domain *d;
+ struct bootdomain hdr;
void *fdt; /* flat device tree */
paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
@@ -34,8 +34,7 @@ struct kernel_info {
paddr_t gnttab_size;
/* boot blob load addresses */
- const struct bootmodule *kernel_bootmodule, *initrd_bootmodule, *dtb_bootmodule;
- const char* cmdline;
+ const struct bootmodule *dtb_bootmodule;
paddr_t dtb_paddr;
paddr_t initrd_paddr;
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v2 07/15] x86: Replace boot_domain with bootdomain
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (5 preceding siblings ...)
2025-06-05 19:47 ` [PATCH v2 06/15] xen: Refactor kernel_info to have a header like boot_domain Alejandro Vallejo
@ 2025-06-05 19:47 ` Alejandro Vallejo
2025-06-06 23:28 ` Stefano Stabellini
2025-06-05 19:48 ` [PATCH v2 08/15] xen/dt: Move bootfdt functions to xen/bootfdt.h Alejandro Vallejo
` (7 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:47 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Daniel P. Smith
No functional change intended.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
v2:
* Replaces the previous patch in which kernel_info replaced boot_domain
---
xen/arch/x86/dom0_build.c | 2 +-
xen/arch/x86/hvm/dom0_build.c | 6 ++---
xen/arch/x86/include/asm/boot-domain.h | 33 --------------------------
xen/arch/x86/include/asm/bootfdt.h | 7 ++++++
xen/arch/x86/include/asm/bootinfo.h | 3 +--
xen/arch/x86/include/asm/dom0_build.h | 6 ++---
xen/arch/x86/include/asm/setup.h | 4 ++--
xen/arch/x86/pv/dom0_build.c | 6 ++---
xen/arch/x86/setup.c | 16 +++++++------
xen/include/xen/bootfdt.h | 4 ++++
10 files changed, 33 insertions(+), 54 deletions(-)
delete mode 100644 xen/arch/x86/include/asm/boot-domain.h
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 0b467fd4a4..1c8c5140a3 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -615,7 +615,7 @@ int __init dom0_setup_permissions(struct domain *d)
return rc;
}
-int __init construct_dom0(const struct boot_domain *bd)
+int __init construct_dom0(const struct bootdomain *bd)
{
int rc;
const struct domain *d = bd->d;
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index 96410344a8..85c000b259 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -644,11 +644,11 @@ static bool __init check_and_adjust_load_address(
}
static int __init pvh_load_kernel(
- const struct boot_domain *bd, paddr_t *entry, paddr_t *start_info_addr)
+ const struct bootdomain *bd, paddr_t *entry, paddr_t *start_info_addr)
{
struct domain *d = bd->d;
struct bootmodule *image = bd->kernel;
- struct bootmodule *initrd = bd->module;
+ struct bootmodule *initrd = bd->initrd;
void *image_base = bootstrap_map_bm(image);
void *image_start = image_base + image->arch.headroom;
unsigned long image_len = image->size;
@@ -1329,7 +1329,7 @@ static void __hwdom_init pvh_setup_mmcfg(struct domain *d)
}
}
-int __init dom0_construct_pvh(const struct boot_domain *bd)
+int __init dom0_construct_pvh(const struct bootdomain *bd)
{
paddr_t entry, start_info;
struct domain *d = bd->d;
diff --git a/xen/arch/x86/include/asm/boot-domain.h b/xen/arch/x86/include/asm/boot-domain.h
deleted file mode 100644
index 242e9c9c2b..0000000000
--- a/xen/arch/x86/include/asm/boot-domain.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Copyright (c) 2024 Apertus Solutions, LLC
- * Author: Daniel P. Smith <dpsmith@apertussolutions.com>
- * Copyright (c) 2024 Christopher Clark <christopher.w.clark@gmail.com>
- */
-
-#ifndef __XEN_X86_BOOTDOMAIN_H__
-#define __XEN_X86_BOOTDOMAIN_H__
-
-#include <public/xen.h>
-
-struct boot_domain {
- domid_t domid;
-
- struct bootmodule *kernel;
- struct bootmodule *module;
- const char *cmdline;
-
- struct domain *d;
-};
-
-#endif
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/x86/include/asm/bootfdt.h b/xen/arch/x86/include/asm/bootfdt.h
index 2fc705a1cd..b30132381e 100644
--- a/xen/arch/x86/include/asm/bootfdt.h
+++ b/xen/arch/x86/include/asm/bootfdt.h
@@ -4,6 +4,13 @@
#include <xen/types.h>
+#include <public/xen.h>
+
+struct arch_bootdomain
+{
+ domid_t domid;
+};
+
struct arch_bootmodule
{
/*
diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index f3210b7d6a..b8280ba357 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -12,7 +12,6 @@
#include <xen/init.h>
#include <xen/multiboot.h>
#include <xen/types.h>
-#include <asm/boot-domain.h>
/* Max number of boot modules a bootloader can provide in addition to Xen */
#define MAX_NR_BOOTMODS 63
@@ -34,7 +33,7 @@ struct boot_info {
unsigned int nr_modules;
struct bootmodule mods[MAX_NR_BOOTMODS + 1];
- struct boot_domain domains[MAX_NR_BOOTDOMS];
+ struct bootdomain domains[MAX_NR_BOOTDOMS];
};
/*
diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
index ff021c24af..df03189870 100644
--- a/xen/arch/x86/include/asm/dom0_build.h
+++ b/xen/arch/x86/include/asm/dom0_build.h
@@ -13,9 +13,9 @@ unsigned long dom0_compute_nr_pages(struct domain *d,
unsigned long initrd_len);
int dom0_setup_permissions(struct domain *d);
-struct boot_domain;
-int dom0_construct_pv(const struct boot_domain *bd);
-int dom0_construct_pvh(const struct boot_domain *bd);
+struct bootdomain;
+int dom0_construct_pv(const struct bootdomain *bd);
+int dom0_construct_pvh(const struct bootdomain *bd);
unsigned long dom0_paging_pages(const struct domain *d,
unsigned long nr_pages);
diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index c7deaba109..a8647f0fdf 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -26,8 +26,8 @@ void subarch_init_memory(void);
void init_IRQ(void);
-struct boot_domain;
-int construct_dom0(const struct boot_domain *bd);
+struct bootdomain;
+int construct_dom0(const struct bootdomain *bd);
void setup_io_bitmap(struct domain *d);
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index e6c77413f5..a6f212fe0a 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -355,7 +355,7 @@ static struct page_info * __init alloc_chunk(struct domain *d,
return page;
}
-static int __init dom0_construct(const struct boot_domain *bd)
+static int __init dom0_construct(const struct bootdomain *bd)
{
unsigned int i;
int rc, order, machine;
@@ -375,7 +375,7 @@ static int __init dom0_construct(const struct boot_domain *bd)
struct vcpu *v = d->vcpu[0];
struct bootmodule *image = bd->kernel;
- struct bootmodule *initrd = bd->module;
+ struct bootmodule *initrd = bd->initrd;
void *image_base;
unsigned long image_len;
void *image_start;
@@ -1070,7 +1070,7 @@ out:
return rc;
}
-int __init dom0_construct_pv(const struct boot_domain *bd)
+int __init dom0_construct_pv(const struct bootdomain *bd)
{
unsigned long cr4 = read_cr4();
int rc;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index e9a70c2c2b..726adad0e5 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -297,7 +297,9 @@ static const char *cmdline_cook(const char *p, const char *loader_name);
struct boot_info __initdata xen_boot_info = {
.loader = "unknown",
.cmdline = "",
- .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = { .domid = DOMID_INVALID } },
+ .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = {
+ .arch.domid = DOMID_INVALID
+ }},
/*
* There's a MAX_NR_BOOTMODS-th entry in the array. It's not off by one.
*
@@ -987,7 +989,7 @@ static unsigned int __init copy_bios_e820(struct e820entry *map, unsigned int li
}
static size_t __init domain_cmdline_size(const struct boot_info *bi,
- const struct boot_domain *bd)
+ const struct bootdomain *bd)
{
size_t s = bi->kextra ? strlen(bi->kextra) : 0;
const struct arch_bootmodule *abm = &bd->kernel->arch;
@@ -1022,7 +1024,7 @@ static struct domain *__init create_dom0(struct boot_info *bi)
.misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
},
};
- struct boot_domain *bd = &bi->domains[0];
+ struct bootdomain *bd = &bi->domains[0];
struct domain *d;
if ( opt_dom0_pvh )
@@ -1039,11 +1041,11 @@ static struct domain *__init create_dom0(struct boot_info *bi)
dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
/* Create initial domain. Not d0 for pvshim. */
- bd->domid = get_initial_domain_id();
- d = domain_create(bd->domid, &dom0_cfg,
+ bd->arch.domid = get_initial_domain_id();
+ d = domain_create(bd->arch.domid, &dom0_cfg,
pv_shim ? 0 : CDF_privileged | CDF_hardware);
if ( IS_ERR(d) )
- panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d));
+ panic("Error creating d%u: %ld\n", bd->arch.domid, PTR_ERR(d));
init_dom0_cpuid_policy(d);
@@ -2162,7 +2164,7 @@ void asmlinkage __init noreturn __start_xen(void)
if ( initrdidx < MAX_NR_BOOTMODS )
{
bi->mods[initrdidx].kind = BOOTMOD_RAMDISK;
- bi->domains[0].module = &bi->mods[initrdidx];
+ bi->domains[0].initrd = &bi->mods[initrdidx];
if ( first_boot_module_index(bi, BOOTMOD_UNKNOWN) < MAX_NR_BOOTMODS )
printk(XENLOG_WARNING
"Multiple initrd candidates, picking module #%u\n",
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index e6d52a599f..19d2ff0f0c 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -108,6 +108,10 @@ struct bootdomain {
struct bootmodule *initrd;
const char* cmdline;
+
+#if __has_include(<asm/bootfdt.h>)
+ struct arch_bootdomain arch;
+#endif
};
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v2 07/15] x86: Replace boot_domain with bootdomain
2025-06-05 19:47 ` [PATCH v2 07/15] x86: Replace boot_domain with bootdomain Alejandro Vallejo
@ 2025-06-06 23:28 ` Stefano Stabellini
2025-06-09 8:57 ` Alejandro Vallejo
0 siblings, 1 reply; 39+ messages in thread
From: Stefano Stabellini @ 2025-06-06 23:28 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Daniel P. Smith
On Thu, 5 Jun 2025, Alejandro Vallejo wrote:
> No functional change intended.
>
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
> ---
> v2:
> * Replaces the previous patch in which kernel_info replaced boot_domain
> ---
> xen/arch/x86/dom0_build.c | 2 +-
> xen/arch/x86/hvm/dom0_build.c | 6 ++---
> xen/arch/x86/include/asm/boot-domain.h | 33 --------------------------
> xen/arch/x86/include/asm/bootfdt.h | 7 ++++++
> xen/arch/x86/include/asm/bootinfo.h | 3 +--
> xen/arch/x86/include/asm/dom0_build.h | 6 ++---
> xen/arch/x86/include/asm/setup.h | 4 ++--
> xen/arch/x86/pv/dom0_build.c | 6 ++---
> xen/arch/x86/setup.c | 16 +++++++------
> xen/include/xen/bootfdt.h | 4 ++++
> 10 files changed, 33 insertions(+), 54 deletions(-)
> delete mode 100644 xen/arch/x86/include/asm/boot-domain.h
>
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index 0b467fd4a4..1c8c5140a3 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -615,7 +615,7 @@ int __init dom0_setup_permissions(struct domain *d)
> return rc;
> }
>
> -int __init construct_dom0(const struct boot_domain *bd)
> +int __init construct_dom0(const struct bootdomain *bd)
> {
> int rc;
> const struct domain *d = bd->d;
> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> index 96410344a8..85c000b259 100644
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -644,11 +644,11 @@ static bool __init check_and_adjust_load_address(
> }
>
> static int __init pvh_load_kernel(
> - const struct boot_domain *bd, paddr_t *entry, paddr_t *start_info_addr)
> + const struct bootdomain *bd, paddr_t *entry, paddr_t *start_info_addr)
> {
> struct domain *d = bd->d;
> struct bootmodule *image = bd->kernel;
> - struct bootmodule *initrd = bd->module;
> + struct bootmodule *initrd = bd->initrd;
> void *image_base = bootstrap_map_bm(image);
> void *image_start = image_base + image->arch.headroom;
> unsigned long image_len = image->size;
> @@ -1329,7 +1329,7 @@ static void __hwdom_init pvh_setup_mmcfg(struct domain *d)
> }
> }
>
> -int __init dom0_construct_pvh(const struct boot_domain *bd)
> +int __init dom0_construct_pvh(const struct bootdomain *bd)
> {
> paddr_t entry, start_info;
> struct domain *d = bd->d;
> diff --git a/xen/arch/x86/include/asm/boot-domain.h b/xen/arch/x86/include/asm/boot-domain.h
> deleted file mode 100644
> index 242e9c9c2b..0000000000
> --- a/xen/arch/x86/include/asm/boot-domain.h
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * Copyright (c) 2024 Apertus Solutions, LLC
> - * Author: Daniel P. Smith <dpsmith@apertussolutions.com>
> - * Copyright (c) 2024 Christopher Clark <christopher.w.clark@gmail.com>
> - */
> -
> -#ifndef __XEN_X86_BOOTDOMAIN_H__
> -#define __XEN_X86_BOOTDOMAIN_H__
> -
> -#include <public/xen.h>
> -
> -struct boot_domain {
> - domid_t domid;
> -
> - struct bootmodule *kernel;
> - struct bootmodule *module;
> - const char *cmdline;
> -
> - struct domain *d;
> -};
> -
> -#endif
> -
> -/*
> - * Local variables:
> - * mode: C
> - * c-file-style: "BSD"
> - * c-basic-offset: 4
> - * tab-width: 4
> - * indent-tabs-mode: nil
> - * End:
> - */
> diff --git a/xen/arch/x86/include/asm/bootfdt.h b/xen/arch/x86/include/asm/bootfdt.h
> index 2fc705a1cd..b30132381e 100644
> --- a/xen/arch/x86/include/asm/bootfdt.h
> +++ b/xen/arch/x86/include/asm/bootfdt.h
> @@ -4,6 +4,13 @@
>
> #include <xen/types.h>
>
> +#include <public/xen.h>
> +
> +struct arch_bootdomain
> +{
> + domid_t domid;
> +};
> +
> struct arch_bootmodule
> {
> /*
> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> index f3210b7d6a..b8280ba357 100644
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -12,7 +12,6 @@
> #include <xen/init.h>
> #include <xen/multiboot.h>
> #include <xen/types.h>
> -#include <asm/boot-domain.h>
>
> /* Max number of boot modules a bootloader can provide in addition to Xen */
> #define MAX_NR_BOOTMODS 63
> @@ -34,7 +33,7 @@ struct boot_info {
>
> unsigned int nr_modules;
> struct bootmodule mods[MAX_NR_BOOTMODS + 1];
> - struct boot_domain domains[MAX_NR_BOOTDOMS];
> + struct bootdomain domains[MAX_NR_BOOTDOMS];
> };
>
> /*
> diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
> index ff021c24af..df03189870 100644
> --- a/xen/arch/x86/include/asm/dom0_build.h
> +++ b/xen/arch/x86/include/asm/dom0_build.h
> @@ -13,9 +13,9 @@ unsigned long dom0_compute_nr_pages(struct domain *d,
> unsigned long initrd_len);
> int dom0_setup_permissions(struct domain *d);
>
> -struct boot_domain;
> -int dom0_construct_pv(const struct boot_domain *bd);
> -int dom0_construct_pvh(const struct boot_domain *bd);
> +struct bootdomain;
> +int dom0_construct_pv(const struct bootdomain *bd);
> +int dom0_construct_pvh(const struct bootdomain *bd);
>
> unsigned long dom0_paging_pages(const struct domain *d,
> unsigned long nr_pages);
> diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
> index c7deaba109..a8647f0fdf 100644
> --- a/xen/arch/x86/include/asm/setup.h
> +++ b/xen/arch/x86/include/asm/setup.h
> @@ -26,8 +26,8 @@ void subarch_init_memory(void);
>
> void init_IRQ(void);
>
> -struct boot_domain;
> -int construct_dom0(const struct boot_domain *bd);
> +struct bootdomain;
> +int construct_dom0(const struct bootdomain *bd);
>
> void setup_io_bitmap(struct domain *d);
>
> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
> index e6c77413f5..a6f212fe0a 100644
> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -355,7 +355,7 @@ static struct page_info * __init alloc_chunk(struct domain *d,
> return page;
> }
>
> -static int __init dom0_construct(const struct boot_domain *bd)
> +static int __init dom0_construct(const struct bootdomain *bd)
> {
> unsigned int i;
> int rc, order, machine;
> @@ -375,7 +375,7 @@ static int __init dom0_construct(const struct boot_domain *bd)
> struct vcpu *v = d->vcpu[0];
>
> struct bootmodule *image = bd->kernel;
> - struct bootmodule *initrd = bd->module;
> + struct bootmodule *initrd = bd->initrd;
> void *image_base;
> unsigned long image_len;
> void *image_start;
> @@ -1070,7 +1070,7 @@ out:
> return rc;
> }
>
> -int __init dom0_construct_pv(const struct boot_domain *bd)
> +int __init dom0_construct_pv(const struct bootdomain *bd)
> {
> unsigned long cr4 = read_cr4();
> int rc;
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index e9a70c2c2b..726adad0e5 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -297,7 +297,9 @@ static const char *cmdline_cook(const char *p, const char *loader_name);
> struct boot_info __initdata xen_boot_info = {
> .loader = "unknown",
> .cmdline = "",
> - .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = { .domid = DOMID_INVALID } },
> + .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = {
> + .arch.domid = DOMID_INVALID
> + }},
> /*
> * There's a MAX_NR_BOOTMODS-th entry in the array. It's not off by one.
> *
> @@ -987,7 +989,7 @@ static unsigned int __init copy_bios_e820(struct e820entry *map, unsigned int li
> }
>
> static size_t __init domain_cmdline_size(const struct boot_info *bi,
> - const struct boot_domain *bd)
> + const struct bootdomain *bd)
> {
> size_t s = bi->kextra ? strlen(bi->kextra) : 0;
> const struct arch_bootmodule *abm = &bd->kernel->arch;
> @@ -1022,7 +1024,7 @@ static struct domain *__init create_dom0(struct boot_info *bi)
> .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
> },
> };
> - struct boot_domain *bd = &bi->domains[0];
> + struct bootdomain *bd = &bi->domains[0];
> struct domain *d;
>
> if ( opt_dom0_pvh )
> @@ -1039,11 +1041,11 @@ static struct domain *__init create_dom0(struct boot_info *bi)
> dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>
> /* Create initial domain. Not d0 for pvshim. */
> - bd->domid = get_initial_domain_id();
> - d = domain_create(bd->domid, &dom0_cfg,
> + bd->arch.domid = get_initial_domain_id();
> + d = domain_create(bd->arch.domid, &dom0_cfg,
> pv_shim ? 0 : CDF_privileged | CDF_hardware);
> if ( IS_ERR(d) )
> - panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d));
> + panic("Error creating d%u: %ld\n", bd->arch.domid, PTR_ERR(d));
>
> init_dom0_cpuid_policy(d);
>
> @@ -2162,7 +2164,7 @@ void asmlinkage __init noreturn __start_xen(void)
> if ( initrdidx < MAX_NR_BOOTMODS )
> {
> bi->mods[initrdidx].kind = BOOTMOD_RAMDISK;
> - bi->domains[0].module = &bi->mods[initrdidx];
> + bi->domains[0].initrd = &bi->mods[initrdidx];
> if ( first_boot_module_index(bi, BOOTMOD_UNKNOWN) < MAX_NR_BOOTMODS )
> printk(XENLOG_WARNING
> "Multiple initrd candidates, picking module #%u\n",
> diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
> index e6d52a599f..19d2ff0f0c 100644
> --- a/xen/include/xen/bootfdt.h
> +++ b/xen/include/xen/bootfdt.h
> @@ -108,6 +108,10 @@ struct bootdomain {
> struct bootmodule *initrd;
>
> const char* cmdline;
> +
> +#if __has_include(<asm/bootfdt.h>)
> + struct arch_bootdomain arch;
> +#endif
> };
One suggestion for improvement:
diff --git a/xen/arch/x86/include/asm/bootfdt.h b/xen/arch/x86/include/asm/bootfdt.h
index b30132381e..892a0e2373 100644
--- a/xen/arch/x86/include/asm/bootfdt.h
+++ b/xen/arch/x86/include/asm/bootfdt.h
@@ -6,11 +6,6 @@
#include <public/xen.h>
-struct arch_bootdomain
-{
- domid_t domid;
-};
-
struct arch_bootmodule
{
/*
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 726adad0e5..39f294b88b 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -297,9 +297,6 @@ static const char *cmdline_cook(const char *p, const char *loader_name);
struct boot_info __initdata xen_boot_info = {
.loader = "unknown",
.cmdline = "",
- .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = {
- .arch.domid = DOMID_INVALID
- }},
/*
* There's a MAX_NR_BOOTMODS-th entry in the array. It's not off by one.
*
@@ -1026,6 +1023,7 @@ static struct domain *__init create_dom0(struct boot_info *bi)
};
struct bootdomain *bd = &bi->domains[0];
struct domain *d;
+ domid_t domid;
if ( opt_dom0_pvh )
{
@@ -1041,11 +1039,11 @@ static struct domain *__init create_dom0(struct boot_info *bi)
dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
/* Create initial domain. Not d0 for pvshim. */
- bd->arch.domid = get_initial_domain_id();
- d = domain_create(bd->arch.domid, &dom0_cfg,
+ domid = get_initial_domain_id();
+ d = domain_create(domid, &dom0_cfg,
pv_shim ? 0 : CDF_privileged | CDF_hardware);
if ( IS_ERR(d) )
- panic("Error creating d%u: %ld\n", bd->arch.domid, PTR_ERR(d));
+ panic("Error creating d%u: %ld\n", domid, PTR_ERR(d));
init_dom0_cpuid_policy(d);
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index 19d2ff0f0c..e6d52a599f 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -108,10 +108,6 @@ struct bootdomain {
struct bootmodule *initrd;
const char* cmdline;
-
-#if __has_include(<asm/bootfdt.h>)
- struct arch_bootdomain arch;
-#endif
};
/*
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v2 07/15] x86: Replace boot_domain with bootdomain
2025-06-06 23:28 ` Stefano Stabellini
@ 2025-06-09 8:57 ` Alejandro Vallejo
2025-06-09 16:56 ` Stefano Stabellini
0 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-09 8:57 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Julien Grall, Bertrand Marquis, Michal Orzel, Daniel P. Smith
On Sat Jun 7, 2025 at 1:28 AM CEST, Stefano Stabellini wrote:
> On Thu, 5 Jun 2025, Alejandro Vallejo wrote:
>> No functional change intended.
>>
>> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
>> ---
>> v2:
>> * Replaces the previous patch in which kernel_info replaced boot_domain
>> ---
>> xen/arch/x86/dom0_build.c | 2 +-
>> xen/arch/x86/hvm/dom0_build.c | 6 ++---
>> xen/arch/x86/include/asm/boot-domain.h | 33 --------------------------
>> xen/arch/x86/include/asm/bootfdt.h | 7 ++++++
>> xen/arch/x86/include/asm/bootinfo.h | 3 +--
>> xen/arch/x86/include/asm/dom0_build.h | 6 ++---
>> xen/arch/x86/include/asm/setup.h | 4 ++--
>> xen/arch/x86/pv/dom0_build.c | 6 ++---
>> xen/arch/x86/setup.c | 16 +++++++------
>> xen/include/xen/bootfdt.h | 4 ++++
>> 10 files changed, 33 insertions(+), 54 deletions(-)
>> delete mode 100644 xen/arch/x86/include/asm/boot-domain.h
>>
>> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
>> index 0b467fd4a4..1c8c5140a3 100644
>> --- a/xen/arch/x86/dom0_build.c
>> +++ b/xen/arch/x86/dom0_build.c
>> @@ -615,7 +615,7 @@ int __init dom0_setup_permissions(struct domain *d)
>> return rc;
>> }
>>
>> -int __init construct_dom0(const struct boot_domain *bd)
>> +int __init construct_dom0(const struct bootdomain *bd)
>> {
>> int rc;
>> const struct domain *d = bd->d;
>> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
>> index 96410344a8..85c000b259 100644
>> --- a/xen/arch/x86/hvm/dom0_build.c
>> +++ b/xen/arch/x86/hvm/dom0_build.c
>> @@ -644,11 +644,11 @@ static bool __init check_and_adjust_load_address(
>> }
>>
>> static int __init pvh_load_kernel(
>> - const struct boot_domain *bd, paddr_t *entry, paddr_t *start_info_addr)
>> + const struct bootdomain *bd, paddr_t *entry, paddr_t *start_info_addr)
>> {
>> struct domain *d = bd->d;
>> struct bootmodule *image = bd->kernel;
>> - struct bootmodule *initrd = bd->module;
>> + struct bootmodule *initrd = bd->initrd;
>> void *image_base = bootstrap_map_bm(image);
>> void *image_start = image_base + image->arch.headroom;
>> unsigned long image_len = image->size;
>> @@ -1329,7 +1329,7 @@ static void __hwdom_init pvh_setup_mmcfg(struct domain *d)
>> }
>> }
>>
>> -int __init dom0_construct_pvh(const struct boot_domain *bd)
>> +int __init dom0_construct_pvh(const struct bootdomain *bd)
>> {
>> paddr_t entry, start_info;
>> struct domain *d = bd->d;
>> diff --git a/xen/arch/x86/include/asm/boot-domain.h b/xen/arch/x86/include/asm/boot-domain.h
>> deleted file mode 100644
>> index 242e9c9c2b..0000000000
>> --- a/xen/arch/x86/include/asm/boot-domain.h
>> +++ /dev/null
>> @@ -1,33 +0,0 @@
>> -/* SPDX-License-Identifier: GPL-2.0-or-later */
>> -/*
>> - * Copyright (c) 2024 Apertus Solutions, LLC
>> - * Author: Daniel P. Smith <dpsmith@apertussolutions.com>
>> - * Copyright (c) 2024 Christopher Clark <christopher.w.clark@gmail.com>
>> - */
>> -
>> -#ifndef __XEN_X86_BOOTDOMAIN_H__
>> -#define __XEN_X86_BOOTDOMAIN_H__
>> -
>> -#include <public/xen.h>
>> -
>> -struct boot_domain {
>> - domid_t domid;
>> -
>> - struct bootmodule *kernel;
>> - struct bootmodule *module;
>> - const char *cmdline;
>> -
>> - struct domain *d;
>> -};
>> -
>> -#endif
>> -
>> -/*
>> - * Local variables:
>> - * mode: C
>> - * c-file-style: "BSD"
>> - * c-basic-offset: 4
>> - * tab-width: 4
>> - * indent-tabs-mode: nil
>> - * End:
>> - */
>> diff --git a/xen/arch/x86/include/asm/bootfdt.h b/xen/arch/x86/include/asm/bootfdt.h
>> index 2fc705a1cd..b30132381e 100644
>> --- a/xen/arch/x86/include/asm/bootfdt.h
>> +++ b/xen/arch/x86/include/asm/bootfdt.h
>> @@ -4,6 +4,13 @@
>>
>> #include <xen/types.h>
>>
>> +#include <public/xen.h>
>> +
>> +struct arch_bootdomain
>> +{
>> + domid_t domid;
>> +};
>> +
>> struct arch_bootmodule
>> {
>> /*
>> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
>> index f3210b7d6a..b8280ba357 100644
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -12,7 +12,6 @@
>> #include <xen/init.h>
>> #include <xen/multiboot.h>
>> #include <xen/types.h>
>> -#include <asm/boot-domain.h>
>>
>> /* Max number of boot modules a bootloader can provide in addition to Xen */
>> #define MAX_NR_BOOTMODS 63
>> @@ -34,7 +33,7 @@ struct boot_info {
>>
>> unsigned int nr_modules;
>> struct bootmodule mods[MAX_NR_BOOTMODS + 1];
>> - struct boot_domain domains[MAX_NR_BOOTDOMS];
>> + struct bootdomain domains[MAX_NR_BOOTDOMS];
>> };
>>
>> /*
>> diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
>> index ff021c24af..df03189870 100644
>> --- a/xen/arch/x86/include/asm/dom0_build.h
>> +++ b/xen/arch/x86/include/asm/dom0_build.h
>> @@ -13,9 +13,9 @@ unsigned long dom0_compute_nr_pages(struct domain *d,
>> unsigned long initrd_len);
>> int dom0_setup_permissions(struct domain *d);
>>
>> -struct boot_domain;
>> -int dom0_construct_pv(const struct boot_domain *bd);
>> -int dom0_construct_pvh(const struct boot_domain *bd);
>> +struct bootdomain;
>> +int dom0_construct_pv(const struct bootdomain *bd);
>> +int dom0_construct_pvh(const struct bootdomain *bd);
>>
>> unsigned long dom0_paging_pages(const struct domain *d,
>> unsigned long nr_pages);
>> diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
>> index c7deaba109..a8647f0fdf 100644
>> --- a/xen/arch/x86/include/asm/setup.h
>> +++ b/xen/arch/x86/include/asm/setup.h
>> @@ -26,8 +26,8 @@ void subarch_init_memory(void);
>>
>> void init_IRQ(void);
>>
>> -struct boot_domain;
>> -int construct_dom0(const struct boot_domain *bd);
>> +struct bootdomain;
>> +int construct_dom0(const struct bootdomain *bd);
>>
>> void setup_io_bitmap(struct domain *d);
>>
>> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
>> index e6c77413f5..a6f212fe0a 100644
>> --- a/xen/arch/x86/pv/dom0_build.c
>> +++ b/xen/arch/x86/pv/dom0_build.c
>> @@ -355,7 +355,7 @@ static struct page_info * __init alloc_chunk(struct domain *d,
>> return page;
>> }
>>
>> -static int __init dom0_construct(const struct boot_domain *bd)
>> +static int __init dom0_construct(const struct bootdomain *bd)
>> {
>> unsigned int i;
>> int rc, order, machine;
>> @@ -375,7 +375,7 @@ static int __init dom0_construct(const struct boot_domain *bd)
>> struct vcpu *v = d->vcpu[0];
>>
>> struct bootmodule *image = bd->kernel;
>> - struct bootmodule *initrd = bd->module;
>> + struct bootmodule *initrd = bd->initrd;
>> void *image_base;
>> unsigned long image_len;
>> void *image_start;
>> @@ -1070,7 +1070,7 @@ out:
>> return rc;
>> }
>>
>> -int __init dom0_construct_pv(const struct boot_domain *bd)
>> +int __init dom0_construct_pv(const struct bootdomain *bd)
>> {
>> unsigned long cr4 = read_cr4();
>> int rc;
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index e9a70c2c2b..726adad0e5 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -297,7 +297,9 @@ static const char *cmdline_cook(const char *p, const char *loader_name);
>> struct boot_info __initdata xen_boot_info = {
>> .loader = "unknown",
>> .cmdline = "",
>> - .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = { .domid = DOMID_INVALID } },
>> + .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = {
>> + .arch.domid = DOMID_INVALID
>> + }},
>> /*
>> * There's a MAX_NR_BOOTMODS-th entry in the array. It's not off by one.
>> *
>> @@ -987,7 +989,7 @@ static unsigned int __init copy_bios_e820(struct e820entry *map, unsigned int li
>> }
>>
>> static size_t __init domain_cmdline_size(const struct boot_info *bi,
>> - const struct boot_domain *bd)
>> + const struct bootdomain *bd)
>> {
>> size_t s = bi->kextra ? strlen(bi->kextra) : 0;
>> const struct arch_bootmodule *abm = &bd->kernel->arch;
>> @@ -1022,7 +1024,7 @@ static struct domain *__init create_dom0(struct boot_info *bi)
>> .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
>> },
>> };
>> - struct boot_domain *bd = &bi->domains[0];
>> + struct bootdomain *bd = &bi->domains[0];
>> struct domain *d;
>>
>> if ( opt_dom0_pvh )
>> @@ -1039,11 +1041,11 @@ static struct domain *__init create_dom0(struct boot_info *bi)
>> dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>>
>> /* Create initial domain. Not d0 for pvshim. */
>> - bd->domid = get_initial_domain_id();
>> - d = domain_create(bd->domid, &dom0_cfg,
>> + bd->arch.domid = get_initial_domain_id();
>> + d = domain_create(bd->arch.domid, &dom0_cfg,
>> pv_shim ? 0 : CDF_privileged | CDF_hardware);
>> if ( IS_ERR(d) )
>> - panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d));
>> + panic("Error creating d%u: %ld\n", bd->arch.domid, PTR_ERR(d));
>>
>> init_dom0_cpuid_policy(d);
>>
>> @@ -2162,7 +2164,7 @@ void asmlinkage __init noreturn __start_xen(void)
>> if ( initrdidx < MAX_NR_BOOTMODS )
>> {
>> bi->mods[initrdidx].kind = BOOTMOD_RAMDISK;
>> - bi->domains[0].module = &bi->mods[initrdidx];
>> + bi->domains[0].initrd = &bi->mods[initrdidx];
>> if ( first_boot_module_index(bi, BOOTMOD_UNKNOWN) < MAX_NR_BOOTMODS )
>> printk(XENLOG_WARNING
>> "Multiple initrd candidates, picking module #%u\n",
>> diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
>> index e6d52a599f..19d2ff0f0c 100644
>> --- a/xen/include/xen/bootfdt.h
>> +++ b/xen/include/xen/bootfdt.h
>> @@ -108,6 +108,10 @@ struct bootdomain {
>> struct bootmodule *initrd;
>>
>> const char* cmdline;
>> +
>> +#if __has_include(<asm/bootfdt.h>)
>> + struct arch_bootdomain arch;
>> +#endif
>> };
>
> One suggestion for improvement:
That would function today, but that field is important later on when we
retrieve the domid from the DTB. That's where it's stored until we create them.
>
> diff --git a/xen/arch/x86/include/asm/bootfdt.h b/xen/arch/x86/include/asm/bootfdt.h
> index b30132381e..892a0e2373 100644
> --- a/xen/arch/x86/include/asm/bootfdt.h
> +++ b/xen/arch/x86/include/asm/bootfdt.h
> @@ -6,11 +6,6 @@
>
> #include <public/xen.h>
>
> -struct arch_bootdomain
> -{
> - domid_t domid;
> -};
> -
> struct arch_bootmodule
> {
> /*
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 726adad0e5..39f294b88b 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -297,9 +297,6 @@ static const char *cmdline_cook(const char *p, const char *loader_name);
> struct boot_info __initdata xen_boot_info = {
> .loader = "unknown",
> .cmdline = "",
> - .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = {
> - .arch.domid = DOMID_INVALID
> - }},
> /*
> * There's a MAX_NR_BOOTMODS-th entry in the array. It's not off by one.
> *
> @@ -1026,6 +1023,7 @@ static struct domain *__init create_dom0(struct boot_info *bi)
> };
> struct bootdomain *bd = &bi->domains[0];
> struct domain *d;
> + domid_t domid;
>
> if ( opt_dom0_pvh )
> {
> @@ -1041,11 +1039,11 @@ static struct domain *__init create_dom0(struct boot_info *bi)
> dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>
> /* Create initial domain. Not d0 for pvshim. */
> - bd->arch.domid = get_initial_domain_id();
> - d = domain_create(bd->arch.domid, &dom0_cfg,
> + domid = get_initial_domain_id();
> + d = domain_create(domid, &dom0_cfg,
> pv_shim ? 0 : CDF_privileged | CDF_hardware);
> if ( IS_ERR(d) )
> - panic("Error creating d%u: %ld\n", bd->arch.domid, PTR_ERR(d));
> + panic("Error creating d%u: %ld\n", domid, PTR_ERR(d));
>
> init_dom0_cpuid_policy(d);
>
> diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
> index 19d2ff0f0c..e6d52a599f 100644
> --- a/xen/include/xen/bootfdt.h
> +++ b/xen/include/xen/bootfdt.h
> @@ -108,10 +108,6 @@ struct bootdomain {
> struct bootmodule *initrd;
>
> const char* cmdline;
> -
> -#if __has_include(<asm/bootfdt.h>)
> - struct arch_bootdomain arch;
> -#endif
> };
>
> /*
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v2 07/15] x86: Replace boot_domain with bootdomain
2025-06-09 8:57 ` Alejandro Vallejo
@ 2025-06-09 16:56 ` Stefano Stabellini
0 siblings, 0 replies; 39+ messages in thread
From: Stefano Stabellini @ 2025-06-09 16:56 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Stefano Stabellini, xen-devel, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Julien Grall, Bertrand Marquis,
Michal Orzel, Daniel P. Smith
On Mon, 9 Jun 2025, Alejandro Vallejo wrote:
> On Sat Jun 7, 2025 at 1:28 AM CEST, Stefano Stabellini wrote:
> > On Thu, 5 Jun 2025, Alejandro Vallejo wrote:
> >> No functional change intended.
> >>
> >> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
> >> ---
> >> v2:
> >> * Replaces the previous patch in which kernel_info replaced boot_domain
> >> ---
> >> xen/arch/x86/dom0_build.c | 2 +-
> >> xen/arch/x86/hvm/dom0_build.c | 6 ++---
> >> xen/arch/x86/include/asm/boot-domain.h | 33 --------------------------
> >> xen/arch/x86/include/asm/bootfdt.h | 7 ++++++
> >> xen/arch/x86/include/asm/bootinfo.h | 3 +--
> >> xen/arch/x86/include/asm/dom0_build.h | 6 ++---
> >> xen/arch/x86/include/asm/setup.h | 4 ++--
> >> xen/arch/x86/pv/dom0_build.c | 6 ++---
> >> xen/arch/x86/setup.c | 16 +++++++------
> >> xen/include/xen/bootfdt.h | 4 ++++
> >> 10 files changed, 33 insertions(+), 54 deletions(-)
> >> delete mode 100644 xen/arch/x86/include/asm/boot-domain.h
> >>
> >> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> >> index 0b467fd4a4..1c8c5140a3 100644
> >> --- a/xen/arch/x86/dom0_build.c
> >> +++ b/xen/arch/x86/dom0_build.c
> >> @@ -615,7 +615,7 @@ int __init dom0_setup_permissions(struct domain *d)
> >> return rc;
> >> }
> >>
> >> -int __init construct_dom0(const struct boot_domain *bd)
> >> +int __init construct_dom0(const struct bootdomain *bd)
> >> {
> >> int rc;
> >> const struct domain *d = bd->d;
> >> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> >> index 96410344a8..85c000b259 100644
> >> --- a/xen/arch/x86/hvm/dom0_build.c
> >> +++ b/xen/arch/x86/hvm/dom0_build.c
> >> @@ -644,11 +644,11 @@ static bool __init check_and_adjust_load_address(
> >> }
> >>
> >> static int __init pvh_load_kernel(
> >> - const struct boot_domain *bd, paddr_t *entry, paddr_t *start_info_addr)
> >> + const struct bootdomain *bd, paddr_t *entry, paddr_t *start_info_addr)
> >> {
> >> struct domain *d = bd->d;
> >> struct bootmodule *image = bd->kernel;
> >> - struct bootmodule *initrd = bd->module;
> >> + struct bootmodule *initrd = bd->initrd;
> >> void *image_base = bootstrap_map_bm(image);
> >> void *image_start = image_base + image->arch.headroom;
> >> unsigned long image_len = image->size;
> >> @@ -1329,7 +1329,7 @@ static void __hwdom_init pvh_setup_mmcfg(struct domain *d)
> >> }
> >> }
> >>
> >> -int __init dom0_construct_pvh(const struct boot_domain *bd)
> >> +int __init dom0_construct_pvh(const struct bootdomain *bd)
> >> {
> >> paddr_t entry, start_info;
> >> struct domain *d = bd->d;
> >> diff --git a/xen/arch/x86/include/asm/boot-domain.h b/xen/arch/x86/include/asm/boot-domain.h
> >> deleted file mode 100644
> >> index 242e9c9c2b..0000000000
> >> --- a/xen/arch/x86/include/asm/boot-domain.h
> >> +++ /dev/null
> >> @@ -1,33 +0,0 @@
> >> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> >> -/*
> >> - * Copyright (c) 2024 Apertus Solutions, LLC
> >> - * Author: Daniel P. Smith <dpsmith@apertussolutions.com>
> >> - * Copyright (c) 2024 Christopher Clark <christopher.w.clark@gmail.com>
> >> - */
> >> -
> >> -#ifndef __XEN_X86_BOOTDOMAIN_H__
> >> -#define __XEN_X86_BOOTDOMAIN_H__
> >> -
> >> -#include <public/xen.h>
> >> -
> >> -struct boot_domain {
> >> - domid_t domid;
> >> -
> >> - struct bootmodule *kernel;
> >> - struct bootmodule *module;
> >> - const char *cmdline;
> >> -
> >> - struct domain *d;
> >> -};
> >> -
> >> -#endif
> >> -
> >> -/*
> >> - * Local variables:
> >> - * mode: C
> >> - * c-file-style: "BSD"
> >> - * c-basic-offset: 4
> >> - * tab-width: 4
> >> - * indent-tabs-mode: nil
> >> - * End:
> >> - */
> >> diff --git a/xen/arch/x86/include/asm/bootfdt.h b/xen/arch/x86/include/asm/bootfdt.h
> >> index 2fc705a1cd..b30132381e 100644
> >> --- a/xen/arch/x86/include/asm/bootfdt.h
> >> +++ b/xen/arch/x86/include/asm/bootfdt.h
> >> @@ -4,6 +4,13 @@
> >>
> >> #include <xen/types.h>
> >>
> >> +#include <public/xen.h>
> >> +
> >> +struct arch_bootdomain
> >> +{
> >> + domid_t domid;
> >> +};
> >> +
> >> struct arch_bootmodule
> >> {
> >> /*
> >> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> >> index f3210b7d6a..b8280ba357 100644
> >> --- a/xen/arch/x86/include/asm/bootinfo.h
> >> +++ b/xen/arch/x86/include/asm/bootinfo.h
> >> @@ -12,7 +12,6 @@
> >> #include <xen/init.h>
> >> #include <xen/multiboot.h>
> >> #include <xen/types.h>
> >> -#include <asm/boot-domain.h>
> >>
> >> /* Max number of boot modules a bootloader can provide in addition to Xen */
> >> #define MAX_NR_BOOTMODS 63
> >> @@ -34,7 +33,7 @@ struct boot_info {
> >>
> >> unsigned int nr_modules;
> >> struct bootmodule mods[MAX_NR_BOOTMODS + 1];
> >> - struct boot_domain domains[MAX_NR_BOOTDOMS];
> >> + struct bootdomain domains[MAX_NR_BOOTDOMS];
> >> };
> >>
> >> /*
> >> diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
> >> index ff021c24af..df03189870 100644
> >> --- a/xen/arch/x86/include/asm/dom0_build.h
> >> +++ b/xen/arch/x86/include/asm/dom0_build.h
> >> @@ -13,9 +13,9 @@ unsigned long dom0_compute_nr_pages(struct domain *d,
> >> unsigned long initrd_len);
> >> int dom0_setup_permissions(struct domain *d);
> >>
> >> -struct boot_domain;
> >> -int dom0_construct_pv(const struct boot_domain *bd);
> >> -int dom0_construct_pvh(const struct boot_domain *bd);
> >> +struct bootdomain;
> >> +int dom0_construct_pv(const struct bootdomain *bd);
> >> +int dom0_construct_pvh(const struct bootdomain *bd);
> >>
> >> unsigned long dom0_paging_pages(const struct domain *d,
> >> unsigned long nr_pages);
> >> diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
> >> index c7deaba109..a8647f0fdf 100644
> >> --- a/xen/arch/x86/include/asm/setup.h
> >> +++ b/xen/arch/x86/include/asm/setup.h
> >> @@ -26,8 +26,8 @@ void subarch_init_memory(void);
> >>
> >> void init_IRQ(void);
> >>
> >> -struct boot_domain;
> >> -int construct_dom0(const struct boot_domain *bd);
> >> +struct bootdomain;
> >> +int construct_dom0(const struct bootdomain *bd);
> >>
> >> void setup_io_bitmap(struct domain *d);
> >>
> >> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
> >> index e6c77413f5..a6f212fe0a 100644
> >> --- a/xen/arch/x86/pv/dom0_build.c
> >> +++ b/xen/arch/x86/pv/dom0_build.c
> >> @@ -355,7 +355,7 @@ static struct page_info * __init alloc_chunk(struct domain *d,
> >> return page;
> >> }
> >>
> >> -static int __init dom0_construct(const struct boot_domain *bd)
> >> +static int __init dom0_construct(const struct bootdomain *bd)
> >> {
> >> unsigned int i;
> >> int rc, order, machine;
> >> @@ -375,7 +375,7 @@ static int __init dom0_construct(const struct boot_domain *bd)
> >> struct vcpu *v = d->vcpu[0];
> >>
> >> struct bootmodule *image = bd->kernel;
> >> - struct bootmodule *initrd = bd->module;
> >> + struct bootmodule *initrd = bd->initrd;
> >> void *image_base;
> >> unsigned long image_len;
> >> void *image_start;
> >> @@ -1070,7 +1070,7 @@ out:
> >> return rc;
> >> }
> >>
> >> -int __init dom0_construct_pv(const struct boot_domain *bd)
> >> +int __init dom0_construct_pv(const struct bootdomain *bd)
> >> {
> >> unsigned long cr4 = read_cr4();
> >> int rc;
> >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> >> index e9a70c2c2b..726adad0e5 100644
> >> --- a/xen/arch/x86/setup.c
> >> +++ b/xen/arch/x86/setup.c
> >> @@ -297,7 +297,9 @@ static const char *cmdline_cook(const char *p, const char *loader_name);
> >> struct boot_info __initdata xen_boot_info = {
> >> .loader = "unknown",
> >> .cmdline = "",
> >> - .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = { .domid = DOMID_INVALID } },
> >> + .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = {
> >> + .arch.domid = DOMID_INVALID
> >> + }},
> >> /*
> >> * There's a MAX_NR_BOOTMODS-th entry in the array. It's not off by one.
> >> *
> >> @@ -987,7 +989,7 @@ static unsigned int __init copy_bios_e820(struct e820entry *map, unsigned int li
> >> }
> >>
> >> static size_t __init domain_cmdline_size(const struct boot_info *bi,
> >> - const struct boot_domain *bd)
> >> + const struct bootdomain *bd)
> >> {
> >> size_t s = bi->kextra ? strlen(bi->kextra) : 0;
> >> const struct arch_bootmodule *abm = &bd->kernel->arch;
> >> @@ -1022,7 +1024,7 @@ static struct domain *__init create_dom0(struct boot_info *bi)
> >> .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
> >> },
> >> };
> >> - struct boot_domain *bd = &bi->domains[0];
> >> + struct bootdomain *bd = &bi->domains[0];
> >> struct domain *d;
> >>
> >> if ( opt_dom0_pvh )
> >> @@ -1039,11 +1041,11 @@ static struct domain *__init create_dom0(struct boot_info *bi)
> >> dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
> >>
> >> /* Create initial domain. Not d0 for pvshim. */
> >> - bd->domid = get_initial_domain_id();
> >> - d = domain_create(bd->domid, &dom0_cfg,
> >> + bd->arch.domid = get_initial_domain_id();
> >> + d = domain_create(bd->arch.domid, &dom0_cfg,
> >> pv_shim ? 0 : CDF_privileged | CDF_hardware);
> >> if ( IS_ERR(d) )
> >> - panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d));
> >> + panic("Error creating d%u: %ld\n", bd->arch.domid, PTR_ERR(d));
> >>
> >> init_dom0_cpuid_policy(d);
> >>
> >> @@ -2162,7 +2164,7 @@ void asmlinkage __init noreturn __start_xen(void)
> >> if ( initrdidx < MAX_NR_BOOTMODS )
> >> {
> >> bi->mods[initrdidx].kind = BOOTMOD_RAMDISK;
> >> - bi->domains[0].module = &bi->mods[initrdidx];
> >> + bi->domains[0].initrd = &bi->mods[initrdidx];
> >> if ( first_boot_module_index(bi, BOOTMOD_UNKNOWN) < MAX_NR_BOOTMODS )
> >> printk(XENLOG_WARNING
> >> "Multiple initrd candidates, picking module #%u\n",
> >> diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
> >> index e6d52a599f..19d2ff0f0c 100644
> >> --- a/xen/include/xen/bootfdt.h
> >> +++ b/xen/include/xen/bootfdt.h
> >> @@ -108,6 +108,10 @@ struct bootdomain {
> >> struct bootmodule *initrd;
> >>
> >> const char* cmdline;
> >> +
> >> +#if __has_include(<asm/bootfdt.h>)
> >> + struct arch_bootdomain arch;
> >> +#endif
> >> };
> >
> > One suggestion for improvement:
>
> That would function today, but that field is important later on when we
> retrieve the domid from the DTB. That's where it's stored until we create them.
Oh OK. In that case please add a note to the commit message
> > diff --git a/xen/arch/x86/include/asm/bootfdt.h b/xen/arch/x86/include/asm/bootfdt.h
> > index b30132381e..892a0e2373 100644
> > --- a/xen/arch/x86/include/asm/bootfdt.h
> > +++ b/xen/arch/x86/include/asm/bootfdt.h
> > @@ -6,11 +6,6 @@
> >
> > #include <public/xen.h>
> >
> > -struct arch_bootdomain
> > -{
> > - domid_t domid;
> > -};
> > -
> > struct arch_bootmodule
> > {
> > /*
> > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> > index 726adad0e5..39f294b88b 100644
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -297,9 +297,6 @@ static const char *cmdline_cook(const char *p, const char *loader_name);
> > struct boot_info __initdata xen_boot_info = {
> > .loader = "unknown",
> > .cmdline = "",
> > - .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = {
> > - .arch.domid = DOMID_INVALID
> > - }},
> > /*
> > * There's a MAX_NR_BOOTMODS-th entry in the array. It's not off by one.
> > *
> > @@ -1026,6 +1023,7 @@ static struct domain *__init create_dom0(struct boot_info *bi)
> > };
> > struct bootdomain *bd = &bi->domains[0];
> > struct domain *d;
> > + domid_t domid;
> >
> > if ( opt_dom0_pvh )
> > {
> > @@ -1041,11 +1039,11 @@ static struct domain *__init create_dom0(struct boot_info *bi)
> > dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
> >
> > /* Create initial domain. Not d0 for pvshim. */
> > - bd->arch.domid = get_initial_domain_id();
> > - d = domain_create(bd->arch.domid, &dom0_cfg,
> > + domid = get_initial_domain_id();
> > + d = domain_create(domid, &dom0_cfg,
> > pv_shim ? 0 : CDF_privileged | CDF_hardware);
> > if ( IS_ERR(d) )
> > - panic("Error creating d%u: %ld\n", bd->arch.domid, PTR_ERR(d));
> > + panic("Error creating d%u: %ld\n", domid, PTR_ERR(d));
> >
> > init_dom0_cpuid_policy(d);
> >
> > diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
> > index 19d2ff0f0c..e6d52a599f 100644
> > --- a/xen/include/xen/bootfdt.h
> > +++ b/xen/include/xen/bootfdt.h
> > @@ -108,10 +108,6 @@ struct bootdomain {
> > struct bootmodule *initrd;
> >
> > const char* cmdline;
> > -
> > -#if __has_include(<asm/bootfdt.h>)
> > - struct arch_bootdomain arch;
> > -#endif
> > };
> >
> > /*
>
> Cheers,
> Alejandro
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 08/15] xen/dt: Move bootfdt functions to xen/bootfdt.h
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (6 preceding siblings ...)
2025-06-05 19:47 ` [PATCH v2 07/15] x86: Replace boot_domain with bootdomain Alejandro Vallejo
@ 2025-06-05 19:48 ` Alejandro Vallejo
2025-06-06 8:59 ` Orzel, Michal
2025-06-05 19:48 ` [PATCH v2 09/15] xen/dt: Move bootinfo functions to a new bootinfo.h Alejandro Vallejo
` (6 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:48 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk,
Daniel P. Smith
Part of an unpicking process to extract bootfdt contents independent of bootinfo
to a separate file for x86 to take.
Move functions required for early FDT parsing from device_tree.h and arm's
setup.h onto bootfdt.h
Declaration motion only. Not a functional change.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
v2:
* Remove the u32 identifiers in the device_tree_get_u32() implementation
* Fix an existing const-stripping case.
---
xen/arch/arm/include/asm/setup.h | 6 ----
xen/common/device-tree/bootfdt.c | 8 ++---
xen/include/xen/bootfdt.h | 62 ++++++++++++++++++++++++++++++++
xen/include/xen/device_tree.h | 34 +-----------------
4 files changed, 67 insertions(+), 43 deletions(-)
diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
index 6cf272c160..2b58549c1a 100644
--- a/xen/arch/arm/include/asm/setup.h
+++ b/xen/arch/arm/include/asm/setup.h
@@ -53,12 +53,6 @@ void setup_mm(void);
extern uint32_t hyp_traps_vector[];
void init_traps(void);
-void device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
- uint32_t size_cells, paddr_t *start, paddr_t *size);
-
-u32 device_tree_get_u32(const void *fdt, int node,
- const char *prop_name, u32 dflt);
-
int handle_device(struct domain *d, struct dt_device_node *dev, p2m_type_t p2mt,
struct rangeset *iomem_ranges, struct rangeset *irq_ranges);
diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c
index 9daea06e57..ab449db8d6 100644
--- a/xen/common/device-tree/bootfdt.c
+++ b/xen/common/device-tree/bootfdt.c
@@ -202,16 +202,16 @@ static int __init device_tree_get_meminfo(const void *fdt, int node,
return 0;
}
-u32 __init device_tree_get_u32(const void *fdt, int node,
- const char *prop_name, u32 dflt)
+uint32_t __init device_tree_get_u32(const void *fdt, int node,
+ const char *prop_name, uint32_t dflt)
{
const struct fdt_property *prop;
prop = fdt_get_property(fdt, node, prop_name, NULL);
- if ( !prop || prop->len < sizeof(u32) )
+ if ( !prop || prop->len < sizeof(uint32_t) )
return dflt;
- return fdt32_to_cpu(*(uint32_t*)prop->data);
+ return fdt32_to_cpu(*(const uint32_t*)prop->data);
}
/**
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index 19d2ff0f0c..1b89986e10 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -2,6 +2,7 @@
#ifndef XEN_BOOTFDT_H
#define XEN_BOOTFDT_H
+#include <xen/byteorder.h>
#include <xen/types.h>
#include <xen/kernel.h>
#include <xen/macros.h>
@@ -16,8 +17,53 @@
#define NR_MEM_BANKS 256
#define NR_SHMEM_BANKS 32
+/* Default #address and #size cells */
+#define DT_ROOT_NODE_ADDR_CELLS_DEFAULT 2
+#define DT_ROOT_NODE_SIZE_CELLS_DEFAULT 1
+
#define MAX_MODULES 32 /* Current maximum useful modules */
+#define DEVICE_TREE_MAX_DEPTH 16
+
+/* Helper to read a big number; size is in cells (not bytes) */
+static inline u64 dt_read_number(const __be32 *cell, int size)
+{
+ u64 r = 0;
+
+ while ( size-- )
+ r = (r << 32) | be32_to_cpu(*(cell++));
+ return r;
+}
+
+static inline u64 dt_next_cell(int s, const __be32 **cellp)
+{
+ const __be32 *p = *cellp;
+
+ *cellp = p + s;
+ return dt_read_number(p, s);
+}
+
+typedef int (*device_tree_node_func)(const void *fdt,
+ int node, const char *name, int depth,
+ u32 address_cells, u32 size_cells,
+ void *data);
+
+/**
+ * device_tree_for_each_node - iterate over all device tree sub-nodes
+ * @fdt: flat device tree.
+ * @node: parent node to start the search from
+ * @func: function to call for each sub-node.
+ * @data: data to pass to @func.
+ *
+ * Any nodes nested at DEVICE_TREE_MAX_DEPTH or deeper are ignored.
+ *
+ * Returns 0 if all nodes were iterated over successfully. If @func
+ * returns a value different from 0, that value is returned immediately.
+ */
+int device_tree_for_each_node(const void *fdt, int node,
+ device_tree_node_func func,
+ void *data);
+
typedef enum {
BOOTMOD_XEN,
BOOTMOD_FDT,
@@ -261,4 +307,20 @@ static inline struct membanks *membanks_xzalloc(unsigned int nr,
return banks;
}
+/*
+ * Interpret the property `prop_name` of `node` as a u32.
+ *
+ * Returns the property value on success; otherwise returns `dflt`.
+ */
+uint32_t device_tree_get_u32(const void *fdt, int node,
+ const char *prop_name, uint32_t dflt);
+
+/*
+ * Interpret the property `prop_name` of `node` as a "reg".
+ *
+ * Returns outputs in `start` and `size`.
+ */
+void device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
+ uint32_t size_cells, paddr_t *start, paddr_t *size);
+
#endif /* XEN_BOOTFDT_H */
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 6dc1fb5159..0a22b1ba1d 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -10,6 +10,7 @@
#ifndef __XEN_DEVICE_TREE_H__
#define __XEN_DEVICE_TREE_H__
+#include <xen/bootfdt.h>
#include <xen/byteorder.h>
#include <asm/device.h>
@@ -22,8 +23,6 @@
#include <xen/list.h>
#include <xen/rwlock.h>
-#define DEVICE_TREE_MAX_DEPTH 16
-
/*
* Struct used for matching a device
*/
@@ -164,17 +163,8 @@ struct dt_raw_irq {
u32 specifier[DT_MAX_IRQ_SPEC];
};
-typedef int (*device_tree_node_func)(const void *fdt,
- int node, const char *name, int depth,
- u32 address_cells, u32 size_cells,
- void *data);
-
extern const void *device_tree_flattened;
-int device_tree_for_each_node(const void *fdt, int node,
- device_tree_node_func func,
- void *data);
-
/**
* dt_unflatten_host_device_tree - Unflatten the host device tree
*
@@ -245,10 +235,6 @@ void intc_dt_preinit(void);
#define dt_node_cmp(s1, s2) strcasecmp((s1), (s2))
#define dt_compat_cmp(s1, s2) strcasecmp((s1), (s2))
-/* Default #address and #size cells */
-#define DT_ROOT_NODE_ADDR_CELLS_DEFAULT 2
-#define DT_ROOT_NODE_SIZE_CELLS_DEFAULT 1
-
#define dt_for_each_property_node(dn, pp) \
for ( pp = (dn)->properties; (pp) != NULL; pp = (pp)->next )
@@ -258,16 +244,6 @@ void intc_dt_preinit(void);
#define dt_for_each_child_node(dt, dn) \
for ( dn = (dt)->child; (dn) != NULL; dn = (dn)->sibling )
-/* Helper to read a big number; size is in cells (not bytes) */
-static inline u64 dt_read_number(const __be32 *cell, int size)
-{
- u64 r = 0;
-
- while ( size-- )
- r = (r << 32) | be32_to_cpu(*(cell++));
- return r;
-}
-
/* Wrapper for dt_read_number() to return paddr_t (instead of uint64_t) */
static inline paddr_t dt_read_paddr(const __be32 *cell, int size)
{
@@ -307,14 +283,6 @@ static inline int dt_size_to_cells(int bytes)
return (bytes / sizeof(u32));
}
-static inline u64 dt_next_cell(int s, const __be32 **cellp)
-{
- const __be32 *p = *cellp;
-
- *cellp = p + s;
- return dt_read_number(p, s);
-}
-
static inline const char *dt_node_full_name(const struct dt_device_node *np)
{
return (np && np->full_name) ? np->full_name : "<no-node>";
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v2 08/15] xen/dt: Move bootfdt functions to xen/bootfdt.h
2025-06-05 19:48 ` [PATCH v2 08/15] xen/dt: Move bootfdt functions to xen/bootfdt.h Alejandro Vallejo
@ 2025-06-06 8:59 ` Orzel, Michal
2025-06-06 10:08 ` Alejandro Vallejo
0 siblings, 1 reply; 39+ messages in thread
From: Orzel, Michal @ 2025-06-06 8:59 UTC (permalink / raw)
To: Alejandro Vallejo, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
Volodymyr Babchuk, Daniel P. Smith
On 05/06/2025 21:48, Alejandro Vallejo wrote:
> Part of an unpicking process to extract bootfdt contents independent of bootinfo
> to a separate file for x86 to take.
>
> Move functions required for early FDT parsing from device_tree.h and arm's
> setup.h onto bootfdt.h
>
> Declaration motion only. Not a functional change.
>
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
> ---
> v2:
> * Remove the u32 identifiers in the device_tree_get_u32() implementation
I don't understand the reasoning behind changing u32->uint32_t only for one
function in this patch while leaving others unmodified. Also what about u64?
Either don't change any or change all.
> * Fix an existing const-stripping case.
These points should be mentioned in the commit msg.
~Michal
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 08/15] xen/dt: Move bootfdt functions to xen/bootfdt.h
2025-06-06 8:59 ` Orzel, Michal
@ 2025-06-06 10:08 ` Alejandro Vallejo
2025-06-06 19:59 ` Stefano Stabellini
0 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-06 10:08 UTC (permalink / raw)
To: Orzel, Michal, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
Volodymyr Babchuk, Daniel P. Smith
On Fri Jun 6, 2025 at 10:59 AM CEST, Michal Orzel wrote:
>
>
> On 05/06/2025 21:48, Alejandro Vallejo wrote:
>> Part of an unpicking process to extract bootfdt contents independent of bootinfo
>> to a separate file for x86 to take.
>>
>> Move functions required for early FDT parsing from device_tree.h and arm's
>> setup.h onto bootfdt.h
>>
>> Declaration motion only. Not a functional change.
>>
>> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
>> ---
>> v2:
>> * Remove the u32 identifiers in the device_tree_get_u32() implementation
> I don't understand the reasoning behind changing u32->uint32_t only for one
> function in this patch while leaving others unmodified. Also what about u64?
> Either don't change any or change all.
Sure. Let's call the original u32->uint32_t change a misplaced mutation and
move on. The point is the motion, not these cleanups on top.
>
>> * Fix an existing const-stripping case.
> These points should be mentioned in the commit msg.
Sure.
>
> ~Michal
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 08/15] xen/dt: Move bootfdt functions to xen/bootfdt.h
2025-06-06 10:08 ` Alejandro Vallejo
@ 2025-06-06 19:59 ` Stefano Stabellini
2025-06-09 8:59 ` Alejandro Vallejo
0 siblings, 1 reply; 39+ messages in thread
From: Stefano Stabellini @ 2025-06-06 19:59 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Orzel, Michal, xen-devel, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Volodymyr Babchuk, Daniel P. Smith
On Fri, 6 Jun 2025, Alejandro Vallejo wrote:
> On Fri Jun 6, 2025 at 10:59 AM CEST, Michal Orzel wrote:
> >
> >
> > On 05/06/2025 21:48, Alejandro Vallejo wrote:
> >> Part of an unpicking process to extract bootfdt contents independent of bootinfo
> >> to a separate file for x86 to take.
> >>
> >> Move functions required for early FDT parsing from device_tree.h and arm's
> >> setup.h onto bootfdt.h
> >>
> >> Declaration motion only. Not a functional change.
> >>
> >> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
> >> ---
> >> v2:
> >> * Remove the u32 identifiers in the device_tree_get_u32() implementation
> > I don't understand the reasoning behind changing u32->uint32_t only for one
> > function in this patch while leaving others unmodified. Also what about u64?
> > Either don't change any or change all.
>
> Sure. Let's call the original u32->uint32_t change a misplaced mutation and
> move on. The point is the motion, not these cleanups on top.
Yes I agree. I know from past experience that Jan doesn't mind changes
during code movements, but for me it is important that changes and code
movement are separate. That is because I have almost automatic ways to
check that code movement is correct if there are no changes. It saves me
a lot of time during review. Then I can look at the individual changes
separately.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 08/15] xen/dt: Move bootfdt functions to xen/bootfdt.h
2025-06-06 19:59 ` Stefano Stabellini
@ 2025-06-09 8:59 ` Alejandro Vallejo
2025-06-09 16:55 ` Stefano Stabellini
0 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-09 8:59 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Orzel, Michal, xen-devel, Julien Grall, Bertrand Marquis,
Volodymyr Babchuk, Daniel P. Smith
On Fri Jun 6, 2025 at 9:59 PM CEST, Stefano Stabellini wrote:
> On Fri, 6 Jun 2025, Alejandro Vallejo wrote:
>> On Fri Jun 6, 2025 at 10:59 AM CEST, Michal Orzel wrote:
>> >
>> >
>> > On 05/06/2025 21:48, Alejandro Vallejo wrote:
>> >> Part of an unpicking process to extract bootfdt contents independent of bootinfo
>> >> to a separate file for x86 to take.
>> >>
>> >> Move functions required for early FDT parsing from device_tree.h and arm's
>> >> setup.h onto bootfdt.h
>> >>
>> >> Declaration motion only. Not a functional change.
>> >>
>> >> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
>> >> ---
>> >> v2:
>> >> * Remove the u32 identifiers in the device_tree_get_u32() implementation
>> > I don't understand the reasoning behind changing u32->uint32_t only for one
>> > function in this patch while leaving others unmodified. Also what about u64?
>> > Either don't change any or change all.
>>
>> Sure. Let's call the original u32->uint32_t change a misplaced mutation and
>> move on. The point is the motion, not these cleanups on top.
>
> Yes I agree. I know from past experience that Jan doesn't mind changes
> during code movements, but for me it is important that changes and code
> movement are separate. That is because I have almost automatic ways to
> check that code movement is correct if there are no changes. It saves me
> a lot of time during review. Then I can look at the individual changes
> separately.
That's interesting. Could you please share the runes? That's one side of
review I still struggle with.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 08/15] xen/dt: Move bootfdt functions to xen/bootfdt.h
2025-06-09 8:59 ` Alejandro Vallejo
@ 2025-06-09 16:55 ` Stefano Stabellini
0 siblings, 0 replies; 39+ messages in thread
From: Stefano Stabellini @ 2025-06-09 16:55 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Stefano Stabellini, Orzel, Michal, xen-devel, Julien Grall,
Bertrand Marquis, Volodymyr Babchuk, Daniel P. Smith
On Mon, 9 Jun 2025, Alejandro Vallejo wrote:
> On Fri Jun 6, 2025 at 9:59 PM CEST, Stefano Stabellini wrote:
> > On Fri, 6 Jun 2025, Alejandro Vallejo wrote:
> >> On Fri Jun 6, 2025 at 10:59 AM CEST, Michal Orzel wrote:
> >> >
> >> >
> >> > On 05/06/2025 21:48, Alejandro Vallejo wrote:
> >> >> Part of an unpicking process to extract bootfdt contents independent of bootinfo
> >> >> to a separate file for x86 to take.
> >> >>
> >> >> Move functions required for early FDT parsing from device_tree.h and arm's
> >> >> setup.h onto bootfdt.h
> >> >>
> >> >> Declaration motion only. Not a functional change.
> >> >>
> >> >> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
> >> >> ---
> >> >> v2:
> >> >> * Remove the u32 identifiers in the device_tree_get_u32() implementation
> >> > I don't understand the reasoning behind changing u32->uint32_t only for one
> >> > function in this patch while leaving others unmodified. Also what about u64?
> >> > Either don't change any or change all.
> >>
> >> Sure. Let's call the original u32->uint32_t change a misplaced mutation and
> >> move on. The point is the motion, not these cleanups on top.
> >
> > Yes I agree. I know from past experience that Jan doesn't mind changes
> > during code movements, but for me it is important that changes and code
> > movement are separate. That is because I have almost automatic ways to
> > check that code movement is correct if there are no changes. It saves me
> > a lot of time during review. Then I can look at the individual changes
> > separately.
>
> That's interesting. Could you please share the runes? That's one side of
> review I still struggle with.
I use vim to generate two source files, one with the code corresponding
to the - lines (with the - prefix) and one with the code corresponding
to the + lines (without the + prefix). Then I diff the two files. This
method is a bit crude but very effective and managed to spot issues this
way more than once.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 09/15] xen/dt: Move bootinfo functions to a new bootinfo.h
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (7 preceding siblings ...)
2025-06-05 19:48 ` [PATCH v2 08/15] xen/dt: Move bootfdt functions to xen/bootfdt.h Alejandro Vallejo
@ 2025-06-05 19:48 ` Alejandro Vallejo
2025-06-06 23:39 ` Stefano Stabellini
2025-06-05 19:48 ` [PATCH v2 10/15] xen/dt: Rename bootfdt.c -> bootinfo-fdt.c Alejandro Vallejo
` (5 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:48 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
Anthony PERARD, Jan Beulich, Roger Pau Monné,
Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko,
Daniel P. Smith
Part of an unpicking process to extract bootfdt contents independent of
bootinfo to a separate file for x86 to take.
With this, bootfdt.h can be cleanly included from x86. A later patch
extracts the definitions so the functions may be called too.
Not a functional change.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
xen/arch/arm/domain_build.c | 1 +
xen/arch/arm/mmu/mm.c | 1 +
xen/arch/arm/setup.c | 1 +
xen/arch/riscv/mm.c | 2 +-
xen/arch/riscv/setup.c | 2 +-
xen/common/device-tree/bootfdt.c | 2 +-
xen/common/device-tree/bootinfo.c | 2 +-
xen/common/device-tree/dom0less-build.c | 2 +-
xen/common/device-tree/domain-build.c | 2 +-
xen/common/device-tree/kernel.c | 2 +-
xen/include/xen/bootfdt.h | 208 -----------------------
xen/include/xen/bootinfo.h | 213 ++++++++++++++++++++++++
xen/include/xen/device_tree.h | 2 +-
xen/include/xen/fdt-domain-build.h | 2 +-
xen/include/xen/fdt-kernel.h | 2 +-
15 files changed, 226 insertions(+), 218 deletions(-)
create mode 100644 xen/include/xen/bootinfo.h
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 5317665555..497cfbe860 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
#include <xen/init.h>
+#include <xen/bootinfo.h>
#include <xen/compile.h>
#include <xen/fdt-domain-build.h>
#include <xen/fdt-kernel.h>
diff --git a/xen/arch/arm/mmu/mm.c b/xen/arch/arm/mmu/mm.c
index 9c50479c63..77f82757bb 100644
--- a/xen/arch/arm/mmu/mm.c
+++ b/xen/arch/arm/mmu/mm.c
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
+#include <xen/bootinfo.h>
#include <xen/init.h>
#include <xen/lib.h>
#include <xen/macros.h>
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 734e23da44..439b1d9df2 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -8,6 +8,7 @@
* Copyright (c) 2011 Citrix Systems.
*/
+#include <xen/bootinfo.h>
#include <xen/compile.h>
#include <xen/device_tree.h>
#include <xen/domain_page.h>
diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index 4047d67c0e..6ccd38091b 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
-#include <xen/bootfdt.h>
+#include <xen/bootinfo.h>
#include <xen/bug.h>
#include <xen/compiler.h>
#include <xen/init.h>
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index 8bcd19218d..3e99e2e194 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -2,7 +2,7 @@
#include <xen/acpi.h>
#include <xen/bug.h>
-#include <xen/bootfdt.h>
+#include <xen/bootinfo.h>
#include <xen/compile.h>
#include <xen/device_tree.h>
#include <xen/init.h>
diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c
index ab449db8d6..7ff62e1e3e 100644
--- a/xen/common/device-tree/bootfdt.c
+++ b/xen/common/device-tree/bootfdt.c
@@ -5,7 +5,7 @@
* Copyright (C) 2012-2014 Citrix Systems, Inc.
*/
-#include <xen/bootfdt.h>
+#include <xen/bootinfo.h>
#include <xen/device_tree.h>
#include <xen/efi.h>
#include <xen/init.h>
diff --git a/xen/common/device-tree/bootinfo.c b/xen/common/device-tree/bootinfo.c
index 717cfa0962..69491bdb0b 100644
--- a/xen/common/device-tree/bootinfo.c
+++ b/xen/common/device-tree/bootinfo.c
@@ -10,7 +10,7 @@
*/
#include <xen/acpi.h>
-#include <xen/bootfdt.h>
+#include <xen/bootinfo.h>
#include <xen/bug.h>
#include <xen/device_tree.h>
#include <xen/init.h>
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index 809cb3c232..42165e78af 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
-#include <xen/bootfdt.h>
+#include <xen/bootinfo.h>
#include <xen/device_tree.h>
#include <xen/domain.h>
#include <xen/domain_page.h>
diff --git a/xen/common/device-tree/domain-build.c b/xen/common/device-tree/domain-build.c
index e602cb91c9..80235aa56a 100644
--- a/xen/common/device-tree/domain-build.c
+++ b/xen/common/device-tree/domain-build.c
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
-#include <xen/bootfdt.h>
+#include <xen/bootinfo.h>
#include <xen/fdt-domain-build.h>
#include <xen/init.h>
#include <xen/lib.h>
diff --git a/xen/common/device-tree/kernel.c b/xen/common/device-tree/kernel.c
index 3960f951f7..0463bfd8df 100644
--- a/xen/common/device-tree/kernel.c
+++ b/xen/common/device-tree/kernel.c
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
-#include <xen/bootfdt.h>
+#include <xen/bootinfo.h>
#include <xen/device_tree.h>
#include <xen/fdt-kernel.h>
#include <xen/errno.h>
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index 1b89986e10..a87b5212a6 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -4,9 +4,6 @@
#include <xen/byteorder.h>
#include <xen/types.h>
-#include <xen/kernel.h>
-#include <xen/macros.h>
-#include <xen/xmalloc.h>
#if __has_include(<asm/bootfdt.h>)
#include <asm/bootfdt.h>
@@ -14,15 +11,10 @@
#define MIN_FDT_ALIGN 8
-#define NR_MEM_BANKS 256
-#define NR_SHMEM_BANKS 32
-
/* Default #address and #size cells */
#define DT_ROOT_NODE_ADDR_CELLS_DEFAULT 2
#define DT_ROOT_NODE_SIZE_CELLS_DEFAULT 1
-#define MAX_MODULES 32 /* Current maximum useful modules */
-
#define DEVICE_TREE_MAX_DEPTH 16
/* Helper to read a big number; size is in cells (not bytes) */
@@ -75,78 +67,6 @@ typedef enum {
BOOTMOD_UNKNOWN
} bootmodule_kind;
-enum membank_type {
- /*
- * The MEMBANK_DEFAULT type refers to either reserved memory for the
- * device/firmware (when the bank is in 'reserved_mem') or any RAM (when
- * the bank is in 'mem').
- */
- MEMBANK_DEFAULT,
- /*
- * The MEMBANK_STATIC_DOMAIN type is used to indicate whether the memory
- * bank is bound to a static Xen domain. It is only valid when the bank
- * is in reserved_mem.
- */
- MEMBANK_STATIC_DOMAIN,
- /*
- * The MEMBANK_STATIC_HEAP type is used to indicate whether the memory
- * bank is reserved as static heap. It is only valid when the bank is
- * in reserved_mem.
- */
- MEMBANK_STATIC_HEAP,
- /*
- * The MEMBANK_FDT_RESVMEM type is used to indicate whether the memory
- * bank is from the FDT reserve map.
- */
- MEMBANK_FDT_RESVMEM,
-};
-
-enum region_type {
- MEMORY,
- RESERVED_MEMORY,
- STATIC_SHARED_MEMORY
-};
-
-/* Indicates the maximum number of characters(\0 included) for shm_id */
-#define MAX_SHM_ID_LENGTH 16
-
-struct shmem_membank_extra {
- char shm_id[MAX_SHM_ID_LENGTH];
- unsigned int nr_shm_borrowers;
-};
-
-struct membank {
- paddr_t start;
- paddr_t size;
- union {
- enum membank_type type;
-#ifdef CONFIG_STATIC_SHM
- struct shmem_membank_extra *shmem_extra;
-#endif
- };
-};
-
-struct membanks {
- __struct_group(membanks_hdr, common, ,
- unsigned int nr_banks;
- unsigned int max_banks;
- enum region_type type;
- );
- struct membank bank[];
-};
-
-struct meminfo {
- struct membanks_hdr common;
- struct membank bank[NR_MEM_BANKS];
-};
-
-struct shared_meminfo {
- struct membanks_hdr common;
- struct membank bank[NR_SHMEM_BANKS];
- struct shmem_membank_extra extra[NR_SHMEM_BANKS];
-};
-
-
struct bootdomain {
struct domain *d;
@@ -179,134 +99,6 @@ struct bootmodule {
#endif
};
-/* DT_MAX_NAME is the node name max length according the DT spec */
-#define DT_MAX_NAME 41
-struct bootcmdline {
- bootmodule_kind kind;
- bool domU;
- paddr_t start;
- char dt_name[DT_MAX_NAME];
- char cmdline[BOOTMOD_MAX_CMDLINE];
-};
-
-struct bootmodules {
- int nr_mods;
- struct bootmodule module[MAX_MODULES];
-};
-
-struct bootcmdlines {
- unsigned int nr_mods;
- struct bootcmdline cmdline[MAX_MODULES];
-};
-
-struct bootinfo {
- struct meminfo mem;
- /* The reserved regions are only used when booting using Device-Tree */
- struct meminfo reserved_mem;
- struct bootmodules modules;
- struct bootcmdlines cmdlines;
-#ifdef CONFIG_ACPI
- struct meminfo acpi;
-#endif
-#ifdef CONFIG_STATIC_SHM
- struct shared_meminfo shmem;
-#endif
-};
-
-#ifdef CONFIG_ACPI
-#define BOOTINFO_ACPI_INIT \
- .acpi.common.max_banks = NR_MEM_BANKS, \
- .acpi.common.type = MEMORY,
-#else
-#define BOOTINFO_ACPI_INIT
-#endif
-
-#ifdef CONFIG_STATIC_SHM
-#define BOOTINFO_SHMEM_INIT \
- .shmem.common.max_banks = NR_SHMEM_BANKS, \
- .shmem.common.type = STATIC_SHARED_MEMORY,
-#else
-#define BOOTINFO_SHMEM_INIT
-#endif
-
-#define BOOTINFO_INIT \
-{ \
- .mem.common.max_banks = NR_MEM_BANKS, \
- .mem.common.type = MEMORY, \
- .reserved_mem.common.max_banks = NR_MEM_BANKS, \
- .reserved_mem.common.type = RESERVED_MEMORY, \
- BOOTINFO_ACPI_INIT \
- BOOTINFO_SHMEM_INIT \
-}
-
-extern struct bootinfo bootinfo;
-
-bool check_reserved_regions_overlap(paddr_t region_start,
- paddr_t region_size,
- bool allow_memreserve_overlap);
-
-struct bootmodule *add_boot_module(bootmodule_kind kind,
- paddr_t start, paddr_t size, bool domU);
-struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
-struct bootmodule * boot_module_find_by_addr_and_kind(bootmodule_kind kind,
- paddr_t start);
-void add_boot_cmdline(const char *name, const char *cmdline,
- bootmodule_kind kind, paddr_t start, bool domU);
-struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind);
-struct bootcmdline * boot_cmdline_find_by_name(const char *name);
-const char *boot_module_kind_as_string(bootmodule_kind kind);
-
-void populate_boot_allocator(void);
-
-size_t boot_fdt_info(const void *fdt, paddr_t paddr);
-
-const char *boot_fdt_cmdline(const void *fdt);
-int domain_fdt_begin_node(void *fdt, const char *name, uint64_t unit);
-
-static inline struct membanks *bootinfo_get_reserved_mem(void)
-{
- return container_of(&bootinfo.reserved_mem.common, struct membanks, common);
-}
-
-static inline struct membanks *bootinfo_get_mem(void)
-{
- return container_of(&bootinfo.mem.common, struct membanks, common);
-}
-
-#ifdef CONFIG_ACPI
-static inline struct membanks *bootinfo_get_acpi(void)
-{
- return container_of(&bootinfo.acpi.common, struct membanks, common);
-}
-#endif
-
-#ifdef CONFIG_STATIC_SHM
-static inline struct membanks *bootinfo_get_shmem(void)
-{
- return container_of(&bootinfo.shmem.common, struct membanks, common);
-}
-
-static inline struct shmem_membank_extra *bootinfo_get_shmem_extra(void)
-{
- return bootinfo.shmem.extra;
-}
-#endif
-
-static inline struct membanks *membanks_xzalloc(unsigned int nr,
- enum region_type type)
-{
- struct membanks *banks = xzalloc_flex_struct(struct membanks, bank, nr);
-
- if ( !banks )
- goto out;
-
- banks->max_banks = nr;
- banks->type = type;
-
- out:
- return banks;
-}
-
/*
* Interpret the property `prop_name` of `node` as a u32.
*
diff --git a/xen/include/xen/bootinfo.h b/xen/include/xen/bootinfo.h
new file mode 100644
index 0000000000..670fe771a4
--- /dev/null
+++ b/xen/include/xen/bootinfo.h
@@ -0,0 +1,213 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef XEN_BOOTINFO_H
+#define XEN_BOOTINFO_H
+
+#include <xen/bootfdt.h>
+#include <xen/kernel.h>
+#include <xen/macros.h>
+#include <xen/xmalloc.h>
+
+#define NR_MEM_BANKS 256
+#define NR_SHMEM_BANKS 32
+
+#define MAX_MODULES 32 /* Current maximum useful modules */
+
+enum membank_type {
+ /*
+ * The MEMBANK_DEFAULT type refers to either reserved memory for the
+ * device/firmware (when the bank is in 'reserved_mem') or any RAM (when
+ * the bank is in 'mem').
+ */
+ MEMBANK_DEFAULT,
+ /*
+ * The MEMBANK_STATIC_DOMAIN type is used to indicate whether the memory
+ * bank is bound to a static Xen domain. It is only valid when the bank
+ * is in reserved_mem.
+ */
+ MEMBANK_STATIC_DOMAIN,
+ /*
+ * The MEMBANK_STATIC_HEAP type is used to indicate whether the memory
+ * bank is reserved as static heap. It is only valid when the bank is
+ * in reserved_mem.
+ */
+ MEMBANK_STATIC_HEAP,
+ /*
+ * The MEMBANK_FDT_RESVMEM type is used to indicate whether the memory
+ * bank is from the FDT reserve map.
+ */
+ MEMBANK_FDT_RESVMEM,
+};
+
+enum region_type {
+ MEMORY,
+ RESERVED_MEMORY,
+ STATIC_SHARED_MEMORY
+};
+
+/* Indicates the maximum number of characters(\0 included) for shm_id */
+#define MAX_SHM_ID_LENGTH 16
+
+struct shmem_membank_extra {
+ char shm_id[MAX_SHM_ID_LENGTH];
+ unsigned int nr_shm_borrowers;
+};
+
+struct membank {
+ paddr_t start;
+ paddr_t size;
+ union {
+ enum membank_type type;
+#ifdef CONFIG_STATIC_SHM
+ struct shmem_membank_extra *shmem_extra;
+#endif
+ };
+};
+
+struct membanks {
+ __struct_group(membanks_hdr, common, ,
+ unsigned int nr_banks;
+ unsigned int max_banks;
+ enum region_type type;
+ );
+ struct membank bank[];
+};
+
+struct meminfo {
+ struct membanks_hdr common;
+ struct membank bank[NR_MEM_BANKS];
+};
+
+struct shared_meminfo {
+ struct membanks_hdr common;
+ struct membank bank[NR_SHMEM_BANKS];
+ struct shmem_membank_extra extra[NR_SHMEM_BANKS];
+};
+
+/* DT_MAX_NAME is the node name max length according the DT spec */
+#define DT_MAX_NAME 41
+struct bootcmdline {
+ bootmodule_kind kind;
+ bool domU;
+ paddr_t start;
+ char dt_name[DT_MAX_NAME];
+ char cmdline[BOOTMOD_MAX_CMDLINE];
+};
+
+struct bootmodules {
+ int nr_mods;
+ struct bootmodule module[MAX_MODULES];
+};
+
+struct bootcmdlines {
+ unsigned int nr_mods;
+ struct bootcmdline cmdline[MAX_MODULES];
+};
+
+struct bootinfo {
+ struct meminfo mem;
+ /* The reserved regions are only used when booting using Device-Tree */
+ struct meminfo reserved_mem;
+ struct bootmodules modules;
+ struct bootcmdlines cmdlines;
+#ifdef CONFIG_ACPI
+ struct meminfo acpi;
+#endif
+#ifdef CONFIG_STATIC_SHM
+ struct shared_meminfo shmem;
+#endif
+};
+
+#ifdef CONFIG_ACPI
+#define BOOTINFO_ACPI_INIT \
+ .acpi.common.max_banks = NR_MEM_BANKS, \
+ .acpi.common.type = MEMORY,
+#else
+#define BOOTINFO_ACPI_INIT
+#endif
+
+#ifdef CONFIG_STATIC_SHM
+#define BOOTINFO_SHMEM_INIT \
+ .shmem.common.max_banks = NR_SHMEM_BANKS, \
+ .shmem.common.type = STATIC_SHARED_MEMORY,
+#else
+#define BOOTINFO_SHMEM_INIT
+#endif
+
+#define BOOTINFO_INIT \
+{ \
+ .mem.common.max_banks = NR_MEM_BANKS, \
+ .mem.common.type = MEMORY, \
+ .reserved_mem.common.max_banks = NR_MEM_BANKS, \
+ .reserved_mem.common.type = RESERVED_MEMORY, \
+ BOOTINFO_ACPI_INIT \
+ BOOTINFO_SHMEM_INIT \
+}
+
+extern struct bootinfo bootinfo;
+
+bool check_reserved_regions_overlap(paddr_t region_start,
+ paddr_t region_size,
+ bool allow_memreserve_overlap);
+
+struct bootmodule *add_boot_module(bootmodule_kind kind,
+ paddr_t start, paddr_t size, bool domU);
+struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
+struct bootmodule * boot_module_find_by_addr_and_kind(bootmodule_kind kind,
+ paddr_t start);
+void add_boot_cmdline(const char *name, const char *cmdline,
+ bootmodule_kind kind, paddr_t start, bool domU);
+struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind);
+struct bootcmdline * boot_cmdline_find_by_name(const char *name);
+const char *boot_module_kind_as_string(bootmodule_kind kind);
+
+void populate_boot_allocator(void);
+
+size_t boot_fdt_info(const void *fdt, paddr_t paddr);
+const char *boot_fdt_cmdline(const void *fdt);
+int domain_fdt_begin_node(void *fdt, const char *name, uint64_t unit);
+
+static inline struct membanks *bootinfo_get_reserved_mem(void)
+{
+ return container_of(&bootinfo.reserved_mem.common, struct membanks, common);
+}
+
+static inline struct membanks *bootinfo_get_mem(void)
+{
+ return container_of(&bootinfo.mem.common, struct membanks, common);
+}
+
+#ifdef CONFIG_ACPI
+static inline struct membanks *bootinfo_get_acpi(void)
+{
+ return container_of(&bootinfo.acpi.common, struct membanks, common);
+}
+#endif
+
+#ifdef CONFIG_STATIC_SHM
+static inline struct membanks *bootinfo_get_shmem(void)
+{
+ return container_of(&bootinfo.shmem.common, struct membanks, common);
+}
+
+static inline struct shmem_membank_extra *bootinfo_get_shmem_extra(void)
+{
+ return bootinfo.shmem.extra;
+}
+#endif
+
+static inline struct membanks *membanks_xzalloc(unsigned int nr,
+ enum region_type type)
+{
+ struct membanks *banks = xzalloc_flex_struct(struct membanks, bank, nr);
+
+ if ( !banks )
+ goto out;
+
+ banks->max_banks = nr;
+ banks->type = type;
+
+ out:
+ return banks;
+}
+
+#endif /* XEN_BOOTINFO_H */
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 0a22b1ba1d..7d1c8bc305 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -10,7 +10,7 @@
#ifndef __XEN_DEVICE_TREE_H__
#define __XEN_DEVICE_TREE_H__
-#include <xen/bootfdt.h>
+#include <xen/bootinfo.h>
#include <xen/byteorder.h>
#include <asm/device.h>
diff --git a/xen/include/xen/fdt-domain-build.h b/xen/include/xen/fdt-domain-build.h
index 45981dbec0..60565fdbf9 100644
--- a/xen/include/xen/fdt-domain-build.h
+++ b/xen/include/xen/fdt-domain-build.h
@@ -3,7 +3,7 @@
#ifndef __XEN_FDT_DOMAIN_BUILD_H__
#define __XEN_FDT_DOMAIN_BUILD_H__
-#include <xen/bootfdt.h>
+#include <xen/bootinfo.h>
#include <xen/device_tree.h>
#include <xen/fdt-kernel.h>
#include <xen/mm.h>
diff --git a/xen/include/xen/fdt-kernel.h b/xen/include/xen/fdt-kernel.h
index dd009f01d0..cb211d242f 100644
--- a/xen/include/xen/fdt-kernel.h
+++ b/xen/include/xen/fdt-kernel.h
@@ -7,7 +7,7 @@
#ifndef __XEN_FDT_KERNEL_H__
#define __XEN_FDT_KERNEL_H__
-#include <xen/bootfdt.h>
+#include <xen/bootinfo.h>
#include <xen/device_tree.h>
#include <xen/types.h>
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v2 09/15] xen/dt: Move bootinfo functions to a new bootinfo.h
2025-06-05 19:48 ` [PATCH v2 09/15] xen/dt: Move bootinfo functions to a new bootinfo.h Alejandro Vallejo
@ 2025-06-06 23:39 ` Stefano Stabellini
0 siblings, 0 replies; 39+ messages in thread
From: Stefano Stabellini @ 2025-06-06 23:39 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: xen-devel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
Michal Orzel, Volodymyr Babchuk, Andrew Cooper, Anthony PERARD,
Jan Beulich, Roger Pau Monné, Alistair Francis, Bob Eshleman,
Connor Davis, Oleksii Kurochko, Daniel P. Smith
On Thu, 5 Jun 2025, Alejandro Vallejo wrote:
> Part of an unpicking process to extract bootfdt contents independent of
> bootinfo to a separate file for x86 to take.
>
> With this, bootfdt.h can be cleanly included from x86. A later patch
> extracts the definitions so the functions may be called too.
>
> Not a functional change.
>
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
> ---
> xen/arch/arm/domain_build.c | 1 +
> xen/arch/arm/mmu/mm.c | 1 +
> xen/arch/arm/setup.c | 1 +
> xen/arch/riscv/mm.c | 2 +-
> xen/arch/riscv/setup.c | 2 +-
> xen/common/device-tree/bootfdt.c | 2 +-
> xen/common/device-tree/bootinfo.c | 2 +-
> xen/common/device-tree/dom0less-build.c | 2 +-
> xen/common/device-tree/domain-build.c | 2 +-
> xen/common/device-tree/kernel.c | 2 +-
> xen/include/xen/bootfdt.h | 208 -----------------------
> xen/include/xen/bootinfo.h | 213 ++++++++++++++++++++++++
> xen/include/xen/device_tree.h | 2 +-
> xen/include/xen/fdt-domain-build.h | 2 +-
> xen/include/xen/fdt-kernel.h | 2 +-
> 15 files changed, 226 insertions(+), 218 deletions(-)
> create mode 100644 xen/include/xen/bootinfo.h
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 5317665555..497cfbe860 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1,5 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> #include <xen/init.h>
> +#include <xen/bootinfo.h>
> #include <xen/compile.h>
> #include <xen/fdt-domain-build.h>
> #include <xen/fdt-kernel.h>
> diff --git a/xen/arch/arm/mmu/mm.c b/xen/arch/arm/mmu/mm.c
> index 9c50479c63..77f82757bb 100644
> --- a/xen/arch/arm/mmu/mm.c
> +++ b/xen/arch/arm/mmu/mm.c
> @@ -1,5 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0-or-later */
>
> +#include <xen/bootinfo.h>
It doesn't look like this is needed.
Other than that:
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> #include <xen/init.h>
> #include <xen/lib.h>
> #include <xen/macros.h>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 734e23da44..439b1d9df2 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -8,6 +8,7 @@
> * Copyright (c) 2011 Citrix Systems.
> */
>
> +#include <xen/bootinfo.h>
> #include <xen/compile.h>
> #include <xen/device_tree.h>
> #include <xen/domain_page.h>
> diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
> index 4047d67c0e..6ccd38091b 100644
> --- a/xen/arch/riscv/mm.c
> +++ b/xen/arch/riscv/mm.c
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
>
> -#include <xen/bootfdt.h>
> +#include <xen/bootinfo.h>
> #include <xen/bug.h>
> #include <xen/compiler.h>
> #include <xen/init.h>
> diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
> index 8bcd19218d..3e99e2e194 100644
> --- a/xen/arch/riscv/setup.c
> +++ b/xen/arch/riscv/setup.c
> @@ -2,7 +2,7 @@
>
> #include <xen/acpi.h>
> #include <xen/bug.h>
> -#include <xen/bootfdt.h>
> +#include <xen/bootinfo.h>
> #include <xen/compile.h>
> #include <xen/device_tree.h>
> #include <xen/init.h>
> diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c
> index ab449db8d6..7ff62e1e3e 100644
> --- a/xen/common/device-tree/bootfdt.c
> +++ b/xen/common/device-tree/bootfdt.c
> @@ -5,7 +5,7 @@
> * Copyright (C) 2012-2014 Citrix Systems, Inc.
> */
>
> -#include <xen/bootfdt.h>
> +#include <xen/bootinfo.h>
> #include <xen/device_tree.h>
> #include <xen/efi.h>
> #include <xen/init.h>
> diff --git a/xen/common/device-tree/bootinfo.c b/xen/common/device-tree/bootinfo.c
> index 717cfa0962..69491bdb0b 100644
> --- a/xen/common/device-tree/bootinfo.c
> +++ b/xen/common/device-tree/bootinfo.c
> @@ -10,7 +10,7 @@
> */
>
> #include <xen/acpi.h>
> -#include <xen/bootfdt.h>
> +#include <xen/bootinfo.h>
> #include <xen/bug.h>
> #include <xen/device_tree.h>
> #include <xen/init.h>
> diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
> index 809cb3c232..42165e78af 100644
> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
>
> -#include <xen/bootfdt.h>
> +#include <xen/bootinfo.h>
> #include <xen/device_tree.h>
> #include <xen/domain.h>
> #include <xen/domain_page.h>
> diff --git a/xen/common/device-tree/domain-build.c b/xen/common/device-tree/domain-build.c
> index e602cb91c9..80235aa56a 100644
> --- a/xen/common/device-tree/domain-build.c
> +++ b/xen/common/device-tree/domain-build.c
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
>
> -#include <xen/bootfdt.h>
> +#include <xen/bootinfo.h>
> #include <xen/fdt-domain-build.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> diff --git a/xen/common/device-tree/kernel.c b/xen/common/device-tree/kernel.c
> index 3960f951f7..0463bfd8df 100644
> --- a/xen/common/device-tree/kernel.c
> +++ b/xen/common/device-tree/kernel.c
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
>
> -#include <xen/bootfdt.h>
> +#include <xen/bootinfo.h>
> #include <xen/device_tree.h>
> #include <xen/fdt-kernel.h>
> #include <xen/errno.h>
> diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
> index 1b89986e10..a87b5212a6 100644
> --- a/xen/include/xen/bootfdt.h
> +++ b/xen/include/xen/bootfdt.h
> @@ -4,9 +4,6 @@
>
> #include <xen/byteorder.h>
> #include <xen/types.h>
> -#include <xen/kernel.h>
> -#include <xen/macros.h>
> -#include <xen/xmalloc.h>
>
> #if __has_include(<asm/bootfdt.h>)
> #include <asm/bootfdt.h>
> @@ -14,15 +11,10 @@
>
> #define MIN_FDT_ALIGN 8
>
> -#define NR_MEM_BANKS 256
> -#define NR_SHMEM_BANKS 32
> -
> /* Default #address and #size cells */
> #define DT_ROOT_NODE_ADDR_CELLS_DEFAULT 2
> #define DT_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>
> -#define MAX_MODULES 32 /* Current maximum useful modules */
> -
> #define DEVICE_TREE_MAX_DEPTH 16
>
> /* Helper to read a big number; size is in cells (not bytes) */
> @@ -75,78 +67,6 @@ typedef enum {
> BOOTMOD_UNKNOWN
> } bootmodule_kind;
>
> -enum membank_type {
> - /*
> - * The MEMBANK_DEFAULT type refers to either reserved memory for the
> - * device/firmware (when the bank is in 'reserved_mem') or any RAM (when
> - * the bank is in 'mem').
> - */
> - MEMBANK_DEFAULT,
> - /*
> - * The MEMBANK_STATIC_DOMAIN type is used to indicate whether the memory
> - * bank is bound to a static Xen domain. It is only valid when the bank
> - * is in reserved_mem.
> - */
> - MEMBANK_STATIC_DOMAIN,
> - /*
> - * The MEMBANK_STATIC_HEAP type is used to indicate whether the memory
> - * bank is reserved as static heap. It is only valid when the bank is
> - * in reserved_mem.
> - */
> - MEMBANK_STATIC_HEAP,
> - /*
> - * The MEMBANK_FDT_RESVMEM type is used to indicate whether the memory
> - * bank is from the FDT reserve map.
> - */
> - MEMBANK_FDT_RESVMEM,
> -};
> -
> -enum region_type {
> - MEMORY,
> - RESERVED_MEMORY,
> - STATIC_SHARED_MEMORY
> -};
> -
> -/* Indicates the maximum number of characters(\0 included) for shm_id */
> -#define MAX_SHM_ID_LENGTH 16
> -
> -struct shmem_membank_extra {
> - char shm_id[MAX_SHM_ID_LENGTH];
> - unsigned int nr_shm_borrowers;
> -};
> -
> -struct membank {
> - paddr_t start;
> - paddr_t size;
> - union {
> - enum membank_type type;
> -#ifdef CONFIG_STATIC_SHM
> - struct shmem_membank_extra *shmem_extra;
> -#endif
> - };
> -};
> -
> -struct membanks {
> - __struct_group(membanks_hdr, common, ,
> - unsigned int nr_banks;
> - unsigned int max_banks;
> - enum region_type type;
> - );
> - struct membank bank[];
> -};
> -
> -struct meminfo {
> - struct membanks_hdr common;
> - struct membank bank[NR_MEM_BANKS];
> -};
> -
> -struct shared_meminfo {
> - struct membanks_hdr common;
> - struct membank bank[NR_SHMEM_BANKS];
> - struct shmem_membank_extra extra[NR_SHMEM_BANKS];
> -};
> -
> -
> struct bootdomain {
> struct domain *d;
>
> @@ -179,134 +99,6 @@ struct bootmodule {
> #endif
> };
>
> -/* DT_MAX_NAME is the node name max length according the DT spec */
> -#define DT_MAX_NAME 41
> -struct bootcmdline {
> - bootmodule_kind kind;
> - bool domU;
> - paddr_t start;
> - char dt_name[DT_MAX_NAME];
> - char cmdline[BOOTMOD_MAX_CMDLINE];
> -};
> -
> -struct bootmodules {
> - int nr_mods;
> - struct bootmodule module[MAX_MODULES];
> -};
> -
> -struct bootcmdlines {
> - unsigned int nr_mods;
> - struct bootcmdline cmdline[MAX_MODULES];
> -};
> -
> -struct bootinfo {
> - struct meminfo mem;
> - /* The reserved regions are only used when booting using Device-Tree */
> - struct meminfo reserved_mem;
> - struct bootmodules modules;
> - struct bootcmdlines cmdlines;
> -#ifdef CONFIG_ACPI
> - struct meminfo acpi;
> -#endif
> -#ifdef CONFIG_STATIC_SHM
> - struct shared_meminfo shmem;
> -#endif
> -};
> -
> -#ifdef CONFIG_ACPI
> -#define BOOTINFO_ACPI_INIT \
> - .acpi.common.max_banks = NR_MEM_BANKS, \
> - .acpi.common.type = MEMORY,
> -#else
> -#define BOOTINFO_ACPI_INIT
> -#endif
> -
> -#ifdef CONFIG_STATIC_SHM
> -#define BOOTINFO_SHMEM_INIT \
> - .shmem.common.max_banks = NR_SHMEM_BANKS, \
> - .shmem.common.type = STATIC_SHARED_MEMORY,
> -#else
> -#define BOOTINFO_SHMEM_INIT
> -#endif
> -
> -#define BOOTINFO_INIT \
> -{ \
> - .mem.common.max_banks = NR_MEM_BANKS, \
> - .mem.common.type = MEMORY, \
> - .reserved_mem.common.max_banks = NR_MEM_BANKS, \
> - .reserved_mem.common.type = RESERVED_MEMORY, \
> - BOOTINFO_ACPI_INIT \
> - BOOTINFO_SHMEM_INIT \
> -}
> -
> -extern struct bootinfo bootinfo;
> -
> -bool check_reserved_regions_overlap(paddr_t region_start,
> - paddr_t region_size,
> - bool allow_memreserve_overlap);
> -
> -struct bootmodule *add_boot_module(bootmodule_kind kind,
> - paddr_t start, paddr_t size, bool domU);
> -struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
> -struct bootmodule * boot_module_find_by_addr_and_kind(bootmodule_kind kind,
> - paddr_t start);
> -void add_boot_cmdline(const char *name, const char *cmdline,
> - bootmodule_kind kind, paddr_t start, bool domU);
> -struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind);
> -struct bootcmdline * boot_cmdline_find_by_name(const char *name);
> -const char *boot_module_kind_as_string(bootmodule_kind kind);
> -
> -void populate_boot_allocator(void);
> -
> -size_t boot_fdt_info(const void *fdt, paddr_t paddr);
> -
> -const char *boot_fdt_cmdline(const void *fdt);
> -int domain_fdt_begin_node(void *fdt, const char *name, uint64_t unit);
> -
> -static inline struct membanks *bootinfo_get_reserved_mem(void)
> -{
> - return container_of(&bootinfo.reserved_mem.common, struct membanks, common);
> -}
> -
> -static inline struct membanks *bootinfo_get_mem(void)
> -{
> - return container_of(&bootinfo.mem.common, struct membanks, common);
> -}
> -
> -#ifdef CONFIG_ACPI
> -static inline struct membanks *bootinfo_get_acpi(void)
> -{
> - return container_of(&bootinfo.acpi.common, struct membanks, common);
> -}
> -#endif
> -
> -#ifdef CONFIG_STATIC_SHM
> -static inline struct membanks *bootinfo_get_shmem(void)
> -{
> - return container_of(&bootinfo.shmem.common, struct membanks, common);
> -}
> -
> -static inline struct shmem_membank_extra *bootinfo_get_shmem_extra(void)
> -{
> - return bootinfo.shmem.extra;
> -}
> -#endif
> -
> -static inline struct membanks *membanks_xzalloc(unsigned int nr,
> - enum region_type type)
> -{
> - struct membanks *banks = xzalloc_flex_struct(struct membanks, bank, nr);
> -
> - if ( !banks )
> - goto out;
> -
> - banks->max_banks = nr;
> - banks->type = type;
> -
> - out:
> - return banks;
> -}
> -
> /*
> * Interpret the property `prop_name` of `node` as a u32.
> *
> diff --git a/xen/include/xen/bootinfo.h b/xen/include/xen/bootinfo.h
> new file mode 100644
> index 0000000000..670fe771a4
> --- /dev/null
> +++ b/xen/include/xen/bootinfo.h
> @@ -0,0 +1,213 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef XEN_BOOTINFO_H
> +#define XEN_BOOTINFO_H
> +
> +#include <xen/bootfdt.h>
> +#include <xen/kernel.h>
> +#include <xen/macros.h>
> +#include <xen/xmalloc.h>
> +
> +#define NR_MEM_BANKS 256
> +#define NR_SHMEM_BANKS 32
> +
> +#define MAX_MODULES 32 /* Current maximum useful modules */
> +
> +enum membank_type {
> + /*
> + * The MEMBANK_DEFAULT type refers to either reserved memory for the
> + * device/firmware (when the bank is in 'reserved_mem') or any RAM (when
> + * the bank is in 'mem').
> + */
> + MEMBANK_DEFAULT,
> + /*
> + * The MEMBANK_STATIC_DOMAIN type is used to indicate whether the memory
> + * bank is bound to a static Xen domain. It is only valid when the bank
> + * is in reserved_mem.
> + */
> + MEMBANK_STATIC_DOMAIN,
> + /*
> + * The MEMBANK_STATIC_HEAP type is used to indicate whether the memory
> + * bank is reserved as static heap. It is only valid when the bank is
> + * in reserved_mem.
> + */
> + MEMBANK_STATIC_HEAP,
> + /*
> + * The MEMBANK_FDT_RESVMEM type is used to indicate whether the memory
> + * bank is from the FDT reserve map.
> + */
> + MEMBANK_FDT_RESVMEM,
> +};
> +
> +enum region_type {
> + MEMORY,
> + RESERVED_MEMORY,
> + STATIC_SHARED_MEMORY
> +};
> +
> +/* Indicates the maximum number of characters(\0 included) for shm_id */
> +#define MAX_SHM_ID_LENGTH 16
> +
> +struct shmem_membank_extra {
> + char shm_id[MAX_SHM_ID_LENGTH];
> + unsigned int nr_shm_borrowers;
> +};
> +
> +struct membank {
> + paddr_t start;
> + paddr_t size;
> + union {
> + enum membank_type type;
> +#ifdef CONFIG_STATIC_SHM
> + struct shmem_membank_extra *shmem_extra;
> +#endif
> + };
> +};
> +
> +struct membanks {
> + __struct_group(membanks_hdr, common, ,
> + unsigned int nr_banks;
> + unsigned int max_banks;
> + enum region_type type;
> + );
> + struct membank bank[];
> +};
> +
> +struct meminfo {
> + struct membanks_hdr common;
> + struct membank bank[NR_MEM_BANKS];
> +};
> +
> +struct shared_meminfo {
> + struct membanks_hdr common;
> + struct membank bank[NR_SHMEM_BANKS];
> + struct shmem_membank_extra extra[NR_SHMEM_BANKS];
> +};
> +
> +/* DT_MAX_NAME is the node name max length according the DT spec */
> +#define DT_MAX_NAME 41
> +struct bootcmdline {
> + bootmodule_kind kind;
> + bool domU;
> + paddr_t start;
> + char dt_name[DT_MAX_NAME];
> + char cmdline[BOOTMOD_MAX_CMDLINE];
> +};
> +
> +struct bootmodules {
> + int nr_mods;
> + struct bootmodule module[MAX_MODULES];
> +};
> +
> +struct bootcmdlines {
> + unsigned int nr_mods;
> + struct bootcmdline cmdline[MAX_MODULES];
> +};
> +
> +struct bootinfo {
> + struct meminfo mem;
> + /* The reserved regions are only used when booting using Device-Tree */
> + struct meminfo reserved_mem;
> + struct bootmodules modules;
> + struct bootcmdlines cmdlines;
> +#ifdef CONFIG_ACPI
> + struct meminfo acpi;
> +#endif
> +#ifdef CONFIG_STATIC_SHM
> + struct shared_meminfo shmem;
> +#endif
> +};
> +
> +#ifdef CONFIG_ACPI
> +#define BOOTINFO_ACPI_INIT \
> + .acpi.common.max_banks = NR_MEM_BANKS, \
> + .acpi.common.type = MEMORY,
> +#else
> +#define BOOTINFO_ACPI_INIT
> +#endif
> +
> +#ifdef CONFIG_STATIC_SHM
> +#define BOOTINFO_SHMEM_INIT \
> + .shmem.common.max_banks = NR_SHMEM_BANKS, \
> + .shmem.common.type = STATIC_SHARED_MEMORY,
> +#else
> +#define BOOTINFO_SHMEM_INIT
> +#endif
> +
> +#define BOOTINFO_INIT \
> +{ \
> + .mem.common.max_banks = NR_MEM_BANKS, \
> + .mem.common.type = MEMORY, \
> + .reserved_mem.common.max_banks = NR_MEM_BANKS, \
> + .reserved_mem.common.type = RESERVED_MEMORY, \
> + BOOTINFO_ACPI_INIT \
> + BOOTINFO_SHMEM_INIT \
> +}
> +
> +extern struct bootinfo bootinfo;
> +
> +bool check_reserved_regions_overlap(paddr_t region_start,
> + paddr_t region_size,
> + bool allow_memreserve_overlap);
> +
> +struct bootmodule *add_boot_module(bootmodule_kind kind,
> + paddr_t start, paddr_t size, bool domU);
> +struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
> +struct bootmodule * boot_module_find_by_addr_and_kind(bootmodule_kind kind,
> + paddr_t start);
> +void add_boot_cmdline(const char *name, const char *cmdline,
> + bootmodule_kind kind, paddr_t start, bool domU);
> +struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind);
> +struct bootcmdline * boot_cmdline_find_by_name(const char *name);
> +const char *boot_module_kind_as_string(bootmodule_kind kind);
> +
> +void populate_boot_allocator(void);
> +
> +size_t boot_fdt_info(const void *fdt, paddr_t paddr);
> +const char *boot_fdt_cmdline(const void *fdt);
> +int domain_fdt_begin_node(void *fdt, const char *name, uint64_t unit);
> +
> +static inline struct membanks *bootinfo_get_reserved_mem(void)
> +{
> + return container_of(&bootinfo.reserved_mem.common, struct membanks, common);
> +}
> +
> +static inline struct membanks *bootinfo_get_mem(void)
> +{
> + return container_of(&bootinfo.mem.common, struct membanks, common);
> +}
> +
> +#ifdef CONFIG_ACPI
> +static inline struct membanks *bootinfo_get_acpi(void)
> +{
> + return container_of(&bootinfo.acpi.common, struct membanks, common);
> +}
> +#endif
> +
> +#ifdef CONFIG_STATIC_SHM
> +static inline struct membanks *bootinfo_get_shmem(void)
> +{
> + return container_of(&bootinfo.shmem.common, struct membanks, common);
> +}
> +
> +static inline struct shmem_membank_extra *bootinfo_get_shmem_extra(void)
> +{
> + return bootinfo.shmem.extra;
> +}
> +#endif
> +
> +static inline struct membanks *membanks_xzalloc(unsigned int nr,
> + enum region_type type)
> +{
> + struct membanks *banks = xzalloc_flex_struct(struct membanks, bank, nr);
> +
> + if ( !banks )
> + goto out;
> +
> + banks->max_banks = nr;
> + banks->type = type;
> +
> + out:
> + return banks;
> +}
> +
> +#endif /* XEN_BOOTINFO_H */
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 0a22b1ba1d..7d1c8bc305 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -10,7 +10,7 @@
> #ifndef __XEN_DEVICE_TREE_H__
> #define __XEN_DEVICE_TREE_H__
>
> -#include <xen/bootfdt.h>
> +#include <xen/bootinfo.h>
> #include <xen/byteorder.h>
>
> #include <asm/device.h>
> diff --git a/xen/include/xen/fdt-domain-build.h b/xen/include/xen/fdt-domain-build.h
> index 45981dbec0..60565fdbf9 100644
> --- a/xen/include/xen/fdt-domain-build.h
> +++ b/xen/include/xen/fdt-domain-build.h
> @@ -3,7 +3,7 @@
> #ifndef __XEN_FDT_DOMAIN_BUILD_H__
> #define __XEN_FDT_DOMAIN_BUILD_H__
>
> -#include <xen/bootfdt.h>
> +#include <xen/bootinfo.h>
> #include <xen/device_tree.h>
> #include <xen/fdt-kernel.h>
> #include <xen/mm.h>
> diff --git a/xen/include/xen/fdt-kernel.h b/xen/include/xen/fdt-kernel.h
> index dd009f01d0..cb211d242f 100644
> --- a/xen/include/xen/fdt-kernel.h
> +++ b/xen/include/xen/fdt-kernel.h
> @@ -7,7 +7,7 @@
> #ifndef __XEN_FDT_KERNEL_H__
> #define __XEN_FDT_KERNEL_H__
>
> -#include <xen/bootfdt.h>
> +#include <xen/bootinfo.h>
> #include <xen/device_tree.h>
> #include <xen/types.h>
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 10/15] xen/dt: Rename bootfdt.c -> bootinfo-fdt.c
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (8 preceding siblings ...)
2025-06-05 19:48 ` [PATCH v2 09/15] xen/dt: Move bootinfo functions to a new bootinfo.h Alejandro Vallejo
@ 2025-06-05 19:48 ` Alejandro Vallejo
2025-06-05 19:48 ` [PATCH v2 11/15] xen/dt: Move bootinfo-independent helpers out of bootinfo-fdt.c Alejandro Vallejo
` (4 subsequent siblings)
14 siblings, 0 replies; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:48 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Daniel P. Smith
This file will eventually contain bootfdt helpers that make heavy use of
bootinfo. To simplify git history do the rename here explicitly. A later
patch extracts bootinfo-independent helpers into bootfdt.c.
Doing so here would needlessly pollute the diffs.
Not a functional change.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
xen/common/device-tree/Makefile | 2 +-
xen/common/device-tree/{bootfdt.c => bootinfo-fdt.c} | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
rename xen/common/device-tree/{bootfdt.c => bootinfo-fdt.c} (99%)
diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
index 13127296cb..8a3f2be89e 100644
--- a/xen/common/device-tree/Makefile
+++ b/xen/common/device-tree/Makefile
@@ -1,4 +1,4 @@
-obj-y += bootfdt.init.o
+obj-y += bootinfo-fdt.init.o
obj-y += bootinfo.init.o
obj-y += device-tree.o
obj-$(CONFIG_DOMAIN_BUILD_HELPERS) += domain-build.init.o
diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootinfo-fdt.c
similarity index 99%
rename from xen/common/device-tree/bootfdt.c
rename to xen/common/device-tree/bootinfo-fdt.c
index 7ff62e1e3e..195249f700 100644
--- a/xen/common/device-tree/bootfdt.c
+++ b/xen/common/device-tree/bootinfo-fdt.c
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * Early Device Tree
+ * Early Device Tree with bootinfo hooks
*
* Copyright (C) 2012-2014 Citrix Systems, Inc.
*/
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v2 11/15] xen/dt: Move bootinfo-independent helpers out of bootinfo-fdt.c
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (9 preceding siblings ...)
2025-06-05 19:48 ` [PATCH v2 10/15] xen/dt: Rename bootfdt.c -> bootinfo-fdt.c Alejandro Vallejo
@ 2025-06-05 19:48 ` Alejandro Vallejo
2025-06-06 23:43 ` Stefano Stabellini
2025-06-05 19:48 ` [PATCH v2 12/15] xen/dt: Extract helper to map nodes to module kinds Alejandro Vallejo
` (3 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:48 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Daniel P. Smith
... back into bootfdt.c
These will be required by x86 later on to detect modules in the early scan of
the FDT. They are independent of bootinfo, so it's cleaner for them to exist in
a separate file.
Not a functional change.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
xen/common/device-tree/Makefile | 1 +
xen/common/device-tree/bootfdt.c | 97 ++++++++++++++++++++++++
xen/common/device-tree/bootinfo-fdt.c | 104 --------------------------
3 files changed, 98 insertions(+), 104 deletions(-)
create mode 100644 xen/common/device-tree/bootfdt.c
diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
index 8a3f2be89e..8abc069c4b 100644
--- a/xen/common/device-tree/Makefile
+++ b/xen/common/device-tree/Makefile
@@ -1,3 +1,4 @@
+obj-y += bootfdt.init.o
obj-y += bootinfo-fdt.init.o
obj-y += bootinfo.init.o
obj-y += device-tree.o
diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c
new file mode 100644
index 0000000000..6acf83d197
--- /dev/null
+++ b/xen/common/device-tree/bootfdt.c
@@ -0,0 +1,97 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <xen/bootfdt.h>
+#include <xen/bug.h>
+#include <xen/lib.h>
+#include <xen/libfdt/libfdt.h>
+
+uint32_t __init device_tree_get_u32(const void *fdt, int node,
+ const char *prop_name, uint32_t dflt)
+{
+ const struct fdt_property *prop;
+
+ prop = fdt_get_property(fdt, node, prop_name, NULL);
+ if ( !prop || prop->len < sizeof(uint32_t) )
+ return dflt;
+
+ return fdt32_to_cpu(*(const uint32_t*)prop->data);
+}
+
+int __init device_tree_for_each_node(const void *fdt, int node,
+ device_tree_node_func func,
+ void *data)
+{
+ /*
+ * We only care about relative depth increments, assume depth of
+ * node is 0 for simplicity.
+ */
+ int depth = 0;
+ const int first_node = node;
+ uint32_t address_cells[DEVICE_TREE_MAX_DEPTH];
+ uint32_t size_cells[DEVICE_TREE_MAX_DEPTH];
+ int ret;
+
+ do {
+ const char *name = fdt_get_name(fdt, node, NULL);
+ uint32_t as, ss;
+
+ if ( depth >= DEVICE_TREE_MAX_DEPTH )
+ {
+ printk("Warning: device tree node `%s' is nested too deep\n",
+ name);
+ continue;
+ }
+
+ as = depth > 0 ? address_cells[depth-1] : DT_ROOT_NODE_ADDR_CELLS_DEFAULT;
+ ss = depth > 0 ? size_cells[depth-1] : DT_ROOT_NODE_SIZE_CELLS_DEFAULT;
+
+ address_cells[depth] = device_tree_get_u32(fdt, node,
+ "#address-cells", as);
+ size_cells[depth] = device_tree_get_u32(fdt, node,
+ "#size-cells", ss);
+
+ /* skip the first node */
+ if ( node != first_node )
+ {
+ ret = func(fdt, node, name, depth, as, ss, data);
+ if ( ret != 0 )
+ return ret;
+ }
+
+ node = fdt_next_node(fdt, node, &depth);
+ } while ( node >= 0 && depth > 0 );
+
+ return 0;
+}
+
+void __init device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
+ uint32_t size_cells, paddr_t *start,
+ paddr_t *size)
+{
+ uint64_t dt_start, dt_size;
+
+ /*
+ * dt_next_cell will return uint64_t whereas paddr_t may not be 64-bit.
+ * Thus, there is an implicit cast from uint64_t to paddr_t.
+ */
+ dt_start = dt_next_cell(address_cells, cell);
+ dt_size = dt_next_cell(size_cells, cell);
+
+ if ( dt_start != (paddr_t)dt_start )
+ {
+ printk("Physical address greater than max width supported\n");
+ WARN();
+ }
+
+ if ( dt_size != (paddr_t)dt_size )
+ {
+ printk("Physical size greater than max width supported\n");
+ WARN();
+ }
+
+ /*
+ * Xen will truncate the address/size if it is greater than the maximum
+ * supported width and it will give an appropriate warning.
+ */
+ *start = dt_start;
+ *size = dt_size;
+}
diff --git a/xen/common/device-tree/bootinfo-fdt.c b/xen/common/device-tree/bootinfo-fdt.c
index 195249f700..695c4bfc49 100644
--- a/xen/common/device-tree/bootinfo-fdt.c
+++ b/xen/common/device-tree/bootinfo-fdt.c
@@ -109,39 +109,6 @@ static bool __init device_tree_is_memory_node(const void *fdt, int node,
return true;
}
-void __init device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
- uint32_t size_cells, paddr_t *start,
- paddr_t *size)
-{
- uint64_t dt_start, dt_size;
-
- /*
- * dt_next_cell will return uint64_t whereas paddr_t may not be 64-bit.
- * Thus, there is an implicit cast from uint64_t to paddr_t.
- */
- dt_start = dt_next_cell(address_cells, cell);
- dt_size = dt_next_cell(size_cells, cell);
-
- if ( dt_start != (paddr_t)dt_start )
- {
- printk("Physical address greater than max width supported\n");
- WARN();
- }
-
- if ( dt_size != (paddr_t)dt_size )
- {
- printk("Physical size greater than max width supported\n");
- WARN();
- }
-
- /*
- * Xen will truncate the address/size if it is greater than the maximum
- * supported width and it will give an appropriate warning.
- */
- *start = dt_start;
- *size = dt_size;
-}
-
static int __init device_tree_get_meminfo(const void *fdt, int node,
const char *prop_name,
u32 address_cells, u32 size_cells,
@@ -202,77 +169,6 @@ static int __init device_tree_get_meminfo(const void *fdt, int node,
return 0;
}
-uint32_t __init device_tree_get_u32(const void *fdt, int node,
- const char *prop_name, uint32_t dflt)
-{
- const struct fdt_property *prop;
-
- prop = fdt_get_property(fdt, node, prop_name, NULL);
- if ( !prop || prop->len < sizeof(uint32_t) )
- return dflt;
-
- return fdt32_to_cpu(*(const uint32_t*)prop->data);
-}
-
-/**
- * device_tree_for_each_node - iterate over all device tree sub-nodes
- * @fdt: flat device tree.
- * @node: parent node to start the search from
- * @func: function to call for each sub-node.
- * @data: data to pass to @func.
- *
- * Any nodes nested at DEVICE_TREE_MAX_DEPTH or deeper are ignored.
- *
- * Returns 0 if all nodes were iterated over successfully. If @func
- * returns a value different from 0, that value is returned immediately.
- */
-int __init device_tree_for_each_node(const void *fdt, int node,
- device_tree_node_func func,
- void *data)
-{
- /*
- * We only care about relative depth increments, assume depth of
- * node is 0 for simplicity.
- */
- int depth = 0;
- const int first_node = node;
- u32 address_cells[DEVICE_TREE_MAX_DEPTH];
- u32 size_cells[DEVICE_TREE_MAX_DEPTH];
- int ret;
-
- do {
- const char *name = fdt_get_name(fdt, node, NULL);
- u32 as, ss;
-
- if ( depth >= DEVICE_TREE_MAX_DEPTH )
- {
- printk("Warning: device tree node `%s' is nested too deep\n",
- name);
- continue;
- }
-
- as = depth > 0 ? address_cells[depth-1] : DT_ROOT_NODE_ADDR_CELLS_DEFAULT;
- ss = depth > 0 ? size_cells[depth-1] : DT_ROOT_NODE_SIZE_CELLS_DEFAULT;
-
- address_cells[depth] = device_tree_get_u32(fdt, node,
- "#address-cells", as);
- size_cells[depth] = device_tree_get_u32(fdt, node,
- "#size-cells", ss);
-
- /* skip the first node */
- if ( node != first_node )
- {
- ret = func(fdt, node, name, depth, as, ss, data);
- if ( ret != 0 )
- return ret;
- }
-
- node = fdt_next_node(fdt, node, &depth);
- } while ( node >= 0 && depth > 0 );
-
- return 0;
-}
-
static int __init process_memory_node(const void *fdt, int node,
const char *name, int depth,
u32 address_cells, u32 size_cells,
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v2 11/15] xen/dt: Move bootinfo-independent helpers out of bootinfo-fdt.c
2025-06-05 19:48 ` [PATCH v2 11/15] xen/dt: Move bootinfo-independent helpers out of bootinfo-fdt.c Alejandro Vallejo
@ 2025-06-06 23:43 ` Stefano Stabellini
0 siblings, 0 replies; 39+ messages in thread
From: Stefano Stabellini @ 2025-06-06 23:43 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: xen-devel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
Michal Orzel, Daniel P. Smith
On Thu, 5 Jun 2025, Alejandro Vallejo wrote:
> ... back into bootfdt.c
>
> These will be required by x86 later on to detect modules in the early scan of
> the FDT. They are independent of bootinfo, so it's cleaner for them to exist in
> a separate file.
>
> Not a functional change.
>
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
In retrospect, it would have been better to keep the code movement as is
and do any changes as a separate patch. But given that I was the one to
ask for it:
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> xen/common/device-tree/Makefile | 1 +
> xen/common/device-tree/bootfdt.c | 97 ++++++++++++++++++++++++
> xen/common/device-tree/bootinfo-fdt.c | 104 --------------------------
> 3 files changed, 98 insertions(+), 104 deletions(-)
> create mode 100644 xen/common/device-tree/bootfdt.c
>
> diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
> index 8a3f2be89e..8abc069c4b 100644
> --- a/xen/common/device-tree/Makefile
> +++ b/xen/common/device-tree/Makefile
> @@ -1,3 +1,4 @@
> +obj-y += bootfdt.init.o
> obj-y += bootinfo-fdt.init.o
> obj-y += bootinfo.init.o
> obj-y += device-tree.o
> diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c
> new file mode 100644
> index 0000000000..6acf83d197
> --- /dev/null
> +++ b/xen/common/device-tree/bootfdt.c
> @@ -0,0 +1,97 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#include <xen/bootfdt.h>
> +#include <xen/bug.h>
> +#include <xen/lib.h>
> +#include <xen/libfdt/libfdt.h>
> +
> +uint32_t __init device_tree_get_u32(const void *fdt, int node,
> + const char *prop_name, uint32_t dflt)
> +{
> + const struct fdt_property *prop;
> +
> + prop = fdt_get_property(fdt, node, prop_name, NULL);
> + if ( !prop || prop->len < sizeof(uint32_t) )
> + return dflt;
> +
> + return fdt32_to_cpu(*(const uint32_t*)prop->data);
> +}
> +
> +int __init device_tree_for_each_node(const void *fdt, int node,
> + device_tree_node_func func,
> + void *data)
> +{
> + /*
> + * We only care about relative depth increments, assume depth of
> + * node is 0 for simplicity.
> + */
> + int depth = 0;
> + const int first_node = node;
> + uint32_t address_cells[DEVICE_TREE_MAX_DEPTH];
> + uint32_t size_cells[DEVICE_TREE_MAX_DEPTH];
> + int ret;
> +
> + do {
> + const char *name = fdt_get_name(fdt, node, NULL);
> + uint32_t as, ss;
> +
> + if ( depth >= DEVICE_TREE_MAX_DEPTH )
> + {
> + printk("Warning: device tree node `%s' is nested too deep\n",
> + name);
> + continue;
> + }
> +
> + as = depth > 0 ? address_cells[depth-1] : DT_ROOT_NODE_ADDR_CELLS_DEFAULT;
> + ss = depth > 0 ? size_cells[depth-1] : DT_ROOT_NODE_SIZE_CELLS_DEFAULT;
> +
> + address_cells[depth] = device_tree_get_u32(fdt, node,
> + "#address-cells", as);
> + size_cells[depth] = device_tree_get_u32(fdt, node,
> + "#size-cells", ss);
> +
> + /* skip the first node */
> + if ( node != first_node )
> + {
> + ret = func(fdt, node, name, depth, as, ss, data);
> + if ( ret != 0 )
> + return ret;
> + }
> +
> + node = fdt_next_node(fdt, node, &depth);
> + } while ( node >= 0 && depth > 0 );
> +
> + return 0;
> +}
> +
> +void __init device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
> + uint32_t size_cells, paddr_t *start,
> + paddr_t *size)
> +{
> + uint64_t dt_start, dt_size;
> +
> + /*
> + * dt_next_cell will return uint64_t whereas paddr_t may not be 64-bit.
> + * Thus, there is an implicit cast from uint64_t to paddr_t.
> + */
> + dt_start = dt_next_cell(address_cells, cell);
> + dt_size = dt_next_cell(size_cells, cell);
> +
> + if ( dt_start != (paddr_t)dt_start )
> + {
> + printk("Physical address greater than max width supported\n");
> + WARN();
> + }
> +
> + if ( dt_size != (paddr_t)dt_size )
> + {
> + printk("Physical size greater than max width supported\n");
> + WARN();
> + }
> +
> + /*
> + * Xen will truncate the address/size if it is greater than the maximum
> + * supported width and it will give an appropriate warning.
> + */
> + *start = dt_start;
> + *size = dt_size;
> +}
> diff --git a/xen/common/device-tree/bootinfo-fdt.c b/xen/common/device-tree/bootinfo-fdt.c
> index 195249f700..695c4bfc49 100644
> --- a/xen/common/device-tree/bootinfo-fdt.c
> +++ b/xen/common/device-tree/bootinfo-fdt.c
> @@ -109,39 +109,6 @@ static bool __init device_tree_is_memory_node(const void *fdt, int node,
> return true;
> }
>
> -void __init device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
> - uint32_t size_cells, paddr_t *start,
> - paddr_t *size)
> -{
> - uint64_t dt_start, dt_size;
> -
> - /*
> - * dt_next_cell will return uint64_t whereas paddr_t may not be 64-bit.
> - * Thus, there is an implicit cast from uint64_t to paddr_t.
> - */
> - dt_start = dt_next_cell(address_cells, cell);
> - dt_size = dt_next_cell(size_cells, cell);
> -
> - if ( dt_start != (paddr_t)dt_start )
> - {
> - printk("Physical address greater than max width supported\n");
> - WARN();
> - }
> -
> - if ( dt_size != (paddr_t)dt_size )
> - {
> - printk("Physical size greater than max width supported\n");
> - WARN();
> - }
> -
> - /*
> - * Xen will truncate the address/size if it is greater than the maximum
> - * supported width and it will give an appropriate warning.
> - */
> - *start = dt_start;
> - *size = dt_size;
> -}
> -
> static int __init device_tree_get_meminfo(const void *fdt, int node,
> const char *prop_name,
> u32 address_cells, u32 size_cells,
> @@ -202,77 +169,6 @@ static int __init device_tree_get_meminfo(const void *fdt, int node,
> return 0;
> }
>
> -uint32_t __init device_tree_get_u32(const void *fdt, int node,
> - const char *prop_name, uint32_t dflt)
> -{
> - const struct fdt_property *prop;
> -
> - prop = fdt_get_property(fdt, node, prop_name, NULL);
> - if ( !prop || prop->len < sizeof(uint32_t) )
> - return dflt;
> -
> - return fdt32_to_cpu(*(const uint32_t*)prop->data);
> -}
> -
> -/**
> - * device_tree_for_each_node - iterate over all device tree sub-nodes
> - * @fdt: flat device tree.
> - * @node: parent node to start the search from
> - * @func: function to call for each sub-node.
> - * @data: data to pass to @func.
> - *
> - * Any nodes nested at DEVICE_TREE_MAX_DEPTH or deeper are ignored.
> - *
> - * Returns 0 if all nodes were iterated over successfully. If @func
> - * returns a value different from 0, that value is returned immediately.
> - */
> -int __init device_tree_for_each_node(const void *fdt, int node,
> - device_tree_node_func func,
> - void *data)
> -{
> - /*
> - * We only care about relative depth increments, assume depth of
> - * node is 0 for simplicity.
> - */
> - int depth = 0;
> - const int first_node = node;
> - u32 address_cells[DEVICE_TREE_MAX_DEPTH];
> - u32 size_cells[DEVICE_TREE_MAX_DEPTH];
> - int ret;
> -
> - do {
> - const char *name = fdt_get_name(fdt, node, NULL);
> - u32 as, ss;
> -
> - if ( depth >= DEVICE_TREE_MAX_DEPTH )
> - {
> - printk("Warning: device tree node `%s' is nested too deep\n",
> - name);
> - continue;
> - }
> -
> - as = depth > 0 ? address_cells[depth-1] : DT_ROOT_NODE_ADDR_CELLS_DEFAULT;
> - ss = depth > 0 ? size_cells[depth-1] : DT_ROOT_NODE_SIZE_CELLS_DEFAULT;
> -
> - address_cells[depth] = device_tree_get_u32(fdt, node,
> - "#address-cells", as);
> - size_cells[depth] = device_tree_get_u32(fdt, node,
> - "#size-cells", ss);
> -
> - /* skip the first node */
> - if ( node != first_node )
> - {
> - ret = func(fdt, node, name, depth, as, ss, data);
> - if ( ret != 0 )
> - return ret;
> - }
> -
> - node = fdt_next_node(fdt, node, &depth);
> - } while ( node >= 0 && depth > 0 );
> -
> - return 0;
> -}
> -
> static int __init process_memory_node(const void *fdt, int node,
> const char *name, int depth,
> u32 address_cells, u32 size_cells,
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 12/15] xen/dt: Extract helper to map nodes to module kinds
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (10 preceding siblings ...)
2025-06-05 19:48 ` [PATCH v2 11/15] xen/dt: Move bootinfo-independent helpers out of bootinfo-fdt.c Alejandro Vallejo
@ 2025-06-05 19:48 ` Alejandro Vallejo
2025-06-05 19:48 ` [PATCH v2 13/15] xen/dt: ifdef out DEV_DT-related bits from device_tree.{c,h} Alejandro Vallejo
` (2 subsequent siblings)
14 siblings, 0 replies; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:48 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Daniel P. Smith
This will be required later by x86 code in order to do early identification
of boot modules when booting off a DTB.
Not a functional change.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
xen/common/device-tree/bootfdt.c | 18 ++++++++++++++++++
xen/common/device-tree/bootinfo-fdt.c | 16 +---------------
xen/include/xen/bootfdt.h | 7 +++++++
3 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c
index 6acf83d197..474d719d2f 100644
--- a/xen/common/device-tree/bootfdt.c
+++ b/xen/common/device-tree/bootfdt.c
@@ -4,6 +4,24 @@
#include <xen/lib.h>
#include <xen/libfdt/libfdt.h>
+bootmodule_kind __init fdt_node_to_kind(const void *fdt, int node)
+{
+ if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 ||
+ fdt_node_check_compatible(fdt, node, "multiboot,kernel") == 0 )
+ return BOOTMOD_KERNEL;
+ if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0 ||
+ fdt_node_check_compatible(fdt, node, "multiboot,ramdisk") == 0 )
+ return BOOTMOD_RAMDISK;
+ if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 )
+ return BOOTMOD_XSM;
+ if ( fdt_node_check_compatible(fdt, node, "multiboot,device-tree") == 0 )
+ return BOOTMOD_GUEST_DTB;
+ if ( fdt_node_check_compatible(fdt, node, "multiboot,microcode") == 0 )
+ return BOOTMOD_MICROCODE;
+
+ return BOOTMOD_UNKNOWN;
+}
+
uint32_t __init device_tree_get_u32(const void *fdt, int node,
const char *prop_name, uint32_t dflt)
{
diff --git a/xen/common/device-tree/bootinfo-fdt.c b/xen/common/device-tree/bootinfo-fdt.c
index 695c4bfc49..20821e9ee8 100644
--- a/xen/common/device-tree/bootinfo-fdt.c
+++ b/xen/common/device-tree/bootinfo-fdt.c
@@ -236,21 +236,7 @@ static void __init process_multiboot_node(const void *fdt, int node,
cell = (const __be32 *)prop->data;
device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
-
- if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 ||
- fdt_node_check_compatible(fdt, node, "multiboot,kernel") == 0 )
- kind = BOOTMOD_KERNEL;
- else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0 ||
- fdt_node_check_compatible(fdt, node, "multiboot,ramdisk") == 0 )
- kind = BOOTMOD_RAMDISK;
- else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 )
- kind = BOOTMOD_XSM;
- else if ( fdt_node_check_compatible(fdt, node, "multiboot,device-tree") == 0 )
- kind = BOOTMOD_GUEST_DTB;
- else if ( fdt_node_check_compatible(fdt, node, "multiboot,microcode") == 0 )
- kind = BOOTMOD_MICROCODE;
- else
- kind = BOOTMOD_UNKNOWN;
+ kind = fdt_node_to_kind(fdt, node);
/**
* Guess the kind of these first two unknowns respectively:
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index a87b5212a6..13d19562c7 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -115,4 +115,11 @@ uint32_t device_tree_get_u32(const void *fdt, int node,
void device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
uint32_t size_cells, paddr_t *start, paddr_t *size);
+/*
+ * Probe an FDT node thought to be a boot module to identify its kind.
+ *
+ * If correctly identified, returns the detected kind, otherwise BOOTMOD_UNKNOWN
+ */
+bootmodule_kind fdt_node_to_kind(const void *fdt, int node);
+
#endif /* XEN_BOOTFDT_H */
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v2 13/15] xen/dt: ifdef out DEV_DT-related bits from device_tree.{c,h}
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (11 preceding siblings ...)
2025-06-05 19:48 ` [PATCH v2 12/15] xen/dt: Extract helper to map nodes to module kinds Alejandro Vallejo
@ 2025-06-05 19:48 ` Alejandro Vallejo
2025-06-06 23:54 ` Stefano Stabellini
2025-06-05 19:48 ` [PATCH v2 14/15] xen/dt: Allow CONFIG_DOM0LESS_BOOT to include device-tree/ Alejandro Vallejo
2025-06-05 19:48 ` [PATCH v2 15/15] kconfig: Allow x86 to pick CONFIG_DOM0LESS_BOOT Alejandro Vallejo
14 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:48 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Daniel P. Smith
... which means, device-tree.c stops requiring strictly CONFIG_HAS_DEVICE_TREE
and may function without it.
Not a functional change on architectures that currently use these files,
as they already select CONFIG_HAS_DEVICE_TREE.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
xen/common/device-tree/device-tree.c | 2 ++
xen/include/xen/device_tree.h | 4 ++++
2 files changed, 6 insertions(+)
diff --git a/xen/common/device-tree/device-tree.c b/xen/common/device-tree/device-tree.c
index 886e6c7712..c8a9c0e46a 100644
--- a/xen/common/device-tree/device-tree.c
+++ b/xen/common/device-tree/device-tree.c
@@ -2028,9 +2028,11 @@ static unsigned long unflatten_dt_node(const void *fdt,
((char *)pp->value)[sz - 1] = 0;
dt_dprintk("fixed up name for %s -> %s\n", pathp,
(char *)pp->value);
+#ifdef CONFIG_HAS_DEVICE_TREE
/* Generic device initialization */
np->dev.type = DEV_DT;
np->dev.of_node = np;
+#endif /* CONFIG_HAS_DEVICE_TREE */
}
}
if ( allnextpp )
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 7d1c8bc305..641f24518d 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -109,9 +109,12 @@ struct dt_device_node {
*/
struct list_head domain_list;
+#ifdef CONFIG_HAS_DEVICE_TREE
struct device dev;
+#endif /* CONFIG_HAS_DEVICE_TREE */
};
+#ifdef CONFIG_HAS_DEVICE_TREE
#define dt_to_dev(dt_node) (&(dt_node)->dev)
static inline struct dt_device_node *dev_to_dt(struct device *dev)
@@ -120,6 +123,7 @@ static inline struct dt_device_node *dev_to_dt(struct device *dev)
return container_of(dev, struct dt_device_node, dev);
}
+#endif /* CONFIG_HAS_DEVICE_TREE */
#define MAX_PHANDLE_ARGS 16
struct dt_phandle_args {
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v2 13/15] xen/dt: ifdef out DEV_DT-related bits from device_tree.{c,h}
2025-06-05 19:48 ` [PATCH v2 13/15] xen/dt: ifdef out DEV_DT-related bits from device_tree.{c,h} Alejandro Vallejo
@ 2025-06-06 23:54 ` Stefano Stabellini
0 siblings, 0 replies; 39+ messages in thread
From: Stefano Stabellini @ 2025-06-06 23:54 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: xen-devel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
Michal Orzel, Daniel P. Smith
On Thu, 5 Jun 2025, Alejandro Vallejo wrote:
> ... which means, device-tree.c stops requiring strictly CONFIG_HAS_DEVICE_TREE
> and may function without it.
>
> Not a functional change on architectures that currently use these files,
> as they already select CONFIG_HAS_DEVICE_TREE.
>
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 14/15] xen/dt: Allow CONFIG_DOM0LESS_BOOT to include device-tree/
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (12 preceding siblings ...)
2025-06-05 19:48 ` [PATCH v2 13/15] xen/dt: ifdef out DEV_DT-related bits from device_tree.{c,h} Alejandro Vallejo
@ 2025-06-05 19:48 ` Alejandro Vallejo
2025-06-06 23:55 ` Stefano Stabellini
2025-06-05 19:48 ` [PATCH v2 15/15] kconfig: Allow x86 to pick CONFIG_DOM0LESS_BOOT Alejandro Vallejo
14 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:48 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Andrew Cooper, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Roger Pau Monné,
Stefano Stabellini, Bertrand Marquis, Daniel P. Smith
... without CONFIG_HAS_DEVICE_TREE
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
xen/common/Kconfig | 1 +
xen/common/Makefile | 2 +-
xen/common/device-tree/Makefile | 8 ++++----
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index eece1370a3..5353874e87 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -14,6 +14,7 @@ config CORE_PARKING
config DOM0LESS_BOOT
bool "Dom0less boot support" if EXPERT
+ select LIBFDT
depends on HAS_DOM0LESS && HAS_DEVICE_TREE && DOMAIN_BUILD_HELPERS
default y
help
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 98f0873056..2717c81f9c 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -8,7 +8,7 @@ obj-y += cpu.o
obj-$(CONFIG_DEBUG_TRACE) += debugtrace.o
obj-$(CONFIG_HAS_DEVICE_TREE) += device.o
obj-$(filter-out $(CONFIG_X86),$(CONFIG_ACPI)) += device.o
-obj-$(CONFIG_HAS_DEVICE_TREE) += device-tree/
+obj-$(firstword $(CONFIG_HAS_DEVICE_TREE) $(CONFIG_DOM0LESS_BOOT)) += device-tree/
obj-$(CONFIG_IOREQ_SERVER) += dm.o
obj-y += domain.o
obj-y += event_2l.o
diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
index 8abc069c4b..cc56f42df9 100644
--- a/xen/common/device-tree/Makefile
+++ b/xen/common/device-tree/Makefile
@@ -1,11 +1,11 @@
obj-y += bootfdt.init.o
-obj-y += bootinfo-fdt.init.o
-obj-y += bootinfo.init.o
-obj-y += device-tree.o
+obj-$(CONFIG_HAS_DEVICE_TREE) += bootinfo-fdt.init.o
+obj-$(CONFIG_HAS_DEVICE_TREE) += bootinfo.init.o
+obj-$(CONFIG_HAS_DEVICE_TREE) += device-tree.o
obj-$(CONFIG_DOMAIN_BUILD_HELPERS) += domain-build.init.o
obj-$(CONFIG_DOM0LESS_BOOT) += dom0less-build.init.o
obj-$(CONFIG_OVERLAY_DTB) += dt-overlay.o
-obj-y += intc.o
+obj-$(CONFIG_HAS_DEVICE_TREE) += intc.o
obj-$(CONFIG_DOMAIN_BUILD_HELPERS) += kernel.o
obj-$(CONFIG_STATIC_EVTCHN) += static-evtchn.init.o
obj-$(CONFIG_STATIC_MEMORY) += static-memory.init.o
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v2 14/15] xen/dt: Allow CONFIG_DOM0LESS_BOOT to include device-tree/
2025-06-05 19:48 ` [PATCH v2 14/15] xen/dt: Allow CONFIG_DOM0LESS_BOOT to include device-tree/ Alejandro Vallejo
@ 2025-06-06 23:55 ` Stefano Stabellini
0 siblings, 0 replies; 39+ messages in thread
From: Stefano Stabellini @ 2025-06-06 23:55 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: xen-devel, Andrew Cooper, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Roger Pau Monné,
Stefano Stabellini, Bertrand Marquis, Daniel P. Smith
On Thu, 5 Jun 2025, Alejandro Vallejo wrote:
> ... without CONFIG_HAS_DEVICE_TREE
>
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 15/15] kconfig: Allow x86 to pick CONFIG_DOM0LESS_BOOT
2025-06-05 19:47 [PATCH v2 00/15] Allow x86 to unflatten DTs Alejandro Vallejo
` (13 preceding siblings ...)
2025-06-05 19:48 ` [PATCH v2 14/15] xen/dt: Allow CONFIG_DOM0LESS_BOOT to include device-tree/ Alejandro Vallejo
@ 2025-06-05 19:48 ` Alejandro Vallejo
2025-06-06 23:57 ` Stefano Stabellini
14 siblings, 1 reply; 39+ messages in thread
From: Alejandro Vallejo @ 2025-06-05 19:48 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Anthony PERARD, Michal Orzel, Julien Grall,
Stefano Stabellini, Bertrand Marquis, Daniel P. Smith
Without picking CONFIG_HAS_DEVICE_TREE.
In order to do that. Allow CONFIG_DOM0LESS_BOOT to enable a subset
of the common/device-tree/ directory. x86 doesn't want dom0less-build.c,
as that's tightly integrated still to the ARM way of building domains.
Requires "unsupported" for the time being until all required patches
make it through.
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
v2:
* (required on rebase) Add an explicit dependency on HAS_DEVICE_TREE
for STATIC_MEMORY and STATIC_EVTCHN so they're not pulled-in. Could
use !X86 as well.
---
xen/arch/x86/Kconfig | 1 +
xen/common/Kconfig | 12 +++++++-----
xen/common/device-tree/Makefile | 2 +-
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 7afe879710..4344b4289c 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -18,6 +18,7 @@ config X86
select HAS_COMPAT
select HAS_CPUFREQ
select HAS_DIT
+ select HAS_DOM0LESS
select HAS_EHCI
select HAS_EX_TABLE
select HAS_FAST_MULTIPLY
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 5353874e87..bcf87b4d9d 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -13,15 +13,17 @@ config CORE_PARKING
depends on NR_CPUS > 1
config DOM0LESS_BOOT
- bool "Dom0less boot support" if EXPERT
+ bool "Dom0less boot support" if EXPERT && (!X86 || UNSUPPORTED)
select LIBFDT
- depends on HAS_DOM0LESS && HAS_DEVICE_TREE && DOMAIN_BUILD_HELPERS
- default y
+ depends on HAS_DOM0LESS && (X86 || (HAS_DEVICE_TREE && DOMAIN_BUILD_HELPERS))
+ default y if !X86
help
Dom0less boot support enables Xen to create and start domU guests during
Xen boot without the need of a control domain (Dom0), which could be
present anyway.
+ If unsure on x86, say N.
+
config DOMAIN_BUILD_HELPERS
bool
@@ -152,7 +154,7 @@ config NUMA
config STATIC_MEMORY
bool "Static Allocation Support (UNSUPPORTED)" if UNSUPPORTED
- depends on DOM0LESS_BOOT
+ depends on HAS_DEVICE_TREE && DOM0LESS_BOOT
help
Static Allocation refers to system or sub-system(domains) for
which memory areas are pre-defined by configuration using physical
@@ -171,7 +173,7 @@ config STATIC_SHM
config STATIC_EVTCHN
bool "Static event channel support on a dom0less system"
- depends on DOM0LESS_BOOT
+ depends on HAS_DEVICE_TREE && DOM0LESS_BOOT
default y
help
This option enables establishing static event channel communication
diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
index cc56f42df9..f5410e685b 100644
--- a/xen/common/device-tree/Makefile
+++ b/xen/common/device-tree/Makefile
@@ -3,7 +3,7 @@ obj-$(CONFIG_HAS_DEVICE_TREE) += bootinfo-fdt.init.o
obj-$(CONFIG_HAS_DEVICE_TREE) += bootinfo.init.o
obj-$(CONFIG_HAS_DEVICE_TREE) += device-tree.o
obj-$(CONFIG_DOMAIN_BUILD_HELPERS) += domain-build.init.o
-obj-$(CONFIG_DOM0LESS_BOOT) += dom0less-build.init.o
+obj-$(filter-out $(CONFIG_X86),$(CONFIG_DOM0LESS_BOOT)) += dom0less-build.init.o
obj-$(CONFIG_OVERLAY_DTB) += dt-overlay.o
obj-$(CONFIG_HAS_DEVICE_TREE) += intc.o
obj-$(CONFIG_DOMAIN_BUILD_HELPERS) += kernel.o
--
2.43.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v2 15/15] kconfig: Allow x86 to pick CONFIG_DOM0LESS_BOOT
2025-06-05 19:48 ` [PATCH v2 15/15] kconfig: Allow x86 to pick CONFIG_DOM0LESS_BOOT Alejandro Vallejo
@ 2025-06-06 23:57 ` Stefano Stabellini
0 siblings, 0 replies; 39+ messages in thread
From: Stefano Stabellini @ 2025-06-06 23:57 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Anthony PERARD, Michal Orzel, Julien Grall, Stefano Stabellini,
Bertrand Marquis, Daniel P. Smith
On Thu, 5 Jun 2025, Alejandro Vallejo wrote:
> Without picking CONFIG_HAS_DEVICE_TREE.
>
> In order to do that. Allow CONFIG_DOM0LESS_BOOT to enable a subset
> of the common/device-tree/ directory. x86 doesn't want dom0less-build.c,
> as that's tightly integrated still to the ARM way of building domains.
>
> Requires "unsupported" for the time being until all required patches
> make it through.
>
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
^ permalink raw reply [flat|nested] 39+ messages in thread