[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] target/arm: Do alignment check when translation disabled
From: |
Peter Maydell |
Subject: |
Re: [PATCH] target/arm: Do alignment check when translation disabled |
Date: |
Thu, 22 Sep 2022 16:31:41 +0100 |
On Wed, 14 Sept 2022 at 13:47, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> If translation is disabled, the default memory type is Device,
> which requires alignment checking. Document, but defer, the
> more general case of per-page alignment checking.
>
> Reported-by: Idan Horowitz <idan.horowitz@gmail.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1204
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/helper.c | 38 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index d7bc467a2a..79609443aa 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -10713,6 +10713,39 @@ ARMMMUIdx arm_mmu_idx(CPUARMState *env)
> return arm_mmu_idx_el(env, arm_current_el(env));
> }
>
> +/*
> + * Return true if memory alignment should be enforced.
> + */
> +static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t
> sctlr)
> +{
> + /* Check the alignment enable bit. */
> + if (sctlr & SCTLR_A) {
> + return true;
> + }
> +
> + /*
> + * If translation is disabled, then the default memory type
> + * may be Device(-nGnRnE) instead of Normal, which requires that
"may be" ?
> + * alignment be enforced.
> + *
> + * TODO: The more general case is translation enabled, with a per-page
> + * check of the memory type as assigned via MAIR_ELx and the PTE.
> + * We could arrange for a bit in MemTxAttrs to enforce alignment
> + * via forced use of the softmmu slow path. Given that such pages
> + * are intended for MMIO, where the slow path is required anyhow,
> + * this should not result in extra overhead.
> + */
> + if (sctlr & SCTLR_M) {
> + /* Translation enabled: memory type in PTE via MAIR_ELx. */
> + return false;
> + }
> + if (el < 2 && (arm_hcr_el2_eff(env) & (HCR_DC | HCR_VM))) {
> + /* Stage 2 translation enabled: memory type in PTE. */
> + return false;
> + }
> + return true;
The SCTLR_EL1 docs say that if HCR_EL2.{DC,TGE} != {0,0} then we need to
treat SCTLR_EL1.M as if it is 0. DC is covered above, but do we need/want
to do anything special for TGE ? Maybe we just never get into this case
because TGE means regime_sctlr() is never SCTLR_EL1 ? I forget how it
works...
We also need to not do this for anything with ARM_FEATURE_PMSA :
with PMSA, if the MPU is disabled because SCTLR.M is 0 then the
default memory type depends on the address (it's defined by the
"default memory map", DDI0406C.d table B5-1) and isn't always Device.
We should also mention in the comment why we're doing this particular
special case even though we don't care to do full alignment checking
for Device memory accesses: because initial MMU-off code is a common
use-case where the guest will be working with RAM that's set up as
Device memory, and it's nice to be able to detect misaligned-access
bugs in it.
> +}
> +
> static CPUARMTBFlags rebuild_hflags_common(CPUARMState *env, int fp_el,
> ARMMMUIdx mmu_idx,
> CPUARMTBFlags flags)
> @@ -10777,8 +10810,9 @@ static CPUARMTBFlags rebuild_hflags_a32(CPUARMState
> *env, int fp_el,
> {
> CPUARMTBFlags flags = {};
> int el = arm_current_el(env);
> + uint64_t sctlr = arm_sctlr(env, el);
>
> - if (arm_sctlr(env, el) & SCTLR_A) {
> + if (aprofile_require_alignment(env, el, sctlr)) {
> DP_TBFLAG_ANY(flags, ALIGN_MEM, 1);
> }
>
> @@ -10871,7 +10905,7 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState
> *env, int el, int fp_el,
>
> sctlr = regime_sctlr(env, stage1);
>
> - if (sctlr & SCTLR_A) {
> + if (aprofile_require_alignment(env, el, sctlr)) {
> DP_TBFLAG_ANY(flags, ALIGN_MEM, 1);
> }
>
> --
> 2.34.1
thanks
-- PMM
- [PULL 16/20] target/arm: Implement FEAT_PMUv3p5 cycle counter disable bits, (continued)
- [PULL 16/20] target/arm: Implement FEAT_PMUv3p5 cycle counter disable bits, Richard Henderson, 2022/09/14
- [PULL 09/20] target/arm: Don't corrupt high half of PMOVSR when cycle counter overflows, Richard Henderson, 2022/09/14
- [PULL 08/20] target/arm: Add missing space in comment, Richard Henderson, 2022/09/14
- [PULL 12/20] target/arm: Ignore PMCR.D when PMCR.LC is set, Richard Henderson, 2022/09/14
- [PULL 10/20] target/arm: Correct value returned by pmu_counter_mask(), Richard Henderson, 2022/09/14
- [PULL 17/20] target/arm: Support 64-bit event counters for FEAT_PMUv3p5, Richard Henderson, 2022/09/14
- [PULL 18/20] target/arm: Report FEAT_PMUv3p5 for TCG '-cpu max', Richard Henderson, 2022/09/14
- [PULL 19/20] target/arm: Remove useless TARGET_BIG_ENDIAN check in armv7m_load_kernel(), Richard Henderson, 2022/09/14
- [PULL 13/20] target/arm: Honour MDCR_EL2.HPMD in Secure EL2, Richard Henderson, 2022/09/14
- [PATCH] target/arm: Do alignment check when translation disabled, Richard Henderson, 2022/09/14
- Re: [PATCH] target/arm: Do alignment check when translation disabled,
Peter Maydell <=
- [PULL 15/20] target/arm: Rename pmu_8_n feature test functions, Richard Henderson, 2022/09/14
- [PULL 20/20] target/arm: Make boards pass base address to armv7m_load_kernel(), Richard Henderson, 2022/09/14
- Re: [PULL 00/20] target-arm.next patch queue, Stefan Hajnoczi, 2022/09/17