Hi Jan,
On 04/01/2023 10:27, Jan Beulich wrote:
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.
I could be persuaded to look at a more intrusive change if there are a
good reason to do it. To me, at the moment, it mostly seem a matter of
taste.
So what would we gain from a perdomain mapping?
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...
Unfortunately Roger is on parental leaves for the next couple of months.
It would be good to make some progress before hand. Andrew, what do you
think?
--
Julien Grall
