About the fence inst in second_stage_only_translation

[cebarobot]

Hi, I find there is something strange in second_stage_only_translation:

riscv-hyp-tests/translation_tests.c

Lines 130 to 141 in 98701e3

	goto_priv(PRIV_VS);
	TEST_SETUP_EXCEPT();
	bool check1 = read64(vaddr1) == 0x11;
	bool check2 = read64(vaddr2) == 0x22;
	TEST_ASSERT("vs gets right values", excpt.triggered == false && check1 && check2);
	hpt_switch();
	sfence();
	TEST_SETUP_EXCEPT();
	check1 = read64(vaddr1) == 0x22;
	check2 = read64(vaddr2) == 0x11;
	TEST_ASSERT("vs gets right values after changing pt", excpt.triggered == false && check1 && check2);

In this section of code, the program switches the G-stage (2nd stage) page table by hpt_switch() in VS-mode, then tries to flush TLB using sfence.vma by sfence().

Accodring to the latest risc-v priv spec, sfence.vma could not affect 2nd-stage address translation. I think hfence.gvma should be used here.

Section: Hypervisor Memory-Management Fence Instructions
Instruction SFENCE.VMA applies only to the memory-management data structures controlled by the current satp (either the HS-level satp when V=0 or vsatp when V=1).

Also, Switching the G-stage table in VS-mode is really unusual. It should be done in HS-mode.

I think line 136-137 of codes should be modified like below:

 goto_priv(PRIV_HS); 
 hpt_switch(); 
 hfence_gvma(); 
 goto_priv(PRIV_VS);

I'm not sure if my analysis is correct. If correct, I could open an PR to fix this problem.

Thanks a lot.

[defermelowie]

I agree on this. The spec. states that:

The SFENCE.VMA instruction orders stores only to the VS-level address-translation structures with subsequest VS-stage address translations for the same virtual machine...
~ Priv. Arch. Sec. 18.5.3

Thus, an update of G-stage address-translation structures requires HFENCE.GVMA to make sure the ordering is correct. HFENCE.GVMA is illegal when executed from a virtualized mode (causes virtual instruction exception), therefore the switch to HS-mode you proposed is necessary:

--- a/translation_tests.c
+++ b/translation_tests.c
@@ -138,8 +138,10 @@ bool second_stage_only_translation(){
     bool check2 = read64(vaddr2) == 0x22;
     TEST_ASSERT("vs gets right values", excpt.triggered == false && check1 && check2);
 
+    goto_priv(PRIV_HS);
     hpt_switch();
-    sfence();
+    hfence_gvma();
+    goto_priv(PRIV_VS);
     TEST_SETUP_EXCEPT();
     check1 = read64(vaddr1) == 0x22;
     check2 = read64(vaddr2) == 0x11;

In one of the other tests a similar issue is present: the G-stage address-translation structures are updated and a HFENCE.VVMA should make sure the ordering is correct. However, HFENCE.VVMA only provides ordering guarantees for VS-stage address-translation structures.
I think a HFENCE.GVMA should be executed instead:

--- a/hfence_tests.c
+++ b/hfence_tests.c
@@ -22,7 +22,7 @@ bool hfence_test() {
     cond = true;
     hpt_switch();
     cond &= hlvd(vaddr) == val;
-    hfence_vvma();
+    hfence_gvma();
     cond &= hlvd(vaddr) != val;
     hpt_switch();
     cond &= hlvd(vaddr) != val;

Please correct me if you believe I misinterpreted something.