* [Qemu-devel] [PATCH 07/22] memory: add address_space_translate
2013-05-24 17:05 [Qemu-devel] [PATCH " Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
0 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Using phys_page_find to translate an AddressSpace to a MemoryRegionSection
is unwieldy. It requires to pass the page index rather than the address,
and later memory_region_section_addr has to be called. Replace
memory_region_section_addr with a function that does all of it: call
phys_page_find, compute the offset within the region, and check how
big the current mapping is. This way, a large flat region can be written
with a single lookup rather than a page at a time.
address_space_translate will also provide a single point where IOMMU
forwarding is implemented.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cputlb.c | 20 +++---
exec.c | 189 +++++++++++++++++++++++++-------------------------
include/exec/cputlb.h | 12 ++--
include/exec/memory.h | 31 ++++-----
translate-all.c | 6 +-
5 files changed, 128 insertions(+), 130 deletions(-)
diff --git a/cputlb.c b/cputlb.c
index b56bc01..86666c8 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -248,13 +248,18 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
target_ulong code_address;
uintptr_t addend;
CPUTLBEntry *te;
- hwaddr iotlb;
+ hwaddr iotlb, xlat, sz;
assert(size >= TARGET_PAGE_SIZE);
if (size != TARGET_PAGE_SIZE) {
tlb_add_large_page(env, vaddr, size);
}
- section = phys_page_find(address_space_memory.dispatch, paddr >> TARGET_PAGE_BITS);
+
+ sz = size;
+ section = address_space_translate(&address_space_memory, paddr, &xlat, &sz,
+ false);
+ assert(sz >= TARGET_PAGE_SIZE);
+
#if defined(DEBUG_TLB)
printf("tlb_set_page: vaddr=" TARGET_FMT_lx " paddr=0x" TARGET_FMT_plx
" prot=%x idx=%d pd=0x%08lx\n",
@@ -268,13 +273,12 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
addend = 0;
} else {
/* TLB_MMIO for rom/romd handled below */
- addend = (uintptr_t)memory_region_get_ram_ptr(section->mr)
- + memory_region_section_addr(section, paddr);
+ addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) + xlat;
}
code_address = address;
- iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, prot,
- &address);
+ iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, xlat,
+ prot, &address);
index = (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
env->iotlb[mmu_idx][index] = iotlb - vaddr;
@@ -297,9 +301,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
/* Write access calls the I/O callback. */
te->addr_write = address | TLB_MMIO;
} else if (memory_region_is_ram(section->mr)
- && !cpu_physical_memory_is_dirty(
- section->mr->ram_addr
- + memory_region_section_addr(section, paddr))) {
+ && !cpu_physical_memory_is_dirty(section->mr->ram_addr + xlat)) {
te->addr_write = address | TLB_NOTDIRTY;
} else {
te->addr_write = address;
diff --git a/exec.c b/exec.c
index c5100d6..5111327 100644
--- a/exec.c
+++ b/exec.c
@@ -182,7 +182,7 @@ static void phys_page_set(AddressSpaceDispatch *d,
phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS - 1);
}
-MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index)
+static MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index)
{
PhysPageEntry lp = d->phys_map;
PhysPageEntry *p;
@@ -203,6 +203,22 @@ bool memory_region_is_unassigned(MemoryRegion *mr)
return mr != &io_mem_rom && mr != &io_mem_notdirty && !mr->rom_device
&& mr != &io_mem_watch;
}
+
+MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
+ hwaddr *xlat, hwaddr *plen,
+ bool is_write)
+{
+ MemoryRegionSection *section;
+
+ section = phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS);
+ /* Compute offset within MemoryRegionSection */
+ addr -= section->offset_within_address_space;
+ *plen = MIN(section->size - addr, *plen);
+
+ /* Compute offset within MemoryRegion */
+ *xlat = addr + section->offset_within_region;
+ return section;
+}
#endif
void cpu_exec_init_all(void)
@@ -615,11 +631,11 @@ static int cpu_physical_memory_set_dirty_tracking(int enable)
}
hwaddr memory_region_section_get_iotlb(CPUArchState *env,
- MemoryRegionSection *section,
- target_ulong vaddr,
- hwaddr paddr,
- int prot,
- target_ulong *address)
+ MemoryRegionSection *section,
+ target_ulong vaddr,
+ hwaddr paddr, hwaddr xlat,
+ int prot,
+ target_ulong *address)
{
hwaddr iotlb;
CPUWatchpoint *wp;
@@ -627,7 +643,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env,
if (memory_region_is_ram(section->mr)) {
/* Normal RAM. */
iotlb = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
- + memory_region_section_addr(section, paddr);
+ + xlat;
if (!section->readonly) {
iotlb |= phys_section_notdirty;
} else {
@@ -635,7 +651,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env,
}
} else {
iotlb = section - phys_sections;
- iotlb += memory_region_section_addr(section, paddr);
+ iotlb += xlat;
}
/* Make accesses to pages with watchpoints go via the
@@ -1852,24 +1868,18 @@ static void invalidate_and_set_dirty(hwaddr addr,
void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
int len, bool is_write)
{
- AddressSpaceDispatch *d = as->dispatch;
- int l;
+ hwaddr l;
uint8_t *ptr;
uint32_t val;
- hwaddr page;
+ hwaddr addr1;
MemoryRegionSection *section;
while (len > 0) {
- page = addr & TARGET_PAGE_MASK;
- l = (page + TARGET_PAGE_SIZE) - addr;
- if (l > len)
- l = len;
- section = phys_page_find(d, page >> TARGET_PAGE_BITS);
+ l = len;
+ section = address_space_translate(as, addr, &addr1, &l, is_write);
if (is_write) {
if (!memory_region_is_ram(section->mr)) {
- hwaddr addr1;
- addr1 = memory_region_section_addr(section, addr);
/* XXX: could force cpu_single_env to NULL to avoid
potential bugs */
if (l >= 4 && ((addr1 & 3) == 0)) {
@@ -1889,9 +1899,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
l = 1;
}
} else if (!section->readonly) {
- ram_addr_t addr1;
- addr1 = memory_region_get_ram_addr(section->mr)
- + memory_region_section_addr(section, addr);
+ addr1 += memory_region_get_ram_addr(section->mr);
/* RAM case */
ptr = qemu_get_ram_ptr(addr1);
memcpy(ptr, buf, l);
@@ -1900,9 +1908,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
} else {
if (!(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr))) {
- hwaddr addr1;
/* I/O case */
- addr1 = memory_region_section_addr(section, addr);
if (l >= 4 && ((addr1 & 3) == 0)) {
/* 32 bit read access */
val = io_mem_read(section->mr, addr1, 4);
@@ -1921,9 +1927,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
}
} else {
/* RAM case */
- ptr = qemu_get_ram_ptr(section->mr->ram_addr
- + memory_region_section_addr(section,
- addr));
+ ptr = qemu_get_ram_ptr(section->mr->ram_addr + addr1);
memcpy(buf, ptr, l);
}
}
@@ -1962,26 +1966,21 @@ void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
void cpu_physical_memory_write_rom(hwaddr addr,
const uint8_t *buf, int len)
{
- AddressSpaceDispatch *d = address_space_memory.dispatch;
- int l;
+ hwaddr l;
uint8_t *ptr;
- hwaddr page;
+ hwaddr addr1;
MemoryRegionSection *section;
while (len > 0) {
- page = addr & TARGET_PAGE_MASK;
- l = (page + TARGET_PAGE_SIZE) - addr;
- if (l > len)
- l = len;
- section = phys_page_find(d, page >> TARGET_PAGE_BITS);
+ l = len;
+ section = address_space_translate(&address_space_memory,
+ addr, &addr1, &l, true);
if (!(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr))) {
/* do nothing */
} else {
- unsigned long addr1;
- addr1 = memory_region_get_ram_addr(section->mr)
- + memory_region_section_addr(section, addr);
+ addr1 += memory_region_get_ram_addr(section->mr);
/* ROM/RAM case */
ptr = qemu_get_ram_ptr(addr1);
memcpy(ptr, buf, l);
@@ -2051,22 +2050,17 @@ void *address_space_map(AddressSpace *as,
hwaddr *plen,
bool is_write)
{
- AddressSpaceDispatch *d = as->dispatch;
hwaddr len = *plen;
hwaddr todo = 0;
- int l;
- hwaddr page;
+ hwaddr l, xlat;
MemoryRegionSection *section;
ram_addr_t raddr = RAM_ADDR_MAX;
ram_addr_t rlen;
void *ret;
while (len > 0) {
- page = addr & TARGET_PAGE_MASK;
- l = (page + TARGET_PAGE_SIZE) - addr;
- if (l > len)
- l = len;
- section = phys_page_find(d, page >> TARGET_PAGE_BITS);
+ l = len;
+ section = address_space_translate(as, addr, &xlat, &l, is_write);
if (!(memory_region_is_ram(section->mr) && !section->readonly)) {
if (todo || bounce.buffer) {
@@ -2083,8 +2077,11 @@ void *address_space_map(AddressSpace *as,
return bounce.buffer;
}
if (!todo) {
- raddr = memory_region_get_ram_addr(section->mr)
- + memory_region_section_addr(section, addr);
+ raddr = memory_region_get_ram_addr(section->mr) + xlat;
+ } else {
+ if (memory_region_get_ram_addr(section->mr) + xlat != raddr + todo) {
+ break;
+ }
}
len -= l;
@@ -2150,14 +2147,16 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
uint8_t *ptr;
uint32_t val;
MemoryRegionSection *section;
+ hwaddr l = 4;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!(memory_region_is_ram(section->mr) ||
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ false);
+ if (l < 4 ||
+ !(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr))) {
/* I/O case */
- addr = memory_region_section_addr(section, addr);
- val = io_mem_read(section->mr, addr, 4);
+ val = io_mem_read(section->mr, addr1, 4);
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap32(val);
@@ -2171,7 +2170,7 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
/* RAM case */
ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
& TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr));
+ + addr1);
switch (endian) {
case DEVICE_LITTLE_ENDIAN:
val = ldl_le_p(ptr);
@@ -2209,28 +2208,30 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
uint8_t *ptr;
uint64_t val;
MemoryRegionSection *section;
+ hwaddr l = 8;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!(memory_region_is_ram(section->mr) ||
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ false);
+ if (l < 8 ||
+ !(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr))) {
/* I/O case */
- addr = memory_region_section_addr(section, addr);
/* XXX This is broken when device endian != cpu endian.
Fix and add "endian" variable check */
#ifdef TARGET_WORDS_BIGENDIAN
- val = io_mem_read(section->mr, addr, 4) << 32;
- val |= io_mem_read(section->mr, addr + 4, 4);
+ val = io_mem_read(section->mr, addr1, 4) << 32;
+ val |= io_mem_read(section->mr, addr1 + 4, 4);
#else
- val = io_mem_read(section->mr, addr, 4);
- val |= io_mem_read(section->mr, addr + 4, 4) << 32;
+ val = io_mem_read(section->mr, addr1, 4);
+ val |= io_mem_read(section->mr, addr1 + 4, 4) << 32;
#endif
} else {
/* RAM case */
ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
& TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr));
+ + addr1);
switch (endian) {
case DEVICE_LITTLE_ENDIAN:
val = ldq_le_p(ptr);
@@ -2276,14 +2277,16 @@ static inline uint32_t lduw_phys_internal(hwaddr addr,
uint8_t *ptr;
uint64_t val;
MemoryRegionSection *section;
+ hwaddr l = 2;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!(memory_region_is_ram(section->mr) ||
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ false);
+ if (l < 2 ||
+ !(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr))) {
/* I/O case */
- addr = memory_region_section_addr(section, addr);
- val = io_mem_read(section->mr, addr, 2);
+ val = io_mem_read(section->mr, addr1, 2);
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap16(val);
@@ -2297,7 +2300,7 @@ static inline uint32_t lduw_phys_internal(hwaddr addr,
/* RAM case */
ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
& TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr));
+ + addr1);
switch (endian) {
case DEVICE_LITTLE_ENDIAN:
val = lduw_le_p(ptr);
@@ -2335,19 +2338,18 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val)
{
uint8_t *ptr;
MemoryRegionSection *section;
+ hwaddr l = 4;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!memory_region_is_ram(section->mr) || section->readonly) {
- addr = memory_region_section_addr(section, addr);
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ true);
+ if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
if (memory_region_is_ram(section->mr)) {
section = &phys_sections[phys_section_rom];
}
- io_mem_write(section->mr, addr, val, 4);
+ io_mem_write(section->mr, addr1, val, 4);
} else {
- unsigned long addr1 = (memory_region_get_ram_addr(section->mr)
- & TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr);
+ addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
ptr = qemu_get_ram_ptr(addr1);
stl_p(ptr, val);
@@ -2369,11 +2371,12 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val,
{
uint8_t *ptr;
MemoryRegionSection *section;
+ hwaddr l = 4;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!memory_region_is_ram(section->mr) || section->readonly) {
- addr = memory_region_section_addr(section, addr);
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ true);
+ if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
if (memory_region_is_ram(section->mr)) {
section = &phys_sections[phys_section_rom];
}
@@ -2386,12 +2389,10 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val,
val = bswap32(val);
}
#endif
- io_mem_write(section->mr, addr, val, 4);
+ io_mem_write(section->mr, addr1, val, 4);
} else {
- unsigned long addr1;
- addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr);
/* RAM case */
+ addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
ptr = qemu_get_ram_ptr(addr1);
switch (endian) {
case DEVICE_LITTLE_ENDIAN:
@@ -2436,11 +2437,12 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val,
{
uint8_t *ptr;
MemoryRegionSection *section;
+ hwaddr l = 2;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!memory_region_is_ram(section->mr) || section->readonly) {
- addr = memory_region_section_addr(section, addr);
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ true);
+ if (l < 2 || !memory_region_is_ram(section->mr) || section->readonly) {
if (memory_region_is_ram(section->mr)) {
section = &phys_sections[phys_section_rom];
}
@@ -2453,12 +2455,10 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val,
val = bswap16(val);
}
#endif
- io_mem_write(section->mr, addr, val, 2);
+ io_mem_write(section->mr, addr1, val, 2);
} else {
- unsigned long addr1;
- addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr);
/* RAM case */
+ addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
ptr = qemu_get_ram_ptr(addr1);
switch (endian) {
case DEVICE_LITTLE_ENDIAN:
@@ -2561,9 +2561,10 @@ bool virtio_is_big_endian(void)
bool cpu_physical_memory_is_io(hwaddr phys_addr)
{
MemoryRegionSection *section;
+ hwaddr l = 1;
- section = phys_page_find(address_space_memory.dispatch,
- phys_addr >> TARGET_PAGE_BITS);
+ section = address_space_translate(&address_space_memory,
+ phys_addr, &phys_addr, &l, false);
return !(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr));
diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index 733c885..e821660 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -26,8 +26,6 @@ void tlb_unprotect_code_phys(CPUArchState *env, ram_addr_t ram_addr,
target_ulong vaddr);
void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start,
uintptr_t length);
-MemoryRegionSection *phys_page_find(struct AddressSpaceDispatch *d,
- hwaddr index);
void cpu_tlb_reset_dirty_all(ram_addr_t start1, ram_addr_t length);
void tlb_set_dirty(CPUArchState *env, target_ulong vaddr);
extern int tlb_flush_count;
@@ -35,11 +33,11 @@ extern int tlb_flush_count;
/* exec.c */
void tb_flush_jmp_cache(CPUArchState *env, target_ulong addr);
hwaddr memory_region_section_get_iotlb(CPUArchState *env,
- MemoryRegionSection *section,
- target_ulong vaddr,
- hwaddr paddr,
- int prot,
- target_ulong *address);
+ MemoryRegionSection *section,
+ target_ulong vaddr,
+ hwaddr paddr, hwaddr xlat,
+ int prot,
+ target_ulong *address);
bool memory_region_is_unassigned(MemoryRegion *mr);
#endif
diff --git a/include/exec/memory.h b/include/exec/memory.h
index fdf55fe..688d3f0 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -752,23 +752,6 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
hwaddr addr, uint64_t size);
/**
- * memory_region_section_addr: get offset within MemoryRegionSection
- *
- * Returns offset within MemoryRegionSection
- *
- * @section: the memory region section being queried
- * @addr: address in address space
- */
-static inline hwaddr
-memory_region_section_addr(MemoryRegionSection *section,
- hwaddr addr)
-{
- addr -= section->offset_within_address_space;
- addr += section->offset_within_region;
- return addr;
-}
-
-/**
* address_space_sync_dirty_bitmap: synchronize the dirty log for all memory
*
* Synchronizes the dirty page log for an entire address space.
@@ -869,6 +852,20 @@ void address_space_write(AddressSpace *as, hwaddr addr,
*/
void address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len);
+/* address_space_translate: translate an address range into an address space
+ * into a MemoryRegionSection and an address range into that section
+ *
+ * @as: #AddressSpace to be accessed
+ * @addr: address within that address space
+ * @xlat: pointer to address within the returned memory region section's
+ * #MemoryRegion.
+ * @len: pointer to length
+ * @is_write: indicates the transfer direction
+ */
+MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
+ hwaddr *xlat, hwaddr *len,
+ bool is_write);
+
/* address_space_map: map a physical memory region into a host virtual address
*
* May map a subset of the requested range, given by and returned in @plen.
diff --git a/translate-all.c b/translate-all.c
index 0d84b0d..7a7d537 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1355,15 +1355,15 @@ void tb_invalidate_phys_addr(hwaddr addr)
{
ram_addr_t ram_addr;
MemoryRegionSection *section;
+ hwaddr l = 1;
- section = phys_page_find(address_space_memory.dispatch,
- addr >> TARGET_PAGE_BITS);
+ section = address_space_translate(&address_space_memory, addr, &addr, &l, false);
if (!(memory_region_is_ram(section->mr)
|| memory_region_is_romd(section->mr))) {
return;
}
ram_addr = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr);
+ + addr;
tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0);
}
#endif /* TARGET_HAS_ICE && !defined(CONFIG_USER_ONLY) */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection
@ 2013-05-30 21:03 Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 01/22] exec: eliminate io_mem_ram Paolo Bonzini
` (22 more replies)
0 siblings, 23 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
Anthony,
the following changes since commit 6a4e17711442849bf2cc731ccddef5a2a2d92d29:
Remove Sun4c, Sun4d and a few CPUs (2013-05-26 11:37:58 +0000)
are available in the git repository at:
git://github.com/bonzini/qemu.git iommu-for-anthony
This is part 2 of the memory/IOMMU patches. These reorganize the
handling of unassigned accesses so that they are propagated as
errors during I/O dispatch. In the end, a return value is added to
address_space_rw/read/write. This is particularly useful when an IOMMU
is available, because it lets devices detect faulting accesses.
All patches were reviewed by rth.
Paolo
Paolo Bonzini (22):
exec: eliminate io_mem_ram
exec: drop useless #if
cputlb: simplify tlb_set_page
exec: make io_mem_unassigned private
exec: do not use error_mem_read
memory: dispatch unassigned accesses based on .valid.accepts
memory: add address_space_translate
memory: move unassigned_mem_ops to memory.c
memory: assign MemoryRegionOps to all regions
exec: expect mr->ops to be initialized for ROM
exec: introduce memory_access_is_direct
exec: introduce memory_access_size
memory: export memory_region_access_valid to exec.c
exec: implement .valid.accepts for subpages
memory: add address_space_access_valid
memory: accept mismatching sizes in memory_region_access_valid
memory: add big endian support to access_with_adjusted_size
memory: split accesses even when the old MMIO callbacks are used
memory: correctly handle endian-swapped 64-bit accesses
exec: just use io_mem_read/io_mem_write for 8-byte I/O accesses
memory: propagate errors on I/O dispatch
memory: add return value to address_space_rw/read/write
cputlb.c | 31 ++--
dma-helpers.c | 5 +
exec.c | 415 +++++++++++++++++++--------------------
include/exec/cpu-common.h | 2 -
include/exec/cputlb.h | 12 +-
include/exec/exec-all.h | 6 +-
include/exec/memory-internal.h | 5 +
include/exec/memory.h | 58 ++++--
include/exec/softmmu_template.h | 36 +---
include/sysemu/dma.h | 3 +-
memory.c | 215 +++++++++++++--------
translate-all.c | 6 +-
12 files changed, 422 insertions(+), 372 deletions(-)
--
1.7.4.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 01/22] exec: eliminate io_mem_ram
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 02/22] exec: drop useless #if Paolo Bonzini
` (21 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
It is never used, the IOTLB always goes through io_mem_notdirty.
In fact in softmmu_template.h, if it were, QEMU would crash just
below the tests, as soon as io_mem_read/write dispatches to
error_mem_read/write.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 18 ++----------------
include/exec/cpu-common.h | 1 -
include/exec/softmmu_template.h | 4 ++--
3 files changed, 4 insertions(+), 19 deletions(-)
diff --git a/exec.c b/exec.c
index 3a9ddcb..b720be5 100644
--- a/exec.c
+++ b/exec.c
@@ -66,7 +66,7 @@ AddressSpace address_space_io;
AddressSpace address_space_memory;
DMAContext dma_context_memory;
-MemoryRegion io_mem_ram, io_mem_rom, io_mem_unassigned, io_mem_notdirty;
+MemoryRegion io_mem_rom, io_mem_unassigned, io_mem_notdirty;
static MemoryRegion io_mem_subpage_ram;
#endif
@@ -200,8 +200,7 @@ MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index)
bool memory_region_is_unassigned(MemoryRegion *mr)
{
- return mr != &io_mem_ram && mr != &io_mem_rom
- && mr != &io_mem_notdirty && !mr->rom_device
+ return mr != &io_mem_rom && mr != &io_mem_notdirty && !mr->rom_device
&& mr != &io_mem_watch;
}
#endif
@@ -1419,18 +1418,6 @@ static uint64_t error_mem_read(void *opaque, hwaddr addr,
abort();
}
-static void error_mem_write(void *opaque, hwaddr addr,
- uint64_t value, unsigned size)
-{
- abort();
-}
-
-static const MemoryRegionOps error_mem_ops = {
- .read = error_mem_read,
- .write = error_mem_write,
- .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
static const MemoryRegionOps rom_mem_ops = {
.read = error_mem_read,
.write = unassigned_mem_write,
@@ -1691,7 +1678,6 @@ MemoryRegion *iotlb_to_region(hwaddr index)
static void io_mem_init(void)
{
- memory_region_init_io(&io_mem_ram, &error_mem_ops, NULL, "ram", UINT64_MAX);
memory_region_init_io(&io_mem_rom, &rom_mem_ops, NULL, "rom", UINT64_MAX);
memory_region_init_io(&io_mem_unassigned, &unassigned_mem_ops, NULL,
"unassigned", UINT64_MAX);
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index af5258d..1686b8f 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -110,7 +110,6 @@ void stq_phys(hwaddr addr, uint64_t val);
void cpu_physical_memory_write_rom(hwaddr addr,
const uint8_t *buf, int len);
-extern struct MemoryRegion io_mem_ram;
extern struct MemoryRegion io_mem_rom;
extern struct MemoryRegion io_mem_unassigned;
extern struct MemoryRegion io_mem_notdirty;
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index b219191..4501dac 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -68,7 +68,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
env->mem_io_pc = retaddr;
- if (mr != &io_mem_ram && mr != &io_mem_rom
+ if (mr != &io_mem_rom
&& mr != &io_mem_unassigned
&& mr != &io_mem_notdirty
&& !can_do_io(env)) {
@@ -218,7 +218,7 @@ static inline void glue(io_write, SUFFIX)(CPUArchState *env,
MemoryRegion *mr = iotlb_to_region(physaddr);
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
- if (mr != &io_mem_ram && mr != &io_mem_rom
+ if (mr != &io_mem_rom
&& mr != &io_mem_unassigned
&& mr != &io_mem_notdirty
&& !can_do_io(env)) {
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 02/22] exec: drop useless #if
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 01/22] exec: eliminate io_mem_ram Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 03/22] cputlb: simplify tlb_set_page Paolo Bonzini
` (20 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
This code is only compiled for softmmu targets.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/exec.c b/exec.c
index b720be5..7728ea3 100644
--- a/exec.c
+++ b/exec.c
@@ -1430,10 +1430,8 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
int dirty_flags;
dirty_flags = cpu_physical_memory_get_dirty_flags(ram_addr);
if (!(dirty_flags & CODE_DIRTY_FLAG)) {
-#if !defined(CONFIG_USER_ONLY)
tb_invalidate_phys_page_fast(ram_addr, size);
dirty_flags = cpu_physical_memory_get_dirty_flags(ram_addr);
-#endif
}
switch (size) {
case 1:
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 03/22] cputlb: simplify tlb_set_page
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 01/22] exec: eliminate io_mem_ram Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 02/22] exec: drop useless #if Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 04/22] exec: make io_mem_unassigned private Paolo Bonzini
` (19 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
The same "if" condition is repeated twice.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cputlb.c | 13 +++++--------
1 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/cputlb.c b/cputlb.c
index aba7e44..b56bc01 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -262,17 +262,14 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
#endif
address = vaddr;
- if (!(memory_region_is_ram(section->mr) ||
- memory_region_is_romd(section->mr))) {
- /* IO memory case (romd handled later) */
+ if (!memory_region_is_ram(section->mr) && !memory_region_is_romd(section->mr)) {
+ /* IO memory case */
address |= TLB_MMIO;
- }
- if (memory_region_is_ram(section->mr) ||
- memory_region_is_romd(section->mr)) {
+ addend = 0;
+ } else {
+ /* TLB_MMIO for rom/romd handled below */
addend = (uintptr_t)memory_region_get_ram_ptr(section->mr)
+ memory_region_section_addr(section, paddr);
- } else {
- addend = 0;
}
code_address = address;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 04/22] exec: make io_mem_unassigned private
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (2 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 03/22] cputlb: simplify tlb_set_page Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 05/22] exec: do not use error_mem_read Paolo Bonzini
` (18 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
There is no reason to avoid a recompile before accessing unassigned
memory. In the end it will be treated as MMIO anyway.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 4 ++--
include/exec/cpu-common.h | 1 -
include/exec/softmmu_template.h | 10 ++--------
3 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/exec.c b/exec.c
index 7728ea3..7e22980 100644
--- a/exec.c
+++ b/exec.c
@@ -66,8 +66,8 @@ AddressSpace address_space_io;
AddressSpace address_space_memory;
DMAContext dma_context_memory;
-MemoryRegion io_mem_rom, io_mem_unassigned, io_mem_notdirty;
-static MemoryRegion io_mem_subpage_ram;
+MemoryRegion io_mem_rom, io_mem_notdirty;
+static MemoryRegion io_mem_unassigned, io_mem_subpage_ram;
#endif
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 1686b8f..e061e21 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -111,7 +111,6 @@ void cpu_physical_memory_write_rom(hwaddr addr,
const uint8_t *buf, int len);
extern struct MemoryRegion io_mem_rom;
-extern struct MemoryRegion io_mem_unassigned;
extern struct MemoryRegion io_mem_notdirty;
#endif
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index 4501dac..ca91fd0 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -68,10 +68,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
env->mem_io_pc = retaddr;
- if (mr != &io_mem_rom
- && mr != &io_mem_unassigned
- && mr != &io_mem_notdirty
- && !can_do_io(env)) {
+ if (mr != &io_mem_rom && mr != &io_mem_notdirty && !can_do_io(env)) {
cpu_io_recompile(env, retaddr);
}
@@ -218,10 +215,7 @@ static inline void glue(io_write, SUFFIX)(CPUArchState *env,
MemoryRegion *mr = iotlb_to_region(physaddr);
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
- if (mr != &io_mem_rom
- && mr != &io_mem_unassigned
- && mr != &io_mem_notdirty
- && !can_do_io(env)) {
+ if (mr != &io_mem_rom && mr != &io_mem_notdirty && !can_do_io(env)) {
cpu_io_recompile(env, retaddr);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 05/22] exec: do not use error_mem_read
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (3 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 04/22] exec: make io_mem_unassigned private Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts Paolo Bonzini
` (17 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
We will soon reach this case when doing (unaligned) accesses that
span partly past the end of memory. We do not want to crash in
that case.
unassigned_mem_ops and rom_mem_ops are now the same.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 16 ++--------------
1 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/exec.c b/exec.c
index 7e22980..785eeeb 100644
--- a/exec.c
+++ b/exec.c
@@ -1412,18 +1412,6 @@ static const MemoryRegionOps unassigned_mem_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static uint64_t error_mem_read(void *opaque, hwaddr addr,
- unsigned size)
-{
- abort();
-}
-
-static const MemoryRegionOps rom_mem_ops = {
- .read = error_mem_read,
- .write = unassigned_mem_write,
- .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
uint64_t val, unsigned size)
{
@@ -1455,7 +1443,7 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
}
static const MemoryRegionOps notdirty_mem_ops = {
- .read = error_mem_read,
+ .read = unassigned_mem_read,
.write = notdirty_mem_write,
.endianness = DEVICE_NATIVE_ENDIAN,
};
@@ -1676,7 +1664,7 @@ MemoryRegion *iotlb_to_region(hwaddr index)
static void io_mem_init(void)
{
- memory_region_init_io(&io_mem_rom, &rom_mem_ops, NULL, "rom", UINT64_MAX);
+ memory_region_init_io(&io_mem_rom, &unassigned_mem_ops, NULL, "rom", UINT64_MAX);
memory_region_init_io(&io_mem_unassigned, &unassigned_mem_ops, NULL,
"unassigned", UINT64_MAX);
memory_region_init_io(&io_mem_notdirty, ¬dirty_mem_ops, NULL,
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (4 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 05/22] exec: do not use error_mem_read Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-06-01 15:28 ` Blue Swirl
2013-07-16 12:28 ` Jan Kiszka
2013-05-30 21:03 ` [Qemu-devel] [PATCH 07/22] memory: add address_space_translate Paolo Bonzini
` (16 subsequent siblings)
22 siblings, 2 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
This provides the basics for detecting accesses to unassigned memory
as soon as they happen, and also for a simple implementation of
address_space_access_valid.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 36 ++++++++++++------------------------
memory.c | 28 ++++++++++++++++++++++++++--
2 files changed, 38 insertions(+), 26 deletions(-)
diff --git a/exec.c b/exec.c
index 785eeeb..c5100d6 100644
--- a/exec.c
+++ b/exec.c
@@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
return ram_addr;
}
-static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
- unsigned size)
+static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
+ unsigned size, bool is_write)
{
-#ifdef DEBUG_UNASSIGNED
- printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
-#endif
-#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
- cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
-#endif
- return 0;
-}
-
-static void unassigned_mem_write(void *opaque, hwaddr addr,
- uint64_t val, unsigned size)
-{
-#ifdef DEBUG_UNASSIGNED
- printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
-#endif
-#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
- cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
-#endif
+ return false;
}
-static const MemoryRegionOps unassigned_mem_ops = {
- .read = unassigned_mem_read,
- .write = unassigned_mem_write,
+const MemoryRegionOps unassigned_mem_ops = {
+ .valid.accepts = unassigned_mem_accepts,
.endianness = DEVICE_NATIVE_ENDIAN,
};
@@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
}
+static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
+ unsigned size, bool is_write)
+{
+ return is_write;
+}
+
static const MemoryRegionOps notdirty_mem_ops = {
- .read = unassigned_mem_read,
.write = notdirty_mem_write,
+ .valid.accepts = notdirty_mem_accepts,
.endianness = DEVICE_NATIVE_ENDIAN,
};
diff --git a/memory.c b/memory.c
index 99f046d..15da877 100644
--- a/memory.c
+++ b/memory.c
@@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr,
mr->flush_coalesced_mmio = false;
}
+static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+#ifdef DEBUG_UNASSIGNED
+ printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
+#endif
+#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
+ cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
+#endif
+ return 0;
+}
+
+static void unassigned_mem_write(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size)
+{
+#ifdef DEBUG_UNASSIGNED
+ printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
+#endif
+#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
+ cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
+#endif
+}
+
static bool memory_region_access_valid(MemoryRegion *mr,
hwaddr addr,
unsigned size,
@@ -847,7 +870,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
uint64_t data = 0;
if (!memory_region_access_valid(mr, addr, size, false)) {
- return -1U; /* FIXME: better signalling */
+ return unassigned_mem_read(mr, addr, size);
}
if (!mr->ops->read) {
@@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
unsigned size)
{
if (!memory_region_access_valid(mr, addr, size, true)) {
- return; /* FIXME: better signalling */
+ unassigned_mem_write(mr, addr, data, size);
+ return;
}
adjust_endianness(mr, &data, size);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 07/22] memory: add address_space_translate
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (5 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-06-20 13:53 ` Peter Maydell
2013-05-30 21:03 ` [Qemu-devel] [PATCH 08/22] memory: move unassigned_mem_ops to memory.c Paolo Bonzini
` (15 subsequent siblings)
22 siblings, 1 reply; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
Using phys_page_find to translate an AddressSpace to a MemoryRegionSection
is unwieldy. It requires to pass the page index rather than the address,
and later memory_region_section_addr has to be called. Replace
memory_region_section_addr with a function that does all of it: call
phys_page_find, compute the offset within the region, and check how
big the current mapping is. This way, a large flat region can be written
with a single lookup rather than a page at a time.
address_space_translate will also provide a single point where IOMMU
forwarding is implemented.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cputlb.c | 20 +++---
exec.c | 192 +++++++++++++++++++++++++------------------------
include/exec/cputlb.h | 12 ++--
include/exec/memory.h | 31 ++++-----
translate-all.c | 6 +-
5 files changed, 131 insertions(+), 130 deletions(-)
diff --git a/cputlb.c b/cputlb.c
index b56bc01..86666c8 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -248,13 +248,18 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
target_ulong code_address;
uintptr_t addend;
CPUTLBEntry *te;
- hwaddr iotlb;
+ hwaddr iotlb, xlat, sz;
assert(size >= TARGET_PAGE_SIZE);
if (size != TARGET_PAGE_SIZE) {
tlb_add_large_page(env, vaddr, size);
}
- section = phys_page_find(address_space_memory.dispatch, paddr >> TARGET_PAGE_BITS);
+
+ sz = size;
+ section = address_space_translate(&address_space_memory, paddr, &xlat, &sz,
+ false);
+ assert(sz >= TARGET_PAGE_SIZE);
+
#if defined(DEBUG_TLB)
printf("tlb_set_page: vaddr=" TARGET_FMT_lx " paddr=0x" TARGET_FMT_plx
" prot=%x idx=%d pd=0x%08lx\n",
@@ -268,13 +273,12 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
addend = 0;
} else {
/* TLB_MMIO for rom/romd handled below */
- addend = (uintptr_t)memory_region_get_ram_ptr(section->mr)
- + memory_region_section_addr(section, paddr);
+ addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) + xlat;
}
code_address = address;
- iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, prot,
- &address);
+ iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, xlat,
+ prot, &address);
index = (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
env->iotlb[mmu_idx][index] = iotlb - vaddr;
@@ -297,9 +301,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
/* Write access calls the I/O callback. */
te->addr_write = address | TLB_MMIO;
} else if (memory_region_is_ram(section->mr)
- && !cpu_physical_memory_is_dirty(
- section->mr->ram_addr
- + memory_region_section_addr(section, paddr))) {
+ && !cpu_physical_memory_is_dirty(section->mr->ram_addr + xlat)) {
te->addr_write = address | TLB_NOTDIRTY;
} else {
te->addr_write = address;
diff --git a/exec.c b/exec.c
index c5100d6..3ebc46a 100644
--- a/exec.c
+++ b/exec.c
@@ -182,7 +182,7 @@ static void phys_page_set(AddressSpaceDispatch *d,
phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS - 1);
}
-MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index)
+static MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index)
{
PhysPageEntry lp = d->phys_map;
PhysPageEntry *p;
@@ -203,6 +203,25 @@ bool memory_region_is_unassigned(MemoryRegion *mr)
return mr != &io_mem_rom && mr != &io_mem_notdirty && !mr->rom_device
&& mr != &io_mem_watch;
}
+
+MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
+ hwaddr *xlat, hwaddr *plen,
+ bool is_write)
+{
+ MemoryRegionSection *section;
+ Int128 diff;
+
+ section = phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS);
+ /* Compute offset within MemoryRegionSection */
+ addr -= section->offset_within_address_space;
+
+ /* Compute offset within MemoryRegion */
+ *xlat = addr + section->offset_within_region;
+
+ diff = int128_sub(section->mr->size, int128_make64(addr));
+ *plen = MIN(int128_get64(diff), *plen);
+ return section;
+}
#endif
void cpu_exec_init_all(void)
@@ -615,11 +634,11 @@ static int cpu_physical_memory_set_dirty_tracking(int enable)
}
hwaddr memory_region_section_get_iotlb(CPUArchState *env,
- MemoryRegionSection *section,
- target_ulong vaddr,
- hwaddr paddr,
- int prot,
- target_ulong *address)
+ MemoryRegionSection *section,
+ target_ulong vaddr,
+ hwaddr paddr, hwaddr xlat,
+ int prot,
+ target_ulong *address)
{
hwaddr iotlb;
CPUWatchpoint *wp;
@@ -627,7 +646,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env,
if (memory_region_is_ram(section->mr)) {
/* Normal RAM. */
iotlb = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
- + memory_region_section_addr(section, paddr);
+ + xlat;
if (!section->readonly) {
iotlb |= phys_section_notdirty;
} else {
@@ -635,7 +654,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env,
}
} else {
iotlb = section - phys_sections;
- iotlb += memory_region_section_addr(section, paddr);
+ iotlb += xlat;
}
/* Make accesses to pages with watchpoints go via the
@@ -1852,24 +1871,18 @@ static void invalidate_and_set_dirty(hwaddr addr,
void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
int len, bool is_write)
{
- AddressSpaceDispatch *d = as->dispatch;
- int l;
+ hwaddr l;
uint8_t *ptr;
uint32_t val;
- hwaddr page;
+ hwaddr addr1;
MemoryRegionSection *section;
while (len > 0) {
- page = addr & TARGET_PAGE_MASK;
- l = (page + TARGET_PAGE_SIZE) - addr;
- if (l > len)
- l = len;
- section = phys_page_find(d, page >> TARGET_PAGE_BITS);
+ l = len;
+ section = address_space_translate(as, addr, &addr1, &l, is_write);
if (is_write) {
if (!memory_region_is_ram(section->mr)) {
- hwaddr addr1;
- addr1 = memory_region_section_addr(section, addr);
/* XXX: could force cpu_single_env to NULL to avoid
potential bugs */
if (l >= 4 && ((addr1 & 3) == 0)) {
@@ -1889,9 +1902,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
l = 1;
}
} else if (!section->readonly) {
- ram_addr_t addr1;
- addr1 = memory_region_get_ram_addr(section->mr)
- + memory_region_section_addr(section, addr);
+ addr1 += memory_region_get_ram_addr(section->mr);
/* RAM case */
ptr = qemu_get_ram_ptr(addr1);
memcpy(ptr, buf, l);
@@ -1900,9 +1911,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
} else {
if (!(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr))) {
- hwaddr addr1;
/* I/O case */
- addr1 = memory_region_section_addr(section, addr);
if (l >= 4 && ((addr1 & 3) == 0)) {
/* 32 bit read access */
val = io_mem_read(section->mr, addr1, 4);
@@ -1921,9 +1930,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
}
} else {
/* RAM case */
- ptr = qemu_get_ram_ptr(section->mr->ram_addr
- + memory_region_section_addr(section,
- addr));
+ ptr = qemu_get_ram_ptr(section->mr->ram_addr + addr1);
memcpy(buf, ptr, l);
}
}
@@ -1962,26 +1969,21 @@ void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
void cpu_physical_memory_write_rom(hwaddr addr,
const uint8_t *buf, int len)
{
- AddressSpaceDispatch *d = address_space_memory.dispatch;
- int l;
+ hwaddr l;
uint8_t *ptr;
- hwaddr page;
+ hwaddr addr1;
MemoryRegionSection *section;
while (len > 0) {
- page = addr & TARGET_PAGE_MASK;
- l = (page + TARGET_PAGE_SIZE) - addr;
- if (l > len)
- l = len;
- section = phys_page_find(d, page >> TARGET_PAGE_BITS);
+ l = len;
+ section = address_space_translate(&address_space_memory,
+ addr, &addr1, &l, true);
if (!(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr))) {
/* do nothing */
} else {
- unsigned long addr1;
- addr1 = memory_region_get_ram_addr(section->mr)
- + memory_region_section_addr(section, addr);
+ addr1 += memory_region_get_ram_addr(section->mr);
/* ROM/RAM case */
ptr = qemu_get_ram_ptr(addr1);
memcpy(ptr, buf, l);
@@ -2051,22 +2053,17 @@ void *address_space_map(AddressSpace *as,
hwaddr *plen,
bool is_write)
{
- AddressSpaceDispatch *d = as->dispatch;
hwaddr len = *plen;
hwaddr todo = 0;
- int l;
- hwaddr page;
+ hwaddr l, xlat;
MemoryRegionSection *section;
ram_addr_t raddr = RAM_ADDR_MAX;
ram_addr_t rlen;
void *ret;
while (len > 0) {
- page = addr & TARGET_PAGE_MASK;
- l = (page + TARGET_PAGE_SIZE) - addr;
- if (l > len)
- l = len;
- section = phys_page_find(d, page >> TARGET_PAGE_BITS);
+ l = len;
+ section = address_space_translate(as, addr, &xlat, &l, is_write);
if (!(memory_region_is_ram(section->mr) && !section->readonly)) {
if (todo || bounce.buffer) {
@@ -2083,8 +2080,11 @@ void *address_space_map(AddressSpace *as,
return bounce.buffer;
}
if (!todo) {
- raddr = memory_region_get_ram_addr(section->mr)
- + memory_region_section_addr(section, addr);
+ raddr = memory_region_get_ram_addr(section->mr) + xlat;
+ } else {
+ if (memory_region_get_ram_addr(section->mr) + xlat != raddr + todo) {
+ break;
+ }
}
len -= l;
@@ -2150,14 +2150,16 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
uint8_t *ptr;
uint32_t val;
MemoryRegionSection *section;
+ hwaddr l = 4;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!(memory_region_is_ram(section->mr) ||
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ false);
+ if (l < 4 ||
+ !(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr))) {
/* I/O case */
- addr = memory_region_section_addr(section, addr);
- val = io_mem_read(section->mr, addr, 4);
+ val = io_mem_read(section->mr, addr1, 4);
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap32(val);
@@ -2171,7 +2173,7 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
/* RAM case */
ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
& TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr));
+ + addr1);
switch (endian) {
case DEVICE_LITTLE_ENDIAN:
val = ldl_le_p(ptr);
@@ -2209,28 +2211,30 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
uint8_t *ptr;
uint64_t val;
MemoryRegionSection *section;
+ hwaddr l = 8;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!(memory_region_is_ram(section->mr) ||
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ false);
+ if (l < 8 ||
+ !(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr))) {
/* I/O case */
- addr = memory_region_section_addr(section, addr);
/* XXX This is broken when device endian != cpu endian.
Fix and add "endian" variable check */
#ifdef TARGET_WORDS_BIGENDIAN
- val = io_mem_read(section->mr, addr, 4) << 32;
- val |= io_mem_read(section->mr, addr + 4, 4);
+ val = io_mem_read(section->mr, addr1, 4) << 32;
+ val |= io_mem_read(section->mr, addr1 + 4, 4);
#else
- val = io_mem_read(section->mr, addr, 4);
- val |= io_mem_read(section->mr, addr + 4, 4) << 32;
+ val = io_mem_read(section->mr, addr1, 4);
+ val |= io_mem_read(section->mr, addr1 + 4, 4) << 32;
#endif
} else {
/* RAM case */
ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
& TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr));
+ + addr1);
switch (endian) {
case DEVICE_LITTLE_ENDIAN:
val = ldq_le_p(ptr);
@@ -2276,14 +2280,16 @@ static inline uint32_t lduw_phys_internal(hwaddr addr,
uint8_t *ptr;
uint64_t val;
MemoryRegionSection *section;
+ hwaddr l = 2;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!(memory_region_is_ram(section->mr) ||
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ false);
+ if (l < 2 ||
+ !(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr))) {
/* I/O case */
- addr = memory_region_section_addr(section, addr);
- val = io_mem_read(section->mr, addr, 2);
+ val = io_mem_read(section->mr, addr1, 2);
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap16(val);
@@ -2297,7 +2303,7 @@ static inline uint32_t lduw_phys_internal(hwaddr addr,
/* RAM case */
ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
& TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr));
+ + addr1);
switch (endian) {
case DEVICE_LITTLE_ENDIAN:
val = lduw_le_p(ptr);
@@ -2335,19 +2341,18 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val)
{
uint8_t *ptr;
MemoryRegionSection *section;
+ hwaddr l = 4;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!memory_region_is_ram(section->mr) || section->readonly) {
- addr = memory_region_section_addr(section, addr);
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ true);
+ if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
if (memory_region_is_ram(section->mr)) {
section = &phys_sections[phys_section_rom];
}
- io_mem_write(section->mr, addr, val, 4);
+ io_mem_write(section->mr, addr1, val, 4);
} else {
- unsigned long addr1 = (memory_region_get_ram_addr(section->mr)
- & TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr);
+ addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
ptr = qemu_get_ram_ptr(addr1);
stl_p(ptr, val);
@@ -2369,11 +2374,12 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val,
{
uint8_t *ptr;
MemoryRegionSection *section;
+ hwaddr l = 4;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!memory_region_is_ram(section->mr) || section->readonly) {
- addr = memory_region_section_addr(section, addr);
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ true);
+ if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
if (memory_region_is_ram(section->mr)) {
section = &phys_sections[phys_section_rom];
}
@@ -2386,12 +2392,10 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val,
val = bswap32(val);
}
#endif
- io_mem_write(section->mr, addr, val, 4);
+ io_mem_write(section->mr, addr1, val, 4);
} else {
- unsigned long addr1;
- addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr);
/* RAM case */
+ addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
ptr = qemu_get_ram_ptr(addr1);
switch (endian) {
case DEVICE_LITTLE_ENDIAN:
@@ -2436,11 +2440,12 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val,
{
uint8_t *ptr;
MemoryRegionSection *section;
+ hwaddr l = 2;
+ hwaddr addr1;
- section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
- if (!memory_region_is_ram(section->mr) || section->readonly) {
- addr = memory_region_section_addr(section, addr);
+ section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+ true);
+ if (l < 2 || !memory_region_is_ram(section->mr) || section->readonly) {
if (memory_region_is_ram(section->mr)) {
section = &phys_sections[phys_section_rom];
}
@@ -2453,12 +2458,10 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val,
val = bswap16(val);
}
#endif
- io_mem_write(section->mr, addr, val, 2);
+ io_mem_write(section->mr, addr1, val, 2);
} else {
- unsigned long addr1;
- addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr);
/* RAM case */
+ addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
ptr = qemu_get_ram_ptr(addr1);
switch (endian) {
case DEVICE_LITTLE_ENDIAN:
@@ -2561,9 +2564,10 @@ bool virtio_is_big_endian(void)
bool cpu_physical_memory_is_io(hwaddr phys_addr)
{
MemoryRegionSection *section;
+ hwaddr l = 1;
- section = phys_page_find(address_space_memory.dispatch,
- phys_addr >> TARGET_PAGE_BITS);
+ section = address_space_translate(&address_space_memory,
+ phys_addr, &phys_addr, &l, false);
return !(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr));
diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index 733c885..e821660 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -26,8 +26,6 @@ void tlb_unprotect_code_phys(CPUArchState *env, ram_addr_t ram_addr,
target_ulong vaddr);
void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start,
uintptr_t length);
-MemoryRegionSection *phys_page_find(struct AddressSpaceDispatch *d,
- hwaddr index);
void cpu_tlb_reset_dirty_all(ram_addr_t start1, ram_addr_t length);
void tlb_set_dirty(CPUArchState *env, target_ulong vaddr);
extern int tlb_flush_count;
@@ -35,11 +33,11 @@ extern int tlb_flush_count;
/* exec.c */
void tb_flush_jmp_cache(CPUArchState *env, target_ulong addr);
hwaddr memory_region_section_get_iotlb(CPUArchState *env,
- MemoryRegionSection *section,
- target_ulong vaddr,
- hwaddr paddr,
- int prot,
- target_ulong *address);
+ MemoryRegionSection *section,
+ target_ulong vaddr,
+ hwaddr paddr, hwaddr xlat,
+ int prot,
+ target_ulong *address);
bool memory_region_is_unassigned(MemoryRegion *mr);
#endif
diff --git a/include/exec/memory.h b/include/exec/memory.h
index fdf55fe..688d3f0 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -752,23 +752,6 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
hwaddr addr, uint64_t size);
/**
- * memory_region_section_addr: get offset within MemoryRegionSection
- *
- * Returns offset within MemoryRegionSection
- *
- * @section: the memory region section being queried
- * @addr: address in address space
- */
-static inline hwaddr
-memory_region_section_addr(MemoryRegionSection *section,
- hwaddr addr)
-{
- addr -= section->offset_within_address_space;
- addr += section->offset_within_region;
- return addr;
-}
-
-/**
* address_space_sync_dirty_bitmap: synchronize the dirty log for all memory
*
* Synchronizes the dirty page log for an entire address space.
@@ -869,6 +852,20 @@ void address_space_write(AddressSpace *as, hwaddr addr,
*/
void address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len);
+/* address_space_translate: translate an address range into an address space
+ * into a MemoryRegionSection and an address range into that section
+ *
+ * @as: #AddressSpace to be accessed
+ * @addr: address within that address space
+ * @xlat: pointer to address within the returned memory region section's
+ * #MemoryRegion.
+ * @len: pointer to length
+ * @is_write: indicates the transfer direction
+ */
+MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
+ hwaddr *xlat, hwaddr *len,
+ bool is_write);
+
/* address_space_map: map a physical memory region into a host virtual address
*
* May map a subset of the requested range, given by and returned in @plen.
diff --git a/translate-all.c b/translate-all.c
index 211be31..40b8f3d 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1354,15 +1354,15 @@ void tb_invalidate_phys_addr(hwaddr addr)
{
ram_addr_t ram_addr;
MemoryRegionSection *section;
+ hwaddr l = 1;
- section = phys_page_find(address_space_memory.dispatch,
- addr >> TARGET_PAGE_BITS);
+ section = address_space_translate(&address_space_memory, addr, &addr, &l, false);
if (!(memory_region_is_ram(section->mr)
|| memory_region_is_romd(section->mr))) {
return;
}
ram_addr = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
- + memory_region_section_addr(section, addr);
+ + addr;
tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0);
}
#endif /* TARGET_HAS_ICE && !defined(CONFIG_USER_ONLY) */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 08/22] memory: move unassigned_mem_ops to memory.c
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (6 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 07/22] memory: add address_space_translate Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 09/22] memory: assign MemoryRegionOps to all regions Paolo Bonzini
` (14 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
reservation_ops is already doing the same thing.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 12 ----------
include/exec/memory-internal.h | 2 +
memory.c | 44 ++++++++++++---------------------------
3 files changed, 16 insertions(+), 42 deletions(-)
diff --git a/exec.c b/exec.c
index 3ebc46a..03003b2 100644
--- a/exec.c
+++ b/exec.c
@@ -50,7 +50,6 @@
#include "exec/memory-internal.h"
-//#define DEBUG_UNASSIGNED
//#define DEBUG_SUBPAGE
#if !defined(CONFIG_USER_ONLY)
@@ -1402,17 +1401,6 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
return ram_addr;
}
-static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
- unsigned size, bool is_write)
-{
- return false;
-}
-
-const MemoryRegionOps unassigned_mem_ops = {
- .valid.accepts = unassigned_mem_accepts,
- .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
uint64_t val, unsigned size)
{
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 8d15f90..c18b36c 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -43,6 +43,8 @@ struct AddressSpaceDispatch {
void address_space_init_dispatch(AddressSpace *as);
void address_space_destroy_dispatch(AddressSpace *as);
+extern const MemoryRegionOps unassigned_mem_ops;
+
ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
MemoryRegion *mr);
ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr);
diff --git a/memory.c b/memory.c
index 15da877..2e4f547 100644
--- a/memory.c
+++ b/memory.c
@@ -22,6 +22,8 @@
#include "exec/memory-internal.h"
+//#define DEBUG_UNASSIGNED
+
static unsigned memory_region_transaction_depth;
static bool memory_region_update_pending;
static bool global_dirty_log = false;
@@ -837,6 +839,17 @@ static void unassigned_mem_write(void *opaque, hwaddr addr,
#endif
}
+static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
+ unsigned size, bool is_write)
+{
+ return false;
+}
+
+const MemoryRegionOps unassigned_mem_ops = {
+ .valid.accepts = unassigned_mem_accepts,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
static bool memory_region_access_valid(MemoryRegion *mr,
hwaddr addr,
unsigned size,
@@ -1001,40 +1014,11 @@ void memory_region_init_rom_device(MemoryRegion *mr,
mr->ram_addr = qemu_ram_alloc(size, mr);
}
-static uint64_t invalid_read(void *opaque, hwaddr addr,
- unsigned size)
-{
- MemoryRegion *mr = opaque;
-
- if (!mr->warning_printed) {
- fprintf(stderr, "Invalid read from memory region %s\n", mr->name);
- mr->warning_printed = true;
- }
- return -1U;
-}
-
-static void invalid_write(void *opaque, hwaddr addr, uint64_t data,
- unsigned size)
-{
- MemoryRegion *mr = opaque;
-
- if (!mr->warning_printed) {
- fprintf(stderr, "Invalid write to memory region %s\n", mr->name);
- mr->warning_printed = true;
- }
-}
-
-static const MemoryRegionOps reservation_ops = {
- .read = invalid_read,
- .write = invalid_write,
- .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
void memory_region_init_reservation(MemoryRegion *mr,
const char *name,
uint64_t size)
{
- memory_region_init_io(mr, &reservation_ops, mr, name, size);
+ memory_region_init_io(mr, &unassigned_mem_ops, mr, name, size);
}
void memory_region_destroy(MemoryRegion *mr)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 09/22] memory: assign MemoryRegionOps to all regions
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (7 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 08/22] memory: move unassigned_mem_ops to memory.c Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 10/22] exec: expect mr->ops to be initialized for ROM Paolo Bonzini
` (13 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
This allows to remove the checks on section->readonly. Simply,
write accesses to ROM will not be considered "direct" and will
go through mr->ops without any special intervention.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
memory.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/memory.c b/memory.c
index 2e4f547..f2135d1 100644
--- a/memory.c
+++ b/memory.c
@@ -788,7 +788,8 @@ void memory_region_init(MemoryRegion *mr,
const char *name,
uint64_t size)
{
- mr->ops = NULL;
+ mr->ops = &unassigned_mem_ops;
+ mr->opaque = NULL;
mr->parent = NULL;
mr->size = int128_make64(size);
if (size == UINT64_MAX) {
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 10/22] exec: expect mr->ops to be initialized for ROM
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (8 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 09/22] memory: assign MemoryRegionOps to all regions Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 11/22] exec: introduce memory_access_is_direct Paolo Bonzini
` (12 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
There is no need to use the special phys_section_rom section.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/exec.c b/exec.c
index 03003b2..f6cebd6 100644
--- a/exec.c
+++ b/exec.c
@@ -2335,9 +2335,6 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val)
section = address_space_translate(&address_space_memory, addr, &addr1, &l,
true);
if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
- if (memory_region_is_ram(section->mr)) {
- section = &phys_sections[phys_section_rom];
- }
io_mem_write(section->mr, addr1, val, 4);
} else {
addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
@@ -2368,9 +2365,6 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val,
section = address_space_translate(&address_space_memory, addr, &addr1, &l,
true);
if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
- if (memory_region_is_ram(section->mr)) {
- section = &phys_sections[phys_section_rom];
- }
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap32(val);
@@ -2434,9 +2428,6 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val,
section = address_space_translate(&address_space_memory, addr, &addr1, &l,
true);
if (l < 2 || !memory_region_is_ram(section->mr) || section->readonly) {
- if (memory_region_is_ram(section->mr)) {
- section = &phys_sections[phys_section_rom];
- }
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap16(val);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 11/22] exec: introduce memory_access_is_direct
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (9 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 10/22] exec: expect mr->ops to be initialized for ROM Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 12/22] exec: introduce memory_access_size Paolo Bonzini
` (11 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
After the previous patches, this is a common test for all read/write
functions.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 39 ++++++++++++++++++++++-----------------
1 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/exec.c b/exec.c
index f6cebd6..881f6ec 100644
--- a/exec.c
+++ b/exec.c
@@ -1856,6 +1856,18 @@ static void invalidate_and_set_dirty(hwaddr addr,
xen_modified_memory(addr, length);
}
+static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
+{
+ if (memory_region_is_ram(mr)) {
+ return !(is_write && mr->readonly);
+ }
+ if (memory_region_is_romd(mr)) {
+ return !is_write;
+ }
+
+ return false;
+}
+
void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
int len, bool is_write)
{
@@ -1870,7 +1882,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
section = address_space_translate(as, addr, &addr1, &l, is_write);
if (is_write) {
- if (!memory_region_is_ram(section->mr)) {
+ if (!memory_access_is_direct(section->mr, is_write)) {
/* XXX: could force cpu_single_env to NULL to avoid
potential bugs */
if (l >= 4 && ((addr1 & 3) == 0)) {
@@ -1889,7 +1901,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
io_mem_write(section->mr, addr1, val, 1);
l = 1;
}
- } else if (!section->readonly) {
+ } else {
addr1 += memory_region_get_ram_addr(section->mr);
/* RAM case */
ptr = qemu_get_ram_ptr(addr1);
@@ -1897,8 +1909,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
invalidate_and_set_dirty(addr1, l);
}
} else {
- if (!(memory_region_is_ram(section->mr) ||
- memory_region_is_romd(section->mr))) {
+ if (!memory_access_is_direct(section->mr, is_write)) {
/* I/O case */
if (l >= 4 && ((addr1 & 3) == 0)) {
/* 32 bit read access */
@@ -2053,7 +2064,7 @@ void *address_space_map(AddressSpace *as,
l = len;
section = address_space_translate(as, addr, &xlat, &l, is_write);
- if (!(memory_region_is_ram(section->mr) && !section->readonly)) {
+ if (!memory_access_is_direct(section->mr, is_write)) {
if (todo || bounce.buffer) {
break;
}
@@ -2143,9 +2154,7 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
section = address_space_translate(&address_space_memory, addr, &addr1, &l,
false);
- if (l < 4 ||
- !(memory_region_is_ram(section->mr) ||
- memory_region_is_romd(section->mr))) {
+ if (l < 4 || !memory_access_is_direct(section->mr, false)) {
/* I/O case */
val = io_mem_read(section->mr, addr1, 4);
#if defined(TARGET_WORDS_BIGENDIAN)
@@ -2204,9 +2213,7 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
section = address_space_translate(&address_space_memory, addr, &addr1, &l,
false);
- if (l < 8 ||
- !(memory_region_is_ram(section->mr) ||
- memory_region_is_romd(section->mr))) {
+ if (l < 8 || !memory_access_is_direct(section->mr, false)) {
/* I/O case */
/* XXX This is broken when device endian != cpu endian.
@@ -2273,9 +2280,7 @@ static inline uint32_t lduw_phys_internal(hwaddr addr,
section = address_space_translate(&address_space_memory, addr, &addr1, &l,
false);
- if (l < 2 ||
- !(memory_region_is_ram(section->mr) ||
- memory_region_is_romd(section->mr))) {
+ if (l < 2 || !memory_access_is_direct(section->mr, false)) {
/* I/O case */
val = io_mem_read(section->mr, addr1, 2);
#if defined(TARGET_WORDS_BIGENDIAN)
@@ -2334,7 +2339,7 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val)
section = address_space_translate(&address_space_memory, addr, &addr1, &l,
true);
- if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
+ if (l < 4 || !memory_access_is_direct(section->mr, true)) {
io_mem_write(section->mr, addr1, val, 4);
} else {
addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
@@ -2364,7 +2369,7 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val,
section = address_space_translate(&address_space_memory, addr, &addr1, &l,
true);
- if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
+ if (l < 4 || !memory_access_is_direct(section->mr, true)) {
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap32(val);
@@ -2427,7 +2432,7 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val,
section = address_space_translate(&address_space_memory, addr, &addr1, &l,
true);
- if (l < 2 || !memory_region_is_ram(section->mr) || section->readonly) {
+ if (l < 2 || !memory_access_is_direct(section->mr, true)) {
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap16(val);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 12/22] exec: introduce memory_access_size
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (10 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 11/22] exec: introduce memory_access_is_direct Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 13/22] memory: export memory_region_access_valid to exec.c Paolo Bonzini
` (10 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
This will be used by address_space_access_valid too.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 27 +++++++++++++++++----------
1 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/exec.c b/exec.c
index 881f6ec..d640d07 100644
--- a/exec.c
+++ b/exec.c
@@ -1868,6 +1868,17 @@ static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
return false;
}
+static inline int memory_access_size(int l, hwaddr addr)
+{
+ if (l >= 4 && ((addr & 3) == 0)) {
+ return 4;
+ }
+ if (l >= 2 && ((addr & 1) == 0)) {
+ return 2;
+ }
+ return 1;
+}
+
void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
int len, bool is_write)
{
@@ -1883,23 +1894,21 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
if (is_write) {
if (!memory_access_is_direct(section->mr, is_write)) {
+ l = memory_access_size(l, addr1);
/* XXX: could force cpu_single_env to NULL to avoid
potential bugs */
- if (l >= 4 && ((addr1 & 3) == 0)) {
+ if (l == 4) {
/* 32 bit write access */
val = ldl_p(buf);
io_mem_write(section->mr, addr1, val, 4);
- l = 4;
- } else if (l >= 2 && ((addr1 & 1) == 0)) {
+ } else if (l == 2) {
/* 16 bit write access */
val = lduw_p(buf);
io_mem_write(section->mr, addr1, val, 2);
- l = 2;
} else {
/* 8 bit write access */
val = ldub_p(buf);
io_mem_write(section->mr, addr1, val, 1);
- l = 1;
}
} else {
addr1 += memory_region_get_ram_addr(section->mr);
@@ -1911,21 +1920,19 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
} else {
if (!memory_access_is_direct(section->mr, is_write)) {
/* I/O case */
- if (l >= 4 && ((addr1 & 3) == 0)) {
+ l = memory_access_size(l, addr1);
+ if (l == 4) {
/* 32 bit read access */
val = io_mem_read(section->mr, addr1, 4);
stl_p(buf, val);
- l = 4;
- } else if (l >= 2 && ((addr1 & 1) == 0)) {
+ } else if (l == 2) {
/* 16 bit read access */
val = io_mem_read(section->mr, addr1, 2);
stw_p(buf, val);
- l = 2;
} else {
/* 8 bit read access */
val = io_mem_read(section->mr, addr1, 1);
stb_p(buf, val);
- l = 1;
}
} else {
/* RAM case */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 13/22] memory: export memory_region_access_valid to exec.c
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (11 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 12/22] exec: introduce memory_access_size Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 14/22] exec: implement .valid.accepts for subpages Paolo Bonzini
` (9 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
We'll use it to implement address_space_access_valid.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/exec/memory-internal.h | 3 +++
memory.c | 8 ++++----
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index c18b36c..799c02a 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -45,6 +45,9 @@ void address_space_destroy_dispatch(AddressSpace *as);
extern const MemoryRegionOps unassigned_mem_ops;
+bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr,
+ unsigned size, bool is_write);
+
ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
MemoryRegion *mr);
ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr);
diff --git a/memory.c b/memory.c
index f2135d1..9e1c1a3 100644
--- a/memory.c
+++ b/memory.c
@@ -851,10 +851,10 @@ const MemoryRegionOps unassigned_mem_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static bool memory_region_access_valid(MemoryRegion *mr,
- hwaddr addr,
- unsigned size,
- bool is_write)
+bool memory_region_access_valid(MemoryRegion *mr,
+ hwaddr addr,
+ unsigned size,
+ bool is_write)
{
if (mr->ops->valid.accepts
&& !mr->ops->valid.accepts(mr->opaque, addr, size, is_write)) {
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 14/22] exec: implement .valid.accepts for subpages
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (12 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 13/22] memory: export memory_region_access_valid to exec.c Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 15/22] memory: add address_space_access_valid Paolo Bonzini
` (8 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/exec.c b/exec.c
index d640d07..ab4b4d2 100644
--- a/exec.c
+++ b/exec.c
@@ -1558,9 +1558,29 @@ static void subpage_write(void *opaque, hwaddr addr,
io_mem_write(section->mr, addr, value, len);
}
+static bool subpage_accepts(void *opaque, hwaddr addr,
+ unsigned size, bool is_write)
+{
+ subpage_t *mmio = opaque;
+ unsigned int idx = SUBPAGE_IDX(addr);
+ MemoryRegionSection *section;
+#if defined(DEBUG_SUBPAGE)
+ printf("%s: subpage %p %c len %d addr " TARGET_FMT_plx
+ " idx %d\n", __func__, mmio,
+ is_write ? 'w' : 'r', len, addr, idx);
+#endif
+
+ section = &phys_sections[mmio->sub_section[idx]];
+ addr += mmio->base;
+ addr -= section->offset_within_address_space;
+ addr += section->offset_within_region;
+ return memory_region_access_valid(section->mr, addr, size, is_write);
+}
+
static const MemoryRegionOps subpage_ops = {
.read = subpage_read,
.write = subpage_write,
+ .valid.accepts = subpage_accepts,
.endianness = DEVICE_NATIVE_ENDIAN,
};
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 15/22] memory: add address_space_access_valid
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (13 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 14/22] exec: implement .valid.accepts for subpages Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 16/22] memory: accept mismatching sizes in memory_region_access_valid Paolo Bonzini
` (7 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
The old-style IOMMU lets you check whether an access is valid in a
given DMAContext. There is no equivalent for AddressSpace in the
memory API, implement it with a lookup of the dispatch tree.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
dma-helpers.c | 5 +++++
exec.c | 21 +++++++++++++++++++++
include/exec/memory.h | 15 +++++++++++++++
include/sysemu/dma.h | 3 ++-
4 files changed, 43 insertions(+), 1 deletions(-)
diff --git a/dma-helpers.c b/dma-helpers.c
index 272632f..2e298b6 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -298,6 +298,11 @@ bool iommu_dma_memory_valid(DMAContext *dma, dma_addr_t addr, dma_addr_t len,
plen = len;
}
+ if (!address_space_access_valid(dma->as, paddr, len,
+ dir == DMA_DIRECTION_FROM_DEVICE)) {
+ return false;
+ }
+
len -= plen;
addr += plen;
}
diff --git a/exec.c b/exec.c
index ab4b4d2..1c4c466 100644
--- a/exec.c
+++ b/exec.c
@@ -2067,6 +2067,27 @@ static void cpu_notify_map_clients(void)
}
}
+bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_write)
+{
+ MemoryRegionSection *section;
+ hwaddr l, xlat;
+
+ while (len > 0) {
+ l = len;
+ section = address_space_translate(as, addr, &xlat, &l, is_write);
+ if (!memory_access_is_direct(section->mr, is_write)) {
+ l = memory_access_size(l, addr);
+ if (!memory_region_access_valid(section->mr, xlat, l, is_write)) {
+ return false;
+ }
+ }
+
+ len -= l;
+ addr += l;
+ }
+ return true;
+}
+
/* Map a physical memory region into a host virtual address.
* May map a subset of the requested range, given by and returned in *plen.
* May return NULL if resources needed to perform the mapping are exhausted.
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 688d3f0..81e0e41 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -866,6 +866,21 @@ MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
hwaddr *xlat, hwaddr *len,
bool is_write);
+/* address_space_access_valid: check for validity of accessing an address
+ * space range
+ *
+ * Check whether memory is assigned to the given address space range.
+ *
+ * For now, addr and len should be aligned to a page size. This limitation
+ * will be lifted in the future.
+ *
+ * @as: #AddressSpace to be accessed
+ * @addr: address within that address space
+ * @len: length of the area to be checked
+ * @is_write: indicates the transfer direction
+ */
+bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_write);
+
/* address_space_map: map a physical memory region into a host virtual address
*
* May map a subset of the requested range, given by and returned in @plen.
diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
index a52c93a..02e0dcd 100644
--- a/include/sysemu/dma.h
+++ b/include/sysemu/dma.h
@@ -113,7 +113,8 @@ static inline bool dma_memory_valid(DMAContext *dma,
DMADirection dir)
{
if (!dma_has_iommu(dma)) {
- return true;
+ return address_space_access_valid(dma->as, addr, len,
+ dir == DMA_DIRECTION_FROM_DEVICE);
} else {
return iommu_dma_memory_valid(dma, addr, len, dir);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 16/22] memory: accept mismatching sizes in memory_region_access_valid
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (14 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 15/22] memory: add address_space_access_valid Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 17/22] memory: add big endian support to access_with_adjusted_size Paolo Bonzini
` (6 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
The memory API is able to use smaller/wider accesses than requested,
match that in memory_region_access_valid. Of course, the accepts
callback is still free to reject those accesses.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
memory.c | 29 ++++++++++++++++++++---------
1 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/memory.c b/memory.c
index 9e1c1a3..c72f56d 100644
--- a/memory.c
+++ b/memory.c
@@ -856,24 +856,35 @@ bool memory_region_access_valid(MemoryRegion *mr,
unsigned size,
bool is_write)
{
- if (mr->ops->valid.accepts
- && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write)) {
- return false;
- }
+ int access_size_min, access_size_max;
+ int access_size, i;
if (!mr->ops->valid.unaligned && (addr & (size - 1))) {
return false;
}
- /* Treat zero as compatibility all valid */
- if (!mr->ops->valid.max_access_size) {
+ if (!mr->ops->valid.accepts) {
return true;
}
- if (size > mr->ops->valid.max_access_size
- || size < mr->ops->valid.min_access_size) {
- return false;
+ access_size_min = mr->ops->valid.min_access_size;
+ if (!mr->ops->valid.min_access_size) {
+ access_size_min = 1;
+ }
+
+ access_size_max = mr->ops->valid.max_access_size;
+ if (!mr->ops->valid.max_access_size) {
+ access_size_max = 4;
+ }
+
+ access_size = MAX(MIN(size, access_size_max), access_size_min);
+ for (i = 0; i < size; i += access_size) {
+ if (!mr->ops->valid.accepts(mr->opaque, addr + i, access_size,
+ is_write)) {
+ return false;
+ }
}
+
return true;
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 17/22] memory: add big endian support to access_with_adjusted_size
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (15 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 16/22] memory: accept mismatching sizes in memory_region_access_valid Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 18/22] memory: split accesses even when the old MMIO callbacks are used Paolo Bonzini
` (5 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
This will be used to split 8-byte access down to two four-byte accesses.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
memory.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/memory.c b/memory.c
index c72f56d..679bd8d 100644
--- a/memory.c
+++ b/memory.c
@@ -362,8 +362,12 @@ static void access_with_adjusted_size(hwaddr addr,
access_size = MAX(MIN(size, access_size_max), access_size_min);
access_mask = -1ULL >> (64 - access_size * 8);
for (i = 0; i < size; i += access_size) {
- /* FIXME: big-endian support */
+#ifdef TARGET_WORDS_BIGENDIAN
+ access(opaque, addr + i, value, access_size,
+ (size - access_size - i) * 8, access_mask);
+#else
access(opaque, addr + i, value, access_size, i * 8, access_mask);
+#endif
}
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 18/22] memory: split accesses even when the old MMIO callbacks are used
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (16 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 17/22] memory: add big endian support to access_with_adjusted_size Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 19/22] memory: correctly handle endian-swapped 64-bit accesses Paolo Bonzini
` (4 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
This is useful for 64-bit memory accesses.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
memory.c | 63 +++++++++++++++++++++++++++++++++++++++++++++----------------
1 files changed, 46 insertions(+), 17 deletions(-)
diff --git a/memory.c b/memory.c
index 679bd8d..ca27102 100644
--- a/memory.c
+++ b/memory.c
@@ -302,6 +302,20 @@ static void flatview_simplify(FlatView *view)
}
}
+static void memory_region_oldmmio_read_accessor(void *opaque,
+ hwaddr addr,
+ uint64_t *value,
+ unsigned size,
+ unsigned shift,
+ uint64_t mask)
+{
+ MemoryRegion *mr = opaque;
+ uint64_t tmp;
+
+ tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
+ *value |= (tmp & mask) << shift;
+}
+
static void memory_region_read_accessor(void *opaque,
hwaddr addr,
uint64_t *value,
@@ -319,6 +333,20 @@ static void memory_region_read_accessor(void *opaque,
*value |= (tmp & mask) << shift;
}
+static void memory_region_oldmmio_write_accessor(void *opaque,
+ hwaddr addr,
+ uint64_t *value,
+ unsigned size,
+ unsigned shift,
+ uint64_t mask)
+{
+ MemoryRegion *mr = opaque;
+ uint64_t tmp;
+
+ tmp = (*value >> shift) & mask;
+ mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp);
+}
+
static void memory_region_write_accessor(void *opaque,
hwaddr addr,
uint64_t *value,
@@ -359,6 +387,8 @@ static void access_with_adjusted_size(hwaddr addr,
if (!access_size_max) {
access_size_max = 4;
}
+
+ /* FIXME: support unaligned access? */
access_size = MAX(MIN(size, access_size_max), access_size_min);
access_mask = -1ULL >> (64 - access_size * 8);
for (i = 0; i < size; i += access_size) {
@@ -902,16 +932,16 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
return unassigned_mem_read(mr, addr, size);
}
- if (!mr->ops->read) {
- return mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
+ if (mr->ops->read) {
+ access_with_adjusted_size(addr, &data, size,
+ mr->ops->impl.min_access_size,
+ mr->ops->impl.max_access_size,
+ memory_region_read_accessor, mr);
+ } else {
+ access_with_adjusted_size(addr, &data, size, 1, 4,
+ memory_region_oldmmio_read_accessor, mr);
}
- /* FIXME: support unaligned access */
- access_with_adjusted_size(addr, &data, size,
- mr->ops->impl.min_access_size,
- mr->ops->impl.max_access_size,
- memory_region_read_accessor, mr);
-
return data;
}
@@ -956,16 +986,15 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
adjust_endianness(mr, &data, size);
- if (!mr->ops->write) {
- mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, data);
- return;
+ if (mr->ops->write) {
+ access_with_adjusted_size(addr, &data, size,
+ mr->ops->impl.min_access_size,
+ mr->ops->impl.max_access_size,
+ memory_region_write_accessor, mr);
+ } else {
+ access_with_adjusted_size(addr, &data, size, 1, 4,
+ memory_region_oldmmio_write_accessor, mr);
}
-
- /* FIXME: support unaligned access */
- access_with_adjusted_size(addr, &data, size,
- mr->ops->impl.min_access_size,
- mr->ops->impl.max_access_size,
- memory_region_write_accessor, mr);
}
void memory_region_init_io(MemoryRegion *mr,
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 19/22] memory: correctly handle endian-swapped 64-bit accesses
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (17 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 18/22] memory: split accesses even when the old MMIO callbacks are used Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 20/22] exec: just use io_mem_read/io_mem_write for 8-byte I/O accesses Paolo Bonzini
` (3 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 12 +++++++++---
memory.c | 3 +++
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/exec.c b/exec.c
index 1c4c466..453946e 100644
--- a/exec.c
+++ b/exec.c
@@ -2263,9 +2263,6 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
false);
if (l < 8 || !memory_access_is_direct(section->mr, false)) {
/* I/O case */
-
- /* XXX This is broken when device endian != cpu endian.
- Fix and add "endian" variable check */
#ifdef TARGET_WORDS_BIGENDIAN
val = io_mem_read(section->mr, addr1, 4) << 32;
val |= io_mem_read(section->mr, addr1 + 4, 4);
@@ -2273,6 +2270,15 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
val = io_mem_read(section->mr, addr1, 4);
val |= io_mem_read(section->mr, addr1 + 4, 4) << 32;
#endif
+#if defined(TARGET_WORDS_BIGENDIAN)
+ if (endian == DEVICE_LITTLE_ENDIAN) {
+ val = bswap64(val);
+ }
+#else
+ if (endian == DEVICE_BIG_ENDIAN) {
+ val = bswap64(val);
+ }
+#endif
} else {
/* RAM case */
ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
diff --git a/memory.c b/memory.c
index ca27102..f84fc53 100644
--- a/memory.c
+++ b/memory.c
@@ -957,6 +957,9 @@ static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
case 4:
*data = bswap32(*data);
break;
+ case 8:
+ *data = bswap64(*data);
+ break;
default:
abort();
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 20/22] exec: just use io_mem_read/io_mem_write for 8-byte I/O accesses
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (18 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 19/22] memory: correctly handle endian-swapped 64-bit accesses Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 21/22] memory: propagate errors on I/O dispatch Paolo Bonzini
` (2 subsequent siblings)
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
The memory API is able to split it in two 4-byte accesses.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 8 +-------
include/exec/softmmu_template.h | 24 +-----------------------
2 files changed, 2 insertions(+), 30 deletions(-)
diff --git a/exec.c b/exec.c
index 453946e..878f021 100644
--- a/exec.c
+++ b/exec.c
@@ -2263,13 +2263,7 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
false);
if (l < 8 || !memory_access_is_direct(section->mr, false)) {
/* I/O case */
-#ifdef TARGET_WORDS_BIGENDIAN
- val = io_mem_read(section->mr, addr1, 4) << 32;
- val |= io_mem_read(section->mr, addr1 + 4, 4);
-#else
- val = io_mem_read(section->mr, addr1, 4);
- val |= io_mem_read(section->mr, addr1 + 4, 4) << 32;
-#endif
+ val = io_mem_read(section->mr, addr1, 8);
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap64(val);
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index ca91fd0..292ca02 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -63,7 +63,6 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
target_ulong addr,
uintptr_t retaddr)
{
- DATA_TYPE res;
MemoryRegion *mr = iotlb_to_region(physaddr);
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
@@ -73,18 +72,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
}
env->mem_io_vaddr = addr;
-#if SHIFT <= 2
- res = io_mem_read(mr, physaddr, 1 << SHIFT);
-#else
-#ifdef TARGET_WORDS_BIGENDIAN
- res = io_mem_read(mr, physaddr, 4) << 32;
- res |= io_mem_read(mr, physaddr + 4, 4);
-#else
- res = io_mem_read(mr, physaddr, 4);
- res |= io_mem_read(mr, physaddr + 4, 4) << 32;
-#endif
-#endif /* SHIFT > 2 */
- return res;
+ return io_mem_read(mr, physaddr, 1 << SHIFT);
}
/* handle all cases except unaligned access which span two pages */
@@ -221,17 +209,7 @@ static inline void glue(io_write, SUFFIX)(CPUArchState *env,
env->mem_io_vaddr = addr;
env->mem_io_pc = retaddr;
-#if SHIFT <= 2
io_mem_write(mr, physaddr, val, 1 << SHIFT);
-#else
-#ifdef TARGET_WORDS_BIGENDIAN
- io_mem_write(mr, physaddr, (val >> 32), 4);
- io_mem_write(mr, physaddr + 4, (uint32_t)val, 4);
-#else
- io_mem_write(mr, physaddr, (uint32_t)val, 4);
- io_mem_write(mr, physaddr + 4, val >> 32, 4);
-#endif
-#endif /* SHIFT > 2 */
}
void glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env,
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 21/22] memory: propagate errors on I/O dispatch
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (19 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 20/22] exec: just use io_mem_read/io_mem_write for 8-byte I/O accesses Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 22/22] memory: add return value to address_space_rw/read/write Paolo Bonzini
2013-06-17 21:18 ` [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Anthony Liguori
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 21 ++++++++++++---------
include/exec/exec-all.h | 6 +++---
include/exec/softmmu_template.h | 4 +++-
memory.c | 35 ++++++++++++++++++-----------------
4 files changed, 36 insertions(+), 30 deletions(-)
diff --git a/exec.c b/exec.c
index 878f021..520d4c4 100644
--- a/exec.c
+++ b/exec.c
@@ -1526,6 +1526,8 @@ static uint64_t subpage_read(void *opaque, hwaddr addr,
{
subpage_t *mmio = opaque;
unsigned int idx = SUBPAGE_IDX(addr);
+ uint64_t val;
+
MemoryRegionSection *section;
#if defined(DEBUG_SUBPAGE)
printf("%s: subpage %p len %d addr " TARGET_FMT_plx " idx %d\n", __func__,
@@ -1536,7 +1538,8 @@ static uint64_t subpage_read(void *opaque, hwaddr addr,
addr += mmio->base;
addr -= section->offset_within_address_space;
addr += section->offset_within_region;
- return io_mem_read(section->mr, addr, len);
+ io_mem_read(section->mr, addr, &val, len);
+ return val;
}
static void subpage_write(void *opaque, hwaddr addr,
@@ -1904,7 +1907,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
{
hwaddr l;
uint8_t *ptr;
- uint32_t val;
+ uint64_t val;
hwaddr addr1;
MemoryRegionSection *section;
@@ -1943,15 +1946,15 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
l = memory_access_size(l, addr1);
if (l == 4) {
/* 32 bit read access */
- val = io_mem_read(section->mr, addr1, 4);
+ io_mem_read(section->mr, addr1, &val, 4);
stl_p(buf, val);
} else if (l == 2) {
/* 16 bit read access */
- val = io_mem_read(section->mr, addr1, 2);
+ io_mem_read(section->mr, addr1, &val, 2);
stw_p(buf, val);
} else {
/* 8 bit read access */
- val = io_mem_read(section->mr, addr1, 1);
+ io_mem_read(section->mr, addr1, &val, 1);
stb_p(buf, val);
}
} else {
@@ -2195,7 +2198,7 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
enum device_endian endian)
{
uint8_t *ptr;
- uint32_t val;
+ uint64_t val;
MemoryRegionSection *section;
hwaddr l = 4;
hwaddr addr1;
@@ -2204,7 +2207,7 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
false);
if (l < 4 || !memory_access_is_direct(section->mr, false)) {
/* I/O case */
- val = io_mem_read(section->mr, addr1, 4);
+ io_mem_read(section->mr, addr1, &val, 4);
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap32(val);
@@ -2263,7 +2266,7 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
false);
if (l < 8 || !memory_access_is_direct(section->mr, false)) {
/* I/O case */
- val = io_mem_read(section->mr, addr1, 8);
+ io_mem_read(section->mr, addr1, &val, 8);
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap64(val);
@@ -2330,7 +2333,7 @@ static inline uint32_t lduw_phys_internal(hwaddr addr,
false);
if (l < 2 || !memory_access_is_direct(section->mr, false)) {
/* I/O case */
- val = io_mem_read(section->mr, addr1, 2);
+ io_mem_read(section->mr, addr1, &val, 2);
#if defined(TARGET_WORDS_BIGENDIAN)
if (endian == DEVICE_LITTLE_ENDIAN) {
val = bswap16(val);
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 6362074..17fde25 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -367,9 +367,9 @@ bool is_tcg_gen_code(uintptr_t pc_ptr);
#if !defined(CONFIG_USER_ONLY)
struct MemoryRegion *iotlb_to_region(hwaddr index);
-uint64_t io_mem_read(struct MemoryRegion *mr, hwaddr addr,
- unsigned size);
-void io_mem_write(struct MemoryRegion *mr, hwaddr addr,
+bool io_mem_read(struct MemoryRegion *mr, hwaddr addr,
+ uint64_t *pvalue, unsigned size);
+bool io_mem_write(struct MemoryRegion *mr, hwaddr addr,
uint64_t value, unsigned size);
void tlb_fill(CPUArchState *env1, target_ulong addr, int is_write, int mmu_idx,
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index 292ca02..8584902 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -63,6 +63,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
target_ulong addr,
uintptr_t retaddr)
{
+ uint64_t val;
MemoryRegion *mr = iotlb_to_region(physaddr);
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
@@ -72,7 +73,8 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
}
env->mem_io_vaddr = addr;
- return io_mem_read(mr, physaddr, 1 << SHIFT);
+ io_mem_read(mr, physaddr, &val, 1 << SHIFT);
+ return val;
}
/* handle all cases except unaligned access which span two pages */
diff --git a/memory.c b/memory.c
index f84fc53..5cb8f4a 100644
--- a/memory.c
+++ b/memory.c
@@ -928,10 +928,6 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
{
uint64_t data = 0;
- if (!memory_region_access_valid(mr, addr, size, false)) {
- return unassigned_mem_read(mr, addr, size);
- }
-
if (mr->ops->read) {
access_with_adjusted_size(addr, &data, size,
mr->ops->impl.min_access_size,
@@ -966,25 +962,29 @@ static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
}
}
-static uint64_t memory_region_dispatch_read(MemoryRegion *mr,
- hwaddr addr,
- unsigned size)
+static bool memory_region_dispatch_read(MemoryRegion *mr,
+ hwaddr addr,
+ uint64_t *pval,
+ unsigned size)
{
- uint64_t ret;
+ if (!memory_region_access_valid(mr, addr, size, false)) {
+ *pval = unassigned_mem_read(mr, addr, size);
+ return true;
+ }
- ret = memory_region_dispatch_read1(mr, addr, size);
- adjust_endianness(mr, &ret, size);
- return ret;
+ *pval = memory_region_dispatch_read1(mr, addr, size);
+ adjust_endianness(mr, pval, size);
+ return false;
}
-static void memory_region_dispatch_write(MemoryRegion *mr,
+static bool memory_region_dispatch_write(MemoryRegion *mr,
hwaddr addr,
uint64_t data,
unsigned size)
{
if (!memory_region_access_valid(mr, addr, size, true)) {
unassigned_mem_write(mr, addr, data, size);
- return;
+ return true;
}
adjust_endianness(mr, &data, size);
@@ -998,6 +998,7 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
access_with_adjusted_size(addr, &data, size, 1, 4,
memory_region_oldmmio_write_accessor, mr);
}
+ return false;
}
void memory_region_init_io(MemoryRegion *mr,
@@ -1650,15 +1651,15 @@ void address_space_destroy(AddressSpace *as)
g_free(as->ioeventfds);
}
-uint64_t io_mem_read(MemoryRegion *mr, hwaddr addr, unsigned size)
+bool io_mem_read(MemoryRegion *mr, hwaddr addr, uint64_t *pval, unsigned size)
{
- return memory_region_dispatch_read(mr, addr, size);
+ return memory_region_dispatch_read(mr, addr, pval, size);
}
-void io_mem_write(MemoryRegion *mr, hwaddr addr,
+bool io_mem_write(MemoryRegion *mr, hwaddr addr,
uint64_t val, unsigned size)
{
- memory_region_dispatch_write(mr, addr, val, size);
+ return memory_region_dispatch_write(mr, addr, val, size);
}
typedef struct MemoryRegionList MemoryRegionList;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 22/22] memory: add return value to address_space_rw/read/write
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (20 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 21/22] memory: propagate errors on I/O dispatch Paolo Bonzini
@ 2013-05-30 21:03 ` Paolo Bonzini
2013-06-17 21:18 ` [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Anthony Liguori
22 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:03 UTC (permalink / raw)
To: qemu-devel
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 34 +++++++++++++++-------------------
include/exec/memory.h | 12 +++++++++---
2 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/exec.c b/exec.c
index 520d4c4..5b8b40d 100644
--- a/exec.c
+++ b/exec.c
@@ -1902,7 +1902,7 @@ static inline int memory_access_size(int l, hwaddr addr)
return 1;
}
-void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
+bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
int len, bool is_write)
{
hwaddr l;
@@ -1910,6 +1910,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
uint64_t val;
hwaddr addr1;
MemoryRegionSection *section;
+ bool error = false;
while (len > 0) {
l = len;
@@ -1923,15 +1924,15 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
if (l == 4) {
/* 32 bit write access */
val = ldl_p(buf);
- io_mem_write(section->mr, addr1, val, 4);
+ error |= io_mem_write(section->mr, addr1, val, 4);
} else if (l == 2) {
/* 16 bit write access */
val = lduw_p(buf);
- io_mem_write(section->mr, addr1, val, 2);
+ error |= io_mem_write(section->mr, addr1, val, 2);
} else {
/* 8 bit write access */
val = ldub_p(buf);
- io_mem_write(section->mr, addr1, val, 1);
+ error |= io_mem_write(section->mr, addr1, val, 1);
}
} else {
addr1 += memory_region_get_ram_addr(section->mr);
@@ -1946,15 +1947,15 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
l = memory_access_size(l, addr1);
if (l == 4) {
/* 32 bit read access */
- io_mem_read(section->mr, addr1, &val, 4);
+ error |= io_mem_read(section->mr, addr1, &val, 4);
stl_p(buf, val);
} else if (l == 2) {
/* 16 bit read access */
- io_mem_read(section->mr, addr1, &val, 2);
+ error |= io_mem_read(section->mr, addr1, &val, 2);
stw_p(buf, val);
} else {
/* 8 bit read access */
- io_mem_read(section->mr, addr1, &val, 1);
+ error |= io_mem_read(section->mr, addr1, &val, 1);
stb_p(buf, val);
}
} else {
@@ -1967,31 +1968,26 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
buf += l;
addr += l;
}
+
+ return error;
}
-void address_space_write(AddressSpace *as, hwaddr addr,
+bool address_space_write(AddressSpace *as, hwaddr addr,
const uint8_t *buf, int len)
{
- address_space_rw(as, addr, (uint8_t *)buf, len, true);
+ return address_space_rw(as, addr, (uint8_t *)buf, len, true);
}
-/**
- * address_space_read: read from an address space.
- *
- * @as: #AddressSpace to be accessed
- * @addr: address within that address space
- * @buf: buffer with the data transferred
- */
-void address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len)
+bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len)
{
- address_space_rw(as, addr, buf, len, false);
+ return address_space_rw(as, addr, buf, len, false);
}
void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
int len, int is_write)
{
- return address_space_rw(&address_space_memory, addr, buf, len, is_write);
+ address_space_rw(&address_space_memory, addr, buf, len, is_write);
}
/* used for ROM loading : can write in RAM and ROM */
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 81e0e41..d53a6a1 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -825,32 +825,38 @@ void address_space_destroy(AddressSpace *as);
/**
* address_space_rw: read from or write to an address space.
*
+ * Return true if the operation hit any unassigned memory.
+ *
* @as: #AddressSpace to be accessed
* @addr: address within that address space
* @buf: buffer with the data transferred
* @is_write: indicates the transfer direction
*/
-void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
+bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
int len, bool is_write);
/**
* address_space_write: write to address space.
*
+ * Return true if the operation hit any unassigned memory.
+ *
* @as: #AddressSpace to be accessed
* @addr: address within that address space
* @buf: buffer with the data transferred
*/
-void address_space_write(AddressSpace *as, hwaddr addr,
+bool address_space_write(AddressSpace *as, hwaddr addr,
const uint8_t *buf, int len);
/**
* address_space_read: read from an address space.
*
+ * Return true if the operation hit any unassigned memory.
+ *
* @as: #AddressSpace to be accessed
* @addr: address within that address space
* @buf: buffer with the data transferred
*/
-void address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len);
+bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len);
/* address_space_translate: translate an address range into an address space
* into a MemoryRegionSection and an address range into that section
--
1.7.4.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts
2013-05-30 21:03 ` [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts Paolo Bonzini
@ 2013-06-01 15:28 ` Blue Swirl
2013-06-03 7:31 ` Paolo Bonzini
2013-07-16 12:28 ` Jan Kiszka
1 sibling, 1 reply; 36+ messages in thread
From: Blue Swirl @ 2013-06-01 15:28 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Thu, May 30, 2013 at 9:03 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This provides the basics for detecting accesses to unassigned memory
> as soon as they happen, and also for a simple implementation of
> address_space_access_valid.
>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> exec.c | 36 ++++++++++++------------------------
> memory.c | 28 ++++++++++++++++++++++++++--
> 2 files changed, 38 insertions(+), 26 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 785eeeb..c5100d6 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
> return ram_addr;
> }
>
> -static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
> - unsigned size)
> +static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
> + unsigned size, bool is_write)
> {
> -#ifdef DEBUG_UNASSIGNED
> - printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
> -#endif
> -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
> - cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
> -#endif
> - return 0;
> -}
> -
> -static void unassigned_mem_write(void *opaque, hwaddr addr,
> - uint64_t val, unsigned size)
> -{
> -#ifdef DEBUG_UNASSIGNED
> - printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
> -#endif
> -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
> - cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
> -#endif
> + return false;
> }
>
> -static const MemoryRegionOps unassigned_mem_ops = {
> - .read = unassigned_mem_read,
> - .write = unassigned_mem_write,
> +const MemoryRegionOps unassigned_mem_ops = {
> + .valid.accepts = unassigned_mem_accepts,
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> @@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
> tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
> }
>
> +static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
> + unsigned size, bool is_write)
> +{
> + return is_write;
> +}
> +
> static const MemoryRegionOps notdirty_mem_ops = {
> - .read = unassigned_mem_read,
> .write = notdirty_mem_write,
> + .valid.accepts = notdirty_mem_accepts,
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> diff --git a/memory.c b/memory.c
> index 99f046d..15da877 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr,
> mr->flush_coalesced_mmio = false;
> }
>
> +static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
> + unsigned size)
> +{
> +#ifdef DEBUG_UNASSIGNED
> + printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
> +#endif
> +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
This means that memory.c is getting knowledge about CPU types and it
becomes more specific to current target. I think memory.c should be
generic and target agnostic (maybe one day compiled just once) with
exec.c implementing the target specific functionality.
> +#endif
> + return 0;
> +}
> +
> +static void unassigned_mem_write(void *opaque, hwaddr addr,
> + uint64_t val, unsigned size)
> +{
> +#ifdef DEBUG_UNASSIGNED
> + printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
> +#endif
> +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
> +#endif
> +}
> +
> static bool memory_region_access_valid(MemoryRegion *mr,
> hwaddr addr,
> unsigned size,
> @@ -847,7 +870,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
> uint64_t data = 0;
>
> if (!memory_region_access_valid(mr, addr, size, false)) {
> - return -1U; /* FIXME: better signalling */
> + return unassigned_mem_read(mr, addr, size);
> }
>
> if (!mr->ops->read) {
> @@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
> unsigned size)
> {
> if (!memory_region_access_valid(mr, addr, size, true)) {
> - return; /* FIXME: better signalling */
> + unassigned_mem_write(mr, addr, data, size);
> + return;
> }
>
> adjust_endianness(mr, &data, size);
> --
> 1.7.4.1
>
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts
2013-06-01 15:28 ` Blue Swirl
@ 2013-06-03 7:31 ` Paolo Bonzini
2013-06-03 10:14 ` Andreas Färber
0 siblings, 1 reply; 36+ messages in thread
From: Paolo Bonzini @ 2013-06-03 7:31 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel, Andreas Färber
Il 01/06/2013 17:28, Blue Swirl ha scritto:
> This means that memory.c is getting knowledge about CPU types and it
> becomes more specific to current target. I think memory.c should be
> generic and target agnostic (maybe one day compiled just once) with
> exec.c implementing the target specific functionality.
You're right, but the solution is simply to make cpu_unassigned_access a
CPU method.
Paolo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts
2013-06-03 7:31 ` Paolo Bonzini
@ 2013-06-03 10:14 ` Andreas Färber
0 siblings, 0 replies; 36+ messages in thread
From: Andreas Färber @ 2013-06-03 10:14 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Blue Swirl, qemu-devel
Am 03.06.2013 09:31, schrieb Paolo Bonzini:
> Il 01/06/2013 17:28, Blue Swirl ha scritto:
>> This means that memory.c is getting knowledge about CPU types and it
>> becomes more specific to current target. I think memory.c should be
>> generic and target agnostic (maybe one day compiled just once) with
>> exec.c implementing the target specific functionality.
>
> You're right, but the solution is simply to make cpu_unassigned_access a
> CPU method.
I already have a patch queued to that effect on qom-cpu-10 branch that I
needed to rebase after your merge. :)
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
` (21 preceding siblings ...)
2013-05-30 21:03 ` [Qemu-devel] [PATCH 22/22] memory: add return value to address_space_rw/read/write Paolo Bonzini
@ 2013-06-17 21:18 ` Anthony Liguori
22 siblings, 0 replies; 36+ messages in thread
From: Anthony Liguori @ 2013-06-17 21:18 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
Pulled. Thanks.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 07/22] memory: add address_space_translate
2013-05-30 21:03 ` [Qemu-devel] [PATCH 07/22] memory: add address_space_translate Paolo Bonzini
@ 2013-06-20 13:53 ` Peter Maydell
2013-06-20 14:19 ` Paolo Bonzini
0 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2013-06-20 13:53 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 30 May 2013 22:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
> +MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
> + hwaddr *xlat, hwaddr *plen,
> + bool is_write)
> +{
> + MemoryRegionSection *section;
> + Int128 diff;
> +
> + section = phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS);
> + /* Compute offset within MemoryRegionSection */
> + addr -= section->offset_within_address_space;
> +
> + /* Compute offset within MemoryRegion */
> + *xlat = addr + section->offset_within_region;
> +
> + diff = int128_sub(section->mr->size, int128_make64(addr));
> + *plen = MIN(int128_get64(diff), *plen);
I've just run into a situation where the assertion in
int128_get64() that the value fits into a 64 bit integer
fires. This happened to me for an access to address zero
in the 'unassigned' region:
* io_mem_init() sets the size of these to UINT64_MAX
* memory_region_init() special-cases that size as meaning
2^64, ie {hi=1,lo=0}
* since the addr is zero diff is also {hi=1,lo=0}, and
then int128_get64() asserts.
There are other places in memory.c which do an int128_get64()
on mr->size, which also look suspicious...
-- PMM
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 07/22] memory: add address_space_translate
2013-06-20 13:53 ` Peter Maydell
@ 2013-06-20 14:19 ` Paolo Bonzini
2013-06-20 14:43 ` Peter Maydell
0 siblings, 1 reply; 36+ messages in thread
From: Paolo Bonzini @ 2013-06-20 14:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel
Il 20/06/2013 15:53, Peter Maydell ha scritto:
> On 30 May 2013 22:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> +MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
>> + hwaddr *xlat, hwaddr *plen,
>> + bool is_write)
>> +{
>> + MemoryRegionSection *section;
>> + Int128 diff;
>> +
>> + section = phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS);
>> + /* Compute offset within MemoryRegionSection */
>> + addr -= section->offset_within_address_space;
>> +
>> + /* Compute offset within MemoryRegion */
>> + *xlat = addr + section->offset_within_region;
>> +
>> + diff = int128_sub(section->mr->size, int128_make64(addr));
>> + *plen = MIN(int128_get64(diff), *plen);
>
> I've just run into a situation where the assertion in
> int128_get64() that the value fits into a 64 bit integer
> fires. This happened to me for an access to address zero
> in the 'unassigned' region:
> * io_mem_init() sets the size of these to UINT64_MAX
> * memory_region_init() special-cases that size as meaning
> 2^64, ie {hi=1,lo=0}
> * since the addr is zero diff is also {hi=1,lo=0}, and
> then int128_get64() asserts.
This would be fixed by:
*plen = int128_get64(int128_min(diff, int128_make64(*plen)));
(We can optimize int128 so that it produces more than decent code for this).
> There are other places in memory.c which do an int128_get64()
> on mr->size, which also look suspicious...
They are all on I/O regions so they are safe, except those
in "info mtree". I'm going to squash this:
diff --git a/memory.c b/memory.c
index c500d8d..47b005a 100644
--- a/memory.c
+++ b/memory.c
@@ -1744,7 +1744,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
"-" TARGET_FMT_plx "\n",
base + mr->addr,
base + mr->addr
- + (hwaddr)int128_get64(mr->size) - 1,
+ + (hwaddr)int128_get64(int128_sub(mr->size, int128_make64(1))),
mr->priority,
mr->romd_mode ? 'R' : '-',
!mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
@@ -1759,7 +1759,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %c%c): %s\n",
base + mr->addr,
base + mr->addr
- + (hwaddr)int128_get64(mr->size) - 1,
+ + (hwaddr)int128_get64(int128_sub(mr->size, int128_make64(1))),
mr->priority,
mr->romd_mode ? 'R' : '-',
!mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
Paolo
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 07/22] memory: add address_space_translate
2013-06-20 14:19 ` Paolo Bonzini
@ 2013-06-20 14:43 ` Peter Maydell
2013-06-20 14:53 ` Paolo Bonzini
0 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2013-06-20 14:43 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 20 June 2013 15:19, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 20/06/2013 15:53, Peter Maydell ha scritto:
>> There are other places in memory.c which do an int128_get64()
>> on mr->size, which also look suspicious...
>
> They are all on I/O regions so they are safe
Not entirely sure I understand this. There's no particular
reason I can't create a 2^64 sized I/O memory region
and put it in an address space, is there?
thanks
-- PMM
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 07/22] memory: add address_space_translate
2013-06-20 14:43 ` Peter Maydell
@ 2013-06-20 14:53 ` Paolo Bonzini
0 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2013-06-20 14:53 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel
Il 20/06/2013 16:43, Peter Maydell ha scritto:
>>> >> There are other places in memory.c which do an int128_get64()
>>> >> on mr->size, which also look suspicious...
>> >
>> > They are all on I/O regions so they are safe
> Not entirely sure I understand this. There's no particular
> reason I can't create a 2^64 sized I/O memory region
> and put it in an address space, is there?
I think there are problems in the core if you do that (probably part of
it is fixed now). Still, in cases like this:
memory_region_add_coalescing(mr, 0, int128_get64(mr->size));
the API simply doesn't support it.
Paolo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts
2013-05-30 21:03 ` [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts Paolo Bonzini
2013-06-01 15:28 ` Blue Swirl
@ 2013-07-16 12:28 ` Jan Kiszka
2013-07-16 12:33 ` Paolo Bonzini
1 sibling, 1 reply; 36+ messages in thread
From: Jan Kiszka @ 2013-07-16 12:28 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Stefan Hajnoczi
On 2013-05-30 23:03, Paolo Bonzini wrote:
> This provides the basics for detecting accesses to unassigned memory
> as soon as they happen, and also for a simple implementation of
> address_space_access_valid.
>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> exec.c | 36 ++++++++++++------------------------
> memory.c | 28 ++++++++++++++++++++++++++--
> 2 files changed, 38 insertions(+), 26 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 785eeeb..c5100d6 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
> return ram_addr;
> }
>
> -static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
> - unsigned size)
> +static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
> + unsigned size, bool is_write)
> {
> -#ifdef DEBUG_UNASSIGNED
> - printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
> -#endif
> -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
> - cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
> -#endif
> - return 0;
> -}
> -
> -static void unassigned_mem_write(void *opaque, hwaddr addr,
> - uint64_t val, unsigned size)
> -{
> -#ifdef DEBUG_UNASSIGNED
> - printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
> -#endif
> -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
> - cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
> -#endif
> + return false;
> }
>
> -static const MemoryRegionOps unassigned_mem_ops = {
> - .read = unassigned_mem_read,
> - .write = unassigned_mem_write,
> +const MemoryRegionOps unassigned_mem_ops = {
> + .valid.accepts = unassigned_mem_accepts,
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> @@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
> tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
> }
>
> +static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
> + unsigned size, bool is_write)
> +{
> + return is_write;
> +}
> +
> static const MemoryRegionOps notdirty_mem_ops = {
> - .read = unassigned_mem_read,
> .write = notdirty_mem_write,
> + .valid.accepts = notdirty_mem_accepts,
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> diff --git a/memory.c b/memory.c
> index 99f046d..15da877 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr,
> mr->flush_coalesced_mmio = false;
> }
>
> +static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
> + unsigned size)
> +{
> +#ifdef DEBUG_UNASSIGNED
> + printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
> +#endif
> +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
> +#endif
> + return 0;
This changed the value read from unassigned memory from -1 to 0. Any
particular reason or an unintentional change?
Note that this also breaks unassigned portio accesses, specifically the
case Stefan reported around IPMI access of the Linux kernel.
Jan
> +}
> +
> +static void unassigned_mem_write(void *opaque, hwaddr addr,
> + uint64_t val, unsigned size)
> +{
> +#ifdef DEBUG_UNASSIGNED
> + printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
> +#endif
> +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
> +#endif
> +}
> +
> static bool memory_region_access_valid(MemoryRegion *mr,
> hwaddr addr,
> unsigned size,
> @@ -847,7 +870,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
> uint64_t data = 0;
>
> if (!memory_region_access_valid(mr, addr, size, false)) {
> - return -1U; /* FIXME: better signalling */
> + return unassigned_mem_read(mr, addr, size);
> }
>
> if (!mr->ops->read) {
> @@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
> unsigned size)
> {
> if (!memory_region_access_valid(mr, addr, size, true)) {
> - return; /* FIXME: better signalling */
> + unassigned_mem_write(mr, addr, data, size);
> + return;
> }
>
> adjust_endianness(mr, &data, size);
>
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts
2013-07-16 12:28 ` Jan Kiszka
@ 2013-07-16 12:33 ` Paolo Bonzini
2013-07-16 12:38 ` Peter Maydell
0 siblings, 1 reply; 36+ messages in thread
From: Paolo Bonzini @ 2013-07-16 12:33 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel, Stefan Hajnoczi
Il 16/07/2013 14:28, Jan Kiszka ha scritto:
> On 2013-05-30 23:03, Paolo Bonzini wrote:
>> This provides the basics for detecting accesses to unassigned memory
>> as soon as they happen, and also for a simple implementation of
>> address_space_access_valid.
>>
>> Reviewed-by: Richard Henderson <rth@twiddle.net>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> exec.c | 36 ++++++++++++------------------------
>> memory.c | 28 ++++++++++++++++++++++++++--
>> 2 files changed, 38 insertions(+), 26 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 785eeeb..c5100d6 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
>> return ram_addr;
>> }
>>
>> -static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
>> - unsigned size)
>> +static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
>> + unsigned size, bool is_write)
>> {
>> -#ifdef DEBUG_UNASSIGNED
>> - printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> -#endif
>> -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
>> - cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
>> -#endif
>> - return 0;
>> -}
>> -
>> -static void unassigned_mem_write(void *opaque, hwaddr addr,
>> - uint64_t val, unsigned size)
>> -{
>> -#ifdef DEBUG_UNASSIGNED
>> - printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
>> -#endif
>> -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
>> - cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
>> -#endif
>> + return false;
>> }
>>
>> -static const MemoryRegionOps unassigned_mem_ops = {
>> - .read = unassigned_mem_read,
>> - .write = unassigned_mem_write,
>> +const MemoryRegionOps unassigned_mem_ops = {
>> + .valid.accepts = unassigned_mem_accepts,
>> .endianness = DEVICE_NATIVE_ENDIAN,
>> };
>>
>> @@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
>> tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
>> }
>>
>> +static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
>> + unsigned size, bool is_write)
>> +{
>> + return is_write;
>> +}
>> +
>> static const MemoryRegionOps notdirty_mem_ops = {
>> - .read = unassigned_mem_read,
>> .write = notdirty_mem_write,
>> + .valid.accepts = notdirty_mem_accepts,
>> .endianness = DEVICE_NATIVE_ENDIAN,
>> };
>>
>> diff --git a/memory.c b/memory.c
>> index 99f046d..15da877 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr,
>> mr->flush_coalesced_mmio = false;
>> }
>>
>> +static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
>> + unsigned size)
>> +{
>> +#ifdef DEBUG_UNASSIGNED
>> + printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> +#endif
>> +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
>> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
>> +#endif
>> + return 0;
>
> This changed the value read from unassigned memory from -1 to 0. Any
> particular reason or an unintentional change?
Cut-and-paste (unassigned RAM used to return 0, invalid MMIO used to
return -1, unifying the paths dropped the difference).
I guess unassigned RAM can read as -1 just fine, so we can just change
unassigned_mem_read to return -1.
Paolo
> Jan
>
>> +}
>> +
>> +static void unassigned_mem_write(void *opaque, hwaddr addr,
>> + uint64_t val, unsigned size)
>> +{
>> +#ifdef DEBUG_UNASSIGNED
>> + printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
>> +#endif
>> +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
>> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
>> +#endif
>> +}
>> +
>> static bool memory_region_access_valid(MemoryRegion *mr,
>> hwaddr addr,
>> unsigned size,
>> @@ -847,7 +870,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
>> uint64_t data = 0;
>>
>> if (!memory_region_access_valid(mr, addr, size, false)) {
>> - return -1U; /* FIXME: better signalling */
>> + return unassigned_mem_read(mr, addr, size);
>> }
>>
>> if (!mr->ops->read) {
>> @@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
>> unsigned size)
>> {
>> if (!memory_region_access_valid(mr, addr, size, true)) {
>> - return; /* FIXME: better signalling */
>> + unassigned_mem_write(mr, addr, data, size);
>> + return;
>> }
>>
>> adjust_endianness(mr, &data, size);
>>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts
2013-07-16 12:33 ` Paolo Bonzini
@ 2013-07-16 12:38 ` Peter Maydell
2013-07-16 12:44 ` Jan Kiszka
0 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2013-07-16 12:38 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Jan Kiszka, qemu-devel, Stefan Hajnoczi
On 16 July 2013 13:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 16/07/2013 14:28, Jan Kiszka ha scritto:
>> This changed the value read from unassigned memory from -1 to 0. Any
>> particular reason or an unintentional change?
>
> Cut-and-paste (unassigned RAM used to return 0, invalid MMIO used to
> return -1, unifying the paths dropped the difference).
>
> I guess unassigned RAM can read as -1 just fine, so we can just change
> unassigned_mem_read to return -1.
Behaviour for accesses to unassigned addresses should really
be both bus and CPU specific...
thanks
-- PMM
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts
2013-07-16 12:38 ` Peter Maydell
@ 2013-07-16 12:44 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2013-07-16 12:44 UTC (permalink / raw)
To: Peter Maydell; +Cc: Paolo Bonzini, qemu-devel@nongnu.org, Stefan Hajnoczi
On 2013-07-16 14:38, Peter Maydell wrote:
> On 16 July 2013 13:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 16/07/2013 14:28, Jan Kiszka ha scritto:
>>> This changed the value read from unassigned memory from -1 to 0. Any
>>> particular reason or an unintentional change?
>>
>> Cut-and-paste (unassigned RAM used to return 0, invalid MMIO used to
>> return -1, unifying the paths dropped the difference).
>>
>> I guess unassigned RAM can read as -1 just fine, so we can just change
>> unassigned_mem_read to return -1.
>
> Behaviour for accesses to unassigned addresses should really
> be both bus and CPU specific...
Yes, that's what I was thinking as well when reading it. Still, lets
restore what we did before, then refine.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2013-07-16 12:44 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-30 21:03 [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 01/22] exec: eliminate io_mem_ram Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 02/22] exec: drop useless #if Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 03/22] cputlb: simplify tlb_set_page Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 04/22] exec: make io_mem_unassigned private Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 05/22] exec: do not use error_mem_read Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts Paolo Bonzini
2013-06-01 15:28 ` Blue Swirl
2013-06-03 7:31 ` Paolo Bonzini
2013-06-03 10:14 ` Andreas Färber
2013-07-16 12:28 ` Jan Kiszka
2013-07-16 12:33 ` Paolo Bonzini
2013-07-16 12:38 ` Peter Maydell
2013-07-16 12:44 ` Jan Kiszka
2013-05-30 21:03 ` [Qemu-devel] [PATCH 07/22] memory: add address_space_translate Paolo Bonzini
2013-06-20 13:53 ` Peter Maydell
2013-06-20 14:19 ` Paolo Bonzini
2013-06-20 14:43 ` Peter Maydell
2013-06-20 14:53 ` Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 08/22] memory: move unassigned_mem_ops to memory.c Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 09/22] memory: assign MemoryRegionOps to all regions Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 10/22] exec: expect mr->ops to be initialized for ROM Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 11/22] exec: introduce memory_access_is_direct Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 12/22] exec: introduce memory_access_size Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 13/22] memory: export memory_region_access_valid to exec.c Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 14/22] exec: implement .valid.accepts for subpages Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 15/22] memory: add address_space_access_valid Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 16/22] memory: accept mismatching sizes in memory_region_access_valid Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 17/22] memory: add big endian support to access_with_adjusted_size Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 18/22] memory: split accesses even when the old MMIO callbacks are used Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 19/22] memory: correctly handle endian-swapped 64-bit accesses Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 20/22] exec: just use io_mem_read/io_mem_write for 8-byte I/O accesses Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 21/22] memory: propagate errors on I/O dispatch Paolo Bonzini
2013-05-30 21:03 ` [Qemu-devel] [PATCH 22/22] memory: add return value to address_space_rw/read/write Paolo Bonzini
2013-06-17 21:18 ` [Qemu-devel] [PULL 00/22] Memory/IOMMU patches, part 2: unassigned access detection Anthony Liguori
-- strict thread matches above, loose matches on Subject: below --
2013-05-24 17:05 [Qemu-devel] [PATCH " Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 07/22] memory: add address_space_translate Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).