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. 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. thanks -- PMM