Re: [PATCH 03/11] RISC-V: Adding T-Head SYNC instructions

[LIU Zhiwei]

On 2022/9/8 15:29, Richard Henderson wrote:
> On 9/6/22 13:22, Christoph Muellner wrote:
> > +NOP_PRIVCHECK(th_sfence_vmas, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_sync, REQUIRE_PRIV_MHSU)
> > +NOP_PRIVCHECK(th_sync_i, REQUIRE_PRIV_MHSU)
> > +NOP_PRIVCHECK(th_sync_is, REQUIRE_PRIV_MHSU)
> > +NOP_PRIVCHECK(th_sync_s, REQUIRE_PRIV_MHSU)
>
> These should not be nops: th_sfence_vmas requires a tlb flush; 
> th.sync{,.i} needs to end the current translation block; 
> th.sync.{s,is} needs multiprocessor sync, which involves a call to 
> async_safe_run_on_cpu.
Hi Richard,

I have fixed the description of T-Head custom synchronization 
instructions according to the implementation of hardware. Sorry for the 
misleading.

https://github.com/T-head-Semi/thead-extension-spec/tree/master/xtheadsync


The fix is as below:

1）th.sync.s has the same function with th.sync.

2) th.sync.is has the same function with th.sync.i

3) th.sync has the function of memory barrier, but it is stricter than 
RISC-V fence instruction as it order all the instructions instead of 
load/store instructions.

4) th.sync.i has the function to flush the pipeline besides the function 
of th.sync.


On QEMU,  I think they should be emulated them as below:

1) th.sync should be emulated as " 'tcg_gen_mb()'  and  'end the current 
translation block'" on QEMU.

2) th.sync.i should be emulated as " 'tcg_gen_mb()'  and  'end the 
current translation block'" on QEMU because we don't have the cache 
model for guest on QEMU. Thus we don't have to synchronize between the 
icache and dcache for guest.


'tcg_gen_mb' is for the function of memory barrier. And  'end the 
current translation block' is to reflect the influence of other 
instructions, such as taking interrupts which happens only at the end of 
a translation block.
Maybe we can also just implement these instructions as 'tcg_gen_mb' 
because currently all CSR instructions which may influence the 
interrupts taking have ended the TB on QEMU.


Is it right?

Thanks,
Zhiwei
> r~