Peter Maydell <peter.mayd...@linaro.org> writes: > On Wed, 16 Jun 2021 at 13:53, Alex Bennée <alex.ben...@linaro.org> wrote: >> >> Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> writes: >> > diff --git a/exec.c b/exec.c >> > index 67e520d18e..7f4074f95e 100644 >> > --- a/exec.c >> > +++ b/exec.c >> > @@ -1019,14 +1019,13 @@ void tb_invalidate_phys_addr(AddressSpace *as, >> > hwaddr addr, MemTxAttrs attrs) >> > >> > static void breakpoint_invalidate(CPUState *cpu, target_ulong pc) >> > { >> > - MemTxAttrs attrs; >> > - hwaddr phys = cpu_get_phys_page_attrs_debug(cpu, pc, &attrs); >> > - int asidx = cpu_asidx_from_attrs(cpu, attrs); >> > - if (phys != -1) { >> > - /* Locks grabbed by tb_invalidate_phys_addr */ >> > - tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as, >> > - phys | (pc & ~TARGET_PAGE_MASK), attrs); >> > - } >> > + /* >> > + * There may not be a virtual to physical translation for the pc >> > + * right now, but there may exist cached TB for this pc. >> > + * Flush the whole TB cache to force re-translation of such TBs. >> > + * This is heavyweight, but we're debugging anyway. >> > + */ >> > + tb_flush(cpu); >> > } >> > #endif >> > >> > >> > Unfortunately my x86-fu isn't really enough to understand what the >> > solution should be in this case. >> >> It's not really an x86 issue here but that we don't have any easy way of >> finding the subset of TranslationBlock's that might be affected. We can >> only query the QHT for a head address + flags. Meanwhile when there is >> an active mapping we go through the page tables > > Could we do something where we zap the TBs here where there is an active > virtual-to-physical mapping for this PC, and also make a record of affected > PCs (or PC ranges) so that before we add a new entry to the > virtual-to-physical mapping we check the record to see if we actually need > to flush this TB? I think if you flush all the TLBs at this point then > you can do the "check before adding new entry" part in > tlb_set_page_with_attrs(), > but I'm not super familiar with the execution flow of TCG so that might be > wrong.
So in breakpoint_invalidate can we actually probe for the existence of an active mapping for a given virt<->phys entry? If there is we call the tb_invalidate_phys_addr as before, if not save the data which we check when updating the softmmu tlb. > Also there needs to be a point where we can discard entries from > our "dump this TB for this PC" records so they don't just grow indefinitely, > and I'm not sure what that would be. I wondered if there was a way to use a bloom filter for this? But there doesn't seem to be an easy way of removing entries once you've done the thing you wanted to do. I guess we could just reset when all breakpoints are cleared or we do a tb_flush() for other reasons. -- Alex Bennée