|
[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 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;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |