Re: [PATCH v6 00/25] target/riscv: MSTATUS_SUM + cleanups

[Richard Henderson]

On 3/25/23 03:54, Richard Henderson wrote:
> This builds on Fei and Zhiwei's SUM and TB_FLAGS changes.
>
>    * Reclaim 5 TB_FLAGS bits, since we nearly ran out.
>
>    * Using cpu_mmu_index(env, true) is insufficient to implement
>      HLVX properly.  While that chooses the correct mmu_idx, it
>      does not perform the read with execute permission.
>      I add a new tcg interface to perform a read-for-execute with
>      an arbitrary mmu_idx.  This is still not 100% compliant, but
>      it's closer.
>
>    * Handle mstatus.MPV in cpu_mmu_index.
>    * Use vsstatus.SUM when required for MMUIdx_S_SUM.
>    * Cleanups for get_physical_address.
>
> While this passes check-avocado, I'm sure that's insufficient.
> Please have a close look.

Somewhere after either patch 16 or 17, when env->virt is considered in riscv_cpu_mmu_index 
and a few other bugs are fixed, we can do

--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -591,11 +591,6 @@ void riscv_cpu_set_virt_enabled
         return;
     }

-    /* Flush the TLB on all virt mode changes. */
-    if (get_field(env->virt, VIRT_ONOFF) != enable) {
-        tlb_flush(env_cpu(env));
-    }
-
     env->virt = set_field(env->virt, VIRT_ONOFF, enable);

     if (enable) {
-- %< --

Because we're no longer trying to overlap the VS and HS tlbs on the same mmuidx.

I have been pondering additional changes that would avoid flushing for MXR changes, so 
that user-lookups from M-mode firmware gets the same advantage as has just been done for 
SUM.  But this is complicated by needing 12 (!) more mmuidx, which cannot currently be 
represented, nor does it even seem like the best idea.


r~