Introduce the ability to assign capabilities to a domain via its definition in
device tree. The first capability enabled to select is the control domain
capability. The capability property is a bitfield in both the device tree and
`struct boot_domain`.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
xen/arch/x86/domain_builder/core.c | 2 +-
xen/arch/x86/domain_builder/fdt.c | 13 +++++++++++++
xen/arch/x86/include/asm/bootdomain.h | 4 ++++
xen/arch/x86/setup.c | 6 +++++-
4 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/domain_builder/core.c b/xen/arch/x86/domain_builder/core.c
index 95cab06e6159..eaa019472724 100644
--- a/xen/arch/x86/domain_builder/core.c
+++ b/xen/arch/x86/domain_builder/core.c
@@ -93,9 +93,9 @@ void __init builder_init(struct boot_info *bi)
i = first_boot_module_index(bi, BOOTMOD_UNKNOWN);
bi->mods[i].type = BOOTMOD_KERNEL;
bi->domains[0].kernel = &bi->mods[i];
+ bi->domains[0].capabilities |= BUILD_CAPS_CONTROL;
bi->nr_domains = 1;
}
-
}
/*
diff --git a/xen/arch/x86/domain_builder/fdt.c b/xen/arch/x86/domain_builder/fdt.c
index d24e265f2378..9499e337938c 100644
--- a/xen/arch/x86/domain_builder/fdt.c
+++ b/xen/arch/x86/domain_builder/fdt.c
@@ -209,6 +209,19 @@ static int __init process_domain_node(
bd->max_vcpus = val;
printk(" max vcpus: %d\n", bd->max_vcpus);
}
+ if ( match_fdt_property(fdt, prop, "capabilities" ) )
+ {
+ if ( fdt_prop_as_u32(prop, &bd->capabilities) != 0 )
+ {
+ printk(" failed processing domain id for domain %s\n",
+ name == NULL ? "unknown" : name);
+ return -EINVAL;
+ }
+ printk(" caps: ");
+ if ( bd->capabilities & BUILD_CAPS_CONTROL )
+ printk("c");
+ printk("\n");
+ }
}
fdt_for_each_subnode(node, fdt, dom_node)
diff --git a/xen/arch/x86/include/asm/bootdomain.h b/xen/arch/x86/include/asm/bootdomain.h
index d144d6173400..51ebf1f68189 100644
--- a/xen/arch/x86/include/asm/bootdomain.h
+++ b/xen/arch/x86/include/asm/bootdomain.h
@@ -18,6 +18,10 @@ struct boot_domain {
domid_t domid;
+#define BUILD_CAPS_NONE (0)
+#define BUILD_CAPS_CONTROL (1 << 0)
+ uint32_t capabilities;
+
/* On | Off */
#define BUILD_MODE_PARAVIRT (1 << 0) /* PV | PVH/HVM */
#define BUILD_MODE_ENABLE_DM (1 << 1) /* HVM | PVH */
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index dae25721994d..28e750a420e8 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -992,6 +992,7 @@ static size_t __init domain_cmdline_size(
static struct domain *__init create_dom0(struct boot_info *bi)
{
char *cmdline = NULL;
+ int create_flags = 0;
struct xen_domctl_createdomain dom0_cfg = {
.flags = IS_ENABLED(CONFIG_TBOOT) ? XEN_DOMCTL_CDF_s3_integrity : 0,
.max_evtchn_port = -1,
@@ -1023,7 +1024,10 @@ static struct domain *__init create_dom0(struct boot_info *bi)
/* Create initial domain. Not d0 for pvshim. */
if ( bd->domid == DOMID_INVALID )
bd->domid = get_initial_domain_id();
- d = domain_create(bd->domid, &dom0_cfg, pv_shim ? 0 : CDF_privileged);
+ if ( bd->capabilities & BUILD_CAPS_CONTROL )
+ create_flags |= CDF_privileged;
+ d = domain_create(bd->domid, &dom0_cfg,
+ pv_shim ? 0 : create_flags);
if ( IS_ERR(d) )
panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d));
--
2.30.2
On 23.11.2024 19:20, Daniel P. Smith wrote: > --- a/xen/arch/x86/domain_builder/fdt.c > +++ b/xen/arch/x86/domain_builder/fdt.c > @@ -209,6 +209,19 @@ static int __init process_domain_node( > bd->max_vcpus = val; > printk(" max vcpus: %d\n", bd->max_vcpus); > } > + if ( match_fdt_property(fdt, prop, "capabilities" ) ) > + { > + if ( fdt_prop_as_u32(prop, &bd->capabilities) != 0 ) > + { > + printk(" failed processing domain id for domain %s\n", > + name == NULL ? "unknown" : name); > + return -EINVAL; > + } > + printk(" caps: "); > + if ( bd->capabilities & BUILD_CAPS_CONTROL ) > + printk("c"); > + printk("\n"); > + } What if any of the other bits is set? > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -992,6 +992,7 @@ static size_t __init domain_cmdline_size( > static struct domain *__init create_dom0(struct boot_info *bi) > { > char *cmdline = NULL; > + int create_flags = 0; Once again unsigned int please. > @@ -1023,7 +1024,10 @@ static struct domain *__init create_dom0(struct boot_info *bi) > /* Create initial domain. Not d0 for pvshim. */ > if ( bd->domid == DOMID_INVALID ) > bd->domid = get_initial_domain_id(); > - d = domain_create(bd->domid, &dom0_cfg, pv_shim ? 0 : CDF_privileged); > + if ( bd->capabilities & BUILD_CAPS_CONTROL ) > + create_flags |= CDF_privileged; Nit: Indentation. Jan
On 12/2/24 07:23, Jan Beulich wrote: > On 23.11.2024 19:20, Daniel P. Smith wrote: >> --- a/xen/arch/x86/domain_builder/fdt.c >> +++ b/xen/arch/x86/domain_builder/fdt.c >> @@ -209,6 +209,19 @@ static int __init process_domain_node( >> bd->max_vcpus = val; >> printk(" max vcpus: %d\n", bd->max_vcpus); >> } >> + if ( match_fdt_property(fdt, prop, "capabilities" ) ) >> + { >> + if ( fdt_prop_as_u32(prop, &bd->capabilities) != 0 ) >> + { >> + printk(" failed processing domain id for domain %s\n", >> + name == NULL ? "unknown" : name); >> + return -EINVAL; >> + } >> + printk(" caps: "); >> + if ( bd->capabilities & BUILD_CAPS_CONTROL ) >> + printk("c"); >> + printk("\n"); >> + } > > What if any of the other bits is set? I'm not sure what you are getting at, but there is another cap added later for HARDWARE and it will print an 'h' next to the 'c' if set. >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -992,6 +992,7 @@ static size_t __init domain_cmdline_size( >> static struct domain *__init create_dom0(struct boot_info *bi) >> { >> char *cmdline = NULL; >> + int create_flags = 0; > > Once again unsigned int please. ack. >> @@ -1023,7 +1024,10 @@ static struct domain *__init create_dom0(struct boot_info *bi) >> /* Create initial domain. Not d0 for pvshim. */ >> if ( bd->domid == DOMID_INVALID ) >> bd->domid = get_initial_domain_id(); >> - d = domain_create(bd->domid, &dom0_cfg, pv_shim ? 0 : CDF_privileged); >> + if ( bd->capabilities & BUILD_CAPS_CONTROL ) >> + create_flags |= CDF_privileged; > > Nit: Indentation. ack. v/r, dps
On 11.12.2024 20:56, Daniel P. Smith wrote: > On 12/2/24 07:23, Jan Beulich wrote: >> On 23.11.2024 19:20, Daniel P. Smith wrote: >>> --- a/xen/arch/x86/domain_builder/fdt.c >>> +++ b/xen/arch/x86/domain_builder/fdt.c >>> @@ -209,6 +209,19 @@ static int __init process_domain_node( >>> bd->max_vcpus = val; >>> printk(" max vcpus: %d\n", bd->max_vcpus); >>> } >>> + if ( match_fdt_property(fdt, prop, "capabilities" ) ) >>> + { >>> + if ( fdt_prop_as_u32(prop, &bd->capabilities) != 0 ) >>> + { >>> + printk(" failed processing domain id for domain %s\n", >>> + name == NULL ? "unknown" : name); >>> + return -EINVAL; >>> + } >>> + printk(" caps: "); >>> + if ( bd->capabilities & BUILD_CAPS_CONTROL ) >>> + printk("c"); >>> + printk("\n"); >>> + } >> >> What if any of the other bits is set? > > I'm not sure what you are getting at, but there is another cap added > later for HARDWARE and it will print an 'h' next to the 'c' if set. And that bit, when set, will likely have meaning beyond this mere printing? If so, what's the effect on the domain when the bit is set, but the code consuming the bit isn't there yet? Will the domain function correctly? IOW shouldn't you reject any set bits that the code doesn't know how to handle? And then perhaps report all unknown bits in a numeric value here? Jan
On 2024-11-23 13:20, Daniel P. Smith wrote: > Introduce the ability to assign capabilities to a domain via its definition in > device tree. The first capability enabled to select is the control domain > capability. The capability property is a bitfield in both the device tree and > `struct boot_domain`. > > Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> > --- > xen/arch/x86/domain_builder/core.c | 2 +- > xen/arch/x86/domain_builder/fdt.c | 13 +++++++++++++ > xen/arch/x86/include/asm/bootdomain.h | 4 ++++ > xen/arch/x86/setup.c | 6 +++++- > 4 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/domain_builder/core.c b/xen/arch/x86/domain_builder/core.c > index 95cab06e6159..eaa019472724 100644 > --- a/xen/arch/x86/domain_builder/core.c > +++ b/xen/arch/x86/domain_builder/core.c > @@ -93,9 +93,9 @@ void __init builder_init(struct boot_info *bi) > i = first_boot_module_index(bi, BOOTMOD_UNKNOWN); > bi->mods[i].type = BOOTMOD_KERNEL; > bi->domains[0].kernel = &bi->mods[i]; > + bi->domains[0].capabilities |= BUILD_CAPS_CONTROL; > bi->nr_domains = 1; > } > - This will get cleaned up earlier. With that: Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
On 11/25/24 19:09, Jason Andryuk wrote: > On 2024-11-23 13:20, Daniel P. Smith wrote: >> Introduce the ability to assign capabilities to a domain via its >> definition in >> device tree. The first capability enabled to select is the control domain >> capability. The capability property is a bitfield in both the device >> tree and >> `struct boot_domain`. >> >> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> >> --- >> xen/arch/x86/domain_builder/core.c | 2 +- >> xen/arch/x86/domain_builder/fdt.c | 13 +++++++++++++ >> xen/arch/x86/include/asm/bootdomain.h | 4 ++++ >> xen/arch/x86/setup.c | 6 +++++- >> 4 files changed, 23 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/x86/domain_builder/core.c b/xen/arch/x86/ >> domain_builder/core.c >> index 95cab06e6159..eaa019472724 100644 >> --- a/xen/arch/x86/domain_builder/core.c >> +++ b/xen/arch/x86/domain_builder/core.c >> @@ -93,9 +93,9 @@ void __init builder_init(struct boot_info *bi) >> i = first_boot_module_index(bi, BOOTMOD_UNKNOWN); >> bi->mods[i].type = BOOTMOD_KERNEL; >> bi->domains[0].kernel = &bi->mods[i]; >> + bi->domains[0].capabilities |= BUILD_CAPS_CONTROL; >> bi->nr_domains = 1; >> } >> - > > This will get cleaned up earlier. > > With that: > > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> Thanks!
© 2016 - 2024 Red Hat, Inc.