Message ID | 20230803105830.8722-1-m.tyutin@yadro.com |
---|---|
State | New |
Headers | show |
Series | [v2] Fix SEGFAULT on getting physical address of MMIO region. | expand |
On 8/3/23 03:58, Mikhail Tyutin wrote: > Apply save_iotlb_data() to io_readx() as well as to io_writex(). > > Signed-off-by: Dmitriy Solovev <d.solovev@yadro.com> > Signed-off-by: Mikhail Tyutin <m.tyutin@yadro.com> > --- > accel/tcg/cputlb.c | 36 +++++++++++++++++++++--------------- > 1 file changed, 21 insertions(+), 15 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ > > diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c > index ba44501a7c..addce3be38 100644 > --- a/accel/tcg/cputlb.c > +++ b/accel/tcg/cputlb.c > @@ -1363,6 +1363,21 @@ static inline void cpu_transaction_failed(CPUState *cpu, hwaddr physaddr, > } > } > > +/* > + * Save a potentially trashed CPUTLBEntryFull for later lookup by plugin. > + * This is read by tlb_plugin_lookup if the fulltlb entry doesn't match > + * because of the side effect of io_writex changing memory layout. > + */ > +static void save_iotlb_data(CPUState *cs, MemoryRegionSection *section, > + hwaddr mr_offset) > +{ > +#ifdef CONFIG_PLUGIN > + SavedIOTLB *saved = &cs->saved_iotlb; > + saved->section = section; > + saved->mr_offset = mr_offset; > +#endif > +} > + > static uint64_t io_readx(CPUArchState *env, CPUTLBEntryFull *full, > int mmu_idx, vaddr addr, uintptr_t retaddr, > MMUAccessType access_type, MemOp op) > @@ -1382,6 +1397,12 @@ static uint64_t io_readx(CPUArchState *env, CPUTLBEntryFull *full, > cpu_io_recompile(cpu, retaddr); > } > > + /* > + * The memory_region_dispatch may trigger a flush/resize > + * so for plugins we save the iotlb_data just in case. > + */ > + save_iotlb_data(cpu, section, mr_offset); > + > { > QEMU_IOTHREAD_LOCK_GUARD(); > r = memory_region_dispatch_read(mr, mr_offset, &val, op, full->attrs); > @@ -1398,21 +1419,6 @@ static uint64_t io_readx(CPUArchState *env, CPUTLBEntryFull *full, > return val; > } > > -/* > - * Save a potentially trashed CPUTLBEntryFull for later lookup by plugin. > - * This is read by tlb_plugin_lookup if the fulltlb entry doesn't match > - * because of the side effect of io_writex changing memory layout. > - */ > -static void save_iotlb_data(CPUState *cs, MemoryRegionSection *section, > - hwaddr mr_offset) > -{ > -#ifdef CONFIG_PLUGIN > - SavedIOTLB *saved = &cs->saved_iotlb; > - saved->section = section; > - saved->mr_offset = mr_offset; > -#endif > -} > - > static void io_writex(CPUArchState *env, CPUTLBEntryFull *full, > int mmu_idx, uint64_t val, vaddr addr, > uintptr_t retaddr, MemOp op)
On Thu, 3 Aug 2023 at 12:00, Mikhail Tyutin <m.tyutin@yadro.com> wrote: > > Apply save_iotlb_data() to io_readx() as well as to io_writex(). Could we have a bit more detail in the commit message about when you can get this segfault? In particular, does this happen only if you're using plugins? thanks -- PMM
> > Apply save_iotlb_data() to io_readx() as well as to io_writex(). > > Could we have a bit more detail in the commit message about > when you can get this segfault? > > In particular, does this happen only if you're using plugins? > I think so. It crashes on specific addresses when plugin calls qemu_plugin_hwaddr_phys_addr(). I haven't seen this crash without a plugin.
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index ba44501a7c..addce3be38 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -1363,6 +1363,21 @@ static inline void cpu_transaction_failed(CPUState *cpu, hwaddr physaddr, } } +/* + * Save a potentially trashed CPUTLBEntryFull for later lookup by plugin. + * This is read by tlb_plugin_lookup if the fulltlb entry doesn't match + * because of the side effect of io_writex changing memory layout. + */ +static void save_iotlb_data(CPUState *cs, MemoryRegionSection *section, + hwaddr mr_offset) +{ +#ifdef CONFIG_PLUGIN + SavedIOTLB *saved = &cs->saved_iotlb; + saved->section = section; + saved->mr_offset = mr_offset; +#endif +} + static uint64_t io_readx(CPUArchState *env, CPUTLBEntryFull *full, int mmu_idx, vaddr addr, uintptr_t retaddr, MMUAccessType access_type, MemOp op) @@ -1382,6 +1397,12 @@ static uint64_t io_readx(CPUArchState *env, CPUTLBEntryFull *full, cpu_io_recompile(cpu, retaddr); } + /* + * The memory_region_dispatch may trigger a flush/resize + * so for plugins we save the iotlb_data just in case. + */ + save_iotlb_data(cpu, section, mr_offset); + { QEMU_IOTHREAD_LOCK_GUARD(); r = memory_region_dispatch_read(mr, mr_offset, &val, op, full->attrs); @@ -1398,21 +1419,6 @@ static uint64_t io_readx(CPUArchState *env, CPUTLBEntryFull *full, return val; } -/* - * Save a potentially trashed CPUTLBEntryFull for later lookup by plugin. - * This is read by tlb_plugin_lookup if the fulltlb entry doesn't match - * because of the side effect of io_writex changing memory layout. - */ -static void save_iotlb_data(CPUState *cs, MemoryRegionSection *section, - hwaddr mr_offset) -{ -#ifdef CONFIG_PLUGIN - SavedIOTLB *saved = &cs->saved_iotlb; - saved->section = section; - saved->mr_offset = mr_offset; -#endif -} - static void io_writex(CPUArchState *env, CPUTLBEntryFull *full, int mmu_idx, uint64_t val, vaddr addr, uintptr_t retaddr, MemOp op)