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

[Henry Wang <xin.wang2@xxxxxxx>]

Hi Michal,

On 2/26/2024 6:29 PM, Michal Orzel wrote:
> Hi Henry,
>
> On 26/02/2024 02:19, Henry Wang wrote:
> > An error message can seen from the init-dom0less application on
> > direct-mapped 1:1 domains:
> > ```
> > Allocating magic pages
> > memory.c:238:d0v0 mfn 0x39000 doesn't belong to d1
> > Error on alloc magic pages
> > ```
> >
> > This is because populate_physmap() automatically assumes gfn == mfn
> > for direct mapped domains. This cannot be true for the magic pages
> > that are allocated later for Dom0less DomUs from the init-dom0less
> > helper application executed in Dom0.
> >
> > 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
> > Create helper is_magic_gpfn() for Arm to assist this and stub helpers
> > for non-Arm architectures to avoid #ifdef. Move the definition of the
> > magic pages on Arm to a more common place.
> >
> > Note that the init-dom0less application of the diffenent Xen version
> > may allocate all or part of four magic pages for each DomU.
> >
> > Reported-by: Alec Kwapis <alec.kwapis@xxxxxxxxxxxxx>
> > Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
> NIT: Generally, the first SOB is the same as author of the patch.
I will fix in the next version. Thanks.

> [...]
> > diff --git a/xen/common/memory.c b/xen/common/memory.c
> > index b3b05c2ec0..ab4bad79e2 100644
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -219,7 +219,7 @@ static void populate_physmap(struct memop_args *a)
> >           }
> >           else
> >           {
> > -            if ( is_domain_direct_mapped(d) )
> > +            if ( is_domain_direct_mapped(d) && !is_magic_gpfn(gpfn) )
> I struggle to understand the goal of this patch and the proposed solution.
> The problem with magic pages applies to static mem domUs in general.
> A direct mapped domU is a static mem domU whose memory is 1:1 mapped.
> Let's say we try to map a magic page for a direct mapped domU. That check will 
> be false
> and the execution will move to the next one i.e. is_domain_using_staticmem(d).
> This check will be true and acquire_reserved_page() will fail instead (similar 
> to the
> static mem (no direct map) scenario). The only thing that has changed is the 
> message.
Yes you are definitely correct, I missed the check for static mem 
domains. Will fix in v2 once the proposal in the other thread is agreed. 
Thanks for spotting this issue!
Kind regards,
Henry

> ~Michal