diff mbox series

[v2,5/6] x86/emulate: remove HAVE_AS_RDRAND and HAVE_AS_RDSEED

Message ID 20250403182250.3329498-6-dmukhin@ford.com (mailing list archive)
State New
Headers show
Series x86/asm: cleanups after toolchain baseline upgrade | expand

Commit Message

Denis Mukhin April 3, 2025, 6:23 p.m. UTC 
From: Denis Mukhin <dmukhin@ford.com>

The new toolchain baseline knows the rdrand/rdseed instructions,
no need to carry the workaround in the code.

Resolves: https://gitlab.com/xen-project/xen/-/work_items/208
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/arch.mk            |  2 --
 xen/arch/x86/x86_emulate/0fc7.c | 15 +++++----------
 2 files changed, 5 insertions(+), 12 deletions(-)

Comments

Andrew Cooper April 3, 2025, 7:21 p.m. UTC | #1 
On 03/04/2025 7:23 pm, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> The new toolchain baseline knows the rdrand/rdseed instructions,
> no need to carry the workaround in the code.
>
> Resolves: https://gitlab.com/xen-project/xen/-/work_items/208
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>  xen/arch/x86/arch.mk            |  2 --
>  xen/arch/x86/x86_emulate/0fc7.c | 15 +++++----------

Huh...

I was expecting a hunk in xen/arch/x86/include/asm/random.h but lookin
at it, I see that didn't even get the HAVE_AS_RDRAND treatment in the
first place.  Also, the + constraint, rather than =, looks suspicious.

Could I ask you to clean those up too please?  The rest of this patch
looks fine.

~Andrew
Jan Beulich April 4, 2025, 7:11 a.m. UTC | #2 
On 03.04.2025 21:21, Andrew Cooper wrote:
> On 03/04/2025 7:23 pm, dmkhn@proton.me wrote:
>> From: Denis Mukhin <dmukhin@ford.com>
>>
>> The new toolchain baseline knows the rdrand/rdseed instructions,
>> no need to carry the workaround in the code.
>>
>> Resolves: https://gitlab.com/xen-project/xen/-/work_items/208
>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>> ---
>>  xen/arch/x86/arch.mk            |  2 --
>>  xen/arch/x86/x86_emulate/0fc7.c | 15 +++++----------
> 
> Huh...
> 
> I was expecting a hunk in xen/arch/x86/include/asm/random.h but lookin
> at it, I see that didn't even get the HAVE_AS_RDRAND treatment in the
> first place.  Also, the + constraint, rather than =, looks suspicious.

Might that have been for a similar reason as the one you recently added
a XOR ahead of POPCNT for? IOW us wanting the clearing of the output to
happen irrespective of whether the if() condition is true?

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index 3bbaee2a44..5577bf6241 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -10,9 +10,7 @@  CFLAGS += -msoft-float
 
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 $(call cc-option-add,CFLAGS,CC,-Wnested-externs)
-$(call as-option-add,CFLAGS,CC,"rdrand %eax",-DHAVE_AS_RDRAND)
 $(call as-option-add,CFLAGS,CC,"xsaveopt (%rax)",-DHAVE_AS_XSAVEOPT)
-$(call as-option-add,CFLAGS,CC,"rdseed %eax",-DHAVE_AS_RDSEED)
 $(call as-option-add,CFLAGS,CC,"clwb (%rax)",-DHAVE_AS_CLWB)
 $(call as-option-add,CFLAGS,CC,".equ \"x\"$(comma)1",-DHAVE_AS_QUOTED_SYM)
 $(call as-option-add,CFLAGS,CC,"movdiri %rax$(comma)(%rax)",-DHAVE_AS_MOVDIR)
diff --git a/xen/arch/x86/x86_emulate/0fc7.c b/xen/arch/x86/x86_emulate/0fc7.c
index 5268d5cafd..58c8f79501 100644
--- a/xen/arch/x86/x86_emulate/0fc7.c
+++ b/xen/arch/x86/x86_emulate/0fc7.c
@@ -32,7 +32,6 @@  int x86emul_0fc7(struct x86_emulate_state *s,
             return X86EMUL_UNRECOGNIZED;
 
         case 6: /* rdrand */
-#ifdef HAVE_AS_RDRAND
             generate_exception_if(s->vex.pfx >= vex_f3, X86_EXC_UD);
             host_and_vcpu_must_have(rdrand);
             *dst = s->ea;
@@ -43,12 +42,12 @@  int x86emul_0fc7(struct x86_emulate_state *s,
                       : "=r" (dst->val), ASM_FLAG_OUT("=@ccc", "=qm") (carry) );
                 break;
             default:
-# ifdef __x86_64__
+#ifdef __x86_64__
                 asm ( "rdrand %k0" ASM_FLAG_OUT(, "; setc %1")
                       : "=r" (dst->val), ASM_FLAG_OUT("=@ccc", "=qm") (carry) );
                 break;
             case 8:
-# endif
+#endif
                 asm ( "rdrand %0" ASM_FLAG_OUT(, "; setc %1")
                       : "=r" (dst->val), ASM_FLAG_OUT("=@ccc", "=qm") (carry) );
                 break;
@@ -57,9 +56,6 @@  int x86emul_0fc7(struct x86_emulate_state *s,
             if ( carry )
                 regs->eflags |= X86_EFLAGS_CF;
             break;
-#else
-            return X86EMUL_UNIMPLEMENTED;
-#endif
 
         case 7: /* rdseed / rdpid */
             if ( s->vex.pfx == vex_f3 ) /* rdpid */
@@ -77,7 +73,7 @@  int x86emul_0fc7(struct x86_emulate_state *s,
                 dst->bytes = 4;
                 break;
             }
-#ifdef HAVE_AS_RDSEED
+
             generate_exception_if(s->vex.pfx >= vex_f3, X86_EXC_UD);
             host_and_vcpu_must_have(rdseed);
             *dst = s->ea;
@@ -88,12 +84,12 @@  int x86emul_0fc7(struct x86_emulate_state *s,
                       : "=r" (dst->val), ASM_FLAG_OUT("=@ccc", "=qm") (carry) );
                 break;
             default:
-# ifdef __x86_64__
+#ifdef __x86_64__
                 asm ( "rdseed %k0" ASM_FLAG_OUT(, "; setc %1")
                       : "=r" (dst->val), ASM_FLAG_OUT("=@ccc", "=qm") (carry) );
                 break;
             case 8:
-# endif
+#endif
                 asm ( "rdseed %0" ASM_FLAG_OUT(, "; setc %1")
                       : "=r" (dst->val), ASM_FLAG_OUT("=@ccc", "=qm") (carry) );
                 break;
@@ -102,7 +98,6 @@  int x86emul_0fc7(struct x86_emulate_state *s,
             if ( carry )
                 regs->eflags |= X86_EFLAGS_CF;
             break;
-#endif
         }
     }
     else