On Tuesday 16 November 2010 16:06:45 Tim Deegan wrote:
> At 18:44 +0000 on 12 Nov (1289587459), Christoph Egger wrote:
> > diff -r 3bfc06e2e41a -r b18448601670 xen/arch/x86/hvm/svm/nestedsvm.c
> > --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> > +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> > @@ -24,6 +24,30 @@
> > #include <asm/hvm/svm/nestedsvm.h>
> > #include <asm/hvm/svm/svmdebug.h>
> > #include <asm/paging.h> /* paging_mode_hap */
> > +#include <asm/event.h> /* for local_event_delivery_(en|dis)able */
> > +
> > +static int
> > +nestedsvm_vcpu_clgi(struct vcpu *v)
> > +{
> > + struct nestedsvm *svm = &vcpu_nestedsvm(v);
> > +
> > + /* clear gif flag */
> > + svm->ns_gif = 0;
> > + local_event_delivery_disable(); /* mask events for PV drivers */
>
> This function, and the stgi one below, can only operate safely on
> current; if you want to keep the argument for performance then maybe
> ASSERT(v == current) for sanity.
No, it's not about performance. It's about consistency.
Do you want me to remove the parameter?
> > + return 0;
>
> Also, since they only ever return 0, please make them return void
> instead.
Fixed.
>
> > +}
> > +
> > +static int
> > +nestedsvm_vcpu_stgi(struct vcpu *v)
> > +{
> > + struct nestedsvm *svm = &vcpu_nestedsvm(v);
> > +
> > + /* Always set the GIF to make hvm_interrupt_blocked work. */
> > + svm->ns_gif = 1;
> > +
> > + local_event_delivery_enable(); /* unmask events for PV drivers */
> > + return 0;
> > +}
> >
> > +void svm_vmexit_do_stgi(struct cpu_user_regs *regs, struct vcpu *v)
> > +{
> > + int ret;
> > + unsigned int inst_len;
> > +
> > + if ( !nestedhvm_enabled(v->domain) ) {
> > + hvm_inject_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE,
> > 0); + return;
> > + }
> > +
> > + if ( (inst_len = __get_instruction_length(v, INSTR_STGI)) == 0 )
> > + return;
> > +
> > + ret = nestedsvm_vcpu_stgi(v);
> > + if (ret)
> > + /* On failure, nestedsvm_vcpu_stgi injected an exception,
> > + * almost a #GP or #UD.
>
> No, it never fails and always returns 0. :) Likewise below.
Both fixed.
Christoph
>
> Cheers,
>
> Tim.
>
> > + */
> > + return;
> > +
> > + __update_guest_eip(regs, inst_len);
> > +}
> > +
> > +void svm_vmexit_do_clgi(struct cpu_user_regs *regs, struct vcpu *v)
> > +{
> > + int ret;
> > + struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
> > + unsigned int inst_len;
> > +
> > + if ( !nestedhvm_enabled(v->domain) ) {
> > + hvm_inject_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE,
> > 0); + return;
> > + }
> > +
> > + if ( (inst_len = __get_instruction_length(v, INSTR_CLGI)) == 0 )
> > + return;
> > +
> > + ret = nestedsvm_vcpu_clgi(v);
> > + if (ret)
> > + /* On failure, nestedsvm_vcpu_clgi injected an exception,
> > + * almost a #GP or #UD.
> > + */
> > + return;
> > +
> > + /* After a CLGI no interrupts should come */
> > + vmcb->vintr.fields.irq = 0;
> > + vmcb->general1_intercepts &= ~GENERAL1_INTERCEPT_VINTR;
> > +
> > + __update_guest_eip(regs, inst_len);
> > +}
--
---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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home