From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40355) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UX96z-0000AC-VQ for qemu-devel@nongnu.org; Tue, 30 Apr 2013 07:52:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UX96y-0006cG-Gt for qemu-devel@nongnu.org; Tue, 30 Apr 2013 07:52:45 -0400 Received: from mail-pb0-x231.google.com ([2607:f8b0:400e:c01::231]:54519) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UX96y-0006bW-AA for qemu-devel@nongnu.org; Tue, 30 Apr 2013 07:52:44 -0400 Received: by mail-pb0-f49.google.com with SMTP id rp8so224862pbb.36 for ; Tue, 30 Apr 2013 04:52:42 -0700 (PDT) Message-ID: <517FB083.4090105@ozlabs.ru> Date: Tue, 30 Apr 2013 21:52:35 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1367296434-15453-1-git-send-email-aik@ozlabs.ru> <1367296434-15453-4-git-send-email-aik@ozlabs.ru> <5DE88D0C-C94D-4C8F-B762-E0EDFC751F04@suse.de> <517FA7BF.8010804@ozlabs.ru> <4E3C0546-C723-4911-A1D5-61A934CCDB14@suse.de> In-Reply-To: <4E3C0546-C723-4911-A1D5-61A934CCDB14@suse.de> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/4] spapr: Code more defensively against lack of primary PCI bus List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: "qemu-ppc@nongnu.org" , "qemu-devel@nongnu.org" , "david@gibson.dropbear.id.au" On 04/30/2013 09:30 PM, Alexander Graf wrote: > > > Am 30.04.2013 um 13:15 schrieb Alexey Kardashevskiy : > >> On 04/30/2013 07:45 PM, Alexander Graf wrote: >>> >>> On 30.04.2013, at 06:33, Alexey Kardashevskiy wrote: >>> >>>> Currently sPAPR always creates a primary PCI host bridge for emulated PCI >>>> devices. However, because the platform supports native virtual IO, and >>>> can also support multiple independent PCI host bridges, it's quite often >>>> useful to disable the primary bridge for debugging purposes. >>>> >>>> This patch, therefore, makes the code cope more gracefully with a missing >>>> primary host bridge. >>> >>> Does this handle the -net case too? What about disks? >>> >>> If those use a different mechanism to find their bus, maybe it'd be better to instead do >>> >>> if (phb) { >>> pcibus = phb->bus; >>> } else { >>> pcibus = find_pci_bus(); >>> } >>> >>> In fact, maybe the code already deals with bus==NULL? >> >> >> I needed it to deal with phb==NULL. >> >> The story behind this patch is that while doing VFIO stuff, I have to deal >> with multiple PHBs. And sometime I want to be 100% sure that it is just >> VFIO PHB and no emulated PCI stuff (especially when I debug USB PCI card in >> VFIO) so I simply commented out the call which creates default emulated PHB >> and that would be it if the existing code did not try to use phb->bus. > > Then this code does nor belong in upstream. If you want to have full control over instantiated devices, implement -nodefaults. -nodefaults means no default devices but it is still expected to create PCI bus and libvirt heavily uses this feature. > > > Alex > >> >> >> >> >>> >>> >>> Alex >>> >>>> >>>> Signed-off-by: Alexey Kardashevskiy >>>> Signed-off-by: David Gibson >>>> --- >>>> hw/ppc/spapr.c | 6 +++--- >>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>>> index c96ac81..d07c74a 100644 >>>> --- a/hw/ppc/spapr.c >>>> +++ b/hw/ppc/spapr.c >>>> @@ -732,7 +732,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) >>>> const char *boot_device = args->boot_device; >>>> PowerPCCPU *cpu; >>>> CPUPPCState *env; >>>> - PCIHostState *phb; >>>> + PCIHostState *phb = NULL; >>>> int i; >>>> MemoryRegion *sysmem = get_system_memory(); >>>> MemoryRegion *ram = g_new(MemoryRegion, 1); >>>> @@ -898,11 +898,11 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) >>>> } >>>> >>>> /* Graphics */ >>>> - if (spapr_vga_init(phb->bus)) { >>>> + if (phb && spapr_vga_init(phb->bus)) { >>>> spapr->has_graphics = true; >>>> } >>>> >>>> - if (usb_enabled(spapr->has_graphics)) { >>>> + if (phb && usb_enabled(spapr->has_graphics)) { >>>> pci_create_simple(phb->bus, -1, "pci-ohci"); >>>> if (spapr->has_graphics) { >>>> usbdevice_create("keyboard"); >>>> -- >>>> 1.7.10.4 >> >> >> -- >> Alexey -- Alexey