| Message ID | 20250403214608.152954-5-jason.andryuk@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ARM split hardware and control domains | expand |
On Thu, 3 Apr 2025, 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. > > C xenstored uses grants, so it can map the xenstore pages from a > non-dom0 xenstore domain. OCaml xenstored uses foreign mappings, so it > can only run from a privileged domain (dom0). > > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > v3: > Expand commit message about C vs. OCaml xenstored. > Remove __init and flags from gnttab_seed_entry() > Change frame to uint32_t > ASSERT gfn fits in a uint32_t > Rebase on mem paging changes > > v2: > Tweak commit message > Mark gnttab_seed_entry() __init and put inside CONFIG_DOM0LESS_BOOT > Add ASSERT(!d->creation_finished) and ASSERT(gt->gt_version == 1); > const struct domain & struct grant_table > --- > xen/arch/arm/dom0less-build.c | 6 ++++++ > xen/common/grant_table.c | 14 ++++++++++++++ > xen/include/xen/grant_table.h | 7 +++++++ > 3 files changed, 27 insertions(+) > > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c > index bb8cc3be43..284190d54f 100644 > --- a/xen/arch/arm/dom0less-build.c > +++ b/xen/arch/arm/dom0less-build.c > @@ -788,6 +788,12 @@ 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 ) > + { > + ASSERT(gfn <= UINT_MAX); > + gnttab_seed_entry(d, GNTTAB_RESERVED_XENSTORE, xs_domid, gfn); > + } > } > } > > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c > index 6c77867f8c..e75ff98aff 100644 > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -4346,6 +4346,20 @@ 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, uint32_t frame) > +{ > + const struct grant_table *gt = d->grant_table; > + > + ASSERT(!d->creation_finished); > + ASSERT(gt->gt_version == 1); > + shared_entry_v1(gt, idx).flags = GTF_permit_access; > + shared_entry_v1(gt, idx).domid = be_domid; > + shared_entry_v1(gt, idx).frame = frame; > +} > +#endif > + > static void cf_check gnttab_usage_print_all(unsigned char key) > { > struct domain *d; > diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h > index 50edfecfb6..936a52ff10 100644 > --- a/xen/include/xen/grant_table.h > +++ b/xen/include/xen/grant_table.h > @@ -45,6 +45,10 @@ void grant_table_destroy( > struct domain *d); > void grant_table_init_vcpu(struct vcpu *v); > > +/* Seed a gnttab entry for Hyperlaunch/dom0less. */ > +void gnttab_seed_entry(const struct domain *d, unsigned int idx, > + domid_t be_domid, uint32_t frame); > + > /* > * Check if domain has active grants and log first 10 of them. > */ > @@ -85,6 +89,9 @@ static inline void grant_table_destroy(struct domain *d) {} > > static inline void grant_table_init_vcpu(struct vcpu *v) {} > > +static inline void gnttab_seed_entry(struct domain *d, int idx, > + domid_t be_domid, uint32_t frame) {} > + > static inline void grant_table_warn_active_grants(struct domain *d) {} > > static inline int gnttab_release_mappings(struct domain *d) { return 0; } > -- > 2.49.0 >
On 03.04.2025 23:46, 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. > > C xenstored uses grants, so it can map the xenstore pages from a > non-dom0 xenstore domain. OCaml xenstored uses foreign mappings, so it > can only run from a privileged domain (dom0). > > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> > --- > v3: > Expand commit message about C vs. OCaml xenstored. > Remove __init and flags from gnttab_seed_entry() > Change frame to uint32_t > ASSERT gfn fits in a uint32_t Ehem. For this you need to use ... > --- a/xen/arch/arm/dom0less-build.c > +++ b/xen/arch/arm/dom0less-build.c > @@ -788,6 +788,12 @@ 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 ) > + { > + ASSERT(gfn <= UINT_MAX); ... UINT32_MAX here. Furthermore may I remind you that INVALID_GFN == UINT32_MAX in 32-bit environments. The ~0ULL may also better be UINT64_MAX. > @@ -85,6 +89,9 @@ static inline void grant_table_destroy(struct domain *d) {} > > static inline void grant_table_init_vcpu(struct vcpu *v) {} > > +static inline void gnttab_seed_entry(struct domain *d, int idx, > + domid_t be_domid, uint32_t frame) {} Hmm. While generally I prefer using such wrappers, I wonder if in this case it wouldn't end up more clear if a conditional was added in initialize_domU_xenstore(). Ideally using IS_ENABLED(), which - aiui - would require moving the declaration of the function. Jan
On 2025-04-04 03:34, Jan Beulich wrote: > On 03.04.2025 23:46, 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. >> >> C xenstored uses grants, so it can map the xenstore pages from a >> non-dom0 xenstore domain. OCaml xenstored uses foreign mappings, so it >> can only run from a privileged domain (dom0). >> >> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> >> --- >> v3: >> Expand commit message about C vs. OCaml xenstored. >> Remove __init and flags from gnttab_seed_entry() >> Change frame to uint32_t >> ASSERT gfn fits in a uint32_t > > Ehem. For this you need to use ... > >> --- a/xen/arch/arm/dom0less-build.c >> +++ b/xen/arch/arm/dom0less-build.c >> @@ -788,6 +788,12 @@ 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 ) >> + { >> + ASSERT(gfn <= UINT_MAX); > > ... UINT32_MAX here. Furthermore may I remind you that INVALID_GFN == > UINT32_MAX in 32-bit environments. Yes, thanks. > The ~0ULL may also better be UINT64_MAX. I'll also add #define XENSTORE_PFN_LATE_ALLOC UINT64_MAX >> @@ -85,6 +89,9 @@ static inline void grant_table_destroy(struct domain *d) {} >> >> static inline void grant_table_init_vcpu(struct vcpu *v) {} >> >> +static inline void gnttab_seed_entry(struct domain *d, int idx, >> + domid_t be_domid, uint32_t frame) {} > > Hmm. While generally I prefer using such wrappers, I wonder if in this > case it wouldn't end up more clear if a conditional was added in > initialize_domU_xenstore(). Ideally using IS_ENABLED(), which - aiui - > would require moving the declaration of the function. Ok. Thanks, Jason
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c index bb8cc3be43..284190d54f 100644 --- a/xen/arch/arm/dom0less-build.c +++ b/xen/arch/arm/dom0less-build.c @@ -788,6 +788,12 @@ 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 ) + { + ASSERT(gfn <= UINT_MAX); + gnttab_seed_entry(d, GNTTAB_RESERVED_XENSTORE, xs_domid, gfn); + } } } diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index 6c77867f8c..e75ff98aff 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -4346,6 +4346,20 @@ 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, uint32_t frame) +{ + const struct grant_table *gt = d->grant_table; + + ASSERT(!d->creation_finished); + ASSERT(gt->gt_version == 1); + shared_entry_v1(gt, idx).flags = GTF_permit_access; + shared_entry_v1(gt, idx).domid = be_domid; + shared_entry_v1(gt, idx).frame = frame; +} +#endif + static void cf_check gnttab_usage_print_all(unsigned char key) { struct domain *d; diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h index 50edfecfb6..936a52ff10 100644 --- a/xen/include/xen/grant_table.h +++ b/xen/include/xen/grant_table.h @@ -45,6 +45,10 @@ void grant_table_destroy( struct domain *d); void grant_table_init_vcpu(struct vcpu *v); +/* Seed a gnttab entry for Hyperlaunch/dom0less. */ +void gnttab_seed_entry(const struct domain *d, unsigned int idx, + domid_t be_domid, uint32_t frame); + /* * Check if domain has active grants and log first 10 of them. */ @@ -85,6 +89,9 @@ static inline void grant_table_destroy(struct domain *d) {} static inline void grant_table_init_vcpu(struct vcpu *v) {} +static inline void gnttab_seed_entry(struct domain *d, int idx, + domid_t be_domid, uint32_t frame) {} + static inline void grant_table_warn_active_grants(struct domain *d) {} static inline int gnttab_release_mappings(struct domain *d) { return 0; }
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. C xenstored uses grants, so it can map the xenstore pages from a non-dom0 xenstore domain. OCaml xenstored uses foreign mappings, so it can only run from a privileged domain (dom0). Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> --- v3: Expand commit message about C vs. OCaml xenstored. Remove __init and flags from gnttab_seed_entry() Change frame to uint32_t ASSERT gfn fits in a uint32_t Rebase on mem paging changes v2: Tweak commit message Mark gnttab_seed_entry() __init and put inside CONFIG_DOM0LESS_BOOT Add ASSERT(!d->creation_finished) and ASSERT(gt->gt_version == 1); const struct domain & struct grant_table --- xen/arch/arm/dom0less-build.c | 6 ++++++ xen/common/grant_table.c | 14 ++++++++++++++ xen/include/xen/grant_table.h | 7 +++++++ 3 files changed, 27 insertions(+)