Re: [Xen-devel] [PATCH] x86/hvm/viridian: save APIC assist vector

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 29 March 2016 09:42
> To: Paul Durrant
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH] x86/hvm/viridian: save APIC assist vector
> 
> >>> On 24.03.16 at 18:19, <paul.durrant@xxxxxxxxxx> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel