Re: [Xen-devel] [PATCH for-4.9 v3] x86/vioapic: allow holes in the GSI range for PVH Dom0

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Tue, Apr 18, 2017 at 05:52:47AM -0600, Jan Beulich wrote:
> >>> On 18.04.17 at 13:42, <roger.pau@xxxxxxxxxx> wrote:
> > @@ -538,7 +523,8 @@ void vioapic_reset(struct domain *d)
> >      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;
> > +        unsigned int nr_pins = vioapic->nr_pins, base_gsi = 
> > vioapic->base_gsi;
> > +        unsigned int pin;
> >  
> >          memset(vioapic, 0, hvm_vioapic_size(nr_pins));
> >          for ( pin = 0; pin < nr_pins; pin++ )
> > @@ -546,7 +532,7 @@ void vioapic_reset(struct domain *d)
> >  
> >          if ( !is_hardware_domain(d) )
> >          {
> > -            ASSERT(!i);
> > +            ASSERT(!i && base_gsi == 0);
> 
> If you really want to check this here, then please don't mix styles:
> Either ! in both cases or, less preferred, == 0.

Please fix that while committing.

> > @@ -554,6 +540,7 @@ void vioapic_reset(struct domain *d)
> >          {
> >              vioapic->base_address = mp_ioapics[i].mpc_apicaddr;
> >              vioapic->id = mp_ioapics[i].mpc_apicid;
> > +            vioapic->base_gsi = base_gsi;
> 
> Why did you leave this in place? There's no need to write back what
> you've just read.

vioapic_reset zeroes the whole hvm_vioapic structure, so this is needed in
order to set the base_gsi to it's value.

> I can offer to drop all three hunks left in place above while
> committing; with them dropped
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

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