Message ID | 20250206010843.618280-3-stefano.stabellini@amd.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Guest XenStore page allocation for 11 Dom0less domUs | expand |
On 06/02/2025 02:08, Stefano Stabellini wrote: > From: Henry Wang <xin.wang2@amd.com> > > There are use cases (for example using the PV driver) in Dom0less > setup that require Dom0less DomUs start immediately with Dom0, but > initialize XenStore later after Dom0's successful boot and call to > the init-dom0less application. > > An error message can seen from the init-dom0less application on > 1:1 direct-mapped domains: > ``` > Allocating magic pages > memory.c:238:d0v0 mfn 0x39000 doesn't belong to d1 > Error on alloc magic pages > ``` > > The "magic page" is a terminology used in the toolstack as reserved > pages for the VM to have access to virtual platform capabilities. > Currently the magic pages for Dom0less DomUs are populated by the > init-dom0less app through populate_physmap(), and populate_physmap() > automatically assumes gfn == mfn for 1:1 direct mapped domains. This > cannot be true for the magic pages that are allocated later from the > init-dom0less application executed in Dom0. For domain using statically > allocated memory but not 1:1 direct-mapped, similar error "failed to > retrieve a reserved page" can be seen as the reserved memory list is > empty at that time. > > Since for init-dom0less, the magic page region is only for XenStore. > To solve above issue, this commit allocates the XenStore page for > Dom0less DomUs at the domain construction time. The PFN will be > noted and communicated to the init-dom0less application executed > from Dom0. To keep the XenStore late init protocol, set the connection > status to XENSTORE_RECONNECT. > > Reported-by: Alec Kwapis <alec.kwapis@medtronic.com> > Suggested-by: Daniel P. Smith <dpsmith@apertussolutions.com> > Signed-off-by: Henry Wang <xin.wang2@amd.com> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> > --- > xen/arch/arm/dom0less-build.c | 55 ++++++++++++++++++++++++++++++++++- > 1 file changed, 54 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c > index 49d1f14d65..046439eb87 100644 > --- a/xen/arch/arm/dom0less-build.c > +++ b/xen/arch/arm/dom0less-build.c > @@ -1,5 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0-only */ > #include <xen/device_tree.h> > +#include <xen/domain_page.h> > #include <xen/err.h> > #include <xen/event.h> > #include <xen/grant_table.h> > @@ -11,6 +12,8 @@ > #include <xen/sizes.h> > #include <xen/vmap.h> > > +#include <public/io/xs_wire.h> > + > #include <asm/arm64/sve.h> > #include <asm/dom0less-build.h> > #include <asm/domain_build.h> > @@ -704,6 +707,53 @@ static int __init alloc_xenstore_evtchn(struct domain *d) > return 0; > } > > +#define XENSTORE_PFN_OFFSET 1 > +static int __init alloc_xenstore_page(struct domain *d) > +{ > + struct page_info *xenstore_pg; > + struct xenstore_domain_interface *interface; > + mfn_t mfn; > + gfn_t gfn; > + int rc; > + > + if ( (UINT_MAX - d->max_pages) < 1 ) > + { > + printk(XENLOG_ERR "%pd: Over-allocation for d->max_pages by 1 page.\n", > + d); > + return -EINVAL; > + } empty line here > + d->max_pages += 1; If this patch is separate from modifying init-dom0less, max_pages will be bumped twice. Here and in init-dom0less. Shouldn't we fold it in? The rest is ok. ~Michal
On Thu, 6 Feb 2025, Orzel, Michal wrote: > On 06/02/2025 02:08, Stefano Stabellini wrote: > > From: Henry Wang <xin.wang2@amd.com> > > > > There are use cases (for example using the PV driver) in Dom0less > > setup that require Dom0less DomUs start immediately with Dom0, but > > initialize XenStore later after Dom0's successful boot and call to > > the init-dom0less application. > > > > An error message can seen from the init-dom0less application on > > 1:1 direct-mapped domains: > > ``` > > Allocating magic pages > > memory.c:238:d0v0 mfn 0x39000 doesn't belong to d1 > > Error on alloc magic pages > > ``` > > > > The "magic page" is a terminology used in the toolstack as reserved > > pages for the VM to have access to virtual platform capabilities. > > Currently the magic pages for Dom0less DomUs are populated by the > > init-dom0less app through populate_physmap(), and populate_physmap() > > automatically assumes gfn == mfn for 1:1 direct mapped domains. This > > cannot be true for the magic pages that are allocated later from the > > init-dom0less application executed in Dom0. For domain using statically > > allocated memory but not 1:1 direct-mapped, similar error "failed to > > retrieve a reserved page" can be seen as the reserved memory list is > > empty at that time. > > > > Since for init-dom0less, the magic page region is only for XenStore. > > To solve above issue, this commit allocates the XenStore page for > > Dom0less DomUs at the domain construction time. The PFN will be > > noted and communicated to the init-dom0less application executed > > from Dom0. To keep the XenStore late init protocol, set the connection > > status to XENSTORE_RECONNECT. > > > > Reported-by: Alec Kwapis <alec.kwapis@medtronic.com> > > Suggested-by: Daniel P. Smith <dpsmith@apertussolutions.com> > > Signed-off-by: Henry Wang <xin.wang2@amd.com> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> > > --- > > xen/arch/arm/dom0less-build.c | 55 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 54 insertions(+), 1 deletion(-) > > > > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c > > index 49d1f14d65..046439eb87 100644 > > --- a/xen/arch/arm/dom0less-build.c > > +++ b/xen/arch/arm/dom0less-build.c > > @@ -1,5 +1,6 @@ > > /* SPDX-License-Identifier: GPL-2.0-only */ > > #include <xen/device_tree.h> > > +#include <xen/domain_page.h> > > #include <xen/err.h> > > #include <xen/event.h> > > #include <xen/grant_table.h> > > @@ -11,6 +12,8 @@ > > #include <xen/sizes.h> > > #include <xen/vmap.h> > > > > +#include <public/io/xs_wire.h> > > + > > #include <asm/arm64/sve.h> > > #include <asm/dom0less-build.h> > > #include <asm/domain_build.h> > > @@ -704,6 +707,53 @@ static int __init alloc_xenstore_evtchn(struct domain *d) > > return 0; > > } > > > > +#define XENSTORE_PFN_OFFSET 1 > > +static int __init alloc_xenstore_page(struct domain *d) > > +{ > > + struct page_info *xenstore_pg; > > + struct xenstore_domain_interface *interface; > > + mfn_t mfn; > > + gfn_t gfn; > > + int rc; > > + > > + if ( (UINT_MAX - d->max_pages) < 1 ) > > + { > > + printk(XENLOG_ERR "%pd: Over-allocation for d->max_pages by 1 page.\n", > > + d); > > + return -EINVAL; > > + } > empty line here Sure > > + d->max_pages += 1; > If this patch is separate from modifying init-dom0less, max_pages will be bumped twice. Here and in init-dom0less. > Shouldn't we fold it in? The rest is ok. OK
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c index 49d1f14d65..046439eb87 100644 --- a/xen/arch/arm/dom0less-build.c +++ b/xen/arch/arm/dom0less-build.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <xen/device_tree.h> +#include <xen/domain_page.h> #include <xen/err.h> #include <xen/event.h> #include <xen/grant_table.h> @@ -11,6 +12,8 @@ #include <xen/sizes.h> #include <xen/vmap.h> +#include <public/io/xs_wire.h> + #include <asm/arm64/sve.h> #include <asm/dom0less-build.h> #include <asm/domain_build.h> @@ -704,6 +707,53 @@ static int __init alloc_xenstore_evtchn(struct domain *d) return 0; } +#define XENSTORE_PFN_OFFSET 1 +static int __init alloc_xenstore_page(struct domain *d) +{ + struct page_info *xenstore_pg; + struct xenstore_domain_interface *interface; + mfn_t mfn; + gfn_t gfn; + int rc; + + if ( (UINT_MAX - d->max_pages) < 1 ) + { + printk(XENLOG_ERR "%pd: Over-allocation for d->max_pages by 1 page.\n", + d); + return -EINVAL; + } + d->max_pages += 1; + xenstore_pg = alloc_domheap_page(d, MEMF_bits(32)); + if ( xenstore_pg == NULL && is_64bit_domain(d) ) + xenstore_pg = alloc_domheap_page(d, 0); + if ( xenstore_pg == NULL ) + return -ENOMEM; + + mfn = page_to_mfn(xenstore_pg); + if ( !mfn_x(mfn) ) + return -ENOMEM; + + if ( !is_domain_direct_mapped(d) ) + gfn = gaddr_to_gfn(GUEST_MAGIC_BASE + + (XENSTORE_PFN_OFFSET << PAGE_SHIFT)); + else + gfn = gaddr_to_gfn(mfn_to_maddr(mfn)); + + rc = guest_physmap_add_page(d, gfn, mfn, 0); + if ( rc ) + { + free_domheap_page(xenstore_pg); + return rc; + } + + d->arch.hvm.params[HVM_PARAM_STORE_PFN] = gfn_x(gfn); + interface = map_domain_page(mfn); + interface->connection = XENSTORE_RECONNECT; + unmap_domain_page(interface); + + return 0; +} + static int __init construct_domU(struct domain *d, const struct dt_device_node *node) { @@ -804,7 +854,10 @@ static int __init construct_domU(struct domain *d, rc = alloc_xenstore_evtchn(d); if ( rc < 0 ) return rc; - d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL; + + rc = alloc_xenstore_page(d); + if ( rc < 0 ) + return rc; } return rc;