Re: [Xen-devel] [PATCH v2 7/7] x86/vioapic: allow PVHv2 Dom0 to have more than one IO APIC

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.03.17 at 12:18, <roger.pau@xxxxxxxxxx> wrote:
> The base address, id and number of pins of the vIO APICs exposed to PVHv2 Dom0
> is the same as the values found on bare metal.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with ...

> --- a/xen/arch/x86/hvm/vioapic.c
> +++ b/xen/arch/x86/hvm/vioapic.c
> @@ -533,9 +533,19 @@ void vioapic_reset(struct domain *d)
>          memset(vioapic, 0, hvm_vioapic_size(nr_pins));
>          for ( j = 0; j < nr_pins; j++ )
>              vioapic->redirtbl[j].fields.mask = 1;
> -        vioapic->base_address = VIOAPIC_DEFAULT_BASE_ADDRESS +
> -                                VIOAPIC_MEM_LENGTH * i;
> -        vioapic->id = i;
> +
> +        if ( !is_hardware_domain(d) )
> +        {
> +            vioapic->base_address = VIOAPIC_DEFAULT_BASE_ADDRESS +
> +                                    VIOAPIC_MEM_LENGTH * i;
> +            vioapic->id = i;
> +        }

... this suitably adjusted according to the earlier requested change,
avoiding to leave bogus address calculations here. One option would
be to remove the dependency on i from the calculation, adding
ASSERT(!i).

> @@ -571,15 +583,21 @@ int vioapic_init(struct domain *d)
>  
>      for ( i = 0; i < nr_vioapics; i++ )
>      {
> +        unsigned int nr_pins = is_hardware_domain(d) ? nr_ioapic_entries[i]
> +                                                     : VIOAPIC_NUM_PINS;
> +
>          if ( (domain_vioapic(d, i) =
> -              xmalloc_bytes(hvm_vioapic_size(VIOAPIC_NUM_PINS))) == NULL )
> +              xmalloc_bytes(hvm_vioapic_size(nr_pins))) == NULL )
>          {
>              vioapic_free(d, nr_vioapics);
>              return -ENOMEM;
>          }
> -        domain_vioapic(d, i)->nr_pins = VIOAPIC_NUM_PINS;
> +        domain_vioapic(d, i)->nr_pins = nr_pins;
> +        nr_gsis += nr_pins;
>      }

Hmm, seeing this I can accept how patch 5 altered this function.
As per above this doesn't extend to ioapic_reset() though.

Jan

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