On 2025-04-01 08:16, Jan Beulich wrote:
On 31.03.2025 23:43, Jason Andryuk wrote:
xenstored maps other domains' xenstore pages. Currently this relies on
init-dom0less or xl to seed the grants from Dom0. With split
hardware/control/xenstore domains, this is problematic since we don't
want the hardware domain to be able to map other domains' resources
without their permission. Instead have the hypervisor seed the grant
table entry for every dom0less domain. The grant is then accessible as
normal.
Yet aiui the original idea was to specifically not do this in the hypervisor.
I agree it shouldn't be the hardware domain, but what's wrong with having the
control domain do that? It is what is responsible for creating new domains as
well. The question of where to do that when there's no control domain must
also have been solved already (without me knowing the answer), as that's
where init-dom0less must be running.
dom0less does not allow configuring a domU to have xenstore without
using dom0 today. This series changes the dependency to the xenstore
domain.
I want to move away from relying on the control domain to set up
xenstore connections. Domain boot can achieve more parallelism if
xenstore domain can independently configure its connections. If
xenstore connections need to wait until dom0/control userspace runs,
that is a measurable delay.
From the permission side, with a preseeded grant, and the event channel
places in the xenstore_domain_interface page, a xenstore domain can
connect and introduce existing domains when it starts up without needing
additional permissions.
Generally, I'm looking at making xenstored perform more configuration on
its own. It doesn't matter for dom0, but it will help for
dom0less/Hyperlaunch. Seeding grants from the hypervisor is a small
change, but it helps remove dependencies for xenstore.
This works with C xenstored. OCaml xenstored does not use grants and
would fail to foreign map the page.
From the sentence it's not clear whether this is unchanged behavior or
a deliberate regression.
I was trying to highlight existing compatibility. xenstored uses
grants, so it can take advantage of the pre-seeded grant and therefore
does not need privilege to map foreign pages. OCaml does not use
grants, so this seeding is irrelevant. With a combined
hardware|xenstore domain, C xenstored using grants works, but Ocaml does
not.
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -865,6 +865,10 @@ static void __init initialize_domU_xenstore(void)
rc = alloc_xenstore_evtchn(d);
if ( rc < 0 )
panic("%pd: Failed to allocate xenstore_evtchn\n", d);
+
+ if ( gfn != ~0ULL )
Is this an odd open-coding of INVALID_GFN? And even if not - why ULL when
"gfn" is unsigned long? The way you have it the condition will always be
false on Arm32, if I'm not mistaken.
I'll have to double check that. Thanks.
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -4346,6 +4346,21 @@ static void gnttab_usage_print(struct domain *rd)
printk("no active grant table entries\n");
}
+#ifdef CONFIG_DOM0LESS_BOOT
+void __init gnttab_seed_entry(const struct domain *d, unsigned int idx,
+ domid_t be_domid, uint64_t frame,
+ unsigned int flags)
+{
+ const struct grant_table *gt = d->grant_table;
+
+ ASSERT(!d->creation_finished);
While I don't mind the assertion, it's a little funny to see such in an
__init function.
This check was added in response to v1 comment. Yes, I too considered
just relying on __init, but the ASSERT explicitly checks the desired
property holds.
+ ASSERT(gt->gt_version == 1);
+ shared_entry_v1(gt, idx).flags = flags;
Does this really need to be a function parameter? The sole caller passes
a constant (GTF_permit_access).
I suppose it could be dropped. The toolstack side passes flags, which I
mirrored when writing this.
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -45,6 +45,11 @@ void grant_table_destroy(
struct domain *d);
void grant_table_init_vcpu(struct vcpu *v);
+/* Seed a gnttab entry for Hyperlaunch/dom0less. */
+void __init gnttab_seed_entry(const struct domain *d, unsigned int idx,
No __init on declarations please. They have no effect (as long as the definition
properly has the attribute) and hence are only at risk of going stale.
Oh, okay. I thought they were necessary, but clear I am wrong.
Thanks,
Jason
