Re: [PATCH 05/12] x86/io-apic: purge usage of logical mode

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/11/2025 2:27 pm, Jan Beulich wrote:
> On 20.11.2025 14:18, Andrew Cooper wrote:
>> On 20/11/2025 9:06 am, Roger Pau Monne wrote:
>>> The IO-APIC RTEs are unconditionally programmed with physical destination
>>> mode, and hence the field to set in the RTE is always physical_dest.
>>>
>>> Remove the mode parameter from SET_DEST() and take the opportunity to
>>> convert it into a function, there's no need for it to be a macro.
>>>
>>> This is a benign fix, because due to the endianness of x86 the start of the
>>> physical_dest and logical_dest fields on the RTE overlap.
>> RTEs do not have overlapping fields; it's Xen's abstraction of the
>> IO-APIC which is buggy.
> I wouldn't put it this negatively. In the old days, ...
>
>> For starters, Xen's IO_APIC_route_entry still only has a 4-bit
>> physical_dest field which hasn't been true since the Pentium 4 days. 
>> This might explain some of the interrupt bugs we see.
> ... as you mention here, the two fields were distinct (and hence overlapping).

Since when?

Even in the oldest manuals I can find, it's a single field called
destination, and who's contents is interpreted differently depending on
the logical mode bit.

>From a hardware point of view, there will either be 4 or 8 bits of
storage, and it will have nothing to do with the lower bits.

> In a number of places we passed "logical" to SET_DEST() as the middle 
> argument,
> thus covering for the too narrow field width of physical_dest. Dropping that
> parameter and always using physical_dest requires that field to be widened,
> though (or else we'll end up chopping off the top 4 bits, as we already do in
> disable_IO_APIC() and unlock_ExtINT_logic() - both benign as long as the CPU
> used always has APIC ID 0, which will at least typically be the case, I 
> think).

Latent or not, it's still in need of fixing.

It looks like the code Xen inherited was added to Linux in e1d919786
(Jan 2008, even then only x86_32) and deleted by d83e94acd957 (August 2008).

It looks like we're 17 years late undoing this...

~Andrew