xen: arm: support for save restore and dead migration

offline_page and compression are under a CONFIG_MIGRATE (out of context) and along with xen-hptool these seem to rely on features currently implemented only on x86.

I am shortly going to be enabling CONFIG_MIGRATE for ARM, but these features are not yet available there.

I'm not sure if these are intrinsically x86 or if they deserve their own options.

Signed-off-by Ian Campbell <[email protected]>

Cc andyhhp

---
xc_compression.c seems to actually be unused, possibly since migration v2, perhaps pending colo?

I wasn't sure of the best way to achieve this, but a pair of per-arch hooks seemed to be preferable to ifdeffery.

I also wasn't sure about the change to guest_type for save. The restore half of the ctxt already has such a field but since the save side treats it as an input to the process as opposed to the restore side which determines it from the stream it seemed like keeping them separate was best.

Signed-off-by Ian Campbell <[email protected]>

Cc andyhhp

---
 tools/libxc/xc_sr_common.h     | 15 +++++++++++++++
 tools/libxc/xc_sr_common_x86.c | 22 ++++++++++++++++++++++
 tools/libxc/xc_sr_restore.c    | 15 +++------------
 tools/libxc/xc_sr_save.c       | 22 +++++++---------------
 4 files changed, 47 insertions(+), 27 deletions(-)

tools/libxc/xc_sr_common.h +3 -0

index 64f6082..0d36c8d 100644
@@ -174,6 +174,9 @@ struct xc_sr_context
             struct xc_sr_save_ops ops;
             struct save_callbacks *callbacks;
 
+            /* For Domain Header */
+            uint32_t guest_type;
+
             /* Live migrate vs non live suspend. */
             bool live;
 

Quoted 2 lines · Ian Campbell

I wasn't sure of the best way to achieve this, but a pair of per-arch hooks seemed to be preferable to ifdeffery.

I agree.  The patch looks good.

Quoted 6 lines

I also wasn't sure about the change to guest_type for save. The restore half of the ctxt already has such a field but since the save side treats it as an input to the process as opposed to the restore side which determines it from the stream it seemed like keeping them separate was best.

I would recommend introducing a const uint32_t field in xc_sr_save_ops, to make it harder to accidentally get the guest type out of sync with the ops in use.

The save/restore difference is because of the asymetry in their use. The save side needs to select the correct one to used, based on the domid, while the restore side gets one in the migration stream and has to verify it against the domain it is restoring into.

Quoted 4 lines

Signed-off-by Ian Campbell <[email protected]>

Cc andyhhp

I don't think that is quite enough routing information to find me ;)

This seems almost too easy.

tools/libxc/xc_sr_common_arm.h new file +5 -0

new file mode 100644
index 0000000..efbea70
@@ -0,0 +1,15 @@
+#ifndef __COMMON_ARM__H
+#define __COMMON_ARM__H
+
+#include "xc_sr_common.h"
+

For arm32 + gicv2 systems the following supports apparently successful save/restore as well dead migrate a domain.

There are several caveats/blockers, hence RFC.

* GIC v2 support only, no GIC v3 at all
* ARM32 only. Doesn't even build for ARM64 (vfp state handling needs adjustment)
* No live migration, only "dead" (no logdirty support yet)
* Have not fully audited the set of state which needs to be saved (i.e. I might have missed some registers or something)

On the flip side, thanks to migration v2 the toolstack side of this practically trivial.

This is (of course) using the current Xen hvm blob handling strata, which I believe there are plans afoot to rework along the lines of the libxc/libxl migration v2 protocol.

For now I have no problem waiting and reworking in that vein as things progress, clearly the blockers/caveats (logdirty support in particular) are going to take some time to sort out.
This is all based (very distantly) on some original patches from Evgeny Fedotov and Junghyun Yoo (both of Samsung) however very little of that code remains here.

As well as the Xen series there will be a single Linux patch too, I'll only CC ARM folks on that one.

Quoted 13 lines · Ian Campbell

For arm32 + gicv2 systems the following supports apparently successful save/restore as well dead migrate a domain.

There are several caveats/blockers, hence RFC.

 * GIC v2 support only, no GIC v3 at all
 * ARM32 only. Doesn't even build for ARM64 (vfp state handling needs adjustment)
 * No live migration, only "dead" (no logdirty support yet)
 * Have not fully audited the set of state which needs to be saved (i.e. I
   might have missed some registers or something)

Quoted 2 lines

On the flip side, thanks to migration v2 the toolstack side of this practically trivial.

Glad to hear this.

Quoted 5 lines

This is (of course) using the current Xen hvm blob handling strata, which I
believe there are plans afoot to rework along the lines of the libxc/libxl
migration v2 protocol.

Yes - I have plans as part of fixing cpuid handling for x86.

Quoted 10 lines

For now I have no problem waiting and reworking in that vein as things progress, clearly the blockers/caveats (logdirty support in particular) are
going to take some time to sort out.
This is all based (very distantly) on some original patches from Evgeny Fedotov and Junghyun Yoo (both of Samsung) however very little of that code
remains here.

Quoted 3 lines

As well as the Xen series there will be a single Linux patch too, I'll only
CC ARM folks on that one.

Something towards the end of this series should update docs/features/migration.pandoc to indicate the presence of ARM support.

Because the enum gic_version values do not correspond to the gic version (in order to allow space for variants such as GICv2m, although that is currently not present) logging the raw value is not terribly useful. Provide gic_hw_desc which provides a string describing each GIC version.

Will be used in a later patch.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/gic.c        | 14 ++++++++++++++
 xen/include/asm-arm/gic.h |  1 +
 2 files changed, 15 insertions(+)

xen/arch/arm/gic.c +2 -0

index 1e1e5ba..bea39d6 100644
@@ -62,6 +62,20 @@ enum gic_version gic_hw_version(void)
    return gic_hw_ops->info->hw_version;
 }
 
+const char *gic_hw_desc(enum gic_version v)
+{

xen/arch/arm/gic.c +2 -0

+}
+
 unsigned int gic_number_lines(void)
 {
     return gic_hw_ops->info->nr_lines;

xen/include/asm-arm/gic.h +1 -0

index 42a2eec..818384b 100644
@@ -221,6 +221,7 @@ enum gic_version {
 };
 
 extern enum gic_version gic_hw_version(void);
+extern const char *gic_hw_desc(enum gic_version v);
 
 /* Program the GIC to route an interrupt */
 extern void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,

Signature · 1 line

2.6.1

Quoted 28 lines · Ian Campbell

Because the enum gic_version values do not correspond to the gic version (in order to allow space for variants such as GICv2m, although that is currently not present) logging the raw value is not terribly useful. Provide gic_hw_desc which provides a string describing each GIC version.

Will be used in a later patch.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/gic.c        | 14 ++++++++++++++
 xen/include/asm-arm/gic.h |  1 +

 2 files changed, 15 insertions(+)

xen/arch/arm/gic.c +0 -0

index 1e1e5ba..bea39d6 100644
@@ -62,6 +62,20 @@ enum gic_version gic_hw_version(void)
     return gic_hw_ops->info->hw_version;
  }

xen/arch/arm/gic.c +6 -0

+const char *gic_hw_desc(enum gic_version v)
+{
+    switch (v) {
+    case GIC_V2: return "GIC v2";
+    case GIC_V3: return "GIC v3";
+    }

Quoted 6 lines

+
+    /*
+     * The compiler ought to gripe if the above doesn't cover all enum +     * gic_version, in case some version doesn't.
+     */
+    return "Unknown";

default: ?

In any case

Acked-by Stefano Stabellini <[email protected]>

Quoted 12 lines

xen/arch/arm/gic.c +2 -0

+}
+

 unsigned int gic_number_lines(void)
 {
     return gic_hw_ops->info->nr_lines;

xen/include/asm-arm/gic.h +0 -0

index 42a2eec..818384b 100644
@@ -221,6 +221,7 @@ enum gic_version {

Quoted 8 lines

xen/include/asm-arm/gic.h +0 -0

  };

 extern enum gic_version gic_hw_version(void); +extern const char *gic_hw_desc(enum gic_version v);
 /* Program the GIC to route an interrupt */
 extern void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,

This will allow generic vgic save code to get at this state without needing to know about gic v2 vs v3 etc.

Signed-off-by Ian Campbell <[email protected]>

 xen/arch/arm/gic-v2.c     | 20 ++++++++++++++++----
 xen/arch/arm/gic-v3.c     | 21 ++++++++++++++++-----
 xen/arch/arm/gic.c        |  5 +++++
 xen/include/asm-arm/gic.h |  7 +++++++
 4 files changed, 44 insertions(+), 9 deletions(-)

xen/arch/arm/gic-v2.c +16 -4

index 01e36b5..5cc4cc3 100644
@@ -417,11 +417,8 @@ static void gicv2_clear_lr(int lr)
     writel_gich(0, GICH_LR + lr * 4);
 }
 
-static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
+static void gicv2_decode_lr_value(uint32_t lrv, struct gic_lr *lr_reg)
 {
-    uint32_t lrv;
-
-    lrv          = readl_gich(GICH_LR + lr * 4);
     lr_reg->pirq = (lrv >> GICH_V2_LR_PHYSICAL_SHIFT) & GICH_V2_LR_PHYSICAL_MASK;
     lr_reg->virq = (lrv >> GICH_V2_LR_VIRTUAL_SHIFT) & GICH_V2_LR_VIRTUAL_MASK;
     lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & GICH_V2_LR_PRIORITY_MASK;

@@ -430,6 +427,14 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
     lr_reg->grp       = (lrv >> GICH_V2_LR_GRP_SHIFT) & GICH_V2_LR_GRP_MASK;
 }
 
+static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
+{
+    uint32_t lrv;
+
+    lrv = readl_gich(GICH_LR + lr * 4);
+    gicv2_decode_lr_value(lrv, lr_reg);
+}
+
 static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg)
 {
     uint32_t lrv = 0;

@@ -447,6 +452,12 @@ static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg)
     writel_gich(lrv, GICH_LR + lr * 4);
 }
 
+static void gicv2_vcpu_saved_lr(const struct vcpu *v, int lr,
+                                struct gic_lr *lr_val)
+{
+    gicv2_decode_lr_value(v->arch.gic.v2.lr[lr], lr_val);
+}
+
 static void gicv2_hcr_status(uint32_t flag, bool_t status)
 {
     uint32_t hcr = readl_gich(GICH_HCR);

@@ -746,6 +757,7 @@ const static struct gic_hw_operations gicv2_ops = {
     .save_state          = gicv2_save_state,
     .restore_state       = gicv2_restore_state,
     .dump_state          = gicv2_dump_state,
+    .vcpu_saved_lr       = gicv2_vcpu_saved_lr,
     .gic_host_irq_type   = &gicv2_host_irq_type,
     .gic_guest_irq_type  = &gicv2_guest_irq_type,
     .eoi_irq             = gicv2_eoi_irq,

xen/arch/arm/gic-v3.c +16 -5

index 4fe0c37..342aa4a 100644
@@ -908,12 +908,8 @@ static void gicv3_clear_lr(int lr)
     gicv3_ich_write_lr(lr, 0);
 }
 
-static void gicv3_read_lr(int lr, struct gic_lr *lr_reg)
+static void gicv3_decode_lr_value(uint64_t lrv, struct gic_lr *lr_reg)
 {
-    uint64_t lrv;
-
-    lrv = gicv3_ich_read_lr(lr);
-
     lr_reg->pirq = (lrv >> GICH_LR_PHYSICAL_SHIFT) & GICH_LR_PHYSICAL_MASK;
     lr_reg->virq = (lrv >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK;
 

@@ -923,6 +919,14 @@ static void gicv3_read_lr(int lr, struct gic_lr *lr_reg)
     lr_reg->grp       = (lrv >> GICH_LR_GRP_SHIFT) & GICH_LR_GRP_MASK;
 }
 
+static void gicv3_read_lr(int lr, struct gic_lr *lr_reg)
+{
+    uint64_t lrv;
+
+    lrv = gicv3_ich_read_lr(lr);
+    gicv3_decode_lr_value(lrv, lr_reg);
+}
+
 static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr)
 {
     uint64_t lrv = 0;

@@ -937,6 +941,12 @@ static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr)
     gicv3_ich_write_lr(lr_reg, lrv);
 }
 
+static void gicv3_vcpu_saved_lr(const struct vcpu *v, int lr,
+                                struct gic_lr *lr_val)
+{
+    gicv3_decode_lr_value(v->arch.gic.v3.lr[lr], lr_val);
+}
+
 static void gicv3_hcr_status(uint32_t flag, bool_t status)
 {
     uint32_t hcr;

@@ -1298,6 +1308,7 @@ static const struct gic_hw_operations gicv3_ops = {
     .save_state          = gicv3_save_state,
     .restore_state       = gicv3_restore_state,
     .dump_state          = gicv3_dump_state,
+    .vcpu_saved_lr       = gicv3_vcpu_saved_lr,
     .gic_host_irq_type   = &gicv3_host_irq_type,
     .gic_guest_irq_type  = &gicv3_guest_irq_type,
     .eoi_irq             = gicv3_eoi_irq,

xen/arch/arm/gic.c +5 -0

index bea39d6..8d219e3 100644
@@ -108,6 +108,11 @@ void gic_restore_state(struct vcpu *v)
     gic_restore_pending_irqs(v);
 }
 
+void gic_vcpu_saved_lr(const struct vcpu *v, int lr, struct gic_lr *lr_val)
+{
+    gic_hw_ops->vcpu_saved_lr(v, lr, lr_val);
+}
+
 /*
  * needs to be called with a valid cpu_mask, ie each cpu in the mask has
  * already called gic_cpu_init

xen/include/asm-arm/gic.h +7 -0

index 818384b..b6516f4 100644
@@ -263,6 +263,10 @@ extern int gicv_setup(struct domain *d);
 extern void gic_save_state(struct vcpu *v);
 extern void gic_restore_state(struct vcpu *v);
 
+/* Save/restore */
+extern void gic_vcpu_saved_lr(const struct vcpu *v, int lr,
+                              struct gic_lr *lr_val);
+
 /* SGI (AKA IPIs) */
 enum gic_sgi {
     GIC_SGI_EVENT_CHECK = 0,

@@ -318,6 +322,9 @@ struct gic_hw_operations {
     /* Dump GIC LR register information */
     void (*dump_state)(const struct vcpu *);
 
+    /* Get saved LR state */
+    void (*vcpu_saved_lr)(const struct vcpu *v, int lr, struct gic_lr *lr_val);
+
     /* hw_irq_controller to enable/disable/eoi host irq */
     hw_irq_controller *gic_host_irq_type;
 

Signature · 1 line

2.6.1

... with copyback functionality. A future domctl is going to want this, rather than end up with different ops having different return behaviour, simply switch everything over to a single exit path scheme.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/domctl.c | 76 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 51 insertions(+), 25 deletions(-)

xen/arch/arm/domctl.c +45 -23

index 30453d8..d42b2bf 100644
@@ -12,11 +12,15 @@
 #include <xen/hypercall.h>
 #include <xen/iocap.h>
 #include <xsm/xsm.h>
+#include <xen/guest_access.h>
 #include <public/domctl.h>
 
 long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
                     XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 {
+    long rc;
+    bool_t copyback = 0;
+
     switch ( domctl->cmd )
     {
     case XEN_DOMCTL_cacheflush:

@@ -25,30 +29,36 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
         unsigned long e = s + domctl->u.cacheflush.nr_pfns;
 
         if ( domctl->u.cacheflush.nr_pfns > (1U<<MAX_ORDER) )
-            return -EINVAL;
-
-        if ( e < s )
-            return -EINVAL;
+            rc = -EINVAL;
+        else if ( e < s )
+            rc = -EINVAL;
+        else
+            rc = p2m_cache_flush(d, s, e);
 
-        return p2m_cache_flush(d, s, e);
+        break;
     }
     case XEN_DOMCTL_bind_pt_irq:
     {
-        int rc;
         xen_domctl_bind_pt_irq_t *bind = &domctl->u.bind_pt_irq;
         uint32_t irq = bind->u.spi.spi;
         uint32_t virq = bind->machine_irq;
 
         /* We only support PT_IRQ_TYPE_SPI */
         if ( bind->irq_type != PT_IRQ_TYPE_SPI )
-            return -EOPNOTSUPP;
+        {
+            rc = -EOPNOTSUPP;
+            break;
+        }
 
         /*
          * XXX: For now map the interrupt 1:1. Other support will require to
          * modify domain_pirq_to_irq macro.
          */
         if ( irq != virq )
-            return -EINVAL;
+        {
+            rc = -EINVAL;
+            break;
+        }
 
         /*
          * ARM doesn't require separating IRQ assignation into 2

@@ -60,66 +70,82 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
          */
         rc = xsm_map_domain_irq(XSM_HOOK, d, irq, NULL);
         if ( rc )
-            return rc;
+            break;
 
         rc = xsm_bind_pt_irq(XSM_HOOK, d, bind);
         if ( rc )
-            return rc;
+            break;
 
         if ( !irq_access_permitted(current->domain, irq) )
-            return -EPERM;
+        {
+            rc = -EPERM;
+            break;
+        }
 
         if ( !vgic_reserve_virq(d, virq) )
-            return -EBUSY;
+        {
+            rc = -EBUSY;
+            break;
+        }
 
         rc = route_irq_to_guest(d, virq, irq, "routed IRQ");
         if ( rc )
             vgic_free_virq(d, virq);
 
-        return rc;
+        break;
     }
     case XEN_DOMCTL_unbind_pt_irq:
     {
-        int rc;
         xen_domctl_bind_pt_irq_t *bind = &domctl->u.bind_pt_irq;
         uint32_t irq = bind->u.spi.spi;
         uint32_t virq = bind->machine_irq;
 
         /* We only support PT_IRQ_TYPE_SPI */
         if ( bind->irq_type != PT_IRQ_TYPE_SPI )
-            return -EOPNOTSUPP;
+        {
+            rc = -EOPNOTSUPP;
+            break;
+        }
 
         /* For now map the interrupt 1:1 */
         if ( irq != virq )
-            return -EINVAL;
+        {
+            rc = -EINVAL;
+            break;
+        }
 
         rc = xsm_unbind_pt_irq(XSM_HOOK, d, bind);
         if ( rc )
-            return rc;
+            break;
 
         if ( !irq_access_permitted(current->domain, irq) )
-            return -EPERM;
+        {
+            rc = -EPERM;
+            break;
+        }
 
         rc = release_guest_irq(d, virq);
         if ( rc )
-            return rc;
+            break;
 
         vgic_free_virq(d, virq);
 
-        return 0;
+        rc = 0;
+        break;
     }
     default:
-    {
-        int rc;
-
         rc = subarch_do_domctl(domctl, d, u_domctl);
 

xen/arch/arm/domctl.c +1 -0

+    return rc;
 }
 
 void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)

Signature · 1 line

2.6.1

Quoted 23 lines · Stefano Stabellini

Quoted 22 lines · Ian Campbell

Because the enum gic_version values do not correspond to the gic version (in order to allow space for variants such as GICv2m, although that is currently not present) logging the raw value is not terribly useful. Provide gic_hw_desc which provides a string describing each GIC version.

Will be used in a later patch.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/gic.c        | 14 ++++++++++++++
 xen/include/asm-arm/gic.h |  1 +
 2 files changed, 15 insertions(+)

xen/arch/arm/gic.c +0 -0

index 1e1e5ba..bea39d6 100644
@@ -62,6 +62,20 @@ enum gic_version gic_hw_version(void)
    return gic_hw_ops->info->hw_version;
 }
 

Quoted 13 lines

xen/arch/arm/gic.c enum gic_version gic_hw_version(void) · line 76

Quoted 12 lines

xen/arch/arm/gic.c +12 -0

+const char *gic_hw_desc(enum gic_version v)
+{
+    switch (v) {
+    case GIC_V2: return "GIC v2";
+    case GIC_V3: return "GIC v3";
+    }
+
+    /*
+     * The compiler ought to gripe if the above doesn't cover all enum
+     * gic_version, in case some version doesn't.
+     */
+    return "Unknown";

default: ?

Deliberately not, since that would quash the expected compiler warning.

This is used by the save/restore code.

On ARM we only have RAM (0) or not-RAM (XTAB) types.

Signed-off-by Ian Campbell <[email protected]>

 xen/arch/arm/domctl.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

xen/arch/arm/domctl.c +0 -0

index d42b2bf..d3459ee 100644
@@ -133,6 +133,55 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
         rc = 0;
         break;

xen/arch/arm/domctl.c +40 -0

+        {
+            rc = -E2BIG;
+            break;
+        }
+
+        for ( i = 0; i < num; ++i )
+        {
+            xen_pfn_t gfn = 0, type = 0;
+            struct page_info *page;
+            p2m_type_t t;
+
+            if ( copy_from_guest_offset(&gfn, array, i, 1) )
+            {
+                rc = -EFAULT;
+                break;
+            }
+
+            page = get_page_from_gfn(d, gfn, &t, P2M_ALLOC);
+
+            if ( unlikely(!page) ||
+                 unlikely(is_xen_heap_page(page)) )
+                type = XEN_DOMCTL_PFINFO_XTAB;
+            else
+                type = 0; /* Just regular RAM */
+
+            if ( page )
+                put_page(page);
+
+            if ( copy_to_guest_offset(array, i, &type, 1) )
+            {
+                rc = -EFAULT;
+                break;
+            }
+        }
+
+        rc = 0;
+
+        break;
+    }
+
     default:
         rc = subarch_do_domctl(domctl, d, u_domctl);
 

Signature · 1 line

2.6.1

Quoted 40 lines · Ian Campbell

Quoted 39 lines · Stefano Stabellini

Quoted 33 lines · Ian Campbell

Because the enum gic_version values do not correspond to the gic version (in order to allow space for variants such as GICv2m, although
that is currently not present) logging the raw value is not terribly
useful. Provide gic_hw_desc which provides a string describing each GIC version.

Will be used in a later patch.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/gic.c        | 14 ++++++++++++++
 xen/include/asm-arm/gic.h |  1 +

 2 files changed, 15 insertions(+)

xen/arch/arm/gic.c +0 -0

index 1e1e5ba..bea39d6 100644
@@ -62,6 +62,20 @@ enum gic_version gic_hw_version(void)
     return gic_hw_ops->info->hw_version;
  }

xen/arch/arm/gic.c +9 -0

+const char *gic_hw_desc(enum gic_version v)
+{
+    switch (v) {
+    case GIC_V2: return "GIC v2";
+    case GIC_V3: return "GIC v3";
+    }
+
+    /*
+     * The compiler ought to gripe if the above doesn't cover all

Quoted 4 lines

enum
+     * gic_version, in case some version doesn't. +     */
+    return "Unknown";

default: ?

Quoted 1 line

Deliberately not, since that would quash the expected compiler warning.

What expected compiler warning?

That switch statement, complete or not, is always valid.

 .. .arch_do_domclt...

Quoted 3 lines

+
+    if ( copyback && __copy_to_guest(u_domctl, domctl, 1) ) +        rc = -EFAULT;

This code is already present in do_domctl (the caller of this function). I wonder if it makes sense to pass a copyback* parameter to arch_do_domctl and just rely on the caller's exit path.

Quoted 7 lines · Stefano Stabellini

.. .arch_do_domclt...

xen/arch/arm/domctl.c long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, · line 146

Quoted 3 lines

xen/arch/arm/domctl.c +3 -0

+
+    if ( copyback && __copy_to_guest(u_domctl, domctl, 1) )
+        rc = -EFAULT;

This code is already present in do_domctl (the caller of this function). I wonder if it makes sense to pass a copyback* parameter to arch_do_domctl and just rely on the caller's exit path.

Could do, cc-ing Jan in addition to Andy for x86 perspective (since this would be a generic change.

Quoted 1 line

Quoted 1 line · On 15.12.15 at 17:57, <[email protected]> wrote:

Quoted 5 lines · Stefano Stabellini

.. .arch_do_domclt...

Quoted 3 lines

xen/arch/arm/domctl.c +3 -0

+
+    if ( copyback && __copy_to_guest(u_domctl, domctl, 1) )
+        rc = -EFAULT;

Quoted 3 lines

This code is already present in do_domctl (the caller of this function). I wonder if it makes sense to pass a copyback* parameter to arch_do_domctl and just rely on the caller's exit path.

Could do, cc-ing Jan in addition to Andy for x86 perspective (since this would be a generic change.

This is just the minimally required basic infra and header, no actual useful data is saved yet.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/Makefile                  |  1 +
 xen/arch/arm/domctl.c                  | 74 ++++++++++++++++++++++++++++++++++
 xen/arch/arm/save.c                    | 53 ++++++++++++++++++++++++
 xen/common/Makefile                    |  2 +
 xen/include/asm-arm/hvm/support.h      | 25 ++++++++++++
 xen/include/public/arch-arm/hvm/save.h | 17 ++++++++
 6 files changed, 172 insertions(+)
 create mode 100644 xen/arch/arm/save.c
 create mode 100644 xen/include/asm-arm/hvm/support.h

xen/arch/arm/Makefile +1 -0

index 4ac5edd..5a96486 100644
@@ -40,6 +40,7 @@ obj-y += device.o
 obj-y += decode.o
 obj-y += processor.o
 obj-y += smc.o
+obj-y += save.o
 
 #obj-bin-y += ....o
 

xen/arch/arm/domctl.c +1 -0

index d3459ee..3403e23 100644
@@ -12,6 +12,7 @@
 #include <xen/hypercall.h>
 #include <xen/iocap.h>
 #include <xsm/xsm.h>
+#include <xen/hvm/save.h>
 #include <xen/guest_access.h>
 #include <public/domctl.h>
 

xen/arch/arm/domctl.c +16 -0

+        if ( (c.data = xmalloc_bytes(c.size)) == NULL )
+        {
+            rc = -ENOMEM;
+            goto sethvmcontext_out;
+        }
+
+        if ( copy_from_guest(c.data, domctl->u.hvmcontext.buffer, c.size) != 0 )
+        {
+            rc = -EFAULT;
+            goto sethvmcontext_out;
+        }
+
+        domain_pause(d);
+        rc = hvm_load(d, &c);
+        domain_unpause(d);
+

xen/arch/arm/domctl.c +31 -0

+        c.size = hvm_save_size(d);
+
+        if ( guest_handle_is_null(domctl->u.hvmcontext.buffer) )
+        {
+            /* Client is querying for the correct buffer size */
+            domctl->u.hvmcontext.size = c.size;
+            rc = 0;
+            goto gethvmcontext_out;
+        }
+
+        /* Check that the client has a big enough buffer */
+        if ( domctl->u.hvmcontext.size < c.size )
+        {
+            rc = -ENOSPC;
+            goto gethvmcontext_out;
+        }
+
+        /* Allocate our own marshalling buffer */
+        if ( (c.data = xmalloc_bytes(c.size)) == NULL )
+        {
+            rc = -ENOMEM;
+            goto gethvmcontext_out;
+        }
+
+        domain_pause(d);
+        rc = hvm_save(d, &c);
+        domain_unpause(d);
+
+        domctl->u.hvmcontext.size = c.cur;
+        if ( copy_to_guest(domctl->u.hvmcontext.buffer, c.data, c.size) != 0 )
+            rc = -EFAULT;

xen/arch/arm/domctl.c +0 -0

    case XEN_DOMCTL_getpageframeinfo3:
     {
         unsigned int i;

xen/arch/arm/save.c new file +53 -0

new file mode 100644
index 0000000..6a1934b
@@ -0,0 +1,53 @@
+/*
+ * hvm/save.c: Save and restore HVM guest's emulated hardware state for ARM.
+ *
+ * Copyright (c) 2013, Samsung Electronics.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <asm/hvm/support.h>
+#include <public/hvm/save.h>
+
+void arch_hvm_save(struct domain *d, struct hvm_save_header *hdr)
+{
+}
+
+int arch_hvm_load(struct domain *d, struct hvm_save_header *hdr)
+{
+    if ( hdr->magic != HVM_FILE_MAGIC )
+    {
+        printk(XENLOG_G_ERR "HVM%d restore: bad magic number %#"PRIx32"\n",
+               d->domain_id, hdr->magic);
+        return -1;
+    }
+
+    if ( hdr->version != HVM_FILE_VERSION )
+    {
+        printk(XENLOG_G_ERR "HVM%d restore: unsupported version %u\n",
+               d->domain_id, hdr->version);
+        return -1;
+    }
+
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */

xen/common/Makefile +2 -0

index 3547c8e..ce8078f 100644
@@ -67,6 +67,8 @@ obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o multicall
 
 subdir-$(x86_64) += hvm
 
+subdir-$(CONFIG_ARM) += hvm
+
 subdir-$(coverage) += gcov
 
 subdir-y += libelf

xen/include/asm-arm/hvm/support.h new file +15 -0

new file mode 100644
index 0000000..842d8a3
@@ -0,0 +1,25 @@
+/*
+ * support.h: HVM support routines used by ARM.
+ *
+ * Copyright (c) 2013, Samsung Electronics
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with

xen/include/public/arch-arm/hvm/save.h +16 -0

+#define HVM_FILE_VERSION 0x00000001
+
+struct hvm_save_header
+{
+    uint32_t magic;             /* Must be HVM_FILE_MAGIC */
+    uint32_t version;           /* File format version */
+    uint64_t changeset;         /* Version of Xen that saved this file */
+};
+
+DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header);
+
+/*
+ * Largest type-code in use
+ */
+#define HVM_SAVE_CODE_MAX 1
+
 #endif
 
 /*

Signature · 1 line

2.6.1

These correspond to the content of struct xen_arch_domainconfig.

On restore various things are checked, mostly to ensure they match the hardcoded things of the restoring Xen.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/save.c                    | 44 ++++++++++++++++++++++++++++++++++
 xen/include/public/arch-arm/hvm/save.h |  6 +++++
 2 files changed, 50 insertions(+)

xen/arch/arm/save.c +44 -0

index 6a1934b..7b34782 100644
@@ -21,6 +21,17 @@
 
 void arch_hvm_save(struct domain *d, struct hvm_save_header *hdr)
 {
+    switch ( d->arch.vgic.version )
+    {
+    case GIC_V2: hdr->gic_version = XEN_DOMCTL_CONFIG_GIC_V2; break;
+    case GIC_V3: hdr->gic_version = XEN_DOMCTL_CONFIG_GIC_V3; break;
+    default: BUG();
+    }
+
+    hdr->nr_spis = d->arch.vgic.nr_spis;
+    hdr->clock_frequency = cpu_khz;
+
+    hdr->evtchn_irq = GUEST_EVTCHN_PPI;
 }
 
 int arch_hvm_load(struct domain *d, struct hvm_save_header *hdr)

@@ -39,6 +50,39 @@ int arch_hvm_load(struct domain *d, struct hvm_save_header *hdr)
         return -1;
     }
 
+    switch ( hdr->gic_version )
+    {
+    case XEN_DOMCTL_CONFIG_GIC_V2: d->arch.vgic.version = GIC_V2; break;
+    case XEN_DOMCTL_CONFIG_GIC_V3: d->arch.vgic.version = GIC_V3; break;
+    default:
+        printk(XENLOG_G_ERR "HVM%d restore: unsupported gic version %u\n",
+               d->domain_id, hdr->gic_version);
+        return -1;
+    }
+
+    if ( hdr->nr_spis )
+    {
+        printk(XENLOG_G_ERR "HVM%d restore: cannot support nr_spis != 0, %u\n",
+               d->domain_id, hdr->nr_spis);
+        return -1;
+    }
+
+    if ( hdr->clock_frequency != cpu_khz )
+    {
+        printk(XENLOG_G_ERR
+            "HVM%d restore: unsupported guest clock %"PRId32"kHz on host @ %ldkHz\n",
+            d->domain_id, hdr->clock_frequency, cpu_khz);
+        return -1;
+    }
+
+    if ( hdr->evtchn_irq != GUEST_EVTCHN_PPI )
+    {
+        printk(XENLOG_G_ERR
+            "HVM%d restore: unsupported guest evtchn IRQ%u host uses IRQ%u\n",
+            d->domain_id, hdr->evtchn_irq, GUEST_EVTCHN_PPI);
+        return -1;
+    }
+
     return 0;
 }
 

xen/include/public/arch-arm/hvm/save.h +0 -0

index 5f4de94..6f1be37 100644

xen/include/public/arch-arm/hvm/save.h +0 -0

 };
 
 DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header);

Signature · 1 line

2.6.1

Quoted 48 lines · Ian Campbell

This is just the minimally required basic infra and header, no actual useful data is saved yet.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/Makefile                  |  1 + xen/arch/arm/domctl.c                  | 74
++++++++++++++++++++++++++++++++++
 xen/arch/arm/save.c                    | 53 ++++++++++++++++++++++++ xen/common/Makefile                    |  2 + xen/include/asm-arm/hvm/support.h      | 25 ++++++++++++ xen/include/public/arch-arm/hvm/save.h | 17 ++++++++

 6 files changed, 172 insertions(+)

 create mode 100644 xen/arch/arm/save.c create mode 100644 xen/include/asm-arm/hvm/support.h

xen/arch/arm/Makefile +0 -0

index 4ac5edd..5a96486 100644
@@ -40,6 +40,7 @@ obj-y += device.o

 obj-y += decode.o
 obj-y += processor.o
 obj-y += smc.o
+obj-y += save.o

 #obj-bin-y += ....o

xen/arch/arm/domctl.c +1 -0

index d3459ee..3403e23 100644
@@ -12,6 +12,7 @@
  #include <xen/hypercall.h>
  #include <xen/iocap.h>
  #include <xsm/xsm.h>
+#include <xen/hvm/save.h>
  #include <xen/guest_access.h>
  #include <public/domctl.h>

xen/arch/arm/domctl.c +0 -0

@@ -134,6 +135,79 @@ long arch_do_domctl(struct xen_domctl *domctl,
 struct domain *d,
          break;
      }

+    case XEN_DOMCTL_sethvmcontext:
+    {
+        struct hvm_domain_context c = { .size = domctl-

Quoted 1 line

Quoted 1 line

u.hvmcontext.size };

if ( d == currend->domain )
    return some_kind_of_error;
?

Actually now that I have read the patch, I think that rather than duplicating the code, we should just move the x86 implementation of XEN_DOMCTL_sethvmcontext and XEN_DOMCTL_gethvmcontext to common code. The differences are negligible.

xen/arch/arm/domctl.c

Quoted 7 lines

xen/arch/arm/domctl.c +7 -0

+#ifndef __ASM_ARM_HVM_SUPPORT_H__
+#define __ASM_ARM_HVM_SUPPORT_H__
+
+#include <xen/sched.h>
+#include <xen/hvm/save.h>
+
+#endif /* __ASM_ARM_HVM_SUPPORT_H__ */

Interestingly I can compile Xen on x86_64 just fine removing the #include <asm/hvm/support.h> in xen/common/hvm/save.c. I think that is a better option than introducing this empty header file.

Quoted 5 lines

xen/include/public/arch-arm/hvm/save.h +0 -0

index 75b8e65..5f4de94 100644

Quoted 5 lines

xen/include/public/arch-arm/hvm/save.h +0 -0

@@ -26,6 +26,23 @@
  #ifndef __XEN_PUBLIC_HVM_SAVE_ARM_H__
  #define __XEN_PUBLIC_HVM_SAVE_ARM_H__

+#define HVM_FILE_MAGIC   0x92385520

Out of curiousity, how did you come up with the number? Just making sure there is no secret message in there.

Quoted 95 lines · Ian Campbell

These correspond to the content of struct xen_arch_domainconfig.

On restore various things are checked, mostly to ensure they match the hardcoded things of the restoring Xen.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/save.c                    | 44
++++++++++++++++++++++++++++++++++
 xen/include/public/arch-arm/hvm/save.h |  6 +++++

 2 files changed, 50 insertions(+)

xen/arch/arm/save.c +0 -0

index 6a1934b..7b34782 100644
@@ -21,6 +21,17 @@

 void arch_hvm_save(struct domain *d, struct hvm_save_header *hdr) {
+    switch ( d->arch.vgic.version )
+    {
+    case GIC_V2: hdr->gic_version = XEN_DOMCTL_CONFIG_GIC_V2; break; +    case GIC_V3: hdr->gic_version = XEN_DOMCTL_CONFIG_GIC_V3; break; +    default: BUG();
+    }

xen/arch/arm/save.c +5 -0

+
+    hdr->nr_spis = d->arch.vgic.nr_spis;
+    hdr->clock_frequency = cpu_khz;
+
+    hdr->evtchn_irq = GUEST_EVTCHN_PPI;

 }

 int arch_hvm_load(struct domain *d, struct hvm_save_header *hdr)

xen/arch/arm/save.c +0 -0

@@ -39,6 +50,39 @@ int arch_hvm_load(struct domain *d, struct
 hvm_save_header *hdr)
          return -1;
      }

+    switch ( hdr->gic_version )
+    {
+    case XEN_DOMCTL_CONFIG_GIC_V2: d->arch.vgic.version = GIC_V2; break;
+    case XEN_DOMCTL_CONFIG_GIC_V3: d->arch.vgic.version = GIC_V3; break;
+    default:
+        printk(XENLOG_G_ERR "HVM%d restore: unsupported gic version %u\n",
+               d->domain_id, hdr->gic_version);
+        return -1;
+    }
+
+    if ( hdr->nr_spis )
+    {
+        printk(XENLOG_G_ERR "HVM%d restore: cannot support nr_spis != 0, %u\n",
+               d->domain_id, hdr->nr_spis);
+        return -1;
+    }
+
+    if ( hdr->clock_frequency != cpu_khz )
+    {
+        printk(XENLOG_G_ERR
+            "HVM%d restore: unsupported guest clock %"PRId32"kHz on host @ %ldkHz\n",
+            d->domain_id, hdr->clock_frequency, cpu_khz); +        return -1;
+    }
+
+    if ( hdr->evtchn_irq != GUEST_EVTCHN_PPI )
+    {
+        printk(XENLOG_G_ERR
+            "HVM%d restore: unsupported guest evtchn IRQ%u host uses IRQ%u\n",
+            d->domain_id, hdr->evtchn_irq, GUEST_EVTCHN_PPI); +        return -1;
+    }
+
     return 0;
 }

xen/include/public/arch-arm/hvm/save.h +0 -0

index 5f4de94..6f1be37 100644
@@ -34,6 +34,12 @@ struct hvm_save_header

     uint32_t magic;             /* Must be HVM_FILE_MAGIC */ uint32_t version;           /* File format version */ uint64_t changeset;         /* Version of Xen that saved this file */
+
+    uint8_t  gic_version;       /* XEN_DOMCTL_CONFIG_GIC_v* (_NOT_ _NATIVE) */
+    uint32_t nr_spis;           /* Currently must be 0 */

Quoted 3 lines

+    uint32_t clock_frequency;   /* kHz */
+
+    uint32_t evtchn_irq;

I would prefer if you added these in the previous patch, or maybe better, you introduced HVM_FILE_VERSION only with the last patch. Otherwise theoretically you would have to bump HVM_FILE_VERSION with this patch.

The rest looks good.

XXX TBD No support for arm64 (or even 32-bit guest on arm64). XXX In particular the handling of save/restore of VFP state doesn't XXX even compile for arm32. I need to investigate the best way to XXX reflect the differing possible VFB states in the save record.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/hvm.c                     | 167 +++++++++++++++++++++++++++++++++
 xen/include/public/arch-arm/hvm/save.h |  38 +++++++-
 2 files changed, 204 insertions(+), 1 deletion(-)

xen/arch/arm/hvm.c +1 -0

index 5fd0753..3c59e63 100644
@@ -7,6 +7,7 @@
 
 #include <xsm/xsm.h>
 
+#include <xen/hvm/save.h>
 #include <public/xen.h>
 #include <public/hvm/params.h>
 #include <public/hvm/hvm_op.h>

xen/arch/arm/hvm.c +7 -0

+    /* Save the state of CPU */
+    for_each_vcpu( d, v )
+    {
+        /*
+         * We don't need to save state for a vcpu that is down; the restore
+         * code will leave it down if there is nothing saved.
+         */

xen/arch/arm/hvm.c +7 -0

+        ctxt.cpacr = v->arch.cpacr;
+
+        ctxt.contextidr = v->arch.contextidr;
+        ctxt.tpidr_el0 = v->arch.tpidr_el0;
+        ctxt.tpidr_el1 = v->arch.tpidr_el1;
+        ctxt.tpidrro_el0 = v->arch.tpidrro_el0;
+

xen/arch/arm/hvm.c +108 -0

+#ifdef CONFIG_ARM_32 /* XXX 32on64 */
+        ctxt.joscr = v->arch.joscr;
+        ctxt.jmcr = v->arch.jmcr;
+#endif
+
+        memset(&ctxt.core_regs, 0, sizeof(ctxt.core_regs));
+
+        /* get guest core registers */
+        vcpu_regs_hyp_to_user(v, &ctxt.core_regs);
+
+        /* check VFP state size before dumping */
+        BUILD_BUG_ON(sizeof(v->arch.vfp) > sizeof (ctxt.vfp));
+        memcpy((void*) &ctxt.vfp, (void*) &v->arch.vfp, sizeof(v->arch.vfp));
+
+        if ( hvm_save_entry(VCPU, v->vcpu_id, h, &ctxt) != 0 )
+            return 1;
+    }
+    return 0;
+}
+
+static int cpu_load(struct domain *d, hvm_domain_context_t *h)
+{
+    int vcpuid;
+    struct hvm_hw_cpu ctxt;
+    struct vcpu *v;
+
+    /* Which vcpu is this? */
+    vcpuid = hvm_load_instance(h);
+    if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
+    {
+        dprintk(XENLOG_G_ERR, "HVM restore: dom%u has no vcpu%u\n",
+                d->domain_id, vcpuid);
+        return -EINVAL;
+    }
+
+    if ( hvm_load_entry(VCPU, h, &ctxt) != 0 )
+        return -EINVAL;
+
+    v->arch.sctlr = ctxt.sctlr;
+    v->arch.ttbr0 = ctxt.ttbr0;
+    v->arch.ttbr1 = ctxt.ttbr1;
+    v->arch.ttbcr = ctxt.ttbcr;
+
+    v->arch.dacr = ctxt.dacr;
+#ifdef CONFIG_ARM_32 /* XXX 32on64 */
+    v->arch.ifar = ctxt.ifar;
+    v->arch.ifsr = ctxt.ifsr;
+    v->arch.dfar = ctxt.dfar;
+    v->arch.dfsr = ctxt.dfsr;
+#else
+    /* XXX 64-bit */
+#endif
+
+#ifdef CONFIG_ARM_32
+    v->arch.mair0 = ctxt.mair0;
+    v->arch.mair1 = ctxt.mair1;
+#else
+    v->arch.mair = ctxt.mair0;
+#endif
+
+    /* Control Registers */
+    v->arch.actlr = ctxt.actlr;
+    v->arch.cpacr = ctxt.cpacr;
+    v->arch.contextidr = ctxt.contextidr;
+    v->arch.tpidr_el0 = ctxt.tpidr_el0;
+    v->arch.tpidr_el1 = ctxt.tpidr_el1;
+    v->arch.tpidrro_el0 = ctxt.tpidrro_el0;
+
+    /* CP 15 */
+    v->arch.csselr = ctxt.csselr;
+    v->arch.vmpidr = ctxt.mpidr;
+
+    v->arch.afsr0 = ctxt.afsr0;
+    v->arch.afsr1 = ctxt.afsr1;
+    v->arch.vbar = ctxt.vbar;
+    v->arch.par = ctxt.par;
+    v->arch.teecr = ctxt.teecr;
+    v->arch.teehbr = ctxt.teehbr;
+#ifdef CONFIG_ARM_32 /* XXX 32on64 */
+    v->arch.joscr = ctxt.joscr;
+    v->arch.jmcr = ctxt.jmcr;
+#endif
+
+    /* set guest core registers */
+    vcpu_regs_user_to_hyp(v, &ctxt.core_regs);
+
+    /* restore VFP */
+    BUILD_BUG_ON(sizeof(v->arch.vfp) > sizeof (ctxt.vfp));
+    memcpy(&v->arch.vfp, &ctxt.vfp,  sizeof(v->arch.vfp));
+
+    v->is_initialised = 1;
+    clear_bit(_VPF_down, &v->pause_flags);
+
+    /* we don't need vcpu_wake(v) here */
+    return 0;
+}
+
+HVM_REGISTER_SAVE_RESTORE(VCPU, cpu_save, cpu_load, 1, HVMSR_PER_VCPU);
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */

xen/include/public/arch-arm/hvm/save.h +37 -1

index 6f1be37..72474e5 100644
@@ -44,10 +44,46 @@ struct hvm_save_header
 
 DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header);
 
+struct hvm_hw_cpu
+{
+    uint64_t vfp[34]; /* Vector floating pointer */
+    /* VFP v3 state is 34x64 bit, VFP v4 is not yet supported */
+
+    /* Guest core registers */
+    struct vcpu_guest_core_regs core_regs;
+
+    uint32_t sctlr, ttbcr;
+    uint64_t ttbr0, ttbr1;
+
+    uint32_t ifar, dfar;
+    uint32_t ifsr, dfsr;
+    uint32_t dacr;
+    uint64_t par;
+
+    uint64_t mair0, mair1;
+    uint64_t tpidr_el0;
+    uint64_t tpidr_el1;
+    uint64_t tpidrro_el0;
+    uint64_t vbar;
+
+    /* Control Registers */
+    uint32_t actlr;
+    uint32_t cpacr;
+    uint32_t afsr0, afsr1;
+    uint32_t contextidr;
+    uint32_t teecr, teehbr; /* ThumbEE, 32-bit guests only */
+    uint32_t joscr, jmcr;
+    /* CP 15 */
+    uint32_t csselr;
+    uint64_t mpidr;
+};
+
+DECLARE_HVM_SAVE_TYPE(VCPU, 2, struct hvm_hw_cpu);
+
 /*
  * Largest type-code in use
  */
-#define HVM_SAVE_CODE_MAX 1
+#define HVM_SAVE_CODE_MAX 2
 
 #endif
 

Signature · 1 line

2.6.1

Quoted 29 lines · Ian Campbell

XXX TBD No support for arm64 (or even 32-bit guest on arm64). XXX In particular the handling of save/restore of VFP state doesn't XXX even compile for arm32. I need to investigate the best way to XXX reflect the differing possible VFB states in the save record.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/hvm.c                     | 167
+++++++++++++++++++++++++++++++++
 xen/include/public/arch-arm/hvm/save.h |  38 +++++++-

 2 files changed, 204 insertions(+), 1 deletion(-)

xen/arch/arm/hvm.c +0 -0

index 5fd0753..3c59e63 100644
@@ -7,6 +7,7 @@

 #include <xsm/xsm.h>

+#include <xen/hvm/save.h>
 #include <public/xen.h>
 #include <public/hvm/params.h>
 #include <public/hvm/hvm_op.h>

xen/arch/arm/hvm.c +0 -0

@@ -65,3 +66,169 @@ long do_hvm_op(unsigned long op,
 XEN_GUEST_HANDLE_PARAM(void) arg)

     return rc;
 }

xen/arch/arm/hvm.c long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) · line 73

Quoted 5 lines

xen/arch/arm/hvm.c +5 -0

+
+static int cpu_save(struct domain *d, hvm_domain_context_t *h)
+{
+    struct hvm_hw_cpu ctxt;
+    struct vcpu *v;

I wonder whether we could refactor the code to share the same underlying function between cpu_save and ctxt_switch_from.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/vtimer.c                  | 72 ++++++++++++++++++++++++++++++++++
 xen/include/public/arch-arm/hvm/save.h | 15 ++++++-
 2 files changed, 86 insertions(+), 1 deletion(-)

xen/arch/arm/vtimer.c +18 -0

index 629feb4..9dfc699 100644
@@ -22,6 +22,7 @@
 #include <xen/timer.h>
 #include <xen/sched.h>
 #include <xen/perfc.h>
+#include <xen/hvm/save.h>
 #include <asm/div64.h>
 #include <asm/irq.h>
 #include <asm/time.h>

@@ -355,6 +356,77 @@ int vtimer_emulate(struct cpu_user_regs *regs, union hsr hsr)
     }
 }
 
+static int timer_save_one(hvm_domain_context_t *h,
+                          struct vcpu *v, struct vtimer *t,
+                          int type, uint64_t offset)
+{
+   struct hvm_hw_timer ctxt;
+
+   ctxt.cval = t->cval;
+   ctxt.ctl = t->ctl;
+   ctxt.vtb_offset = offset;
+   ctxt.type = type;
+   if ( hvm_save_entry(A15_TIMER, v->vcpu_id, h, &ctxt) != 0 )
+       return 1;
+   return 0;
+}
+
+static int timer_save(struct domain *d, hvm_domain_context_t *h)
+{

xen/arch/arm/vtimer.c +24 -0

+    return 0;
+}
+
+static int timer_load(struct domain *d, hvm_domain_context_t *h)
+{
+    int vcpuid;
+    struct hvm_hw_timer ctxt;
+    struct vcpu *v;
+    struct vtimer *t = NULL;
+
+    /* Which vcpu is this? */
+    vcpuid = hvm_load_instance(h);
+
+    if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
+    {
+        dprintk(XENLOG_G_ERR, "HVM restore: dom%u has no vcpu%u\n",
+                d->domain_id, vcpuid);
+        return -EINVAL;
+    }
+
+    if ( hvm_load_entry(A15_TIMER, h, &ctxt) != 0 )
+        return -EINVAL;
+
+    if ( ctxt.type == HVM_SAVE_TIMER_TYPE_VIRT )

xen/arch/arm/vtimer.c +9 -0

+    t->cval = ctxt.cval;
+    t->ctl = ctxt.ctl;
+    t->v = v;
+
+    return 0;
+}
+
+HVM_REGISTER_SAVE_RESTORE(A15_TIMER, timer_save, timer_load, 2, HVMSR_PER_VCPU);
+
 /*
  * Local variables:
  * mode: C

xen/include/public/arch-arm/hvm/save.h +0 -0

index 72474e5..7b92c9c 100644