All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 00/15] target-arm queue
@ 2022-07-18 13:59 Peter Maydell
  2022-07-18 13:59 ` [PULL 01/15] hw/intc/armv7m_nvic: ICPRn must not unpend an IRQ that is being held high Peter Maydell
                   ` (15 more replies)
  0 siblings, 16 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

Some arm patches before softfreeze. These are all bug fixes.

-- PMM

The following changes since commit 0ebf76aae58324b8f7bf6af798696687f5f4c2a9:

  Merge tag 'nvme-next-pull-request' of git://git.infradead.org/qemu-nvme into staging (2022-07-15 15:38:13 +0100)

are available in the Git repository at:

  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220718

for you to fetch changes up to 004c8a8bc569c8b18fca6fc90ffe3223daaf17b7:

  Align Raspberry Pi DMA interrupts with Linux DTS (2022-07-18 13:25:13 +0100)

----------------------------------------------------------------
target-arm queue:
 * hw/intc/armv7m_nvic: ICPRn must not unpend an IRQ that is being held high
 * target/arm: Fill in VL for tbflags when SME enabled and SVE disabled
 * target/arm: Fix aarch64_sve_change_el for SME
 * linux-user/aarch64: Do not clear PROT_MTE on mprotect
 * target/arm: Honour VTCR_EL2 bits in Secure EL2
 * hw/adc: Fix CONV bit in NPCM7XX ADC CON register
 * hw/adc: Make adci[*] R/W in NPCM7XX ADC
 * target/arm: Don't set syndrome ISS for loads and stores with writeback
 * Align Raspberry Pi DMA interrupts with Linux DTS

----------------------------------------------------------------
Andrey Makarov (1):
      Align Raspberry Pi DMA interrupts with Linux DTS

Hao Wu (2):
      hw/adc: Fix CONV bit in NPCM7XX ADC CON register
      hw/adc: Make adci[*] R/W in NPCM7XX ADC

Peter Maydell (9):
      hw/intc/armv7m_nvic: ICPRn must not unpend an IRQ that is being held high
      target/arm: Define and use new regime_tcr_value() function
      target/arm: Calculate mask/base_mask in get_level1_table_address()
      target/arm: Fold regime_tcr() and regime_tcr_value() together
      target/arm: Fix big-endian host handling of VTCR
      target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t
      target/arm: Store TCR_EL* registers as uint64_t
      target/arm: Honour VTCR_EL2 bits in Secure EL2
      target/arm: Don't set syndrome ISS for loads and stores with writeback

Richard Henderson (3):
      target/arm: Fill in VL for tbflags when SME enabled and SVE disabled
      target/arm: Fix aarch64_sve_change_el for SME
      linux-user/aarch64: Do not clear PROT_MTE on mprotect

 include/hw/arm/bcm2835_peripherals.h |   2 +
 target/arm/cpu.h                     |  38 ++++++++---
 target/arm/internals.h               |  34 +++++++---
 accel/tcg/translate-all.c            |  13 +++-
 hw/adc/npcm7xx_adc.c                 |   4 +-
 hw/arm/bcm2835_peripherals.c         |  26 ++++++-
 hw/intc/armv7m_nvic.c                |   9 ++-
 target/arm/cpu.c                     |   2 +-
 target/arm/debug_helper.c            |   2 +-
 target/arm/helper.c                  | 128 ++++++++++++++++-------------------
 target/arm/ptw.c                     |  38 ++++++-----
 target/arm/tlb_helper.c              |   2 +-
 target/arm/translate-a64.c           |   4 +-
 tests/qtest/bcm2835-dma-test.c       | 118 ++++++++++++++++++++++++++++++++
 tests/qtest/npcm7xx_adc-test.c       |   2 +-
 tests/qtest/meson.build              |   3 +-
 16 files changed, 306 insertions(+), 119 deletions(-)
 create mode 100644 tests/qtest/bcm2835-dma-test.c


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PULL 01/15] hw/intc/armv7m_nvic: ICPRn must not unpend an IRQ that is being held high
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 02/15] target/arm: Fill in VL for tbflags when SME enabled and SVE disabled Peter Maydell
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

In the M-profile Arm ARM, rule R_CVJS defines when an interrupt should
be set to the Pending state:
 A) when the input line is high and the interrupt is not Active
 B) when the input line transitions from low to high and the interrupt
    is Active
(Note that the first of these is an ongoing condition, and the
second is a point-in-time event.)

This can be rephrased as:
 1 when the line goes from low to high, set Pending
 2 when Active goes from 1 to 0, if line is high then set Pending
 3 ignore attempts to clear Pending when the line is high
   and Active is 0

where 1 covers both B and one of the "transition into condition A"
cases, 2 deals with the other "transition into condition A"
possibility, and 3 is "don't drop Pending if we're already in
condition A".  Transitions out of condition A don't affect Pending
state.

We handle case 1 in set_irq_level(). For an interrupt (as opposed
to other kinds of exception) the only place where we clear Active
is in armv7m_nvic_complete_irq(), where we handle case 2 by
checking for whether we need to re-pend the exception. For case 3,
the only places where we clear Pending state on an interrupt are in
armv7m_nvic_acknowledge_irq() (where we are setting Active so it
doesn't count) and for writes to NVIC_ICPRn.

It is the "write to NVIC_ICPRn" case that we missed: we must ignore
this if the input line is high and the interrupt is not Active.
(This required behaviour is differently and perhaps more clearly
stated in the v7M Arm ARM, which has pseudocode in section B3.4.1
that implies it.)

Reported-by: Igor Kotrasiński <i.kotrasinsk@samsung.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220628154724.3297442-1-peter.maydell@linaro.org
---
 hw/intc/armv7m_nvic.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 13df002ce4d..1f7763964c3 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -2389,8 +2389,15 @@ static MemTxResult nvic_sysreg_write(void *opaque, hwaddr addr,
         startvec = 8 * (offset - 0x280) + NVIC_FIRST_IRQ; /* vector # */
 
         for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) {
+            /*
+             * Note that if the input line is still held high and the interrupt
+             * is not active then rule R_CVJS requires that the Pending state
+             * remains set; in that case we mustn't let it be cleared.
+             */
             if (value & (1 << i) &&
-                (attrs.secure || s->itns[startvec + i])) {
+                (attrs.secure || s->itns[startvec + i]) &&
+                !(setval == 0 && s->vectors[startvec + i].level &&
+                  !s->vectors[startvec + i].active)) {
                 s->vectors[startvec + i].pending = setval;
             }
         }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 02/15] target/arm: Fill in VL for tbflags when SME enabled and SVE disabled
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
  2022-07-18 13:59 ` [PULL 01/15] hw/intc/armv7m_nvic: ICPRn must not unpend an IRQ that is being held high Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 03/15] target/arm: Fix aarch64_sve_change_el for SME Peter Maydell
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

From: Richard Henderson <richard.henderson@linaro.org>

When PSTATE.SM, VL = SVL even if SVE is disabled.
This is visible in kselftest ssve-test.

Reported-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220713045848.217364-2-richard.henderson@linaro.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index cfcad97ce07..6fff7fc64fd 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10882,13 +10882,19 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
     }
     if (cpu_isar_feature(aa64_sme, env_archcpu(env))) {
         int sme_el = sme_exception_el(env, el);
+        bool sm = FIELD_EX64(env->svcr, SVCR, SM);
 
         DP_TBFLAG_A64(flags, SMEEXC_EL, sme_el);
         if (sme_el == 0) {
             /* Similarly, do not compute SVL if SME is disabled. */
-            DP_TBFLAG_A64(flags, SVL, sve_vqm1_for_el_sm(env, el, true));
+            int svl = sve_vqm1_for_el_sm(env, el, true);
+            DP_TBFLAG_A64(flags, SVL, svl);
+            if (sm) {
+                /* If SVE is disabled, we will not have set VL above. */
+                DP_TBFLAG_A64(flags, VL, svl);
+            }
         }
-        if (FIELD_EX64(env->svcr, SVCR, SM)) {
+        if (sm) {
             DP_TBFLAG_A64(flags, PSTATE_SM, 1);
             DP_TBFLAG_A64(flags, SME_TRAP_NONSTREAMING, !sme_fa64(env, el));
         }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 03/15] target/arm: Fix aarch64_sve_change_el for SME
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
  2022-07-18 13:59 ` [PULL 01/15] hw/intc/armv7m_nvic: ICPRn must not unpend an IRQ that is being held high Peter Maydell
  2022-07-18 13:59 ` [PULL 02/15] target/arm: Fill in VL for tbflags when SME enabled and SVE disabled Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 04/15] linux-user/aarch64: Do not clear PROT_MTE on mprotect Peter Maydell
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

From: Richard Henderson <richard.henderson@linaro.org>

We were only checking for SVE disabled and not taking into
account PSTATE.SM to check SME disabled, which resulted in
vectors being incorrectly truncated.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220713045848.217364-3-richard.henderson@linaro.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 6fff7fc64fd..24c45a9bf31 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11228,6 +11228,21 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq)
     }
 }
 
+static uint32_t sve_vqm1_for_el_sm_ena(CPUARMState *env, int el, bool sm)
+{
+    int exc_el;
+
+    if (sm) {
+        exc_el = sme_exception_el(env, el);
+    } else {
+        exc_el = sve_exception_el(env, el);
+    }
+    if (exc_el) {
+        return 0; /* disabled */
+    }
+    return sve_vqm1_for_el_sm(env, el, sm);
+}
+
 /*
  * Notice a change in SVE vector size when changing EL.
  */
@@ -11236,7 +11251,7 @@ void aarch64_sve_change_el(CPUARMState *env, int old_el,
 {
     ARMCPU *cpu = env_archcpu(env);
     int old_len, new_len;
-    bool old_a64, new_a64;
+    bool old_a64, new_a64, sm;
 
     /* Nothing to do if no SVE.  */
     if (!cpu_isar_feature(aa64_sve, cpu)) {
@@ -11256,7 +11271,8 @@ void aarch64_sve_change_el(CPUARMState *env, int old_el,
      * invoke ResetSVEState when taking an exception from, or
      * returning to, AArch32 state when PSTATE.SM is enabled.
      */
-    if (old_a64 != new_a64 && FIELD_EX64(env->svcr, SVCR, SM)) {
+    sm = FIELD_EX64(env->svcr, SVCR, SM);
+    if (old_a64 != new_a64 && sm) {
         arm_reset_sve_state(env);
         return;
     }
@@ -11273,10 +11289,13 @@ void aarch64_sve_change_el(CPUARMState *env, int old_el,
      * we already have the correct register contents when encountering the
      * vq0->vq0 transition between EL0->EL1.
      */
-    old_len = (old_a64 && !sve_exception_el(env, old_el)
-               ? sve_vqm1_for_el(env, old_el) : 0);
-    new_len = (new_a64 && !sve_exception_el(env, new_el)
-               ? sve_vqm1_for_el(env, new_el) : 0);
+    old_len = new_len = 0;
+    if (old_a64) {
+        old_len = sve_vqm1_for_el_sm_ena(env, old_el, sm);
+    }
+    if (new_a64) {
+        new_len = sve_vqm1_for_el_sm_ena(env, new_el, sm);
+    }
 
     /* When changing vector length, clear inaccessible state.  */
     if (new_len < old_len) {
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 04/15] linux-user/aarch64: Do not clear PROT_MTE on mprotect
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (2 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 03/15] target/arm: Fix aarch64_sve_change_el for SME Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 05/15] target/arm: Define and use new regime_tcr_value() function Peter Maydell
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

From: Richard Henderson <richard.henderson@linaro.org>

The documentation for PROT_MTE says that it cannot be cleared
by mprotect.  Further, the implementation of the VM_ARCH_CLEAR bit,
contains PROT_BTI confiming that bit should be cleared.

Introduce PAGE_TARGET_STICKY to allow target/arch/cpu.h to control
which bits may be reset during page_set_flags.  This is sort of the
opposite of VM_ARCH_CLEAR, but works better with qemu's PAGE_* bits
that are separate from PROT_* bits.

Reported-by: Vitaly Buka <vitalybuka@google.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220711031420.17820-1-richard.henderson@linaro.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h          |  7 +++++--
 accel/tcg/translate-all.c | 13 +++++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1e36a839ee4..6afcc882f2c 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3392,9 +3392,12 @@ static inline MemTxAttrs *typecheck_memtxattrs(MemTxAttrs *x)
 
 /*
  * AArch64 usage of the PAGE_TARGET_* bits for linux-user.
+ * Note that with the Linux kernel, PROT_MTE may not be cleared by mprotect
+ * mprotect but PROT_BTI may be cleared.  C.f. the kernel's VM_ARCH_CLEAR.
  */
-#define PAGE_BTI  PAGE_TARGET_1
-#define PAGE_MTE  PAGE_TARGET_2
+#define PAGE_BTI            PAGE_TARGET_1
+#define PAGE_MTE            PAGE_TARGET_2
+#define PAGE_TARGET_STICKY  PAGE_MTE
 
 #ifdef TARGET_TAGGED_ADDRESSES
 /**
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 8fd23a9d05f..ef62a199c7d 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2256,6 +2256,15 @@ int page_get_flags(target_ulong address)
     return p->flags;
 }
 
+/*
+ * Allow the target to decide if PAGE_TARGET_[12] may be reset.
+ * By default, they are not kept.
+ */
+#ifndef PAGE_TARGET_STICKY
+#define PAGE_TARGET_STICKY  0
+#endif
+#define PAGE_STICKY  (PAGE_ANON | PAGE_TARGET_STICKY)
+
 /* Modify the flags of a page and invalidate the code if necessary.
    The flag PAGE_WRITE_ORG is positioned automatically depending
    on PAGE_WRITE.  The mmap_lock should already be held.  */
@@ -2299,8 +2308,8 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
             p->target_data = NULL;
             p->flags = flags;
         } else {
-            /* Using mprotect on a page does not change MAP_ANON. */
-            p->flags = (p->flags & PAGE_ANON) | flags;
+            /* Using mprotect on a page does not change sticky bits. */
+            p->flags = (p->flags & PAGE_STICKY) | flags;
         }
     }
 }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 05/15] target/arm: Define and use new regime_tcr_value() function
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (3 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 04/15] linux-user/aarch64: Do not clear PROT_MTE on mprotect Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 06/15] target/arm: Calculate mask/base_mask in get_level1_table_address() Peter Maydell
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

The regime_tcr() function returns a pointer to a struct TCR
corresponding to the TCR controlling a translation regime.  The
struct TCR has the raw value of the register, plus two fields mask
and base_mask which are used as a small optimization in the case of
32-bit short-descriptor lookups.  Almost all callers of regime_tcr()
only want the raw register value.  Define and use a new
regime_tcr_value() function which returns only the raw 64-bit
register value.

This is a preliminary to removing the 32-bit short descriptor
optimization -- it only saves a handful of bit operations, which is
tiny compared to the overhead of doing a page table walk at all, and
the TCR struct is awkward and makes fixing
https://gitlab.com/qemu-project/qemu/-/issues/1103 unnecessarily
difficult.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220714132303.1287193-2-peter.maydell@linaro.org
---
 target/arm/internals.h  | 6 ++++++
 target/arm/helper.c     | 6 +++---
 target/arm/ptw.c        | 8 ++++----
 target/arm/tlb_helper.c | 2 +-
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 00e2e710f6c..fa046124fa8 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -793,6 +793,12 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
     return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
 }
 
+/* Return the raw value of the TCR controlling this translation regime */
+static inline uint64_t regime_tcr_value(CPUARMState *env, ARMMMUIdx mmu_idx)
+{
+    return regime_tcr(env, mmu_idx)->raw_tcr;
+}
+
 /**
  * arm_num_brps: Return number of implemented breakpoints.
  * Note that the ID register BRPS field is "number of bps - 1",
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 24c45a9bf31..c245922bb5d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4216,7 +4216,7 @@ static int vae1_tlbmask(CPUARMState *env)
 static int tlbbits_for_regime(CPUARMState *env, ARMMMUIdx mmu_idx,
                               uint64_t addr)
 {
-    uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
+    uint64_t tcr = regime_tcr_value(env, mmu_idx);
     int tbi = aa64_va_parameter_tbi(tcr, mmu_idx);
     int select = extract64(addr, 55, 1);
 
@@ -10158,7 +10158,7 @@ static int aa64_va_parameter_tcma(uint64_t tcr, ARMMMUIdx mmu_idx)
 ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
                                    ARMMMUIdx mmu_idx, bool data)
 {
-    uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
+    uint64_t tcr = regime_tcr_value(env, mmu_idx);
     bool epd, hpd, using16k, using64k, tsz_oob, ds;
     int select, tsz, tbi, max_tsz, min_tsz, ps, sh;
     ARMCPU *cpu = env_archcpu(env);
@@ -10849,7 +10849,7 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
 {
     CPUARMTBFlags flags = {};
     ARMMMUIdx stage1 = stage_1_mmu_idx(mmu_idx);
-    uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
+    uint64_t tcr = regime_tcr_value(env, mmu_idx);
     uint64_t sctlr;
     int tbii, tbid;
 
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index e71fc1f4293..0d7e8ffa41b 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -820,7 +820,7 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
 static ARMVAParameters aa32_va_parameters(CPUARMState *env, uint32_t va,
                                           ARMMMUIdx mmu_idx)
 {
-    uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
+    uint64_t tcr = regime_tcr_value(env, mmu_idx);
     uint32_t el = regime_el(env, mmu_idx);
     int select, tsz;
     bool epd, hpd;
@@ -994,7 +994,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address,
     uint32_t attrs;
     int32_t stride;
     int addrsize, inputsize, outputsize;
-    TCR *tcr = regime_tcr(env, mmu_idx);
+    uint64_t tcr = regime_tcr_value(env, mmu_idx);
     int ap, ns, xn, pxn;
     uint32_t el = regime_el(env, mmu_idx);
     uint64_t descaddrmask;
@@ -1112,8 +1112,8 @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address,
          * For stage 2 translations the starting level is specified by the
          * VTCR_EL2.SL0 field (whose interpretation depends on the page size)
          */
-        uint32_t sl0 = extract32(tcr->raw_tcr, 6, 2);
-        uint32_t sl2 = extract64(tcr->raw_tcr, 33, 1);
+        uint32_t sl0 = extract32(tcr, 6, 2);
+        uint32_t sl2 = extract64(tcr, 33, 1);
         uint32_t startlevel;
         bool ok;
 
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index 7d8a86b3c45..a2f87a5042d 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -20,7 +20,7 @@ bool regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
         return true;
     }
     if (arm_feature(env, ARM_FEATURE_LPAE)
-        && (regime_tcr(env, mmu_idx)->raw_tcr & TTBCR_EAE)) {
+        && (regime_tcr_value(env, mmu_idx) & TTBCR_EAE)) {
         return true;
     }
     return false;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 06/15] target/arm: Calculate mask/base_mask in get_level1_table_address()
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (4 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 05/15] target/arm: Define and use new regime_tcr_value() function Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 07/15] target/arm: Fold regime_tcr() and regime_tcr_value() together Peter Maydell
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

In get_level1_table_address(), instead of using precalculated values
of mask and base_mask from the TCR struct, calculate them directly
(in the same way we currently do in vmsa_ttbcr_raw_write() to
populate the TCR struct fields).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220714132303.1287193-3-peter.maydell@linaro.org
---
 target/arm/ptw.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 0d7e8ffa41b..16226d14233 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -315,20 +315,24 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
                                      uint32_t *table, uint32_t address)
 {
     /* Note that we can only get here for an AArch32 PL0/PL1 lookup */
-    TCR *tcr = regime_tcr(env, mmu_idx);
+    uint64_t tcr = regime_tcr_value(env, mmu_idx);
+    int maskshift = extract32(tcr, 0, 3);
+    uint32_t mask = ~(((uint32_t)0xffffffffu) >> maskshift);
+    uint32_t base_mask;
 
-    if (address & tcr->mask) {
-        if (tcr->raw_tcr & TTBCR_PD1) {
+    if (address & mask) {
+        if (tcr & TTBCR_PD1) {
             /* Translation table walk disabled for TTBR1 */
             return false;
         }
         *table = regime_ttbr(env, mmu_idx, 1) & 0xffffc000;
     } else {
-        if (tcr->raw_tcr & TTBCR_PD0) {
+        if (tcr & TTBCR_PD0) {
             /* Translation table walk disabled for TTBR0 */
             return false;
         }
-        *table = regime_ttbr(env, mmu_idx, 0) & tcr->base_mask;
+        base_mask = ~((uint32_t)0x3fffu >> maskshift);
+        *table = regime_ttbr(env, mmu_idx, 0) & base_mask;
     }
     *table |= (address >> 18) & 0x3ffc;
     return true;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 07/15] target/arm: Fold regime_tcr() and regime_tcr_value() together
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (5 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 06/15] target/arm: Calculate mask/base_mask in get_level1_table_address() Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 08/15] target/arm: Fix big-endian host handling of VTCR Peter Maydell
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

The only caller of regime_tcr() is now regime_tcr_value(); fold the
two together, and use the shorter and more natural 'regime_tcr'
name for the new function.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220714132303.1287193-4-peter.maydell@linaro.org
---
 target/arm/internals.h  | 16 +++++-----------
 target/arm/helper.c     |  6 +++---
 target/arm/ptw.c        |  6 +++---
 target/arm/tlb_helper.c |  2 +-
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index fa046124fa8..0a1eb20afce 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -777,26 +777,20 @@ static inline uint64_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx)
     return env->cp15.sctlr_el[regime_el(env, mmu_idx)];
 }
 
-/* Return the TCR controlling this translation regime */
-static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
+/* Return the value of the TCR controlling this translation regime */
+static inline uint64_t regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
 {
     if (mmu_idx == ARMMMUIdx_Stage2) {
-        return &env->cp15.vtcr_el2;
+        return env->cp15.vtcr_el2.raw_tcr;
     }
     if (mmu_idx == ARMMMUIdx_Stage2_S) {
         /*
          * Note: Secure stage 2 nominally shares fields from VTCR_EL2, but
          * those are not currently used by QEMU, so just return VSTCR_EL2.
          */
-        return &env->cp15.vstcr_el2;
+        return env->cp15.vstcr_el2.raw_tcr;
     }
-    return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
-}
-
-/* Return the raw value of the TCR controlling this translation regime */
-static inline uint64_t regime_tcr_value(CPUARMState *env, ARMMMUIdx mmu_idx)
-{
-    return regime_tcr(env, mmu_idx)->raw_tcr;
+    return env->cp15.tcr_el[regime_el(env, mmu_idx)].raw_tcr;
 }
 
 /**
diff --git a/target/arm/helper.c b/target/arm/helper.c
index c245922bb5d..8847f5b90ad 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4216,7 +4216,7 @@ static int vae1_tlbmask(CPUARMState *env)
 static int tlbbits_for_regime(CPUARMState *env, ARMMMUIdx mmu_idx,
                               uint64_t addr)
 {
-    uint64_t tcr = regime_tcr_value(env, mmu_idx);
+    uint64_t tcr = regime_tcr(env, mmu_idx);
     int tbi = aa64_va_parameter_tbi(tcr, mmu_idx);
     int select = extract64(addr, 55, 1);
 
@@ -10158,7 +10158,7 @@ static int aa64_va_parameter_tcma(uint64_t tcr, ARMMMUIdx mmu_idx)
 ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
                                    ARMMMUIdx mmu_idx, bool data)
 {
-    uint64_t tcr = regime_tcr_value(env, mmu_idx);
+    uint64_t tcr = regime_tcr(env, mmu_idx);
     bool epd, hpd, using16k, using64k, tsz_oob, ds;
     int select, tsz, tbi, max_tsz, min_tsz, ps, sh;
     ARMCPU *cpu = env_archcpu(env);
@@ -10849,7 +10849,7 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
 {
     CPUARMTBFlags flags = {};
     ARMMMUIdx stage1 = stage_1_mmu_idx(mmu_idx);
-    uint64_t tcr = regime_tcr_value(env, mmu_idx);
+    uint64_t tcr = regime_tcr(env, mmu_idx);
     uint64_t sctlr;
     int tbii, tbid;
 
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 16226d14233..e9959848d88 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -315,7 +315,7 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
                                      uint32_t *table, uint32_t address)
 {
     /* Note that we can only get here for an AArch32 PL0/PL1 lookup */
-    uint64_t tcr = regime_tcr_value(env, mmu_idx);
+    uint64_t tcr = regime_tcr(env, mmu_idx);
     int maskshift = extract32(tcr, 0, 3);
     uint32_t mask = ~(((uint32_t)0xffffffffu) >> maskshift);
     uint32_t base_mask;
@@ -824,7 +824,7 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
 static ARMVAParameters aa32_va_parameters(CPUARMState *env, uint32_t va,
                                           ARMMMUIdx mmu_idx)
 {
-    uint64_t tcr = regime_tcr_value(env, mmu_idx);
+    uint64_t tcr = regime_tcr(env, mmu_idx);
     uint32_t el = regime_el(env, mmu_idx);
     int select, tsz;
     bool epd, hpd;
@@ -998,7 +998,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address,
     uint32_t attrs;
     int32_t stride;
     int addrsize, inputsize, outputsize;
-    uint64_t tcr = regime_tcr_value(env, mmu_idx);
+    uint64_t tcr = regime_tcr(env, mmu_idx);
     int ap, ns, xn, pxn;
     uint32_t el = regime_el(env, mmu_idx);
     uint64_t descaddrmask;
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index a2f87a5042d..5a709eab56f 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -20,7 +20,7 @@ bool regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
         return true;
     }
     if (arm_feature(env, ARM_FEATURE_LPAE)
-        && (regime_tcr_value(env, mmu_idx) & TTBCR_EAE)) {
+        && (regime_tcr(env, mmu_idx) & TTBCR_EAE)) {
         return true;
     }
     return false;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 08/15] target/arm: Fix big-endian host handling of VTCR
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (6 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 07/15] target/arm: Fold regime_tcr() and regime_tcr_value() together Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 09/15] target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t Peter Maydell
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

We have a bug in our handling of accesses to the AArch32 VTCR
register on big-endian hosts: we were not adjusting the part of the
uint64_t field within TCR that the generated code would access.  That
can be done with offsetoflow32(), by using an ARM_CP_STATE_BOTH cpreg
struct, or by defining a full set of read/write/reset functions --
the various other TCR cpreg structs used one or another of those
strategies, but for VTCR we did not, so on a big-endian host VTCR
accesses would touch the wrong half of the register.

Use offsetoflow32() in the VTCR register struct.  This works even
though the field in the CPU struct is currently a struct TCR, because
the first field in that struct is the uint64_t raw_tcr.

None of the other TCR registers have this bug -- either they are
AArch64 only, or else they define resetfn, writefn, etc, and
expect to be passed the full struct pointer.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220714132303.1287193-5-peter.maydell@linaro.org
---
 target/arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 8847f5b90ad..7461d4091ef 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5409,7 +5409,7 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .cp = 15, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
       .type = ARM_CP_ALIAS,
       .access = PL2_RW, .accessfn = access_el3_aa32ns,
-      .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
+      .fieldoffset = offsetoflow32(CPUARMState, cp15.vtcr_el2) },
     { .name = "VTCR_EL2", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
       .access = PL2_RW,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 09/15] target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (7 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 08/15] target/arm: Fix big-endian host handling of VTCR Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 10/15] target/arm: Store TCR_EL* " Peter Maydell
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

Change the representation of the VSTCR_EL2 and VTCR_EL2 registers in
the CPU state struct from struct TCR to uint64_t.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220714132303.1287193-6-peter.maydell@linaro.org
---
 target/arm/cpu.h       |  4 ++--
 target/arm/internals.h |  4 ++--
 target/arm/helper.c    |  4 +---
 target/arm/ptw.c       | 14 +++++++-------
 4 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 6afcc882f2c..b14c7c3eec3 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -340,8 +340,8 @@ typedef struct CPUArchState {
         uint64_t vsttbr_el2; /* Secure Virtualization Translation Table. */
         /* MMU translation table base control. */
         TCR tcr_el[4];
-        TCR vtcr_el2; /* Virtualization Translation Control.  */
-        TCR vstcr_el2; /* Secure Virtualization Translation Control. */
+        uint64_t vtcr_el2; /* Virtualization Translation Control.  */
+        uint64_t vstcr_el2; /* Secure Virtualization Translation Control. */
         uint32_t c2_data; /* MPU data cacheable bits.  */
         uint32_t c2_insn; /* MPU instruction cacheable bits.  */
         union { /* MMU domain access control register
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 0a1eb20afce..9f654b12cea 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -781,14 +781,14 @@ static inline uint64_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx)
 static inline uint64_t regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
 {
     if (mmu_idx == ARMMMUIdx_Stage2) {
-        return env->cp15.vtcr_el2.raw_tcr;
+        return env->cp15.vtcr_el2;
     }
     if (mmu_idx == ARMMMUIdx_Stage2_S) {
         /*
          * Note: Secure stage 2 nominally shares fields from VTCR_EL2, but
          * those are not currently used by QEMU, so just return VSTCR_EL2.
          */
-        return env->cp15.vstcr_el2.raw_tcr;
+        return env->cp15.vstcr_el2;
     }
     return env->cp15.tcr_el[regime_el(env, mmu_idx)].raw_tcr;
 }
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7461d4091ef..ea541e4b0c9 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5413,9 +5413,7 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
     { .name = "VTCR_EL2", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
       .access = PL2_RW,
-      /* no .writefn needed as this can't cause an ASID change;
-       * no .raw_writefn or .resetfn needed as we never use mask/base_mask
-       */
+      /* no .writefn needed as this can't cause an ASID change */
       .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
     { .name = "VTTBR", .state = ARM_CP_STATE_AA32,
       .cp = 15, .opc1 = 6, .crm = 2,
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index e9959848d88..8049c67f039 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -241,9 +241,9 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
         if (arm_is_secure_below_el3(env)) {
             /* Check if page table walk is to secure or non-secure PA space. */
             if (*is_secure) {
-                *is_secure = !(env->cp15.vstcr_el2.raw_tcr & VSTCR_SW);
+                *is_secure = !(env->cp15.vstcr_el2 & VSTCR_SW);
             } else {
-                *is_secure = !(env->cp15.vtcr_el2.raw_tcr & VTCR_NSW);
+                *is_secure = !(env->cp15.vtcr_el2 & VTCR_NSW);
             }
         } else {
             assert(!*is_secure);
@@ -2341,9 +2341,9 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
             ipa_secure = attrs->secure;
             if (arm_is_secure_below_el3(env)) {
                 if (ipa_secure) {
-                    attrs->secure = !(env->cp15.vstcr_el2.raw_tcr & VSTCR_SW);
+                    attrs->secure = !(env->cp15.vstcr_el2 & VSTCR_SW);
                 } else {
-                    attrs->secure = !(env->cp15.vtcr_el2.raw_tcr & VTCR_NSW);
+                    attrs->secure = !(env->cp15.vtcr_el2 & VTCR_NSW);
                 }
             } else {
                 assert(!ipa_secure);
@@ -2385,11 +2385,11 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
             if (arm_is_secure_below_el3(env)) {
                 if (ipa_secure) {
                     attrs->secure =
-                        !(env->cp15.vstcr_el2.raw_tcr & (VSTCR_SA | VSTCR_SW));
+                        !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW));
                 } else {
                     attrs->secure =
-                        !((env->cp15.vtcr_el2.raw_tcr & (VTCR_NSA | VTCR_NSW))
-                        || (env->cp15.vstcr_el2.raw_tcr & (VSTCR_SA | VSTCR_SW)));
+                        !((env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))
+                        || (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)));
                 }
             }
             return 0;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 10/15] target/arm: Store TCR_EL* registers as uint64_t
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (8 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 09/15] target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 11/15] target/arm: Honour VTCR_EL2 bits in Secure EL2 Peter Maydell
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

Change the representation of the TCR_EL* registers in the CPU state
struct from struct TCR to uint64_t.  This allows us to drop the
custom vmsa_ttbcr_raw_write() function, moving the "enforce RES0"
checks to their more usual location in the writefn
vmsa_ttbcr_write().  We also don't need the resetfn any more.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220714132303.1287193-7-peter.maydell@linaro.org
---
 target/arm/cpu.h          |  8 +----
 target/arm/internals.h    |  6 ++--
 target/arm/cpu.c          |  2 +-
 target/arm/debug_helper.c |  2 +-
 target/arm/helper.c       | 75 +++++++++++----------------------------
 target/arm/ptw.c          |  2 +-
 6 files changed, 27 insertions(+), 68 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index b14c7c3eec3..b43083c5ef5 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -166,12 +166,6 @@ typedef struct ARMGenericTimer {
 #define GTIMER_HYPVIRT  4
 #define NUM_GTIMERS     5
 
-typedef struct {
-    uint64_t raw_tcr;
-    uint32_t mask;
-    uint32_t base_mask;
-} TCR;
-
 #define VTCR_NSW (1u << 29)
 #define VTCR_NSA (1u << 30)
 #define VSTCR_SW VTCR_NSW
@@ -339,7 +333,7 @@ typedef struct CPUArchState {
         uint64_t vttbr_el2; /* Virtualization Translation Table Base.  */
         uint64_t vsttbr_el2; /* Secure Virtualization Translation Table. */
         /* MMU translation table base control. */
-        TCR tcr_el[4];
+        uint64_t tcr_el[4];
         uint64_t vtcr_el2; /* Virtualization Translation Control.  */
         uint64_t vstcr_el2; /* Secure Virtualization Translation Control. */
         uint32_t c2_data; /* MPU data cacheable bits.  */
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 9f654b12cea..742135ef146 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -252,9 +252,9 @@ unsigned int arm_pamax(ARMCPU *cpu);
  */
 static inline bool extended_addresses_enabled(CPUARMState *env)
 {
-    TCR *tcr = &env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1];
+    uint64_t tcr = env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1];
     return arm_el_is_aa64(env, 1) ||
-           (arm_feature(env, ARM_FEATURE_LPAE) && (tcr->raw_tcr & TTBCR_EAE));
+           (arm_feature(env, ARM_FEATURE_LPAE) && (tcr & TTBCR_EAE));
 }
 
 /* Update a QEMU watchpoint based on the information the guest has set in the
@@ -790,7 +790,7 @@ static inline uint64_t regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
          */
         return env->cp15.vstcr_el2;
     }
-    return env->cp15.tcr_el[regime_el(env, mmu_idx)].raw_tcr;
+    return env->cp15.tcr_el[regime_el(env, mmu_idx)];
 }
 
 /**
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5de7e097e9b..1b7b3d76bb3 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -226,7 +226,7 @@ static void arm_cpu_reset(DeviceState *dev)
          * Enable TBI0 but not TBI1.
          * Note that this must match useronly_clean_ptr.
          */
-        env->cp15.tcr_el[1].raw_tcr = 5 | (1ULL << 37);
+        env->cp15.tcr_el[1] = 5 | (1ULL << 37);
 
         /* Enable MTE */
         if (cpu_isar_feature(aa64_mte, cpu)) {
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index d09fccb0a4f..c21739242c5 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -439,7 +439,7 @@ static uint32_t arm_debug_exception_fsr(CPUARMState *env)
         using_lpae = true;
     } else {
         if (arm_feature(env, ARM_FEATURE_LPAE) &&
-            (env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)) {
+            (env->cp15.tcr_el[target_el] & TTBCR_EAE)) {
             using_lpae = true;
         }
     }
diff --git a/target/arm/helper.c b/target/arm/helper.c
index ea541e4b0c9..1a8b06410e6 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3606,19 +3606,21 @@ static const ARMCPRegInfo pmsav5_cp_reginfo[] = {
       .fieldoffset = offsetof(CPUARMState, cp15.c6_region[7]) },
 };
 
-static void vmsa_ttbcr_raw_write(CPUARMState *env, const ARMCPRegInfo *ri,
-                                 uint64_t value)
+static void vmsa_ttbcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                             uint64_t value)
 {
-    TCR *tcr = raw_ptr(env, ri);
-    int maskshift = extract32(value, 0, 3);
+    ARMCPU *cpu = env_archcpu(env);
 
     if (!arm_feature(env, ARM_FEATURE_V8)) {
         if (arm_feature(env, ARM_FEATURE_LPAE) && (value & TTBCR_EAE)) {
-            /* Pre ARMv8 bits [21:19], [15:14] and [6:3] are UNK/SBZP when
-             * using Long-desciptor translation table format */
+            /*
+             * Pre ARMv8 bits [21:19], [15:14] and [6:3] are UNK/SBZP when
+             * using Long-descriptor translation table format
+             */
             value &= ~((7 << 19) | (3 << 14) | (0xf << 3));
         } else if (arm_feature(env, ARM_FEATURE_EL3)) {
-            /* In an implementation that includes the Security Extensions
+            /*
+             * In an implementation that includes the Security Extensions
              * TTBCR has additional fields PD0 [4] and PD1 [5] for
              * Short-descriptor translation table format.
              */
@@ -3628,55 +3630,23 @@ static void vmsa_ttbcr_raw_write(CPUARMState *env, const ARMCPRegInfo *ri,
         }
     }
 
-    /* Update the masks corresponding to the TCR bank being written
-     * Note that we always calculate mask and base_mask, but
-     * they are only used for short-descriptor tables (ie if EAE is 0);
-     * for long-descriptor tables the TCR fields are used differently
-     * and the mask and base_mask values are meaningless.
-     */
-    tcr->raw_tcr = value;
-    tcr->mask = ~(((uint32_t)0xffffffffu) >> maskshift);
-    tcr->base_mask = ~((uint32_t)0x3fffu >> maskshift);
-}
-
-static void vmsa_ttbcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
-                             uint64_t value)
-{
-    ARMCPU *cpu = env_archcpu(env);
-    TCR *tcr = raw_ptr(env, ri);
-
     if (arm_feature(env, ARM_FEATURE_LPAE)) {
         /* With LPAE the TTBCR could result in a change of ASID
          * via the TTBCR.A1 bit, so do a TLB flush.
          */
         tlb_flush(CPU(cpu));
     }
-    /* Preserve the high half of TCR_EL1, set via TTBCR2.  */
-    value = deposit64(tcr->raw_tcr, 0, 32, value);
-    vmsa_ttbcr_raw_write(env, ri, value);
-}
-
-static void vmsa_ttbcr_reset(CPUARMState *env, const ARMCPRegInfo *ri)
-{
-    TCR *tcr = raw_ptr(env, ri);
-
-    /* Reset both the TCR as well as the masks corresponding to the bank of
-     * the TCR being reset.
-     */
-    tcr->raw_tcr = 0;
-    tcr->mask = 0;
-    tcr->base_mask = 0xffffc000u;
+    raw_write(env, ri, value);
 }
 
 static void vmsa_tcr_el12_write(CPUARMState *env, const ARMCPRegInfo *ri,
                                uint64_t value)
 {
     ARMCPU *cpu = env_archcpu(env);
-    TCR *tcr = raw_ptr(env, ri);
 
     /* For AArch64 the A1 bit could result in a change of ASID, so TLB flush. */
     tlb_flush(CPU(cpu));
-    tcr->raw_tcr = value;
+    raw_write(env, ri, value);
 }
 
 static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3780,15 +3750,15 @@ static const ARMCPRegInfo vmsa_cp_reginfo[] = {
       .opc0 = 3, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 2,
       .access = PL1_RW, .accessfn = access_tvm_trvm,
       .writefn = vmsa_tcr_el12_write,
-      .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
+      .raw_writefn = raw_write,
+      .resetvalue = 0,
       .fieldoffset = offsetof(CPUARMState, cp15.tcr_el[1]) },
     { .name = "TTBCR", .cp = 15, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 2,
       .access = PL1_RW, .accessfn = access_tvm_trvm,
       .type = ARM_CP_ALIAS, .writefn = vmsa_ttbcr_write,
-      .raw_writefn = vmsa_ttbcr_raw_write,
-      /* No offsetoflow32 -- pass the entire TCR to writefn/raw_writefn. */
-      .bank_fieldoffsets = { offsetof(CPUARMState, cp15.tcr_el[3]),
-                             offsetof(CPUARMState, cp15.tcr_el[1])} },
+      .raw_writefn = raw_write,
+      .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.tcr_el[3]),
+                             offsetoflow32(CPUARMState, cp15.tcr_el[1])} },
 };
 
 /* Note that unlike TTBCR, writing to TTBCR2 does not require flushing
@@ -3799,8 +3769,8 @@ static const ARMCPRegInfo ttbcr2_reginfo = {
     .access = PL1_RW, .accessfn = access_tvm_trvm,
     .type = ARM_CP_ALIAS,
     .bank_fieldoffsets = {
-        offsetofhigh32(CPUARMState, cp15.tcr_el[3].raw_tcr),
-        offsetofhigh32(CPUARMState, cp15.tcr_el[1].raw_tcr),
+        offsetofhigh32(CPUARMState, cp15.tcr_el[3]),
+        offsetofhigh32(CPUARMState, cp15.tcr_el[1]),
     },
 };
 
@@ -5403,7 +5373,6 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
     { .name = "TCR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 0, .opc2 = 2,
       .access = PL2_RW, .writefn = vmsa_tcr_el12_write,
-      /* no .raw_writefn or .resetfn needed as we never use mask/base_mask */
       .fieldoffset = offsetof(CPUARMState, cp15.tcr_el[2]) },
     { .name = "VTCR", .state = ARM_CP_STATE_AA32,
       .cp = 15, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
@@ -5643,12 +5612,8 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
     { .name = "TCR_EL3", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 6, .crn = 2, .crm = 0, .opc2 = 2,
       .access = PL3_RW,
-      /* no .writefn needed as this can't cause an ASID change;
-       * we must provide a .raw_writefn and .resetfn because we handle
-       * reset and migration for the AArch32 TTBCR(S), which might be
-       * using mask and base_mask.
-       */
-      .resetfn = vmsa_ttbcr_reset, .raw_writefn = vmsa_ttbcr_raw_write,
+      /* no .writefn needed as this can't cause an ASID change */
+      .resetvalue = 0,
       .fieldoffset = offsetof(CPUARMState, cp15.tcr_el[3]) },
     { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_ALIAS,
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 8049c67f039..3261039d93a 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -2466,7 +2466,7 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
             int r_el = regime_el(env, mmu_idx);
             if (arm_el_is_aa64(env, r_el)) {
                 int pamax = arm_pamax(env_archcpu(env));
-                uint64_t tcr = env->cp15.tcr_el[r_el].raw_tcr;
+                uint64_t tcr = env->cp15.tcr_el[r_el];
                 int addrtop, tbi;
 
                 tbi = aa64_va_parameter_tbi(tcr, mmu_idx);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 11/15] target/arm: Honour VTCR_EL2 bits in Secure EL2
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (9 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 10/15] target/arm: Store TCR_EL* " Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 12/15] hw/adc: Fix CONV bit in NPCM7XX ADC CON register Peter Maydell
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

In regime_tcr() we return the appropriate TCR register for the
translation regime.  For Secure EL2, we return the VSTCR_EL2 value,
but in this translation regime some fields that control behaviour are
in VTCR_EL2.  When this code was originally written (as the comment
notes), QEMU didn't care about any of those fields, but we have since
added support for features such as LPA2 which do need the values from
those fields.

Synthesize a TCR value by merging in the relevant VTCR_EL2 fields to
the VSTCR_EL2 value.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1103
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220714132303.1287193-8-peter.maydell@linaro.org
---
 target/arm/cpu.h       | 19 +++++++++++++++++++
 target/arm/internals.h | 22 +++++++++++++++++++---
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index b43083c5ef5..e890ee074d3 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1412,6 +1412,25 @@ FIELD(CPTR_EL3, TCPAC, 31, 1)
 #define TTBCR_SH1    (1U << 28)
 #define TTBCR_EAE    (1U << 31)
 
+FIELD(VTCR, T0SZ, 0, 6)
+FIELD(VTCR, SL0, 6, 2)
+FIELD(VTCR, IRGN0, 8, 2)
+FIELD(VTCR, ORGN0, 10, 2)
+FIELD(VTCR, SH0, 12, 2)
+FIELD(VTCR, TG0, 14, 2)
+FIELD(VTCR, PS, 16, 3)
+FIELD(VTCR, VS, 19, 1)
+FIELD(VTCR, HA, 21, 1)
+FIELD(VTCR, HD, 22, 1)
+FIELD(VTCR, HWU59, 25, 1)
+FIELD(VTCR, HWU60, 26, 1)
+FIELD(VTCR, HWU61, 27, 1)
+FIELD(VTCR, HWU62, 28, 1)
+FIELD(VTCR, NSW, 29, 1)
+FIELD(VTCR, NSA, 30, 1)
+FIELD(VTCR, DS, 32, 1)
+FIELD(VTCR, SL2, 33, 1)
+
 /* Bit definitions for ARMv8 SPSR (PSTATE) format.
  * Only these are valid when in AArch64 mode; in
  * AArch32 mode SPSRs are basically CPSR-format.
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 742135ef146..b8fefdff675 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -777,6 +777,16 @@ static inline uint64_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx)
     return env->cp15.sctlr_el[regime_el(env, mmu_idx)];
 }
 
+/*
+ * These are the fields in VTCR_EL2 which affect both the Secure stage 2
+ * and the Non-Secure stage 2 translation regimes (and hence which are
+ * not present in VSTCR_EL2).
+ */
+#define VTCR_SHARED_FIELD_MASK \
+    (R_VTCR_IRGN0_MASK | R_VTCR_ORGN0_MASK | R_VTCR_SH0_MASK | \
+     R_VTCR_PS_MASK | R_VTCR_VS_MASK | R_VTCR_HA_MASK | R_VTCR_HD_MASK | \
+     R_VTCR_DS_MASK)
+
 /* Return the value of the TCR controlling this translation regime */
 static inline uint64_t regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
 {
@@ -785,10 +795,16 @@ static inline uint64_t regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
     }
     if (mmu_idx == ARMMMUIdx_Stage2_S) {
         /*
-         * Note: Secure stage 2 nominally shares fields from VTCR_EL2, but
-         * those are not currently used by QEMU, so just return VSTCR_EL2.
+         * Secure stage 2 shares fields from VTCR_EL2. We merge those
+         * in with the VSTCR_EL2 value to synthesize a single VTCR_EL2 format
+         * value so the callers don't need to special case this.
+         *
+         * If a future architecture change defines bits in VSTCR_EL2 that
+         * overlap with these VTCR_EL2 fields we may need to revisit this.
          */
-        return env->cp15.vstcr_el2;
+        uint64_t v = env->cp15.vstcr_el2 & ~VTCR_SHARED_FIELD_MASK;
+        v |= env->cp15.vtcr_el2 & VTCR_SHARED_FIELD_MASK;
+        return v;
     }
     return env->cp15.tcr_el[regime_el(env, mmu_idx)];
 }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 12/15] hw/adc: Fix CONV bit in NPCM7XX ADC CON register
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (10 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 11/15] target/arm: Honour VTCR_EL2 bits in Secure EL2 Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 13/15] hw/adc: Make adci[*] R/W in NPCM7XX ADC Peter Maydell
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

From: Hao Wu <wuhaotsh@google.com>

The correct bit for the CONV bit in NPCM7XX ADC is bit 13. This patch
fixes that in the module, and also lower the IRQ when the guest
is done handling an interrupt event from the ADC module.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Patrick Venture<venture@google.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20220714182836.89602-4-wuhaotsh@google.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/adc/npcm7xx_adc.c           | 2 +-
 tests/qtest/npcm7xx_adc-test.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/adc/npcm7xx_adc.c b/hw/adc/npcm7xx_adc.c
index 0f0a9f63e20..47fb9e5f74c 100644
--- a/hw/adc/npcm7xx_adc.c
+++ b/hw/adc/npcm7xx_adc.c
@@ -36,7 +36,7 @@ REG32(NPCM7XX_ADC_DATA, 0x4)
 #define NPCM7XX_ADC_CON_INT     BIT(18)
 #define NPCM7XX_ADC_CON_EN      BIT(17)
 #define NPCM7XX_ADC_CON_RST     BIT(16)
-#define NPCM7XX_ADC_CON_CONV    BIT(14)
+#define NPCM7XX_ADC_CON_CONV    BIT(13)
 #define NPCM7XX_ADC_CON_DIV(rv) extract32(rv, 1, 8)
 
 #define NPCM7XX_ADC_MAX_RESULT      1023
diff --git a/tests/qtest/npcm7xx_adc-test.c b/tests/qtest/npcm7xx_adc-test.c
index 3fa6d9ece0b..8048044d281 100644
--- a/tests/qtest/npcm7xx_adc-test.c
+++ b/tests/qtest/npcm7xx_adc-test.c
@@ -50,7 +50,7 @@
 #define CON_INT     BIT(18)
 #define CON_EN      BIT(17)
 #define CON_RST     BIT(16)
-#define CON_CONV    BIT(14)
+#define CON_CONV    BIT(13)
 #define CON_DIV(rv) extract32(rv, 1, 8)
 
 #define FST_RDST    BIT(1)
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 13/15] hw/adc: Make adci[*] R/W in NPCM7XX ADC
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (11 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 12/15] hw/adc: Fix CONV bit in NPCM7XX ADC CON register Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 14/15] target/arm: Don't set syndrome ISS for loads and stores with writeback Peter Maydell
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

From: Hao Wu <wuhaotsh@google.com>

Our sensor test requires both reading and writing from a sensor's
QOM property. So we need to make the input of ADC module R/W instead
of write only for that to work.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Titus Rwantare <titusr@google.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20220714182836.89602-5-wuhaotsh@google.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/adc/npcm7xx_adc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/adc/npcm7xx_adc.c b/hw/adc/npcm7xx_adc.c
index 47fb9e5f74c..bc6f3f55e64 100644
--- a/hw/adc/npcm7xx_adc.c
+++ b/hw/adc/npcm7xx_adc.c
@@ -242,7 +242,7 @@ static void npcm7xx_adc_init(Object *obj)
 
     for (i = 0; i < NPCM7XX_ADC_NUM_INPUTS; ++i) {
         object_property_add_uint32_ptr(obj, "adci[*]",
-                &s->adci[i], OBJ_PROP_FLAG_WRITE);
+                &s->adci[i], OBJ_PROP_FLAG_READWRITE);
     }
     object_property_add_uint32_ptr(obj, "vref",
             &s->vref, OBJ_PROP_FLAG_WRITE);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 14/15] target/arm: Don't set syndrome ISS for loads and stores with writeback
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (12 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 13/15] hw/adc: Make adci[*] R/W in NPCM7XX ADC Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 13:59 ` [PULL 15/15] Align Raspberry Pi DMA interrupts with Linux DTS Peter Maydell
  2022-07-18 16:47 ` [PULL 00/15] target-arm queue Peter Maydell
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

The architecture requires that for faults on loads and stores which
do writeback, the syndrome information does not have the ISS
instruction syndrome information (i.e. ISV is 0).  We got this wrong
for the load and store instructions covered by disas_ldst_reg_imm9().
Calculate iss_valid correctly so that if the insn is a writeback one
it is false.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1057
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220715123323.1550983-1-peter.maydell@linaro.org
---
 target/arm/translate-a64.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index b7b64f73584..163df8c6157 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -3138,7 +3138,7 @@ static void disas_ldst_reg_imm9(DisasContext *s, uint32_t insn,
     bool is_store = false;
     bool is_extended = false;
     bool is_unpriv = (idx == 2);
-    bool iss_valid = !is_vector;
+    bool iss_valid;
     bool post_index;
     bool writeback;
     int memidx;
@@ -3191,6 +3191,8 @@ static void disas_ldst_reg_imm9(DisasContext *s, uint32_t insn,
         g_assert_not_reached();
     }
 
+    iss_valid = !is_vector && !writeback;
+
     if (rn == 31) {
         gen_check_sp_alignment(s);
     }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PULL 15/15] Align Raspberry Pi DMA interrupts with Linux DTS
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (13 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 14/15] target/arm: Don't set syndrome ISS for loads and stores with writeback Peter Maydell
@ 2022-07-18 13:59 ` Peter Maydell
  2022-07-18 16:47 ` [PULL 00/15] target-arm queue Peter Maydell
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 13:59 UTC (permalink / raw)
  To: qemu-devel

From: Andrey Makarov <ph.makarov@gmail.com>

There is nothing in the specs on DMA engine interrupt lines: it should have
been in the "BCM2835 ARM Peripherals" datasheet but the appropriate
"ARM peripherals interrupt table" (p.113) is nearly empty.

All Raspberry Pi models 1-3 (based on bcm2835) have
Linux device tree (arch/arm/boot/dts/bcm2835-common.dtsi +25):

    /* dma channel 11-14 share one irq */

This information is repeated in the driver code
(drivers/dma/bcm2835-dma.c +1344):

    /*
     * in case of channel >= 11
     * use the 11th interrupt and that is shared
     */

In this patch channels 0--10 and 11--14 are handled separately.

Signed-off-by: Andrey Makarov <andrey.makarov@auriga.com>
Message-id: 20220716113210.349153-1-andrey.makarov@auriga.com
[PMM: fixed checkpatch nits]
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/bcm2835_peripherals.h |   2 +
 hw/arm/bcm2835_peripherals.c         |  26 +++++-
 tests/qtest/bcm2835-dma-test.c       | 118 +++++++++++++++++++++++++++
 tests/qtest/meson.build              |   3 +-
 4 files changed, 147 insertions(+), 2 deletions(-)
 create mode 100644 tests/qtest/bcm2835-dma-test.c

diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm2835_peripherals.h
index d864879421a..c9d25d493e0 100644
--- a/include/hw/arm/bcm2835_peripherals.h
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -17,6 +17,7 @@
 #include "hw/char/bcm2835_aux.h"
 #include "hw/display/bcm2835_fb.h"
 #include "hw/dma/bcm2835_dma.h"
+#include "hw/or-irq.h"
 #include "hw/intc/bcm2835_ic.h"
 #include "hw/misc/bcm2835_property.h"
 #include "hw/misc/bcm2835_rng.h"
@@ -55,6 +56,7 @@ struct BCM2835PeripheralState {
     BCM2835AuxState aux;
     BCM2835FBState fb;
     BCM2835DMAState dma;
+    qemu_or_irq orgated_dma_irq;
     BCM2835ICState ic;
     BCM2835PropertyState property;
     BCM2835RngState rng;
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 48538c9360c..3c2a4160cd1 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -23,6 +23,13 @@
 /* Capabilities for SD controller: no DMA, high-speed, default clocks etc. */
 #define BCM2835_SDHC_CAPAREG 0x52134b4
 
+/*
+ * According to Linux driver & DTS, dma channels 0--10 have separate IRQ,
+ * while channels 11--14 share one IRQ:
+ */
+#define SEPARATE_DMA_IRQ_MAX 10
+#define ORGATED_DMA_IRQ_COUNT 4
+
 static void create_unimp(BCM2835PeripheralState *ps,
                          UnimplementedDeviceState *uds,
                          const char *name, hwaddr ofs, hwaddr size)
@@ -101,6 +108,11 @@ static void bcm2835_peripherals_init(Object *obj)
     /* DMA Channels */
     object_initialize_child(obj, "dma", &s->dma, TYPE_BCM2835_DMA);
 
+    object_initialize_child(obj, "orgated-dma-irq",
+                            &s->orgated_dma_irq, TYPE_OR_IRQ);
+    object_property_set_int(OBJECT(&s->orgated_dma_irq), "num-lines",
+                            ORGATED_DMA_IRQ_COUNT, &error_abort);
+
     object_property_add_const_link(OBJECT(&s->dma), "dma-mr",
                                    OBJECT(&s->gpu_bus_mr));
 
@@ -322,12 +334,24 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(&s->peri_mr, DMA15_OFFSET,
                 sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->dma), 1));
 
-    for (n = 0; n <= 12; n++) {
+    for (n = 0; n <= SEPARATE_DMA_IRQ_MAX; n++) {
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->dma), n,
                            qdev_get_gpio_in_named(DEVICE(&s->ic),
                                                   BCM2835_IC_GPU_IRQ,
                                                   INTERRUPT_DMA0 + n));
     }
+    if (!qdev_realize(DEVICE(&s->orgated_dma_irq), NULL, errp)) {
+        return;
+    }
+    for (n = 0; n < ORGATED_DMA_IRQ_COUNT; n++) {
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->dma),
+                           SEPARATE_DMA_IRQ_MAX + 1 + n,
+                           qdev_get_gpio_in(DEVICE(&s->orgated_dma_irq), n));
+    }
+    qdev_connect_gpio_out(DEVICE(&s->orgated_dma_irq), 0,
+                          qdev_get_gpio_in_named(DEVICE(&s->ic),
+                              BCM2835_IC_GPU_IRQ,
+                              INTERRUPT_DMA0 + SEPARATE_DMA_IRQ_MAX + 1));
 
     /* THERMAL */
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->thermal), errp)) {
diff --git a/tests/qtest/bcm2835-dma-test.c b/tests/qtest/bcm2835-dma-test.c
new file mode 100644
index 00000000000..8293d822b94
--- /dev/null
+++ b/tests/qtest/bcm2835-dma-test.c
@@ -0,0 +1,118 @@
+/*
+ * QTest testcase for BCM283x DMA engine (on Raspberry Pi 3)
+ * and its interrupts coming to Interrupt Controller.
+ *
+ * Copyright (c) 2022 Auriga LLC
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+
+/* Offsets in raspi3b platform: */
+#define RASPI3_DMA_BASE 0x3f007000
+#define RASPI3_IC_BASE  0x3f00b200
+
+/* Used register/fields definitions */
+
+/* DMA engine registers: */
+#define BCM2708_DMA_CS         0
+#define BCM2708_DMA_ACTIVE     (1 << 0)
+#define BCM2708_DMA_INT        (1 << 2)
+
+#define BCM2708_DMA_ADDR       0x04
+
+#define BCM2708_DMA_INT_STATUS 0xfe0
+
+/* DMA Trasfer Info fields: */
+#define BCM2708_DMA_INT_EN     (1 << 0)
+#define BCM2708_DMA_D_INC      (1 << 4)
+#define BCM2708_DMA_S_INC      (1 << 8)
+
+/* Interrupt controller registers: */
+#define IRQ_PENDING_BASIC      0x00
+#define IRQ_GPU_PENDING1_AGGR  (1 << 8)
+#define IRQ_PENDING_1          0x04
+#define IRQ_ENABLE_1           0x10
+
+/* Data for the test: */
+#define SCB_ADDR   256
+#define S_ADDR     32
+#define D_ADDR     64
+#define TXFR_LEN   32
+const uint32_t check_data = 0x12345678;
+
+static void bcm2835_dma_test_interrupt(int dma_c, int irq_line)
+{
+    uint64_t dma_base = RASPI3_DMA_BASE + dma_c * 0x100;
+    int gpu_irq_line = 16 + irq_line;
+
+    /* Check that interrupts are silent by default: */
+    writel(RASPI3_IC_BASE + IRQ_ENABLE_1, 1 << gpu_irq_line);
+    int isr = readl(dma_base + BCM2708_DMA_INT_STATUS);
+    g_assert_cmpint(isr, ==, 0);
+    uint32_t reg0 = readl(dma_base + BCM2708_DMA_CS);
+    g_assert_cmpint(reg0, ==, 0);
+    uint32_t ic_pending = readl(RASPI3_IC_BASE + IRQ_PENDING_BASIC);
+    g_assert_cmpint(ic_pending, ==, 0);
+    uint32_t gpu_pending1 = readl(RASPI3_IC_BASE + IRQ_PENDING_1);
+    g_assert_cmpint(gpu_pending1, ==, 0);
+
+    /* Prepare Control Block: */
+    writel(SCB_ADDR + 0, BCM2708_DMA_S_INC | BCM2708_DMA_D_INC |
+                         BCM2708_DMA_INT_EN); /* transfer info */
+    writel(SCB_ADDR + 4, S_ADDR);             /* source address */
+    writel(SCB_ADDR + 8, D_ADDR);             /* destination address */
+    writel(SCB_ADDR + 12, TXFR_LEN);          /* transfer length */
+    writel(dma_base + BCM2708_DMA_ADDR, SCB_ADDR);
+
+    writel(S_ADDR, check_data);
+    for (int word = S_ADDR + 4; word < S_ADDR + TXFR_LEN; word += 4) {
+        writel(word, ~check_data);
+    }
+    /* Perform the transfer: */
+    writel(dma_base + BCM2708_DMA_CS, BCM2708_DMA_ACTIVE);
+
+    /* Check that destination == source: */
+    uint32_t data = readl(D_ADDR);
+    g_assert_cmpint(data, ==, check_data);
+    for (int word = D_ADDR + 4; word < D_ADDR + TXFR_LEN; word += 4) {
+        data = readl(word);
+        g_assert_cmpint(data, ==, ~check_data);
+    }
+
+    /* Check that interrupt status is set both in DMA and IC controllers: */
+    isr = readl(RASPI3_DMA_BASE + BCM2708_DMA_INT_STATUS);
+    g_assert_cmpint(isr, ==, 1 << dma_c);
+
+    ic_pending = readl(RASPI3_IC_BASE + IRQ_PENDING_BASIC);
+    g_assert_cmpint(ic_pending, ==, IRQ_GPU_PENDING1_AGGR);
+
+    gpu_pending1 = readl(RASPI3_IC_BASE + IRQ_PENDING_1);
+    g_assert_cmpint(gpu_pending1, ==, 1 << gpu_irq_line);
+
+    /* Clean up, clear interrupt: */
+    writel(dma_base + BCM2708_DMA_CS, BCM2708_DMA_INT);
+}
+
+static void bcm2835_dma_test_interrupts(void)
+{
+    /* DMA engines 0--10 have separate IRQ lines, 11--14 - only one: */
+    bcm2835_dma_test_interrupt(0,  0);
+    bcm2835_dma_test_interrupt(10, 10);
+    bcm2835_dma_test_interrupt(11, 11);
+    bcm2835_dma_test_interrupt(14, 11);
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_func("/bcm2835/dma/test_interrupts",
+                   bcm2835_dma_test_interrupts);
+    qtest_start("-machine raspi3b");
+    ret = g_test_run();
+    qtest_end();
+    return ret;
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 31287a91739..3a474010e49 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -218,7 +218,8 @@ qtests_aarch64 = \
   ['arm-cpu-features',
    'numa-test',
    'boot-serial-test',
-   'migration-test']
+   'migration-test',
+   'bcm2835-dma-test']
 
 qtests_s390x = \
   (slirp.found() ? ['pxe-test', 'test-netfilter'] : []) +                 \
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PULL 00/15] target-arm queue
  2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
                   ` (14 preceding siblings ...)
  2022-07-18 13:59 ` [PULL 15/15] Align Raspberry Pi DMA interrupts with Linux DTS Peter Maydell
@ 2022-07-18 16:47 ` Peter Maydell
  15 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-07-18 16:47 UTC (permalink / raw)
  To: qemu-devel

On Mon, 18 Jul 2022 at 14:59, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Some arm patches before softfreeze. These are all bug fixes.
>
> -- PMM
>
> The following changes since commit 0ebf76aae58324b8f7bf6af798696687f5f4c2a9:
>
>   Merge tag 'nvme-next-pull-request' of git://git.infradead.org/qemu-nvme into staging (2022-07-15 15:38:13 +0100)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220718
>
> for you to fetch changes up to 004c8a8bc569c8b18fca6fc90ffe3223daaf17b7:
>
>   Align Raspberry Pi DMA interrupts with Linux DTS (2022-07-18 13:25:13 +0100)
>
> ----------------------------------------------------------------
> target-arm queue:
>  * hw/intc/armv7m_nvic: ICPRn must not unpend an IRQ that is being held high
>  * target/arm: Fill in VL for tbflags when SME enabled and SVE disabled
>  * target/arm: Fix aarch64_sve_change_el for SME
>  * linux-user/aarch64: Do not clear PROT_MTE on mprotect
>  * target/arm: Honour VTCR_EL2 bits in Secure EL2
>  * hw/adc: Fix CONV bit in NPCM7XX ADC CON register
>  * hw/adc: Make adci[*] R/W in NPCM7XX ADC
>  * target/arm: Don't set syndrome ISS for loads and stores with writeback
>  * Align Raspberry Pi DMA interrupts with Linux DTS
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/7.1
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PULL 00/15] target-arm queue
  2020-01-17 14:28 Peter Maydell
@ 2020-01-17 18:03 ` Peter Maydell
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2020-01-17 18:03 UTC (permalink / raw)
  To: QEMU Developers

On Fri, 17 Jan 2020 at 14:28, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Latest arm queue, a mixed bag of features and bug fixes.
>
> thanks
> -- PMM
>
> The following changes since commit cbf01142b2aef0c0b4e995cecd7e79d342bbc47e:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20200115' into staging (2020-01-17 12:13:17 +0000)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20200117-1
>
> for you to fetch changes up to 1a1fbc6cbb34c26d43d8360c66c1d21681af14a9:
>
>   target/arm: Set ISSIs16Bit in make_issinfo (2020-01-17 14:27:16 +0000)
>
> ----------------------------------------------------------------
> Add model of the Netduino Plus 2 board
> Some allwinner-a10 code cleanup
> New test cases for cubieboard
> target/arm/arm-semi: fix SYS_OPEN to return nonzero filehandle
> i.MX: add an emulation for RNGC device
> target/arm: adjust program counter for wfi exception in AArch32
> arm/gicv3: update virtual irq state after IAR register read
> Set IL bit correctly for syndrome information for data aborts
>

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PULL 00/15] target-arm queue
@ 2020-01-17 14:28 Peter Maydell
  2020-01-17 18:03 ` Peter Maydell
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2020-01-17 14:28 UTC (permalink / raw)
  To: qemu-devel

Latest arm queue, a mixed bag of features and bug fixes.

thanks
-- PMM

The following changes since commit cbf01142b2aef0c0b4e995cecd7e79d342bbc47e:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20200115' into staging (2020-01-17 12:13:17 +0000)

are available in the Git repository at:

  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20200117-1

for you to fetch changes up to 1a1fbc6cbb34c26d43d8360c66c1d21681af14a9:

  target/arm: Set ISSIs16Bit in make_issinfo (2020-01-17 14:27:16 +0000)

----------------------------------------------------------------
Add model of the Netduino Plus 2 board
Some allwinner-a10 code cleanup
New test cases for cubieboard
target/arm/arm-semi: fix SYS_OPEN to return nonzero filehandle
i.MX: add an emulation for RNGC device
target/arm: adjust program counter for wfi exception in AArch32
arm/gicv3: update virtual irq state after IAR register read
Set IL bit correctly for syndrome information for data aborts

----------------------------------------------------------------
Alistair Francis (4):
      hw/misc: Add the STM32F4xx Sysconfig device
      hw/misc: Add the STM32F4xx EXTI device
      hw/arm: Add the STM32F4xx SoC
      hw/arm: Add the Netduino Plus 2

Jeff Kubascik (3):
      target/arm: adjust program counter for wfi exception in AArch32
      arm/gicv3: update virtual irq state after IAR register read
      target/arm: Return correct IL bit in merge_syn_data_abort

Martin Kaiser (1):
      i.MX: add an emulation for RNGC

Masahiro Yamada (1):
      target/arm/arm-semi: fix SYS_OPEN to return nonzero filehandle

Philippe Mathieu-Daudé (5):
      tests/boot_linux_console: Add initrd test for the CubieBoard
      tests/boot_linux_console: Add a SD card test for the CubieBoard
      hw/arm/allwinner-a10: Move SoC definitions out of header
      hw/arm/allwinner-a10: Simplify by passing IRQs with qdev_pass_gpios()
      hw/arm/allwinner-a10: Remove local qemu_irq variables

Richard Henderson (1):
      target/arm: Set ISSIs16Bit in make_issinfo

 hw/arm/Makefile.objs                   |   2 +
 hw/misc/Makefile.objs                  |   3 +
 include/hw/arm/allwinner-a10.h         |   7 -
 include/hw/arm/fsl-imx25.h             |   5 +
 include/hw/arm/stm32f405_soc.h         |  73 ++++++++
 include/hw/misc/imx_rngc.h             |  35 ++++
 include/hw/misc/stm32f4xx_exti.h       |  60 +++++++
 include/hw/misc/stm32f4xx_syscfg.h     |  61 +++++++
 hw/arm/allwinner-a10.c                 |  39 +++--
 hw/arm/fsl-imx25.c                     |  11 ++
 hw/arm/netduinoplus2.c                 |  52 ++++++
 hw/arm/stm32f405_soc.c                 | 302 +++++++++++++++++++++++++++++++++
 hw/intc/arm_gicv3_cpuif.c              |   3 +
 hw/misc/imx_rngc.c                     | 278 ++++++++++++++++++++++++++++++
 hw/misc/stm32f4xx_exti.c               | 188 ++++++++++++++++++++
 hw/misc/stm32f4xx_syscfg.c             | 171 +++++++++++++++++++
 target/arm/arm-semi.c                  |   5 +-
 target/arm/op_helper.c                 |   7 +-
 target/arm/tlb_helper.c                |   2 +-
 target/arm/translate.c                 |   3 +
 MAINTAINERS                            |  14 ++
 default-configs/arm-softmmu.mak        |   1 +
 hw/arm/Kconfig                         |  10 ++
 hw/misc/Kconfig                        |   6 +
 hw/misc/trace-events                   |  11 ++
 tests/acceptance/boot_linux_console.py |  85 ++++++++++
 26 files changed, 1405 insertions(+), 29 deletions(-)
 create mode 100644 include/hw/arm/stm32f405_soc.h
 create mode 100644 include/hw/misc/imx_rngc.h
 create mode 100644 include/hw/misc/stm32f4xx_exti.h
 create mode 100644 include/hw/misc/stm32f4xx_syscfg.h
 create mode 100644 hw/arm/netduinoplus2.c
 create mode 100644 hw/arm/stm32f405_soc.c
 create mode 100644 hw/misc/imx_rngc.c
 create mode 100644 hw/misc/stm32f4xx_exti.c
 create mode 100644 hw/misc/stm32f4xx_syscfg.c


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-07-18 16:49 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-18 13:59 [PULL 00/15] target-arm queue Peter Maydell
2022-07-18 13:59 ` [PULL 01/15] hw/intc/armv7m_nvic: ICPRn must not unpend an IRQ that is being held high Peter Maydell
2022-07-18 13:59 ` [PULL 02/15] target/arm: Fill in VL for tbflags when SME enabled and SVE disabled Peter Maydell
2022-07-18 13:59 ` [PULL 03/15] target/arm: Fix aarch64_sve_change_el for SME Peter Maydell
2022-07-18 13:59 ` [PULL 04/15] linux-user/aarch64: Do not clear PROT_MTE on mprotect Peter Maydell
2022-07-18 13:59 ` [PULL 05/15] target/arm: Define and use new regime_tcr_value() function Peter Maydell
2022-07-18 13:59 ` [PULL 06/15] target/arm: Calculate mask/base_mask in get_level1_table_address() Peter Maydell
2022-07-18 13:59 ` [PULL 07/15] target/arm: Fold regime_tcr() and regime_tcr_value() together Peter Maydell
2022-07-18 13:59 ` [PULL 08/15] target/arm: Fix big-endian host handling of VTCR Peter Maydell
2022-07-18 13:59 ` [PULL 09/15] target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t Peter Maydell
2022-07-18 13:59 ` [PULL 10/15] target/arm: Store TCR_EL* " Peter Maydell
2022-07-18 13:59 ` [PULL 11/15] target/arm: Honour VTCR_EL2 bits in Secure EL2 Peter Maydell
2022-07-18 13:59 ` [PULL 12/15] hw/adc: Fix CONV bit in NPCM7XX ADC CON register Peter Maydell
2022-07-18 13:59 ` [PULL 13/15] hw/adc: Make adci[*] R/W in NPCM7XX ADC Peter Maydell
2022-07-18 13:59 ` [PULL 14/15] target/arm: Don't set syndrome ISS for loads and stores with writeback Peter Maydell
2022-07-18 13:59 ` [PULL 15/15] Align Raspberry Pi DMA interrupts with Linux DTS Peter Maydell
2022-07-18 16:47 ` [PULL 00/15] target-arm queue Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2020-01-17 14:28 Peter Maydell
2020-01-17 18:03 ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.