Re: [Xen-devel] [PATCH 3/5] x86/vioapic: introduce support for multiple vIO APICS

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.03.17 at 14:59, <roger.pau@xxxxxxxxxx> wrote:
> On Tue, Mar 21, 2017 at 07:45:13AM -0600, Jan Beulich wrote:
>> >>> On 21.03.17 at 11:52, <roger.pau@xxxxxxxxxx> wrote:
>> > On Tue, Mar 21, 2017 at 01:56:51AM -0600, Jan Beulich wrote:
>> >> >>> On 20.03.17 at 19:27, <roger.pau@xxxxxxxxxx> wrote:
>> >> > On Fri, Mar 03, 2017 at 10:06:03AM -0700, Jan Beulich wrote:
>> >> >> >>> On 23.02.17 at 12:52, <roger.pau@xxxxxxxxxx> wrote:
>> >> >> > @@ -91,13 +92,16 @@ static int pt_irq_vector(struct periodic_time 
>> >> >> > *pt, 
> enum hvm_intsrc src)
>> >> >> >                  + (isa_irq & 7));
>> >> >> >  
>> >> >> >      ASSERT(src == hvm_intsrc_lapic);
>> >> >> > -    return domain_vioapic(v->domain)->redirtbl[gsi].fields.vector;
>> >> >> > +    vioapic = gsi_vioapic(v->domain, gsi, &pin);
>> >> >> > +
>> >> >> > +    return vioapic->redirtbl[pin].fields.vector;
>> >> >> 
>> >> >> Please don't chance de-referencing NULL here and below.
>> >> > 
>> >> > Done, I've added an ASSERT.
>> >> 
>> >> How about release builds then?
>> > 
>> > OK, I can add a BUG_ON, but maybe it would be better to add a domain_crash 
>> > and
>> > suitable printk in case this triggers.
>> 
>> The latter, please, plus returning the spurious vector (as you still
>> need to return something).
> 
> Do we really need the domain_crash?

It's not strictly needed, but allowing the guest to continue to run
may make the (highly theoretical) issue harder to debug, the more
that ...

> I've added a gdprintk and returned -1.

... gdprintk() expands to nothing in release builds. Returning -1,
otoh, seems fine for all (two) present callers.

Jan


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