Re: [PATCH v3 03/19] target/arm: Restrict DC-CVAP instruction to TCG accel

[Philippe Mathieu-Daudé]

On 3/16/20 9:11 PM, Peter Maydell wrote:
> On Mon, 16 Mar 2020 at 19:36, Richard Henderson
> <address@hidden> wrote:
> > I'm not 100% sure how the system regs function under kvm.
> >
> > If they are not used at all, then we should avoid them all en masse an not
> > piecemeal like this.
> >
> > If they are used for something, then we should keep them registered and change
> > the writefn like so:
> >
> > #ifdef CONFIG_TCG
> >      /* existing stuff */
> > #else
> >      /* Handled by hardware accelerator. */
> >      g_assert_not_reached();
> > #endif

I ended with that patch because dccvap_writefn() calls probe_read() 
which is an inlined call to probe_access(), which itself is only defined 
when using TCG. So with KVM either linking fails or I get:

target/arm/helper.c: In function ‘dccvap_writefn’:
target/arm/helper.c:6898:13: error: implicit declaration of function 
‘probe_read’;
     haddr = probe_read(env, vaddr, dline_size, mem_idx, GETPC());
             ^~~~~~~~~~

I'll use your suggestion which works for me:

-- >8 --
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -330,8 +330,20 @@ static inline void 
tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu,
 {
 }
 #endif
+
+#ifdef CONFIG_TCG
 void *probe_access(CPUArchState *env, target_ulong addr, int size,
                    MMUAccessType access_type, int mmu_idx, uintptr_t 
retaddr);
+#else
+static inline void *probe_access(CPUArchState *env,
+                                 target_ulong addr, int size,
+                                 MMUAccessType access_type,
+                                 int mmu_idx, uintptr_t retaddr)
+{
+     /* Handled by hardware accelerator. */
+     g_assert_not_reached();
+}
+#endif /* CONFIG_TCG */

 static inline void *probe_write(CPUArchState *env, target_ulong addr, 
int size,
                                 int mmu_idx, uintptr_t retaddr)
---

> (1) for those registers where we need to know the value within
> QEMU code (notably anything involved in VA-to-PA translation,
> as this is used by gdbstub accesses, etc, but sometimes we
> want other register values too): the sysreg struct is
> what lets us map from the KVM register to the field in the
> CPU struct when we do a sync of data to/from the kernel.
>
> (2) for other registers, the sync lets us make the register
> visible as an r/o register in the gdbstub. (this is not
> very important, but it's nice)
>
> (3) Either way, the sync works via the raw_read/raw_write
> accessors (this is a big part of what they're for), which are
> supposed to just stuff the data into/out of the underlying
> CPU struct field. (But watch out because we fall back to
> using the non-raw read/writefn if there's no raw version
> provided for a particular register.) If a regdef is marked
> as NO_RAW then it means there is no raw access and we don't
> sync the value.
>
> (4) I think that in KVM mode we won't deliberately do
> non-raw accesses, and a quick grep through of the places
> that do 'readfn' accesses supports that.
>
> thanks
> -- PMM