On Tue, Oct 13 2020 at 11:28, Thomas Gleixner wrote:
> On Tue, Oct 13 2020 at 08:52, David Woodhouse wrote:
>> On Tue, 2020-10-13 at 00:13 +0200, Thomas Gleixner wrote:
>> + dom = irq_find_matching_fwspec(fwspec, DOMAIN_BUS_IR);
>> + if (dom)
>> + return IS_ERR(dom) ? NULL : dom;
>> +
>> + return x86_vector_domain;
>> +}
>>
>> Ick. There's no need for that.
>>
>> Eliminating that awful "if not found then slip the x86_vector_domain in
>> as a special case" was the whole *point* of using
>> irq_find_matching_fwspec() in the first place.
>
> The point was to get rid of irq_remapping_get_irq_domain().
>
> And TBH,
>
> if (apicid_valid(32768))
>
> is just another way to slip the vector domain in. It's just differently
> awful.
>
> Having an explicit answer from the search for IR:
>
> - Here is the domain
> - Your device is not registered properly
> - IR not enabled or not supported
>
> is way more obvious than the above disguised is_remapping_enabled()
> check.
And after becoming more awake, that wont work anyway because there is
more than one IR domain, so there is no way to return an error "You
forgot to register" obviously.
But the APIC id (32768) valid check is also broken because IR can be
enabled even without X2APIC.
Thanks,
tglx
