WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] svm: support VMCB cleanbits
From: Christoph Egger <Christoph.Egger@xxxxxxx>
Date: Wed, 15 Dec 2010 13:36:57 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 15 Dec 2010 04:38:09 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20101215112751.GN9912@xxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <201012151152.54975.Christoph.Egger@xxxxxxx> <20101215112751.GN9912@xxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: KMail/1.9.10
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