Re: [PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions
On 2019-12-02 16:56, Richard Henderson wrote: On 12/2/19 4:45 PM, Marc Zyngier wrote: Annoying that there's a bug in the manual -- FPSID is listed as group 0 in plenty of places, except in the pseudo-code for Accessing the FPSID which uses TID3. Are you sure? I'm looking at DDI0487E_a, ... Or have you spotted a discrepancy somewhere else (which would be oh-so-surprising...)? In DDI0487E_a, page G8-6028: elsif EL2Enabled() && !ELUsingAArch32(EL2) && HCR_EL2.TID3 == '1' then AArch64.AArch32SystemAccessTrap(EL2, 0x08); elsif EL2Enabled() && ELUsingAArch32(EL2) && HCR.TID3 == '1' then AArch32.TakeHypTrapException(0x08); else return FPSID; within the summary documentation for FPSID. Ah, that was too obvious for me to find ;-). Indeed, this looks totally bogus. I'll try and poke a few people... Thanks, M. -- Jazz is not dead. It just smells funny... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions
On 12/2/19 4:45 PM, Marc Zyngier wrote: >> Annoying that there's a bug in the manual -- FPSID is listed as group 0 in >> plenty of places, except in the pseudo-code for Accessing the FPSID >> which uses TID3. > > Are you sure? I'm looking at DDI0487E_a, ... > Or have you spotted a discrepancy > somewhere else (which would be oh-so-surprising...)? In DDI0487E_a, page G8-6028: > elsif EL2Enabled() && !ELUsingAArch32(EL2) && HCR_EL2.TID3 == '1' then > AArch64.AArch32SystemAccessTrap(EL2, 0x08); > elsif EL2Enabled() && ELUsingAArch32(EL2) && HCR.TID3 == '1' then > AArch32.TakeHypTrapException(0x08); > else > return FPSID; within the summary documentation for FPSID. r~ ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions
On 2019-12-02 15:35, Richard Henderson wrote:
On 12/1/19 12:20 PM, Marc Zyngier wrote:
HCR_EL2.TID3 requires that AArch32 reads of MVFR[012] are trapped to
EL2, and HCR_EL2.TID0 does the same for reads of FPSID.
In order to handle this, introduce a new TCG helper function that
checks for these control bits before executing the VMRC instruction.
Tested with a hacked-up version of KVM/arm64 that sets the control
bits for 32bit guests.
Reviewed-by: Edgar E. Iglesias
Signed-off-by: Marc Zyngier
---
target/arm/helper-a64.h| 2 ++
target/arm/translate-vfp.inc.c | 18 +++---
target/arm/vfp_helper.c| 29 +
3 files changed, 46 insertions(+), 3 deletions(-)
Reviewed-by: Richard Henderson
Annoying that there's a bug in the manual -- FPSID is listed as group
0 in
plenty of places, except in the pseudo-code for Accessing the FPSID
which uses TID3.
Are you sure? I'm looking at DDI0487E_a, and the pseudo-code for
AArch32.CheckAdvSIMDOrFPRegisterTraps has this check:
if (tid0 == '1' && reg == '') // FPSID
|| (tid3 == '1' && reg IN {'0101', '0110', '0111'}) then // MVFRx
if ELUsingAArch32(EL2) then
AArch32.SystemAccessTrap(M32_Hyp, 0x8);//
Exception_AdvSIMDFPAccessTrap
else
AArch64.AArch32SystemAccessTrap(EL2, 0x8); //
Exception_AdvSIMDFPAccessTrap
which seems to do the right thing. Or have you spotted a discrepancy
somewhere else (which would be oh-so-surprising...)?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 5/5] target/arm: Add support for missing Jazelle system registers
On 12/1/19 12:20 PM, Marc Zyngier wrote:
> +if (cpu_isar_feature(jazelle, cpu)) {
> +ARMCPRegInfo jazelle_regs[] = {
static const.
Otherwise,
Reviewed-by: Richard Henderson
r~
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/5] target/arm: Handle AArch32 CP15 trapping via HSTR_EL2
On 12/1/19 12:20 PM, Marc Zyngier wrote:
> +/* Check for an EL2 trap due to HSTR_EL2. We expect EL0 accesses
> + * to sysregs non accessible at EL0 to have UNDEF-ed already.
> + */
We're enforcing
/*
* Multi-line comment
*/
for qemu now; checkpatch should be reporting on that.
> +if (!env->aarch64 && arm_current_el(env) < 2 && ri->cp == 15 &&
> +(arm_hcr_el2_eff(env) & (HCR_E2H | HCR_TGE)) != (HCR_E2H | HCR_TGE))
> {
Use is_a64(env) not env->aarch64 directly.
Otherwise,
Reviewed-by: Richard Henderson
r~
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions
On 12/1/19 12:20 PM, Marc Zyngier wrote: > HCR_EL2.TID3 requires that AArch32 reads of MVFR[012] are trapped to > EL2, and HCR_EL2.TID0 does the same for reads of FPSID. > In order to handle this, introduce a new TCG helper function that > checks for these control bits before executing the VMRC instruction. > > Tested with a hacked-up version of KVM/arm64 that sets the control > bits for 32bit guests. > > Reviewed-by: Edgar E. Iglesias > Signed-off-by: Marc Zyngier > --- > target/arm/helper-a64.h| 2 ++ > target/arm/translate-vfp.inc.c | 18 +++--- > target/arm/vfp_helper.c| 29 + > 3 files changed, 46 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson Annoying that there's a bug in the manual -- FPSID is listed as group 0 in plenty of places, except in the pseudo-code for Accessing the FPSID which uses TID3. r~ ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 2/5] target/arm: Honor HCR_EL2.TID1 trapping requirements
On 12/1/19 12:20 PM, Marc Zyngier wrote: > HCR_EL2.TID1 mandates that access from EL1 to REVIDR_EL1, AIDR_EL1 > (and their 32bit equivalents) as well as TCMTR, TLBTR are trapped > to EL2. QEMU ignores it, making it harder for a hypervisor to > virtualize the HW (though to be fair, no known hypervisor actually > cares). > > Do the right thing by trapping to EL2 if HCR_EL2.TID1 is set. > > Reviewed-by: Edgar E. Iglesias > Signed-off-by: Marc Zyngier > --- > target/arm/helper.c | 36 > 1 file changed, 32 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~ ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 1/5] target/arm: Honor HCR_EL2.TID2 trapping requirements
On 12/1/19 12:20 PM, Marc Zyngier wrote: > HCR_EL2.TID2 mandates that access from EL1 to CTR_EL0, CCSIDR_EL1, > CCSIDR2_EL1, CLIDR_EL1, CSSELR_EL1 are trapped to EL2, and QEMU > completely ignores it, making it impossible for hypervisors to > virtualize the cache hierarchy. > > Do the right thing by trapping to EL2 if HCR_EL2.TID2 is set. > > Signed-off-by: Marc Zyngier > --- > target/arm/helper.c | 31 +++ > 1 file changed, 27 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~ ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 5/5] target/arm: Add support for missing Jazelle system registers
On Sun, Dec 01, 2019 at 12:20:18PM +, Marc Zyngier wrote:
> QEMU lacks the minimum Jazelle implementation that is required
> by the architecture (everything is RAZ or RAZ/WI). Add it
> together with the HCR_EL2.TID0 trapping that goes with it.
Looks good to me:
Reviewed-by: Edgar E. Iglesias
>
> Signed-off-by: Marc Zyngier
> ---
> target/arm/helper.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 0ba08d550a..d6fc198a97 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6040,6 +6040,16 @@ static CPAccessResult access_aa32_tid3(CPUARMState
> *env, const ARMCPRegInfo *ri,
> return CP_ACCESS_OK;
> }
>
> +static CPAccessResult access_jazelle(CPUARMState *env, const ARMCPRegInfo
> *ri,
> + bool isread)
> +{
> +if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID0)) {
> +return CP_ACCESS_TRAP_EL2;
> +}
> +
> +return CP_ACCESS_OK;
> +}
> +
> void register_cp_regs_for_features(ARMCPU *cpu)
> {
> /* Register all the coprocessor registers based on feature bits */
> @@ -6057,6 +6067,23 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> define_arm_cp_regs(cpu, not_v8_cp_reginfo);
> }
>
> +if (cpu_isar_feature(jazelle, cpu)) {
> +ARMCPRegInfo jazelle_regs[] = {
> +{ .name = "JIDR",
> + .cp = 14, .crn = 0, .crm = 0, .opc1 = 7, .opc2 = 0,
> + .access = PL1_R, .accessfn = access_jazelle,
> + .type = ARM_CP_CONST, .resetvalue = 0 },
> +{ .name = "JOSCR",
> + .cp = 14, .crn = 1, .crm = 0, .opc1 = 7, .opc2 = 0,
> + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> +{ .name = "JMCR",
> + .cp = 14, .crn = 2, .crm = 0, .opc1 = 7, .opc2 = 0,
> + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> +REGINFO_SENTINEL
> +};
> +
> +define_arm_cp_regs(cpu, jazelle_regs);
> +}
> if (arm_feature(env, ARM_FEATURE_V6)) {
> /* The ID registers all have impdef reset values */
> ARMCPRegInfo v6_idregs[] = {
> --
> 2.20.1
>
>
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 1/5] target/arm: Honor HCR_EL2.TID2 trapping requirements
On Sun, Dec 01, 2019 at 12:20:14PM +, Marc Zyngier wrote:
> HCR_EL2.TID2 mandates that access from EL1 to CTR_EL0, CCSIDR_EL1,
> CCSIDR2_EL1, CLIDR_EL1, CSSELR_EL1 are trapped to EL2, and QEMU
> completely ignores it, making it impossible for hypervisors to
> virtualize the cache hierarchy.
>
> Do the right thing by trapping to EL2 if HCR_EL2.TID2 is set.
Reviewed-by: Edgar E. Iglesias
>
> Signed-off-by: Marc Zyngier
> ---
> target/arm/helper.c | 31 +++
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 0bf8f53d4b..1e546096b8 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -1910,6 +1910,17 @@ static void scr_write(CPUARMState *env, const
> ARMCPRegInfo *ri, uint64_t value)
> raw_write(env, ri, value);
> }
>
> +static CPAccessResult access_aa64_tid2(CPUARMState *env,
> + const ARMCPRegInfo *ri,
> + bool isread)
> +{
> +if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID2)) {
> +return CP_ACCESS_TRAP_EL2;
> +}
> +
> +return CP_ACCESS_OK;
> +}
> +
> static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> {
> ARMCPU *cpu = env_archcpu(env);
> @@ -2110,10 +2121,14 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>.writefn = pmintenclr_write },
> { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
>.opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
> - .access = PL1_R, .readfn = ccsidr_read, .type = ARM_CP_NO_RAW },
> + .access = PL1_R,
> + .accessfn = access_aa64_tid2,
> + .readfn = ccsidr_read, .type = ARM_CP_NO_RAW },
> { .name = "CSSELR", .state = ARM_CP_STATE_BOTH,
>.opc0 = 3, .crn = 0, .crm = 0, .opc1 = 2, .opc2 = 0,
> - .access = PL1_RW, .writefn = csselr_write, .resetvalue = 0,
> + .access = PL1_RW,
> + .accessfn = access_aa64_tid2,
> + .writefn = csselr_write, .resetvalue = 0,
>.bank_fieldoffsets = { offsetof(CPUARMState, cp15.csselr_s),
> offsetof(CPUARMState, cp15.csselr_ns) } },
> /* Auxiliary ID register: this actually has an IMPDEF value but for now
> @@ -5204,6 +5219,11 @@ static CPAccessResult ctr_el0_access(CPUARMState *env,
> const ARMCPRegInfo *ri,
> if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UCT)) {
> return CP_ACCESS_TRAP;
> }
> +
> +if (arm_current_el(env) < 2 && arm_hcr_el2_eff(env) & HCR_TID2) {
> +return CP_ACCESS_TRAP_EL2;
> +}
> +
> return CP_ACCESS_OK;
> }
>
> @@ -6184,7 +6204,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> ARMCPRegInfo clidr = {
> .name = "CLIDR", .state = ARM_CP_STATE_BOTH,
> .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 1,
> -.access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->clidr
> +.access = PL1_R, .type = ARM_CP_CONST,
> +.accessfn = access_aa64_tid2,
> +.resetvalue = cpu->clidr
> };
> define_one_arm_cp_reg(cpu, &clidr);
> define_arm_cp_regs(cpu, v7_cp_reginfo);
> @@ -6717,7 +6739,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> /* These are common to v8 and pre-v8 */
> { .name = "CTR",
>.cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 1,
> - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->ctr
> },
> + .access = PL1_R, .accessfn = ctr_el0_access,
> + .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
> { .name = "CTR_EL0", .state = ARM_CP_STATE_AA64,
>.opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0,
>.access = PL0_R, .accessfn = ctr_el0_access,
> --
> 2.20.1
>
>
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
