> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 29 March 2016 09:42 > To: Paul Durrant > Cc: xen-de...@lists.xenproject.org > Subject: Re: [PATCH] x86/hvm/viridian: save APIC assist vector > > >>> On 24.03.16 at 18:19, <paul.durr...@citrix.com> wrote: > > @@ -293,10 +292,11 @@ void viridian_start_apic_assist(struct vcpu *v, int > vector) > > * wrong and the VM will most likely hang so force a crash now > > * to make the problem clear. > > */ > > - if ( v->arch.hvm_vcpu.viridian.apic_assist.vector >= 0 ) > > + if ( v->arch.hvm_vcpu.viridian.apic_assist.vector != 0 ) > > domain_crash(v->domain); > > > > - v->arch.hvm_vcpu.viridian.apic_assist.vector = vector; > > + /* Increment the value so that zero is always invalid. */ > > + v->arch.hvm_vcpu.viridian.apic_assist.vector = vector + 1; > > From an APIC perspective, aren't vectors below 0x10 invalid > anyway? I.e. can't zero serve the "none" indication just as much > as -1 did without this new biasing? Or otherwise I'd still expect ... >
I can do that, if you're happy with it. If I'm going to do it this way though I will also put in an explicit check to make sure APIC assist is not used for vectors < 0x16. Paul > > @@ -829,13 +830,21 @@ static int viridian_load_vcpu_ctxt(struct domain > *d, hvm_domain_context_t *h) > > return -EINVAL; > > } > > > > - if ( hvm_load_entry(VIRIDIAN_VCPU, h, &ctxt) != 0 ) > > + if ( hvm_load_entry_zeroextend(VIRIDIAN_VCPU, h, &ctxt) != 0 ) > > return -EINVAL; > > > > - v->arch.hvm_vcpu.viridian.apic_assist.msr.raw = ctxt.apic_assist; > > + v->arch.hvm_vcpu.viridian.apic_assist.msr.raw = ctxt.apic_assist_msr; > > if ( v->arch.hvm_vcpu.viridian.apic_assist.msr.fields.enabled ) > > initialize_apic_assist(v); > > > > + /* > > + * Guests that were saved on a host that did not use APIC assist > > + * will clearly never have a pending assist, so reading the zero > > + * value for apic_assist_vector from the zero extended save record > > + * will always be correct. > > + */ > > + v->arch.hvm_vcpu.viridian.apic_assist.vector = ctxt.apic_assist_vector; > > ... the bias to get accounted for here, instead of quite a bit of > other code getting adjusted, and the meaning of the "vector" > field getting slightly mis-used. > > Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
