On 3/24/2023 10:37 AM, Richard Henderson wrote: > On 3/23/23 18:20, Wu, Fei wrote: >> I lack some background here, why should tb_flags be preferred if env has >> the same info? Then for reading from tb_flags, we need to add it to >> tb_flags first. > > We read from tb_flags in translate because that proves we've added the > data to tb_flags for the TB hash. It avoids errors like the one for > vstart. > Got it.
>> Correct me if I'm wrong. The only strong reason we add some flag to >> tb_flags is that it causes codegen generate different code because of >> this flag change, and it looks like priv is not one of them, neither is >> mmu_idx, the generated code does use mmu_idx, but no different code >> generated for it. > > PRIV definitely affects the generated code: for a supervisor > instruction, such as REQUIRE_PRIV_MS, we emit gen_exception_illegal() if > PRIV == U. > You are right, another one is just mentioned semihosting_enabled() in trans_ebreak. > MMU_IDX definitely affects the generated code, because that immediate > value makes its way into the memory offsets in the softmmu lookup > sequence. Have a look at the output of -d op_opt,out_asm. > Yes, I think you mean the fast path for softmmu lookup. >> I think here we have some other reasons to include more, e.g. reference >> env can be error-prone in some way. So there are minimal flags must be >> in tb_flags, but we think it's better to add more? > > We add the ones required for efficiency of execution. > > We had not originally added PRIV, because the (original) few > instructions in trans_privileged.c.inc all call helpers anyway, so it > was easy enough to check PRIV in the helper. > > Since then more uses have grown. We *could* turn those into helper > functions as well, but every other guest arch includes the privilege > level in tb_flags, and it seems natural to do so. Only if you > completely run out of bits would I consider working hard to eliminate > that one. > It makes sense. This is very informative, I appreciate it very much. Thanks, Fei. > > r~