Re: [Qemu-ppc] [PULL 01/22] target/ppc: add external PID support

[Peter Maydell]

On 8 November 2018 at 12:16, David Gibson <address@hidden> wrote:
> From: Roman Kapl <address@hidden>
>
> External PID is a mechanism present on BookE 2.06 that enables application to
> store/load data from different address spaces. There are special version of 
> some
> instructions, which operate on alternate address space, which is specified in
> the EPLC/EPSC regiser.
>
> This implementation uses two additional MMU modes (mmu_idx) to provide the
> address space for the load and store instructions. The QEMU TLB fill code was
> modified to recognize these MMU modes and use the values in EPLC/EPSC to find
> the proper entry in he PPC TLB. These two QEMU TLBs are also flushed on each
> write to EPLC/EPSC.
>
> Following instructions are implemented: dcbfep dcbstep dcbtep dcbtstep dcbzep
> dcbzlep icbiep lbepx ldepx lfdepx lhepx lwepx stbepx stdepx stfdepx sthepx
> stwepx.
>
> Following vector instructions are not: evlddepx evstddepx lvepx lvepxl stvepx
> stvepxl.

Hi; Coverity reports an issue (CID1396864) with this function:

> +/* dcbfep (external PID dcbf) */
> +static void gen_dcbfep(DisasContext *ctx)
> +{
> +    /* XXX: specification says this is treated as a load by the MMU */
> +    TCGv t0;
> +    CHK_SV;
> +    gen_set_access_type(ctx, ACCESS_CACHE);
> +    t0 = tcg_temp_new();
> +    gen_addr_reg_index(ctx, t0);
> +    tcg_gen_qemu_ld_tl(t0, t0, PPC_TLB_EPID_LOAD, DEF_MEMOP(MO_UB));
> +    tcg_temp_free(t0);
> +}

It says that the gen_set_access_type() call is unreachable. I think
this is a false positive (the code is unreachable, but only if
CONFIG_USER_ONLY is defined). On the other hand, all the other
similar gen_* functions in this file that use CHK_SV seem to have
a pattern of

#if defined(CONFIG_USER_ONLY)
    GEN_PRIV;
#else
    TCGv t0;
    CHK_SV;
    [etc]
#endif

so maybe we should do that here too for consistency?

thanks
-- PMM