On 04/30/2013 09:56 PM, Alexander Graf wrote:
>
>
> Am 30.04.2013 um 13:52 schrieb Alexey Kardashevskiy <a...@ozlabs.ru>:
>
>> On 04/30/2013 09:30 PM, Alexander Graf wrote:
>>>
>>>
>>> Am 30.04.2013 um 13:15 schrieb Alexey Kardashevskiy <a...@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.
>
> 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 <a...@ozlabs.ru>
>>>>>> Signed-off-by: David Gibson <da...@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
--
Alexey
