On 23.12.2022 13:22, Julien Grall wrote: > 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).
Yes, I do understand that would be a more intrusive change. > 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? I guess I wouldn't nack it, but I also wouldn't provide an ack. I'm curious what Andrew or Roger think here... Jan
