Re: [PATCH 6/8] x86/entry: Track the IST-ness of an entry for the exit paths

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/09/2023 8:13 am, Jan Beulich wrote:
> On 14.09.2023 21:44, Andrew Cooper wrote:
>> On 14/09/2023 10:32 am, Jan Beulich wrote:
>>> On 13.09.2023 22:27, Andrew Cooper wrote:
>>>> --- a/xen/arch/x86/x86_64/entry.S
>>>> +++ b/xen/arch/x86/x86_64/entry.S
>>>> @@ -142,10 +142,16 @@ process_trap:
>>>>  
>>>>          .section .text.entry, "ax", @progbits
>>>>  
>>>> -/* %rbx: struct vcpu, interrupts disabled */
>>>> +/* %rbx: struct vcpu, %r12: ist_exit, interrupts disabled */
>>>>  restore_all_guest:
>>>> -        ASSERT_INTERRUPTS_DISABLED
>>>>  
>>>> +#ifdef CONFIG_DEBUG
>>>> +        mov   %rsp, %rdi
>>>> +        mov   %r12, %rsi
>>>> +        call  check_ist_exit
>>>> +#endif
>>>> +
>>>> +        ASSERT_INTERRUPTS_DISABLED
>>>>          /* Stash guest SPEC_CTRL value while we can read struct vcpu. */
>>>>          mov VCPU_arch_msrs(%rbx), %rdx
>>>>          mov VCPUMSR_spec_ctrl_raw(%rdx), %r15d
>>> Even here I don't think I see a need for the addition. Plus if the check
>>> is warranted here, is it really necessary for it to live ahead of the
>>> interrupts-disabled check?
>> What makes you think there is a relevance to the order of two assertions
>> in fully irqs-off code?
> You explicitly making it more churn than strictly needed. IOW I was
> simply wondering whether I was overlooking some aspect.

That was just the diff algorithm after accidentally removing the newline
after the ASSERT.  I've undone that, and the hunk is simple additions
for the check, like it is in the other two hunks.

~Andrew