Add ALLINT MSR (immediate) to decodetree. And the EL0 check is necessary
to ALLINT. Avoid the unconditional write to pc and use raise_exception_ra
to unwind.
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
target/arm/tcg/a64.decode | 1 +
target/arm/tcg/helper-a64.c | 24 ++++++++++++++++++++++++
target/arm/tcg/helper-a64.h | 1 +
target/arm/tcg/translate-a64.c | 10 ++++++++++
4 files changed, 36 insertions(+)
diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index 8a20dce3c8..3588080024 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -207,6 +207,7 @@ MSR_i_DIT 1101 0101 0000 0 011 0100 .... 010 11111 @msr_i
MSR_i_TCO 1101 0101 0000 0 011 0100 .... 100 11111 @msr_i
MSR_i_DAIFSET 1101 0101 0000 0 011 0100 .... 110 11111 @msr_i
MSR_i_DAIFCLEAR 1101 0101 0000 0 011 0100 .... 111 11111 @msr_i
+MSR_i_ALLINT 1101 0101 0000 0 001 0100 .... 000 11111 @msr_i
MSR_i_SVCR 1101 0101 0000 0 011 0100 0 mask:2 imm:1 011 11111
# MRS, MSR (register), SYS, SYSL. These are all essentially the
diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index ebaa7f00df..3686926ada 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -66,6 +66,30 @@ void HELPER(msr_i_spsel)(CPUARMState *env, uint32_t imm)
update_spsel(env, imm);
}
+static void allint_check(CPUARMState *env, uint32_t op,
+ uint32_t imm, uintptr_t ra)
+{
+ /* ALLINT update to PSTATE. */
+ if (arm_current_el(env) == 0) {
+ raise_exception_ra(env, EXCP_UDEF,
+ syn_aa64_sysregtrap(0, extract32(op, 0, 3),
+ extract32(op, 3, 3), 4,
+ imm, 0x1f, 0),
+ exception_target_el(env), ra);
+ }
+}
+
+void HELPER(msr_i_allint)(CPUARMState *env, uint32_t imm)
+{
+ allint_check(env, 0x8, imm, GETPC());
+ if (imm == 1) {
+ env->allint |= PSTATE_ALLINT;
+ } else {
+ env->allint &= ~PSTATE_ALLINT;
+ }
+ arm_rebuild_hflags(env);
+}
+
static void daif_check(CPUARMState *env, uint32_t op,
uint32_t imm, uintptr_t ra)
{
diff --git a/target/arm/tcg/helper-a64.h b/target/arm/tcg/helper-a64.h
index 575a5dab7d..3aec703d4a 100644
--- a/target/arm/tcg/helper-a64.h
+++ b/target/arm/tcg/helper-a64.h
@@ -22,6 +22,7 @@ DEF_HELPER_FLAGS_1(rbit64, TCG_CALL_NO_RWG_SE, i64, i64)
DEF_HELPER_2(msr_i_spsel, void, env, i32)
DEF_HELPER_2(msr_i_daifset, void, env, i32)
DEF_HELPER_2(msr_i_daifclear, void, env, i32)
+DEF_HELPER_2(msr_i_allint, void, env, i32)
DEF_HELPER_3(vfp_cmph_a64, i64, f16, f16, ptr)
DEF_HELPER_3(vfp_cmpeh_a64, i64, f16, f16, ptr)
DEF_HELPER_3(vfp_cmps_a64, i64, f32, f32, ptr)
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 340265beb0..f1800f7c71 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -2036,6 +2036,16 @@ static bool trans_MSR_i_DAIFCLEAR(DisasContext *s, arg_i *a)
return true;
}
+static bool trans_MSR_i_ALLINT(DisasContext *s, arg_i *a)
+{
+ if (!dc_isar_feature(aa64_nmi, s) || s->current_el == 0) {
+ return false;
+ }
+ gen_helper_msr_i_allint(tcg_env, tcg_constant_i32(a->imm));
+ s->base.is_jmp = DISAS_TOO_MANY;
+ return true;
+}
+
static bool trans_MSR_i_SVCR(DisasContext *s, arg_MSR_i_SVCR *a)
{
if (!dc_isar_feature(aa64_sme, s) || a->mask == 0) {
--
2.34.1On 2/21/24 03:08, Jinjie Ruan via wrote:
> Add ALLINT MSR (immediate) to decodetree. And the EL0 check is necessary
> to ALLINT. Avoid the unconditional write to pc and use raise_exception_ra
> to unwind.
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> target/arm/tcg/a64.decode | 1 +
> target/arm/tcg/helper-a64.c | 24 ++++++++++++++++++++++++
> target/arm/tcg/helper-a64.h | 1 +
> target/arm/tcg/translate-a64.c | 10 ++++++++++
> 4 files changed, 36 insertions(+)
>
> diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
> index 8a20dce3c8..3588080024 100644
> --- a/target/arm/tcg/a64.decode
> +++ b/target/arm/tcg/a64.decode
> @@ -207,6 +207,7 @@ MSR_i_DIT 1101 0101 0000 0 011 0100 .... 010 11111 @msr_i
> MSR_i_TCO 1101 0101 0000 0 011 0100 .... 100 11111 @msr_i
> MSR_i_DAIFSET 1101 0101 0000 0 011 0100 .... 110 11111 @msr_i
> MSR_i_DAIFCLEAR 1101 0101 0000 0 011 0100 .... 111 11111 @msr_i
> +MSR_i_ALLINT 1101 0101 0000 0 001 0100 .... 000 11111 @msr_i
> MSR_i_SVCR 1101 0101 0000 0 011 0100 0 mask:2 imm:1 011 11111
>
> # MRS, MSR (register), SYS, SYSL. These are all essentially the
> diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
> index ebaa7f00df..3686926ada 100644
> --- a/target/arm/tcg/helper-a64.c
> +++ b/target/arm/tcg/helper-a64.c
> @@ -66,6 +66,30 @@ void HELPER(msr_i_spsel)(CPUARMState *env, uint32_t imm)
> update_spsel(env, imm);
> }
>
> +static void allint_check(CPUARMState *env, uint32_t op,
> + uint32_t imm, uintptr_t ra)
> +{
> + /* ALLINT update to PSTATE. */
> + if (arm_current_el(env) == 0) {
> + raise_exception_ra(env, EXCP_UDEF,
> + syn_aa64_sysregtrap(0, extract32(op, 0, 3),
> + extract32(op, 3, 3), 4,
> + imm, 0x1f, 0),
> + exception_target_el(env), ra);
> + }
> +}
A runtime check for EL0 is not necessary; you've already handled that in
trans_MSR_i_ALLINT(). However, what *is* missing here is the test against TALLINT for EL1.
> +
> +void HELPER(msr_i_allint)(CPUARMState *env, uint32_t imm)
> +{
> + allint_check(env, 0x8, imm, GETPC());
> + if (imm == 1) {
> + env->allint |= PSTATE_ALLINT;
> + } else {
> + env->allint &= ~PSTATE_ALLINT;
> + }
I think you should not write an immediate-specific helper, but one which can also handle
the variable "MSR allint, <xt>". This is no more difficult than
void HELPER(msr_allint)(CPUARMState *env, target_ulong val)
{
... check ...
env->pstate = (env->pstate & ~PSTATE_ALLINT) | (val & PSTATE_ALLINT);
}
> + arm_rebuild_hflags(env);
> +}
allint does not affect hflags; no rebuild required.
> +static bool trans_MSR_i_ALLINT(DisasContext *s, arg_i *a)
> +{
> + if (!dc_isar_feature(aa64_nmi, s) || s->current_el == 0) {
> + return false;
> + }
> + gen_helper_msr_i_allint(tcg_env, tcg_constant_i32(a->imm));
You're passing all of #imm4, not #imm1, which meant the test in your msr_i_allint helper
was wrong.
To work with the generalized helper above, this would be
tcg_constant_tl((a->imm & 1) * PSTATE_ALLINT);
r~
On 2024/2/22 3:09, Richard Henderson wrote:
> On 2/21/24 03:08, Jinjie Ruan via wrote:
>> Add ALLINT MSR (immediate) to decodetree. And the EL0 check is necessary
>> to ALLINT. Avoid the unconditional write to pc and use raise_exception_ra
>> to unwind.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>> target/arm/tcg/a64.decode | 1 +
>> target/arm/tcg/helper-a64.c | 24 ++++++++++++++++++++++++
>> target/arm/tcg/helper-a64.h | 1 +
>> target/arm/tcg/translate-a64.c | 10 ++++++++++
>> 4 files changed, 36 insertions(+)
>>
>> diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
>> index 8a20dce3c8..3588080024 100644
>> --- a/target/arm/tcg/a64.decode
>> +++ b/target/arm/tcg/a64.decode
>> @@ -207,6 +207,7 @@ MSR_i_DIT 1101 0101 0000 0 011 0100 .... 010
>> 11111 @msr_i
>> MSR_i_TCO 1101 0101 0000 0 011 0100 .... 100 11111 @msr_i
>> MSR_i_DAIFSET 1101 0101 0000 0 011 0100 .... 110 11111 @msr_i
>> MSR_i_DAIFCLEAR 1101 0101 0000 0 011 0100 .... 111 11111 @msr_i
>> +MSR_i_ALLINT 1101 0101 0000 0 001 0100 .... 000 11111 @msr_i
>> MSR_i_SVCR 1101 0101 0000 0 011 0100 0 mask:2 imm:1 011 11111
>> # MRS, MSR (register), SYS, SYSL. These are all essentially the
>> diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
>> index ebaa7f00df..3686926ada 100644
>> --- a/target/arm/tcg/helper-a64.c
>> +++ b/target/arm/tcg/helper-a64.c
>> @@ -66,6 +66,30 @@ void HELPER(msr_i_spsel)(CPUARMState *env, uint32_t
>> imm)
>> update_spsel(env, imm);
>> }
>> +static void allint_check(CPUARMState *env, uint32_t op,
>> + uint32_t imm, uintptr_t ra)
>> +{
>> + /* ALLINT update to PSTATE. */
>> + if (arm_current_el(env) == 0) {
>> + raise_exception_ra(env, EXCP_UDEF,
>> + syn_aa64_sysregtrap(0, extract32(op, 0, 3),
>> + extract32(op, 3, 3), 4,
>> + imm, 0x1f, 0),
>> + exception_target_el(env), ra);
>> + }
>> +}
>
> A runtime check for EL0 is not necessary; you've already handled that in
> trans_MSR_i_ALLINT(). However, what *is* missing here is the test
> against TALLINT for EL1.
Thank you! I'll fix it.
>
>> +
>> +void HELPER(msr_i_allint)(CPUARMState *env, uint32_t imm)
>> +{
>> + allint_check(env, 0x8, imm, GETPC());
>> + if (imm == 1) {
>> + env->allint |= PSTATE_ALLINT;
>> + } else {
>> + env->allint &= ~PSTATE_ALLINT;
>> + }
>
> I think you should not write an immediate-specific helper, but one which
> can also handle the variable "MSR allint, <xt>". This is no more
> difficult than
The kernel patches now uses the immediate-specific helper only, so it is
necessary for test.
>
> void HELPER(msr_allint)(CPUARMState *env, target_ulong val)
> {
> ... check ...
> env->pstate = (env->pstate & ~PSTATE_ALLINT) | (val & PSTATE_ALLINT);
> }
>
>> + arm_rebuild_hflags(env);
>> +}
>
> allint does not affect hflags; no rebuild required.
Thank you! I'll fix it.
>
>> +static bool trans_MSR_i_ALLINT(DisasContext *s, arg_i *a)
>> +{
>> + if (!dc_isar_feature(aa64_nmi, s) || s->current_el == 0) {
>> + return false;
>> + }
>> + gen_helper_msr_i_allint(tcg_env, tcg_constant_i32(a->imm));
>
> You're passing all of #imm4, not #imm1, which meant the test in your
> msr_i_allint helper was wrong.
In fact, it's either 0 or 1 for CRm according to the arm manual, so it
is not necessary.
>
> To work with the generalized helper above, this would be
>
> tcg_constant_tl((a->imm & 1) * PSTATE_ALLINT);
>
>
> r~
On 2/21/24 09:09, Richard Henderson wrote:
> On 2/21/24 03:08, Jinjie Ruan via wrote:
>> Add ALLINT MSR (immediate) to decodetree. And the EL0 check is necessary
>> to ALLINT. Avoid the unconditional write to pc and use raise_exception_ra
>> to unwind.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>> target/arm/tcg/a64.decode | 1 +
>> target/arm/tcg/helper-a64.c | 24 ++++++++++++++++++++++++
>> target/arm/tcg/helper-a64.h | 1 +
>> target/arm/tcg/translate-a64.c | 10 ++++++++++
>> 4 files changed, 36 insertions(+)
>>
>> diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
>> index 8a20dce3c8..3588080024 100644
>> --- a/target/arm/tcg/a64.decode
>> +++ b/target/arm/tcg/a64.decode
>> @@ -207,6 +207,7 @@ MSR_i_DIT 1101 0101 0000 0 011 0100 .... 010 11111 @msr_i
>> MSR_i_TCO 1101 0101 0000 0 011 0100 .... 100 11111 @msr_i
>> MSR_i_DAIFSET 1101 0101 0000 0 011 0100 .... 110 11111 @msr_i
>> MSR_i_DAIFCLEAR 1101 0101 0000 0 011 0100 .... 111 11111 @msr_i
>> +MSR_i_ALLINT 1101 0101 0000 0 001 0100 .... 000 11111 @msr_i
>> MSR_i_SVCR 1101 0101 0000 0 011 0100 0 mask:2 imm:1 011 11111
>> # MRS, MSR (register), SYS, SYSL. These are all essentially the
>> diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
>> index ebaa7f00df..3686926ada 100644
>> --- a/target/arm/tcg/helper-a64.c
>> +++ b/target/arm/tcg/helper-a64.c
>> @@ -66,6 +66,30 @@ void HELPER(msr_i_spsel)(CPUARMState *env, uint32_t imm)
>> update_spsel(env, imm);
>> }
>> +static void allint_check(CPUARMState *env, uint32_t op,
>> + uint32_t imm, uintptr_t ra)
>> +{
>> + /* ALLINT update to PSTATE. */
>> + if (arm_current_el(env) == 0) {
>> + raise_exception_ra(env, EXCP_UDEF,
>> + syn_aa64_sysregtrap(0, extract32(op, 0, 3),
>> + extract32(op, 3, 3), 4,
>> + imm, 0x1f, 0),
>> + exception_target_el(env), ra);
>> + }
>> +}
>
> A runtime check for EL0 is not necessary; you've already handled that in
> trans_MSR_i_ALLINT(). However, what *is* missing here is the test against TALLINT for EL1.
>
>> +
>> +void HELPER(msr_i_allint)(CPUARMState *env, uint32_t imm)
>> +{
>> + allint_check(env, 0x8, imm, GETPC());
>> + if (imm == 1) {
>> + env->allint |= PSTATE_ALLINT;
>> + } else {
>> + env->allint &= ~PSTATE_ALLINT;
>> + }
>
> I think you should not write an immediate-specific helper, but one which can also handle
> the variable "MSR allint, <xt>". This is no more difficult than
>
> void HELPER(msr_allint)(CPUARMState *env, target_ulong val)
> {
> ... check ...
> env->pstate = (env->pstate & ~PSTATE_ALLINT) | (val & PSTATE_ALLINT);
> }
Ho hum.. I just noticed that TALLINT only traps immediate write of 1, not also immediate
write of 0. So one helper for both MSR Xt and MSR imm is not practical.
r~
On 2/21/24 10:41, Richard Henderson wrote:
> Ho hum.. I just noticed that TALLINT only traps immediate write of 1, not also immediate
> write of 0. So one helper for both MSR Xt and MSR imm is not practical.
Quick follow up to say that means you can do
static bool trans_MSR_i_ALLINT(DisasContext *s, arg_i *a)
{
if (!dc_isar_feature(aa64_nmi, s)
|| s->current_el == 0
|| (a->imm & ~1)) {
return false;
}
if (!a->imm) {
clear_pstate_bits(PSTATE_ALLINT);
} else if (arm_dc_feature(s, ARM_FEATURE_EL2) && s->current_el == 1) {
/* Use helper for runtime check against HCRX_EL2.TALLINT. */
gen_helper_msr_set_allint_el1(tcg_env);
} else {
set_pstate_bits(PSTATE_ALLINT);
}
return true;
}
Generate inline bit ops whenever TALLINT does not apply.
This also means the helper need not check current_el, because we've already done it here.
r~
On 2024/2/22 4:41, Richard Henderson wrote:
> On 2/21/24 09:09, Richard Henderson wrote:
>> On 2/21/24 03:08, Jinjie Ruan via wrote:
>>> Add ALLINT MSR (immediate) to decodetree. And the EL0 check is necessary
>>> to ALLINT. Avoid the unconditional write to pc and use
>>> raise_exception_ra
>>> to unwind.
>>>
>>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>> ---
>>> target/arm/tcg/a64.decode | 1 +
>>> target/arm/tcg/helper-a64.c | 24 ++++++++++++++++++++++++
>>> target/arm/tcg/helper-a64.h | 1 +
>>> target/arm/tcg/translate-a64.c | 10 ++++++++++
>>> 4 files changed, 36 insertions(+)
>>>
>>> diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
>>> index 8a20dce3c8..3588080024 100644
>>> --- a/target/arm/tcg/a64.decode
>>> +++ b/target/arm/tcg/a64.decode
>>> @@ -207,6 +207,7 @@ MSR_i_DIT 1101 0101 0000 0 011 0100 ....
>>> 010 11111 @msr_i
>>> MSR_i_TCO 1101 0101 0000 0 011 0100 .... 100 11111 @msr_i
>>> MSR_i_DAIFSET 1101 0101 0000 0 011 0100 .... 110 11111 @msr_i
>>> MSR_i_DAIFCLEAR 1101 0101 0000 0 011 0100 .... 111 11111 @msr_i
>>> +MSR_i_ALLINT 1101 0101 0000 0 001 0100 .... 000 11111 @msr_i
>>> MSR_i_SVCR 1101 0101 0000 0 011 0100 0 mask:2 imm:1 011 11111
>>> # MRS, MSR (register), SYS, SYSL. These are all essentially the
>>> diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
>>> index ebaa7f00df..3686926ada 100644
>>> --- a/target/arm/tcg/helper-a64.c
>>> +++ b/target/arm/tcg/helper-a64.c
>>> @@ -66,6 +66,30 @@ void HELPER(msr_i_spsel)(CPUARMState *env,
>>> uint32_t imm)
>>> update_spsel(env, imm);
>>> }
>>> +static void allint_check(CPUARMState *env, uint32_t op,
>>> + uint32_t imm, uintptr_t ra)
>>> +{
>>> + /* ALLINT update to PSTATE. */
>>> + if (arm_current_el(env) == 0) {
>>> + raise_exception_ra(env, EXCP_UDEF,
>>> + syn_aa64_sysregtrap(0, extract32(op, 0, 3),
>>> + extract32(op, 3, 3), 4,
>>> + imm, 0x1f, 0),
>>> + exception_target_el(env), ra);
>>> + }
>>> +}
>>
>> A runtime check for EL0 is not necessary; you've already handled that
>> in trans_MSR_i_ALLINT(). However, what *is* missing here is the test
>> against TALLINT for EL1.
>>
>>> +
>>> +void HELPER(msr_i_allint)(CPUARMState *env, uint32_t imm)
>>> +{
>>> + allint_check(env, 0x8, imm, GETPC());
>>> + if (imm == 1) {
>>> + env->allint |= PSTATE_ALLINT;
>>> + } else {
>>> + env->allint &= ~PSTATE_ALLINT;
>>> + }
>>
>> I think you should not write an immediate-specific helper, but one
>> which can also handle the variable "MSR allint, <xt>". This is no
>> more difficult than
>>
>> void HELPER(msr_allint)(CPUARMState *env, target_ulong val)
>> {
>> ... check ...
>> env->pstate = (env->pstate & ~PSTATE_ALLINT) | (val &
>> PSTATE_ALLINT);
>> }
>
> Ho hum.. I just noticed that TALLINT only traps immediate write of 1,
> not also immediate write of 0. So one helper for both MSR Xt and MSR
> imm is not practical.
it is a real problem.
>
>
> r~
On 2/21/24 09:09, Richard Henderson wrote:
>> +static bool trans_MSR_i_ALLINT(DisasContext *s, arg_i *a)
>> +{
>> + if (!dc_isar_feature(aa64_nmi, s) || s->current_el == 0) {
>> + return false;
>> + }
>> + gen_helper_msr_i_allint(tcg_env, tcg_constant_i32(a->imm));
>
> You're passing all of #imm4, not #imm1, which meant the test in your msr_i_allint helper
> was wrong.
>
> To work with the generalized helper above, this would be
>
> tcg_constant_tl((a->imm & 1) * PSTATE_ALLINT);
Actually, I should have read further in the pseudocode -- (imm & 0xe) != 0 is undefined.
r~
© 2016 - 2024 Red Hat, Inc.