[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 05/10] target/arm: Introduce arm_hcr_el2_eff
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [PATCH v2 05/10] target/arm: Introduce arm_hcr_el2_eff |
Date: |
Thu, 6 Dec 2018 11:23:54 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 |
On 12/6/18 6:09 AM, Peter Maydell wrote:
> On Mon, 3 Dec 2018 at 20:38, Richard Henderson
> <address@hidden> wrote:
>>
>> Replace arm_hcr_el2_{fmo,imo,amo} with a more general routine
>> that also takes SCR_EL3.NS (aka arm_is_secure_below_el3) into
>> account, as documented for the plethora of bits in HCR_EL2.
>>
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>> target/arm/cpu.h | 67 +++++++++------------------------------
>> hw/intc/arm_gicv3_cpuif.c | 21 ++++++------
>> target/arm/helper.c | 65 +++++++++++++++++++++++++++++++------
>> 3 files changed, 82 insertions(+), 71 deletions(-)
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 20d97b66de..e871b946c8 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -1722,6 +1722,14 @@ static inline bool arm_is_secure(CPUARMState *env)
>> }
>> #endif
>>
>> +/**
>> + * arm_hcr_el2_eff(): Return the effective value of HCR_EL2.
>> + * E.g. when in secure state, fields in HCR_EL2 are suppressed,
>> + * "for all purposes other than a direct read or write access of HCR_EL2."
>> + * Not included here are RW, VI, VF.
>> + */
>> +uint64_t arm_hcr_el2_eff(CPUARMState *env);
>
> This comment says the not-included bits are RW, VI, VF...
>
>> +/*
>> + * Return the effective value of HCR_EL2.
>> + * Bits that are not included here:
>> + * RW (read from SCR_EL3.RW as needed)
>> + */
>
> ...but this comment only lists RW. Which is correct ?
>
> Also, if VI, VF are in the list shouldn't VSE be too ?
VI and VF were on the list when I first wrote the patch, then one of the
target-arm.next patches rearranged how those bits are handled.
So correct is that they are *not* excluded.
>> + } else if (ret & HCR_TGE) {
>> + if (ret & HCR_E2H) {
>> + ret &= ~(HCR_TTLBOS | HCR_TTLBIS | HCR_TOCU | HCR_TICAB |
>> + HCR_TID4 | HCR_ID | HCR_CD | HCR_TDZ | HCR_TPU |
>> + HCR_TPCP | HCR_TID2 | HCR_TID1 | HCR_TID0 | HCR_TWE |
>> + HCR_TWI | HCR_DC | HCR_BSU_MASK | HCR_FB | HCR_AMO |
>> + HCR_IMO | HCR_FMO | HCR_VM);
>
>
> This list should also in theory include HCR_MIOCNCE, but the
> QEMU implementation of that bit is going to be RAZ/WI anyway.
Even if we do RAZ/WI, it's probably better to match the docs and exclude it
here. Fixed.
>
> HCR_TID1 is in the "ignore if TGE==1" group, not the "ignore if
> {E2H, TGE} == {1,1}" group.
Fixed.
>
> Maybe we should be also setting HCR_ATA here ? We could perhaps
> leave that til we implement ARMv8.5-MemTag and understand it better.
This list was created with the ARMv8.4 document.
Perhaps a comment to that effect while v8.5 is still in flight?
>
>> + } else {
>> + ret &= ~(HCR_PTW | HCR_FB | HCR_TID1 | HCR_TID3 | HCR_TSC |
>> + HCR_TACR | HCR_TSW | HCR_TTLB | HCR_TVM | HCR_HCD |
>> + HCR_TRVM | HCR_TLOR);
>
> Shouldn't these bits be being cleared regardless of E2H, rather
> than only in the TGE==1 E2H==0 case ?
Fixed.
> Missing HCR_SWIO ?
Fixed.
> The list of bits cleared if TGE && E2H is highest-first, but this
> list is lowest-first...
Both lists now lowest first.
r~
- Re: [Qemu-devel] [PATCH v2 01/10] target/arm: Move id_aa64mmfr* to ARMISARegisters, (continued)
- [Qemu-devel] [PATCH v2 03/10] target/arm: Add SCR_EL3 bits up to ARMv8.5, Richard Henderson, 2018/12/03
- [Qemu-devel] [PATCH v2 02/10] target/arm: Add HCR_EL2 bits up to ARMv8.5, Richard Henderson, 2018/12/03
- [Qemu-devel] [PATCH v2 04/10] target/arm: Fix HCR_EL2.TGE check in arm_phys_excp_target_el, Richard Henderson, 2018/12/03
- [Qemu-devel] [PATCH v2 05/10] target/arm: Introduce arm_hcr_el2_eff, Richard Henderson, 2018/12/03
- [Qemu-devel] [PATCH v2 06/10] target/arm: Use arm_hcr_el2_eff more places, Richard Henderson, 2018/12/03
- [Qemu-devel] [PATCH v2 09/10] target/arm: Implement the ARMv8.1-HPD extension, Richard Henderson, 2018/12/03
- [Qemu-devel] [PATCH v2 07/10] target/arm: Tidy scr_write, Richard Henderson, 2018/12/03
- [Qemu-devel] [PATCH v2 10/10] target/arm: Implement the ARMv8.2-AA32HPD extension, Richard Henderson, 2018/12/03
- [Qemu-devel] [PATCH v2 08/10] target/arm: Implement the ARMv8.1-LOR extension, Richard Henderson, 2018/12/03