Re: [Xen-devel] [PATCH 10/13] Nested Virtualization: interrupts (svm specific)

[Christoph Egger <Christoph.Egger@xxxxxxx>]

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