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

[Peter Maydell]

On Fri, 17 Apr 2020 at 14:49, Philippe Mathieu-Daudé <address@hidden> wrote:
>
> 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());
>               ^~~~~~~~~~

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