From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42194) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLmMu-0002ue-82 for qemu-devel@nongnu.org; Sat, 09 Jul 2016 03:08:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bLmMq-0004GV-Ul for qemu-devel@nongnu.org; Sat, 09 Jul 2016 03:08:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40311) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLmMq-0004GJ-Mu for qemu-devel@nongnu.org; Sat, 09 Jul 2016 03:08:00 -0400 References: <1467650742-17580-1-git-send-email-mst@redhat.com> <20160704194615-mutt-send-email-mst@redhat.com> <2d57d95d-ed56-5621-5d78-686ec7e36b17@ilande.co.uk> From: Marcel Apfelbaum Message-ID: <5780A2C9.8010301@redhat.com> Date: Sat, 9 Jul 2016 10:07:53 +0300 MIME-Version: 1.0 In-Reply-To: <2d57d95d-ed56-5621-5d78-686ec7e36b17@ilande.co.uk> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 03/36] hw/pci: delay bus_master_enable_region initialization List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark Cave-Ayland , "Michael S. Tsirkin" , qemu-devel@nongnu.org Cc: Peter Maydell , Paolo Bonzini On 07/09/2016 04:34 AM, Mark Cave-Ayland wrote: > On 04/07/16 17:46, Michael S. Tsirkin wrote: > >> From: Marcel Apfelbaum >> >> Skip bus_master_enable region creation on PCI device init >> in order to be sure the IOMMU device (if present) would >> be created in advance. Add this memory region at machine_done time. >> >> Signed-off-by: Marcel Apfelbaum >> Reviewed-by: Michael S. Tsirkin >> Signed-off-by: Michael S. Tsirkin >> --- >> include/hw/pci/pci_bus.h | 2 ++ >> include/sysemu/sysemu.h | 1 + >> hw/pci/pci.c | 41 ++++++++++++++++++++++++++++++++--------- >> vl.c | 5 +++++ >> 4 files changed, 40 insertions(+), 9 deletions(-) >> >> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h >> index 403fec6..5484a9b 100644 >> --- a/include/hw/pci/pci_bus.h >> +++ b/include/hw/pci/pci_bus.h >> @@ -39,6 +39,8 @@ struct PCIBus { >> Keep a count of the number of devices with raised IRQs. */ >> int nirq; >> int *irq_count; >> + >> + Notifier machine_done; >> }; >> >> typedef struct PCIBridgeWindows PCIBridgeWindows; >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h >> index 7313673..ee7c760 100644 >> --- a/include/sysemu/sysemu.h >> +++ b/include/sysemu/sysemu.h >> @@ -75,6 +75,7 @@ void qemu_add_exit_notifier(Notifier *notify); >> void qemu_remove_exit_notifier(Notifier *notify); >> >> void qemu_add_machine_init_done_notifier(Notifier *notify); >> +void qemu_remove_machine_init_done_notifier(Notifier *notify); >> >> void hmp_savevm(Monitor *mon, const QDict *qdict); >> int load_vmstate(const char *name); >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index 4b585f4..ee385eb 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -78,10 +78,37 @@ static const VMStateDescription vmstate_pcibus = { >> } >> }; >> >> +static void pci_init_bus_master(PCIDevice *pci_dev) >> +{ >> + AddressSpace *dma_as = pci_device_iommu_address_space(pci_dev); >> + >> + memory_region_init_alias(&pci_dev->bus_master_enable_region, >> + OBJECT(pci_dev), "bus master", >> + dma_as->root, 0, memory_region_size(dma_as->root)); >> + memory_region_set_enabled(&pci_dev->bus_master_enable_region, false); >> + address_space_init(&pci_dev->bus_master_as, >> + &pci_dev->bus_master_enable_region, pci_dev->name); >> +} >> + >> +static void pcibus_machine_done(Notifier *notifier, void *data) >> +{ >> + PCIBus *bus = container_of(notifier, PCIBus, machine_done); >> + int i; >> + >> + for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) { >> + if (bus->devices[i]) { >> + pci_init_bus_master(bus->devices[i]); >> + } >> + } >> +} >> + >> static void pci_bus_realize(BusState *qbus, Error **errp) >> { >> PCIBus *bus = PCI_BUS(qbus); >> >> + bus->machine_done.notify = pcibus_machine_done; >> + qemu_add_machine_init_done_notifier(&bus->machine_done); >> + >> vmstate_register(NULL, -1, &vmstate_pcibus, bus); >> } >> >> @@ -89,6 +116,8 @@ static void pci_bus_unrealize(BusState *qbus, Error **errp) >> { >> PCIBus *bus = PCI_BUS(qbus); >> >> + qemu_remove_machine_init_done_notifier(&bus->machine_done); >> + >> vmstate_unregister(NULL, &vmstate_pcibus, bus); >> } >> >> @@ -920,7 +949,6 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, >> PCIConfigReadFunc *config_read = pc->config_read; >> PCIConfigWriteFunc *config_write = pc->config_write; >> Error *local_err = NULL; >> - AddressSpace *dma_as; >> DeviceState *dev = DEVICE(pci_dev); >> >> pci_dev->bus = bus; >> @@ -961,15 +989,10 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, >> >> pci_dev->devfn = devfn; >> pci_dev->requester_id_cache = pci_req_id_cache_get(pci_dev); >> - dma_as = pci_device_iommu_address_space(pci_dev); >> - >> - memory_region_init_alias(&pci_dev->bus_master_enable_region, >> - OBJECT(pci_dev), "bus master", >> - dma_as->root, 0, memory_region_size(dma_as->root)); >> - memory_region_set_enabled(&pci_dev->bus_master_enable_region, false); >> - address_space_init(&pci_dev->bus_master_as, &pci_dev->bus_master_enable_region, >> - name); >> >> + if (qdev_hotplug) { >> + pci_init_bus_master(pci_dev); >> + } >> pstrcpy(pci_dev->name, sizeof(pci_dev->name), name); >> pci_dev->irq_state = 0; >> pci_config_alloc(pci_dev); >> diff --git a/vl.c b/vl.c >> index 9bb7f4c..5cd0f2a 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2675,6 +2675,11 @@ void qemu_add_machine_init_done_notifier(Notifier *notify) >> } >> } >> >> +void qemu_remove_machine_init_done_notifier(Notifier *notify) >> +{ >> + notifier_remove(notify); >> +} >> + >> static void qemu_run_machine_init_done_notifiers(void) >> { >> notifier_list_notify(&machine_init_done_notifiers, NULL); >> > > Unfortunately this patch causes a regression booting my Debian 7.8.0 and > NetBSD 7.0 test images under qemu-system-sparc64 when accessing the > CDROM device, and rather unusually causes the qemu-system-sparc64 > executable itself to segfault: > Hi Mark, Thank you for finding it, can you please provide a command line and maybe one of your test images? I'll be sure to address it. Thanks, Marcel > > Program received signal SIGSEGV, Segmentation fault. > [Switching to Thread 0x7fffe8453700 (LWP 21454)] > 0x000000000041494a in address_space_lookup_region (d=0x0, > addr=3221225472, resolve_subpage=true) at > /home/build/src/qemu/git/qemu/exec.c:359 > 359 MemoryRegionSection *section = atomic_read(&d->mru_section); > (gdb) bt > #0 0x000000000041494a in address_space_lookup_region (d=0x0, > addr=3221225472, resolve_subpage=true) at > /home/build/src/qemu/git/qemu/exec.c:359 > #1 0x0000000000414aa7 in address_space_translate_internal (d=0x0, > addr=3221225472, xlat=0x7fffe8451c40, plen=0x7fffe8451d10, > resolve_subpage=true) at /home/build/src/qemu/git/qemu/exec.c:390 > #2 0x0000000000414c8f in address_space_translate (as=0x1961600, > addr=3221225472, xlat=0x7fffe8451d18, plen=0x7fffe8451d10, > is_write=false) at /home/build/src/qemu/git/qemu/exec.c:428 > #3 0x000000000041a4be in address_space_read_full (as=0x1961600, > addr=3221225472, attrs=..., buf=0x7fffe8451f10 "\030(\226\001", len=8) > at /home/build/src/qemu/git/qemu/exec.c:2724 > #4 0x000000000041a5b3 in address_space_read (len=8, buf=0x7fffe8451f10 > "\030(\226\001", attrs=..., addr=3221225472, as=0x1961600) at > /home/build/src/qemu/git/qemu/include/exec/memory.h:1460 > #5 address_space_rw (as=0x1961600, addr=3221225472, attrs=..., > buf=0x7fffe8451f10 "\030(\226\001", len=8, is_write=false) at > /home/build/src/qemu/git/qemu/exec.c:2739 > #6 0x00000000005c71e0 in dma_memory_rw_relaxed (as=0x1961600, > addr=3221225472, buf=0x7fffe8451f10, len=8, dir=DMA_DIRECTION_TO_DEVICE) > at /home/build/src/qemu/git/qemu/include/sysemu/dma.h:87 > #7 0x00000000005c7258 in dma_memory_rw (as=0x1961600, addr=3221225472, > buf=0x7fffe8451f10, len=8, dir=DMA_DIRECTION_TO_DEVICE) at > /home/build/src/qemu/git/qemu/include/sysemu/dma.h:110 > #8 0x00000000005c72fa in pci_dma_rw (dev=0x19613f0, addr=3221225472, > buf=0x7fffe8451f10, len=8, dir=DMA_DIRECTION_TO_DEVICE) at > /home/build/src/qemu/git/qemu/include/hw/pci/pci.h:731 > #9 0x00000000005c735a in pci_dma_read (dev=0x19613f0, addr=3221225472, > buf=0x7fffe8451f10, len=8) at > /home/build/src/qemu/git/qemu/include/hw/pci/pci.h:738 > #10 0x00000000005c7980 in bmdma_rw_buf (dma=0x1962fa8, is_write=1) at > hw/ide/pci.c:139 > #11 0x00000000005c4372 in ide_atapi_cmd_read_dma_cb (opaque=0x1962550, > ret=0) at hw/ide/atapi.c:413 > #12 0x00000000005c7dfe in bmdma_cmd_writeb (bm=0x1962fa8, val=9) at > hw/ide/pci.c:245 > #13 0x00000000005c91a9 in bmdma_write (opaque=0x1962fa8, addr=0, val=9, > size=1) at hw/ide/cmd646.c:219 > #14 0x0000000000469fb8 in memory_region_write_accessor (mr=0x1963100, > addr=0, value=0x7fffe8452148, size=1, shift=0, mask=255, attrs=...) at > /home/build/src/qemu/git/qemu/memory.c:525 > #15 0x000000000046a1e4 in access_with_adjusted_size (addr=0, > value=0x7fffe8452148, size=1, access_size_min=1, access_size_max=4, > access=0x469ec1 , mr=0x1963100, attrs=...) > at /home/build/src/qemu/git/qemu/memory.c:586 > #16 0x000000000046d42b in memory_region_dispatch_write (mr=0x1963100, > addr=0, data=9, size=1, attrs=...) at > /home/build/src/qemu/git/qemu/memory.c:1262 > #17 0x000000000041a051 in address_space_write_continue (as=0x12bb4b0, > addr=2190466908936, attrs=..., buf=0x7fffe8452323 "\t\376\001", len=1, > addr1=0, l=1, mr=0x1963100) at /home/build/src/qemu/git/qemu/exec.c:2590 > #18 0x000000000041a1c3 in address_space_write (as=0x12bb4b0, > addr=2190466908936, attrs=..., buf=0x7fffe8452323 "\t\376\001", len=1) > at /home/build/src/qemu/git/qemu/exec.c:2635 > #19 0x000000000041a569 in address_space_rw (as=0x12bb4b0, > addr=2190466908936, attrs=..., buf=0x7fffe8452323 "\t\376\001", len=1, > is_write=true) at /home/build/src/qemu/git/qemu/exec.c:2737 > #20 0x000000000041c27d in address_space_stb (as=0x12bb4b0, > addr=2190466908936, val=9, attrs=..., result=0x0) at > /home/build/src/qemu/git/qemu/exec.c:3477 > #21 0x000000000041c2f5 in stb_phys (as=0x12bb4b0, addr=2190466908936, > val=9) at /home/build/src/qemu/git/qemu/exec.c:3485 > #22 0x0000000000504759 in helper_st_asi (env=0x12d57e8, > addr=2190466908936, val=9, asi=29, size=1) at > /home/build/src/qemu/git/qemu/target-sparc/ldst_helper.c:1801 > #23 0x00000000405bf4b0 in code_gen_buffer () > #24 0x0000000000420c97 in cpu_tb_exec (cpu=0x12cd570, > itb=0x7fffe8fc10c0) at /home/build/src/qemu/git/qemu/cpu-exec.c:166 > #25 0x0000000000421882 in cpu_loop_exec_tb (cpu=0x12cd570, > tb=0x7fffe8fc10c0, last_tb=0x7fffe8452988, tb_exit=0x7fffe8452984, > sc=0x7fffe84529a0) at /home/build/src/qemu/git/qemu/cpu-exec.c:530 > #26 0x0000000000421b40 in cpu_exec (cpu=0x12cd570) at > /home/build/src/qemu/git/qemu/cpu-exec.c:626 > #27 0x0000000000451e6b in tcg_cpu_exec (cpu=0x12cd570) at > /home/build/src/qemu/git/qemu/cpus.c:1541 > #28 0x0000000000451f6d in tcg_exec_all () at > /home/build/src/qemu/git/qemu/cpus.c:1574 > #29 0x00000000004510b4 in qemu_tcg_cpu_thread_fn (arg=0x12cd570) at > /home/build/src/qemu/git/qemu/cpus.c:1171 > #30 0x00007ffff264bb50 in start_thread (arg=) at > pthread_create.c:304 > #31 0x00007ffff2395fbd in clone () at > ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 > #32 0x0000000000000000 in ?? () > (gdb) > > > > ATB, > > Mark. >