From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yang, Sheng" Subject: Re: [PATCH] Only include online cpus in cpu_mask_to_apicid_flat Date: Wed, 1 Sep 2010 18:02:13 +0800 Message-ID: <201009011802.14274.sheng.yang@intel.com> References: <201009011139.47130.sheng.yang@intel.com> <4C7E35AC0200007800013A65@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_mSifMXweJtBrUSY" Return-path: In-Reply-To: <4C7E35AC0200007800013A65@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: "xen-devel@lists.xensource.com" , Keir Fraser List-Id: xen-devel@lists.xenproject.org --Boundary-00=_mSifMXweJtBrUSY Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Wednesday 01 September 2010 17:14:52 Jan Beulich wrote: > >>> On 01.09.10 at 05:39, "Yang, Sheng" wrote: > > Yes, here is the patch with modification of other variants. > > If indeed an adjustment like this is needed, then this (and other similar > instances) > > >@@ -71,6 +72,11 @@ > > > > unsigned int cpu_mask_to_apicid_phys(cpumask_t cpumask) > > { > > > >+ int cpu; > > > > /* As we are using single CPU as destination, pick only one CPU here */ > > > >- return cpu_physical_id(first_cpu(cpumask)); > >+ for_each_cpu_mask(cpu, cpumask) { > >+ if (cpu_online(cpu)) > >+ break; > >+ } > >+ return cpu_physical_id(cpu); > > > > } > > is both insufficient: You need to handle the case where you don't > find any online CPU in the mask (at least by adding a respective > BUG_ON()). Yes, BUG_ON() is needed. > > But I tend to agree with Keir that this shouldn't be done here - > these functions are simple accessors, which shouldn't enforce > any policy. Higher level code, if it doesn't already, should be > adjusted to never allow offline CPUs to slip through. Well, I think it's acceptable to add a wrap function for it. So how about this one? -- regards Yang, Sheng > > Jan --Boundary-00=_mSifMXweJtBrUSY Content-Type: text/x-patch; charset="UTF-8"; name="dest_fix.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="dest_fix.patch" diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c --- a/xen/arch/x86/io_apic.c +++ b/xen/arch/x86/io_apic.c @@ -490,7 +490,7 @@ cpus_copy(desc->affinity, mask); cpus_and(dest_mask, desc->affinity, cfg->cpu_mask); - return cpu_mask_to_apicid(dest_mask); + return cpu_mask_to_apicid_online(dest_mask); } static void @@ -1003,7 +1003,7 @@ } cfg = irq_cfg(irq); SET_DEST(entry.dest.dest32, entry.dest.logical.logical_dest, - cpu_mask_to_apicid(cfg->cpu_mask)); + cpu_mask_to_apicid_online(cfg->cpu_mask)); spin_lock_irqsave(&ioapic_lock, flags); io_apic_write(apic, 0x11+2*pin, *(((int *)&entry)+1)); io_apic_write(apic, 0x10+2*pin, *(((int *)&entry)+0)); @@ -1038,7 +1038,7 @@ entry.dest_mode = INT_DEST_MODE; entry.mask = 0; /* unmask IRQ now */ SET_DEST(entry.dest.dest32, entry.dest.logical.logical_dest, - cpu_mask_to_apicid(TARGET_CPUS)); + cpu_mask_to_apicid_online(TARGET_CPUS)); entry.delivery_mode = INT_DELIVERY_MODE; entry.polarity = 0; entry.trigger = 0; @@ -2253,7 +2253,7 @@ entry.delivery_mode = INT_DELIVERY_MODE; entry.dest_mode = INT_DEST_MODE; SET_DEST(entry.dest.dest32, entry.dest.logical.logical_dest, - cpu_mask_to_apicid(TARGET_CPUS)); + cpu_mask_to_apicid_online(TARGET_CPUS)); entry.trigger = edge_level; entry.polarity = active_high_low; entry.mask = 1; @@ -2446,7 +2446,7 @@ rte.vector = cfg->vector; SET_DEST(rte.dest.dest32, rte.dest.logical.logical_dest, - cpu_mask_to_apicid(cfg->cpu_mask)); + cpu_mask_to_apicid_online(cfg->cpu_mask)); io_apic_write(apic, 0x10 + 2 * pin, *(((int *)&rte) + 0)); io_apic_write(apic, 0x11 + 2 * pin, *(((int *)&rte) + 1)); @@ -2584,3 +2584,12 @@ printk(XENLOG_INFO "IRQ limits: %u GSI, %u MSI/MSI-X\n", nr_irqs_gsi, nr_irqs - nr_irqs_gsi); } + +unsigned int cpu_mask_to_apicid_online(cpumask_t cpu_mask) +{ + cpumask_t dest; + cpus_and(dest, cpu_mask, cpu_online_map); + BUG_ON(cpus_empty(dest)); + return cpu_mask_to_apicid(dest); +} + diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -86,14 +86,14 @@ cpus_and(online_mask, cpu_mask, cpu_online_map); if (cpus_empty(online_mask)) return -EINVAL; - if ((cfg->vector == vector) && cpus_equal(cfg->cpu_mask, online_mask)) + if ((cfg->vector == vector) && cpus_equal(cfg->cpu_mask, cpu_mask)) return 0; if (cfg->vector != IRQ_VECTOR_UNASSIGNED) return -EBUSY; for_each_cpu_mask(cpu, online_mask) per_cpu(vector_irq, cpu)[vector] = irq; cfg->vector = vector; - cfg->cpu_mask = online_mask; + cfg->cpu_mask = cpu_mask; irq_status[irq] = IRQ_USED; if (IO_APIC_IRQ(irq)) irq_vector[irq] = vector; diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c --- a/xen/arch/x86/msi.c +++ b/xen/arch/x86/msi.c @@ -133,7 +133,7 @@ if ( vector ) { - dest = cpu_mask_to_apicid(domain); + dest = cpu_mask_to_apicid_online(domain); msg->address_hi = MSI_ADDR_BASE_HI; msg->address_lo = diff --git a/xen/include/asm-x86/genapic.h b/xen/include/asm-x86/genapic.h --- a/xen/include/asm-x86/genapic.h +++ b/xen/include/asm-x86/genapic.h @@ -52,6 +52,8 @@ extern const struct genapic apic_x2apic_phys; extern const struct genapic apic_x2apic_cluster; +unsigned int cpu_mask_to_apicid_online(cpumask_t cpu_mask); + void init_apic_ldr_flat(void); void clustered_apic_check_flat(void); cpumask_t target_cpus_flat(void); --Boundary-00=_mSifMXweJtBrUSY Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --Boundary-00=_mSifMXweJtBrUSY--