Re: [Xen-devel] [PATCH for-4.9 v4 1/2] x86/vioapic: introduce support for multiple vIO APICS

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.04.17 at 11:23, <roger.pau@xxxxxxxxxx> wrote:
> +static unsigned int base_gsi(const struct domain *d,
> +                             const struct hvm_vioapic *vioapic)
> +{
> +    unsigned int nr_vioapics = d->arch.hvm_domain.nr_vioapics;
> +    unsigned int base_gsi = 0, i = 0;
> +    const struct hvm_vioapic *tmp;
> +
> +    while ( --nr_vioapics && (tmp = domain_vioapic(d, i++)) != vioapic )
> +        base_gsi += tmp->nr_pins;

While for valid input it should be benign, I think the first part of the
condition would better use post-decrement (or "i < nr_vioapics").
That way you'll return an out of range GSI for a not found vioapic
input.

> @@ -454,32 +523,68 @@ HVM_REGISTER_SAVE_RESTORE(IOAPIC, ioapic_save, 
> ioapic_load, 1, HVMSR_PER_DOM);
>  
>  void vioapic_reset(struct domain *d)
>  {
> -    struct hvm_vioapic *vioapic = domain_vioapic(d);
> -    uint32_t nr_pins = vioapic->nr_pins;
> -    int i;
> +    unsigned int i;
>  
>      if ( !has_vioapic(d) )
> +    {
> +        ASSERT(!d->arch.hvm_domain.nr_vioapics);
>          return;
> +    }
>  
> -    memset(vioapic, 0, hvm_vioapic_size(nr_pins));
> -    vioapic->domain = d;
> -    vioapic->nr_pins = nr_pins;
> -    for ( i = 0; i < nr_pins; i++ )
> -        vioapic->redirtbl[i].fields.mask = 1;
> -    vioapic->base_address = VIOAPIC_DEFAULT_BASE_ADDRESS;
> +    for ( i = 0; i < d->arch.hvm_domain.nr_vioapics; i++ )
> +    {
> +        struct hvm_vioapic *vioapic = domain_vioapic(d, i);
> +        unsigned int pin, nr_pins = vioapic->nr_pins;
> +
> +        memset(vioapic, 0, hvm_vioapic_size(nr_pins));
> +        for ( pin = 0; pin < nr_pins; pin++ )
> +            vioapic->redirtbl[pin].fields.mask = 1;
> +        ASSERT(!i);
> +        vioapic->base_address = VIOAPIC_DEFAULT_BASE_ADDRESS +
> +                                VIOAPIC_MEM_LENGTH * 0;

Why not simply

        vioapic->base_address = VIOAPIC_DEFAULT_BASE_ADDRESS;

?

With these taken care of (which can be done while committing, if you
agree)
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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