On Fri, 17 Apr 2020 at 14:49, Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> wrote: > > On 3/16/20 9:11 PM, Peter Maydell wrote: > > On Mon, 16 Mar 2020 at 19:36, Richard Henderson > > <richard.henderson@xxxxxxxxxx> 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()); > ^~~~~~~~~~ IN this particular case, DC CVAP is really a system insn rather than a 'register'; our register struct for it is marked up as ARM_CP_NO_RAW, which means we'll effectively ignore it when running KVM (it will not be migrated, have its state synced against the kernel, or be visible in gdb). If dccvap_writefn() ever gets called somehow that's a bug, so having it end up with an assert is the right thing. > I'll use your suggestion which works for me: Your suggested patch isn't quite the same as RTH's suggestion, because it puts the assert inside a stub probe_read() implementation rather than having the ifdef at the level of the writefn body. I have no opinion on whether one or the other of these is preferable. thanks -- PMM
