Re: [Xen-devel] [PATCH v7 13/17] Update IRTE according to guest interrupt config changes

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.09.15 at 10:29, <feng.wu@xxxxxxxxx> wrote:
> @@ -198,6 +199,103 @@ void free_hvm_irq_dpci(struct hvm_irq_dpci *dpci)
>      xfree(dpci);
>  }
>  
> +/*
> + * This routine handles lowest-priority interrupts using vector-hashing
> + * mechanism. As an example, modern Intel CPUs use this method to handle
> + * lowest-priority interrupts.
> + *
> + * Here is the details about the vector-hashing mechanism:
> + * 1. For lowest-priority interrupts, store all the possible destination
> + *    vCPUs in an array.
> + * 2. Use "gvec % max number of destination vCPUs" to find the right
> + *    destination vCPU in the array for the lowest-priority interrupt.
> + */
> +static struct vcpu *vector_hashing_dest(const struct domain *d,
> +                                        uint32_t dest_id,
> +                                        bool_t dest_mode,
> +                                        uint8_t gvec)
> +
> +{
> +    unsigned long *dest_vcpu_bitmap;
> +    unsigned int dest_vcpus = 0;
> +    unsigned int bitmap_array_size = BITS_TO_LONGS(d->max_vcpus);

You use the variable just once. Ditch it, or at least make it const.

> +    struct vcpu *v, *dest = NULL;
> +    unsigned int i;
> +
> +    dest_vcpu_bitmap = xzalloc_array(unsigned long, bitmap_array_size);
> +    if ( !dest_vcpu_bitmap )
> +        return NULL;
> +
> +    for_each_vcpu ( d, v )
> +    {
> +        if ( !vlapic_match_dest(vcpu_vlapic(v), NULL, APIC_DEST_NOSHORT,
> +                                dest_id, dest_mode) )
> +            continue;
> +
> +        __set_bit(v->vcpu_id, dest_vcpu_bitmap);
> +        dest_vcpus++;
> +    }
> +
> +    if ( dest_vcpus != 0 )
> +    {
> +        unsigned int mod = gvec % dest_vcpus;
> +        unsigned int idx = 0;
> +
> +        for ( i = 0; i <= mod; i++ )
> +        {
> +            idx = find_next_bit(dest_vcpu_bitmap, d->max_vcpus, idx) + 1;
> +            BUG_ON(idx >= d->max_vcpus);
> +        }
> +
> +        dest = d->vcpu[idx-1];

Blanks around the - please.

> +static struct vcpu *pi_find_dest_vcpu(const struct domain *d, uint32_t 
> dest_id,
> +                                      bool_t dest_mode, uint8_t 
> delivery_mode,
> +                                      uint8_t gvec)
> +{
> +    unsigned int dest_vcpus = 0;
> +    struct vcpu *v, *dest = NULL;
> +
> +    if ( delivery_mode == dest_LowestPrio )
> +        return vector_hashing_dest(d, dest_id, dest_mode, gvec);
> +    else if ( delivery_mode == dest_Fixed )

Pointless else. And perhaps this should be switch(delivery_mode)
anyway.

With those cosmetic issues addressed
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>


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