Hi Roger,
On 13/05/2024 16:27, Roger Pau Monné wrote:
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index 2a445bb17b..1b025986f7 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -288,6 +288,21 @@ static void pv_destroy_gdt_ldt_l1tab(struct vcpu *v)
1U << GDT_LDT_VCPU_SHIFT);
}
+static int pv_create_root_pt_l1tab(struct vcpu *v)
+{
+ return create_perdomain_mapping(v->domain,
+ PV_ROOT_PT_MAPPING_VCPU_VIRT_START(v),
+ 1, v->domain->arch.pv.root_pt_l1tab,
+ NULL);
+}
+
+static void pv_destroy_root_pt_l1tab(struct vcpu *v)
The two 'v' parameters could be const here.
I could constify the parameters but the functions wouldn't be consistent
with the two above for gdt/ldt.
@@ -381,6 +412,11 @@ int pv_domain_initialise(struct domain *d)
if ( rc )
goto fail;
+ rc = create_perdomain_mapping(d, PV_ROOT_PT_MAPPING_VIRT_START,
+ PV_ROOT_PT_MAPPING_ENTRIES, NULL, NULL);
Why not use d->max_vcpus here, instead of forcing up to MAX_VIRT_CPUS?
By the time pv_domain_initialise() is called max_vcpus should already
be initialized. AFAICT it doesn't make a difference, because for the
call here only the L3 table is created, as last two parameters are
NULL, but still is more accurate to use max_vcpus.
There is no particular reason. I think we can safely use d->max_vcpus.
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index df015589ce..c1377da7a5 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -162,7 +162,15 @@ FUNC_LOCAL(restore_all_guest)
and %rsi, %rdi
and %r9, %rsi
add %rcx, %rdi
+
+ /*
+ * The address in the vCPU cr3 is always mapped in the per-domain
+ * pv_root_pt virt area.
+ */
+ imul $PAGE_SIZE, VCPU_id(%rbx), %esi
Aren't some of the previous operations against %rsi now useless since
it gets unconditionally overwritten here?
I think I can just get rid off of:
and %r9, %rsi
and %r9, %rsi
[...]
add %rcx, %rsi
The second operation you suggested is actually used to retrieve the VA
of the PV_ROOT_PT.
Cheers,
Elias
