Re: [PATCH v2 4/4] target/arm: Add support for DC CVAP & DC CVADP ins

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 28/11/23 12:34, Peter Maydell wrote:
> On Tue, 28 Nov 2023 at 11:24, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Hi,
>>
>> On 6/11/19 00:41, Beata Michalska wrote:
>>> ARMv8.2 introduced support for Data Cache Clean instructions
>>> to PoP (point-of-persistence) - DC CVAP and PoDP (point-of-deep-persistence)
>>> - DV CVADP. Both specify conceptual points in a memory system where all writes
>>> that are to reach them are considered persistent.
>>> The support provided considers both to be actually the same so there is no
>>> distinction between the two. If none is available (there is no backing store
>>> for given memory) both will result in Data Cache Clean up to the point of
>>> coherency. Otherwise sync for the specified range shall be performed.
>>>
>>> Signed-off-by: Beata Michalska <beata.michalska@linaro.org>
>>> ---
>>>    linux-user/elfload.c |  2 ++
>>>    target/arm/cpu.h     | 10 ++++++++++
>>>    target/arm/cpu64.c   |  1 +
>>>    target/arm/helper.c  | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>    4 files changed, 69 insertions(+)
>>
>>
>>> +#ifndef CONFIG_USER_ONLY
>>> +static void dccvap_writefn(CPUARMState *env, const ARMCPRegInfo *opaque,
>>> +                          uint64_t value)
>>> +{
>>> +    ARMCPU *cpu = env_archcpu(env);
>>> +    /* CTR_EL0 System register -> DminLine, bits [19:16] */
>>> +    uint64_t dline_size = 4 << ((cpu->ctr >> 16) & 0xF);
>>> +    uint64_t vaddr_in = (uint64_t) value;
>>> +    uint64_t vaddr = vaddr_in & ~(dline_size - 1);
>>> +    void *haddr;
>>> +    int mem_idx = cpu_mmu_index(env, false);
>>> +
>>> +    /* This won't be crossing page boundaries */
>>> +    haddr = probe_read(env, vaddr, dline_size, mem_idx, GETPC());
>>> +    if (haddr) {
>>> +
>>> +        ram_addr_t offset;
>>> +        MemoryRegion *mr;
>>> +
>>> +        /* RCU lock is already being held */
>>> +        mr = memory_region_from_host(haddr, &offset);
>>> +
>>> +        if (mr) {
>>> +            memory_region_do_writeback(mr, offset, dline_size);
>>> +        }
>>> +    }
>>> +}
>>
>>
>>> +#ifndef CONFIG_USER_ONLY
>>> +    /* Data Cache clean instructions up to PoP */
>>> +    if (cpu_isar_feature(aa64_dcpop, cpu)) {
>>
>> Am I correct understanding this is a TCG-only feature?
> 
> For KVM, whether the vCPU implements these cache
> maintenance instructions is up to it -- like all insns,
> QEMU doesn't ever see if the guest executes them or not
> (either the host CPU just implements them, or the host
> kernel traps and handles them). The code in this patch is
> specifically for the QEMU TCG emulation of them.

Thank you Peter. In this case I'm compiling HVF, but this is the
same reasoning. I'll add #ifdef'ry similar to ats_write() (commit
9fb005b02d "target/arm: Restrict the Address Translate write operation
to TCG accel"):

-- >8 --
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 99c7da9ca4..a05e613e10 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7629,6 +7629,7 @@ static const ARMCPRegInfo rndr_reginfo[] = {
  static void dccvap_writefn(CPUARMState *env, const ARMCPRegInfo *opaque,
                            uint64_t value)
  {
+#ifdef CONFIG_TCG
      ARMCPU *cpu = env_archcpu(env);
      /* CTR_EL0 System register -> DminLine, bits [19:16] */
      uint64_t dline_size = 4 << ((cpu->ctr >> 16) & 0xF);
@@ -7653,6 +7654,10 @@ static void dccvap_writefn(CPUARMState *env, 
const ARMCPRegInfo *opaque,
          }
  #endif /*CONFIG_USER_ONLY*/
      }
+#else
+    /* Handled by hardware accelerator. */
+    g_assert_not_reached();
+#endif /* CONFIG_TCG */
  }
---

Regards,

Phil.