[Xen-changelog] [xen master] libxc/restore: Don't duplicate state in process_vcpu_basic()

[patchbot@xxxxxxx]

commit e9bd648015dd192121307669555b396c5f68f2f2
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Wed Dec 18 19:43:18 2019 +0000
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Dec 24 14:59:04 2019 +0000

    libxc/restore: Don't duplicate state in process_vcpu_basic()
    
    vcpu_guest_context_any_t is currently allocated on the stack, and copied 
from
    a mutable buffer which is freed immediately after its use here.  Mutate the
    buffer in place instead of duplicating it.
    
    The code is as it is due to how it was developed.  Originally,
    process_vcpu_basic() operated on a const pointer from the X86_VCPU_BASIC
    record, but during upstreaming, the addition of Remus support required
    buffering of X86_VCPU_BASIC records each checkpoint.
    
    By the time process_vcpu_basic() runs, we are commited to completing state
    restoration and unpausing the guest.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
 tools/libxc/xc_sr_restore_x86_pv.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/tools/libxc/xc_sr_restore_x86_pv.c 
b/tools/libxc/xc_sr_restore_x86_pv.c
index 0a5b0016b4..70b8d2ad95 100644
--- a/tools/libxc/xc_sr_restore_x86_pv.c
+++ b/tools/libxc/xc_sr_restore_x86_pv.c
@@ -236,28 +236,25 @@ static int process_vcpu_basic(struct xc_sr_context *ctx,
                               unsigned int vcpuid)
 {
     xc_interface *xch = ctx->xch;
-    vcpu_guest_context_any_t vcpu;
+    vcpu_guest_context_any_t *vcpu = ctx->x86_pv.restore.vcpus[vcpuid].basic;
     xen_pfn_t pfn, mfn;
     unsigned i, gdt_count;
     int rc = -1;
 
-    memcpy(&vcpu, ctx->x86_pv.restore.vcpus[vcpuid].basic,
-           ctx->x86_pv.restore.vcpus[vcpuid].basicsz);
-
     /* Vcpu 0 is special: Convert the suspend record to an mfn. */
     if ( vcpuid == 0 )
     {
-        rc = process_start_info(ctx, &vcpu);
+        rc = process_start_info(ctx, vcpu);
         if ( rc )
             return rc;
         rc = -1;
     }
 
-    SET_FIELD(&vcpu, flags,
-              GET_FIELD(&vcpu, flags, ctx->x86_pv.width) | VGCF_online,
+    SET_FIELD(vcpu, flags,
+              GET_FIELD(vcpu, flags, ctx->x86_pv.width) | VGCF_online,
               ctx->x86_pv.width);
 
-    gdt_count = GET_FIELD(&vcpu, gdt_ents, ctx->x86_pv.width);
+    gdt_count = GET_FIELD(vcpu, gdt_ents, ctx->x86_pv.width);
     if ( gdt_count > FIRST_RESERVED_GDT_ENTRY )
     {
         ERROR("GDT entry count (%u) out of range (max %u)",
@@ -270,7 +267,7 @@ static int process_vcpu_basic(struct xc_sr_context *ctx,
     /* Convert GDT frames to mfns. */
     for ( i = 0; i < gdt_count; ++i )
     {
-        pfn = GET_FIELD(&vcpu, gdt_frames[i], ctx->x86_pv.width);
+        pfn = GET_FIELD(vcpu, gdt_frames[i], ctx->x86_pv.width);
         if ( pfn > ctx->x86_pv.max_pfn )
         {
             ERROR("GDT frame %u (pfn %#lx) out of range", i, pfn);
@@ -293,11 +290,11 @@ static int process_vcpu_basic(struct xc_sr_context *ctx,
             goto err;
         }
 
-        SET_FIELD(&vcpu, gdt_frames[i], mfn, ctx->x86_pv.width);
+        SET_FIELD(vcpu, gdt_frames[i], mfn, ctx->x86_pv.width);
     }
 
     /* Convert CR3 to an mfn. */
-    pfn = cr3_to_mfn(ctx, GET_FIELD(&vcpu, ctrlreg[3], ctx->x86_pv.width));
+    pfn = cr3_to_mfn(ctx, GET_FIELD(vcpu, ctrlreg[3], ctx->x86_pv.width));
     if ( pfn > ctx->x86_pv.max_pfn )
     {
         ERROR("cr3 (pfn %#lx) out of range", pfn);
@@ -323,12 +320,12 @@ static int process_vcpu_basic(struct xc_sr_context *ctx,
         goto err;
     }
 
-    SET_FIELD(&vcpu, ctrlreg[3], mfn_to_cr3(ctx, mfn), ctx->x86_pv.width);
+    SET_FIELD(vcpu, ctrlreg[3], mfn_to_cr3(ctx, mfn), ctx->x86_pv.width);
 
     /* 64bit guests: Convert CR1 (guest pagetables) to mfn. */
-    if ( ctx->x86_pv.levels == 4 && (vcpu.x64.ctrlreg[1] & 1) )
+    if ( ctx->x86_pv.levels == 4 && (vcpu->x64.ctrlreg[1] & 1) )
     {
-        pfn = vcpu.x64.ctrlreg[1] >> PAGE_SHIFT;
+        pfn = vcpu->x64.ctrlreg[1] >> PAGE_SHIFT;
 
         if ( pfn > ctx->x86_pv.max_pfn )
         {
@@ -355,10 +352,10 @@ static int process_vcpu_basic(struct xc_sr_context *ctx,
             goto err;
         }
 
-        vcpu.x64.ctrlreg[1] = (uint64_t)mfn << PAGE_SHIFT;
+        vcpu->x64.ctrlreg[1] = (uint64_t)mfn << PAGE_SHIFT;
     }
 
-    if ( xc_vcpu_setcontext(xch, ctx->domid, vcpuid, &vcpu) )
+    if ( xc_vcpu_setcontext(xch, ctx->domid, vcpuid, vcpu) )
     {
         PERROR("Failed to set vcpu%u's basic info", vcpuid);
         goto err;
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog