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

[Henry Wang <xin.wang2@xxxxxxx>]

Hi Julien,

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.
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, 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?
> 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. But you made a good point and I will 
remember to check if there are
multiple pages or an overlapped region. Thanks!

Kind regards,
Henry