smm is only really useful for softmmu, split in two modules
around the CONFIG_USER_ONLY, in order to remove the ifdef
and use the build system instead.
Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
target/i386/helper.h | 4 ++++
target/i386/tcg/seg_helper.c | 2 ++
target/i386/tcg/{ => softmmu}/smm_helper.c | 19 ++-----------------
target/i386/tcg/translate.c | 2 ++
target/i386/tcg/meson.build | 1 -
target/i386/tcg/softmmu/meson.build | 1 +
6 files changed, 11 insertions(+), 18 deletions(-)
rename target/i386/tcg/{ => softmmu}/smm_helper.c (98%)
diff --git a/target/i386/helper.h b/target/i386/helper.h
index c2ae2f7e61..8ffda4cdc6 100644
--- a/target/i386/helper.h
+++ b/target/i386/helper.h
@@ -70,7 +70,11 @@ DEF_HELPER_1(clac, void, env)
DEF_HELPER_1(stac, void, env)
DEF_HELPER_3(boundw, void, env, tl, int)
DEF_HELPER_3(boundl, void, env, tl, int)
+
+#ifndef CONFIG_USER_ONLY
DEF_HELPER_1(rsm, void, env)
+#endif /* !CONFIG_USER_ONLY */
+
DEF_HELPER_2(into, void, env, int)
DEF_HELPER_2(cmpxchg8b_unlocked, void, env, tl)
DEF_HELPER_2(cmpxchg8b, void, env, tl)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 180d47f0e9..f0cb1bffe7 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -1351,7 +1351,9 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
case CPU_INTERRUPT_SMI:
cpu_svm_check_intercept_param(env, SVM_EXIT_SMI, 0, 0);
cs->interrupt_request &= ~CPU_INTERRUPT_SMI;
+#ifndef CONFIG_USER_ONLY
do_smm_enter(cpu);
+#endif
break;
case CPU_INTERRUPT_NMI:
cpu_svm_check_intercept_param(env, SVM_EXIT_NMI, 0, 0);
diff --git a/target/i386/tcg/smm_helper.c b/target/i386/tcg/softmmu/smm_helper.c
similarity index 98%
rename from target/i386/tcg/smm_helper.c
rename to target/i386/tcg/softmmu/smm_helper.c
index 62d027abd3..ee53b26629 100644
--- a/target/i386/tcg/smm_helper.c
+++ b/target/i386/tcg/softmmu/smm_helper.c
@@ -1,5 +1,5 @@
/*
- * x86 SMM helpers
+ * x86 SMM helpers (softmmu-only)
*
* Copyright (c) 2003 Fabrice Bellard
*
@@ -18,27 +18,14 @@
*/
#include "qemu/osdep.h"
-#include "qemu/main-loop.h"
#include "cpu.h"
#include "exec/helper-proto.h"
#include "exec/log.h"
-#include "helper-tcg.h"
+#include "tcg/helper-tcg.h"
/* SMM support */
-#if defined(CONFIG_USER_ONLY)
-
-void do_smm_enter(X86CPU *cpu)
-{
-}
-
-void helper_rsm(CPUX86State *env)
-{
-}
-
-#else
-
#ifdef TARGET_X86_64
#define SMM_REVISION_ID 0x00020064
#else
@@ -330,5 +317,3 @@ void helper_rsm(CPUX86State *env)
qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");
log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);
}
-
-#endif /* !CONFIG_USER_ONLY */
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index af1faf9342..5075ac4830 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -8321,7 +8321,9 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
goto illegal_op;
gen_update_cc_op(s);
gen_jmp_im(s, s->pc - s->cs_base);
+#ifndef CONFIG_USER_ONLY
gen_helper_rsm(cpu_env);
+#endif /* CONFIG_USER_ONLY */
gen_eob(s);
break;
case 0x1b8: /* SSE4.2 popcnt */
diff --git a/target/i386/tcg/meson.build b/target/i386/tcg/meson.build
index 68fa0c3187..ec5daa1edc 100644
--- a/target/i386/tcg/meson.build
+++ b/target/i386/tcg/meson.build
@@ -8,7 +8,6 @@ i386_ss.add(when: 'CONFIG_TCG', if_true: files(
'misc_helper.c',
'mpx_helper.c',
'seg_helper.c',
- 'smm_helper.c',
'svm_helper.c',
'tcg-cpu.c',
'translate.c'), if_false: files('tcg-stub.c'))
diff --git a/target/i386/tcg/softmmu/meson.build b/target/i386/tcg/softmmu/meson.build
index 4ab30cc32e..35ba16dc3d 100644
--- a/target/i386/tcg/softmmu/meson.build
+++ b/target/i386/tcg/softmmu/meson.build
@@ -1,3 +1,4 @@
i386_softmmu_ss.add(when: ['CONFIG_TCG', 'CONFIG_SOFTMMU'], if_true: files(
'tcg-cpu.c',
+ 'smm_helper.c',
))
--
2.26.2
On 2/12/21 1:36 PM, Claudio Fontana wrote:
> smm is only really useful for softmmu, split in two modules
> around the CONFIG_USER_ONLY, in order to remove the ifdef
> and use the build system instead.
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
> target/i386/helper.h | 4 ++++
> target/i386/tcg/seg_helper.c | 2 ++
> target/i386/tcg/{ => softmmu}/smm_helper.c | 19 ++-----------------
> target/i386/tcg/translate.c | 2 ++
> target/i386/tcg/meson.build | 1 -
> target/i386/tcg/softmmu/meson.build | 1 +
> 6 files changed, 11 insertions(+), 18 deletions(-)
> rename target/i386/tcg/{ => softmmu}/smm_helper.c (98%)
>
> diff --git a/target/i386/helper.h b/target/i386/helper.h
> index c2ae2f7e61..8ffda4cdc6 100644
> --- a/target/i386/helper.h
> +++ b/target/i386/helper.h
> @@ -70,7 +70,11 @@ DEF_HELPER_1(clac, void, env)
> DEF_HELPER_1(stac, void, env)
> DEF_HELPER_3(boundw, void, env, tl, int)
> DEF_HELPER_3(boundl, void, env, tl, int)
> +
> +#ifndef CONFIG_USER_ONLY
> DEF_HELPER_1(rsm, void, env)
> +#endif /* !CONFIG_USER_ONLY */
> +
> DEF_HELPER_2(into, void, env, int)
> DEF_HELPER_2(cmpxchg8b_unlocked, void, env, tl)
> DEF_HELPER_2(cmpxchg8b, void, env, tl)
> diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
> index 180d47f0e9..f0cb1bffe7 100644
> --- a/target/i386/tcg/seg_helper.c
> +++ b/target/i386/tcg/seg_helper.c
> @@ -1351,7 +1351,9 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> case CPU_INTERRUPT_SMI:
> cpu_svm_check_intercept_param(env, SVM_EXIT_SMI, 0, 0);
> cs->interrupt_request &= ~CPU_INTERRUPT_SMI;
> +#ifndef CONFIG_USER_ONLY
> do_smm_enter(cpu);
> +#endif
> break;
> case CPU_INTERRUPT_NMI:
> cpu_svm_check_intercept_param(env, SVM_EXIT_NMI, 0, 0);
> diff --git a/target/i386/tcg/smm_helper.c b/target/i386/tcg/softmmu/smm_helper.c
> similarity index 98%
> rename from target/i386/tcg/smm_helper.c
> rename to target/i386/tcg/softmmu/smm_helper.c
> index 62d027abd3..ee53b26629 100644
> --- a/target/i386/tcg/smm_helper.c
> +++ b/target/i386/tcg/softmmu/smm_helper.c
> @@ -1,5 +1,5 @@
> /*
> - * x86 SMM helpers
> + * x86 SMM helpers (softmmu-only)
> *
> * Copyright (c) 2003 Fabrice Bellard
> *
> @@ -18,27 +18,14 @@
> */
>
> #include "qemu/osdep.h"
> -#include "qemu/main-loop.h"
> #include "cpu.h"
> #include "exec/helper-proto.h"
> #include "exec/log.h"
> -#include "helper-tcg.h"
> +#include "tcg/helper-tcg.h"
>
>
> /* SMM support */
>
> -#if defined(CONFIG_USER_ONLY)
> -
> -void do_smm_enter(X86CPU *cpu)
> -{
> -}
> -
> -void helper_rsm(CPUX86State *env)
> -{
> -}
> -
> -#else
> -
> #ifdef TARGET_X86_64
> #define SMM_REVISION_ID 0x00020064
> #else
> @@ -330,5 +317,3 @@ void helper_rsm(CPUX86State *env)
> qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");
> log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);
> }
> -
> -#endif /* !CONFIG_USER_ONLY */
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index af1faf9342..5075ac4830 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -8321,7 +8321,9 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
> goto illegal_op;
> gen_update_cc_op(s);
> gen_jmp_im(s, s->pc - s->cs_base);
> +#ifndef CONFIG_USER_ONLY
> gen_helper_rsm(cpu_env);
> +#endif /* CONFIG_USER_ONLY */
> gen_eob(s);
> break;
Hello Alex,
this is something I wanted to bring in the foreground:
while before we were generating an empty helper call for CONFIG_USER_ONLY,
now we are not generating anything.
> case 0x1b8: /* SSE4.2 popcnt */
> diff --git a/target/i386/tcg/meson.build b/target/i386/tcg/meson.build
> index 68fa0c3187..ec5daa1edc 100644
> --- a/target/i386/tcg/meson.build
> +++ b/target/i386/tcg/meson.build
> @@ -8,7 +8,6 @@ i386_ss.add(when: 'CONFIG_TCG', if_true: files(
> 'misc_helper.c',
> 'mpx_helper.c',
> 'seg_helper.c',
> - 'smm_helper.c',
> 'svm_helper.c',
> 'tcg-cpu.c',
> 'translate.c'), if_false: files('tcg-stub.c'))
> diff --git a/target/i386/tcg/softmmu/meson.build b/target/i386/tcg/softmmu/meson.build
> index 4ab30cc32e..35ba16dc3d 100644
> --- a/target/i386/tcg/softmmu/meson.build
> +++ b/target/i386/tcg/softmmu/meson.build
> @@ -1,3 +1,4 @@
> i386_softmmu_ss.add(when: ['CONFIG_TCG', 'CONFIG_SOFTMMU'], if_true: files(
> 'tcg-cpu.c',
> + 'smm_helper.c',
> ))
>
Claudio Fontana <cfontana@suse.de> writes:
> On 2/12/21 1:36 PM, Claudio Fontana wrote:
>> smm is only really useful for softmmu, split in two modules
>> around the CONFIG_USER_ONLY, in order to remove the ifdef
>> and use the build system instead.
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> ---
>> target/i386/helper.h | 4 ++++
>> target/i386/tcg/seg_helper.c | 2 ++
>> target/i386/tcg/{ => softmmu}/smm_helper.c | 19 ++-----------------
>> target/i386/tcg/translate.c | 2 ++
>> target/i386/tcg/meson.build | 1 -
>> target/i386/tcg/softmmu/meson.build | 1 +
>> 6 files changed, 11 insertions(+), 18 deletions(-)
>> rename target/i386/tcg/{ => softmmu}/smm_helper.c (98%)
>>
>> diff --git a/target/i386/helper.h b/target/i386/helper.h
>> index c2ae2f7e61..8ffda4cdc6 100644
>> --- a/target/i386/helper.h
>> +++ b/target/i386/helper.h
>> @@ -70,7 +70,11 @@ DEF_HELPER_1(clac, void, env)
>> DEF_HELPER_1(stac, void, env)
>> DEF_HELPER_3(boundw, void, env, tl, int)
>> DEF_HELPER_3(boundl, void, env, tl, int)
>> +
>> +#ifndef CONFIG_USER_ONLY
>> DEF_HELPER_1(rsm, void, env)
>> +#endif /* !CONFIG_USER_ONLY */
>> +
>> DEF_HELPER_2(into, void, env, int)
>> DEF_HELPER_2(cmpxchg8b_unlocked, void, env, tl)
>> DEF_HELPER_2(cmpxchg8b, void, env, tl)
>> diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
>> index 180d47f0e9..f0cb1bffe7 100644
>> --- a/target/i386/tcg/seg_helper.c
>> +++ b/target/i386/tcg/seg_helper.c
>> @@ -1351,7 +1351,9 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>> case CPU_INTERRUPT_SMI:
>> cpu_svm_check_intercept_param(env, SVM_EXIT_SMI, 0, 0);
>> cs->interrupt_request &= ~CPU_INTERRUPT_SMI;
>> +#ifndef CONFIG_USER_ONLY
>> do_smm_enter(cpu);
>> +#endif
>> break;
>> case CPU_INTERRUPT_NMI:
>> cpu_svm_check_intercept_param(env, SVM_EXIT_NMI, 0, 0);
>> diff --git a/target/i386/tcg/smm_helper.c b/target/i386/tcg/softmmu/smm_helper.c
>> similarity index 98%
>> rename from target/i386/tcg/smm_helper.c
>> rename to target/i386/tcg/softmmu/smm_helper.c
>> index 62d027abd3..ee53b26629 100644
>> --- a/target/i386/tcg/smm_helper.c
>> +++ b/target/i386/tcg/softmmu/smm_helper.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * x86 SMM helpers
>> + * x86 SMM helpers (softmmu-only)
>> *
>> * Copyright (c) 2003 Fabrice Bellard
>> *
>> @@ -18,27 +18,14 @@
>> */
>>
>> #include "qemu/osdep.h"
>> -#include "qemu/main-loop.h"
>> #include "cpu.h"
>> #include "exec/helper-proto.h"
>> #include "exec/log.h"
>> -#include "helper-tcg.h"
>> +#include "tcg/helper-tcg.h"
>>
>>
>> /* SMM support */
>>
>> -#if defined(CONFIG_USER_ONLY)
>> -
>> -void do_smm_enter(X86CPU *cpu)
>> -{
>> -}
>> -
>> -void helper_rsm(CPUX86State *env)
>> -{
>> -}
>> -
>> -#else
>> -
>> #ifdef TARGET_X86_64
>> #define SMM_REVISION_ID 0x00020064
>> #else
>> @@ -330,5 +317,3 @@ void helper_rsm(CPUX86State *env)
>> qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");
>> log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);
>> }
>> -
>> -#endif /* !CONFIG_USER_ONLY */
>> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
>> index af1faf9342..5075ac4830 100644
>> --- a/target/i386/tcg/translate.c
>> +++ b/target/i386/tcg/translate.c
>> @@ -8321,7 +8321,9 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
>> goto illegal_op;
>> gen_update_cc_op(s);
>> gen_jmp_im(s, s->pc - s->cs_base);
>> +#ifndef CONFIG_USER_ONLY
>> gen_helper_rsm(cpu_env);
>> +#endif /* CONFIG_USER_ONLY */
>> gen_eob(s);
>> break;
>
> Hello Alex,
>
> this is something I wanted to bring in the foreground:
>
> while before we were generating an empty helper call for CONFIG_USER_ONLY,
> now we are not generating anything.
Surely that says we only generate the helper call when we are not
CONFIG_USER_ONLY?
>
>
>
>> case 0x1b8: /* SSE4.2 popcnt */
>> diff --git a/target/i386/tcg/meson.build b/target/i386/tcg/meson.build
>> index 68fa0c3187..ec5daa1edc 100644
>> --- a/target/i386/tcg/meson.build
>> +++ b/target/i386/tcg/meson.build
>> @@ -8,7 +8,6 @@ i386_ss.add(when: 'CONFIG_TCG', if_true: files(
>> 'misc_helper.c',
>> 'mpx_helper.c',
>> 'seg_helper.c',
>> - 'smm_helper.c',
>> 'svm_helper.c',
>> 'tcg-cpu.c',
>> 'translate.c'), if_false: files('tcg-stub.c'))
>> diff --git a/target/i386/tcg/softmmu/meson.build b/target/i386/tcg/softmmu/meson.build
>> index 4ab30cc32e..35ba16dc3d 100644
>> --- a/target/i386/tcg/softmmu/meson.build
>> +++ b/target/i386/tcg/softmmu/meson.build
>> @@ -1,3 +1,4 @@
>> i386_softmmu_ss.add(when: ['CONFIG_TCG', 'CONFIG_SOFTMMU'], if_true: files(
>> 'tcg-cpu.c',
>> + 'smm_helper.c',
>> ))
>>
--
Alex Bennée
On 2/15/21 1:32 PM, Alex Bennée wrote:
>
> Claudio Fontana <cfontana@suse.de> writes:
>
>> On 2/12/21 1:36 PM, Claudio Fontana wrote:
>>> smm is only really useful for softmmu, split in two modules
>>> around the CONFIG_USER_ONLY, in order to remove the ifdef
>>> and use the build system instead.
>>>
>>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>>> ---
>>> target/i386/helper.h | 4 ++++
>>> target/i386/tcg/seg_helper.c | 2 ++
>>> target/i386/tcg/{ => softmmu}/smm_helper.c | 19 ++-----------------
>>> target/i386/tcg/translate.c | 2 ++
>>> target/i386/tcg/meson.build | 1 -
>>> target/i386/tcg/softmmu/meson.build | 1 +
>>> 6 files changed, 11 insertions(+), 18 deletions(-)
>>> rename target/i386/tcg/{ => softmmu}/smm_helper.c (98%)
>>>
>>> diff --git a/target/i386/helper.h b/target/i386/helper.h
>>> index c2ae2f7e61..8ffda4cdc6 100644
>>> --- a/target/i386/helper.h
>>> +++ b/target/i386/helper.h
>>> @@ -70,7 +70,11 @@ DEF_HELPER_1(clac, void, env)
>>> DEF_HELPER_1(stac, void, env)
>>> DEF_HELPER_3(boundw, void, env, tl, int)
>>> DEF_HELPER_3(boundl, void, env, tl, int)
>>> +
>>> +#ifndef CONFIG_USER_ONLY
>>> DEF_HELPER_1(rsm, void, env)
>>> +#endif /* !CONFIG_USER_ONLY */
>>> +
>>> DEF_HELPER_2(into, void, env, int)
>>> DEF_HELPER_2(cmpxchg8b_unlocked, void, env, tl)
>>> DEF_HELPER_2(cmpxchg8b, void, env, tl)
>>> diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
>>> index 180d47f0e9..f0cb1bffe7 100644
>>> --- a/target/i386/tcg/seg_helper.c
>>> +++ b/target/i386/tcg/seg_helper.c
>>> @@ -1351,7 +1351,9 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>>> case CPU_INTERRUPT_SMI:
>>> cpu_svm_check_intercept_param(env, SVM_EXIT_SMI, 0, 0);
>>> cs->interrupt_request &= ~CPU_INTERRUPT_SMI;
>>> +#ifndef CONFIG_USER_ONLY
>>> do_smm_enter(cpu);
>>> +#endif
>>> break;
>>> case CPU_INTERRUPT_NMI:
>>> cpu_svm_check_intercept_param(env, SVM_EXIT_NMI, 0, 0);
>>> diff --git a/target/i386/tcg/smm_helper.c b/target/i386/tcg/softmmu/smm_helper.c
>>> similarity index 98%
>>> rename from target/i386/tcg/smm_helper.c
>>> rename to target/i386/tcg/softmmu/smm_helper.c
>>> index 62d027abd3..ee53b26629 100644
>>> --- a/target/i386/tcg/smm_helper.c
>>> +++ b/target/i386/tcg/softmmu/smm_helper.c
>>> @@ -1,5 +1,5 @@
>>> /*
>>> - * x86 SMM helpers
>>> + * x86 SMM helpers (softmmu-only)
>>> *
>>> * Copyright (c) 2003 Fabrice Bellard
>>> *
>>> @@ -18,27 +18,14 @@
>>> */
>>>
>>> #include "qemu/osdep.h"
>>> -#include "qemu/main-loop.h"
>>> #include "cpu.h"
>>> #include "exec/helper-proto.h"
>>> #include "exec/log.h"
>>> -#include "helper-tcg.h"
>>> +#include "tcg/helper-tcg.h"
>>>
>>>
>>> /* SMM support */
>>>
>>> -#if defined(CONFIG_USER_ONLY)
>>> -
>>> -void do_smm_enter(X86CPU *cpu)
>>> -{
>>> -}
>>> -
>>> -void helper_rsm(CPUX86State *env)
>>> -{
>>> -}
>>> -
>>> -#else
>>> -
>>> #ifdef TARGET_X86_64
>>> #define SMM_REVISION_ID 0x00020064
>>> #else
>>> @@ -330,5 +317,3 @@ void helper_rsm(CPUX86State *env)
>>> qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");
>>> log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);
>>> }
>>> -
>>> -#endif /* !CONFIG_USER_ONLY */
>>> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
>>> index af1faf9342..5075ac4830 100644
>>> --- a/target/i386/tcg/translate.c
>>> +++ b/target/i386/tcg/translate.c
>>> @@ -8321,7 +8321,9 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
>>> goto illegal_op;
>>> gen_update_cc_op(s);
>>> gen_jmp_im(s, s->pc - s->cs_base);
>>> +#ifndef CONFIG_USER_ONLY
>>> gen_helper_rsm(cpu_env);
>>> +#endif /* CONFIG_USER_ONLY */
>>> gen_eob(s);
>>> break;
>>
>> Hello Alex,
>>
>> this is something I wanted to bring in the foreground:
>>
>> while before we were generating an empty helper call for CONFIG_USER_ONLY,
>> now we are not generating anything.
>
> Surely that says we only generate the helper call when we are not
> CONFIG_USER_ONLY?
Yes. The difference between before the patch and after the patch
is that before we were still going through all the code in tcg_gen_callN,
via the call to gen_helper_rsm macro, only to call finally an empty function for CONFIG_USER_ONLY (helper_rsm() {}),
while now we do not generate anything, we do not call the gen_helper_rsm macro at all, so we don't go through tcg_gen_callN.
>
>
>>
>>
>>
>>> case 0x1b8: /* SSE4.2 popcnt */
>>> diff --git a/target/i386/tcg/meson.build b/target/i386/tcg/meson.build
>>> index 68fa0c3187..ec5daa1edc 100644
>>> --- a/target/i386/tcg/meson.build
>>> +++ b/target/i386/tcg/meson.build
>>> @@ -8,7 +8,6 @@ i386_ss.add(when: 'CONFIG_TCG', if_true: files(
>>> 'misc_helper.c',
>>> 'mpx_helper.c',
>>> 'seg_helper.c',
>>> - 'smm_helper.c',
>>> 'svm_helper.c',
>>> 'tcg-cpu.c',
>>> 'translate.c'), if_false: files('tcg-stub.c'))
>>> diff --git a/target/i386/tcg/softmmu/meson.build b/target/i386/tcg/softmmu/meson.build
>>> index 4ab30cc32e..35ba16dc3d 100644
>>> --- a/target/i386/tcg/softmmu/meson.build
>>> +++ b/target/i386/tcg/softmmu/meson.build
>>> @@ -1,3 +1,4 @@
>>> i386_softmmu_ss.add(when: ['CONFIG_TCG', 'CONFIG_SOFTMMU'], if_true: files(
>>> 'tcg-cpu.c',
>>> + 'smm_helper.c',
>>> ))
>>>
>
>
On 15/02/21 13:59, Claudio Fontana wrote:
> Yes. The difference between before the patch and after the patch
> is that before we were still going through all the code in tcg_gen_callN,
> via the call to gen_helper_rsm macro, only to call finally an empty function for CONFIG_USER_ONLY (helper_rsm() {}),
>
> while now we do not generate anything, we do not call the gen_helper_rsm macro at all, so we don't go through tcg_gen_callN.
>
Can we even have an abort() for such cases?
Paolo
On 2/15/21 2:30 PM, Paolo Bonzini wrote:
> On 15/02/21 13:59, Claudio Fontana wrote:
>> Yes. The difference between before the patch and after the patch
>> is that before we were still going through all the code in tcg_gen_callN,
>> via the call to gen_helper_rsm macro, only to call finally an empty function for CONFIG_USER_ONLY (helper_rsm() {}),
>>
>> while now we do not generate anything, we do not call the gen_helper_rsm macro at all, so we don't go through tcg_gen_callN.
>>
>
> Can we even have an abort() for such cases?
>
> Paolo
>
Hi Paolo,
where are you suggesting to have an abort()?
You mean that we should abort() QEMU as soon as we detect in translate.c an RSM instruction in user-mode?
Again the translate.c code for reference:
case 0x1aa: /* rsm */
gen_svm_check_intercept(s, pc_start, SVM_EXIT_RSM);
if (!(s->flags & HF_SMM_MASK))
goto illegal_op;
gen_update_cc_op(s);
gen_jmp_im(s, s->pc - s->cs_base);
#ifndef CONFIG_USER_ONLY
gen_helper_rsm(cpu_env);
#endif /* CONFIG_USER_ONLY */
gen_eob(s);
break;
---
Thanks,
CLaudio
On 15/02/21 15:05, Claudio Fontana wrote:
> On 2/15/21 2:30 PM, Paolo Bonzini wrote:
>> On 15/02/21 13:59, Claudio Fontana wrote:
>>> Yes. The difference between before the patch and after the patch
>>> is that before we were still going through all the code in
>>> tcg_gen_callN, via the call to gen_helper_rsm macro, only to call
>>> finally an empty function for CONFIG_USER_ONLY (helper_rsm()
>>> {}),
>>>
>>> while now we do not generate anything, we do not call the
>>> gen_helper_rsm macro at all, so we don't go through
>>> tcg_gen_callN.
>>>
>>
>> Can we even have an abort() for such cases?
>>
>> Paolo
>>
>
> Hi Paolo,
>
> where are you suggesting to have an abort()?
>
> You mean that we should abort() QEMU as soon as we detect in
> translate.c an RSM instruction in user-mode?
Translating it is okay (it's just a guaranteed SIGILL), but I'm thinking
of aborting if s->flags & HF_SMM_MASK is true. Likewise if we see
CPU_INTERRUPT_SMI.
Paolo
>
> case 0x1aa: /* rsm */
> gen_svm_check_intercept(s, pc_start, SVM_EXIT_RSM);
> if (!(s->flags & HF_SMM_MASK))
> goto illegal_op;
> gen_update_cc_op(s);
> gen_jmp_im(s, s->pc - s->cs_base);
> #ifndef CONFIG_USER_ONLY
> gen_helper_rsm(cpu_env);
> #endif /* CONFIG_USER_ONLY */
> gen_eob(s);
> break;
On 2/15/21 3:13 PM, Paolo Bonzini wrote:
> On 15/02/21 15:05, Claudio Fontana wrote:
>> On 2/15/21 2:30 PM, Paolo Bonzini wrote:
>>> On 15/02/21 13:59, Claudio Fontana wrote:
>>>> Yes. The difference between before the patch and after the patch
>>>> is that before we were still going through all the code in
>>>> tcg_gen_callN, via the call to gen_helper_rsm macro, only to call
>>>> finally an empty function for CONFIG_USER_ONLY (helper_rsm()
>>>> {}),
>>>>
>>>> while now we do not generate anything, we do not call the
>>>> gen_helper_rsm macro at all, so we don't go through
>>>> tcg_gen_callN.
>>>>
>>>
>>> Can we even have an abort() for such cases?
>>>
>>> Paolo
>>>
>>
>> Hi Paolo,
>>
>> where are you suggesting to have an abort()?
>>
>> You mean that we should abort() QEMU as soon as we detect in
>> translate.c an RSM instruction in user-mode?
>
> Translating it is okay (it's just a guaranteed SIGILL), but I'm thinking
> of aborting if s->flags & HF_SMM_MASK is true. Likewise if we see
> CPU_INTERRUPT_SMI.
>
> Paolo
>
Ok, will rework as you suggest, thanks!
>>
>> case 0x1aa: /* rsm */
>> gen_svm_check_intercept(s, pc_start, SVM_EXIT_RSM);
>> if (!(s->flags & HF_SMM_MASK))
>> goto illegal_op;
>> gen_update_cc_op(s);
>> gen_jmp_im(s, s->pc - s->cs_base);
>> #ifndef CONFIG_USER_ONLY
>> gen_helper_rsm(cpu_env);
>> #endif /* CONFIG_USER_ONLY */
>> gen_eob(s);
>> break;
>
>
On 2/15/21 3:39 PM, Claudio Fontana wrote:
> On 2/15/21 3:13 PM, Paolo Bonzini wrote:
>> On 15/02/21 15:05, Claudio Fontana wrote:
>>> On 2/15/21 2:30 PM, Paolo Bonzini wrote:
>>>> On 15/02/21 13:59, Claudio Fontana wrote:
>>>>> Yes. The difference between before the patch and after the patch
>>>>> is that before we were still going through all the code in
>>>>> tcg_gen_callN, via the call to gen_helper_rsm macro, only to call
>>>>> finally an empty function for CONFIG_USER_ONLY (helper_rsm()
>>>>> {}),
>>>>>
>>>>> while now we do not generate anything, we do not call the
>>>>> gen_helper_rsm macro at all, so we don't go through
>>>>> tcg_gen_callN.
>>>>>
>>>>
>>>> Can we even have an abort() for such cases?
>>>>
>>>> Paolo
>>>>
>>>
>>> Hi Paolo,
>>>
>>> where are you suggesting to have an abort()?
>>>
>>> You mean that we should abort() QEMU as soon as we detect in
>>> translate.c an RSM instruction in user-mode?
>>
>> Translating it is okay (it's just a guaranteed SIGILL), but I'm thinking
>> of aborting if s->flags & HF_SMM_MASK is true. Likewise if we see
>> CPU_INTERRUPT_SMI.
>>
>> Paolo
>>
>
> Ok, will rework as you suggest, thanks!
By the way, in the case of gen_bpt_io, is it a similar situation,
where we should abort in user-mode if we see s->flags & HF_IOBPT_MASK ?
static void gen_bpt_io(DisasContext *s, TCGv_i32 t_port, int ot)
{
#ifndef CONFIG_USER_ONLY
if (s->flags & HF_IOBPT_MASK) {
TCGv_i32 t_size = tcg_const_i32(1 << ot);
TCGv t_next = tcg_const_tl(s->pc - s->cs_base);
gen_helper_bpt_io(cpu_env, t_port, t_size, t_next);
tcg_temp_free_i32(t_size);
tcg_temp_free(t_next);
}
#endif /* !CONFIG_USER_ONLY */
}
What about other cases like
case 0xd8: /* VMRUN */
if (!(s->flags & HF_SVME_MASK) || !s->pe) {
goto illegal_op;
}
...
gen_helper_vmrun(cpu_env, tcg_const_i32(s->aflag - 1),
tcg_const_i32(s->pc - pc_start));
should we abort there as well if CONFIG_USER_ONLY?
And there are many more probably, should it be its own patch?
Ciao,
Claudio
>
>>>
>>> case 0x1aa: /* rsm */
>>> gen_svm_check_intercept(s, pc_start, SVM_EXIT_RSM);
>>> if (!(s->flags & HF_SMM_MASK))
>>> goto illegal_op;
>>> gen_update_cc_op(s);
>>> gen_jmp_im(s, s->pc - s->cs_base);
>>> #ifndef CONFIG_USER_ONLY
>>> gen_helper_rsm(cpu_env);
>>> #endif /* CONFIG_USER_ONLY */
>>> gen_eob(s);
>>> break;
>>
>>
>
>
© 2016 - 2026 Red Hat, Inc.