josecm / riscv-hyp-tests

A bare-metal application to test specific features of the risc-v hypervisor extension
GNU General Public License v3.0
36 stars 22 forks source link

About the fence inst in second_stage_only_translation #18

Open cebarobot opened 3 months ago

cebarobot commented 3 months ago

Hi, I find there is something strange in second_stage_only_translation:

https://github.com/josecm/riscv-hyp-tests/blob/98701e3b9846d99e4df649c8f8a733e4ba4baffe/translation_tests.c#L130-L141

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 commented 3 weeks ago

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.