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

[Richard Henderson <richard.henderson@linaro.org>]

On 3/3/21 11:39 AM, David Hildenbrand wrote:
> Should we start wrapping that stuff into #ifdef CONFIG_TCG ?
> 
>> +    uint64_t tlb_fill_tec;   /* translation exception code during tlb_fill */
>> +    int tlb_fill_exc;        /* exception number seen during tlb_fill */

Eh, probably not.  At least not until we elide the softmmu tlb, which is 
fantastically larger.

>> +    if (unlikely(flags & TLB_INVALID_MASK)) {
>> +        return false;
> 
> ^ I recall PAGE_WRITE_INV handling where we immediately set TLB_INVALID_MASK 
> again on write access (to handle low-address protection cleanly). I suspect 
> that TLB_INVALID_MASK will be set in that case (I could be wrong, though).
> 
> What certainly would work is checking for "haddr != NULL".
> 
> /* Don't rely on TLB_INVALID_MASK - see PAGE_WRITE_INV handling. */
> if (unlikely(!haddr1)) {
>      return false;
> }

Ah, right.  I consider TLB_INVALID_MASK being set in the return from 
probe_access_flags for PAGE_WRITE_INV a bug.  I'm not sure how to fix that 
right away.

Well, !haddr1 is also false for TLB_MMIO, so you'd need to check for that as well.


r~