From patchwork Mon Apr 17 10:38:33 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Roger_Pau_Monn=C3=A9?= X-Patchwork-Id: 9683719 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4CAE2602C2 for ; Mon, 17 Apr 2017 10:41:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 31C5626E4C for ; Mon, 17 Apr 2017 10:41:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2666126E73; Mon, 17 Apr 2017 10:41:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4973A26E4C for ; Mon, 17 Apr 2017 10:41:11 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d043O-0000UO-Bz; Mon, 17 Apr 2017 10:38:42 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d043M-0000UI-V4 for xen-devel@lists.xenproject.org; Mon, 17 Apr 2017 10:38:41 +0000 Received: from [85.158.143.35] by server-8.bemta-6.messagelabs.com id 5D/2A-03648-03B94F85; Mon, 17 Apr 2017 10:38:40 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrLIsWRWlGSWpSXmKPExsWyU9JRQld/9pc IgyV9khbft0xmcmD0OPzhCksAYxRrZl5SfkUCa8btLzeZC3qsKmZ8OsTcwHhEu4uRk0NCwE/i UPMGti5GDg4WAVWJfWvcQUw2AXuJ6V8rQEwRgXSJrztsQYqFBUwkns74ygJi8wp4SnTsWs7ex cjFISRwkUni6dMbbBAJQYmTM5+AFTEL6EncmDoFbDqzgLTE8n8cEGF5ieats5lBbE4BS4neFV fYQWxRARWJja9fgcWFBBQl+uc9YIM4Ml1i4rMelgmM/LOQbJiFZMMshA2zkGxYwMiyilGjOLW oLLVI18hIL6koMz2jJDcxM0fX0MBMLze1uDgxPTUnMalYLzk/dxMjMCgZgGAH45r5gYcYJTmY lER5xVS/RAjxJeWnVGYkFmfEF5XmpBYfYtTg4BDYvHb1BUYplrz8vFQlCd5tM4HqBItS01Mr0 jJzgHEDUyrBwaMkwpsGkuYtLkjMLc5Mh0idYlSUEuc9DJIQAElklObBtcFi9RKjrJQwLyPQUU I8BalFuZklqPKvGMU5GJWEeZeCTOHJzCuBm/4KaDET0OKIALDFJYkIKakGRpZJZRy8V+03liy 8JL7btr90R/6KizN0f70/PSfNyvnO+aiPsb+1KuwUusOte+TXyG54uKUuVpxP+oie7bb3v9XV Tv7/k9K5i1VkqdmbY398L6sV5X1ed8nsVa2KXPrBeWetF75u2aPumSfubXNug5DpW5sahx2rS 98ec0u0VI/S5nZkutCgr8RSnJFoqMVcVJwIAHlSavTQAgAA X-Env-Sender: prvs=2732fb06d=roger.pau@citrix.com X-Msg-Ref: server-16.tower-21.messagelabs.com!1492425519!59463620!1 X-Originating-IP: [185.25.65.24] X-SpamReason: No, hits=0.0 required=7.0 tests=received_headers: No Received headers X-StarScan-Received: X-StarScan-Version: 9.4.12; banners=-,-,- X-VirusChecked: Checked Received: (qmail 56770 invoked from network); 17 Apr 2017 10:38:39 -0000 Received: from smtp.eu.citrix.com (HELO SMTP.EU.CITRIX.COM) (185.25.65.24) by server-16.tower-21.messagelabs.com with RC4-SHA encrypted SMTP; 17 Apr 2017 10:38:39 -0000 X-IronPort-AV: E=Sophos;i="5.37,214,1488844800"; d="scan'208";a="44417631" Date: Mon, 17 Apr 2017 11:38:33 +0100 From: Roger Pau =?iso-8859-1?Q?Monn=E9?= To: , Kevin Tian , Chao Gao Message-ID: <20170417103833.vwofneibjm46zbty@dhcp-3-128.uk.xensource.com> References: <20170414153441.nd76764ufwq5uwpc@dhcp-3-128.uk.xensource.com> <20170416233245.GA5141@skl-2s3.sh.intel.com> <20170417074748.335yjh6dtgp5mi3b@dhcp-3-128.uk.xensource.com> <20170417010312.GA6267@skl-2s3.sh.intel.com> <20170417083854.sbsyxhfxgrttrgt2@dhcp-3-128.uk.xensource.com> <20170417024945.GA5906@skl-2s3.sh.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170417024945.GA5906@skl-2s3.sh.intel.com> User-Agent: NeoMutt/20170306 (1.8.0) X-ClientProxiedBy: AMSPEX02CAS02.citrite.net (10.69.22.113) To AMSPEX02CL02.citrite.net (10.69.22.126) Subject: Re: [Xen-devel] PVH Dom0 Intel IOMMU issues X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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;