On Tue, Jul 16, 2019 at 10:20:10AM +0000, Jan Beulich wrote: > On 16.07.2019 11:12, Roger Pau Monné wrote: > > On Tue, Jul 16, 2019 at 07:40:57AM +0000, Jan Beulich wrote: > >> In line with "x86/IRQ: desc->affinity should strictly represent the > >> requested value" the internally used IRQ(s) also shouldn't be restricted > >> to online ones. Make set_desc_affinity() (set_msi_affinity() then does > >> by implication) cope with a NULL mask being passed (just like > >> assign_irq_vector() does), and have IOMMU code pass NULL instead of > >> &cpu_online_map (when, for VT-d, there's no NUMA node information > >> available). > >> > >> Signed-off-by: Jan Beulich <jbeul...@suse.com> > > > > LGTM, just one patch style comment and one code comment: > > > > Reviewed-by: Roger Pau Monné <roger....@citrix.com> > > Thanks. > > >> --- a/xen/arch/x86/irq.c > >> +++ b/xen/arch/x86/irq.c > >> @@ -796,18 +796,26 @@ unsigned int set_desc_affinity(struct ir > >> unsigned long flags; > >> cpumask_t dest_mask; > >> > >> - if (!cpumask_intersects(mask, &cpu_online_map)) > >> + if ( mask && !cpumask_intersects(mask, &cpu_online_map) ) > >> return BAD_APICID; > >> > >> spin_lock_irqsave(&vector_lock, flags); > >> - ret = _assign_irq_vector(desc, mask); > >> + ret = _assign_irq_vector(desc, mask ?: TARGET_CPUS); > >> spin_unlock_irqrestore(&vector_lock, flags); > > > > I think the patch is somehow mangled at least on my end, there's one > > prepended extra space in the non-modified lines AFAICT. > > Well, yes, hence the last sentence in the cover letter and the attached > patches there. It is the mail system (more likely server than client) > over here which causes this issue (everywhere for me).
Oh, sorry to hear that. Hope you get that sorted out, I guess it's causing quite a lot of pain for more people at SUSE also. > >> > >> - if (ret < 0) > >> + if ( ret < 0 ) > >> return BAD_APICID; > >> > >> - cpumask_copy(desc->affinity, mask); > > > > AFAICT you could also avoid the if and just do the same as in the > > assign_irq_vector call above and pass TARGET_CPUS if mask is NULL? > > Are you talking about the if() in context above, or the one you've > stripped (immediately following the last quoted line of the patch)? > For the one in context I don't see how the rest of your remark is > related. For the other one - no, strictly not, as that would be > against the purpose of this change: We specifically want to _not_ > restrict desc->affinity to online CPUs only (yet that's what > TARGET_CPUS resolves to). Yes, that was my remark - which is wrong as you pointed out. I guess you could use cpu_possible_map, but anyway the current approach is OK IMO. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel