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

[Julien Grall <julien@xxxxxxx>]

On 01/03/2024 03:03, Henry Wang wrote:
> Hi Julien,
Hi Henry,

> On 2/28/2024 8:27 PM, Julien Grall wrote:
> > Hi Henry,
> > > ...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".
> Would below design make sense to you:
This looks good in principle. Some comments below.

> (1) Similarly as the e820, we can firstly introduce some data structures 
> in struct arch_domain:
> #define MEM_REGIONS_MAX 4
>
> // For now we only care the magic regions, but in the future this can be 
> easily extended with other types such as RAM, MMIO etc.
> enum mem_region_type {
>      MEM_REGION_MAGIC,
> };
>
> struct mem_region {
>      paddr_t start;
>      paddr_t size;
>      enum mem_region_type type;
> };
>
> struct domain_mem_map {
>      unsigned int nr_mem_regions;
>      struct mem_region region[MEM_REGIONS_MAX];
> };
If you plan to expose the same interface externally, then you will need 
to replace paddr_t with uint64_t and avoid using an enum in the structure.
You will also want to expose a dynamic array (even if this is fixed in 
the hypervisor).
> struct arch_domain {
> ...
>      struct domain_mem_map mem_map;
> };
>
> (2) Then in domain creation time, for normal and static non-directmapped 
> Dom0less DomU, set d->arch.mem_map.region[0].start = GUEST_MAGIC_BASE 
> and the size to 4 pages. For static and directmapped Dom0less DomU, find 
> a free region of 4 pages for magic pages. The finding of a free region 
> can reuse the approach for extended region and be called before 
> make_hypervisor_node(), and when make_hypervisor_node() is called. We 
> carve the magic pages out from the actual extended region.
> (3) Add a new hypercall XENMEM_memory_map_domid (or a domctl). Input 
> will be domid and output will be the memory map of the domain recorded 
> in struct arch_domain.
XENMEM_* are supposed to be stable interface. I am not aware of any use 
in the guest yet, so I think it would be best to use a DOMCTL.
> (4) In alloc_xs_page() and alloc_magic_pages, replace the hardcoded base 
> address with the new address found by the hypercall.
Cheers,

--
Julien Grall