Il 14/06/2013 12:32, Nikunj A Dadhania ha scritto:
> Nikunj A Dadhania <nik...@linux.vnet.ibm.com> writes:
>> commit 08521e28c7e6e8cc1f53424a0f845f58d2ed9546
>> Author: Paolo Bonzini <pbonz...@redhat.com>
>> Date: Fri May 24 12:54:01 2013 +0200
>>
>> memory: add big endian support to access_with_adjusted_size
>>
>> This will be used to split 8-byte access down to two four-byte accesses.
>>
>> Reviewed-by: Richard Henderson <r...@twiddle.net>
>> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
>>
>>
>> If I hack the above funniness in my USB EHCI driver, somewhere down the
>> qemu crashes at code introduced by this patch:
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x0000000000000000 in ?? ()
>> (gdb) bt
>> #0 0x0000000000000000 in ?? ()
>> #1 0x00005555557a0ea4 in access_with_adjusted_size (addr=addr@entry=12,
>> value=value@entry=0x7fffd5a86680, size=size@entry=1,
>> access_size_min=<optimized out>, access_size_max=<optimized out>,
>> access=0x5555557a1f80 <memory_region_oldmmio_write_accessor>,
>> opaque=0x5555567f8ab8) at /home/nikunj/work/power/code/qemu/memory.c:396
>> #2 0x00005555557a5ebb in memory_region_dispatch_write (size=1, data=0,
>> addr=12, mr=0x5555567f8ab8) at /home/nikunj/work/power/code/qemu/memory.c:998
>>
>> Reverting this, I can safely boot using a usb-storage device put on ehci
>> controller.
>
> Just reverting this patch does not help though, i will need to figure
> which all commits are bad.
Hi Nikunj,
can you try the attached patch?
Alexey, with some luck it may even fix virtio-blk too.
Paolo
diff --git a/exec.c b/exec.c
index c99a883..c8658c6 100644
--- a/exec.c
+++ b/exec.c
@@ -1379,7 +1379,7 @@ static void *qemu_safe_ram_ptr(ram_addr_t addr)
/* Return a host pointer to guest's ram. Similar to qemu_get_ram_ptr
* but takes a size argument */
-static void *qemu_ram_ptr_length(ram_addr_t addr, ram_addr_t *size)
+static void *qemu_ram_ptr_length(ram_addr_t addr, hwaddr *size)
{
if (*size == 0) {
return NULL;
@@ -1898,14 +1898,10 @@ static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
{
- unsigned access_size_min = mr->ops->impl.min_access_size;
- unsigned access_size_max = mr->ops->impl.max_access_size;
+ unsigned access_size_max = mr->ops->valid.max_access_size;
/* Regions are assumed to support 1-4 byte accesses unless
otherwise specified. */
- if (access_size_min == 0) {
- access_size_min = 1;
- }
if (access_size_max == 0) {
access_size_max = 4;
}
@@ -1922,9 +1918,6 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
if (l > access_size_max) {
l = access_size_max;
}
- /* ??? The users of this function are wrong, not supporting minimums larger
- than the remaining length. C.f. memory.c:access_with_adjusted_size. */
- assert(l >= access_size_min);
return l;
}
diff --git a/memory.c b/memory.c
index c8f9a2b..a8db78c 100644
--- a/memory.c
+++ b/memory.c
@@ -339,65 +339,104 @@ static void flatview_simplify(FlatView *view)
}
}
-static void memory_region_oldmmio_read_accessor(void *opaque,
+static bool memory_region_big_endian(MemoryRegion *mr)
+{
+#ifdef TARGET_WORDS_BIGENDIAN
+ return mr->ops->endianness != DEVICE_LITTLE_ENDIAN;
+#else
+ return mr->ops->endianness == DEVICE_BIG_ENDIAN;
+#endif
+}
+
+static bool memory_region_wrong_endianness(MemoryRegion *mr)
+{
+#ifdef TARGET_WORDS_BIGENDIAN
+ return mr->ops->endianness == DEVICE_LITTLE_ENDIAN;
+#else
+ return mr->ops->endianness == DEVICE_BIG_ENDIAN;
+#endif
+}
+
+static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
+{
+ if (memory_region_wrong_endianness(mr)) {
+ switch (size) {
+ case 1:
+ break;
+ case 2:
+ *data = bswap16(*data);
+ break;
+ case 4:
+ *data = bswap32(*data);
+ break;
+ case 8:
+ *data = bswap64(*data);
+ break;
+ default:
+ abort();
+ }
+ }
+}
+
+static void memory_region_oldmmio_read_accessor(MemoryRegion *mr,
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);
+ adjust_endianness(mr, &tmp, size);
*value |= (tmp & mask) << shift;
}
-static void memory_region_read_accessor(void *opaque,
+static void memory_region_read_accessor(MemoryRegion *mr,
hwaddr addr,
uint64_t *value,
unsigned size,
unsigned shift,
uint64_t mask)
{
- MemoryRegion *mr = opaque;
uint64_t tmp;
if (mr->flush_coalesced_mmio) {
qemu_flush_coalesced_mmio_buffer();
}
tmp = mr->ops->read(mr->opaque, addr, size);
+ adjust_endianness(mr, &tmp, size);
*value |= (tmp & mask) << shift;
}
-static void memory_region_oldmmio_write_accessor(void *opaque,
+static void memory_region_oldmmio_write_accessor(MemoryRegion *mr,
hwaddr addr,
uint64_t *value,
unsigned size,
unsigned shift,
uint64_t mask)
{
- MemoryRegion *mr = opaque;
uint64_t tmp;
tmp = (*value >> shift) & mask;
+ adjust_endianness(mr, &tmp, size);
mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp);
}
-static void memory_region_write_accessor(void *opaque,
+static void memory_region_write_accessor(MemoryRegion *mr,
hwaddr addr,
uint64_t *value,
unsigned size,
unsigned shift,
uint64_t mask)
{
- MemoryRegion *mr = opaque;
uint64_t tmp;
if (mr->flush_coalesced_mmio) {
qemu_flush_coalesced_mmio_buffer();
}
tmp = (*value >> shift) & mask;
+ adjust_endianness(mr, &tmp, size);
mr->ops->write(mr->opaque, addr, tmp, size);
}
@@ -406,13 +445,13 @@ static void access_with_adjusted_size(hwaddr addr,
unsigned size,
unsigned access_size_min,
unsigned access_size_max,
- void (*access)(void *opaque,
+ void (*access)(MemoryRegion *mr,
hwaddr addr,
uint64_t *value,
unsigned size,
unsigned shift,
uint64_t mask),
- void *opaque)
+ MemoryRegion *mr)
{
uint64_t access_mask;
unsigned access_size;
@@ -428,13 +467,15 @@ static void access_with_adjusted_size(hwaddr addr,
/* 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) {
-#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
+ if (memory_region_big_endian(mr)) {
+ for (i = 0; i < size; i += access_size) {
+ access(mr, addr + i, value, access_size,
+ (size - access_size - i) * 8, access_mask);
+ }
+ } else {
+ for (i = 0; i < size; i += access_size) {
+ access(mr, addr + i, value, access_size, i * 8, access_mask);
+ }
}
}
@@ -786,15 +827,6 @@ static void memory_region_destructor_rom_device(MemoryRegion *mr)
qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
}
-static bool memory_region_wrong_endianness(MemoryRegion *mr)
-{
-#ifdef TARGET_WORDS_BIGENDIAN
- return mr->ops->endianness == DEVICE_LITTLE_ENDIAN;
-#else
- return mr->ops->endianness == DEVICE_BIG_ENDIAN;
-#endif
-}
-
void memory_region_init(MemoryRegion *mr,
Object *owner,
const char *name,
@@ -841,7 +872,7 @@ static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
if (current_cpu != NULL) {
cpu_unassigned_access(current_cpu, addr, false, false, 0, size);
}
- return 0;
+ return -1ULL;
}
static void unassigned_mem_write(void *opaque, hwaddr addr,
@@ -922,27 +953,6 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
return data;
}
-static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
-{
- if (memory_region_wrong_endianness(mr)) {
- switch (size) {
- case 1:
- break;
- case 2:
- *data = bswap16(*data);
- break;
- case 4:
- *data = bswap32(*data);
- break;
- case 8:
- *data = bswap64(*data);
- break;
- default:
- abort();
- }
- }
-}
-
static bool memory_region_dispatch_read(MemoryRegion *mr,
hwaddr addr,
uint64_t *pval,
@@ -954,7 +964,6 @@ static bool memory_region_dispatch_read(MemoryRegion *mr,
}
*pval = memory_region_dispatch_read1(mr, addr, size);
- adjust_endianness(mr, pval, size);
return false;
}
@@ -968,8 +977,6 @@ static bool memory_region_dispatch_write(MemoryRegion *mr,
return true;
}
- adjust_endianness(mr, &data, size);
-
if (mr->ops->write) {
access_with_adjusted_size(addr, &data, size,
mr->ops->impl.min_access_size,
