Jan Beulich
2011-Oct-14 10:24 UTC
[Xen-devel] [PATCH] x86/MSI: drop local cpumask_t variable from msi_compose_msg()
The function gets called only during initialization/resume (when no
other CPUs are running) or with the IRQ descriptor lock held, so
there''s no way for the CPU mask to change under its feet.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -123,18 +123,16 @@ static void msix_put_fixmap(struct pci_d
void msi_compose_msg(struct irq_desc *desc, struct msi_msg *msg)
{
unsigned dest;
- cpumask_t domain;
struct irq_cfg *cfg = desc->chip_data;
int vector = cfg->vector;
- domain = cfg->cpu_mask;
- if ( cpus_empty( domain ) ) {
+ if ( cpus_empty(cfg->cpu_mask) ) {
dprintk(XENLOG_ERR,"%s, compose msi message error!!\n",
__func__);
- return;
+ return;
}
if ( vector ) {
- dest = cpu_mask_to_apicid(&domain);
+ dest = cpu_mask_to_apicid(&cfg->cpu_mask);
msg->address_hi = MSI_ADDR_BASE_HI;
msg->address_lo
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2011-Oct-14 12:08 UTC
Re: [Xen-devel] [PATCH] x86/MSI: drop local cpumask_t variable from msi_compose_msg()
On 14/10/2011 11:24, "Jan Beulich" <JBeulich@suse.com> wrote:> The function gets called only during initialization/resume (when no > other CPUs are running) or with the IRQ descriptor lock held, so > there''s no way for the CPU mask to change under its feet. > > Signed-off-by: Jan Beulich <jbeulich@suse.com>Acked-by: Keir Fraser <keir@xen.org> I wonder whether the cpus_empty() check should be a BUG_ON. Or an ASSERT pushed into cpu_mask_to_apicid. -- Keir> --- a/xen/arch/x86/msi.c > +++ b/xen/arch/x86/msi.c > @@ -123,18 +123,16 @@ static void msix_put_fixmap(struct pci_d > void msi_compose_msg(struct irq_desc *desc, struct msi_msg *msg) > { > unsigned dest; > - cpumask_t domain; > struct irq_cfg *cfg = desc->chip_data; > int vector = cfg->vector; > - domain = cfg->cpu_mask; > > - if ( cpus_empty( domain ) ) { > + if ( cpus_empty(cfg->cpu_mask) ) { > dprintk(XENLOG_ERR,"%s, compose msi message error!!\n", __func__); > - return; > + return; > } > > if ( vector ) { > - dest = cpu_mask_to_apicid(&domain); > + dest = cpu_mask_to_apicid(&cfg->cpu_mask); > > msg->address_hi = MSI_ADDR_BASE_HI; > msg->address_lo > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2011-Oct-14 12:42 UTC
Re: [Xen-devel] [PATCH] x86/MSI: drop local cpumask_t variable from msi_compose_msg()
>>> On 14.10.11 at 14:08, Keir Fraser <keir.xen@gmail.com> wrote: > On 14/10/2011 11:24, "Jan Beulich" <JBeulich@suse.com> wrote: > >> The function gets called only during initialization/resume (when no >> other CPUs are running) or with the IRQ descriptor lock held, so >> there''s no way for the CPU mask to change under its feet. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Acked-by: Keir Fraser <keir@xen.org> > > I wonder whether the cpus_empty() check should be a BUG_ON. Or an ASSERT > pushed into cpu_mask_to_apicid.An ASSERT may be reasonable, but simply dropping the check here may be too - no other code path invoking cpu_mask_to_apicid() has a similar check. Jan> -- Keir > >> --- a/xen/arch/x86/msi.c >> +++ b/xen/arch/x86/msi.c >> @@ -123,18 +123,16 @@ static void msix_put_fixmap(struct pci_d >> void msi_compose_msg(struct irq_desc *desc, struct msi_msg *msg) >> { >> unsigned dest; >> - cpumask_t domain; >> struct irq_cfg *cfg = desc->chip_data; >> int vector = cfg->vector; >> - domain = cfg->cpu_mask; >> >> - if ( cpus_empty( domain ) ) { >> + if ( cpus_empty(cfg->cpu_mask) ) { >> dprintk(XENLOG_ERR,"%s, compose msi message error!!\n", __func__); >> - return; >> + return; >> } >> >> if ( vector ) { >> - dest = cpu_mask_to_apicid(&domain); >> + dest = cpu_mask_to_apicid(&cfg->cpu_mask); >> >> msg->address_hi = MSI_ADDR_BASE_HI; >> msg->address_lo >> >> >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2011-Oct-14 14:55 UTC
Re: [Xen-devel] [PATCH] x86/MSI: drop local cpumask_t variable from msi_compose_msg()
On 14/10/2011 13:42, "Jan Beulich" <JBeulich@suse.com> wrote:>> I wonder whether the cpus_empty() check should be a BUG_ON. Or an ASSERT >> pushed into cpu_mask_to_apicid. > > An ASSERT may be reasonable, but simply dropping the check here > may be too - no other code path invoking cpu_mask_to_apicid() has > a similar check.It is certainly not valid to call cpu_mask_to_apicid with an empty mask. Hence an ASSERT would check that precondition, and for all callers. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel