From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:51850) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RTrrT-0003ps-JY for qemu-devel@nongnu.org; Fri, 25 Nov 2011 04:14:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RTrrR-0007FA-Dh for qemu-devel@nongnu.org; Fri, 25 Nov 2011 04:14:23 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:44173) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RTrrQ-0007ER-Lj for qemu-devel@nongnu.org; Fri, 25 Nov 2011 04:14:21 -0500 Received: from huawei.com (szxga03-in [172.24.2.9]) by szxga03-in.huawei.com (iPlanet Messaging Server 5.2 HotFix 2.14 (built Aug 8 2006)) with ESMTP id <0LV7006ERLKX04@szxga03-in.huawei.com> for qemu-devel@nongnu.org; Fri, 25 Nov 2011 17:12:34 +0800 (CST) Received: from szxrg02-dlp.huawei.com ([172.24.2.119]) by szxga03-in.huawei.com (iPlanet Messaging Server 5.2 HotFix 2.14 (built Aug 8 2006)) with ESMTP id <0LV700GBHLKOIS@szxga03-in.huawei.com> for qemu-devel@nongnu.org; Fri, 25 Nov 2011 17:12:33 +0800 (CST) Date: Fri, 25 Nov 2011 17:12:11 +0800 From: Zang Hongyong In-reply-to: Message-id: <4ECF5BEB.3080403@huawei.com> MIME-version: 1.0 Content-type: text/plain; format=flowed; charset=UTF-8 Content-transfer-encoding: QUOTED-PRINTABLE References: <1322129109-18140-1-git-send-email-zanghongyong@huawei.com> Subject: Re: [Qemu-devel] [PATCH] ivshmem: fix guest unable to start with ioeventfd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cam Macdonell Cc: wusongwei@huawei.com, kvm@vger.kernel.org, hanweidong@huawei.com, qemu-devel@nongnu.org, louzhengwei@huawei.com, xiaowei.yang@huawei.com, avi@redhat.com =E4=BA=8E 2011/11/25,=E6=98=9F=E6=9C=9F=E4=BA=94 4:29, Cam Macdonell = =E5=86=99=E9=81=93: > On Thu, Nov 24, 2011 at 3:05 AM, wrote: >> From: Hongyong Zang >> >> When a guest boots with ioeventfd, an error (by gdb) occurs: >> Program received signal SIGSEGV, Segmentation fault. >> 0x00000000006009cc in setup_ioeventfds (s=3D0x171dc40) >> at /home/louzhengwei/git_source/qemu-kvm/hw/ivshmem.c:363 >> 363 for (j =3D 0; j< s->peers[i].nb_eventfds; j++) = { >> The bug is due to accessing s->peers which is NULL. > Can you share the command-line that caused the fault? The command-lines: 1) ivshmem_server -m 4 -p /tmp/nahanni 2) qemu-system-x86_64 -smp 1 -m 1024 -boot c -drive=20 file=3D./rhel6_ivsh1.img,if=3Dvirtio -chardev socket,path=3D/tmp/nahanni,id=3Dlzw -device=20 ivshmem,chardev=3Dlzw,size=3D4m,ioeventfd=3Don,vectors=3D8 -vnc :41 >> This patch uses the memory region API to replace the old one kvm_s= et_ioeventfd_mmio_long(). >> And this patch makes memory_region_add_eventfd() called in ivshmem= _read() when qemu receives >> eventfd information from ivshmem_server. > Should this patch be split into two patches, to separate the bug fi= x > from the other changes related to the Memory API? Unless I > misunderstand how the two are necessarily related. This bug locates in setup_ioeventfds(). The setup_ioeventfds() functi= on=20 wants to call memory_region_add_eventfd() to configure eventfd info for every s->peer, but s->peers are=20 uninitialized at this moment. When qemu receives eventfd info from "ivshmem_server", ivshmem_read()= is=20 called to set eventfd info. Function ivshmem_read() calls kvm_set_ioeventfd_mmio_long() which can= be=20 encapsulated in the new Memory API. So this patch uses memory_region_add_eventfd() to replace=20 kvm_set_ioeventfd_mmio_long(). In this way, each qemu has already configured the proper eventfd info, and there's no need t= o=20 use the function setup_ioeventfds(). Furthermore, nobody uses IVShmemState's member "pcibus_t mmio_addr". = So=20 we can remove the member as well. Regards, Hongyong Zang > > Cam > >> Signed-off-by: Hongyong Zang >> --- >> hw/ivshmem.c | 41 ++++++++++++++--------------------------- >> 1 files changed, 14 insertions(+), 27 deletions(-) >> >> diff --git a/hw/ivshmem.c b/hw/ivshmem.c >> index 242fbea..be26f03 100644 >> --- a/hw/ivshmem.c >> +++ b/hw/ivshmem.c >> @@ -58,7 +58,6 @@ typedef struct IVShmemState { >> CharDriverState *server_chr; >> MemoryRegion ivshmem_mmio; >> >> - pcibus_t mmio_addr; >> /* We might need to register the BAR before we actually have = the memory. >> * So prepare a container MemoryRegion for the BAR immediatel= y and >> * add a subregion when we have the memory. >> @@ -346,8 +345,14 @@ static void close_guest_eventfds(IVShmemState= *s, int posn) >> guest_curr_max =3D s->peers[posn].nb_eventfds; >> >> for (i =3D 0; i< guest_curr_max; i++) { >> - kvm_set_ioeventfd_mmio_long(s->peers[posn].eventfds[i], >> - s->mmio_addr + DOORBELL, (posn<< 16) | i, 0)= ; >> + if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD)) { >> + memory_region_del_eventfd(&s->ivshmem_mmio, >> + DOORBELL, >> + 4, >> + true, >> + (posn<< 16) | i, >> + s->peers[posn].eventfds[i]); >> + } >> close(s->peers[posn].eventfds[i]); >> } >> >> @@ -355,22 +360,6 @@ static void close_guest_eventfds(IVShmemState= *s, int posn) >> s->peers[posn].nb_eventfds =3D 0; >> } >> >> -static void setup_ioeventfds(IVShmemState *s) { >> - >> - int i, j; >> - >> - for (i =3D 0; i<=3D s->max_peer; i++) { >> - for (j =3D 0; j< s->peers[i].nb_eventfds; j++) { >> - memory_region_add_eventfd(&s->ivshmem_mmio, >> - DOORBELL, >> - 4, >> - true, >> - (i<< 16) | j, >> - s->peers[i].eventfds[j]); >> - } >> - } >> -} >> - >> /* this function increase the dynamic storage need to store data= about other >> * guests */ >> static void increase_dynamic_storage(IVShmemState *s, int new_mi= n_size) { >> @@ -491,10 +480,12 @@ static void ivshmem_read(void *opaque, const= uint8_t * buf, int flags) >> } >> >> if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD)) { >> - if (kvm_set_ioeventfd_mmio_long(incoming_fd, s->mmio_addr= + DOORBELL, >> - (incoming_posn<< 16) | guest_max_eventfd= , 1)< 0) { >> - fprintf(stderr, "ivshmem: ioeventfd not available\n")= ; >> - } >> + memory_region_add_eventfd(&s->ivshmem_mmio, >> + DOORBELL, >> + 4, >> + true, >> + (incoming_posn<< 16) | guest_m= ax_eventfd, >> + incoming_fd); >> } >> >> return; >> @@ -659,10 +650,6 @@ static int pci_ivshmem_init(PCIDevice *dev) >> memory_region_init_io(&s->ivshmem_mmio,&ivshmem_mmio_ops, s, >> "ivshmem-mmio", IVSHMEM_REG_BAR_SIZE); >> >> - if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD)) { >> - setup_ioeventfds(s); >> - } >> - >> /* region for registers*/ >> pci_register_bar(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, >> &s->ivshmem_mmio); >> -- >> 1.7.1 >> >> > . >