From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52448) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V15hO-0004it-TB for qemu-devel@nongnu.org; Sun, 21 Jul 2013 22:18:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V15hM-0008GD-3S for qemu-devel@nongnu.org; Sun, 21 Jul 2013 22:18:06 -0400 Received: from e23smtp06.au.ibm.com ([202.81.31.148]:52689) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V15hJ-00089K-Oi for qemu-devel@nongnu.org; Sun, 21 Jul 2013 22:18:04 -0400 Received: from /spool/local by e23smtp06.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 22 Jul 2013 12:09:45 +1000 Message-ID: <51EC962E.5000108@linux.vnet.ibm.com> Date: Mon, 22 Jul 2013 10:17:18 +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> <51E8A3C9.7090901@linux.vnet.ibm.com> <51E9494F.6050607@suse.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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: Peter Maydell Cc: agraf@suse.de, aik@ozlabs.ru, qemu-devel@nongnu.org, xiaoguangrong@linux.vnet.ibm.com, qemu-ppc@nongnu.org, Gerd Hoffmann , Paolo Bonzini , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= 于 2013/7/19 22:32, Peter Maydell 写道: > On 19 July 2013 15:12, Andreas Färber wrote: >> No, I don't. There were other segfault avoidance patches like yours over >> the past months - they're all fixing individual segfault symptoms. >> Question for Paolo is whether we want to continue to discover them one >> by one or whether to implement a fallback inside memory code if .read or >> .write is NULL. > I think that the correct behaviour is "if neither > .read nor .oldmmio.read[x] are set then behave as if > .valid.accepts returned false" (ie "device has not responded > to the read access, bus error"). > > That said, if we want to add do-nothing functions instead, > then (a) having memory.c provide a single set of nop functions > that devices can use would be nicer than lots of individual > nop functions and (b) a list to start with: Yes, can we add one step in memory_region_oldmmio_read_accessor() and memory_region_oldmmio_write_accessor() to check if the oldmmio read and write has been implement, if no, for .write function, we just drop and do nothing, for .read function, we drop read and return the value param with 0xFF or other to show read fault. Thus, we do not need to fix the segment fault in special field. Thanks Mike > > $ for f in $(find . -name '*.c'); do perl -e '$s = 0; while (<>) { if > (/MemoryRegionOps (.*) =/) { $n = $1; $s = 1; next; } next if $s == 0; > if (/\.read = /) { $s |= 2; } if (/\.write = /) { $s |= 4; } if (/;/) > { print "$ARGV: $n: missing read\n" unless $s & 2; print "$ARGV: $n: > missing write\n" unless $s & 4; $s = 0; }}' $f; done > ./memory.c: unassigned_mem_ops: missing read > ./memory.c: unassigned_mem_ops: missing write > ./exec.c: notdirty_mem_ops: missing read > ./hw/pci-host/prep.c: PPC_intack_ops: missing write > ./hw/ssi/xilinx_spips.c: lqspi_ops: missing write > ./hw/arm/omap1.c: omap_pwt_ops: missing read > ./hw/arm/musicpal.c: mv88w8618_wlan_ops: missing write > ./hw/scsi/megasas.c: megasas_queue_ops: missing write > ./hw/usb/hcd-ehci.c: ehci_mmio_caps_ops: missing write > ./hw/usb/hcd-uhci.c: uhci_ioport_ops: missing read > ./hw/intc/openpic_kvm.c: kvm_openpic_mem_ops: missing read > ./hw/intc/openpic.c: openpic_glb_ops_le: missing read > ./hw/intc/openpic.c: openpic_glb_ops_be: missing read > ./hw/intc/openpic.c: openpic_tmr_ops_le: missing read > ./hw/intc/openpic.c: openpic_tmr_ops_be: missing read > ./hw/intc/openpic.c: openpic_cpu_ops_le: missing read > ./hw/intc/openpic.c: openpic_cpu_ops_be: missing read > ./hw/intc/openpic.c: openpic_src_ops_le: missing read > ./hw/intc/openpic.c: openpic_src_ops_be: missing read > ./hw/pci/msix.c: msix_pba_mmio_ops: missing write > ./hw/xen/xen_platform.c: xen_pci_io_ops: missing read > ./hw/misc/lm32_sys.c: sys_ops: missing read > ./hw/misc/pc-testdev.c: test_irq_ops: missing read > ./hw/misc/pc-testdev.c: test_flush_ops: missing read > ./hw/misc/vfio.c: vfio_ati_3c3_quirk: missing write > ./hw/misc/debugexit.c: debug_exit_ops: missing read > ./hw/net/lan9118.c: *mem_ops: missing read > ./hw/net/lan9118.c: *mem_ops: missing write > ./hw/char/grlib_apbuart.c: grlib_apbuart_ops: missing read > ./hw/char/grlib_apbuart.c: grlib_apbuart_ops: missing write > ./hw/isa/pc87312.c: pc87312_io_ops: missing read > ./hw/nvram/fw_cfg.c: fw_cfg_ctl_mem_ops: missing read > > No doubt there are some false positives in there (eg fw_cfg.c > provides a valid function so we'll never try to do reads) > and it may miss some. > > -- PMM > >