On 12/11/20 7:37 AM, Thomas Gleixner wrote: > On Fri, Dec 11 2020 at 13:10, Jürgen Groß wrote: >> On 11.12.20 00:20, boris.ostrovsky@xxxxxxxxxx wrote: >>> On 12/10/20 2:26 PM, Thomas Gleixner wrote: >>>> All event channel setups bind the interrupt on CPU0 or the target CPU for >>>> percpu interrupts and overwrite the affinity mask with the corresponding >>>> cpumask. That does not make sense. >>>> >>>> The XEN implementation of irqchip::irq_set_affinity() already picks a >>>> single target CPU out of the affinity mask and the actual target is stored >>>> in the effective CPU mask, so destroying the user chosen affinity mask >>>> which might contain more than one CPU is wrong. >>>> >>>> Change the implementation so that the channel is bound to CPU0 at the XEN >>>> level and leave the affinity mask alone. At startup of the interrupt >>>> affinity will be assigned out of the affinity mask and the XEN binding will >>>> be updated. >>> >>> If that's the case then I wonder whether we need this call at all and instead bind at startup time. >> After some discussion with Thomas on IRC and xen-devel archaeology the >> result is: this will be needed especially for systems running on a >> single vcpu (e.g. small guests), as the .irq_set_affinity() callback >> won't be called in this case when starting the irq. On UP are we not then going to end up with an empty affinity mask? Or are we guaranteed to have it set to 1 by interrupt generic code? This is actually why I brought this up in the first place --- a potential mismatch between the affinity mask and Xen-specific data (e.g. info->cpu and then protocol-specific data in event channel code). Even if they are re-synchronized later, at startup time (for SMP). I don't see anything that would cause a problem right now but I worry that this inconsistency may come up at some point. -boris > That's right, but not limited to ARM. The same problem exists on x86 UP. > So yes, the call makes sense, but the changelog is not really useful. > Let me add a comment to this. > > Thanks, > > tglx >
