Re: [Xen-devel] [PATCH] svm: support VMCB cleanbits

[Christoph Egger <Christoph.Egger@xxxxxxx>]

On Wednesday 15 December 2010 12:27:51 Tim Deegan wrote:
> Hi,
>
> Thanks for the patch!
>
> At 10:52 +0000 on 15 Dec (1292410374), Christoph Egger wrote:
> > @@ -660,16 +671,17 @@ static void svm_ctxt_switch_to(struct vc
> >
> >  static void svm_do_resume(struct vcpu *v)
> >  {
> > +    struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
> >      bool_t debug_state = v->domain->debugger_attached;
> >
> >      if ( unlikely(v->arch.hvm_vcpu.debug_state_latch != debug_state) )
> >      {
> > -        uint32_t mask = (1U << TRAP_debug) | (1U << TRAP_int3);
> >          v->arch.hvm_vcpu.debug_state_latch = debug_state;
> >          if ( debug_state )
> > -            v->arch.hvm_svm.vmcb->exception_intercepts |= mask;
> > -        else
> > -            v->arch.hvm_svm.vmcb->exception_intercepts &= ~mask;
> > +        {
> > +            svm_set_exception_intercept(vmcb, TRAP_debug);
> > +            svm_set_exception_intercept(vmcb, TRAP_int3);
> > +        }
>
> This seems to change the logic so it doesn't clear the intercepts if
> debug_state == 0.  Is that OK?

No, that's not ok. I fixed that in the new patch.

> More generally, I'm not sure I like having all the VMCB accessor
> functions in files called "cleanbits" -- wouldn't it make sense to have
> all that in the vmcb files so people will see them and know to use them?
> You could rename the actual vmcb fields as well to catch anyone writing
> them directly, e.g. in forward-ported patches.

I renamed the 'svmcleanbits.[ch]' files to 'vmc_funcs.[ch]'

Thanks for your review.

Signed-off-by: Wei Huang <Wei.Huang2@xxxxxxx>
Signed-off-by: Christoph Egger <Christoph.Egger@xxxxxxx>

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Attachment: xen_vmcbcleanbits.diff
Description: xen_vmcbcleanbits.diff

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel