Re: [PATCH RFC 3/4] Implement save and restore for gic (template impl)

Re: [PATCH RFC 3/4] Implement save and restore for gic (template impl)

[Jaeyong Yoo]

> > +static int gic_save(struct domain *d, hvm_domain_context_t *h)
> > +{
> > +    struct hvm_hw_gic ctxt;
> > +    struct vcpu *v;
> > +
> > +    /* Save the state of GICs */
> > +    for_each_vcpu( d, v )
> > +    {
> > +        ctxt.gic_hcr = v->arch.gic_hcr;
> > +        ctxt.gic_vmcr = v->arch.gic_vmcr;
> > +        ctxt.gic_apr = v->arch.gic_apr;
> > +        memcpy( ctxt.gic_lr, v->arch.gic_lr, sizeof(v->arch.gic_lr) );
> > +        ctxt.event_mask = v->arch.event_mask;
> > +        ctxt.lr_mask = v->arch.lr_mask;
> 
> In general on x86 the policy is that hvm headers should store only
> architectural state. I think this is a good policy to carry over to ARM.
> 
> With that in mind I don't think either event_mask or lr_mask are
> architectural state but are actually internal state of the vgic
> "emulation" which can and should be reconstructed when loading the
> architectural state.

Thanks! That is very helpful :)

Jaeyong

[PATCH RFC 0/4] arm: regarding live migration

[Jaeyong Yoo]

Hi all,
I'm interested in developing live migration in xen arm and possibly 
the contribution to the community and I hope this patch series could be a start.

For this matter, I have following questions:

(1) Is it OK to keep using the keyword "hvm"? Or, is it better to use pvh?

(2) After some overview of source code, I think the required parts
   for save/restore are the following:
      - xen-store info
      - shared info page
      - memory contents (no need for p2m table)
      - cpu/vcpu 
      - gic/vgic
      - drivers
   I think there are still important parts that I'm missing.
   I appreciate if you could give some advice :)

(3) Regarding split drivers, come to think of it, we have to store 
   both side (front/back) states, in-flight event channels, IRQs, etc.
   And those look like quite a work (although evtchn is migrated within vcpu)
   I appreciate  if you guys could share any hints from the experience of 
   migrating split drivers in x86.

Lastly I would like to note that the following patch series is just the 
concept work for reviewing my idea and they are quite preliminary.


Jaeyong Yoo (4):
  Create new directory for stroing hvm-related files in ARM.
  Implement arch_hvm_save and arch_hvm_load functions
  Implement save and restore for gic (template impl)
  Implement XEN_DOMCTL_gethvmcontext part of arch_do_domctl

 xen/arch/arm/Makefile                  |    2 +-
 xen/arch/arm/domctl.c                  |   58 +++++++++++++++-
 xen/arch/arm/hvm.c                     |   67 ------------------
 xen/arch/arm/hvm/Makefile              |    2 +
 xen/arch/arm/hvm/hvm.c                 |  118 ++++++++++++++++++++++++++++++++
 xen/arch/arm/hvm/save.c                |   69 +++++++++++++++++++
 xen/common/Makefile                    |    2 +
 xen/include/asm-arm/hvm/support.h      |   29 ++++++++
 xen/include/public/arch-arm/hvm/save.h |   36 ++++++++++
 9 files changed, 314 insertions(+), 69 deletions(-)
 delete mode 100644 xen/arch/arm/hvm.c
 create mode 100644 xen/arch/arm/hvm/Makefile
 create mode 100644 xen/arch/arm/hvm/hvm.c
 create mode 100644 xen/arch/arm/hvm/save.c
 create mode 100644 xen/include/asm-arm/hvm/support.h

-- 
1.7.9.5

[PATCH RFC 3/4] Implement save and restore for gic (template impl)

[Jaeyong Yoo]

Signed-off-by: Jaeyong Yoo <jaeyong.yoo@samsung.com>
---
 xen/arch/arm/hvm/hvm.c                 |   51 ++++++++++++++++++++++++++++++++
 xen/common/Makefile                    |    2 ++
 xen/include/asm-arm/hvm/support.h      |   29 ++++++++++++++++++
 xen/include/public/arch-arm/hvm/save.h |   20 +++++++++++++
 4 files changed, 102 insertions(+)
 create mode 100644 xen/include/asm-arm/hvm/support.h

diff --git a/xen/arch/arm/hvm/hvm.c b/xen/arch/arm/hvm/hvm.c
index 471c4cd..45cc4fd 100644
--- a/xen/arch/arm/hvm/hvm.c
+++ b/xen/arch/arm/hvm/hvm.c
@@ -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>
@@ -65,3 +66,53 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
 
     return rc;
 }
+
+static int gic_save(struct domain *d, hvm_domain_context_t *h)
+{
+    struct hvm_hw_gic ctxt;
+    struct vcpu *v;
+
+    /* Save the state of GICs */
+    for_each_vcpu( d, v )
+    {
+        ctxt.gic_hcr = v->arch.gic_hcr;
+        ctxt.gic_vmcr = v->arch.gic_vmcr;
+        ctxt.gic_apr = v->arch.gic_apr;
+        memcpy( ctxt.gic_lr, v->arch.gic_lr, sizeof(v->arch.gic_lr) );
+        ctxt.event_mask = v->arch.event_mask;
+        ctxt.lr_mask = v->arch.lr_mask;
+        if ( hvm_save_entry(GIC, v->vcpu_id, h, &ctxt) != 0 )
+            return 1;
+    }
+    return 0;
+}
+
+static int gic_load(struct domain *d, hvm_domain_context_t *h)
+{
+    int vcpuid;
+    struct hvm_hw_gic 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(GIC, h, &ctxt) != 0 )
+        return -EINVAL;
+
+    v->arch.gic_hcr = ctxt.gic_hcr;
+    v->arch.gic_vmcr = ctxt.gic_vmcr;
+    v->arch.gic_apr = ctxt.gic_apr;
+    memcpy( v->arch.gic_lr, ctxt.gic_lr, sizeof(v->arch.gic_lr) );
+    v->arch.event_mask = ctxt.event_mask;
+    v->arch.lr_mask = ctxt.lr_mask;
+
+    return 0;
+}
+
+HVM_REGISTER_SAVE_RESTORE(GIC, gic_save, gic_load, 1, HVMSR_PER_VCPU);
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 0dc2050..956cf29 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -60,6 +60,8 @@ subdir-$(CONFIG_COMPAT) += compat
 
 subdir-$(x86_64) += hvm
 
+subdir-$(CONFIG_ARM) += hvm
+
 subdir-$(coverage) += gcov
 
 subdir-y += libelf
diff --git a/xen/include/asm-arm/hvm/support.h b/xen/include/asm-arm/hvm/support.h
new file mode 100644
index 0000000..33390b0
--- /dev/null
+++ b/xen/include/asm-arm/hvm/support.h
@@ -0,0 +1,29 @@
+/*
+ * support.h: HVM support routines used by ARMv7 VE.
+ *
+ * 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, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ */
+
+#ifndef __ASM_ARM_HVM_SUPPORT_H__
+#define __ASM_ARM_HVM_SUPPORT_H__
+
+#include <xen/types.h>
+#include <public/hvm/ioreq.h>
+#include <xen/sched.h>
+#include <xen/hvm/save.h>
+#include <asm/processor.h>
+
+#endif /* __ASM_ARM_HVM_SUPPORT_H__ */
diff --git a/xen/include/public/arch-arm/hvm/save.h b/xen/include/public/arch-arm/hvm/save.h
index 570809e..bbd9dba 100644
--- a/xen/include/public/arch-arm/hvm/save.h
+++ b/xen/include/public/arch-arm/hvm/save.h
@@ -42,6 +42,26 @@ struct hvm_save_header {
 
 DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header);
 
+
+/*
+ * GIC 
+ */
+struct hvm_hw_gic {
+    uint32_t gic_hcr;
+    uint32_t gic_vmcr;
+    uint32_t gic_apr;
+    uint32_t gic_lr[64];
+    uint64_t event_mask;
+    uint64_t lr_mask;
+};
+
+DECLARE_HVM_SAVE_TYPE(GIC, 3, struct hvm_hw_gic); /* leave 2 for CPU */
+
+/*  
+ * Largest type-code in use
+ */ 
+#define HVM_SAVE_CODE_MAX 19
+
 #endif
 
 /*
-- 
1.7.9.5

Re: [PATCH RFC 3/4] Implement save and restore for gic (template impl)

[Ian Campbell]

> @@ -65,3 +66,53 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
>  
>      return rc;
>  }
> +
> +static int gic_save(struct domain *d, hvm_domain_context_t *h)
> +{
> +    struct hvm_hw_gic ctxt;
> +    struct vcpu *v;
> +
> +    /* Save the state of GICs */
> +    for_each_vcpu( d, v )
> +    {
> +        ctxt.gic_hcr = v->arch.gic_hcr;
> +        ctxt.gic_vmcr = v->arch.gic_vmcr;
> +        ctxt.gic_apr = v->arch.gic_apr;
> +        memcpy( ctxt.gic_lr, v->arch.gic_lr, sizeof(v->arch.gic_lr) );
> +        ctxt.event_mask = v->arch.event_mask;
> +        ctxt.lr_mask = v->arch.lr_mask;

In general on x86 the policy is that hvm headers should store only
architectural state. I think this is a good policy to carry over to ARM.

With that in mind I don't think either event_mask or lr_mask are
architectural state but are actually internal state of the vgic
"emulation" which can and should be reconstructed when loading the
architectural state.

Ian