[v1,04/13] cputlb: ensure we save the IOTLB data in case of reset

Message ID 20200709141327.14631-5-alex.bennee@linaro.org
State New
Headers show
Series
  • misc rc0 fixes (docs, plugins, docker)
Related show

Commit Message

Alex Bennée July 9, 2020, 2:13 p.m.
Any write to a device might cause a re-arrangement of memory
triggering a TLB flush and potential re-size of the TLB invalidating
previous entries. This would cause users of qemu_plugin_get_hwaddr()
to see the warning:

  invalid use of qemu_plugin_get_hwaddr

because of the failed tlb_lookup which should always succeed. To
prevent this we save the IOTLB data in case it is later needed by a
plugin doing a lookup.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>


---
v2
  - save the entry instead of re-running the tlb_fill.
v3
  - don't abuse TLS, use CPUState to store data
  - just use g_free_rcu() to avoid ugliness
  - verify addr matches before returning data
  - ws fix
---
 include/hw/core/cpu.h   |  4 +++
 include/qemu/typedefs.h |  1 +
 accel/tcg/cputlb.c      | 57 +++++++++++++++++++++++++++++++++++++++--
 3 files changed, 60 insertions(+), 2 deletions(-)

-- 
2.20.1

Comments

Richard Henderson July 10, 2020, 9:03 p.m. | #1
On 7/9/20 7:13 AM, Alex Bennée wrote:
> Any write to a device might cause a re-arrangement of memory

> triggering a TLB flush and potential re-size of the TLB invalidating

> previous entries. This would cause users of qemu_plugin_get_hwaddr()

> to see the warning:

> 

>   invalid use of qemu_plugin_get_hwaddr

> 

> because of the failed tlb_lookup which should always succeed. To

> prevent this we save the IOTLB data in case it is later needed by a

> plugin doing a lookup.

> 

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> 

> ---

> v2

>   - save the entry instead of re-running the tlb_fill.

> v3

>   - don't abuse TLS, use CPUState to store data

>   - just use g_free_rcu() to avoid ugliness

>   - verify addr matches before returning data

>   - ws fix

> ---

>  include/hw/core/cpu.h   |  4 +++

>  include/qemu/typedefs.h |  1 +

>  accel/tcg/cputlb.c      | 57 +++++++++++++++++++++++++++++++++++++++--

>  3 files changed, 60 insertions(+), 2 deletions(-)

> 

> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h

> index b3f4b7931823..bedbf098dc57 100644

> --- a/include/hw/core/cpu.h

> +++ b/include/hw/core/cpu.h

> @@ -417,7 +417,11 @@ struct CPUState {

>  

>      DECLARE_BITMAP(plugin_mask, QEMU_PLUGIN_EV_MAX);

>  

> +#ifdef CONFIG_PLUGIN

>      GArray *plugin_mem_cbs;

> +    /* saved iotlb data from io_writex */

> +    SavedIOTLB *saved_iotlb;

> +#endif

>  

>      /* TODO Move common fields from CPUArchState here. */

>      int cpu_index;

> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h

> index 15f5047bf1dc..427027a9707a 100644

> --- a/include/qemu/typedefs.h

> +++ b/include/qemu/typedefs.h

> @@ -116,6 +116,7 @@ typedef struct QObject QObject;

>  typedef struct QString QString;

>  typedef struct RAMBlock RAMBlock;

>  typedef struct Range Range;

> +typedef struct SavedIOTLB SavedIOTLB;

>  typedef struct SHPCDevice SHPCDevice;

>  typedef struct SSIBus SSIBus;

>  typedef struct VirtIODevice VirtIODevice;

> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c

> index 1e815357c709..8636b66e036a 100644

> --- a/accel/tcg/cputlb.c

> +++ b/accel/tcg/cputlb.c

> @@ -1073,6 +1073,42 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>      return val;

>  }

>  

> +#ifdef CONFIG_PLUGIN

> +

> +typedef struct SavedIOTLB {

> +    struct rcu_head rcu;

> +    hwaddr addr;

> +    MemoryRegionSection *section;

> +    hwaddr mr_offset;

> +} SavedIOTLB;

> +

> +/*

> + * Save a potentially trashed IOTLB entry for later lookup by plugin.

> + *

> + * We also need to track the thread storage address because the RCU

> + * cleanup that runs when we leave the critical region (the current

> + * execution) is actually in a different thread.

> + */

> +static void save_iotlb_data(CPUState *cs, hwaddr addr, MemoryRegionSection *section, hwaddr mr_offset)


Overlong line.

> +{

> +    SavedIOTLB *old, *new = g_new(SavedIOTLB, 1);

> +    new->addr = addr;

> +    new->section = section;

> +    new->mr_offset = mr_offset;

> +    old = atomic_rcu_read(&cs->saved_iotlb);

> +    atomic_rcu_set(&cs->saved_iotlb, new);

> +    if (old) {

> +        g_free_rcu(old, rcu);

> +    }

> +}


I'm a bit confused by this.  Why all the multiple allocation?  How many
consumers are you expecting, and more are you expecting multiple memory
operations in flight at once?

If multiple memory operations in flight, then why aren't we chaining them
together, so that you can search through multiple alternatives.

If only one memory operation in flight, why are you allocating memory at all,
much less managing it with rcu?  Just put one structure (or a collection of
fields) into CPUState and be done.

> +

> +#else

> +static void save_iotlb_data(CPUState *cs, hwaddr addr, MemoryRegionSection *section, hwaddr mr_offset)

> +{

> +    /* do nothing */

> +}

> +#endif


Surely better to move the ifdef inside the function so that you don't have to
replicate the definition?

> +        SavedIOTLB *saved = atomic_rcu_read(&cpu->saved_iotlb);

> +        if (saved && saved->addr == tlb_addr) {

> +            data->is_io = true;

> +            data->v.io.section = saved->section;

> +            data->v.io.offset = saved->mr_offset;

> +            return true;

> +        }


Should that test in fact be an assert?  Why would this fail?


r~
Emilio G. Cota July 11, 2020, 9:10 p.m. | #2
On Thu, Jul 09, 2020 at 15:13:18 +0100, Alex Bennée wrote:
> Any write to a device might cause a re-arrangement of memory

> triggering a TLB flush and potential re-size of the TLB invalidating

> previous entries. This would cause users of qemu_plugin_get_hwaddr()

> to see the warning:

> 

>   invalid use of qemu_plugin_get_hwaddr

> 

> because of the failed tlb_lookup which should always succeed. To

> prevent this we save the IOTLB data in case it is later needed by a

> plugin doing a lookup.

> 

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>


Reviewed-by: Emilio G. Cota <cota@braap.org>


Some minor comments below.


> --- a/accel/tcg/cputlb.c

> +++ b/accel/tcg/cputlb.c

> @@ -1073,6 +1073,42 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>      return val;

>  }

>  

> +#ifdef CONFIG_PLUGIN

> +

> +typedef struct SavedIOTLB {

> +    struct rcu_head rcu;

> +    hwaddr addr;

> +    MemoryRegionSection *section;

> +    hwaddr mr_offset;

> +} SavedIOTLB;

> +

> +/*

> + * Save a potentially trashed IOTLB entry for later lookup by plugin.

> + *

> + * We also need to track the thread storage address because the RCU

> + * cleanup that runs when we leave the critical region (the current

> + * execution) is actually in a different thread.


Mentioning the thread storage is now outdated -- I think this comment
(starting from 'We') can be removed.

> + */

> +static void save_iotlb_data(CPUState *cs, hwaddr addr, MemoryRegionSection *section, hwaddr mr_offset)

> +{

> +    SavedIOTLB *old, *new = g_new(SavedIOTLB, 1);

> +    new->addr = addr;

> +    new->section = section;

> +    new->mr_offset = mr_offset;

> +    old = atomic_rcu_read(&cs->saved_iotlb);

> +    atomic_rcu_set(&cs->saved_iotlb, new);

> +    if (old) {

> +        g_free_rcu(old, rcu);

> +    }


Using atomic_rcu_read here is not necessary (only this thread ever writes
to this field) and might confuse a reader when trying to find the
atomic_rcu_read that matches the atomic_rcu_set (that read is in
tlb_plugin_lookup).

Consider doing
	old = cs->saved_iotlb;
instead.

Thanks,
		Emilio
Emilio G. Cota July 11, 2020, 9:30 p.m. | #3
On Fri, Jul 10, 2020 at 14:03:27 -0700, Richard Henderson wrote:
> On 7/9/20 7:13 AM, Alex Bennée wrote:

> > Any write to a device might cause a re-arrangement of memory

> > triggering a TLB flush and potential re-size of the TLB invalidating

> > previous entries. This would cause users of qemu_plugin_get_hwaddr()

> > to see the warning:

> > 

> >   invalid use of qemu_plugin_get_hwaddr

> > 

> > because of the failed tlb_lookup which should always succeed. To

> > prevent this we save the IOTLB data in case it is later needed by a

> > plugin doing a lookup.

> > 

> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> > 

> > ---

> > v2

> >   - save the entry instead of re-running the tlb_fill.

> > v3

> >   - don't abuse TLS, use CPUState to store data

> >   - just use g_free_rcu() to avoid ugliness

> >   - verify addr matches before returning data

> >   - ws fix

> > ---

> >  include/hw/core/cpu.h   |  4 +++

> >  include/qemu/typedefs.h |  1 +

> >  accel/tcg/cputlb.c      | 57 +++++++++++++++++++++++++++++++++++++++--

> >  3 files changed, 60 insertions(+), 2 deletions(-)

> > 

> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h

> > index b3f4b7931823..bedbf098dc57 100644

> > --- a/include/hw/core/cpu.h

> > +++ b/include/hw/core/cpu.h

> > @@ -417,7 +417,11 @@ struct CPUState {

> >  

> >      DECLARE_BITMAP(plugin_mask, QEMU_PLUGIN_EV_MAX);

> >  

> > +#ifdef CONFIG_PLUGIN

> >      GArray *plugin_mem_cbs;

> > +    /* saved iotlb data from io_writex */

> > +    SavedIOTLB *saved_iotlb;

> > +#endif

> >  

> >      /* TODO Move common fields from CPUArchState here. */

> >      int cpu_index;

> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h

> > index 15f5047bf1dc..427027a9707a 100644

> > --- a/include/qemu/typedefs.h

> > +++ b/include/qemu/typedefs.h

> > @@ -116,6 +116,7 @@ typedef struct QObject QObject;

> >  typedef struct QString QString;

> >  typedef struct RAMBlock RAMBlock;

> >  typedef struct Range Range;

> > +typedef struct SavedIOTLB SavedIOTLB;

> >  typedef struct SHPCDevice SHPCDevice;

> >  typedef struct SSIBus SSIBus;

> >  typedef struct VirtIODevice VirtIODevice;

> > diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c

> > index 1e815357c709..8636b66e036a 100644

> > --- a/accel/tcg/cputlb.c

> > +++ b/accel/tcg/cputlb.c

> > @@ -1073,6 +1073,42 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

> >      return val;

> >  }

> >  

> > +#ifdef CONFIG_PLUGIN

> > +

> > +typedef struct SavedIOTLB {

> > +    struct rcu_head rcu;

> > +    hwaddr addr;

> > +    MemoryRegionSection *section;

> > +    hwaddr mr_offset;

> > +} SavedIOTLB;

> > +

> > +/*

> > + * Save a potentially trashed IOTLB entry for later lookup by plugin.

> > + *

> > + * We also need to track the thread storage address because the RCU

> > + * cleanup that runs when we leave the critical region (the current

> > + * execution) is actually in a different thread.

> > + */

> > +static void save_iotlb_data(CPUState *cs, hwaddr addr, MemoryRegionSection *section, hwaddr mr_offset)

> 

> Overlong line.

> 

> > +{

> > +    SavedIOTLB *old, *new = g_new(SavedIOTLB, 1);

> > +    new->addr = addr;

> > +    new->section = section;

> > +    new->mr_offset = mr_offset;

> > +    old = atomic_rcu_read(&cs->saved_iotlb);

> > +    atomic_rcu_set(&cs->saved_iotlb, new);

> > +    if (old) {

> > +        g_free_rcu(old, rcu);

> > +    }

> > +}

> 

> I'm a bit confused by this.  Why all the multiple allocation?  How many

> consumers are you expecting, and more are you expecting multiple memory

> operations in flight at once?

> 

> If multiple memory operations in flight, then why aren't we chaining them

> together, so that you can search through multiple alternatives.

> 

> If only one memory operation in flight, why are you allocating memory at all,

> much less managing it with rcu?  Just put one structure (or a collection of

> fields) into CPUState and be done.


Oh I just saw this reply. I subscribe all of the above, please shelve my R-b
tag until these are resolved.

An alternative is to emit the hwaddr directly in the mem_cb -- IIRC this is
how I did it originally. The API is a larger/uglier (plugins can subscribe
to either hwaddr or vaddr callbacks) but there is no state to keep and
no overhead of calling several functions in a hot path.

Thanks,
		E.
Alex Bennée July 12, 2020, 9:58 a.m. | #4
Emilio G. Cota <cota@braap.org> writes:

> On Fri, Jul 10, 2020 at 14:03:27 -0700, Richard Henderson wrote:

>> On 7/9/20 7:13 AM, Alex Bennée wrote:

>> > Any write to a device might cause a re-arrangement of memory

>> > triggering a TLB flush and potential re-size of the TLB invalidating

>> > previous entries. This would cause users of qemu_plugin_get_hwaddr()

>> > to see the warning:

>> > 

>> >   invalid use of qemu_plugin_get_hwaddr

>> > 

>> > because of the failed tlb_lookup which should always succeed. To

>> > prevent this we save the IOTLB data in case it is later needed by a

>> > plugin doing a lookup.

>> > 

>> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

>> > 

>> > ---

>> > v2

>> >   - save the entry instead of re-running the tlb_fill.

>> > v3

>> >   - don't abuse TLS, use CPUState to store data

>> >   - just use g_free_rcu() to avoid ugliness

>> >   - verify addr matches before returning data

>> >   - ws fix

>> > ---

>> >  include/hw/core/cpu.h   |  4 +++

>> >  include/qemu/typedefs.h |  1 +

>> >  accel/tcg/cputlb.c      | 57 +++++++++++++++++++++++++++++++++++++++--

>> >  3 files changed, 60 insertions(+), 2 deletions(-)

>> > 

>> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h

>> > index b3f4b7931823..bedbf098dc57 100644

>> > --- a/include/hw/core/cpu.h

>> > +++ b/include/hw/core/cpu.h

>> > @@ -417,7 +417,11 @@ struct CPUState {

>> >  

>> >      DECLARE_BITMAP(plugin_mask, QEMU_PLUGIN_EV_MAX);

>> >  

>> > +#ifdef CONFIG_PLUGIN

>> >      GArray *plugin_mem_cbs;

>> > +    /* saved iotlb data from io_writex */

>> > +    SavedIOTLB *saved_iotlb;

>> > +#endif

>> >  

>> >      /* TODO Move common fields from CPUArchState here. */

>> >      int cpu_index;

>> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h

>> > index 15f5047bf1dc..427027a9707a 100644

>> > --- a/include/qemu/typedefs.h

>> > +++ b/include/qemu/typedefs.h

>> > @@ -116,6 +116,7 @@ typedef struct QObject QObject;

>> >  typedef struct QString QString;

>> >  typedef struct RAMBlock RAMBlock;

>> >  typedef struct Range Range;

>> > +typedef struct SavedIOTLB SavedIOTLB;

>> >  typedef struct SHPCDevice SHPCDevice;

>> >  typedef struct SSIBus SSIBus;

>> >  typedef struct VirtIODevice VirtIODevice;

>> > diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c

>> > index 1e815357c709..8636b66e036a 100644

>> > --- a/accel/tcg/cputlb.c

>> > +++ b/accel/tcg/cputlb.c

>> > @@ -1073,6 +1073,42 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>> >      return val;

>> >  }

>> >  

>> > +#ifdef CONFIG_PLUGIN

>> > +

>> > +typedef struct SavedIOTLB {

>> > +    struct rcu_head rcu;

>> > +    hwaddr addr;

>> > +    MemoryRegionSection *section;

>> > +    hwaddr mr_offset;

>> > +} SavedIOTLB;

>> > +

>> > +/*

>> > + * Save a potentially trashed IOTLB entry for later lookup by plugin.

>> > + *

>> > + * We also need to track the thread storage address because the RCU

>> > + * cleanup that runs when we leave the critical region (the current

>> > + * execution) is actually in a different thread.

>> > + */

>> > +static void save_iotlb_data(CPUState *cs, hwaddr addr, MemoryRegionSection *section, hwaddr mr_offset)

>> 

>> Overlong line.

>> 

>> > +{

>> > +    SavedIOTLB *old, *new = g_new(SavedIOTLB, 1);

>> > +    new->addr = addr;

>> > +    new->section = section;

>> > +    new->mr_offset = mr_offset;

>> > +    old = atomic_rcu_read(&cs->saved_iotlb);

>> > +    atomic_rcu_set(&cs->saved_iotlb, new);

>> > +    if (old) {

>> > +        g_free_rcu(old, rcu);

>> > +    }

>> > +}

>> 

>> I'm a bit confused by this.  Why all the multiple allocation?  How many

>> consumers are you expecting, and more are you expecting multiple memory

>> operations in flight at once?

>> 

>> If multiple memory operations in flight, then why aren't we chaining them

>> together, so that you can search through multiple alternatives.

>> 

>> If only one memory operation in flight, why are you allocating memory at all,

>> much less managing it with rcu?  Just put one structure (or a collection of

>> fields) into CPUState and be done.

>

> Oh I just saw this reply. I subscribe all of the above, please shelve my R-b

> tag until these are resolved.


I was just conscious the data is not always valid - clearing it up with
RCU at least ensured it went away eventually. However we could certainly
just park it in the general CPUState and assert the match on the fall
through case of tlb_plugin_lookup.

My only worry is do we ever see us storing data that is valid but the
re-filled data in the real TLB could still match and be incorrect?

> An alternative is to emit the hwaddr directly in the mem_cb -- IIRC this is

> how I did it originally. The API is a larger/uglier (plugins can subscribe

> to either hwaddr or vaddr callbacks) but there is no state to keep and

> no overhead of calling several functions in a hot path.


I think that is certainly an option for evolving the API but I'd rather
just get this fix in for now while we ponder what else will be in v2 of
the API.

So far my proposal for qemu_plugin_hwaddr_device_name has some mild push
back from Peter which I'd like to resolve first:

  Date: Wed, 3 Jun 2020 16:48:33 +0100
  Message-ID: <CAFEAcA-kPZoumxfLgjxPfCPDmPgsAFCjB-zdicsiGeqSOPOH7Q@mail.gmail.com>

So far I've been adding stuff to solve any particular problems I've had
and I'd like to see what other API/hooks are being proposed so we don't
have browser style version ticks ;-)

>

> Thanks,

> 		E.



-- 
Alex Bennée

Patch

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index b3f4b7931823..bedbf098dc57 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -417,7 +417,11 @@  struct CPUState {
 
     DECLARE_BITMAP(plugin_mask, QEMU_PLUGIN_EV_MAX);
 
+#ifdef CONFIG_PLUGIN
     GArray *plugin_mem_cbs;
+    /* saved iotlb data from io_writex */
+    SavedIOTLB *saved_iotlb;
+#endif
 
     /* TODO Move common fields from CPUArchState here. */
     int cpu_index;
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 15f5047bf1dc..427027a9707a 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -116,6 +116,7 @@  typedef struct QObject QObject;
 typedef struct QString QString;
 typedef struct RAMBlock RAMBlock;
 typedef struct Range Range;
+typedef struct SavedIOTLB SavedIOTLB;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct SSIBus SSIBus;
 typedef struct VirtIODevice VirtIODevice;
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 1e815357c709..8636b66e036a 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1073,6 +1073,42 @@  static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
     return val;
 }
 
+#ifdef CONFIG_PLUGIN
+
+typedef struct SavedIOTLB {
+    struct rcu_head rcu;
+    hwaddr addr;
+    MemoryRegionSection *section;
+    hwaddr mr_offset;
+} SavedIOTLB;
+
+/*
+ * Save a potentially trashed IOTLB entry for later lookup by plugin.
+ *
+ * We also need to track the thread storage address because the RCU
+ * cleanup that runs when we leave the critical region (the current
+ * execution) is actually in a different thread.
+ */
+static void save_iotlb_data(CPUState *cs, hwaddr addr, MemoryRegionSection *section, hwaddr mr_offset)
+{
+    SavedIOTLB *old, *new = g_new(SavedIOTLB, 1);
+    new->addr = addr;
+    new->section = section;
+    new->mr_offset = mr_offset;
+    old = atomic_rcu_read(&cs->saved_iotlb);
+    atomic_rcu_set(&cs->saved_iotlb, new);
+    if (old) {
+        g_free_rcu(old, rcu);
+    }
+}
+
+#else
+static void save_iotlb_data(CPUState *cs, hwaddr addr, MemoryRegionSection *section, hwaddr mr_offset)
+{
+    /* do nothing */
+}
+#endif
+
 static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
                       int mmu_idx, uint64_t val, target_ulong addr,
                       uintptr_t retaddr, MemOp op)
@@ -1092,6 +1128,12 @@  static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
     }
     cpu->mem_io_pc = 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, iotlbentry->addr, section, mr_offset);
+
     if (mr->global_locking && !qemu_mutex_iothread_locked()) {
         qemu_mutex_lock_iothread();
         locked = true;
@@ -1381,8 +1423,11 @@  void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
  * in the softmmu lookup code (or helper). We don't handle re-fills or
  * checking the victim table. This is purely informational.
  *
- * This should never fail as the memory access being instrumented
- * should have just filled the TLB.
+ * This almost never fails as the memory access being instrumented
+ * should have just filled the TLB. The one corner case is io_writex
+ * which can cause TLB flushes and potential resizing of the TLBs
+ * loosing the information we need. In those cases we need to recover
+ * data from a copy of the io_tlb entry.
  */
 
 bool tlb_plugin_lookup(CPUState *cpu, target_ulong addr, int mmu_idx,
@@ -1406,6 +1451,14 @@  bool tlb_plugin_lookup(CPUState *cpu, target_ulong addr, int mmu_idx,
             data->v.ram.hostaddr = addr + tlbe->addend;
         }
         return true;
+    } else {
+        SavedIOTLB *saved = atomic_rcu_read(&cpu->saved_iotlb);
+        if (saved && saved->addr == tlb_addr) {
+            data->is_io = true;
+            data->v.io.section = saved->section;
+            data->v.io.offset = saved->mr_offset;
+            return true;
+        }
     }
     return false;
 }