From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49551) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UX9bW-00026t-51 for qemu-devel@nongnu.org; Tue, 30 Apr 2013 08:24:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UX9LM-0003Yi-7i for qemu-devel@nongnu.org; Tue, 30 Apr 2013 08:08:05 -0400 Received: from mail-da0-x22d.google.com ([2607:f8b0:400e:c00::22d]:40543) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UX9LL-0003YD-Uc for qemu-devel@nongnu.org; Tue, 30 Apr 2013 08:07:36 -0400 Received: by mail-da0-f45.google.com with SMTP id v40so219577dad.32 for ; Tue, 30 Apr 2013 05:07:35 -0700 (PDT) Message-ID: <517FB3FA.3040007@ozlabs.ru> Date: Tue, 30 Apr 2013 22:07:22 +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> <517FB083.4090105@ozlabs.ru> <76A4381C-0268-4685-901B-7E9BD51A7AF7@suse.de> In-Reply-To: <76A4381C-0268-4685-901B-7E9BD51A7AF7@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:56 PM, Alexander Graf wrote: > > > Am 30.04.2013 um 13:52 schrieb Alexey Kardashevskiy : > >> 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. > > Your patch only addresses default device creation. It makes it easier to disable emulated PHB by commenting out one single line in the code. Profit. If there was a way to tell QEMU via command line not to create default emulated PHB, sure I would have implemented it but it is not there. And this is for debug, not for a normal use so there is no point in inventing such a parameter for qemu. > > Alex > >> >> >> >>> >>> >>> 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 -- Alexey