Re: [Xen-devel] [RFC v2 11/15] vmx: Add a global wake-up vector for VT-d Posted-Interrupts

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.05.15 at 11:07, <feng.wu@xxxxxxxxx> wrote:
> @@ -3262,6 +3271,28 @@ void vmx_vmenter_helper(const struct cpu_user_regs 
> *regs)
>  }
>  
>  /*
> + * Handle VT-d posted-interrupt when VCPU is blocked.
> + */
> +void pi_wakeup_interrupt(struct cpu_user_regs *regs)

static (and perhaps move it up so you don't need to forward declare it).

> +{
> +    struct vcpu *v;
> +    unsigned int cpu = smp_processor_id();
> +
> +    spin_lock(&per_cpu(blocked_vcpu_lock, cpu));
> +    list_for_each_entry(v, &per_cpu(blocked_vcpu, cpu),
> +                    blocked_vcpu_list) {

How long do you think such a list can grow? I'm afraid you might
be adding quite a bit of latency here to the system.

> +        struct pi_desc *pi_desc = &v->arch.hvm_vmx.pi_desc;
> +
> +        if ( pi_desc->on == 1 )

Isn't this a single-bit (i.e. boolean) field? In which case - no reason
to compare with 1.

I also don't see the value of the local variable - it's being used only
once, and without it the line wouldn't get overly long or unreadable.

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -148,6 +148,8 @@ struct vcpu
>  
>      struct vcpu     *next_in_list;
>  
> +    struct list_head blocked_vcpu_list;

Again - why here instead of in VT-d/VMX specific structures?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel