* [PATCH v2] Fix SEGFAULT on getting physical address of MMIO region.
@ 2023-08-03 10:58 Mikhail Tyutin
2023-08-03 14:57 ` Richard Henderson
2023-08-03 15:15 ` Peter Maydell
0 siblings, 2 replies; 4+ messages in thread
From: Mikhail Tyutin @ 2023-08-03 10:58 UTC (permalink / raw)
To: qemu-devel
Cc: richard.henderson, pbonzini, alex.bennee, Mikhail Tyutin,
Dmitriy Solovev
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(-)
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)
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] Fix SEGFAULT on getting physical address of MMIO region.
2023-08-03 10:58 [PATCH v2] Fix SEGFAULT on getting physical address of MMIO region Mikhail Tyutin
@ 2023-08-03 14:57 ` Richard Henderson
2023-08-03 15:15 ` Peter Maydell
1 sibling, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2023-08-03 14:57 UTC (permalink / raw)
To: Mikhail Tyutin, qemu-devel; +Cc: pbonzini, alex.bennee, Dmitriy Solovev
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)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] Fix SEGFAULT on getting physical address of MMIO region.
2023-08-03 10:58 [PATCH v2] Fix SEGFAULT on getting physical address of MMIO region Mikhail Tyutin
2023-08-03 14:57 ` Richard Henderson
@ 2023-08-03 15:15 ` Peter Maydell
2023-08-03 16:07 ` Mikhail Tyutin
1 sibling, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2023-08-03 15:15 UTC (permalink / raw)
To: Mikhail Tyutin
Cc: qemu-devel, richard.henderson, pbonzini, alex.bennee, Dmitriy Solovev
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] Fix SEGFAULT on getting physical address of MMIO region.
2023-08-03 15:15 ` Peter Maydell
@ 2023-08-03 16:07 ` Mikhail Tyutin
0 siblings, 0 replies; 4+ messages in thread
From: Mikhail Tyutin @ 2023-08-03 16:07 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, richard.henderson, pbonzini, alex.bennee, Dmitriy Solovev
> > 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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-03 16:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-03 10:58 [PATCH v2] Fix SEGFAULT on getting physical address of MMIO region Mikhail Tyutin
2023-08-03 14:57 ` Richard Henderson
2023-08-03 15:15 ` Peter Maydell
2023-08-03 16:07 ` Mikhail Tyutin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.