Re: [PATCH] xen/common: Do not allocate magic pages 1:1 for direct mapped domains

[Julien Grall <julien@xxxxxxx>]

Hi Henry,

On 28/02/2024 11:53, Henry Wang wrote:
> On 2/28/2024 6:35 PM, Julien Grall wrote:
> > Hi Henry,
> > > > > Force populate_physmap to take the "normal" memory allocation route 
> > > > > for
> > > > > the magic pages even for 1:1 Dom0less DomUs. This should work as long
> > > > > as the 1:1 Dom0less DomU doesn't have anything else mapped at the same
> > > > > guest address as the magic pages:
> > > > > - gfn 0x39000 address 0x39000000
> > > > > - gfn 0x39001 address 0x39001000
> > > > > - gfn 0x39002 address 0x39002000
> > > > > - gfn 0x39003 address 0x39003000
> > > > This is very fragile. You are making the assumption that the magic 
> > > > pages are not clashing with any RAM region. The layout defined in 
> > > > arch-arm.h has been designed for guest where Xen is in full control 
> > > > of the layout. This is not the case for directmapped domain. I don't 
> > > > think it is correct to try to re-use part of the layout.
> > > Apologies for the (a bit) late reply, it took a bit longer for me to 
> > > understand the story about directmap stuff, and yes, now I agree with 
> > > you, for those directmapped domains we should not reuse the guest 
> > > layout directly.
> > > > If you want to use 1:1 dom0less with xenstore & co, then you should 
> > > > find a different place in memory for the magic pages (TDB how to 
> > > > find that area). 
> > > Yes, and maybe we can use similar strategy in 
> > > find_unallocated_memory() or find_domU_holes() to do that.
> > > > You will still have the problem of the 1:1 allocation, but I think 
> > > > this could be solved bty adding a flag to force a non-1:1 allocation.
> > > After checking the code flow, below rough plan came to my mind, I 
> > > think what we need to do is:
> > > (1) Find a range of un-used memory using similar method in 
> > > find_unallocated_memory()/find_domU_holes()
> > AFAIK, the toolstack doesn't have any knowledge of the memeory layout 
> > for dom0less domUs today. We would need to expose it first.
> If I understand correctly, I think the issue you mentioned here and ...
>
> > Then the region could either be statically allocated (i.e. the admin 
> > provides it in the DTB) or dynamically.
> > > (2) Change the base address, i.e. GUEST_MAGIC_BASE in alloc_xs_page() 
> > > in init-dom0less.c to point to the address in (1) if static mem or 11 
> > > directmap. (I think this is a bit tricky though, do you have any 
> > > method that in your mind?)
> > AFAIK, the toolstack doesn't know whether a domain is direct mapped or 
> > using static mem.
> ...here basically means we want to do the finding of the unused region 
> in toolstack. Since currently what we care about is only a couple of 
> pages instead of the whole memory map, could it be possible that we do 
> the opposite: in alloc_xs_page(), we issue a domctl hypercall to Xen and 
> do the finding work in Xen and return with the found gfn? Then the page 
> can be mapped by populate_physmap() from alloc_xs_page() and used for 
> XenStore.
I know that DOMCTL hypercalls are not stable. But I am not overly happy 
with creating an hypercall which is just "fetch the magic regions". I 
think it need to be a more general one that would expose all the regions.
Also, you can't really find the magic regions when the hypercall is done 
as we don't have the guest memory layout. This will need to be done in 
advance.
Overall, I think it would be better if we provide an hypercall listing 
the regions currently occupied (similar to e820). One will have the type 
"magic pages".
> If above approach makes sense to you, I have a further question: Since I 
> understand that the extended region is basically for safely foreign 
> mapping pages
This is not about safety. The extended region is optional. It was 
introduced so it is easy for Linux to find an unallocated region to map 
external pages (e.g. vCPU shared info) so it doesn't waste RAM pages.
, and init_dom0less.c uses foreign memory map for this
> XenStore page, 
> should we find the wanted page in the extended region? or 
> even extended region should be excluded?
How is the extended region found for dom0less domUs today? It would be 
fine to steal some part of the extended regions for the magic pages. But 
they would need to be reserved *before* the guest is first unpaused.
> > I think this ties with what I just wrote above. For dom0less domUs, we 
> > probably want Xen to prepare a memory layout (similar to the e820) 
> > that will then be retrieved by the toolstack.
> > > (3) Use a flag or combination of existing flags (CDF_staticmem + 
> > > CDF_directmap) in populate_physmap() to force the allocation of these 
> > > magic pages using alloc_domheap_pages() - i.e. the "else" condition 
> > > in the bottom
> > If I am not mistaken, CDF_* are per-domain. So we would want to use 
> > MEMF_*.
> Ah yes you are correct, I indeed missed MEMF_*
>
> > > > Also, why are you only checking the first GFN? What if the caller 
> > > > pass an overlapped region?
> > > I am a bit confused. My understanding is at this point we are 
> > > handling one page at a time.
> > We are handling one "extent" at the time. This could be one or 
> > multiple pages (see extent_order).
> I agree, sorry I didn't express myself well. For this specific XenStore 
> page, I think the extent_order is
> fixed as 0 so there is only 1 page.
Correct. But you should not rely on this :).

Cheers,

--
Julien Grall