Re: [Qemu-devel] [PATCH 4/4] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough

["Chen, Tiejun" <tiejun.chen@intel.com>]

On 2014/7/29 19:29, Michael S. Tsirkin wrote:
> On Thu, Jul 24, 2014 at 07:30:29PM +0800, Tiejun Chen wrote:
>> Now we can introduce a new machine, xenigd, specific to IGD
>> passthrough. This can avoid involving other common codes.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>>   hw/i386/pc_piix.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 87 insertions(+)
>>
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 2391fda..46e5901 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -206,6 +206,33 @@ static void pc_machine_pci_bus_init(MachineState *machine,
>>       }
>>   }
>>
>> +static void xen_igd_pc_machine_pci_bus_init(MachineState *machine,
>> +                     int pci_enabled,
>> +                     PCII440FXState *i440fx_state,
>> +                     int piix3_devfn,
>> +                     PCIBus *pci_bus,
>> +                     ISABus *isa_bus,
>> +                     qemu_irq *gsi,
>> +                     MemoryRegion *pci_memory,
>> +                     MemoryRegion *ram_memory)
>> +{
>> +    MemoryRegion *system_memory = get_system_memory();
>> +    MemoryRegion *system_io = get_system_io();
>> +
>> +    if (pci_enabled) {
>> +        pci_bus = xen_igd_i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus,
>> +                              gsi, system_memory, system_io, machine->ram_size,
>> +                              below_4g_mem_size,
>> +                              above_4g_mem_size,
>> +                              pci_memory, ram_memory);
>> +    } else {
>> +        pci_bus = NULL;
>> +        i440fx_state = NULL;
>
> what does this do?

With patch #1, I split pc_init1() as three sub functions:

#1 pc_machine_base_init()
#2 pc_machine_pci_bus_init()
#3 pc_machine_device_init()

Here xen_igd_pc_machine_pci_bus_init() is mostly same as 
pc_machine_pci_bus_init(), just except we would call xen_igd_i440fx_init().

>
>> +        isa_bus = isa_bus_new(NULL, system_io);
>> +        no_hpet = 1;
>> +    }
>
> no_hpet is code duplicated from pc in chunk above, better to move to
> a common function.

Ditto.

Thanks
Tiejun

>
>> +}
>> +
>>   static void pc_machine_device_init(MachineState *machine,
>>                        int pci_enabled,
>>                        GSIState *gsi_state,
>> @@ -337,11 +364,39 @@ static void pc_init1(MachineState *machine,
>>                       piix3_devfn, fw_cfg, pci_bus, isa_bus, gsi);
>>   }
>>
>> +static void xen_igd_pc_init1(MachineState *machine,
>> +                     int pci_enabled,
>> +                     int kvmclock_enabled)
>> +{
>> +    PCIBus *pci_bus = NULL;
>> +    ISABus *isa_bus = NULL;
>> +    PCII440FXState *i440fx_state = NULL;
>> +    int piix3_devfn = -1;
>> +    qemu_irq *gsi = NULL;
>> +    GSIState *gsi_state = NULL;
>> +    MemoryRegion *ram_memory = NULL;
>> +    MemoryRegion *pci_memory = NULL;
>> +    DeviceState *icc_bridge = NULL;
>> +    FWCfgState *fw_cfg = NULL;
>> +
>> +    pc_machine_base_init(machine, pci_enabled, kvmclock_enabled, icc_bridge,
>> +                    ram_memory, pci_memory, gsi, gsi_state, fw_cfg);
>> +    xen_igd_pc_machine_pci_bus_init(machine, pci_enabled, i440fx_state,
>> +                    piix3_devfn, pci_bus, isa_bus, gsi, pci_memory, ram_memory);
>> +    pc_machine_device_init(machine, pci_enabled, gsi_state, icc_bridge,
>> +                    piix3_devfn, fw_cfg, pci_bus, isa_bus, gsi);
>> +}
>> +
>>   static void pc_init_pci(MachineState *machine)
>>   {
>>       pc_init1(machine, 1, 1);
>>   }
>>
>> +static void xen_igd_pc_init_pci(MachineState *machine)
>> +{
>> +    xen_igd_pc_init1(machine, 1, 1);
>> +}
>> +
>>   static void pc_compat_2_0(MachineState *machine)
>>   {
>>       smbios_legacy_mode = true;
>> @@ -470,6 +525,17 @@ static void pc_xen_hvm_init(MachineState *machine)
>>           pci_create_simple(bus, -1, "xen-platform");
>>       }
>>   }
>> +static void xen_igd_pc_hvm_init(MachineState *machine)
>> +{
>> +    PCIBus *bus;
>> +
>> +    xen_igd_pc_init_pci(machine);
>> +
>> +    bus = pci_find_primary_bus();
>> +    if (bus != NULL) {
>> +        pci_create_simple(bus, -1, "xen-platform");
>> +    }
>> +}
>>   #endif
>>
>>   #define PC_I440FX_MACHINE_OPTIONS \
>> @@ -919,6 +985,26 @@ static QEMUMachine xenfv_machine = {
>>           { /* end of list */ }
>>       },
>>   };
>> +static QEMUMachine xenigd_machine = {
>> +    PC_COMMON_MACHINE_OPTIONS,
>> +    .name = "xenigd",
>> +    .desc = "Xen Fully-virtualized PC specific to IGD",
>> +    .init = xen_igd_pc_hvm_init,
>> +    .max_cpus = HVM_MAX_VCPUS,
>> +    .default_machine_opts = "accel=xen",
>> +    .hot_add_cpu = pc_hot_add_cpu,
>> +    .compat_props = (GlobalProperty[]) {
>> +        /* xenfv has no fwcfg and so does not load acpi from QEMU.
>> +         * as such new acpi features don't work.
>> +         */
>> +        {
>> +            .driver   = "PIIX4_PM",
>> +            .property = "acpi-pci-hotplug-with-bridge-support",
>> +            .value    = "off",
>> +        },
>> +        { /* end of list */ }
>> +    },
>> +};
>>   #endif
>>
>>   static void pc_machine_init(void)
>> @@ -942,6 +1028,7 @@ static void pc_machine_init(void)
>>       qemu_register_pc_machine(&isapc_machine);
>>   #ifdef CONFIG_XEN
>>       qemu_register_pc_machine(&xenfv_machine);
>> +    qemu_register_pc_machine(&xenigd_machine);
>>   #endif
>>   }
>>
>> --
>> 1.9.1
>