Re: [PATCH v5 1/2] target/s390x: Implement the MVPG condition-code-option bit

[David Hildenbrand]

On 11.03.21 18:02, Richard Henderson wrote:
> On 3/11/21 10:17 AM, David Hildenbrand wrote:
> > +#if !defined(CONFIG_USER_ONLY)
> > +    if (env->tlb_fill_exc) {
> > +        return env->tlb_fill_exc;
> > +    }
> > +#else
> > +    if (!haddr1) {
> > +        env->__excp_addr = vaddr1;
> > +        return PGM_ADDRESSING;
> > +    }
> > +#endif
>
> For user-only, we can rely on TLB_INVALID_MASK, and check once for both pages.

Then, I cannot set the proper vaddr1 vs. vaddr2 (see patch #2).


> > @@ -858,13 +925,26 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, 
> > uint64_t r1, uint64_t r2)
> >        /*
> >         * TODO:
> >         * - Access key handling
> > -     * - CC-option with surpression of page-translation exceptions
> >         * - Store r1/r2 register identifiers at real location 162
> >         */
> > -    srca = access_prepare(env, r2, TARGET_PAGE_SIZE, MMU_DATA_LOAD, mmu_idx,
> > -                          ra);
> > -    desta = access_prepare(env, r1, TARGET_PAGE_SIZE, MMU_DATA_STORE, mmu_idx,
> > -                           ra);
> > +    exc = access_prepare_nf(&srca, env, cco, r2, TARGET_PAGE_SIZE,
> > +                            MMU_DATA_LOAD, mmu_idx, ra);
> > +    if (exc) {
> > +        return 2;
> > +    }
> > +    exc = access_prepare_nf(&desta, env, cco, r1, TARGET_PAGE_SIZE,
> > +                            MMU_DATA_STORE, mmu_idx, ra);
> > +    if (exc) {
> > +#if !defined(CONFIG_USER_ONLY)
> > +        if (exc == PGM_PROTECTION) {
> > +            stq_phys(env_cpu(env)->as,
> > +                     env->psa + offsetof(LowCore, trans_exc_code),
> > +                     env->tlb_fill_tec);
> > +            tcg_s390_program_interrupt(env, PGM_PROTECTION, ra);
> > +        }
> > +#endif
> > +        return 1;
> > +    }
> >        access_memmove(env, &desta, &srca, ra);
> >        return 0; /* data moved */
> >    }
>
> If we're going to have an ifdef at all here, it should be around the entire
> helper -- this is a privileged operation.


Privileged operation (access key specified, and
selected PSW-key-mask bit is zero in the prob-
lem state)

Without an access key in GR0, we're using the PSW key - which should 
always work, no?

What am I missing?

--
Thanks,

David / dhildenb