* [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor}
@ 2021-02-26 16:20 Richard Henderson
2021-02-26 16:20 ` [PATCH 1/2] target/i386: Rename helper_fldt, helper_fstt Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Richard Henderson @ 2021-02-26 16:20 UTC (permalink / raw
To: qemu-devel; +Cc: pbonzini, philmd, cfontana, ehabkost
As discussed during review of Claudio's "i386 cleanup" patch set.
r~
Richard Henderson (2):
target/i386: Rename helper_fldt, helper_fstt
target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor
target/i386/tcg/fpu_helper.c | 65 +++++++++++++++++++++++-------------
1 file changed, 41 insertions(+), 24 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] target/i386: Rename helper_fldt, helper_fstt
2021-02-26 16:20 [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor} Richard Henderson
@ 2021-02-26 16:20 ` Richard Henderson
2021-02-26 16:45 ` Philippe Mathieu-Daudé
2021-02-26 16:20 ` [PATCH 2/2] target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor Richard Henderson
2021-02-26 16:59 ` [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor} Claudio Fontana
2 siblings, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2021-02-26 16:20 UTC (permalink / raw
To: qemu-devel; +Cc: pbonzini, philmd, cfontana, ehabkost
Change the prefix from "helper" to "do". The former should be
reserved for those functions that are called from TCG; the latter
is in use within the file already for those functions that are
called from the helper functions, adding a "retaddr" argument.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/i386/tcg/fpu_helper.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index 60ed93520a..3d9b192901 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -117,8 +117,7 @@ static inline void fpop(CPUX86State *env)
env->fpstt = (env->fpstt + 1) & 7;
}
-static inline floatx80 helper_fldt(CPUX86State *env, target_ulong ptr,
- uintptr_t retaddr)
+static floatx80 do_fldt(CPUX86State *env, target_ulong ptr, uintptr_t retaddr)
{
CPU_LDoubleU temp;
@@ -127,8 +126,8 @@ static inline floatx80 helper_fldt(CPUX86State *env, target_ulong ptr,
return temp.d;
}
-static inline void helper_fstt(CPUX86State *env, floatx80 f, target_ulong ptr,
- uintptr_t retaddr)
+static void do_fstt(CPUX86State *env, floatx80 f, target_ulong ptr,
+ uintptr_t retaddr)
{
CPU_LDoubleU temp;
@@ -405,14 +404,14 @@ void helper_fldt_ST0(CPUX86State *env, target_ulong ptr)
int new_fpstt;
new_fpstt = (env->fpstt - 1) & 7;
- env->fpregs[new_fpstt].d = helper_fldt(env, ptr, GETPC());
+ env->fpregs[new_fpstt].d = do_fldt(env, ptr, GETPC());
env->fpstt = new_fpstt;
env->fptags[new_fpstt] = 0; /* validate stack entry */
}
void helper_fstt_ST0(CPUX86State *env, target_ulong ptr)
{
- helper_fstt(env, ST0, ptr, GETPC());
+ do_fstt(env, ST0, ptr, GETPC());
}
void helper_fpush(CPUX86State *env)
@@ -2468,7 +2467,7 @@ void helper_fsave(CPUX86State *env, target_ulong ptr, int data32)
ptr += (14 << data32);
for (i = 0; i < 8; i++) {
tmp = ST(i);
- helper_fstt(env, tmp, ptr, GETPC());
+ do_fstt(env, tmp, ptr, GETPC());
ptr += 10;
}
@@ -2495,7 +2494,7 @@ void helper_frstor(CPUX86State *env, target_ulong ptr, int data32)
ptr += (14 << data32);
for (i = 0; i < 8; i++) {
- tmp = helper_fldt(env, ptr, GETPC());
+ tmp = do_fldt(env, ptr, GETPC());
ST(i) = tmp;
ptr += 10;
}
@@ -2539,7 +2538,7 @@ static void do_xsave_fpu(CPUX86State *env, target_ulong ptr, uintptr_t ra)
addr = ptr + XO(legacy.fpregs);
for (i = 0; i < 8; i++) {
floatx80 tmp = ST(i);
- helper_fstt(env, tmp, addr, ra);
+ do_fstt(env, tmp, addr, ra);
addr += 16;
}
}
@@ -2703,7 +2702,7 @@ static void do_xrstor_fpu(CPUX86State *env, target_ulong ptr, uintptr_t ra)
addr = ptr + XO(legacy.fpregs);
for (i = 0; i < 8; i++) {
- floatx80 tmp = helper_fldt(env, addr, ra);
+ floatx80 tmp = do_fldt(env, addr, ra);
ST(i) = tmp;
addr += 16;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor
2021-02-26 16:20 [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor} Richard Henderson
2021-02-26 16:20 ` [PATCH 1/2] target/i386: Rename helper_fldt, helper_fstt Richard Henderson
@ 2021-02-26 16:20 ` Richard Henderson
2021-02-26 16:47 ` Philippe Mathieu-Daudé
2021-02-26 16:59 ` [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor} Claudio Fontana
2 siblings, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2021-02-26 16:20 UTC (permalink / raw
To: qemu-devel; +Cc: pbonzini, philmd, cfontana, ehabkost
The helper_* functions must use GETPC() to unwind from TCG.
The cpu_x86_* functions cannot, and directly calling the
helper_* functions is a bug. Split out new functions that
perform the work and can be used by both.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/i386/tcg/fpu_helper.c | 50 ++++++++++++++++++++++++------------
1 file changed, 34 insertions(+), 16 deletions(-)
diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index 3d9b192901..20e4d2e715 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -2457,17 +2457,18 @@ void helper_fldenv(CPUX86State *env, target_ulong ptr, int data32)
do_fldenv(env, ptr, data32, GETPC());
}
-void helper_fsave(CPUX86State *env, target_ulong ptr, int data32)
+static void do_fsave(CPUX86State *env, target_ulong ptr, int data32,
+ uintptr_t retaddr)
{
floatx80 tmp;
int i;
- do_fstenv(env, ptr, data32, GETPC());
+ do_fstenv(env, ptr, data32, retaddr);
ptr += (14 << data32);
for (i = 0; i < 8; i++) {
tmp = ST(i);
- do_fstt(env, tmp, ptr, GETPC());
+ do_fstt(env, tmp, ptr, retaddr);
ptr += 10;
}
@@ -2485,30 +2486,41 @@ void helper_fsave(CPUX86State *env, target_ulong ptr, int data32)
env->fptags[7] = 1;
}
-void helper_frstor(CPUX86State *env, target_ulong ptr, int data32)
+void helper_fsave(CPUX86State *env, target_ulong ptr, int data32)
+{
+ do_fsave(env, ptr, data32, GETPC());
+}
+
+static void do_frstor(CPUX86State *env, target_ulong ptr, int data32,
+ uintptr_t retaddr)
{
floatx80 tmp;
int i;
- do_fldenv(env, ptr, data32, GETPC());
+ do_fldenv(env, ptr, data32, retaddr);
ptr += (14 << data32);
for (i = 0; i < 8; i++) {
- tmp = do_fldt(env, ptr, GETPC());
+ tmp = do_fldt(env, ptr, retaddr);
ST(i) = tmp;
ptr += 10;
}
}
+void helper_frstor(CPUX86State *env, target_ulong ptr, int data32)
+{
+ do_frstor(env, ptr, data32, GETPC());
+}
+
#if defined(CONFIG_USER_ONLY)
void cpu_x86_fsave(CPUX86State *env, target_ulong ptr, int data32)
{
- helper_fsave(env, ptr, data32);
+ do_fsave(env, ptr, data32, 0);
}
void cpu_x86_frstor(CPUX86State *env, target_ulong ptr, int data32)
{
- helper_frstor(env, ptr, data32);
+ do_frstor(env, ptr, data32, 0);
}
#endif
@@ -2593,10 +2605,8 @@ static void do_xsave_pkru(CPUX86State *env, target_ulong ptr, uintptr_t ra)
cpu_stq_data_ra(env, ptr, env->pkru, ra);
}
-void helper_fxsave(CPUX86State *env, target_ulong ptr)
+static void do_fxsave(CPUX86State *env, target_ulong ptr, uintptr_t ra)
{
- uintptr_t ra = GETPC();
-
/* The operand must be 16 byte aligned */
if (ptr & 0xf) {
raise_exception_ra(env, EXCP0D_GPF, ra);
@@ -2615,6 +2625,11 @@ void helper_fxsave(CPUX86State *env, target_ulong ptr)
}
}
+void helper_fxsave(CPUX86State *env, target_ulong ptr)
+{
+ do_fxsave(env, ptr, GETPC());
+}
+
static uint64_t get_xinuse(CPUX86State *env)
{
uint64_t inuse = -1;
@@ -2757,10 +2772,8 @@ static void do_xrstor_pkru(CPUX86State *env, target_ulong ptr, uintptr_t ra)
env->pkru = cpu_ldq_data_ra(env, ptr, ra);
}
-void helper_fxrstor(CPUX86State *env, target_ulong ptr)
+static void do_fxrstor(CPUX86State *env, target_ulong ptr, uintptr_t ra)
{
- uintptr_t ra = GETPC();
-
/* The operand must be 16 byte aligned */
if (ptr & 0xf) {
raise_exception_ra(env, EXCP0D_GPF, ra);
@@ -2779,15 +2792,20 @@ void helper_fxrstor(CPUX86State *env, target_ulong ptr)
}
}
+void helper_fxrstor(CPUX86State *env, target_ulong ptr)
+{
+ do_fxrstor(env, ptr, GETPC());
+}
+
#if defined(CONFIG_USER_ONLY)
void cpu_x86_fxsave(CPUX86State *env, target_ulong ptr)
{
- helper_fxsave(env, ptr);
+ do_fxsave(env, ptr, 0);
}
void cpu_x86_fxrstor(CPUX86State *env, target_ulong ptr)
{
- helper_fxrstor(env, ptr);
+ do_fxrstor(env, ptr, 0);
}
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] target/i386: Rename helper_fldt, helper_fstt
2021-02-26 16:20 ` [PATCH 1/2] target/i386: Rename helper_fldt, helper_fstt Richard Henderson
@ 2021-02-26 16:45 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-26 16:45 UTC (permalink / raw
To: Richard Henderson, qemu-devel; +Cc: pbonzini, cfontana, ehabkost
On 2/26/21 5:20 PM, Richard Henderson wrote:
> Change the prefix from "helper" to "do". The former should be
> reserved for those functions that are called from TCG; the latter
> is in use within the file already for those functions that are
> called from the helper functions, adding a "retaddr" argument.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/i386/tcg/fpu_helper.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor
2021-02-26 16:20 ` [PATCH 2/2] target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor Richard Henderson
@ 2021-02-26 16:47 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-26 16:47 UTC (permalink / raw
To: Richard Henderson, qemu-devel; +Cc: pbonzini, cfontana, ehabkost
On 2/26/21 5:20 PM, Richard Henderson wrote:
> The helper_* functions must use GETPC() to unwind from TCG.
> The cpu_x86_* functions cannot, and directly calling the
> helper_* functions is a bug. Split out new functions that
> perform the work and can be used by both.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/i386/tcg/fpu_helper.c | 50 ++++++++++++++++++++++++------------
> 1 file changed, 34 insertions(+), 16 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor}
2021-02-26 16:20 [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor} Richard Henderson
2021-02-26 16:20 ` [PATCH 1/2] target/i386: Rename helper_fldt, helper_fstt Richard Henderson
2021-02-26 16:20 ` [PATCH 2/2] target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor Richard Henderson
@ 2021-02-26 16:59 ` Claudio Fontana
2021-02-26 17:12 ` Paolo Bonzini
2 siblings, 1 reply; 8+ messages in thread
From: Claudio Fontana @ 2021-02-26 16:59 UTC (permalink / raw
To: Richard Henderson, qemu-devel; +Cc: pbonzini, philmd, ehabkost
On 2/26/21 5:20 PM, Richard Henderson wrote:
> As discussed during review of Claudio's "i386 cleanup" patch set.
>
>
> r~
>
>
> Richard Henderson (2):
> target/i386: Rename helper_fldt, helper_fstt
> target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor
>
> target/i386/tcg/fpu_helper.c | 65 +++++++++++++++++++++++-------------
> 1 file changed, 41 insertions(+), 24 deletions(-)
>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
Tested-by: Claudio Fontana <cfontana@suse.de>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor}
2021-02-26 16:59 ` [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor} Claudio Fontana
@ 2021-02-26 17:12 ` Paolo Bonzini
2021-02-26 17:13 ` Claudio Fontana
0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2021-02-26 17:12 UTC (permalink / raw
To: Claudio Fontana, Richard Henderson, qemu-devel; +Cc: philmd, ehabkost
On 26/02/21 17:59, Claudio Fontana wrote:
> On 2/26/21 5:20 PM, Richard Henderson wrote:
>> As discussed during review of Claudio's "i386 cleanup" patch set.
>>
>>
>> r~
>>
>>
>> Richard Henderson (2):
>> target/i386: Rename helper_fldt, helper_fstt
>> target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor
>>
>> target/i386/tcg/fpu_helper.c | 65 +++++++++++++++++++++++-------------
>> 1 file changed, 41 insertions(+), 24 deletions(-)
>>
> Reviewed-by: Claudio Fontana <cfontana@suse.de>
> Tested-by: Claudio Fontana <cfontana@suse.de>
>
Claudio, can you merge them in your patch set?
Thanks,
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor}
2021-02-26 17:12 ` Paolo Bonzini
@ 2021-02-26 17:13 ` Claudio Fontana
0 siblings, 0 replies; 8+ messages in thread
From: Claudio Fontana @ 2021-02-26 17:13 UTC (permalink / raw
To: Paolo Bonzini, Richard Henderson, qemu-devel; +Cc: philmd, ehabkost
On 2/26/21 6:12 PM, Paolo Bonzini wrote:
> On 26/02/21 17:59, Claudio Fontana wrote:
>> On 2/26/21 5:20 PM, Richard Henderson wrote:
>>> As discussed during review of Claudio's "i386 cleanup" patch set.
>>>
>>>
>>> r~
>>>
>>>
>>> Richard Henderson (2):
>>> target/i386: Rename helper_fldt, helper_fstt
>>> target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor
>>>
>>> target/i386/tcg/fpu_helper.c | 65 +++++++++++++++++++++++-------------
>>> 1 file changed, 41 insertions(+), 24 deletions(-)
>>>
>> Reviewed-by: Claudio Fontana <cfontana@suse.de>
>> Tested-by: Claudio Fontana <cfontana@suse.de>
>>
>
> Claudio, can you merge them in your patch set?
>
> Thanks,
>
> Paolo
>
Yep, doing that.
Ciao,
Claudio
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-02-26 17:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-26 16:20 [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor} Richard Henderson
2021-02-26 16:20 ` [PATCH 1/2] target/i386: Rename helper_fldt, helper_fstt Richard Henderson
2021-02-26 16:45 ` Philippe Mathieu-Daudé
2021-02-26 16:20 ` [PATCH 2/2] target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor Richard Henderson
2021-02-26 16:47 ` Philippe Mathieu-Daudé
2021-02-26 16:59 ` [PATCH 0/2] target/i386: Fix cpu_x86_{fsave,frstor,fxsave,fxrstor} Claudio Fontana
2021-02-26 17:12 ` Paolo Bonzini
2021-02-26 17:13 ` Claudio Fontana
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.