Re: [PATCH 5/8] x86/IDT: Make idt_tables[] be per_cpu(idt)

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 25/02/2025 4:33 pm, Jan Beulich wrote:
> On 25.02.2025 16:40, Andrew Cooper wrote:
>> On 25/02/2025 9:07 am, Jan Beulich wrote:
>>> On 24.02.2025 17:05, Andrew Cooper wrote:
>>>> --- a/xen/arch/x86/cpu/common.c
>>>> +++ b/xen/arch/x86/cpu/common.c
>>>> @@ -819,6 +819,7 @@ void load_system_tables(void)
>>>>  	 * support using ARRAY_SIZE against per-cpu variables.
>>>>  	 */
>>>>  	struct tss_page *tss_page = &this_cpu(tss_page);
>>>> +        idt_entry_t *idt = this_cpu(idt);
>>> Nit: Tab indentation here.
>> Yeah, I noticed that only after sending the email.  Other parts of the
>> FRED series vastly changes this function.
>>
>>>> @@ -830,7 +831,7 @@ void load_system_tables(void)
>>>>  		.limit = LAST_RESERVED_GDT_BYTE,
>>>>  	};
>>>>  	const struct desc_ptr idtr = {
>>>> -		.base = (unsigned long)idt_tables[cpu],
>>>> +		.base = (unsigned long)idt,
>>>>  		.limit = sizeof(bsp_idt) - 1,
>>>>  	};
>>> Coming back to the comment on the earlier patch: Now that you touch all
>>> of the idt_tables[] uses anyway, ...
>>>
>>>> @@ -30,7 +31,7 @@ typedef union {
>>>>  } idt_entry_t;
>>>>  
>>>>  extern idt_entry_t bsp_idt[X86_IDT_VECTORS];
>>>> -extern idt_entry_t *idt_tables[];
>>>> +DECLARE_PER_CPU(idt_entry_t *, idt);
>>> ... this probably really ought to become
>>>
>>> DECLARE_PER_CPU(idt_entry_t (*)[X86_IDT_VECTORS], idt);
>>>
>>> ?
>> I'm afraid this doesn't compile.
>>
>> arch/x86/crash.c:66:17: error: passing argument 1 of ‘set_ist’ from
>> incompatible pointer type [-Werror=incompatible-pointer-types]
>> ...
>> note: expected ‘volatile idt_entry_t *’ but argument is of type
>> ‘idt_entry_t (*)[256]’
>>
>> Similarly {en,dis}able_each_ist() and _set_gate_lower().
> Well, did you adjust the use sites? As said in the respective comment on
> patch 4, that'll be necessary (to account for the abstract extra level of
> indirection; generated code ought to not change).

Having spent even more time, I'm giving up and rejecting this suggestion.

Some can be made to compile.  Some I can't make compile, and the result
is a good demonstration of why exotic types like this don't get common use.

If you want to try once this series is in, then feel free, but the
result is not pretty.

~Andrew