On Wed, Oct 07 2020 at 08:48, David Woodhouse wrote:
> On Tue, 2020-10-06 at 23:54 +0200, Thomas Gleixner wrote:
>> On Mon, Oct 05 2020 at 16:28, David Woodhouse wrote:
> This is the case I called out in the cover letter:
>
> This patch series implements a per-domain "maximum affinity" set and
> uses it for the non-remapped IOAPIC and MSI domains on x86. As well as
> allowing more CPUs to be used without interrupt remapping, this also
> fixes the case where some IOAPICs or PCI devices aren't actually in
> scope of any active IOMMU and are operating without remapping.
>
> To fix *that* case, we really do need the whole series giving us per-
> domain restricted affinity, and to use it for those MSIs/IOAPICs that
> the IRQ remapping doesn't cover.
Which do not exist today.
>> > ip->irqdomain->parent = parent;
>> > + if (parent == x86_vector_domain)
>> > + irq_domain_set_affinity(ip->irqdomain, &x86_non_ir_cpumask);
>>
>> OMG
>
> This IOAPIC function may or may not be behind remapping.
>> > d->flags |= IRQ_DOMAIN_MSI_NOMASK_QUIRK;
>> > + irq_domain_set_affinity(d, &x86_non_ir_cpumask);
>>
>> So here it's unconditional
>
> Yes, this code is only for the non-remapped case and there's a separate
> arch_create_remap_msi_irq_domain() function a few lines further down
> which creates the IR-PCI-MSI IRQ domain. So no need for a condition
> here.
>
>> > + if (parent == x86_vector_domain)
>> > + irq_domain_set_affinity(d, &x86_non_ir_cpumask);
>>
>> And here we need a condition again. Completely obvious and reviewable - NOT.
>
> And HPET may or may not be behind remapping so again needs the
> condition. I had figured that part was fairly obvious but can note it
> in the commit comment.
And all of this is completely wrong to begin with.
The information has to property of the relevant irq domains and the
hierarchy allows you nicely to retrieve it from there instead of
sprinkling this all over the place.
Thanks,
tglx
