Re: [PATCH] x86/traps: Drop incorrect BUILD_BUG_ON() and comment in load_system_tables()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/03/2025 9:03 am, Jan Beulich wrote:
> On 14.03.2025 19:33, Andrew Cooper wrote:
>> It is only the hardware task switching mechanism which cares about a TSS 
>> being
>> at least 0x67 bytes long.
> I/O bitmap accesses are where this particular limit comes into play. For
> 32-bit task switching a slightly shorter one would still do, I think?

Even by x86 standards its a terrible hack.  32-bit task switching
mandates 0x67, even though the IO bitmap is not accessed for the
outgoing or incoming task.

For IO accesses in general, a limit shorter than the IO bitmap pointer
means no IO bitmap, and IO accesses in Ring3 take #GP.

>
>>  Furthermore, since this check was added, the limit is now 0x6b if CET-SS is
>> active.
> Which isn't reflected at all in struct tss64: Aiui that's an addition to the
> 32-bit TSS only.

0x67 isn't relevant to tss64 either.  It's strictly for hardware task
switching, which is strictly for 32bit.


>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -900,8 +900,6 @@ void load_system_tables(void)
>>              wrmsrl(MSR_INTERRUPT_SSP_TABLE, (unsigned long)ist_ssp);
>>      }
>>  
>> -    BUILD_BUG_ON(sizeof(*tss) <= 0x67); /* Mandated by the architecture. */
>> -
>>      _set_tssldt_desc(gdt + TSS_ENTRY, (unsigned long)tss,
>>                       sizeof(*tss) - 1, SYS_DESC_tss_avail);
> All of the above said, the removal worries me primarily with the sizeof()
> still in use here.

Xen uses IST4 but not IST5.

Xen could set the limit to 67 (== 0x43) and everything would continue to
be fine.  In fact, this is quite possibly a better option than poisoning
IST[5..7].

I'm deleting the BUILD_BUG_ON() because everything about it, even the
comment, is incorrect for Xen.

~Andrew