* [PATCH] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags
@ 2017-11-27 18:05 Boris Ostrovsky
2017-11-27 19:22 ` Juergen Gross
2017-11-27 19:22 ` Juergen Gross
0 siblings, 2 replies; 6+ messages in thread
From: Boris Ostrovsky @ 2017-11-27 18:05 UTC (permalink / raw)
To: linux-kernel, xen-devel; +Cc: tglx, mingo, hpa, jgross, x86, luto
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
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
+#endif
#else
#define INTERRUPT_RETURN iret
#define ENABLE_INTERRUPTS_SYSEXIT sti; sysexit
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 283efca..892df37 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -927,6 +927,15 @@ static inline notrace unsigned long arch_local_irq_save(void)
PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_usergs_sysret64), \
CLBR_NONE, \
jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_usergs_sysret64))
+
+#ifdef CONFIG_DEBUG_ENTRY
+#define SAVE_FLAGS(clobbers) \
+ PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_save_fl), clobbers, \
+ PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE); \
+ call PARA_INDIRECT(pv_irq_ops+PV_IRQ_save_fl); \
+ PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
+#endif
+
#endif /* CONFIG_X86_32 */
#endif /* __ASSEMBLY__ */
diff --git a/arch/x86/kernel/asm-offsets_64.c b/arch/x86/kernel/asm-offsets_64.c
index 630212f..e3a5175 100644
--- a/arch/x86/kernel/asm-offsets_64.c
+++ b/arch/x86/kernel/asm-offsets_64.c
@@ -23,6 +23,9 @@ int main(void)
#ifdef CONFIG_PARAVIRT
OFFSET(PV_CPU_usergs_sysret64, pv_cpu_ops, usergs_sysret64);
OFFSET(PV_CPU_swapgs, pv_cpu_ops, swapgs);
+#ifdef CONFIG_DEBUG_ENTRY
+ OFFSET(PV_IRQ_save_fl, pv_irq_ops, save_fl);
+#endif
BLANK();
#endif
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags
2017-11-27 18:05 [PATCH] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags Boris Ostrovsky
@ 2017-11-27 19:22 ` Juergen Gross
2017-11-27 21:40 ` Boris Ostrovsky
2017-11-27 19:22 ` Juergen Gross
1 sibling, 1 reply; 6+ messages in thread
From: Juergen Gross @ 2017-11-27 19:22 UTC (permalink / raw)
To: Boris Ostrovsky, linux-kernel, xen-devel; +Cc: tglx, mingo, hpa, x86, luto
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) ?
Juergen
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags
2017-11-27 18:05 [PATCH] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags Boris Ostrovsky
2017-11-27 19:22 ` Juergen Gross
@ 2017-11-27 19:22 ` Juergen Gross
1 sibling, 0 replies; 6+ messages in thread
From: Juergen Gross @ 2017-11-27 19:22 UTC (permalink / raw)
To: Boris Ostrovsky, linux-kernel, xen-devel; +Cc: tglx, mingo, x86, luto, hpa
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) ?
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags
2017-11-27 19:22 ` Juergen Gross
@ 2017-11-27 21:40 ` Boris Ostrovsky
0 siblings, 0 replies; 6+ messages in thread
From: Boris Ostrovsky @ 2017-11-27 21:40 UTC (permalink / raw)
To: Juergen Gross, linux-kernel, xen-devel; +Cc: tglx, mingo, hpa, x86, luto
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags
@ 2017-11-27 21:40 ` Boris Ostrovsky
0 siblings, 0 replies; 6+ messages in thread
From: Boris Ostrovsky @ 2017-11-27 21:40 UTC (permalink / raw)
To: Juergen Gross, linux-kernel, xen-devel; +Cc: tglx, mingo, x86, luto, hpa
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags
@ 2017-11-27 18:05 Boris Ostrovsky
0 siblings, 0 replies; 6+ messages in thread
From: Boris Ostrovsky @ 2017-11-27 18:05 UTC (permalink / raw)
To: linux-kernel, xen-devel; +Cc: jgross, x86, mingo, luto, hpa, tglx
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
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
+#endif
#else
#define INTERRUPT_RETURN iret
#define ENABLE_INTERRUPTS_SYSEXIT sti; sysexit
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 283efca..892df37 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -927,6 +927,15 @@ static inline notrace unsigned long arch_local_irq_save(void)
PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_usergs_sysret64), \
CLBR_NONE, \
jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_usergs_sysret64))
+
+#ifdef CONFIG_DEBUG_ENTRY
+#define SAVE_FLAGS(clobbers) \
+ PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_save_fl), clobbers, \
+ PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE); \
+ call PARA_INDIRECT(pv_irq_ops+PV_IRQ_save_fl); \
+ PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
+#endif
+
#endif /* CONFIG_X86_32 */
#endif /* __ASSEMBLY__ */
diff --git a/arch/x86/kernel/asm-offsets_64.c b/arch/x86/kernel/asm-offsets_64.c
index 630212f..e3a5175 100644
--- a/arch/x86/kernel/asm-offsets_64.c
+++ b/arch/x86/kernel/asm-offsets_64.c
@@ -23,6 +23,9 @@ int main(void)
#ifdef CONFIG_PARAVIRT
OFFSET(PV_CPU_usergs_sysret64, pv_cpu_ops, usergs_sysret64);
OFFSET(PV_CPU_swapgs, pv_cpu_ops, swapgs);
+#ifdef CONFIG_DEBUG_ENTRY
+ OFFSET(PV_IRQ_save_fl, pv_irq_ops, save_fl);
+#endif
BLANK();
#endif
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-27 21:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-27 18:05 [PATCH] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags Boris Ostrovsky
2017-11-27 19:22 ` Juergen Gross
2017-11-27 21:40 ` Boris Ostrovsky
2017-11-27 21:40 ` Boris Ostrovsky
2017-11-27 19:22 ` Juergen Gross
2017-11-27 18:05 Boris Ostrovsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.