[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v2 1/4] x86/domctl: Stop using XLAT_cpu_user_regs()



In order to support FRED, we're going to have to remove the {ds..gs} fields
from struct cpu_user_regs, meaning that it is going to have to become a
different type to the structure embedded in vcpu_guest_context_u.

In both arch_{get,set}_info_guest(), expand the memcpy()/XLAT_cpu_user_regs()
to copy the fields individually.  This will allow us to eventually make them
different types.

This does cause some minor changes in behaviour for the hypercalls.

It is specifically not the case that a toolstack could set_info(); get_info();
and get an identical bit pattern back.  Amongst other things, the
architectural sticky bits in registers are applied during setting.

Previously, XLAT_cpu_user_regs() omitted the _pad fields in the compat case
whereas the non-compat case included them owing to the single memcpy().

Omit the _pad fields in the non-compat case too; for all but the oldest of
CPUs, the segment selectors are zero-extended by hardware when pushed onto the
stack, so non-zero values here get lost naturally.  Furthermore, FRED reuses
the space above cs and ss for extra state, and a PV guest for now at least
must not be able to write the control state.

Omit the error_code and entry_vector fields too.  They're already identified
as private fields in the public API, and are stale outside of Xen's
interrupt/exception/syscall handler.  They're also a very minor information
leak of which event caused the last deschedule of a vCPU.

Finally, omit saved_upcall_mask.  Xen doesn't consume this, and only produces
it in {compat_,}create_bounce_frame(), based on the vcpu_info page and
settings about the event being injected.  Similar to error_code/entry_vector,
it is stale outside of the guest's event handler.

No change that toolstacks or guests are expected to notice or care about.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>

v2:
 * Zero memory before a partial copy, or note that it already is.
 * Omit error_code/entry_vector/saved_upcall_mask too.

I can't see why saved_upcall_mask exists in the first place, given that it is
also reflected in (v)rflags.IF.  None of MiniOS, Linux or NetBSD use it at
all.  I suspect the reflecting in IF was a slightly later addition dicovered
when running non-toy PV guests, and saved_upcall_mask got left behind as a
wart in the ABI.
---
 xen/arch/x86/domain.c | 38 ++++++++++++++++++++++++++++++++++++--
 xen/arch/x86/domctl.c | 38 ++++++++++++++++++++++++++++++++++++--
 xen/include/xlat.lst  |  2 --
 3 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 56c381618712..56111eac3d94 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1233,7 +1233,24 @@ int arch_set_info_guest(
 
     if ( !compat )
     {
-        memcpy(&v->arch.user_regs, &c.nat->user_regs, 
sizeof(c.nat->user_regs));
+        memset(&v->arch.user_regs, 0, sizeof(v->arch.user_regs));
+        v->arch.user_regs.rbx               = c.nat->user_regs.rbx;
+        v->arch.user_regs.rcx               = c.nat->user_regs.rcx;
+        v->arch.user_regs.rdx               = c.nat->user_regs.rdx;
+        v->arch.user_regs.rsi               = c.nat->user_regs.rsi;
+        v->arch.user_regs.rdi               = c.nat->user_regs.rdi;
+        v->arch.user_regs.rbp               = c.nat->user_regs.rbp;
+        v->arch.user_regs.rax               = c.nat->user_regs.rax;
+        v->arch.user_regs.rip               = c.nat->user_regs.rip;
+        v->arch.user_regs.cs                = c.nat->user_regs.cs;
+        v->arch.user_regs.rflags            = c.nat->user_regs.rflags;
+        v->arch.user_regs.rsp               = c.nat->user_regs.rsp;
+        v->arch.user_regs.ss                = c.nat->user_regs.ss;
+        v->arch.user_regs.es                = c.nat->user_regs.es;
+        v->arch.user_regs.ds                = c.nat->user_regs.ds;
+        v->arch.user_regs.fs                = c.nat->user_regs.fs;
+        v->arch.user_regs.gs                = c.nat->user_regs.gs;
+
         if ( is_pv_domain(d) )
             memcpy(v->arch.pv.trap_ctxt, c.nat->trap_ctxt,
                    sizeof(c.nat->trap_ctxt));
@@ -1241,7 +1258,24 @@ int arch_set_info_guest(
 #ifdef CONFIG_COMPAT
     else
     {
-        XLAT_cpu_user_regs(&v->arch.user_regs, &c.cmp->user_regs);
+        memset(&v->arch.user_regs, 0, sizeof(v->arch.user_regs));
+        v->arch.user_regs.ebx               = c.cmp->user_regs.ebx;
+        v->arch.user_regs.ecx               = c.cmp->user_regs.ecx;
+        v->arch.user_regs.edx               = c.cmp->user_regs.edx;
+        v->arch.user_regs.esi               = c.cmp->user_regs.esi;
+        v->arch.user_regs.edi               = c.cmp->user_regs.edi;
+        v->arch.user_regs.ebp               = c.cmp->user_regs.ebp;
+        v->arch.user_regs.eax               = c.cmp->user_regs.eax;
+        v->arch.user_regs.eip               = c.cmp->user_regs.eip;
+        v->arch.user_regs.cs                = c.cmp->user_regs.cs;
+        v->arch.user_regs.eflags            = c.cmp->user_regs.eflags;
+        v->arch.user_regs.esp               = c.cmp->user_regs.esp;
+        v->arch.user_regs.ss                = c.cmp->user_regs.ss;
+        v->arch.user_regs.es                = c.cmp->user_regs.es;
+        v->arch.user_regs.ds                = c.cmp->user_regs.ds;
+        v->arch.user_regs.fs                = c.cmp->user_regs.fs;
+        v->arch.user_regs.gs                = c.cmp->user_regs.gs;
+
         if ( is_pv_domain(d) )
         {
             for ( i = 0; i < ARRAY_SIZE(c.cmp->trap_ctxt); ++i )
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 3044f706de1c..28fec0e12dbb 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1399,7 +1399,24 @@ void arch_get_info_guest(struct vcpu *v, 
vcpu_guest_context_u c)
         c(flags |= VGCF_online);
     if ( !compat )
     {
-        memcpy(&c.nat->user_regs, &v->arch.user_regs, 
sizeof(c.nat->user_regs));
+        /* Backing memory is pre-zeroed. */
+        c.nat->user_regs.rbx               = v->arch.user_regs.rbx;
+        c.nat->user_regs.rcx               = v->arch.user_regs.rcx;
+        c.nat->user_regs.rdx               = v->arch.user_regs.rdx;
+        c.nat->user_regs.rsi               = v->arch.user_regs.rsi;
+        c.nat->user_regs.rdi               = v->arch.user_regs.rdi;
+        c.nat->user_regs.rbp               = v->arch.user_regs.rbp;
+        c.nat->user_regs.rax               = v->arch.user_regs.rax;
+        c.nat->user_regs.rip               = v->arch.user_regs.rip;
+        c.nat->user_regs.cs                = v->arch.user_regs.cs;
+        c.nat->user_regs.rflags            = v->arch.user_regs.rflags;
+        c.nat->user_regs.rsp               = v->arch.user_regs.rsp;
+        c.nat->user_regs.ss                = v->arch.user_regs.ss;
+        c.nat->user_regs.es                = v->arch.user_regs.es;
+        c.nat->user_regs.ds                = v->arch.user_regs.ds;
+        c.nat->user_regs.fs                = v->arch.user_regs.fs;
+        c.nat->user_regs.gs                = v->arch.user_regs.gs;
+
         if ( is_pv_domain(d) )
             memcpy(c.nat->trap_ctxt, v->arch.pv.trap_ctxt,
                    sizeof(c.nat->trap_ctxt));
@@ -1407,7 +1424,24 @@ void arch_get_info_guest(struct vcpu *v, 
vcpu_guest_context_u c)
 #ifdef CONFIG_COMPAT
     else
     {
-        XLAT_cpu_user_regs(&c.cmp->user_regs, &v->arch.user_regs);
+        /* Backing memory is pre-zeroed. */
+        c.cmp->user_regs.ebx               = v->arch.user_regs.ebx;
+        c.cmp->user_regs.ecx               = v->arch.user_regs.ecx;
+        c.cmp->user_regs.edx               = v->arch.user_regs.edx;
+        c.cmp->user_regs.esi               = v->arch.user_regs.esi;
+        c.cmp->user_regs.edi               = v->arch.user_regs.edi;
+        c.cmp->user_regs.ebp               = v->arch.user_regs.ebp;
+        c.cmp->user_regs.eax               = v->arch.user_regs.eax;
+        c.cmp->user_regs.eip               = v->arch.user_regs.eip;
+        c.cmp->user_regs.cs                = v->arch.user_regs.cs;
+        c.cmp->user_regs.eflags            = v->arch.user_regs.eflags;
+        c.cmp->user_regs.esp               = v->arch.user_regs.esp;
+        c.cmp->user_regs.ss                = v->arch.user_regs.ss;
+        c.cmp->user_regs.es                = v->arch.user_regs.es;
+        c.cmp->user_regs.ds                = v->arch.user_regs.ds;
+        c.cmp->user_regs.fs                = v->arch.user_regs.fs;
+        c.cmp->user_regs.gs                = v->arch.user_regs.gs;
+
         if ( is_pv_domain(d) )
         {
             for ( i = 0; i < ARRAY_SIZE(c.cmp->trap_ctxt); ++i )
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 3c7b6c6830a9..6d6c6cfab251 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -34,8 +34,6 @@
 ?      pmu_intel_ctxt                  arch-x86/pmu.h
 ?      pmu_regs                        arch-x86/pmu.h
 
-!      cpu_user_regs                   arch-x86/xen-@arch@.h
-
 ?      cpu_offline_action              arch-x86/xen-mca.h
 ?      mc                              arch-x86/xen-mca.h
 !      mc_fetch                        arch-x86/xen-mca.h
-- 
2.39.5




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.