Re: [PATCH 06/22] x86: map/unmap pages in restore_all_guests

[Julien Grall <julien@xxxxxxx>]

Hi,

On 22/12/2022 11:12, Jan Beulich wrote:
> On 16.12.2022 12:48, Julien Grall wrote:
> > --- a/xen/arch/x86/x86_64/entry.S
> > +++ b/xen/arch/x86/x86_64/entry.S
> > @@ -165,7 +165,24 @@ restore_all_guest:
> >           and   %rsi, %rdi
> >           and   %r9, %rsi
> >           add   %rcx, %rdi
> > -        add   %rcx, %rsi
> > +
> > +         /*
> > +          * Without a direct map, we have to map first before copying. We only
> > +          * need to map the guest root table but not the per-CPU root_pgt,
> > +          * because the latter is still a xenheap page.
> > +          */
> > +        pushq %r9
> > +        pushq %rdx
> > +        pushq %rax
> > +        pushq %rdi
> > +        mov   %rsi, %rdi
> > +        shr   $PAGE_SHIFT, %rdi
> > +        callq map_domain_page
> > +        mov   %rax, %rsi
> > +        popq  %rdi
> > +        /* Stash the pointer for unmapping later. */
> > +        pushq %rax
> > +
> >           mov   $ROOT_PAGETABLE_FIRST_XEN_SLOT, %ecx
> >           mov   root_table_offset(SH_LINEAR_PT_VIRT_START)*8(%rsi), %r8
> >           mov   %r8, root_table_offset(SH_LINEAR_PT_VIRT_START)*8(%rdi)
> > @@ -177,6 +194,14 @@ restore_all_guest:
> >           sub   $(ROOT_PAGETABLE_FIRST_XEN_SLOT - \
> >                   ROOT_PAGETABLE_LAST_XEN_SLOT - 1) * 8, %rdi
> >           rep movsq
> > +
> > +        /* Unmap the page. */
> > +        popq  %rdi
> > +        callq unmap_domain_page
> > +        popq  %rax
> > +        popq  %rdx
> > +        popq  %r9
> While the PUSH/POP are part of what I dislike here, I think this wants
> doing differently: Establish a mapping when putting in place a new guest
> page table, and use the pointer here. This could be a new per-domain
> mapping, to limit its visibility.
I have looked at a per-domain approach and this looks way more complex 
than the few concise lines here (not mentioning the extra amount of 
memory).
So I am not convinced this is worth the effort here.

I don't have an other approach in mind. So are you disliking this 
approach to the point this will be nacked?
Cheers,

--
Julien Grall