[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] PVH Dom0 Intel IOMMU issues
On Mon, Apr 17, 2017 at 11:38:33AM +0100, Roger Pau Monné wrote: >On Mon, Apr 17, 2017 at 10:49:45AM +0800, Chao Gao wrote: >> On Mon, Apr 17, 2017 at 09:38:54AM +0100, Roger Pau Monné wrote: >> >On Mon, Apr 17, 2017 at 09:03:12AM +0800, Chao Gao wrote: >> >> On Mon, Apr 17, 2017 at 08:47:48AM +0100, Roger Pau Monné wrote: >> >> >On Mon, Apr 17, 2017 at 07:32:45AM +0800, Chao Gao wrote: >> >> >> On Fri, Apr 14, 2017 at 04:34:41PM +0100, Roger Pau Monné wrote: >> >> >> >Hello, >> >> >> > >> >> >> >Although PVHv2 Dom0 is not yet finished, I've been trying the current >> >> >> >code on >> >> >> >different hardware, and found that with pre-Haswell Intel hardware >> >> >> >PVHv2 Dom0 >> >> >> >completely freezes the box when calling iommu_hwdom_init in >> >> >> >dom0_construct_pvh. >> >> >> >OTOH the same doesn't happen when using a newer CPU (ie: haswell or >> >> >> >newer). >> >> >> > >> >> >> >I'm not able to debug that in any meaningful way because the box >> >> >> >seems to lock >> >> >> >up completely, even the watchdog NMI stops working. Here is the boot >> >> >> >log, up to >> >> >> >the point where it freezes: >> >> >> >> >> >> I try "dom0=pvh" with my skylake. An assertion failed. Is it a >> >> >> software bug? >> >> >> >> > >> >It seems like we are not properly adding/accounting the vIO APICs, but I >> >cannot >> >really see how. I have another patch for you to try below. >> > >> >Thanks, Roger. >> > >> >---8<--- >> > diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c >> >index 527ac2aadd..40075e2756 100644 >> >--- a/xen/arch/x86/hvm/vioapic.c >> >+++ b/xen/arch/x86/hvm/vioapic.c >> >@@ -610,11 +610,15 @@ int vioapic_init(struct domain *d) >> > xzalloc_array(struct hvm_vioapic *, nr_vioapics)) == NULL) ) >> > return -ENOMEM; >> > >> >+ printk("Adding %u vIO APICs\n", nr_vioapics); >> >+ >> > for ( i = 0; i < nr_vioapics; i++ ) >> > { >> > unsigned int nr_pins = is_hardware_domain(d) ? >> > nr_ioapic_entries[i] : >> > ARRAY_SIZE(domain_vioapic(d, 0)->domU.redirtbl); >> > >> >+ printk("vIO APIC %u has %u pins\n", i, nr_pins); >> >+ >> > if ( (domain_vioapic(d, i) = >> > xmalloc_bytes(hvm_vioapic_size(nr_pins))) == NULL ) >> > { >> >@@ -623,8 +627,12 @@ int vioapic_init(struct domain *d) >> > } >> > domain_vioapic(d, i)->nr_pins = nr_pins; >> > nr_gsis += nr_pins; >> >+ printk("nr_gsis: %u\n", nr_gsis); >> > } >> > >> >+ printk("domain nr_gsis: %u vioapic gsis: %u nr_irqs_gsi: %u >> >highest_gsi: %u\n", >> >+ hvm_domain_irq(d)->nr_gsis, nr_gsis, nr_irqs_gsi, >> >highest_gsi()); >> >+ >> > ASSERT(hvm_domain_irq(d)->nr_gsis == nr_gsis); >> > >> > d->arch.hvm_domain.nr_vioapics = nr_vioapics; >> > >> >> Please Cc or To me. Is there some holes in all physical IOAPICs gsi ranges? > >That's weird, my MUA (Mutt) seems to automatically remove your address from the >"To:" field. I have no idea why it does that. > >So yes, your box has as GSI gap which is not handled by any IO APIC. TBH, I >didn't even knew that was possible. In any case, patch below should solve it. > >---8<--- >commit f52d05fca03440d771eb56077c9d60bb630eb423 >diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c >index 5157db7a4e..ec87a97651 100644 >--- a/xen/arch/x86/hvm/vioapic.c >+++ b/xen/arch/x86/hvm/vioapic.c >@@ -64,37 +64,23 @@ static struct hvm_vioapic *addr_vioapic(const struct >domain *d, > struct hvm_vioapic *gsi_vioapic(const struct domain *d, unsigned int gsi, > unsigned int *pin) > { >- unsigned int i, base_gsi = 0; >+ unsigned int i; > > for ( i = 0; i < d->arch.hvm_domain.nr_vioapics; i++ ) > { > struct hvm_vioapic *vioapic = domain_vioapic(d, i); > >- if ( gsi >= base_gsi && gsi < base_gsi + vioapic->nr_pins ) >+ if ( gsi >= vioapic->base_gsi && >+ gsi < vioapic->base_gsi + vioapic->nr_pins ) > { >- *pin = gsi - base_gsi; >+ *pin = gsi - vioapic->base_gsi; > return vioapic; > } >- >- base_gsi += vioapic->nr_pins; > } > > return NULL; > } > >-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 ( i < nr_vioapics && (tmp = domain_vioapic(d, i++)) != vioapic ) >- base_gsi += tmp->nr_pins; >- >- return base_gsi; >-} >- > static uint32_t vioapic_read_indirect(const struct hvm_vioapic *vioapic) > { > uint32_t result = 0; >@@ -180,7 +166,7 @@ static void vioapic_write_redirent( > struct hvm_irq *hvm_irq = hvm_domain_irq(d); > union vioapic_redir_entry *pent, ent; > int unmasked = 0; >- unsigned int gsi = base_gsi(d, vioapic) + idx; >+ unsigned int gsi = vioapic->base_gsi + idx; > > spin_lock(&d->arch.hvm_domain.irq_lock); > >@@ -340,7 +326,7 @@ static void vioapic_deliver(struct hvm_vioapic *vioapic, >unsigned int pin) > struct domain *d = vioapic_domain(vioapic); > struct vlapic *target; > struct vcpu *v; >- unsigned int irq = base_gsi(d, vioapic) + pin; >+ unsigned int irq = vioapic->base_gsi + pin; > > ASSERT(spin_is_locked(&d->arch.hvm_domain.irq_lock)); > >@@ -451,7 +437,7 @@ void vioapic_update_EOI(struct domain *d, u8 vector) > { > struct hvm_irq *hvm_irq = hvm_domain_irq(d); > union vioapic_redir_entry *ent; >- unsigned int i, base_gsi = 0; >+ unsigned int i; > > ASSERT(has_vioapic(d)); > >@@ -473,19 +459,18 @@ void vioapic_update_EOI(struct domain *d, u8 vector) > if ( iommu_enabled ) > { > spin_unlock(&d->arch.hvm_domain.irq_lock); >- hvm_dpci_eoi(d, base_gsi + pin, ent); >+ hvm_dpci_eoi(d, vioapic->base_gsi + pin, ent); > spin_lock(&d->arch.hvm_domain.irq_lock); > } > > if ( (ent->fields.trig_mode == VIOAPIC_LEVEL_TRIG) && > !ent->fields.mask && >- hvm_irq->gsi_assert_count[base_gsi + pin] ) >+ hvm_irq->gsi_assert_count[vioapic->base_gsi + pin] ) > { > ent->fields.remote_irr = 1; > vioapic_deliver(vioapic, pin); > } > } >- base_gsi += vioapic->nr_pins; > } > > spin_unlock(&d->arch.hvm_domain.irq_lock); >@@ -554,6 +539,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 = io_apic_gsi_base(i); > } > vioapic->nr_pins = nr_pins; > vioapic->domain = d; >@@ -601,7 +587,12 @@ int vioapic_init(struct domain *d) > nr_gsis += nr_pins; > } > >- ASSERT(hvm_domain_irq(d)->nr_gsis == nr_gsis); >+ /* >+ * NB: hvm_domain_irq(d)->nr_gsis is actually the highest GSI + 1, but >+ * there might be holes in this range (ie: GSIs that don't belong to any >+ * vIO APIC). >+ */ >+ ASSERT(hvm_domain_irq(d)->nr_gsis >= nr_gsis); > > d->arch.hvm_domain.nr_vioapics = nr_vioapics; > vioapic_reset(d); >diff --git a/xen/include/asm-x86/hvm/vioapic.h >b/xen/include/asm-x86/hvm/vioapic.h >index 8ec91d2bd1..2ceb60eaef 100644 >--- a/xen/include/asm-x86/hvm/vioapic.h >+++ b/xen/include/asm-x86/hvm/vioapic.h >@@ -50,6 +50,7 @@ > struct hvm_vioapic { > struct domain *domain; > uint32_t nr_pins; >+ unsigned int base_gsi; > union { > XEN_HVM_VIOAPIC(,); > struct hvm_hw_vioapic domU; > It works. I can test for you when you send out a formal patch. Thanks Chao _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |