On Thu, Nov 05, 2015 at 06:15:50PM +0000, Peter Maydell wrote: > The io_mem_watch MemoryRegion's read and write callbacks pass the > accesses through to an underlying address space. Now that that > might be something other than address_space_memory, we need to > pass the correct AddressSpace in via the opaque pointer. That > means we need to have one io_mem_watch MemoryRegion per address > space, rather than sharing one between all address spaces. > > This should also fix gdbstub watchpoints in x86 SMRAM, which would > previously not have worked correctly.
Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com> > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > exec.c | 40 +++++++++++++++++++++++----------------- > include/exec/memory.h | 2 ++ > 2 files changed, 25 insertions(+), 17 deletions(-) > > diff --git a/exec.c b/exec.c > index bc6ab8a..9998fa0 100644 > --- a/exec.c > +++ b/exec.c > @@ -163,8 +163,6 @@ static void io_mem_init(void); > static void memory_map_init(void); > static void tcg_commit(MemoryListener *listener); > > -static MemoryRegion io_mem_watch; > - > /** > * CPUAddressSpace: all the information a CPU needs about an AddressSpace > * @cpu: the CPU whose AddressSpace this is > @@ -334,12 +332,6 @@ static MemoryRegionSection *phys_page_find(PhysPageEntry > lp, hwaddr addr, > } > } > > -bool memory_region_is_unassigned(MemoryRegion *mr) > -{ > - return mr != &io_mem_rom && mr != &io_mem_notdirty && !mr->rom_device > - && mr != &io_mem_watch; > -} > - > /* Called from RCU critical section */ > static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch > *d, > hwaddr addr, > @@ -2045,17 +2037,18 @@ static MemTxResult watch_mem_read(void *opaque, > hwaddr addr, uint64_t *pdata, > { > MemTxResult res; > uint64_t data; > + AddressSpace *as = opaque; > > check_watchpoint(addr & ~TARGET_PAGE_MASK, size, attrs, BP_MEM_READ); > switch (size) { > case 1: > - data = address_space_ldub(&address_space_memory, addr, attrs, &res); > + data = address_space_ldub(as, addr, attrs, &res); > break; > case 2: > - data = address_space_lduw(&address_space_memory, addr, attrs, &res); > + data = address_space_lduw(as, addr, attrs, &res); > break; > case 4: > - data = address_space_ldl(&address_space_memory, addr, attrs, &res); > + data = address_space_ldl(as, addr, attrs, &res); > break; > default: abort(); > } > @@ -2068,17 +2061,18 @@ static MemTxResult watch_mem_write(void *opaque, > hwaddr addr, > MemTxAttrs attrs) > { > MemTxResult res; > + AddressSpace *as = opaque; > > check_watchpoint(addr & ~TARGET_PAGE_MASK, size, attrs, BP_MEM_WRITE); > switch (size) { > case 1: > - address_space_stb(&address_space_memory, addr, val, attrs, &res); > + address_space_stb(as, addr, val, attrs, &res); > break; > case 2: > - address_space_stw(&address_space_memory, addr, val, attrs, &res); > + address_space_stw(as, addr, val, attrs, &res); > break; > case 4: > - address_space_stl(&address_space_memory, addr, val, attrs, &res); > + address_space_stl(as, addr, val, attrs, &res); > break; > default: abort(); > } > @@ -2091,6 +2085,15 @@ static const MemoryRegionOps watch_mem_ops = { > .endianness = DEVICE_NATIVE_ENDIAN, > }; > > +bool memory_region_is_unassigned(MemoryRegion *mr) > +{ > + /* Checking the ops pointer of the MemoryRegion is strictly > + * speaking looking at private information of the MemoryRegion :-( > + */ > + return mr != &io_mem_rom && mr != &io_mem_notdirty && !mr->rom_device > + && mr->ops != &watch_mem_ops; > +} > + > static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data, > unsigned len, MemTxAttrs attrs) > { > @@ -2251,8 +2254,6 @@ static void io_mem_init(void) > NULL, UINT64_MAX); > memory_region_init_io(&io_mem_notdirty, NULL, ¬dirty_mem_ops, NULL, > NULL, UINT64_MAX); > - memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL, > - NULL, UINT64_MAX); > } > > static void mem_begin(MemoryListener *listener) > @@ -2267,7 +2268,7 @@ static void mem_begin(MemoryListener *listener) > assert(n == PHYS_SECTION_NOTDIRTY); > n = dummy_section(&d->map, as, &io_mem_rom); > assert(n == PHYS_SECTION_ROM); > - n = dummy_section(&d->map, as, &io_mem_watch); > + n = dummy_section(&d->map, as, as->io_mem_watch); > assert(n == PHYS_SECTION_WATCH); > > d->phys_map = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 }; > @@ -2315,6 +2316,10 @@ static void tcg_commit(MemoryListener *listener) > > void address_space_init_dispatch(AddressSpace *as) > { > + as->io_mem_watch = g_new0(MemoryRegion, 1); > + memory_region_init_io(as->io_mem_watch, NULL, &watch_mem_ops, as, > + NULL, UINT64_MAX); > + > as->dispatch = NULL; > as->dispatch_listener = (MemoryListener) { > .begin = mem_begin, > @@ -2339,6 +2344,7 @@ void address_space_destroy_dispatch(AddressSpace *as) > if (d) { > call_rcu(d, address_space_dispatch_free, rcu); > } > + memory_region_unref(as->io_mem_watch); > } > > static void memory_map_init(void) > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 0f07159..e5d98f8 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -246,6 +246,8 @@ struct AddressSpace { > struct AddressSpaceDispatch *next_dispatch; > MemoryListener dispatch_listener; > > + MemoryRegion *io_mem_watch; > + > QTAILQ_ENTRY(AddressSpace) address_spaces_link; > }; > > -- > 1.9.1 >