On 12.05.2025 15:48, Andrew Cooper wrote:
> On 12/05/2025 2:21 pm, Jan Beulich wrote:
>> On 12.05.2025 13:58, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/include/asm/idt.h
>>> +++ b/xen/arch/x86/include/asm/idt.h
>>> @@ -92,15 +92,16 @@ static inline void _set_gate_lower(idt_entry_t *gate,
>>> unsigned long type,
>>> * Update the lower half handler of an IDT entry, without changing any
>>> other
>>> * configuration.
>>> */
>>> -static inline void _update_gate_addr_lower(idt_entry_t *gate, void *addr)
>>> +static inline void _update_gate_addr_lower(idt_entry_t *gate, void *_addr)
>> Considering comment and name of the function, ...
>>
>>> {
>>> + unsigned long addr = (unsigned long)_addr;
>>> + unsigned int addr1 = addr & 0xffff0000U; /* GCC force better codegen.
>>> */
>>> idt_entry_t idte;
>>> - idte.a = gate->a;
>>>
>>> - idte.b = ((unsigned long)(addr) >> 32);
>>> - idte.a &= 0x0000FFFFFFFF0000ULL;
>>> - idte.a |= (((unsigned long)(addr) & 0xFFFF0000UL) << 32) |
>>> - ((unsigned long)(addr) & 0xFFFFUL);
>>> + idte.b = addr >> 32;
>> ... doesn't this line want dropping altogether? Or at best be an assertion?
>
> That's what _write_gate_lower() does, hence why
> _update_gate_addr_lower() needs to calculate .b.
To satisfy that, idte.b = gate.b would be all we need (i.e. just like
_set_gate_lower() does). Or else I think the "lower" would need dropping from
comment and name here.
Jan
