On 2/1/21 6:59 AM, Liren Wei wrote: > On 2/1/21 7:01 AM, Richard Henderson wrote: >> Yes, this is a bug, because we are trying to support e.g. x86 which does not >> require an icache flush. > > That is too bad :( > > I know nothing about the modern hardware, it's really hard to imagine what > is done in CPU to maintain the coherence when this kind of cross-modifying > scenario happens.
Indeed. Complicated cache shootdown stuff, no doubt. It also helps that the cpu is decoding one instruction at a time, whereas qemu is decoding many instructions. >> I think the page lock, the TLB_NOTDIRTY setting, and a possible sync on the >> setting, needs to happen before the bytes are read during translation. >> Otherwise we don't catch the case above, nor do we catch >> >> CPU1 CPU2 >> ------------------ -------------------------- >> TLB check -> fast >> tb_gen_code() -> all of it >> write to ram >> >> Also because of x86 (and other architectures in which a single instruction >> can >> span a page boundary), I think this lock+set+sync sequence needs to happen on >> demand in something called from the function set defined in >> include/exec/translator.h >> >> That also means that any target/cpu/ which has not been converted to use that >> interface remains broken, and should be converted or deprecated. > > I failed to figure out what do you mean by lock+set+sync, I used "lock+set+sync" as shorthand for the sequence I described in the first paragraph above. > in particular: > - What is the use of the page lock here (Is this the lock of PageDesc?) Yes, this would be the PageDesc lock. It would prevent TLB_NOTDIRTY from being removed by the slow-path write until any translation is complete. > - Is the "possible sync" means some kind of wait so that TLB_NOTDIRTY is > definitely able to catch further "write to ram"? That's what the sync is for -- to make sure that no other cpu can have seen a dirty page and not be finished with the write. > Therefore maybe we can mark the RAM backing page in QEMU's page table as > non-writable at an early stage in tb_gen_code() using the ability of the > underlying OS... It's very expensive to change page tables. It's also complicated to capture and re-start with signal handlers. We do that sort of thing for qemu-linux-user, where there *is* no slow path and we have no other option. I think it would be much easier to reason with locks. r~
