Patchwork [PULL,3/5] exec: Support 64-bit operations in address_space_rw

login
register
mail settings
Submitter Richard Henderson
Date July 14, 2013, 10:16 p.m.
Message ID <1373840171-25556-4-git-send-email-rth@twiddle.net>
Download mbox | patch
Permalink /patch/258914/
State New
Headers show

Comments

Richard Henderson - July 14, 2013, 10:16 p.m.
Honor the implementation maximum access size, and at least check
the minimum access size.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 exec.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 56 insertions(+), 12 deletions(-)
Markus Armbruster - July 17, 2013, 9:50 a.m.
Richard Henderson <rth@twiddle.net> writes:

> Honor the implementation maximum access size, and at least check
> the minimum access size.
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Richard Henderson <rth@twiddle.net>

Fails for me:

qemu-system-x86_64: /work/armbru/qemu/exec.c:1927: memory_access_size: Assertion `l >= access_size_min' failed.

Workaround: disable USB.

Backtrace:

#3  0x0000003707a2e752 in __assert_fail () from /lib64/libc.so.6
#4  0x00000000006951e3 in memory_access_size (mr=0x1116c00, l=1, addr=12)
    at /work/armbru/qemu/exec.c:1927
#5  0x000000000069529d in address_space_rw (as=0x1045920, addr=49228, buf=
    0x7ffff5392af8 "@\240\377\367L\300", len=1, is_write=true)
    at /work/armbru/qemu/exec.c:1948
#6  0x000000000069566b in address_space_write (as=0x1045920, addr=49228, buf=
    0x7ffff5392af8 "@\240\377\367L\300", len=1)
    at /work/armbru/qemu/exec.c:2027
#7  0x0000000000701356 in cpu_outb (addr=49228, val=64 '@')
    at /work/armbru/qemu/ioport.c:51
#8  0x0000000000705ab1 in kvm_handle_io (port=49228, data=0x7ffff7ffa000, 
    direction=1, size=1, count=1) at /work/armbru/qemu/kvm-all.c:1517
#9  0x000000000070606f in kvm_cpu_exec (cpu=0x10d3c90)
    at /work/armbru/qemu/kvm-all.c:1664
#10 0x0000000000687da7 in qemu_kvm_cpu_thread_fn (arg=0x10d3c90)
    at /work/armbru/qemu/cpus.c:751
#11 0x0000003708607d14 in start_thread () from /lib64/libpthread.so.0
#12 0x0000003707af168d in clone () from /lib64/libc.so.6

Hopefully useful state:

(gdb) p *mr
$1 = {ops = 0x8b0a20, iommu_ops = 0x0, opaque = 0x1116450, owner = 0x0, 
  parent = 0x10c3a10, size = {lo = 32, hi = 0}, addr = 49216, destructor = 
    0x70b2a4 <memory_region_destructor_none>, ram_addr = 18446744073709551615, 
  subpage = false, terminates = true, romd_mode = true, ram = false, 
  readonly = false, enabled = true, rom_device = false, warning_printed = 
    false, flush_coalesced_mmio = false, alias = 0x0, alias_offset = 0, 
  priority = 1, may_overlap = true, subregions = {tqh_first = 0x0, tqh_last = 
    0x1116c78}, subregions_link = {tqe_next = 0x111b2e8, tqe_prev = 
    0x7ffff4a9c928}, coalesced = {tqh_first = 0x0, tqh_last = 0x1116c98}, 
  name = 0x11174e0 "uhci", dirty_log_mask = 0 '\000', ioeventfd_nb = 0, 
  ioeventfds = 0x0, iommu_notify = {notifiers = {lh_first = 0x0}}}
(gdb) p *mr->ops
$2 = {read = 0x5becdf <uhci_port_read>, write = 0x5be8cd <uhci_port_write>, 
  endianness = DEVICE_LITTLE_ENDIAN, valid = {min_access_size = 1, 
    max_access_size = 4, unaligned = false, accepts = 0}, impl = {
    min_access_size = 2, max_access_size = 2, unaligned = false}, old_mmio = {
    read = {0, 0, 0}, write = {0, 0, 0}}}

info mtree
memory
0000000000000000-7ffffffffffffffe (prio 0, RW): system
  0000000000000000-000000001fffffff (prio 0, RW): alias ram-below-4g @pc.ram 0000000000000000-000000001fffffff
  00000000000a0000-00000000000bffff (prio 1, RW): alias smram-region @pci 00000000000a0000-00000000000bffff
  00000000000c0000-00000000000c3fff (prio 1, RW): alias pam-pci @pci 00000000000c0000-00000000000c3fff
  00000000000c4000-00000000000c7fff (prio 1, RW): alias pam-pci @pci 00000000000c4000-00000000000c7fff
  00000000000c8000-00000000000cbfff (prio 1, RW): alias pam-pci @pci 00000000000c8000-00000000000cbfff
  00000000000cc000-00000000000cffff (prio 1, RW): alias pam-pci @pci 00000000000cc000-00000000000cffff
  00000000000d0000-00000000000d3fff (prio 1, RW): alias pam-pci @pci 00000000000d0000-00000000000d3fff
  00000000000d4000-00000000000d7fff (prio 1, RW): alias pam-pci @pci 00000000000d4000-00000000000d7fff
  00000000000d8000-00000000000dbfff (prio 1, RW): alias pam-pci @pci 00000000000d8000-00000000000dbfff
  00000000000dc000-00000000000dffff (prio 1, RW): alias pam-pci @pci 00000000000dc000-00000000000dffff
  00000000000e0000-00000000000e3fff (prio 1, RW): alias pam-pci @pci 00000000000e0000-00000000000e3fff
  00000000000e4000-00000000000e7fff (prio 1, RW): alias pam-pci @pci 00000000000e4000-00000000000e7fff
  00000000000e8000-00000000000ebfff (prio 1, RW): alias pam-pci @pci 00000000000e8000-00000000000ebfff
  00000000000ec000-00000000000effff (prio 1, RW): alias pam-pci @pci 00000000000ec000-00000000000effff
  00000000000f0000-00000000000fffff (prio 1, RW): alias pam-pci @pci 00000000000f0000-00000000000fffff
  0000000020000000-00000000ffffffff (prio 0, RW): alias pci-hole @pci 0000000020000000-00000000ffffffff
  00000000fec00000-00000000fec00fff (prio 0, RW): kvm-ioapic
  00000000fed00000-00000000fed003ff (prio 0, RW): hpet
  00000000fee00000-00000000feefffff (prio 4096, RW): icc-apic-container
    00000000fee00000-00000000feefffff (prio 0, RW): kvm-apic-msi
  0000000100000000-40000000ffffffff (prio 0, RW): alias pci-hole64 @pci 0000000100000000-40000000ffffffff
I/O
0000000000000000-000000000000ffff (prio 0, RW): io
  0000000000000000-0000000000000007 (prio 0, RW): dma-chan
  0000000000000008-000000000000000f (prio 0, RW): dma-cont
  0000000000000020-0000000000000021 (prio 0, RW): kvm-pic
  0000000000000040-0000000000000043 (prio 0, RW): kvm-pit
  0000000000000060-0000000000000060 (prio 0, RW): i8042-data
  0000000000000061-0000000000000061 (prio 0, RW): elcr
  0000000000000064-0000000000000064 (prio 0, RW): i8042-cmd
  0000000000000070-0000000000000071 (prio 0, RW): rtc
  000000000000007e-000000000000007f (prio 0, RW): kvmvapic
  0000000000000080-0000000000000080 (prio 0, RW): ioport80
  0000000000000081-0000000000000083 (prio 0, RW): dma-page
  0000000000000087-0000000000000087 (prio 0, RW): dma-page
  0000000000000089-000000000000008b (prio 0, RW): dma-page
  000000000000008f-000000000000008f (prio 0, RW): dma-page
  0000000000000092-0000000000000092 (prio 0, RW): port92
  00000000000000a0-00000000000000a1 (prio 0, RW): kvm-pic
  00000000000000b2-00000000000000b3 (prio 0, RW): apm-io
  00000000000000c0-00000000000000cf (prio 0, RW): dma-chan
  00000000000000d0-00000000000000df (prio 0, RW): dma-cont
  00000000000000f0-00000000000000f0 (prio 0, RW): ioportF0
  0000000000000170-0000000000000177 (prio 0, RW): ide
  00000000000001f0-00000000000001f7 (prio 0, RW): ide
  0000000000000376-0000000000000376 (prio 0, RW): ide
  00000000000003b0-00000000000003df (prio 0, RW): cirrus-io
  00000000000003f1-00000000000003f5 (prio 0, RW): fdc
  00000000000003f6-00000000000003f6 (prio 0, RW): ide
  00000000000003f7-00000000000003f7 (prio 0, RW): fdc
  00000000000003f8-00000000000003ff (prio 0, RW): serial
  00000000000004d0-00000000000004d0 (prio 0, RW): kvm-elcr
  00000000000004d1-00000000000004d1 (prio 0, RW): kvm-elcr
  0000000000000505-0000000000000505 (prio 0, RW): pvpanic
  0000000000000510-0000000000000511 (prio 0, RW): fwcfg
  0000000000000cf8-0000000000000cfb (prio 0, RW): pci-conf-idx
  0000000000000cf9-0000000000000cf9 (prio 1, RW): piix3-reset-control
  0000000000000cfc-0000000000000cff (prio 0, RW): pci-conf-data
  0000000000005658-0000000000005658 (prio 0, RW): vmport
  000000000000ae00-000000000000ae0e (prio 0, RW): acpi-pci-hotplug
  000000000000af00-000000000000af1f (prio 0, RW): acpi-cpu-hotplug
  000000000000afe0-000000000000afe3 (prio 0, RW): acpi-gpe0
  000000000000b100-000000000000b13f (prio 0, RW): pm-smbus
i440FX
PIIX3
piix3-ide
piix3-usb-uhci
PIIX4_PM
cirrus-vga
e1000
aliases
pc.ram
0000000000000000-000000001fffffff (prio 0, RW): pc.ram
pci
0000000000000000-7ffffffffffffffe (prio 0, RW): pci
  00000000000a0000-00000000000bffff (prio 1, RW): cirrus-lowmem-container
    00000000000a0000-00000000000bffff (prio 0, RW): cirrus-low-memory
  00000000000c0000-00000000000dffff (prio 1, RW): pc.rom
  00000000000e0000-00000000000fffff (prio 1, R-): alias isa-bios @pc.bios 0000000000000000-000000000001ffff
  00000000fffe0000-00000000ffffffff (prio 0, R-): pc.bios
pc.bios
00000000fffe0000-00000000ffffffff (prio 0, R-): pc.bios

Patch

diff --git a/exec.c b/exec.c
index 80ee2ab..c99a883 100644
--- a/exec.c
+++ b/exec.c
@@ -1896,15 +1896,37 @@  static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
     return false;
 }
 
-static inline int memory_access_size(MemoryRegion *mr, int l, hwaddr addr)
+static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
 {
-    if (l >= 4 && (((addr & 3) == 0 || mr->ops->impl.unaligned))) {
-        return 4;
+    unsigned access_size_min = mr->ops->impl.min_access_size;
+    unsigned access_size_max = mr->ops->impl.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;
+    }
+
+    /* Bound the maximum access by the alignment of the address.  */
+    if (!mr->ops->impl.unaligned) {
+        unsigned align_size_max = addr & -addr;
+        if (align_size_max != 0 && align_size_max < access_size_max) {
+            access_size_max = align_size_max;
+        }
     }
-    if (l >= 2 && (((addr & 1) == 0) || mr->ops->impl.unaligned)) {
-        return 2;
+
+    /* Don't attempt accesses larger than the maximum.  */
+    if (l > access_size_max) {
+        l = access_size_max;
     }
-    return 1;
+    /* ??? 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;
 }
 
 bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
@@ -1926,18 +1948,29 @@  bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                 l = memory_access_size(mr, l, addr1);
                 /* XXX: could force current_cpu to NULL to avoid
                    potential bugs */
-                if (l == 4) {
+                switch (l) {
+                case 8:
+                    /* 64 bit write access */
+                    val = ldq_p(buf);
+                    error |= io_mem_write(mr, addr1, val, 8);
+                    break;
+                case 4:
                     /* 32 bit write access */
                     val = ldl_p(buf);
                     error |= io_mem_write(mr, addr1, val, 4);
-                } else if (l == 2) {
+                    break;
+                case 2:
                     /* 16 bit write access */
                     val = lduw_p(buf);
                     error |= io_mem_write(mr, addr1, val, 2);
-                } else {
+                    break;
+                case 1:
                     /* 8 bit write access */
                     val = ldub_p(buf);
                     error |= io_mem_write(mr, addr1, val, 1);
+                    break;
+                default:
+                    abort();
                 }
             } else {
                 addr1 += memory_region_get_ram_addr(mr);
@@ -1950,18 +1983,29 @@  bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
             if (!memory_access_is_direct(mr, is_write)) {
                 /* I/O case */
                 l = memory_access_size(mr, l, addr1);
-                if (l == 4) {
+                switch (l) {
+                case 8:
+                    /* 64 bit read access */
+                    error |= io_mem_read(mr, addr1, &val, 8);
+                    stq_p(buf, val);
+                    break;
+                case 4:
                     /* 32 bit read access */
                     error |= io_mem_read(mr, addr1, &val, 4);
                     stl_p(buf, val);
-                } else if (l == 2) {
+                    break;
+                case 2:
                     /* 16 bit read access */
                     error |= io_mem_read(mr, addr1, &val, 2);
                     stw_p(buf, val);
-                } else {
+                    break;
+                case 1:
                     /* 8 bit read access */
                     error |= io_mem_read(mr, addr1, &val, 1);
                     stb_p(buf, val);
+                    break;
+                default:
+                    abort();
                 }
             } else {
                 /* RAM case */