This series is *very* RFC at the moment - it is only compile tested. I would very much appreciate review on the design. (and I shall see about sorting out some dev testing tomorrow) The ultimate purpose is to fix that the fact that XEN_DOMCTL_gethvmcontext_partial is very unreliable at providing the correct subset of information. All the changes are supposed to be internal to Xen. There should be no difference at all to the result of XEN_DOMCTL_gethvmcontext (which can be used as a useful dev test for the validity of the changes). George: As for whether this is intended for 4.5, I have no idea. On the one hand, it is fixing a hypercall which doesn''t function correctly, but on the other hand, it is a very large and complicated set of changes relevant to migration. Then again, the majority of the changes were largely mechanical, and there is a simple test to prove whether there are bugs or not. I shall defer to others for comments on the risk/rewards of this series. Andrew Cooper (3): xen/hvm-save: Refactor HVM_REGISTER_SAVE_RESTORE xen/hvm-save: Extend hvm_save_handler to take an instance parameter xen/hvm-save: Adjust calling of multi-instance save handlers. xen/arch/x86/cpu/mcheck/vmce.c | 34 +++--- xen/arch/x86/hvm/hpet.c | 8 +- xen/arch/x86/hvm/hvm.c | 249 ++++++++++++++++++++-------------------- xen/arch/x86/hvm/i8254.c | 8 +- xen/arch/x86/hvm/irq.c | 24 ++-- xen/arch/x86/hvm/mtrr.c | 61 +++++----- xen/arch/x86/hvm/pmtimer.c | 8 +- xen/arch/x86/hvm/rtc.c | 8 +- xen/arch/x86/hvm/vioapic.c | 7 +- xen/arch/x86/hvm/viridian.c | 31 ++--- xen/arch/x86/hvm/vlapic.c | 38 ++---- xen/arch/x86/hvm/vpic.c | 18 +-- xen/common/hvm/save.c | 108 ++++++++++------- xen/include/xen/hvm/save.h | 65 +++++++---- 14 files changed, 337 insertions(+), 330 deletions(-) Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> CC: Keir Fraser <keir@xen.org> CC: Jan Beulich <JBeulich@suse.com> CC: Don Slutz <dslutz@verizon.com> CC: George Dunlap <george.dunlap@eu.citrix.com> -- 1.7.10.4
Andrew Cooper
2013-Dec-16 02:17 UTC
[RFC PATCH 1/3] xen/hvm-save: Refactor HVM_REGISTER_SAVE_RESTORE
The instance id in a save record served two purposes. For a PER_VCPU record, it was the VCPU id while for a PER_DOM it was just an index. As the number of instances needs to be stored to help fix hvm_save_one() later in this series, refactor HVM_REGISTER_SAVE_RESTORE to simplify the interface and prevent the buggy case of registering a PER_VCPU record with multiple instances. The ''kind'' can now be inferred from the number of instances. There is now HVM_REGISTER_SAVE_RESTORE_PER_DOM() and HVM_REGISTER_SAVE_RESTORE_PER_VCPU() which both take fewer arguments, and only PER_DOM() allows setting a number of instances. There is no observable change as a result of this patch. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> CC: Keir Fraser <keir@xen.org> CC: Jan Beulich <JBeulich@suse.com> CC: Don Slutz <dslutz@verizon.com> CC: George Dunlap <george.dunlap@eu.citrix.com> --- xen/arch/x86/cpu/mcheck/vmce.c | 4 ++-- xen/arch/x86/hvm/hpet.c | 2 +- xen/arch/x86/hvm/hvm.c | 11 +++++------ xen/arch/x86/hvm/i8254.c | 2 +- xen/arch/x86/hvm/irq.c | 9 +++------ xen/arch/x86/hvm/mtrr.c | 3 +-- xen/arch/x86/hvm/pmtimer.c | 3 +-- xen/arch/x86/hvm/rtc.c | 2 +- xen/arch/x86/hvm/vioapic.c | 2 +- xen/arch/x86/hvm/viridian.c | 8 ++++---- xen/arch/x86/hvm/vlapic.c | 6 ++---- xen/arch/x86/hvm/vpic.c | 2 +- xen/common/hvm/save.c | 13 ++++++++----- xen/include/xen/hvm/save.h | 35 ++++++++++++++++++++--------------- 14 files changed, 51 insertions(+), 51 deletions(-) diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c index f6c35db..a88368a 100644 --- a/xen/arch/x86/cpu/mcheck/vmce.c +++ b/xen/arch/x86/cpu/mcheck/vmce.c @@ -335,8 +335,8 @@ static int vmce_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h) return err ?: vmce_restore_vcpu(v, &ctxt); } -HVM_REGISTER_SAVE_RESTORE(VMCE_VCPU, vmce_save_vcpu_ctxt, - vmce_load_vcpu_ctxt, 1, HVMSR_PER_VCPU); +HVM_REGISTER_SAVE_RESTORE_PER_VCPU(VMCE_VCPU, vmce_save_vcpu_ctxt, + vmce_load_vcpu_ctxt); /* * for Intel MCE, broadcast vMCE to all vcpus diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c index 4324b52..fb2c098 100644 --- a/xen/arch/x86/hvm/hpet.c +++ b/xen/arch/x86/hvm/hpet.c @@ -569,7 +569,7 @@ static int hpet_load(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(HPET, hpet_save, hpet_load, 1, HVMSR_PER_DOM); +HVM_REGISTER_SAVE_RESTORE_PER_DOM(HPET, hpet_save, hpet_load, 1); void hpet_init(struct vcpu *v) { diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 69f7e74..eb21fc4 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -699,8 +699,8 @@ static int hvm_load_tsc_adjust(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(TSC_ADJUST, hvm_save_tsc_adjust, - hvm_load_tsc_adjust, 1, HVMSR_PER_VCPU); +HVM_REGISTER_SAVE_RESTORE_PER_VCPU(TSC_ADJUST, hvm_save_tsc_adjust, + hvm_load_tsc_adjust); static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h) { @@ -999,8 +999,7 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(CPU, hvm_save_cpu_ctxt, hvm_load_cpu_ctxt, - 1, HVMSR_PER_VCPU); +HVM_REGISTER_SAVE_RESTORE_PER_VCPU(CPU, hvm_save_cpu_ctxt, hvm_load_cpu_ctxt); #define HVM_CPU_XSAVE_SIZE(xcr0) (offsetof(struct hvm_hw_cpu_xsave, \ save_area) + \ @@ -1136,9 +1135,9 @@ static int __init __hvm_register_CPU_XSAVE_save_and_restore(void) "CPU_XSAVE", hvm_save_cpu_xsave_states, hvm_load_cpu_xsave_states, + 0, HVM_CPU_XSAVE_SIZE(xfeature_mask) + - sizeof(struct hvm_save_descriptor), - HVMSR_PER_VCPU); + sizeof(struct hvm_save_descriptor)); return 0; } __initcall(__hvm_register_CPU_XSAVE_save_and_restore); diff --git a/xen/arch/x86/hvm/i8254.c b/xen/arch/x86/hvm/i8254.c index c0d6bc2..139812a 100644 --- a/xen/arch/x86/hvm/i8254.c +++ b/xen/arch/x86/hvm/i8254.c @@ -423,7 +423,7 @@ static int pit_load(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(PIT, pit_save, pit_load, 1, HVMSR_PER_DOM); +HVM_REGISTER_SAVE_RESTORE_PER_DOM(PIT, pit_save, pit_load, 1); void pit_reset(struct domain *d) { diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c index 677fbcd..04ce739 100644 --- a/xen/arch/x86/hvm/irq.c +++ b/xen/arch/x86/hvm/irq.c @@ -658,9 +658,6 @@ static int irq_load_link(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(PCI_IRQ, irq_save_pci, irq_load_pci, - 1, HVMSR_PER_DOM); -HVM_REGISTER_SAVE_RESTORE(ISA_IRQ, irq_save_isa, irq_load_isa, - 1, HVMSR_PER_DOM); -HVM_REGISTER_SAVE_RESTORE(PCI_LINK, irq_save_link, irq_load_link, - 1, HVMSR_PER_DOM); +HVM_REGISTER_SAVE_RESTORE_PER_DOM(PCI_IRQ, irq_save_pci, irq_load_pci, 1); +HVM_REGISTER_SAVE_RESTORE_PER_DOM(ISA_IRQ, irq_save_isa, irq_load_isa, 1); +HVM_REGISTER_SAVE_RESTORE_PER_DOM(PCI_LINK, irq_save_link, irq_load_link, 1); diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c index 9937f5a..61c785c 100644 --- a/xen/arch/x86/hvm/mtrr.c +++ b/xen/arch/x86/hvm/mtrr.c @@ -677,8 +677,7 @@ static int hvm_load_mtrr_msr(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(MTRR, hvm_save_mtrr_msr, hvm_load_mtrr_msr, - 1, HVMSR_PER_VCPU); +HVM_REGISTER_SAVE_RESTORE_PER_VCPU(MTRR, hvm_save_mtrr_msr, hvm_load_mtrr_msr); uint8_t epte_get_entry_emt(struct domain *d, unsigned long gfn, mfn_t mfn, uint8_t *ipat, bool_t direct_mmio) diff --git a/xen/arch/x86/hvm/pmtimer.c b/xen/arch/x86/hvm/pmtimer.c index 01ae31d..282e8ee 100644 --- a/xen/arch/x86/hvm/pmtimer.c +++ b/xen/arch/x86/hvm/pmtimer.c @@ -292,8 +292,7 @@ static int pmtimer_load(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(PMTIMER, pmtimer_save, pmtimer_load, - 1, HVMSR_PER_DOM); +HVM_REGISTER_SAVE_RESTORE_PER_DOM(PMTIMER, pmtimer_save, pmtimer_load, 1); int pmtimer_change_ioport(struct domain *d, unsigned int version) { diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c index cdedefe..8d9d634 100644 --- a/xen/arch/x86/hvm/rtc.c +++ b/xen/arch/x86/hvm/rtc.c @@ -741,7 +741,7 @@ static int rtc_load(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(RTC, rtc_save, rtc_load, 1, HVMSR_PER_DOM); +HVM_REGISTER_SAVE_RESTORE_PER_DOM(RTC, rtc_save, rtc_load, 1); void rtc_reset(struct domain *d) { diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c index d3c681b..7c75192 100644 --- a/xen/arch/x86/hvm/vioapic.c +++ b/xen/arch/x86/hvm/vioapic.c @@ -428,7 +428,7 @@ static int ioapic_load(struct domain *d, hvm_domain_context_t *h) return hvm_load_entry(IOAPIC, h, s); } -HVM_REGISTER_SAVE_RESTORE(IOAPIC, ioapic_save, ioapic_load, 1, HVMSR_PER_DOM); +HVM_REGISTER_SAVE_RESTORE_PER_DOM(IOAPIC, ioapic_save, ioapic_load, 1); void vioapic_reset(struct domain *d) { diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c index 2b86d66..0ba85b3 100644 --- a/xen/arch/x86/hvm/viridian.c +++ b/xen/arch/x86/hvm/viridian.c @@ -449,8 +449,8 @@ static int viridian_load_domain_ctxt(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_DOMAIN, viridian_save_domain_ctxt, - viridian_load_domain_ctxt, 1, HVMSR_PER_DOM); +HVM_REGISTER_SAVE_RESTORE_PER_DOM(VIRIDIAN_DOMAIN, viridian_save_domain_ctxt, + viridian_load_domain_ctxt, 1); static int viridian_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h) { @@ -493,5 +493,5 @@ static int viridian_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_VCPU, viridian_save_vcpu_ctxt, - viridian_load_vcpu_ctxt, 1, HVMSR_PER_VCPU); +HVM_REGISTER_SAVE_RESTORE_PER_VCPU(VIRIDIAN_VCPU, viridian_save_vcpu_ctxt, + viridian_load_vcpu_ctxt); diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index bc06010..b64b9ee 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -1220,10 +1220,8 @@ static int lapic_load_regs(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(LAPIC, lapic_save_hidden, lapic_load_hidden, - 1, HVMSR_PER_VCPU); -HVM_REGISTER_SAVE_RESTORE(LAPIC_REGS, lapic_save_regs, lapic_load_regs, - 1, HVMSR_PER_VCPU); +HVM_REGISTER_SAVE_RESTORE_PER_VCPU(LAPIC, lapic_save_hidden, lapic_load_hidden); +HVM_REGISTER_SAVE_RESTORE_PER_VCPU(LAPIC_REGS, lapic_save_regs, lapic_load_regs); int vlapic_init(struct vcpu *v) { diff --git a/xen/arch/x86/hvm/vpic.c b/xen/arch/x86/hvm/vpic.c index fea3f68..e882fe1 100644 --- a/xen/arch/x86/hvm/vpic.c +++ b/xen/arch/x86/hvm/vpic.c @@ -398,7 +398,7 @@ static int vpic_load(struct domain *d, hvm_domain_context_t *h) return 0; } -HVM_REGISTER_SAVE_RESTORE(PIC, vpic_save, vpic_load, 2, HVMSR_PER_DOM); +HVM_REGISTER_SAVE_RESTORE_PER_DOM(PIC, vpic_save, vpic_load, 2); void vpic_reset(struct domain *d) { diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c index de76ada..2800c5b 100644 --- a/xen/common/hvm/save.c +++ b/xen/common/hvm/save.c @@ -36,15 +36,18 @@ static struct { hvm_load_handler load; const char *name; size_t size; - int kind; + unsigned int num; } hvm_sr_handlers [HVM_SAVE_CODE_MAX + 1] = {{NULL, NULL, "<?>"},}; +#define is_per_vcpu_handler(_h) ((_h).num == 0) + /* Init-time function to add entries to that list */ void __init hvm_register_savevm(uint16_t typecode, const char *name, hvm_save_handler save_state, hvm_load_handler load_state, - size_t size, int kind) + unsigned int num, + size_t size) { ASSERT(typecode <= HVM_SAVE_CODE_MAX); ASSERT(hvm_sr_handlers[typecode].save == NULL); @@ -53,7 +56,7 @@ void __init hvm_register_savevm(uint16_t typecode, hvm_sr_handlers[typecode].load = load_state; hvm_sr_handlers[typecode].name = name; hvm_sr_handlers[typecode].size = size; - hvm_sr_handlers[typecode].kind = kind; + hvm_sr_handlers[typecode].num = num; } size_t hvm_save_size(struct domain *d) @@ -67,7 +70,7 @@ size_t hvm_save_size(struct domain *d) /* Plus space for each thing we will be saving */ for ( i = 0; i <= HVM_SAVE_CODE_MAX; i++ ) - if ( hvm_sr_handlers[i].kind == HVMSR_PER_VCPU ) + if ( is_per_vcpu_handler(hvm_sr_handlers[i]) ) for_each_vcpu(d, v) sz += hvm_sr_handlers[i].size; else @@ -92,7 +95,7 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance, || hvm_sr_handlers[typecode].save == NULL ) return -EINVAL; - if ( hvm_sr_handlers[typecode].kind == HVMSR_PER_VCPU ) + if ( is_per_vcpu_handler(hvm_sr_handlers[typecode]) ) for_each_vcpu(d, v) sz += hvm_sr_handlers[typecode].size; else diff --git a/xen/include/xen/hvm/save.h b/xen/include/xen/hvm/save.h index ae6f0bb..0e3ef13 100644 --- a/xen/include/xen/hvm/save.h +++ b/xen/include/xen/hvm/save.h @@ -94,30 +94,35 @@ typedef int (*hvm_save_handler) (struct domain *d, typedef int (*hvm_load_handler) (struct domain *d, hvm_domain_context_t *h); -/* Init-time function to declare a pair of handlers for a type, - * and the maximum buffer space needed to save this type of state */ +/* Init-time function to declare a pair of handlers for a type, and the + * maximum buffer space needed to save this type of state. ''num'' of 0 + * indicates a per-vcpu record, while ''num'' of >0 indicates a per-domain + * record. */ void hvm_register_savevm(uint16_t typecode, const char *name, hvm_save_handler save_state, hvm_load_handler load_state, - size_t size, int kind); - -/* The space needed for saving can be per-domain or per-vcpu: */ -#define HVMSR_PER_DOM 0 -#define HVMSR_PER_VCPU 1 + unsigned int num, size_t size); /* Syntactic sugar around that function: specify the max number of * saves, and this calculates the size of buffer needed */ -#define HVM_REGISTER_SAVE_RESTORE(_x, _save, _load, _num, _k) \ +#define HVM_REGISTER_SAVE_RESTORE_PER_DOM(_x, _save, _load, _num) \ +static int __init __hvm_register_##_x##_save_and_restore(void) \ +{ \ + hvm_register_savevm( \ + HVM_SAVE_CODE(_x), #_x, &_save, &_load, _num, \ + (_num) * (HVM_SAVE_LENGTH(_x) \ + + sizeof (struct hvm_save_descriptor))); \ + return 0; \ +} \ +__initcall(__hvm_register_##_x##_save_and_restore); + +#define HVM_REGISTER_SAVE_RESTORE_PER_VCPU(_x, _save, _load) \ static int __init __hvm_register_##_x##_save_and_restore(void) \ { \ - hvm_register_savevm(HVM_SAVE_CODE(_x), \ - #_x, \ - &_save, \ - &_load, \ - (_num) * (HVM_SAVE_LENGTH(_x) \ - + sizeof (struct hvm_save_descriptor)), \ - _k); \ + hvm_register_savevm( \ + HVM_SAVE_CODE(_x), #_x, &_save, &_load, 0, \ + HVM_SAVE_LENGTH(_x) + sizeof (struct hvm_save_descriptor)); \ return 0; \ } \ __initcall(__hvm_register_##_x##_save_and_restore); -- 1.7.10.4
Andrew Cooper
2013-Dec-16 02:17 UTC
[RFC PATCH 2/3] xen/hvm-save: Extend hvm_save_handler to take an instance parameter
The per-domain save handlers take this new parameter and (other than the PIC which is the special case), use it in their hvm_save_entry() calls. The per-vcpu save handlers take this new parameter and currently ignore it. The return value for hvm_save_handler has been redefined to be more useful. All save handlers have been modified to return negative on failure. The per-domain ones return the number of bytes written, but the per-vcpu ones still return 0 on success. This involved tweaking hvm_save_entry() and _hvm_{init,write}_entry(), and most uses of them. The callsites for the save handlers currently have the instance parameter hardwired to 0, and have their error detection code suitably altered. There should be no observable difference as a result of this patch. There are a few scattered style fixes in the save functions. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Suggested-by: Jan Beulich <JBeulich@suse.com> CC: Keir Fraser <keir@xen.org> CC: Jan Beulich <JBeulich@suse.com> CC: Don Slutz <dslutz@verizon.com> CC: George Dunlap <george.dunlap@eu.citrix.com> --- xen/arch/x86/cpu/mcheck/vmce.c | 5 +++-- xen/arch/x86/hvm/hpet.c | 6 +++--- xen/arch/x86/hvm/hvm.c | 19 +++++++++++-------- xen/arch/x86/hvm/i8254.c | 6 +++--- xen/arch/x86/hvm/irq.c | 15 +++++++++------ xen/arch/x86/hvm/mtrr.c | 7 ++++--- xen/arch/x86/hvm/pmtimer.c | 5 +++-- xen/arch/x86/hvm/rtc.c | 6 ++++-- xen/arch/x86/hvm/vioapic.c | 5 +++-- xen/arch/x86/hvm/viridian.c | 12 +++++++----- xen/arch/x86/hvm/vlapic.c | 10 ++++++---- xen/arch/x86/hvm/vpic.c | 7 ++++--- xen/common/hvm/save.c | 15 ++++++++------- xen/include/xen/hvm/save.h | 30 ++++++++++++++++++++---------- 14 files changed, 88 insertions(+), 60 deletions(-) diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c index a88368a..8ef40c3 100644 --- a/xen/arch/x86/cpu/mcheck/vmce.c +++ b/xen/arch/x86/cpu/mcheck/vmce.c @@ -296,7 +296,8 @@ int vmce_wrmsr(uint32_t msr, uint64_t val) return ret; } -static int vmce_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h) +static int vmce_save_vcpu_ctxt(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct vcpu *v; int err = 0; @@ -309,7 +310,7 @@ static int vmce_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h) }; err = hvm_save_entry(VMCE_VCPU, v->vcpu_id, h, &ctxt); - if ( err ) + if ( err < 0 ) break; } diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c index fb2c098..fbde99d 100644 --- a/xen/arch/x86/hvm/hpet.c +++ b/xen/arch/x86/hvm/hpet.c @@ -468,7 +468,7 @@ const struct hvm_mmio_handler hpet_mmio_handler = { }; -static int hpet_save(struct domain *d, hvm_domain_context_t *h) +static int hpet_save(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { HPETState *hp = domain_vhpet(d); int rc; @@ -479,8 +479,8 @@ static int hpet_save(struct domain *d, hvm_domain_context_t *h) hp->hpet.mc64 = hp->mc_offset + guest_time_hpet(hp); /* Save the HPET registers */ - rc = _hvm_init_entry(h, HVM_SAVE_CODE(HPET), 0, HVM_SAVE_LENGTH(HPET)); - if ( rc == 0 ) + rc = _hvm_init_entry(h, HVM_SAVE_CODE(HPET), inst, HVM_SAVE_LENGTH(HPET)); + if ( rc >= 0 ) { struct hvm_hw_hpet *rec = (struct hvm_hw_hpet *)&h->data[h->cur]; h->cur += HVM_SAVE_LENGTH(HPET); diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index eb21fc4..925e792 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -662,7 +662,8 @@ void hvm_domain_destroy(struct domain *d) vioapic_deinit(d); } -static int hvm_save_tsc_adjust(struct domain *d, hvm_domain_context_t *h) +static int hvm_save_tsc_adjust(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct vcpu *v; struct hvm_tsc_adjust ctxt; @@ -672,7 +673,7 @@ static int hvm_save_tsc_adjust(struct domain *d, hvm_domain_context_t *h) { ctxt.tsc_adjust = v->arch.hvm_vcpu.msr_tsc_adjust; err = hvm_save_entry(TSC_ADJUST, v->vcpu_id, h, &ctxt); - if ( err ) + if ( err < 0 ) break; } @@ -702,7 +703,8 @@ static int hvm_load_tsc_adjust(struct domain *d, hvm_domain_context_t *h) HVM_REGISTER_SAVE_RESTORE_PER_VCPU(TSC_ADJUST, hvm_save_tsc_adjust, hvm_load_tsc_adjust); -static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h) +static int hvm_save_cpu_ctxt(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct vcpu *v; struct hvm_hw_cpu ctxt; @@ -806,8 +808,8 @@ static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h) ctxt.dr6 = v->arch.debugreg[6]; ctxt.dr7 = v->arch.debugreg[7]; - if ( hvm_save_entry(CPU, v->vcpu_id, h, &ctxt) != 0 ) - return 1; + if ( hvm_save_entry(CPU, v->vcpu_id, h, &ctxt) < 0 ) + return -ENOSPC; } return 0; } @@ -1005,7 +1007,8 @@ HVM_REGISTER_SAVE_RESTORE_PER_VCPU(CPU, hvm_save_cpu_ctxt, hvm_load_cpu_ctxt); save_area) + \ xstate_ctxt_size(xcr0)) -static int hvm_save_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h) +static int hvm_save_cpu_xsave_states(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct vcpu *v; struct hvm_hw_cpu_xsave *ctxt; @@ -1019,8 +1022,8 @@ static int hvm_save_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h) if ( !xsave_enabled(v) ) continue; - if ( _hvm_init_entry(h, CPU_XSAVE_CODE, v->vcpu_id, size) ) - return 1; + if ( _hvm_init_entry(h, CPU_XSAVE_CODE, v->vcpu_id, size) < 0 ) + return -ENOSPC; ctxt = (struct hvm_hw_cpu_xsave *)&h->data[h->cur]; h->cur += size; diff --git a/xen/arch/x86/hvm/i8254.c b/xen/arch/x86/hvm/i8254.c index 139812a..33e6990 100644 --- a/xen/arch/x86/hvm/i8254.c +++ b/xen/arch/x86/hvm/i8254.c @@ -382,14 +382,14 @@ void pit_stop_channel0_irq(PITState *pit) spin_unlock(&pit->lock); } -static int pit_save(struct domain *d, hvm_domain_context_t *h) +static int pit_save(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { PITState *pit = domain_vpit(d); int rc; spin_lock(&pit->lock); - - rc = hvm_save_entry(PIT, 0, h, &pit->hw); + + rc = hvm_save_entry(PIT, inst, h, &pit->hw); spin_unlock(&pit->lock); diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c index 04ce739..a23bd0a 100644 --- a/xen/arch/x86/hvm/irq.c +++ b/xen/arch/x86/hvm/irq.c @@ -524,7 +524,8 @@ static int __init dump_irq_info_key_init(void) } __initcall(dump_irq_info_key_init); -static int irq_save_pci(struct domain *d, hvm_domain_context_t *h) +static int irq_save_pci(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct hvm_irq *hvm_irq = &d->arch.hvm_domain.irq; unsigned int asserted, pdev, pintx; @@ -546,7 +547,7 @@ static int irq_save_pci(struct domain *d, hvm_domain_context_t *h) __hvm_pci_intx_deassert(d, pdev, pintx); /* Save PCI IRQ lines */ - rc = hvm_save_entry(PCI_IRQ, 0, h, &hvm_irq->pci_intx); + rc = hvm_save_entry(PCI_IRQ, inst, h, &hvm_irq->pci_intx); if ( asserted ) __hvm_pci_intx_assert(d, pdev, pintx); @@ -556,20 +557,22 @@ static int irq_save_pci(struct domain *d, hvm_domain_context_t *h) return rc; } -static int irq_save_isa(struct domain *d, hvm_domain_context_t *h) +static int irq_save_isa(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct hvm_irq *hvm_irq = &d->arch.hvm_domain.irq; /* Save ISA IRQ lines */ - return ( hvm_save_entry(ISA_IRQ, 0, h, &hvm_irq->isa_irq) ); + return hvm_save_entry(ISA_IRQ, inst, h, &hvm_irq->isa_irq); } -static int irq_save_link(struct domain *d, hvm_domain_context_t *h) +static int irq_save_link(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct hvm_irq *hvm_irq = &d->arch.hvm_domain.irq; /* Save PCI-ISA link state */ - return ( hvm_save_entry(PCI_LINK, 0, h, &hvm_irq->pci_link) ); + return hvm_save_entry(PCI_LINK, inst, h, &hvm_irq->pci_link); } static int irq_load_pci(struct domain *d, hvm_domain_context_t *h) diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c index 61c785c..40f58ed 100644 --- a/xen/arch/x86/hvm/mtrr.c +++ b/xen/arch/x86/hvm/mtrr.c @@ -600,7 +600,8 @@ int32_t hvm_set_mem_pinned_cacheattr( return 0; } -static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h) +static int hvm_save_mtrr_msr(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { int i; struct vcpu *v; @@ -631,8 +632,8 @@ static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h) hw_mtrr.msr_mtrr_fixed[i] ((uint64_t*)mtrr_state->fixed_ranges)[i]; - if ( hvm_save_entry(MTRR, v->vcpu_id, h, &hw_mtrr) != 0 ) - return 1; + if ( hvm_save_entry(MTRR, v->vcpu_id, h, &hw_mtrr) < 0 ) + return -ENOSPC; } return 0; } diff --git a/xen/arch/x86/hvm/pmtimer.c b/xen/arch/x86/hvm/pmtimer.c index 282e8ee..db27950 100644 --- a/xen/arch/x86/hvm/pmtimer.c +++ b/xen/arch/x86/hvm/pmtimer.c @@ -241,7 +241,8 @@ static int handle_pmt_io( return X86EMUL_UNHANDLEABLE; } -static int pmtimer_save(struct domain *d, hvm_domain_context_t *h) +static int pmtimer_save(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { PMTState *s = &d->arch.hvm_domain.pl_time.vpmt; uint32_t x, msb = s->pm.tmr_val & TMR_VAL_MSB; @@ -260,7 +261,7 @@ static int pmtimer_save(struct domain *d, hvm_domain_context_t *h) /* No point in setting the SCI here because we''ll already have saved the * IRQ and *PIC state; we''ll fix it up when we restore the domain */ - rc = hvm_save_entry(PMTIMER, 0, h, &s->pm); + rc = hvm_save_entry(PMTIMER, inst, h, &s->pm); spin_unlock(&s->lock); diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c index 8d9d634..e3fe472 100644 --- a/xen/arch/x86/hvm/rtc.c +++ b/xen/arch/x86/hvm/rtc.c @@ -702,13 +702,15 @@ void rtc_migrate_timers(struct vcpu *v) } /* Save RTC hardware state */ -static int rtc_save(struct domain *d, hvm_domain_context_t *h) +static int rtc_save(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { RTCState *s = domain_vrtc(d); int rc; + spin_lock(&s->lock); - rc = hvm_save_entry(RTC, 0, h, &s->hw); + rc = hvm_save_entry(RTC, inst, h, &s->hw); spin_unlock(&s->lock); + return rc; } diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c index 7c75192..3efd2d1 100644 --- a/xen/arch/x86/hvm/vioapic.c +++ b/xen/arch/x86/hvm/vioapic.c @@ -416,10 +416,11 @@ void vioapic_update_EOI(struct domain *d, int vector) spin_unlock(&d->arch.hvm_domain.irq_lock); } -static int ioapic_save(struct domain *d, hvm_domain_context_t *h) +static int ioapic_save(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { struct hvm_hw_vioapic *s = domain_vioapic(d); - return hvm_save_entry(IOAPIC, 0, h, s); + + return hvm_save_entry(IOAPIC, inst, h, s); } static int ioapic_load(struct domain *d, hvm_domain_context_t *h) diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c index 0ba85b3..4f9186d 100644 --- a/xen/arch/x86/hvm/viridian.c +++ b/xen/arch/x86/hvm/viridian.c @@ -423,7 +423,8 @@ out: return HVM_HCALL_completed; } -static int viridian_save_domain_ctxt(struct domain *d, hvm_domain_context_t *h) +static int viridian_save_domain_ctxt(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct hvm_viridian_domain_context ctxt; @@ -433,7 +434,7 @@ static int viridian_save_domain_ctxt(struct domain *d, hvm_domain_context_t *h) ctxt.hypercall_gpa = d->arch.hvm_domain.viridian.hypercall_gpa.raw; ctxt.guest_os_id = d->arch.hvm_domain.viridian.guest_os_id.raw; - return (hvm_save_entry(VIRIDIAN_DOMAIN, 0, h, &ctxt) != 0); + return hvm_save_entry(VIRIDIAN_DOMAIN, inst, h, &ctxt); } static int viridian_load_domain_ctxt(struct domain *d, hvm_domain_context_t *h) @@ -452,7 +453,8 @@ static int viridian_load_domain_ctxt(struct domain *d, hvm_domain_context_t *h) HVM_REGISTER_SAVE_RESTORE_PER_DOM(VIRIDIAN_DOMAIN, viridian_save_domain_ctxt, viridian_load_domain_ctxt, 1); -static int viridian_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h) +static int viridian_save_vcpu_ctxt(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct vcpu *v; @@ -464,8 +466,8 @@ static int viridian_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h) ctxt.apic_assist = v->arch.hvm_vcpu.viridian.apic_assist.raw; - if ( hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, &ctxt) != 0 ) - return 1; + if ( hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, &ctxt) < 0 ) + return -ENOSPC; } return 0; diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index b64b9ee..81dfd3f 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -1137,7 +1137,8 @@ static void lapic_rearm(struct vlapic *s) s->timer_last_update = s->pt.last_plt_gtime; } -static int lapic_save_hidden(struct domain *d, hvm_domain_context_t *h) +static int lapic_save_hidden(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct vcpu *v; struct vlapic *s; @@ -1146,14 +1147,15 @@ static int lapic_save_hidden(struct domain *d, hvm_domain_context_t *h) for_each_vcpu ( d, v ) { s = vcpu_vlapic(v); - if ( (rc = hvm_save_entry(LAPIC, v->vcpu_id, h, &s->hw)) != 0 ) + if ( (rc = hvm_save_entry(LAPIC, v->vcpu_id, h, &s->hw)) < 0 ) break; } return rc; } -static int lapic_save_regs(struct domain *d, hvm_domain_context_t *h) +static int lapic_save_regs(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct vcpu *v; struct vlapic *s; @@ -1162,7 +1164,7 @@ static int lapic_save_regs(struct domain *d, hvm_domain_context_t *h) for_each_vcpu ( d, v ) { s = vcpu_vlapic(v); - if ( (rc = hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, s->regs)) != 0 ) + if ( (rc = hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, s->regs)) < 0 ) break; } diff --git a/xen/arch/x86/hvm/vpic.c b/xen/arch/x86/hvm/vpic.c index e882fe1..7e4b64b 100644 --- a/xen/arch/x86/hvm/vpic.c +++ b/xen/arch/x86/hvm/vpic.c @@ -364,7 +364,8 @@ static int vpic_intercept_elcr_io( return X86EMUL_OKAY; } -static int vpic_save(struct domain *d, hvm_domain_context_t *h) +static int vpic_save(struct domain *d, uint16_t inst, + hvm_domain_context_t *h) { struct hvm_hw_vpic *s; int i; @@ -373,8 +374,8 @@ static int vpic_save(struct domain *d, hvm_domain_context_t *h) for ( i = 0; i < 2 ; i++ ) { s = &d->arch.hvm_domain.vpic[i]; - if ( hvm_save_entry(PIC, i, h, s) ) - return 1; + if ( hvm_save_entry(PIC, i, h, s) < 0 ) + return -ENOSPC; } return 0; diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c index 2800c5b..e9723e3 100644 --- a/xen/common/hvm/save.c +++ b/xen/common/hvm/save.c @@ -109,7 +109,7 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance, if ( !ctxt.data ) return -ENOMEM; - if ( hvm_sr_handlers[typecode].save(d, &ctxt) != 0 ) + if ( hvm_sr_handlers[typecode].save(d, 0, &ctxt) < 0 ) { printk(XENLOG_G_ERR "HVM%d save: failed to save type %"PRIu16"\n", d->domain_id, typecode); @@ -150,7 +150,7 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h) arch_hvm_save(d, &hdr); - if ( hvm_save_entry(HEADER, 0, h, &hdr) != 0 ) + if ( hvm_save_entry(HEADER, 0, h, &hdr) < 0 ) { printk(XENLOG_G_ERR "HVM%d save: failed to write header\n", d->domain_id); @@ -165,7 +165,7 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h) { printk(XENLOG_G_INFO "HVM%d save: %s\n", d->domain_id, hvm_sr_handlers[i].name); - if ( handler(d, h) != 0 ) + if ( handler(d, 0, h) < 0 ) { printk(XENLOG_G_ERR "HVM%d save: failed to save type %"PRIu16"\n", @@ -176,7 +176,7 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h) } /* Save an end-of-file marker */ - if ( hvm_save_entry(END, 0, h, &end) != 0 ) + if ( hvm_save_entry(END, 0, h, &end) < 0 ) { /* Run out of data */ printk(XENLOG_G_ERR "HVM%d save: no room for end marker\n", @@ -260,20 +260,21 @@ int _hvm_init_entry(struct hvm_domain_context *h, printk(XENLOG_G_WARNING "HVM save: no room for" " %"PRIu32" + %zu bytes for typecode %"PRIu16"\n", len, sizeof(*d), tc); - return -1; + return -ENOSPC; } d->typecode = tc; d->instance = inst; d->length = len; h->cur += sizeof(*d); - return 0; + return sizeof(*d); } -void _hvm_write_entry(struct hvm_domain_context *h, +int _hvm_write_entry(struct hvm_domain_context *h, void *src, uint32_t src_len) { memcpy(&h->data[h->cur], src, src_len); h->cur += src_len; + return src_len; } int _hvm_check_entry(struct hvm_domain_context *h, diff --git a/xen/include/xen/hvm/save.h b/xen/include/xen/hvm/save.h index 0e3ef13..b2dca48 100644 --- a/xen/include/xen/hvm/save.h +++ b/xen/include/xen/hvm/save.h @@ -30,21 +30,23 @@ typedef struct hvm_domain_context { uint8_t *data; } hvm_domain_context_t; -/* Marshalling an entry: check space and fill in the header */ +/* Marshalling an entry: check space and fill in the header. Returns the + * number of bytes written, or negative for an error. */ int _hvm_init_entry(struct hvm_domain_context *h, uint16_t tc, uint16_t inst, uint32_t len); -/* Marshalling: copy the contents in a type-safe way */ -void _hvm_write_entry(struct hvm_domain_context *h, - void *src, uint32_t src_len); +/* Marshalling: copy the contents in a type-safe way. Returns the number of + * bytes written. */ +int _hvm_write_entry(struct hvm_domain_context *h, + void *src, uint32_t src_len); /* Marshalling: init and copy; evaluates to zero on success */ #define hvm_save_entry(_x, _inst, _h, _src) ({ \ int r; \ r = _hvm_init_entry((_h), HVM_SAVE_CODE(_x), \ (_inst), HVM_SAVE_LENGTH(_x)); \ - if ( r == 0 ) \ - _hvm_write_entry((_h), (_src), HVM_SAVE_LENGTH(_x)); \ + if ( r >= 0 ) \ + r += _hvm_write_entry((_h), (_src), HVM_SAVE_LENGTH(_x)); \ r; }) /* Unmarshalling: test an entry''s size and typecode and record the instance */ @@ -85,11 +87,19 @@ static inline uint16_t hvm_load_instance(struct hvm_domain_context *h) return d->instance; } -/* Handler types for different types of save-file entry. - * The save handler may save multiple instances of a type into the buffer; - * the load handler will be called once for each instance found when - * restoring. Both return non-zero on error. */ +/* Handler types for different types of save-file entry. + * + * The save handler will be called once for each instance (either the number + * of per-domain instances, or for each vcpu). The caller ensures that the + * instance parameter is always valid in context (i.e. for a per-vcpu type, + * instance refers to a valid vcpu). It returns negative for an error, or the + * number of bytes written. + * + * The load handler will be called once for each instance found when + * restoring. It returns non-zero for an error. + */ typedef int (*hvm_save_handler) (struct domain *d, + uint16_t instance, hvm_domain_context_t *h); typedef int (*hvm_load_handler) (struct domain *d, hvm_domain_context_t *h); -- 1.7.10.4
Andrew Cooper
2013-Dec-16 02:17 UTC
[RFC PATCH 3/3] xen/hvm-save: Adjust calling of multi-instance save handlers.
Alter the calling logic so hvm_save and hvm_save_one are responsible for calling the save handlers with appropriate instance IDs (and are responsible for ensuring the validity of the instance parameter). This involves fairly substantial changes to each of the save handler bodies for records expecting to use multiple instances (all the per-vcpu ones, and the PIC record). Where sensible, refactoring has also involved changing the functions to write directly into the context buffer, rather than writing to a context structure on the stack and pointlessly copying. The only observable change should be that hvm_save_one now extracts the correct data in all cases, rather than being wrong for PIC records, variable length records and per-vcpu records when one or more vcpus are offline. There should be no difference whatsoever in the result from hvm_save. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Discovered-by: Don Slutz <dslutz@verizon.com> CC: Keir Fraser <keir@xen.org> CC: Jan Beulich <JBeulich@suse.com> CC: Don Slutz <dslutz@verizon.com> CC: George Dunlap <george.dunlap@eu.citrix.com> --- xen/arch/x86/cpu/mcheck/vmce.c | 27 ++--- xen/arch/x86/hvm/hvm.c | 225 +++++++++++++++++++--------------------- xen/arch/x86/hvm/mtrr.c | 55 +++++----- xen/arch/x86/hvm/viridian.c | 15 +-- xen/arch/x86/hvm/vlapic.c | 26 +---- xen/arch/x86/hvm/vpic.c | 13 +-- xen/common/hvm/save.c | 86 +++++++++------ 7 files changed, 213 insertions(+), 234 deletions(-) diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c index 8ef40c3..18ef18a 100644 --- a/xen/arch/x86/cpu/mcheck/vmce.c +++ b/xen/arch/x86/cpu/mcheck/vmce.c @@ -299,22 +299,23 @@ int vmce_wrmsr(uint32_t msr, uint64_t val) static int vmce_save_vcpu_ctxt(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { - struct vcpu *v; - int err = 0; + struct vcpu *v = d->vcpu[inst]; + int rc = 0; + struct hvm_vmce_vcpu *rec; - for_each_vcpu( d, v ) { - struct hvm_vmce_vcpu ctxt = { - .caps = v->arch.vmce.mcg_cap, - .mci_ctl2_bank0 = v->arch.vmce.bank[0].mci_ctl2, - .mci_ctl2_bank1 = v->arch.vmce.bank[1].mci_ctl2 - }; + rc = _hvm_init_entry(h, HVM_SAVE_CODE(VMCE_VCPU), + inst, HVM_SAVE_LENGTH(VMCE_VCPU)); + if ( rc < 0 ) + return rc; - err = hvm_save_entry(VMCE_VCPU, v->vcpu_id, h, &ctxt); - if ( err < 0 ) - break; - } + rec = (struct hvm_vmce_vcpu *)&h->data[h->cur]; + h->cur += HVM_SAVE_LENGTH(VMCE_VCPU); + + rec->caps = v->arch.vmce.mcg_cap; + rec->mci_ctl2_bank0 = v->arch.vmce.bank[0].mci_ctl2; + rec->mci_ctl2_bank1 = v->arch.vmce.bank[1].mci_ctl2; - return err; + return rc + HVM_SAVE_LENGTH(VMCE_VCPU); } static int vmce_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 925e792..bf1901a 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -665,19 +665,8 @@ void hvm_domain_destroy(struct domain *d) static int hvm_save_tsc_adjust(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { - struct vcpu *v; - struct hvm_tsc_adjust ctxt; - int err = 0; - - for_each_vcpu ( d, v ) - { - ctxt.tsc_adjust = v->arch.hvm_vcpu.msr_tsc_adjust; - err = hvm_save_entry(TSC_ADJUST, v->vcpu_id, h, &ctxt); - if ( err < 0 ) - break; - } - - return err; + return hvm_save_entry(TSC_ADJUST, inst, h, + &d->vcpu[inst]->arch.hvm_vcpu.msr_tsc_adjust); } static int hvm_load_tsc_adjust(struct domain *d, hvm_domain_context_t *h) @@ -706,112 +695,116 @@ HVM_REGISTER_SAVE_RESTORE_PER_VCPU(TSC_ADJUST, hvm_save_tsc_adjust, static int hvm_save_cpu_ctxt(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { - struct vcpu *v; - struct hvm_hw_cpu ctxt; + struct vcpu *v = d->vcpu[inst]; + struct hvm_hw_cpu *rec; struct segment_register seg; + int rc = 0; - 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. */ - if ( test_bit(_VPF_down, &v->pause_flags) ) - continue; + /* 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. */ + if ( test_bit(_VPF_down, &v->pause_flags) ) + return rc; - /* Architecture-specific vmcs/vmcb bits */ - hvm_funcs.save_cpu_ctxt(v, &ctxt); - - ctxt.msr_tsc_aux = hvm_msr_tsc_aux(v); - - hvm_get_segment_register(v, x86_seg_idtr, &seg); - ctxt.idtr_limit = seg.limit; - ctxt.idtr_base = seg.base; - - hvm_get_segment_register(v, x86_seg_gdtr, &seg); - ctxt.gdtr_limit = seg.limit; - ctxt.gdtr_base = seg.base; - - hvm_get_segment_register(v, x86_seg_cs, &seg); - ctxt.cs_sel = seg.sel; - ctxt.cs_limit = seg.limit; - ctxt.cs_base = seg.base; - ctxt.cs_arbytes = seg.attr.bytes; - - hvm_get_segment_register(v, x86_seg_ds, &seg); - ctxt.ds_sel = seg.sel; - ctxt.ds_limit = seg.limit; - ctxt.ds_base = seg.base; - ctxt.ds_arbytes = seg.attr.bytes; - - hvm_get_segment_register(v, x86_seg_es, &seg); - ctxt.es_sel = seg.sel; - ctxt.es_limit = seg.limit; - ctxt.es_base = seg.base; - ctxt.es_arbytes = seg.attr.bytes; - - hvm_get_segment_register(v, x86_seg_ss, &seg); - ctxt.ss_sel = seg.sel; - ctxt.ss_limit = seg.limit; - ctxt.ss_base = seg.base; - ctxt.ss_arbytes = seg.attr.bytes; - - hvm_get_segment_register(v, x86_seg_fs, &seg); - ctxt.fs_sel = seg.sel; - ctxt.fs_limit = seg.limit; - ctxt.fs_base = seg.base; - ctxt.fs_arbytes = seg.attr.bytes; - - hvm_get_segment_register(v, x86_seg_gs, &seg); - ctxt.gs_sel = seg.sel; - ctxt.gs_limit = seg.limit; - ctxt.gs_base = seg.base; - ctxt.gs_arbytes = seg.attr.bytes; - - hvm_get_segment_register(v, x86_seg_tr, &seg); - ctxt.tr_sel = seg.sel; - ctxt.tr_limit = seg.limit; - ctxt.tr_base = seg.base; - ctxt.tr_arbytes = seg.attr.bytes; - - hvm_get_segment_register(v, x86_seg_ldtr, &seg); - ctxt.ldtr_sel = seg.sel; - ctxt.ldtr_limit = seg.limit; - ctxt.ldtr_base = seg.base; - ctxt.ldtr_arbytes = seg.attr.bytes; - - if ( v->fpu_initialised ) - memcpy(ctxt.fpu_regs, v->arch.fpu_ctxt, sizeof(ctxt.fpu_regs)); - else - memset(ctxt.fpu_regs, 0, sizeof(ctxt.fpu_regs)); - - ctxt.rax = v->arch.user_regs.eax; - ctxt.rbx = v->arch.user_regs.ebx; - ctxt.rcx = v->arch.user_regs.ecx; - ctxt.rdx = v->arch.user_regs.edx; - ctxt.rbp = v->arch.user_regs.ebp; - ctxt.rsi = v->arch.user_regs.esi; - ctxt.rdi = v->arch.user_regs.edi; - ctxt.rsp = v->arch.user_regs.esp; - ctxt.rip = v->arch.user_regs.eip; - ctxt.rflags = v->arch.user_regs.eflags; - ctxt.r8 = v->arch.user_regs.r8; - ctxt.r9 = v->arch.user_regs.r9; - ctxt.r10 = v->arch.user_regs.r10; - ctxt.r11 = v->arch.user_regs.r11; - ctxt.r12 = v->arch.user_regs.r12; - ctxt.r13 = v->arch.user_regs.r13; - ctxt.r14 = v->arch.user_regs.r14; - ctxt.r15 = v->arch.user_regs.r15; - ctxt.dr0 = v->arch.debugreg[0]; - ctxt.dr1 = v->arch.debugreg[1]; - ctxt.dr2 = v->arch.debugreg[2]; - ctxt.dr3 = v->arch.debugreg[3]; - ctxt.dr6 = v->arch.debugreg[6]; - ctxt.dr7 = v->arch.debugreg[7]; - - if ( hvm_save_entry(CPU, v->vcpu_id, h, &ctxt) < 0 ) - return -ENOSPC; - } - return 0; + rc = _hvm_init_entry(h, HVM_SAVE_CODE(CPU), + inst, HVM_SAVE_LENGTH(CPU)); + if ( rc < 0 ) + return rc; + + rec = (struct hvm_hw_cpu *)&h->data[h->cur]; + h->cur += HVM_SAVE_LENGTH(CPU); + + /* Architecture-specific vmcs/vmcb bits */ + hvm_funcs.save_cpu_ctxt(v, rec); + + rec->msr_tsc_aux = hvm_msr_tsc_aux(v); + + hvm_get_segment_register(v, x86_seg_idtr, &seg); + rec->idtr_limit = seg.limit; + rec->idtr_base = seg.base; + + hvm_get_segment_register(v, x86_seg_gdtr, &seg); + rec->gdtr_limit = seg.limit; + rec->gdtr_base = seg.base; + + hvm_get_segment_register(v, x86_seg_cs, &seg); + rec->cs_sel = seg.sel; + rec->cs_limit = seg.limit; + rec->cs_base = seg.base; + rec->cs_arbytes = seg.attr.bytes; + + hvm_get_segment_register(v, x86_seg_ds, &seg); + rec->ds_sel = seg.sel; + rec->ds_limit = seg.limit; + rec->ds_base = seg.base; + rec->ds_arbytes = seg.attr.bytes; + + hvm_get_segment_register(v, x86_seg_es, &seg); + rec->es_sel = seg.sel; + rec->es_limit = seg.limit; + rec->es_base = seg.base; + rec->es_arbytes = seg.attr.bytes; + + hvm_get_segment_register(v, x86_seg_ss, &seg); + rec->ss_sel = seg.sel; + rec->ss_limit = seg.limit; + rec->ss_base = seg.base; + rec->ss_arbytes = seg.attr.bytes; + + hvm_get_segment_register(v, x86_seg_fs, &seg); + rec->fs_sel = seg.sel; + rec->fs_limit = seg.limit; + rec->fs_base = seg.base; + rec->fs_arbytes = seg.attr.bytes; + + hvm_get_segment_register(v, x86_seg_gs, &seg); + rec->gs_sel = seg.sel; + rec->gs_limit = seg.limit; + rec->gs_base = seg.base; + rec->gs_arbytes = seg.attr.bytes; + + hvm_get_segment_register(v, x86_seg_tr, &seg); + rec->tr_sel = seg.sel; + rec->tr_limit = seg.limit; + rec->tr_base = seg.base; + rec->tr_arbytes = seg.attr.bytes; + + hvm_get_segment_register(v, x86_seg_ldtr, &seg); + rec->ldtr_sel = seg.sel; + rec->ldtr_limit = seg.limit; + rec->ldtr_base = seg.base; + rec->ldtr_arbytes = seg.attr.bytes; + + if ( v->fpu_initialised ) + memcpy(rec->fpu_regs, v->arch.fpu_ctxt, sizeof(rec->fpu_regs)); + else + memset(rec->fpu_regs, 0, sizeof(rec->fpu_regs)); + + rec->rax = v->arch.user_regs.eax; + rec->rbx = v->arch.user_regs.ebx; + rec->rcx = v->arch.user_regs.ecx; + rec->rdx = v->arch.user_regs.edx; + rec->rbp = v->arch.user_regs.ebp; + rec->rsi = v->arch.user_regs.esi; + rec->rdi = v->arch.user_regs.edi; + rec->rsp = v->arch.user_regs.esp; + rec->rip = v->arch.user_regs.eip; + rec->rflags = v->arch.user_regs.eflags; + rec->r8 = v->arch.user_regs.r8; + rec->r9 = v->arch.user_regs.r9; + rec->r10 = v->arch.user_regs.r10; + rec->r11 = v->arch.user_regs.r11; + rec->r12 = v->arch.user_regs.r12; + rec->r13 = v->arch.user_regs.r13; + rec->r14 = v->arch.user_regs.r14; + rec->r15 = v->arch.user_regs.r15; + rec->dr0 = v->arch.debugreg[0]; + rec->dr1 = v->arch.debugreg[1]; + rec->dr2 = v->arch.debugreg[2]; + rec->dr3 = v->arch.debugreg[3]; + rec->dr6 = v->arch.debugreg[6]; + rec->dr7 = v->arch.debugreg[7]; + + return rc + HVM_SAVE_LENGTH(CPU); } static bool_t hvm_efer_valid(struct domain *d, diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c index 40f58ed..389138d 100644 --- a/xen/arch/x86/hvm/mtrr.c +++ b/xen/arch/x86/hvm/mtrr.c @@ -603,39 +603,40 @@ int32_t hvm_set_mem_pinned_cacheattr( static int hvm_save_mtrr_msr(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { - int i; - struct vcpu *v; - struct hvm_hw_mtrr hw_mtrr; - struct mtrr_state *mtrr_state; - /* save mtrr&pat */ - for_each_vcpu(d, v) - { - mtrr_state = &v->arch.hvm_vcpu.mtrr; + int i, rc; + struct vcpu *v = d->vcpu[inst]; + struct mtrr_state *mtrr_state = &v->arch.hvm_vcpu.mtrr; + struct hvm_hw_mtrr *rec; - hvm_get_guest_pat(v, &hw_mtrr.msr_pat_cr); + rc = _hvm_init_entry(h, HVM_SAVE_CODE(MTRR), + inst, HVM_SAVE_LENGTH(MTRR)); + if ( rc < 0 ) + return rc; - hw_mtrr.msr_mtrr_def_type = mtrr_state->def_type - | (mtrr_state->enabled << 10); - hw_mtrr.msr_mtrr_cap = mtrr_state->mtrr_cap; + rec = (struct hvm_hw_mtrr *)&h->data[h->cur]; + h->cur += HVM_SAVE_LENGTH(MTRR); - for ( i = 0; i < MTRR_VCNT; i++ ) - { - /* save physbase */ - hw_mtrr.msr_mtrr_var[i*2] - ((uint64_t*)mtrr_state->var_ranges)[i*2]; - /* save physmask */ - hw_mtrr.msr_mtrr_var[i*2+1] - ((uint64_t*)mtrr_state->var_ranges)[i*2+1]; - } + hvm_get_guest_pat(v, &rec->msr_pat_cr); - for ( i = 0; i < NUM_FIXED_MSR; i++ ) - hw_mtrr.msr_mtrr_fixed[i] - ((uint64_t*)mtrr_state->fixed_ranges)[i]; + rec->msr_mtrr_def_type = mtrr_state->def_type + | (mtrr_state->enabled << 10); + rec->msr_mtrr_cap = mtrr_state->mtrr_cap; - if ( hvm_save_entry(MTRR, v->vcpu_id, h, &hw_mtrr) < 0 ) - return -ENOSPC; + for ( i = 0; i < MTRR_VCNT; i++ ) + { + /* save physbase */ + rec->msr_mtrr_var[i*2] + ((uint64_t*)mtrr_state->var_ranges)[i*2]; + /* save physmask */ + rec->msr_mtrr_var[i*2+1] + ((uint64_t*)mtrr_state->var_ranges)[i*2+1]; } - return 0; + + for ( i = 0; i < NUM_FIXED_MSR; i++ ) + rec->msr_mtrr_fixed[i] + ((uint64_t*)mtrr_state->fixed_ranges)[i]; + + return rc + HVM_SAVE_LENGTH(MTRR); } static int hvm_load_mtrr_msr(struct domain *d, hvm_domain_context_t *h) diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c index 4f9186d..dfb01d5 100644 --- a/xen/arch/x86/hvm/viridian.c +++ b/xen/arch/x86/hvm/viridian.c @@ -456,21 +456,12 @@ HVM_REGISTER_SAVE_RESTORE_PER_DOM(VIRIDIAN_DOMAIN, viridian_save_domain_ctxt, static int viridian_save_vcpu_ctxt(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { - struct vcpu *v; - if ( !is_viridian_domain(d) ) return 0; - for_each_vcpu( d, v ) { - struct hvm_viridian_vcpu_context ctxt; - - ctxt.apic_assist = v->arch.hvm_vcpu.viridian.apic_assist.raw; - - if ( hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, &ctxt) < 0 ) - return -ENOSPC; - } - - return 0; + return hvm_save_entry( + VIRIDIAN_VCPU, inst, h, + &d->vcpu[inst]->arch.hvm_vcpu.viridian.apic_assist.raw); } static int viridian_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h) diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 81dfd3f..bb1438a 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -1140,35 +1140,17 @@ static void lapic_rearm(struct vlapic *s) static int lapic_save_hidden(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { - struct vcpu *v; - struct vlapic *s; - int rc = 0; - - for_each_vcpu ( d, v ) - { - s = vcpu_vlapic(v); - if ( (rc = hvm_save_entry(LAPIC, v->vcpu_id, h, &s->hw)) < 0 ) - break; - } + struct vlapic *s = vcpu_vlapic(d->vcpu[inst]); - return rc; + return hvm_save_entry(LAPIC, inst, h, &s->hw); } static int lapic_save_regs(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { - struct vcpu *v; - struct vlapic *s; - int rc = 0; - - for_each_vcpu ( d, v ) - { - s = vcpu_vlapic(v); - if ( (rc = hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, s->regs)) < 0 ) - break; - } + struct vlapic *s = vcpu_vlapic(d->vcpu[inst]); - return rc; + return hvm_save_entry(LAPIC_REGS, inst, h, &s->regs); } static int lapic_load_hidden(struct domain *d, hvm_domain_context_t *h) diff --git a/xen/arch/x86/hvm/vpic.c b/xen/arch/x86/hvm/vpic.c index 7e4b64b..416e970 100644 --- a/xen/arch/x86/hvm/vpic.c +++ b/xen/arch/x86/hvm/vpic.c @@ -367,18 +367,9 @@ static int vpic_intercept_elcr_io( static int vpic_save(struct domain *d, uint16_t inst, hvm_domain_context_t *h) { - struct hvm_hw_vpic *s; - int i; + struct hvm_hw_vpic *s = &d->arch.hvm_domain.vpic[inst]; - /* Save the state of both PICs */ - for ( i = 0; i < 2 ; i++ ) - { - s = &d->arch.hvm_domain.vpic[i]; - if ( hvm_save_entry(PIC, i, h, s) < 0 ) - return -ENOSPC; - } - - return 0; + return hvm_save_entry(PIC, inst, h, s); } static int vpic_load(struct domain *d, hvm_domain_context_t *h) diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c index e9723e3..8eb9672 100644 --- a/xen/common/hvm/save.c +++ b/xen/common/hvm/save.c @@ -84,44 +84,42 @@ size_t hvm_save_size(struct domain *d) int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance, XEN_GUEST_HANDLE_64(uint8) handle) { - int rv = 0; - size_t sz = 0; - struct vcpu *v; - hvm_domain_context_t ctxt = { 0, }; + int rv; + hvm_domain_context_t ctxt = { 0 }; - if ( d->is_dying - || typecode > HVM_SAVE_CODE_MAX - || hvm_sr_handlers[typecode].size < sizeof(struct hvm_save_descriptor) - || hvm_sr_handlers[typecode].save == NULL ) + if ( d->is_dying || typecode > HVM_SAVE_CODE_MAX ) return -EINVAL; - if ( is_per_vcpu_handler(hvm_sr_handlers[typecode]) ) - for_each_vcpu(d, v) - sz += hvm_sr_handlers[typecode].size; - else - sz = hvm_sr_handlers[typecode].size; - - if ( (instance + 1) * hvm_sr_handlers[typecode].size > sz ) + if ( hvm_sr_handlers[typecode].size < sizeof(struct hvm_save_descriptor) || + hvm_sr_handlers[typecode].save == NULL ) return -EINVAL; - ctxt.size = sz; - ctxt.data = xmalloc_bytes(sz); + if ( (is_per_vcpu_handler(hvm_sr_handlers[typecode]) && + (instance >= d->max_vcpus || d->vcpu[instance] == NULL)) || + (instance >= hvm_sr_handlers[typecode].num) ) + return -EBADSLT; + + ctxt.size = hvm_sr_handlers[typecode].size; + ctxt.data = xmalloc_bytes(hvm_sr_handlers[typecode].size); if ( !ctxt.data ) return -ENOMEM; - if ( hvm_sr_handlers[typecode].save(d, 0, &ctxt) < 0 ) + rv = hvm_sr_handlers[typecode].save(d, instance, &ctxt); + + if ( rv < 0 ) { - printk(XENLOG_G_ERR "HVM%d save: failed to save type %"PRIu16"\n", - d->domain_id, typecode); + printk(XENLOG_G_ERR "HVM%d save: failed to save type %"PRIu16 + ", instance %"PRIu16"\n", + d->domain_id, typecode, instance); rv = -EFAULT; } - else if ( copy_to_guest(handle, - ctxt.data - + (instance * hvm_sr_handlers[typecode].size) - + sizeof (struct hvm_save_descriptor), - hvm_sr_handlers[typecode].size - - sizeof (struct hvm_save_descriptor)) ) - rv = -EFAULT; + else if ( rv <= sizeof (struct hvm_save_descriptor) ) + rv = -ENODATA; + else + rv = copy_to_guest(handle, + ctxt.data + sizeof (struct hvm_save_descriptor), + rv - sizeof (struct hvm_save_descriptor)) + ? -EFAULT : 0; xfree(ctxt.data); return rv; @@ -165,13 +163,35 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h) { printk(XENLOG_G_INFO "HVM%d save: %s\n", d->domain_id, hvm_sr_handlers[i].name); - if ( handler(d, 0, h) < 0 ) + + if ( is_per_vcpu_handler(hvm_sr_handlers[i]) ) + { + struct vcpu *v; + + for_each_vcpu( d, v ) + if ( handler(d, v->vcpu_id, h) < 0 ) + { + printk(XENLOG_G_ERR + "HVM%d save: failed to save type %"PRIu16 + ", instance %"PRIu16"\n", + d->domain_id, i, v->vcpu_id); + return -EFAULT; + } + } + else { - printk(XENLOG_G_ERR - "HVM%d save: failed to save type %"PRIu16"\n", - d->domain_id, i); - return -EFAULT; - } + int j; + + for ( j = 0; j < hvm_sr_handlers[i].num; ++j ) + if ( handler(d, j, h) < 0 ) + { + printk(XENLOG_G_ERR + "HVM%d save: failed to save type %"PRIu16 + ", instance %"PRIu16"\n", + d->domain_id, i, j); + return -EFAULT; + } + } } } -- 1.7.10.4