Re: [Qemu-devel] [PATCH 3/4] spapr: Code more defensively against lack of primary PCI bus

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 04/30/2013 09:30 PM, Alexander Graf wrote:
> 
> 
> Am 30.04.2013 um 13:15 schrieb Alexey Kardashevskiy <aik@ozlabs.ru>:
> 
>> 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 <aik@ozlabs.ru>
>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>>> ---
>>>> 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