From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60705) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V00P5-0005a6-T6 for qemu-devel@nongnu.org; Thu, 18 Jul 2013 22:26:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V00P0-0008Ba-9Q for qemu-devel@nongnu.org; Thu, 18 Jul 2013 22:26:43 -0400 Received: from e28smtp07.in.ibm.com ([122.248.162.7]:40855) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V00Oz-0008Aw-If for qemu-devel@nongnu.org; Thu, 18 Jul 2013 22:26:38 -0400 Received: from /spool/local by e28smtp07.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 19 Jul 2013 07:48:37 +0530 Message-ID: <51E8A3C9.7090901@linux.vnet.ibm.com> Date: Fri, 19 Jul 2013 10:26:17 +0800 From: Mike Qiu MIME-Version: 1.0 References: <1373946643-10609-1-git-send-email-qiudayu@linux.vnet.ibm.com> <51E80954.1030708@linux.vnet.ibm.com> <51E82268.3060809@suse.de> In-Reply-To: <51E82268.3060809@suse.de> Content-Type: multipart/alternative; boundary="------------050606080106080108000506" Subject: Re: [Qemu-devel] [PATCH] Bug Fix:Segmentation fault when use usb-ehci device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: agraf@suse.de, aik@ozlabs.ru, qemu-devel@nongnu.org, xiaoguangrong@linux.vnet.ibm.com, qemu-ppc@nongnu.org, Gerd Hoffmann , Paolo Bonzini This is a multi-part message in MIME format. --------------050606080106080108000506 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 于 2013/7/19 1:14, Andreas Färber 写道: > Hi, > > Am 18.07.2013 17:27, schrieb Mike Qiu: >> Hi all >> >> Any comments ? > You should've CCed the USB maintainer whose file you are touching for > review rather than just ppc people, see ./MAINTAINERS. I have CC to the usb naintainer Gerd Hoffmann, his files are hw/usb/*. > > There's some typos in the commit message, but the change looks okay to > me - although there were discussions to catch this on the memory API > side of things instead. You mean this patch: see below: exec: Support 64-bit operations in address_s if so it is very different. BTW, this bug has been opened before? Thanks Mike > > Regards, > Andreas > >> Thanks >> Mike >> 2013/7/16 11:50, Mike Qiu wrote: >>> For usb-ehci in qemu, its caps just has read() operation, >>> the write() operation does not exist. >>> >>> This cause a Segmentation fault when use usb-ehci device in ppc64 >>> platform. >>> >>> here is gdb output: >>> >>> Program received signal SIGSEGV, Segmentation fault. >>> [Switching to Thread 0x3fffa7fcef20 (LWP 6793)] >>> 0x00000000103f5244 in memory_region_oldmmio_write_accessor >>> (opaque=0x113e9e78, addr=9, value=0x3fffa7fce088, >>> size=1, shift=0, mask=255) at /home/Mike/qemu-impreza/memory.c:384 >>> 384 mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp); >>> (gdb) p *mr->ops >>> $1 = {read = @0x10716f68: 0x1020699c , write = 0, >>> endianness = DEVICE_LITTLE_ENDIAN, valid = {min_access_size = 1, >>> max_access_size = 4, unaligned = false, accepts = 0}, impl = >>> {min_access_size = 1, max_access_size = 1, unaligned = false}, >>> old_mmio = {read = {0, 0, 0}, write = {0, 0, 0}}} >>> >>> Becasue function write() of mr->ops has not been implement, in >>> function memory_region_dispatch_write(), it call >>> oldmmio write accessor, but at the same time old_mmio still not >>> been implement by default. >>> >>> That is the root cause of the Segmentation fault. >>> >>> To solve this problem, add empty function: ehci_caps_write() >>> >>> Signed-off-by: Mike Qiu >>> --- >>> hw/usb/hcd-ehci.c | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c >>> index 67e4b24..6c8a439 100644 >>> --- a/hw/usb/hcd-ehci.c >>> +++ b/hw/usb/hcd-ehci.c >>> @@ -1072,6 +1072,12 @@ static void ehci_port_write(void *ptr, hwaddr addr, >>> trace_usb_ehci_portsc_change(addr + s->portscbase, addr >> 2, *portsc, old); >>> } >>> >>> +static void ehci_caps_write(void *ptr, hwaddr addr, uint64_t val, >>> + unsigned size) >>> +{ >>> + /* nothing */ >>> +} >>> + >>> static void ehci_opreg_write(void *ptr, hwaddr addr, >>> uint64_t val, unsigned size) >>> { >>> @@ -2380,6 +2386,7 @@ static void ehci_frame_timer(void *opaque) >>> >>> static const MemoryRegionOps ehci_mmio_caps_ops = { >>> .read = ehci_caps_read, >>> + .write = ehci_caps_write, >>> .valid.min_access_size = 1, >>> .valid.max_access_size = 4, >>> .impl.min_access_size = 1, >> > --------------050606080106080108000506 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit
于 2013/7/19 1:14, Andreas Färber 写道:
Hi,

Am 18.07.2013 17:27, schrieb Mike Qiu:

Hi all

Any comments ?

You should've CCed the USB maintainer whose file you are touching for
review rather than just ppc people, see ./MAINTAINERS.
I have CC to the usb naintainer Gerd Hoffmann, his files are hw/usb/*.


There's some typos in the commit message, but the change looks okay to
me - although there were discussions to catch this on the memory API
side of things instead.
You mean this patch: see below:

exec: Support 64-bit operations in address_s

if so it is very different.

BTW, this bug has been opened before?

Thanks
Mike

Regards,
Andreas


Thanks
Mike
2013/7/16 11:50, Mike Qiu wrote:

For usb-ehci in qemu, its caps just has read() operation,
the write() operation does not exist.

This cause a Segmentation fault when use usb-ehci device in ppc64
platform.

here is gdb output:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x3fffa7fcef20 (LWP 6793)]
0x00000000103f5244 in memory_region_oldmmio_write_accessor
(opaque=0x113e9e78, addr=9, value=0x3fffa7fce088,
size=1, shift=0, mask=255) at /home/Mike/qemu-impreza/memory.c:384
384  mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp);
(gdb) p *mr->ops
$1 = {read = @0x10716f68: 0x1020699c <ehci_caps_read>, write = 0,
   endianness = DEVICE_LITTLE_ENDIAN, valid = {min_access_size = 1,
   max_access_size = 4, unaligned = false, accepts = 0}, impl =
   {min_access_size = 1, max_access_size = 1, unaligned = false},
   old_mmio = {read = {0, 0, 0}, write = {0, 0, 0}}}

Becasue function write() of mr->ops has not been implement, in
function memory_region_dispatch_write(), it call
oldmmio write accessor, but at the same time old_mmio still not
been implement by default.

That is the root cause of the Segmentation fault.

To solve this problem, add empty function: ehci_caps_write()

Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
---
 hw/usb/hcd-ehci.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 67e4b24..6c8a439 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1072,6 +1072,12 @@ static void ehci_port_write(void *ptr, hwaddr addr,
     trace_usb_ehci_portsc_change(addr + s->portscbase, addr >> 2, *portsc, old);
 }

+static void ehci_caps_write(void *ptr, hwaddr addr, uint64_t val,
+                           unsigned size)
+{
+    /* nothing */
+}
+
 static void ehci_opreg_write(void *ptr, hwaddr addr,
                              uint64_t val, unsigned size)
 {
@@ -2380,6 +2386,7 @@ static void ehci_frame_timer(void *opaque)

 static const MemoryRegionOps ehci_mmio_caps_ops = {
     .read = ehci_caps_read,
+    .write = ehci_caps_write,
     .valid.min_access_size = 1,
     .valid.max_access_size = 4,
     .impl.min_access_size = 1,

--------------050606080106080108000506--