| Message ID | 20240511005611.83125-2-xin.wang2@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Guest magic region allocation for 11 Dom0less domUs - Take two | expand |
Hi Henry, On 11/05/2024 01:56, Henry Wang wrote: > 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. > > To solve above issue, this commit allocates hypervisor reserved pages > (currently used as the magic pages) for Arm Dom0less DomUs at the > domain construction time. The base address/PFN of the region will be > noted and communicated to the init-dom0less application in Dom0. > > 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> > --- > v2: > - Reword the commit msg to explain what is "magic page" and use generic > terminology "hypervisor reserved pages" in commit msg. (Daniel) > - Also move the offset definition of magic pages. (Michal) > - Extract the magic page allocation logic to a function. (Michal) > --- > tools/libs/guest/xg_dom_arm.c | 6 ------ > xen/arch/arm/dom0less-build.c | 32 ++++++++++++++++++++++++++++++++ > xen/include/public/arch-arm.h | 6 ++++++ > 3 files changed, 38 insertions(+), 6 deletions(-) > > diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c > index 2fd8ee7ad4..8c579d7576 100644 > --- a/tools/libs/guest/xg_dom_arm.c > +++ b/tools/libs/guest/xg_dom_arm.c > @@ -25,12 +25,6 @@ > > #include "xg_private.h" > > -#define NR_MAGIC_PAGES 4 > -#define CONSOLE_PFN_OFFSET 0 > -#define XENSTORE_PFN_OFFSET 1 > -#define MEMACCESS_PFN_OFFSET 2 > -#define VUART_PFN_OFFSET 3 > - > #define LPAE_SHIFT 9 > > #define PFN_4K_SHIFT (0) > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c > index 74f053c242..4b96ddd9ce 100644 > --- a/xen/arch/arm/dom0less-build.c > +++ b/xen/arch/arm/dom0less-build.c > @@ -739,6 +739,34 @@ static int __init alloc_xenstore_evtchn(struct domain *d) > return 0; > } > > +static int __init alloc_magic_pages(struct domain *d) > +{ > + struct page_info *magic_pg; > + mfn_t mfn; > + gfn_t gfn; > + int rc; > + > + d->max_pages += NR_MAGIC_PAGES; Here you bump d->max_mages by NR_MAGIC_PAGES but... > + magic_pg = alloc_domheap_pages(d, get_order_from_pages(NR_MAGIC_PAGES), 0); ... here you will allocate using a power-of-two. Which may end up to fail as there is nothing guaranteeing that NR_MAGIC_PAGES is suitably aligned. For now NR_MAGIC_PAGES seems suitably aligned, so it BUILD_BUG_ON() woudl be ok. > + if ( magic_pg == NULL ) > + return -ENOMEM; > + > + mfn = page_to_mfn(magic_pg); > + if ( !is_domain_direct_mapped(d) ) > + gfn = gaddr_to_gfn(GUEST_MAGIC_BASE); > + else > + gfn = gaddr_to_gfn(mfn_to_maddr(mfn)); Allocating the magic pages contiguously is only necessary for direct mapped domain. For the other it might be preferable to allocate page by page. That said, NR_MAGIC_PAGES is not big enough. So it would be okay. > + > + rc = guest_physmap_add_pages(d, gfn, mfn, NR_MAGIC_PAGES); > + if ( rc ) > + { > + free_domheap_pages(magic_pg, get_order_from_pages(NR_MAGIC_PAGES)); > + return rc; > + } > + > + return 0; > +} > + > static int __init construct_domU(struct domain *d, > const struct dt_device_node *node) > { > @@ -840,6 +868,10 @@ static int __init construct_domU(struct domain *d, > if ( rc < 0 ) > return rc; > d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL; > + > + rc = alloc_magic_pages(d); > + if ( rc < 0 ) > + return rc; This will only be allocated xenstore is enabled. But I don't think some of the magic pages really require xenstore to work. In the future we may need some more fine graine choice (see my comment in patch #2 as well). > } > > return rc; > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > index 289af81bd6..186520d01f 100644 > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -476,6 +476,12 @@ typedef uint64_t xen_callback_t; > #define GUEST_MAGIC_BASE xen_mk_ullong(0x39000000) > #define GUEST_MAGIC_SIZE xen_mk_ullong(0x01000000) > > +#define NR_MAGIC_PAGES 4 This name and the other below are far too generic to be part of the shared header. For NR_MAGIC_PAGES, can you explain why GUEST_MAGIC_SIZE cannot be used? Is it because it is "too" big? For the rest below... > +#define CONSOLE_PFN_OFFSET 0 > +#define XENSTORE_PFN_OFFSET 1 > +#define MEMACCESS_PFN_OFFSET 2 > +#define VUART_PFN_OFFSET 3 ... the order we allocate the magic pages is purely an internal decision of the toolstack. For the rest of the system, they need to access HVM_PARAM_*. So it doesn't feel like they should be part of the shared headers. If there is a strong reason to include them, then I think they all should be prefixed with GUEST_MAGIC_*. Cheers, > + > #define GUEST_RAM_BANKS 2 > > /*
Hi Julien, On 5/11/2024 4:46 PM, Julien Grall wrote: > Hi Henry, > > On 11/05/2024 01:56, Henry Wang wrote: >> 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. >> >> To solve above issue, this commit allocates hypervisor reserved pages >> (currently used as the magic pages) for Arm Dom0less DomUs at the >> domain construction time. The base address/PFN of the region will be >> noted and communicated to the init-dom0less application in Dom0. >> >> 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> >> --- >> v2: >> - Reword the commit msg to explain what is "magic page" and use generic >> terminology "hypervisor reserved pages" in commit msg. (Daniel) >> - Also move the offset definition of magic pages. (Michal) >> - Extract the magic page allocation logic to a function. (Michal) >> --- >> tools/libs/guest/xg_dom_arm.c | 6 ------ >> xen/arch/arm/dom0less-build.c | 32 ++++++++++++++++++++++++++++++++ >> xen/include/public/arch-arm.h | 6 ++++++ >> 3 files changed, 38 insertions(+), 6 deletions(-) >> >> diff --git a/tools/libs/guest/xg_dom_arm.c >> b/tools/libs/guest/xg_dom_arm.c >> index 2fd8ee7ad4..8c579d7576 100644 >> --- a/tools/libs/guest/xg_dom_arm.c >> +++ b/tools/libs/guest/xg_dom_arm.c >> @@ -25,12 +25,6 @@ >> #include "xg_private.h" >> -#define NR_MAGIC_PAGES 4 >> -#define CONSOLE_PFN_OFFSET 0 >> -#define XENSTORE_PFN_OFFSET 1 >> -#define MEMACCESS_PFN_OFFSET 2 >> -#define VUART_PFN_OFFSET 3 >> - >> #define LPAE_SHIFT 9 >> #define PFN_4K_SHIFT (0) >> diff --git a/xen/arch/arm/dom0less-build.c >> b/xen/arch/arm/dom0less-build.c >> index 74f053c242..4b96ddd9ce 100644 >> --- a/xen/arch/arm/dom0less-build.c >> +++ b/xen/arch/arm/dom0less-build.c >> @@ -739,6 +739,34 @@ static int __init alloc_xenstore_evtchn(struct >> domain *d) >> return 0; >> } >> +static int __init alloc_magic_pages(struct domain *d) >> +{ >> + struct page_info *magic_pg; >> + mfn_t mfn; >> + gfn_t gfn; >> + int rc; >> + >> + d->max_pages += NR_MAGIC_PAGES; > > Here you bump d->max_mages by NR_MAGIC_PAGES but... > >> + magic_pg = alloc_domheap_pages(d, >> get_order_from_pages(NR_MAGIC_PAGES), 0); > > ... here you will allocate using a power-of-two. Which may end up to > fail as there is nothing guaranteeing that NR_MAGIC_PAGES is suitably > aligned. > > For now NR_MAGIC_PAGES seems suitably aligned, so it BUILD_BUG_ON() > woudl be ok. Great catch! I will add BUILD_BUG_ON(NR_MAGIC_PAGES & (NR_MAGIC_PAGES - 1)); Thanks. >> + if ( magic_pg == NULL ) >> + return -ENOMEM; >> + >> + mfn = page_to_mfn(magic_pg); >> + if ( !is_domain_direct_mapped(d) ) >> + gfn = gaddr_to_gfn(GUEST_MAGIC_BASE); >> + else >> + gfn = gaddr_to_gfn(mfn_to_maddr(mfn)); > > Allocating the magic pages contiguously is only necessary for direct > mapped domain. For the other it might be preferable to allocate page > by page. That said, NR_MAGIC_PAGES is not big enough. So it would be > okay. > >> + >> + rc = guest_physmap_add_pages(d, gfn, mfn, NR_MAGIC_PAGES); >> + if ( rc ) >> + { >> + free_domheap_pages(magic_pg, >> get_order_from_pages(NR_MAGIC_PAGES)); >> + return rc; >> + } >> + >> + return 0; >> +} >> + >> static int __init construct_domU(struct domain *d, >> const struct dt_device_node *node) >> { >> @@ -840,6 +868,10 @@ static int __init construct_domU(struct domain *d, >> if ( rc < 0 ) >> return rc; >> d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL; >> + >> + rc = alloc_magic_pages(d); >> + if ( rc < 0 ) >> + return rc; > > This will only be allocated xenstore is enabled. But I don't think > some of the magic pages really require xenstore to work. In the future > we may need some more fine graine choice (see my comment in patch #2 > as well). Sorry, but it seems that by the time that I am writing this reply, I didn't get the email for patch #2 comment. I will reply both together when I see it. >> } >> return rc; >> diff --git a/xen/include/public/arch-arm.h >> b/xen/include/public/arch-arm.h >> index 289af81bd6..186520d01f 100644 >> --- a/xen/include/public/arch-arm.h >> +++ b/xen/include/public/arch-arm.h >> @@ -476,6 +476,12 @@ typedef uint64_t xen_callback_t; >> #define GUEST_MAGIC_BASE xen_mk_ullong(0x39000000) >> #define GUEST_MAGIC_SIZE xen_mk_ullong(0x01000000) >> +#define NR_MAGIC_PAGES 4 > This name and the other below are far too generic to be part of the > shared header. For NR_MAGIC_PAGES, can you explain why > GUEST_MAGIC_SIZE cannot be used? Is it because it is "too" big? Yes, I don't really want to allocate whole 16MB when we merely need 4 pages. > For the rest below... > >> +#define CONSOLE_PFN_OFFSET 0 >> +#define XENSTORE_PFN_OFFSET 1 >> +#define MEMACCESS_PFN_OFFSET 2 >> +#define VUART_PFN_OFFSET 3 > > ... the order we allocate the magic pages is purely an internal > decision of the toolstack. For the rest of the system, they need to > access HVM_PARAM_*. So it doesn't feel like they should be part of the > shared headers. > > If there is a strong reason to include them, then I think they all > should be prefixed with GUEST_MAGIC_*. One of the benefits as Michal pointed out in comments for v1 [1] would be this would also allow init-dom0less.h not to re-define XENSTORE_PFN_OFFSET. I will rename them as suggested if both of you are ok with moving them to the arch header. [1] https://lore.kernel.org/xen-devel/5d3ead96-5079-4fa3-b5fd-4d9803c251b6@amd.com/ Kind regards, Henry
Hi Henry, On 11/05/2024 10:02, Henry Wang wrote: >>> + >>> + rc = guest_physmap_add_pages(d, gfn, mfn, NR_MAGIC_PAGES); >>> + if ( rc ) >>> + { >>> + free_domheap_pages(magic_pg, >>> get_order_from_pages(NR_MAGIC_PAGES)); >>> + return rc; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static int __init construct_domU(struct domain *d, >>> const struct dt_device_node *node) >>> { >>> @@ -840,6 +868,10 @@ static int __init construct_domU(struct domain *d, >>> if ( rc < 0 ) >>> return rc; >>> d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL; >>> + >>> + rc = alloc_magic_pages(d); >>> + if ( rc < 0 ) >>> + return rc; >> >> This will only be allocated xenstore is enabled. But I don't think >> some of the magic pages really require xenstore to work. In the future >> we may need some more fine graine choice (see my comment in patch #2 >> as well). > > Sorry, but it seems that by the time that I am writing this reply, I > didn't get the email for patch #2 comment. I will reply both together > when I see it. That was expected, I knew what I wanted to mention about patch #2 but the e-mail was not fully written. You should have it in your inbox now. > >>> } >>> return rc; >>> diff --git a/xen/include/public/arch-arm.h >>> b/xen/include/public/arch-arm.h >>> index 289af81bd6..186520d01f 100644 >>> --- a/xen/include/public/arch-arm.h >>> +++ b/xen/include/public/arch-arm.h >>> @@ -476,6 +476,12 @@ typedef uint64_t xen_callback_t; >>> #define GUEST_MAGIC_BASE xen_mk_ullong(0x39000000) >>> #define GUEST_MAGIC_SIZE xen_mk_ullong(0x01000000) >>> +#define NR_MAGIC_PAGES 4 >> This name and the other below are far too generic to be part of the >> shared header. For NR_MAGIC_PAGES, can you explain why >> GUEST_MAGIC_SIZE cannot be used? Is it because it is "too" big? > > Yes, I don't really want to allocate whole 16MB when we merely need 4 > pages. What about updating GUEST_MAGIC_SIZE to 0x4000? It doesn't make any sense to have two define which have pretty much the same meaning. Then on the toolstack side, you can check that NR_MAGIC_PAGES is smaller or equal to GUEST_MAGIC_SIZE. > >> For the rest below... >> >>> +#define CONSOLE_PFN_OFFSET 0 >>> +#define XENSTORE_PFN_OFFSET 1 >>> +#define MEMACCESS_PFN_OFFSET 2 >>> +#define VUART_PFN_OFFSET 3 >> >> ... the order we allocate the magic pages is purely an internal >> decision of the toolstack. For the rest of the system, they need to >> access HVM_PARAM_*. So it doesn't feel like they should be part of the >> shared headers. >> >> If there is a strong reason to include them, then I think they all >> should be prefixed with GUEST_MAGIC_*. > > One of the benefits as Michal pointed out in comments for v1 [1] would > be this would also allow init-dom0less.h not to re-define > XENSTORE_PFN_OFFSET. At the moment, yes they have the same values. But with series, XENSTORE_PFN_OFFSET should technically be 0 in init-dom0less. This is because Xen may only allocate one page (you don't need 4) for the reserved area. So... > I will rename them as suggested if both of you are > ok with moving them to the arch header. ... I don't think they should be shared. Cheers,
Hi Henry, On 11/05/2024 01:56, Henry Wang wrote: > 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. > > To solve above issue, this commit allocates hypervisor reserved pages > (currently used as the magic pages) for Arm Dom0less DomUs at the > domain construction time. The base address/PFN of the region will be > noted and communicated to the init-dom0less application in Dom0. > > 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> > --- > v2: > - Reword the commit msg to explain what is "magic page" and use generic > terminology "hypervisor reserved pages" in commit msg. (Daniel) > - Also move the offset definition of magic pages. (Michal) > - Extract the magic page allocation logic to a function. (Michal) > --- > tools/libs/guest/xg_dom_arm.c | 6 ------ > xen/arch/arm/dom0less-build.c | 32 ++++++++++++++++++++++++++++++++ > xen/include/public/arch-arm.h | 6 ++++++ > 3 files changed, 38 insertions(+), 6 deletions(-) > > diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c > index 2fd8ee7ad4..8c579d7576 100644 > --- a/tools/libs/guest/xg_dom_arm.c > +++ b/tools/libs/guest/xg_dom_arm.c > @@ -25,12 +25,6 @@ > > #include "xg_private.h" > > -#define NR_MAGIC_PAGES 4 > -#define CONSOLE_PFN_OFFSET 0 > -#define XENSTORE_PFN_OFFSET 1 > -#define MEMACCESS_PFN_OFFSET 2 > -#define VUART_PFN_OFFSET 3 > - > #define LPAE_SHIFT 9 > > #define PFN_4K_SHIFT (0) > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c > index 74f053c242..4b96ddd9ce 100644 > --- a/xen/arch/arm/dom0less-build.c > +++ b/xen/arch/arm/dom0less-build.c > @@ -739,6 +739,34 @@ static int __init alloc_xenstore_evtchn(struct domain *d) > return 0; > } > > +static int __init alloc_magic_pages(struct domain *d) > +{ > + struct page_info *magic_pg; > + mfn_t mfn; > + gfn_t gfn; > + int rc; > + > + d->max_pages += NR_MAGIC_PAGES; > + magic_pg = alloc_domheap_pages(d, get_order_from_pages(NR_MAGIC_PAGES), 0); > + if ( magic_pg == NULL ) > + return -ENOMEM; > + > + mfn = page_to_mfn(magic_pg); > + if ( !is_domain_direct_mapped(d) ) > + gfn = gaddr_to_gfn(GUEST_MAGIC_BASE); > + else > + gfn = gaddr_to_gfn(mfn_to_maddr(mfn)); Summarizing the discussion we had on Matrix. Regions like the extend area and shared memory may not be direct mapped. So unfortunately, I think it is possible that the GFN could clash with one of those. At least in the shared memory case, the user can provide the address. But as you use the domheap allocator, the address returned could easily change if you tweak your setup. I am not entirely sure what's the best solution. We could ask the user to provide the information for reserved region. But it feels like we are exposing a bit too much to the user. So possibly we would want to use the same approach as extended regions. Once we processed all the mappings, find some space for the hypervisor regions. Any other suggestions? Cheers,
Hi Julien, On 5/11/2024 7:03 PM, Julien Grall wrote: > Hi Henry, > > On 11/05/2024 01:56, Henry Wang wrote: >> +static int __init alloc_magic_pages(struct domain *d) >> +{ >> + struct page_info *magic_pg; >> + mfn_t mfn; >> + gfn_t gfn; >> + int rc; >> + >> + d->max_pages += NR_MAGIC_PAGES; >> + magic_pg = alloc_domheap_pages(d, >> get_order_from_pages(NR_MAGIC_PAGES), 0); >> + if ( magic_pg == NULL ) >> + return -ENOMEM; >> + >> + mfn = page_to_mfn(magic_pg); >> + if ( !is_domain_direct_mapped(d) ) >> + gfn = gaddr_to_gfn(GUEST_MAGIC_BASE); >> + else >> + gfn = gaddr_to_gfn(mfn_to_maddr(mfn)); > > Summarizing the discussion we had on Matrix. Regions like the extend > area and shared memory may not be direct mapped. So unfortunately, I > think it is possible that the GFN could clash with one of those. > > At least in the shared memory case, the user can provide the address. > But as you use the domheap allocator, the address returned could > easily change if you tweak your setup. > > I am not entirely sure what's the best solution. We could ask the user > to provide the information for reserved region. But it feels like we > are exposing a bit too much to the user. > > So possibly we would want to use the same approach as extended > regions. Once we processed all the mappings, find some space for the > hypervisor regions. One thing that I noticed when I re-visit the extended region finding code from the hypervisor side is: When the domain is direct-mapped, when we find extended region for the domain, we either use find_unallocated_memory() or find_memory_holes(). It looks like the removal of shared memory regions in both functions uses the paddr parsed from the device tree to remove the regions, which indicates there is an assumption that when a domain is direct-mapped, the shared memory should also be direct-mapped. I might be wrong, but otherwise I don't think the extended region finding logic will carve out the correct shared memory region gpaddr range for guests. So I think we are missing the documentation (and the corresponding checking when we parse the device tree) for above assumption for the static shared memory, i.e., when the domain is direct-mapped, the static shared memory should also be direct-mapped, and user should make sure this is satisfied in the device tree otherwise Xen should complain. If we add this assumption and related checking code, I think your concern of clashing with static shared memory can be addressed. Do you agree? Kind regards, Henry > > Any other suggestions? > > Cheers, >
diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c index 2fd8ee7ad4..8c579d7576 100644 --- a/tools/libs/guest/xg_dom_arm.c +++ b/tools/libs/guest/xg_dom_arm.c @@ -25,12 +25,6 @@ #include "xg_private.h" -#define NR_MAGIC_PAGES 4 -#define CONSOLE_PFN_OFFSET 0 -#define XENSTORE_PFN_OFFSET 1 -#define MEMACCESS_PFN_OFFSET 2 -#define VUART_PFN_OFFSET 3 - #define LPAE_SHIFT 9 #define PFN_4K_SHIFT (0) diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c index 74f053c242..4b96ddd9ce 100644 --- a/xen/arch/arm/dom0less-build.c +++ b/xen/arch/arm/dom0less-build.c @@ -739,6 +739,34 @@ static int __init alloc_xenstore_evtchn(struct domain *d) return 0; } +static int __init alloc_magic_pages(struct domain *d) +{ + struct page_info *magic_pg; + mfn_t mfn; + gfn_t gfn; + int rc; + + d->max_pages += NR_MAGIC_PAGES; + magic_pg = alloc_domheap_pages(d, get_order_from_pages(NR_MAGIC_PAGES), 0); + if ( magic_pg == NULL ) + return -ENOMEM; + + mfn = page_to_mfn(magic_pg); + if ( !is_domain_direct_mapped(d) ) + gfn = gaddr_to_gfn(GUEST_MAGIC_BASE); + else + gfn = gaddr_to_gfn(mfn_to_maddr(mfn)); + + rc = guest_physmap_add_pages(d, gfn, mfn, NR_MAGIC_PAGES); + if ( rc ) + { + free_domheap_pages(magic_pg, get_order_from_pages(NR_MAGIC_PAGES)); + return rc; + } + + return 0; +} + static int __init construct_domU(struct domain *d, const struct dt_device_node *node) { @@ -840,6 +868,10 @@ static int __init construct_domU(struct domain *d, if ( rc < 0 ) return rc; d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL; + + rc = alloc_magic_pages(d); + if ( rc < 0 ) + return rc; } return rc; diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h index 289af81bd6..186520d01f 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -476,6 +476,12 @@ typedef uint64_t xen_callback_t; #define GUEST_MAGIC_BASE xen_mk_ullong(0x39000000) #define GUEST_MAGIC_SIZE xen_mk_ullong(0x01000000) +#define NR_MAGIC_PAGES 4 +#define CONSOLE_PFN_OFFSET 0 +#define XENSTORE_PFN_OFFSET 1 +#define MEMACCESS_PFN_OFFSET 2 +#define VUART_PFN_OFFSET 3 + #define GUEST_RAM_BANKS 2 /*
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. To solve above issue, this commit allocates hypervisor reserved pages (currently used as the magic pages) for Arm Dom0less DomUs at the domain construction time. The base address/PFN of the region will be noted and communicated to the init-dom0less application in Dom0. 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> --- v2: - Reword the commit msg to explain what is "magic page" and use generic terminology "hypervisor reserved pages" in commit msg. (Daniel) - Also move the offset definition of magic pages. (Michal) - Extract the magic page allocation logic to a function. (Michal) --- tools/libs/guest/xg_dom_arm.c | 6 ------ xen/arch/arm/dom0less-build.c | 32 ++++++++++++++++++++++++++++++++ xen/include/public/arch-arm.h | 6 ++++++ 3 files changed, 38 insertions(+), 6 deletions(-)