Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] x86/entry/64/paravirt: Use paravirt-safe macro to access
 eflags
To:     Juergen Gross <jgross@suse.com>, linux-kernel@vger.kernel.org,
        xen-devel@lists.xenproject.org
References: <1511805918-8041-1-git-send-email-boris.ostrovsky@oracle.com>
 <be726c34-865b-e238-9fa2-96d0df859e13@suse.com>
Cc:     tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
        x86@kernel.org, luto@kernel.org
From:   Boris Ostrovsky <boris.ostrovsky@oracle.com>
Message-ID: <b6829487-ba7b-42d8-f80c-6528ddfc3c5a@oracle.com>
Date:   Mon, 27 Nov 2017 16:40:25 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <be726c34-865b-e238-9fa2-96d0df859e13@suse.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 11/27/2017 02:22 PM, Juergen Gross wrote:
> On 27/11/17 19:05, Boris Ostrovsky wrote:
>> Commit 1d3e53e8624a ("x86/entry/64: Refactor IRQ stacks and make
>> them NMI-safe") added DEBUG_ENTRY_ASSERT_IRQS_OFF macro that acceses
>> eflags using 'pushfq' instruction when testing for IF bit. On PV Xen
>> guests looking at IF flag directly will always see it set, resulting
>> in 'ud2'.
>>
>> Introduce SAVE_FLAGS() macro that will use appropriate save_fl pv op
>> when running paravirt.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> ---
>>  arch/x86/entry/entry_64.S        |    5 ++---
>>  arch/x86/include/asm/irqflags.h  |    3 +++
>>  arch/x86/include/asm/paravirt.h  |    9 +++++++++
>>  arch/x86/kernel/asm-offsets_64.c |    3 +++
>>  4 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
>> index f81d50d..4bb7719 100644
>> --- a/arch/x86/entry/entry_64.S
>> +++ b/arch/x86/entry/entry_64.S
>> @@ -466,12 +466,11 @@ END(irq_entries_start)
>>  
>>  .macro DEBUG_ENTRY_ASSERT_IRQS_OFF
>>  #ifdef CONFIG_DEBUG_ENTRY
>> -	pushfq
>> -	testl $X86_EFLAGS_IF, (%rsp)
>> +	SAVE_FLAGS(CLBR_ANY)
>> +	testl $X86_EFLAGS_IF, %eax
> Are you sure %eax is allowed to be modified?
>
>>  	jz .Lokay_\@
>>  	ud2
>>  .Lokay_\@:
>> -	addq $8, %rsp
>>  #endif
>>  .endm
>>  
>> diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
>> index c8ef23f..7f65f3f 100644
>> --- a/arch/x86/include/asm/irqflags.h
>> +++ b/arch/x86/include/asm/irqflags.h
>> @@ -142,6 +142,9 @@ static inline notrace unsigned long arch_local_irq_save(void)
>>  	swapgs;					\
>>  	sysretl
>>  
>> +#ifdef CONFIG_DEBUG_ENTRY
>> +#define SAVE_FLAGS(x)		pushfq
> Isn't there a "pop %rax" missing (assuming %rax is allowed to be
> modified) ?


Yes to both. Need to save %rax in DEBUG_ENTRY_ASSERT_IRQS_OFF.

Thanks.
-boris

From 1585248338307706885@xxx Mon Nov 27 19:23:45 +0000 2017
X-GM-THRID: 1585242480710091344
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread