qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/4] spapr: Code more defensively against lack o


From: Alexander Graf
Subject: Re: [Qemu-devel] [PATCH 3/4] spapr: Code more defensively against lack of primary PCI bus
Date: Tue, 30 Apr 2013 13:56:06 +0200


Am 30.04.2013 um 13:52 schrieb Alexey Kardashevskiy <address@hidden>:

> On 04/30/2013 09:30 PM, Alexander Graf wrote:
>> 
>> 
>> Am 30.04.2013 um 13:15 schrieb Alexey Kardashevskiy <address@hidden>:
>> 
>>> 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.

Alex

> 
> 
> 
>> 
>> 
>> Alex
>> 
>>> 
>>> 
>>> 
>>> 
>>>> 
>>>> 
>>>> Alex
>>>> 
>>>>> 
>>>>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>>>>> Signed-off-by: David Gibson <address@hidden>
>>>>> ---
>>>>> 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]