[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 3/5] x86/vioapic: introduce support for multiple vIO APICS
>>> On 23.02.17 at 12:52, <roger.pau@xxxxxxxxxx> wrote: > +struct hvm_hw_vioapic *gsi_vioapic(struct domain *d, unsigned int gsi, const struct domain *d ? > +static unsigned int pin_gsi(struct domain *d, struct hvm_hw_vioapic *vioapic, const for both? > + unsigned int pin) > +{ > + struct hvm_hw_vioapic *tmp; And here. Also wouldn't this function more naturally (i.e. more generally useful) simply return the base GSI, leaving it to the caller to add the pin number? > @@ -157,21 +210,22 @@ static void vioapic_write_redirent( > pent->fields.remote_irr = 0; > else if ( !ent.fields.mask && > !ent.fields.remote_irr && > - hvm_irq->gsi_assert_count[idx] ) > + hvm_irq->gsi_assert_count[gsi] ) Neither this nor any earlier patch modified the size of this array, afaics. > -static void vioapic_write_indirect(struct domain *d, uint32_t val) > +static void vioapic_write_indirect(struct domain *d, unsigned long addr, > + uint32_t val) > { > - struct hvm_hw_vioapic *vioapic = domain_vioapic(d); > + struct hvm_hw_vioapic *vioapic = addr_vioapic(d, addr); I think it would be better for the caller to pass the vioapic it already established (and ASSERT()ed). > @@ -430,11 +493,14 @@ void vioapic_update_EOI(struct domain *d, u8 vector) > > static int vioapic_save(struct domain *d, hvm_domain_context_t *h) > { > - struct hvm_hw_vioapic *vioapic = domain_vioapic(d); > + struct hvm_hw_vioapic *vioapic = domain_vioapic(d, 0); > > if ( !has_vioapic(d) ) > return 0; > > + if ( d->arch.hvm_domain.nr_vioapics != 1 ) > + return -ENOSYS; > + > if ( vioapic->nr_pins != VIOAPIC_NUM_PINS ) > return -ENOSYS; Perhaps merge the two if()s? Otherwise -EOPNOTSUPP again. > int vioapic_init(struct domain *d) > { > if ( !has_vioapic(d) ) > + { > + d->arch.hvm_domain.nr_vioapics = 0; I don't think this is needed - if anything you may want to again ASSERT() it being zero. > --- a/xen/arch/x86/hvm/vlapic.c > +++ b/xen/arch/x86/hvm/vlapic.c > @@ -1120,7 +1120,7 @@ static int __vlapic_accept_pic_intr(struct vcpu *v) > if ( !has_vioapic(d) ) > return 0; > > - redir0 = domain_vioapic(d)->redirtbl[0]; > + redir0 = domain_vioapic(d, 0)->redirtbl[0]; What if the first IO-APIC has less than 16 pins? > --- a/xen/arch/x86/hvm/vpt.c > +++ b/xen/arch/x86/hvm/vpt.c > @@ -78,7 +78,8 @@ void hvm_set_guest_time(struct vcpu *v, u64 guest_time) > static int pt_irq_vector(struct periodic_time *pt, enum hvm_intsrc src) > { > struct vcpu *v = pt->vcpu; > - unsigned int gsi, isa_irq; > + struct hvm_hw_vioapic *vioapic; > + unsigned int gsi, isa_irq, pin; > > if ( pt->source == PTSRC_lapic ) > return pt->irq; > @@ -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. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |