On 12/6/20 10:46 PM, LIU Zhiwei wrote:
> From DDI0487Fc_armv8_arm.pdf, the CPTR_EL2 has two kinds
> of layouts according to HCR_EL2.E2H.
>
> When HCR_EL2.E2H is 1, fp_exception_el should refer to
> HCR_EL2.FPEN and sve_exception_el should refer to HCR_EL2.ZEN.
>
> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
> Signed-off-by: LIU Zhiwei <zhiwei_...@c-sky.com>
> ---
> target/arm/helper.c | 55 +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 38cd35c049..6cc9f2bb50 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6147,11 +6147,30 @@ int sve_exception_el(CPUARMState *env, int el)
> * they will be zero when EL2 is not present.
> */
> if (el <= 2 && !arm_is_secure_below_el3(env)) {
> - if (env->cp15.cptr_el[2] & CPTR_TZ) {
> - return 2;
> - }
> - if (env->cp15.cptr_el[2] & CPTR_TFP) {
> - return 0;
> + /* Since we exclude secure first, we may read HCR_EL2 directly. */
> + if (env->cp15.hcr_el2 & HCR_E2H) {
We have already pulled the correct value into a local variable here: hcr_el2.
No need for the comment.
> + int zen = extract32(env->cp15.cptr_el[2], 16, 2);
> + switch (zen) {
> + case 0:
> + case 2:
> + return 2;
> + case 1:
> + if (env->cp15.hcr_el2 & HCR_TGE) {
Likewise.
> + if (el == 0) {
> + return 2;
> + }
> + }
> + break;
> + case 3:
> + break;
> + }
> + } else {
> + if (env->cp15.cptr_el[2] & CPTR_TZ) {
> + return 2;
> + }
> + if (env->cp15.cptr_el[2] & CPTR_TFP) {
> + return 0;
> + }
> }
> }
>
> @@ -12635,12 +12654,30 @@ int fp_exception_el(CPUARMState *env, int cur_el)
> */
>
> /* CPTR_EL2 : present in v7VE or v8 */
> - if (cur_el <= 2 && extract32(env->cp15.cptr_el[2], 10, 1)
> - && !arm_is_secure_below_el3(env)) {
> + if ((cur_el <= 2) && !arm_is_secure_below_el3(env)) {
> /* Trap FP ops at EL2, NS-EL1 or NS-EL0 to EL2 */
> - return 2;
> + if ((arm_hcr_el2_eff(env) & HCR_E2H) == HCR_E2H) {
There is a prior use of arm_hcr_el2_eff in this function. It should be called
once and stored in a local variable, just like in sve_exception_el.
No need for == in testing a single bit.
> + int fpen = extract32(env->cp15.cptr_el[2], 20, 2);
> + switch (fpen) {
> + case 0:
> + case 2:
> + return 2;
> + case 1:
> + if ((arm_hcr_el2_eff(env) & HCR_TGE) == HCR_TGE) {
Likewise.
> + if (cur_el == 0) {
> + return 2;
> + }
> + }
> + break;
> + case 3:
> + break;
> + }
> + } else {
> + if (extract32(env->cp15.cptr_el[2], 10, 1)) {
Use CPTR_TFP.
> + return 2;
> + }
> + }
> }
> -
Whitespace removal.
> /* CPTR_EL3 : present in v8 */
> if (extract32(env->cp15.cptr_el[3], 10, 1)) {
> /* Trap all FP ops to EL3 */
>
Would you also change this to use CPTR_TFP please?
r~
