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