* [Qemu-devel] [PATCH 1/4] hw:i386:pc_piix: split pc_init1()
2014-07-24 11:30 [Qemu-devel] [PATCH 0/4] xen:passthrough: introduce a separate machine to igd passthrough Tiejun Chen
@ 2014-07-24 11:30 ` Tiejun Chen
2014-07-29 11:26 ` Michael S. Tsirkin
2014-07-24 11:30 ` [Qemu-devel] [PATCH 2/4] xen:hw:pci-host:piix: create host bridge to passthrough Tiejun Chen
` (3 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Tiejun Chen @ 2014-07-24 11:30 UTC (permalink / raw)
To: mst, stefano.stabellini, pbonzini; +Cc: qemu-devel, xen-devel
We'd like to split pc_init1 and then we can share something
with other stuff.
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
hw/i386/pc_piix.c | 93 +++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 70 insertions(+), 23 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7081c08..2391fda 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -70,34 +70,21 @@ static bool smbios_legacy_mode;
static bool gigabyte_align = true;
static bool has_reserved_memory = true;
-/* PC hardware initialisation */
-static void pc_init1(MachineState *machine,
+static ram_addr_t below_4g_mem_size;
+static ram_addr_t above_4g_mem_size;
+static void pc_machine_base_init(MachineState *machine,
int pci_enabled,
- int kvmclock_enabled)
+ int kvmclock_enabled,
+ DeviceState *icc_bridge,
+ MemoryRegion *ram_memory,
+ MemoryRegion *pci_memory,
+ qemu_irq *gsi,
+ GSIState *gsi_state,
+ FWCfgState *fw_cfg)
{
PCMachineState *pc_machine = PC_MACHINE(machine);
MemoryRegion *system_memory = get_system_memory();
- MemoryRegion *system_io = get_system_io();
- int i;
- ram_addr_t below_4g_mem_size, above_4g_mem_size;
- PCIBus *pci_bus;
- ISABus *isa_bus;
- PCII440FXState *i440fx_state;
- int piix3_devfn = -1;
- qemu_irq *cpu_irq;
- qemu_irq *gsi;
- qemu_irq *i8259;
- qemu_irq *smi_irq;
- GSIState *gsi_state;
- DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
- BusState *idebus[MAX_IDE_BUS];
- ISADevice *rtc_state;
- ISADevice *floppy;
- MemoryRegion *ram_memory;
- MemoryRegion *pci_memory;
MemoryRegion *rom_memory;
- DeviceState *icc_bridge;
- FWCfgState *fw_cfg = NULL;
PcGuestInfo *guest_info;
ram_addr_t lowmem;
@@ -190,6 +177,20 @@ static void pc_init1(MachineState *machine,
} else {
gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
}
+}
+
+static void 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 = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
@@ -203,6 +204,28 @@ static void pc_init1(MachineState *machine,
isa_bus = isa_bus_new(NULL, system_io);
no_hpet = 1;
}
+}
+
+static void pc_machine_device_init(MachineState *machine,
+ int pci_enabled,
+ GSIState *gsi_state,
+ DeviceState *icc_bridge,
+ int piix3_devfn,
+ FWCfgState *fw_cfg,
+ PCIBus *pci_bus,
+ ISABus *isa_bus,
+ qemu_irq *gsi)
+{
+ int i;
+ DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
+ BusState *idebus[MAX_IDE_BUS];
+ qemu_irq *smi_irq;
+ PCMachineState *pc_machine = PC_MACHINE(machine);
+ qemu_irq *cpu_irq;
+ qemu_irq *i8259;
+ ISADevice *rtc_state;
+ ISADevice *floppy;
+
isa_bus_irqs(isa_bus, gsi);
if (kvm_irqchip_in_kernel()) {
@@ -290,6 +313,30 @@ static void pc_init1(MachineState *machine,
}
}
+/* PC hardware initialisation */
+static void 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);
+ 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);
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 1/4] hw:i386:pc_piix: split pc_init1()
2014-07-24 11:30 ` [Qemu-devel] [PATCH 1/4] hw:i386:pc_piix: split pc_init1() Tiejun Chen
@ 2014-07-29 11:26 ` Michael S. Tsirkin
2014-07-30 8:19 ` Chen, Tiejun
0 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2014-07-29 11:26 UTC (permalink / raw)
To: Tiejun Chen; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
On Thu, Jul 24, 2014 at 07:30:26PM +0800, Tiejun Chen wrote:
> We'd like to split pc_init1 and then we can share something
> with other stuff.
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Did you test this patch? It does not look like it can work.
> ---
> hw/i386/pc_piix.c | 93 +++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 70 insertions(+), 23 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 7081c08..2391fda 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -70,34 +70,21 @@ static bool smbios_legacy_mode;
> static bool gigabyte_align = true;
> static bool has_reserved_memory = true;
>
> -/* PC hardware initialisation */
> -static void pc_init1(MachineState *machine,
> +static ram_addr_t below_4g_mem_size;
> +static ram_addr_t above_4g_mem_size;
> +static void pc_machine_base_init(MachineState *machine,
> int pci_enabled,
> - int kvmclock_enabled)
> + int kvmclock_enabled,
> + DeviceState *icc_bridge,
> + MemoryRegion *ram_memory,
> + MemoryRegion *pci_memory,
> + qemu_irq *gsi,
> + GSIState *gsi_state,
> + FWCfgState *fw_cfg)
> {
> PCMachineState *pc_machine = PC_MACHINE(machine);
> MemoryRegion *system_memory = get_system_memory();
> - MemoryRegion *system_io = get_system_io();
> - int i;
> - ram_addr_t below_4g_mem_size, above_4g_mem_size;
> - PCIBus *pci_bus;
> - ISABus *isa_bus;
> - PCII440FXState *i440fx_state;
> - int piix3_devfn = -1;
> - qemu_irq *cpu_irq;
> - qemu_irq *gsi;
> - qemu_irq *i8259;
> - qemu_irq *smi_irq;
> - GSIState *gsi_state;
> - DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> - BusState *idebus[MAX_IDE_BUS];
> - ISADevice *rtc_state;
> - ISADevice *floppy;
> - MemoryRegion *ram_memory;
> - MemoryRegion *pci_memory;
> MemoryRegion *rom_memory;
> - DeviceState *icc_bridge;
> - FWCfgState *fw_cfg = NULL;
> PcGuestInfo *guest_info;
> ram_addr_t lowmem;
>
> @@ -190,6 +177,20 @@ static void pc_init1(MachineState *machine,
> } else {
> gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
> }
> +}
> +
> +static void 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 = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
> @@ -203,6 +204,28 @@ static void pc_init1(MachineState *machine,
> isa_bus = isa_bus_new(NULL, system_io);
> no_hpet = 1;
> }
> +}
> +
> +static void pc_machine_device_init(MachineState *machine,
> + int pci_enabled,
> + GSIState *gsi_state,
> + DeviceState *icc_bridge,
> + int piix3_devfn,
> + FWCfgState *fw_cfg,
> + PCIBus *pci_bus,
> + ISABus *isa_bus,
> + qemu_irq *gsi)
> +{
> + int i;
> + DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> + BusState *idebus[MAX_IDE_BUS];
> + qemu_irq *smi_irq;
> + PCMachineState *pc_machine = PC_MACHINE(machine);
> + qemu_irq *cpu_irq;
> + qemu_irq *i8259;
> + ISADevice *rtc_state;
> + ISADevice *floppy;
> +
> isa_bus_irqs(isa_bus, gsi);
>
> if (kvm_irqchip_in_kernel()) {
> @@ -290,6 +313,30 @@ static void pc_init1(MachineState *machine,
> }
> }
>
> +/* PC hardware initialisation */
> +static void 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;
These are set to NULL here and never modified below.
Why does it make sense?
> +
> + pc_machine_base_init(machine, pci_enabled, kvmclock_enabled, icc_bridge,
> + ram_memory, pci_memory, gsi, gsi_state, fw_cfg);
> + 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);
> --
> 1.9.1
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 1/4] hw:i386:pc_piix: split pc_init1()
2014-07-29 11:26 ` Michael S. Tsirkin
@ 2014-07-30 8:19 ` Chen, Tiejun
2014-07-30 17:48 ` Michael S. Tsirkin
0 siblings, 1 reply; 18+ messages in thread
From: Chen, Tiejun @ 2014-07-30 8:19 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
On 2014/7/29 19:26, Michael S. Tsirkin wrote:
> On Thu, Jul 24, 2014 at 07:30:26PM +0800, Tiejun Chen wrote:
>> We'd like to split pc_init1 and then we can share something
>> with other stuff.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>
> Did you test this patch? It does not look like it can work.
Just compile but I think you're right. Here something is really wrong
here, so this is my fault.
I will regenerate this patch and do test to double check.
Thanks
Tiejun
>
>> ---
>> hw/i386/pc_piix.c | 93 +++++++++++++++++++++++++++++++++++++++++--------------
>> 1 file changed, 70 insertions(+), 23 deletions(-)
>>
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 7081c08..2391fda 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -70,34 +70,21 @@ static bool smbios_legacy_mode;
>> static bool gigabyte_align = true;
>> static bool has_reserved_memory = true;
>>
>> -/* PC hardware initialisation */
>> -static void pc_init1(MachineState *machine,
>> +static ram_addr_t below_4g_mem_size;
>> +static ram_addr_t above_4g_mem_size;
>> +static void pc_machine_base_init(MachineState *machine,
>> int pci_enabled,
>> - int kvmclock_enabled)
>> + int kvmclock_enabled,
>> + DeviceState *icc_bridge,
>> + MemoryRegion *ram_memory,
>> + MemoryRegion *pci_memory,
>> + qemu_irq *gsi,
>> + GSIState *gsi_state,
>> + FWCfgState *fw_cfg)
>> {
>> PCMachineState *pc_machine = PC_MACHINE(machine);
>> MemoryRegion *system_memory = get_system_memory();
>> - MemoryRegion *system_io = get_system_io();
>> - int i;
>> - ram_addr_t below_4g_mem_size, above_4g_mem_size;
>> - PCIBus *pci_bus;
>> - ISABus *isa_bus;
>> - PCII440FXState *i440fx_state;
>> - int piix3_devfn = -1;
>> - qemu_irq *cpu_irq;
>> - qemu_irq *gsi;
>> - qemu_irq *i8259;
>> - qemu_irq *smi_irq;
>> - GSIState *gsi_state;
>> - DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>> - BusState *idebus[MAX_IDE_BUS];
>> - ISADevice *rtc_state;
>> - ISADevice *floppy;
>> - MemoryRegion *ram_memory;
>> - MemoryRegion *pci_memory;
>> MemoryRegion *rom_memory;
>> - DeviceState *icc_bridge;
>> - FWCfgState *fw_cfg = NULL;
>> PcGuestInfo *guest_info;
>> ram_addr_t lowmem;
>>
>> @@ -190,6 +177,20 @@ static void pc_init1(MachineState *machine,
>> } else {
>> gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
>> }
>> +}
>> +
>> +static void 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 = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
>> @@ -203,6 +204,28 @@ static void pc_init1(MachineState *machine,
>> isa_bus = isa_bus_new(NULL, system_io);
>> no_hpet = 1;
>> }
>> +}
>> +
>> +static void pc_machine_device_init(MachineState *machine,
>> + int pci_enabled,
>> + GSIState *gsi_state,
>> + DeviceState *icc_bridge,
>> + int piix3_devfn,
>> + FWCfgState *fw_cfg,
>> + PCIBus *pci_bus,
>> + ISABus *isa_bus,
>> + qemu_irq *gsi)
>> +{
>> + int i;
>> + DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>> + BusState *idebus[MAX_IDE_BUS];
>> + qemu_irq *smi_irq;
>> + PCMachineState *pc_machine = PC_MACHINE(machine);
>> + qemu_irq *cpu_irq;
>> + qemu_irq *i8259;
>> + ISADevice *rtc_state;
>> + ISADevice *floppy;
>> +
>> isa_bus_irqs(isa_bus, gsi);
>>
>> if (kvm_irqchip_in_kernel()) {
>> @@ -290,6 +313,30 @@ static void pc_init1(MachineState *machine,
>> }
>> }
>>
>> +/* PC hardware initialisation */
>> +static void 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;
>
> These are set to NULL here and never modified below.
> Why does it make sense?
>
>
>> +
>> + pc_machine_base_init(machine, pci_enabled, kvmclock_enabled, icc_bridge,
>> + ram_memory, pci_memory, gsi, gsi_state, fw_cfg);
>> + 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);
>> --
>> 1.9.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 1/4] hw:i386:pc_piix: split pc_init1()
2014-07-30 8:19 ` Chen, Tiejun
@ 2014-07-30 17:48 ` Michael S. Tsirkin
0 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2014-07-30 17:48 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
On Wed, Jul 30, 2014 at 04:19:58PM +0800, Chen, Tiejun wrote:
> On 2014/7/29 19:26, Michael S. Tsirkin wrote:
> >On Thu, Jul 24, 2014 at 07:30:26PM +0800, Tiejun Chen wrote:
> >>We'd like to split pc_init1 and then we can share something
> >>with other stuff.
> >>
> >>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >
> >Did you test this patch? It does not look like it can work.
>
> Just compile but I think you're right. Here something is really wrong here,
> so this is my fault.
>
> I will regenerate this patch and do test to double check.
>
> Thanks
> Tiejun
Yea, and pls tell us what you tested exactly in the cover letter.
> >
> >>---
> >> hw/i386/pc_piix.c | 93 +++++++++++++++++++++++++++++++++++++++++--------------
> >> 1 file changed, 70 insertions(+), 23 deletions(-)
> >>
> >>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >>index 7081c08..2391fda 100644
> >>--- a/hw/i386/pc_piix.c
> >>+++ b/hw/i386/pc_piix.c
> >>@@ -70,34 +70,21 @@ static bool smbios_legacy_mode;
> >> static bool gigabyte_align = true;
> >> static bool has_reserved_memory = true;
> >>
> >>-/* PC hardware initialisation */
> >>-static void pc_init1(MachineState *machine,
> >>+static ram_addr_t below_4g_mem_size;
> >>+static ram_addr_t above_4g_mem_size;
> >>+static void pc_machine_base_init(MachineState *machine,
> >> int pci_enabled,
> >>- int kvmclock_enabled)
> >>+ int kvmclock_enabled,
> >>+ DeviceState *icc_bridge,
> >>+ MemoryRegion *ram_memory,
> >>+ MemoryRegion *pci_memory,
> >>+ qemu_irq *gsi,
> >>+ GSIState *gsi_state,
> >>+ FWCfgState *fw_cfg)
> >> {
> >> PCMachineState *pc_machine = PC_MACHINE(machine);
> >> MemoryRegion *system_memory = get_system_memory();
> >>- MemoryRegion *system_io = get_system_io();
> >>- int i;
> >>- ram_addr_t below_4g_mem_size, above_4g_mem_size;
> >>- PCIBus *pci_bus;
> >>- ISABus *isa_bus;
> >>- PCII440FXState *i440fx_state;
> >>- int piix3_devfn = -1;
> >>- qemu_irq *cpu_irq;
> >>- qemu_irq *gsi;
> >>- qemu_irq *i8259;
> >>- qemu_irq *smi_irq;
> >>- GSIState *gsi_state;
> >>- DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> >>- BusState *idebus[MAX_IDE_BUS];
> >>- ISADevice *rtc_state;
> >>- ISADevice *floppy;
> >>- MemoryRegion *ram_memory;
> >>- MemoryRegion *pci_memory;
> >> MemoryRegion *rom_memory;
> >>- DeviceState *icc_bridge;
> >>- FWCfgState *fw_cfg = NULL;
> >> PcGuestInfo *guest_info;
> >> ram_addr_t lowmem;
> >>
> >>@@ -190,6 +177,20 @@ static void pc_init1(MachineState *machine,
> >> } else {
> >> gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
> >> }
> >>+}
> >>+
> >>+static void 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 = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
> >>@@ -203,6 +204,28 @@ static void pc_init1(MachineState *machine,
> >> isa_bus = isa_bus_new(NULL, system_io);
> >> no_hpet = 1;
> >> }
> >>+}
> >>+
> >>+static void pc_machine_device_init(MachineState *machine,
> >>+ int pci_enabled,
> >>+ GSIState *gsi_state,
> >>+ DeviceState *icc_bridge,
> >>+ int piix3_devfn,
> >>+ FWCfgState *fw_cfg,
> >>+ PCIBus *pci_bus,
> >>+ ISABus *isa_bus,
> >>+ qemu_irq *gsi)
> >>+{
> >>+ int i;
> >>+ DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> >>+ BusState *idebus[MAX_IDE_BUS];
> >>+ qemu_irq *smi_irq;
> >>+ PCMachineState *pc_machine = PC_MACHINE(machine);
> >>+ qemu_irq *cpu_irq;
> >>+ qemu_irq *i8259;
> >>+ ISADevice *rtc_state;
> >>+ ISADevice *floppy;
> >>+
> >> isa_bus_irqs(isa_bus, gsi);
> >>
> >> if (kvm_irqchip_in_kernel()) {
> >>@@ -290,6 +313,30 @@ static void pc_init1(MachineState *machine,
> >> }
> >> }
> >>
> >>+/* PC hardware initialisation */
> >>+static void 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;
> >
> >These are set to NULL here and never modified below.
> >Why does it make sense?
> >
> >
> >>+
> >>+ pc_machine_base_init(machine, pci_enabled, kvmclock_enabled, icc_bridge,
> >>+ ram_memory, pci_memory, gsi, gsi_state, fw_cfg);
> >>+ 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);
> >>--
> >>1.9.1
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 2/4] xen:hw:pci-host:piix: create host bridge to passthrough
2014-07-24 11:30 [Qemu-devel] [PATCH 0/4] xen:passthrough: introduce a separate machine to igd passthrough Tiejun Chen
2014-07-24 11:30 ` [Qemu-devel] [PATCH 1/4] hw:i386:pc_piix: split pc_init1() Tiejun Chen
@ 2014-07-24 11:30 ` Tiejun Chen
2014-07-29 11:17 ` Michael S. Tsirkin
2014-07-24 11:30 ` [Qemu-devel] [PATCH 3/4] xen:hw:pci-host:piix: introduce xen_igd_i440fx_init Tiejun Chen
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Tiejun Chen @ 2014-07-24 11:30 UTC (permalink / raw)
To: mst, stefano.stabellini, pbonzini; +Cc: qemu-devel, xen-devel
Implement that pci host bridge to specific to passthrough. Actually
this just inherit the standard one.
This is based on http://patchwork.ozlabs.org/patch/363810/.
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
hw/pci-host/piix.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index e0e0946..9feddf5 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -34,6 +34,7 @@
#include "sysemu/sysemu.h"
#include "hw/i386/ioapic.h"
#include "qapi/visitor.h"
+#include "hw/xen/xen_pt.h"
/*
* I440FX chipset data sheet.
@@ -44,6 +45,10 @@
#define I440FX_PCI_HOST_BRIDGE(obj) \
OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_PCI_HOST_BRIDGE)
+#define TYPE_I440FX_XEN_PCI_DEVICE "i440FX-xen"
+#define I440FX_XEN_PCI_DEVICE(obj) \
+ OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_XEN_PCI_DEVICE)
+
typedef struct I440FXState {
PCIHostState parent_obj;
PcPciInfo pci_info;
@@ -305,6 +310,16 @@ static int i440fx_initfn(PCIDevice *dev)
return 0;
}
+static int i440fx_xen_initfn(PCIDevice *dev)
+{
+ PCII440FXState *d = I440FX_XEN_PCI_DEVICE(dev);
+
+ dev->config[I440FX_SMRAM] = 0x02;
+
+ cpu_smm_register(&i440fx_set_smm, d);
+ return 0;
+}
+
PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
int *piix3_devfn,
ISABus **isa_bus, qemu_irq *pic,
@@ -704,6 +719,33 @@ static const TypeInfo i440fx_info = {
.class_init = i440fx_class_init,
};
+static void i440fx_xen_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+ k->init = i440fx_xen_initfn;
+ k->vendor_id = PCI_VENDOR_ID_INTEL;
+ k->device_id = PCI_DEVICE_ID_INTEL_82441;
+ k->revision = 0x02;
+ k->class_id = PCI_CLASS_BRIDGE_ISA;
+ dc->desc = "XEN Host bridge";
+ dc->vmsd = &vmstate_i440fx;
+ /*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
+ */
+ dc->cannot_instantiate_with_device_add_yet = true;
+ dc->hotpluggable = false;
+}
+
+static const TypeInfo i440fx_xen_info = {
+ .name = TYPE_I440FX_XEN_PCI_DEVICE,
+ .parent = TYPE_PCI_DEVICE,
+ .instance_size = sizeof(PCII440FXState),
+ .class_init = i440fx_xen_class_init,
+};
+
static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
PCIBus *rootbus)
{
@@ -745,6 +787,7 @@ static const TypeInfo i440fx_pcihost_info = {
static void i440fx_register_types(void)
{
type_register_static(&i440fx_info);
+ type_register_static(&i440fx_xen_info);
type_register_static(&piix3_info);
type_register_static(&piix3_xen_info);
type_register_static(&i440fx_pcihost_info);
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 2/4] xen:hw:pci-host:piix: create host bridge to passthrough
2014-07-24 11:30 ` [Qemu-devel] [PATCH 2/4] xen:hw:pci-host:piix: create host bridge to passthrough Tiejun Chen
@ 2014-07-29 11:17 ` Michael S. Tsirkin
2014-07-30 8:20 ` Chen, Tiejun
0 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2014-07-29 11:17 UTC (permalink / raw)
To: Tiejun Chen; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
On Thu, Jul 24, 2014 at 07:30:27PM +0800, Tiejun Chen wrote:
> Implement that pci
s/that/a/
> host bridge to specific
s/to specific/specific/
> to passthrough. Actually
> this just inherit
s/inherit/inherits/
> the standard one.
>
> This is based on http://patchwork.ozlabs.org/patch/363810/.
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
> hw/pci-host/piix.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index e0e0946..9feddf5 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -34,6 +34,7 @@
> #include "sysemu/sysemu.h"
> #include "hw/i386/ioapic.h"
> #include "qapi/visitor.h"
> +#include "hw/xen/xen_pt.h"
What call needs this, exactly?
>
> /*
> * I440FX chipset data sheet.
> @@ -44,6 +45,10 @@
> #define I440FX_PCI_HOST_BRIDGE(obj) \
> OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_PCI_HOST_BRIDGE)
>
OK cool, but don't you want to put "igd" and "passthrough"
somewhere in the name? Maybe "legacy" as well since
future drivers will work with existing machine type, right?
Applies to functions and macro names as well.
> +#define TYPE_I440FX_XEN_PCI_DEVICE "i440FX-xen"
> +#define I440FX_XEN_PCI_DEVICE(obj) \
> + OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_XEN_PCI_DEVICE)
> +
> typedef struct I440FXState {
> PCIHostState parent_obj;
> PcPciInfo pci_info;
> @@ -305,6 +310,16 @@ static int i440fx_initfn(PCIDevice *dev)
> return 0;
> }
>
> +static int i440fx_xen_initfn(PCIDevice *dev)
> +{
> + PCII440FXState *d = I440FX_XEN_PCI_DEVICE(dev);
> +
> + dev->config[I440FX_SMRAM] = 0x02;
> +
> + cpu_smm_register(&i440fx_set_smm, d);
> + return 0;
> +}
> +
> PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> int *piix3_devfn,
> ISABus **isa_bus, qemu_irq *pic,
> @@ -704,6 +719,33 @@ static const TypeInfo i440fx_info = {
> .class_init = i440fx_class_init,
> };
>
> +static void i440fx_xen_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> + k->init = i440fx_xen_initfn;
> + k->vendor_id = PCI_VENDOR_ID_INTEL;
> + k->device_id = PCI_DEVICE_ID_INTEL_82441;
> + k->revision = 0x02;
> + k->class_id = PCI_CLASS_BRIDGE_ISA;
> + dc->desc = "XEN Host bridge";
> + dc->vmsd = &vmstate_i440fx;
> + /*
> + * PCI-facing part of the host bridge, not usable without the
> + * host-facing part, which can't be device_add'ed, yet.
> + */
> + dc->cannot_instantiate_with_device_add_yet = true;
> + dc->hotpluggable = false;
> +}
> +
> +static const TypeInfo i440fx_xen_info = {
> + .name = TYPE_I440FX_XEN_PCI_DEVICE,
> + .parent = TYPE_PCI_DEVICE,
> + .instance_size = sizeof(PCII440FXState),
> + .class_init = i440fx_xen_class_init,
> +};
> +
> static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
> PCIBus *rootbus)
> {
> @@ -745,6 +787,7 @@ static const TypeInfo i440fx_pcihost_info = {
> static void i440fx_register_types(void)
> {
> type_register_static(&i440fx_info);
> + type_register_static(&i440fx_xen_info);
> type_register_static(&piix3_info);
> type_register_static(&piix3_xen_info);
> type_register_static(&i440fx_pcihost_info);
> --
> 1.9.1
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 2/4] xen:hw:pci-host:piix: create host bridge to passthrough
2014-07-29 11:17 ` Michael S. Tsirkin
@ 2014-07-30 8:20 ` Chen, Tiejun
2014-07-30 17:45 ` Michael S. Tsirkin
0 siblings, 1 reply; 18+ messages in thread
From: Chen, Tiejun @ 2014-07-30 8:20 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
On 2014/7/29 19:17, Michael S. Tsirkin wrote:
> On Thu, Jul 24, 2014 at 07:30:27PM +0800, Tiejun Chen wrote:
>> Implement that pci
>
> s/that/a/
Fixed.
>
>> host bridge to specific
>
> s/to specific/specific/
Fixed.
>
>> to passthrough. Actually
>> this just inherit
>
> s/inherit/inherits/
Fixed.
>
>> the standard one.
>>
>> This is based on http://patchwork.ozlabs.org/patch/363810/.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>> hw/pci-host/piix.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 43 insertions(+)
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index e0e0946..9feddf5 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -34,6 +34,7 @@
>> #include "sysemu/sysemu.h"
>> #include "hw/i386/ioapic.h"
>> #include "qapi/visitor.h"
>> +#include "hw/xen/xen_pt.h"
>
> What call needs this, exactly?
Another fault when I grab something from the original patches.
>
>>
>> /*
>> * I440FX chipset data sheet.
>> @@ -44,6 +45,10 @@
>> #define I440FX_PCI_HOST_BRIDGE(obj) \
>> OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_PCI_HOST_BRIDGE)
>>
>
> OK cool, but don't you want to put "igd" and "passthrough"
> somewhere in the name? Maybe "legacy" as well since
> future drivers will work with existing machine type, right?
Looks good so what about this prefix, IGD_PT_XXX/igd_pt_xxx?
>
> Applies to functions and macro names as well.
>
>> +#define TYPE_I440FX_XEN_PCI_DEVICE "i440FX-xen"
>> +#define I440FX_XEN_PCI_DEVICE(obj) \
>> + OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_XEN_PCI_DEVICE)
>> +
>> typedef struct I440FXState {
>> PCIHostState parent_obj;
>> PcPciInfo pci_info;
>> @@ -305,6 +310,16 @@ static int i440fx_initfn(PCIDevice *dev)
>> return 0;
>> }
>>
>> +static int i440fx_xen_initfn(PCIDevice *dev)
>> +{
>> + PCII440FXState *d = I440FX_XEN_PCI_DEVICE(dev);
>> +
>> + dev->config[I440FX_SMRAM] = 0x02;
>> +
>> + cpu_smm_register(&i440fx_set_smm, d);
>> + return 0;
>> +}
>> +
>> PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
>> int *piix3_devfn,
>> ISABus **isa_bus, qemu_irq *pic,
>> @@ -704,6 +719,33 @@ static const TypeInfo i440fx_info = {
>> .class_init = i440fx_class_init,
>> };
>>
>> +static void i440fx_xen_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> +
>> + k->init = i440fx_xen_initfn;
>> + k->vendor_id = PCI_VENDOR_ID_INTEL;
>> + k->device_id = PCI_DEVICE_ID_INTEL_82441;
>> + k->revision = 0x02;
>> + k->class_id = PCI_CLASS_BRIDGE_ISA;
>> + dc->desc = "XEN Host bridge";
>> + dc->vmsd = &vmstate_i440fx;
>> + /*
>> + * PCI-facing part of the host bridge, not usable without the
>> + * host-facing part, which can't be device_add'ed, yet.
>> + */
>> + dc->cannot_instantiate_with_device_add_yet = true;
>> + dc->hotpluggable = false;
>> +}
>> +
>> +static const TypeInfo i440fx_xen_info = {
>> + .name = TYPE_I440FX_XEN_PCI_DEVICE,
>> + .parent = TYPE_PCI_DEVICE,
>> + .instance_size = sizeof(PCII440FXState),
>> + .class_init = i440fx_xen_class_init,
>> +};
>> +
>> static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>> PCIBus *rootbus)
>> {
>> @@ -745,6 +787,7 @@ static const TypeInfo i440fx_pcihost_info = {
>> static void i440fx_register_types(void)
>> {
>> type_register_static(&i440fx_info);
>> + type_register_static(&i440fx_xen_info);
>> type_register_static(&piix3_info);
>> type_register_static(&piix3_xen_info);
>> type_register_static(&i440fx_pcihost_info);
>> --
>> 1.9.1
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 2/4] xen:hw:pci-host:piix: create host bridge to passthrough
2014-07-30 8:20 ` Chen, Tiejun
@ 2014-07-30 17:45 ` Michael S. Tsirkin
0 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2014-07-30 17:45 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
On Wed, Jul 30, 2014 at 04:20:13PM +0800, Chen, Tiejun wrote:
> On 2014/7/29 19:17, Michael S. Tsirkin wrote:
> >On Thu, Jul 24, 2014 at 07:30:27PM +0800, Tiejun Chen wrote:
> >>Implement that pci
> >
> >s/that/a/
>
> Fixed.
>
> >
> >>host bridge to specific
> >
> >s/to specific/specific/
>
> Fixed.
>
> >
> >>to passthrough. Actually
> >>this just inherit
> >
> >s/inherit/inherits/
>
> Fixed.
>
> >
> >>the standard one.
> >>
> >>This is based on http://patchwork.ozlabs.org/patch/363810/.
> >>
> >>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>---
> >> hw/pci-host/piix.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 43 insertions(+)
> >>
> >>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>index e0e0946..9feddf5 100644
> >>--- a/hw/pci-host/piix.c
> >>+++ b/hw/pci-host/piix.c
> >>@@ -34,6 +34,7 @@
> >> #include "sysemu/sysemu.h"
> >> #include "hw/i386/ioapic.h"
> >> #include "qapi/visitor.h"
> >>+#include "hw/xen/xen_pt.h"
> >
> >What call needs this, exactly?
>
> Another fault when I grab something from the original patches.
>
> >
> >>
> >> /*
> >> * I440FX chipset data sheet.
> >>@@ -44,6 +45,10 @@
> >> #define I440FX_PCI_HOST_BRIDGE(obj) \
> >> OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_PCI_HOST_BRIDGE)
> >>
> >
> >OK cool, but don't you want to put "igd" and "passthrough"
> >somewhere in the name? Maybe "legacy" as well since
> >future drivers will work with existing machine type, right?
>
> Looks good so what about this prefix, IGD_PT_XXX/igd_pt_xxx?
OK though I think a more verbose PASSTROUGH is a bit clearer.
Is it xen-specific in some way, or usable with kvm/tcg as well?
If specific it needs to include XEN_ prefix as well.
> >
> >Applies to functions and macro names as well.
> >
> >>+#define TYPE_I440FX_XEN_PCI_DEVICE "i440FX-xen"
> >>+#define I440FX_XEN_PCI_DEVICE(obj) \
> >>+ OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_XEN_PCI_DEVICE)
> >>+
> >> typedef struct I440FXState {
> >> PCIHostState parent_obj;
> >> PcPciInfo pci_info;
> >>@@ -305,6 +310,16 @@ static int i440fx_initfn(PCIDevice *dev)
> >> return 0;
> >> }
> >>
> >>+static int i440fx_xen_initfn(PCIDevice *dev)
> >>+{
> >>+ PCII440FXState *d = I440FX_XEN_PCI_DEVICE(dev);
> >>+
> >>+ dev->config[I440FX_SMRAM] = 0x02;
> >>+
> >>+ cpu_smm_register(&i440fx_set_smm, d);
> >>+ return 0;
> >>+}
> >>+
> >> PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> >> int *piix3_devfn,
> >> ISABus **isa_bus, qemu_irq *pic,
> >>@@ -704,6 +719,33 @@ static const TypeInfo i440fx_info = {
> >> .class_init = i440fx_class_init,
> >> };
> >>
> >>+static void i440fx_xen_class_init(ObjectClass *klass, void *data)
> >>+{
> >>+ DeviceClass *dc = DEVICE_CLASS(klass);
> >>+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> >>+
> >>+ k->init = i440fx_xen_initfn;
> >>+ k->vendor_id = PCI_VENDOR_ID_INTEL;
> >>+ k->device_id = PCI_DEVICE_ID_INTEL_82441;
> >>+ k->revision = 0x02;
> >>+ k->class_id = PCI_CLASS_BRIDGE_ISA;
> >>+ dc->desc = "XEN Host bridge";
> >>+ dc->vmsd = &vmstate_i440fx;
> >>+ /*
> >>+ * PCI-facing part of the host bridge, not usable without the
> >>+ * host-facing part, which can't be device_add'ed, yet.
> >>+ */
> >>+ dc->cannot_instantiate_with_device_add_yet = true;
> >>+ dc->hotpluggable = false;
> >>+}
> >>+
> >>+static const TypeInfo i440fx_xen_info = {
> >>+ .name = TYPE_I440FX_XEN_PCI_DEVICE,
> >>+ .parent = TYPE_PCI_DEVICE,
> >>+ .instance_size = sizeof(PCII440FXState),
> >>+ .class_init = i440fx_xen_class_init,
> >>+};
> >>+
> >> static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
> >> PCIBus *rootbus)
> >> {
> >>@@ -745,6 +787,7 @@ static const TypeInfo i440fx_pcihost_info = {
> >> static void i440fx_register_types(void)
> >> {
> >> type_register_static(&i440fx_info);
> >>+ type_register_static(&i440fx_xen_info);
> >> type_register_static(&piix3_info);
> >> type_register_static(&piix3_xen_info);
> >> type_register_static(&i440fx_pcihost_info);
> >>--
> >>1.9.1
> >
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 3/4] xen:hw:pci-host:piix: introduce xen_igd_i440fx_init
2014-07-24 11:30 [Qemu-devel] [PATCH 0/4] xen:passthrough: introduce a separate machine to igd passthrough Tiejun Chen
2014-07-24 11:30 ` [Qemu-devel] [PATCH 1/4] hw:i386:pc_piix: split pc_init1() Tiejun Chen
2014-07-24 11:30 ` [Qemu-devel] [PATCH 2/4] xen:hw:pci-host:piix: create host bridge to passthrough Tiejun Chen
@ 2014-07-24 11:30 ` Tiejun Chen
2014-07-29 11:19 ` Michael S. Tsirkin
2014-07-24 11:30 ` [Qemu-devel] [PATCH 4/4] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough Tiejun Chen
2014-07-29 5:45 ` [Qemu-devel] [PATCH 0/4] xen:passthrough: introduce a separate machine to igd passthrough Chen, Tiejun
4 siblings, 1 reply; 18+ messages in thread
From: Tiejun Chen @ 2014-07-24 11:30 UTC (permalink / raw)
To: mst, stefano.stabellini, pbonzini; +Cc: qemu-devel, xen-devel
This is almost same as an original i440fx_init but just
work with that xen igd host bridge to passthrough.
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
hw/pci-host/piix.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/i386/pc.h | 10 +++++++
2 files changed, 89 insertions(+)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 9feddf5..7ef08d7 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -407,6 +407,85 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
return b;
}
+PCIBus *xen_igd_i440fx_init(PCII440FXState **pi440fx_state,
+ int *piix3_devfn,
+ ISABus **isa_bus, qemu_irq *pic,
+ MemoryRegion *address_space_mem,
+ MemoryRegion *address_space_io,
+ ram_addr_t ram_size,
+ ram_addr_t below_4g_mem_size,
+ ram_addr_t above_4g_mem_size,
+ MemoryRegion *pci_address_space,
+ MemoryRegion *ram_memory)
+{
+ DeviceState *dev;
+ PCIBus *b;
+ PCIDevice *d;
+ PCIHostState *s;
+ PIIX3State *piix3;
+ PCII440FXState *f;
+ unsigned i;
+ I440FXState *i440fx;
+
+ dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
+ s = PCI_HOST_BRIDGE(dev);
+ b = pci_bus_new(dev, NULL, pci_address_space,
+ address_space_io, 0, TYPE_PCI_BUS);
+ s->bus = b;
+ object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
+ qdev_init_nofail(dev);
+
+ d = pci_create_simple(b, 0, TYPE_I440FX_XEN_PCI_DEVICE);
+ *pi440fx_state = I440FX_XEN_PCI_DEVICE(d);
+ f = *pi440fx_state;
+ f->system_memory = address_space_mem;
+ f->pci_address_space = pci_address_space;
+ f->ram_memory = ram_memory;
+
+ i440fx = I440FX_PCI_HOST_BRIDGE(dev);
+ i440fx->pci_info.w32.begin = below_4g_mem_size;
+
+ /* setup pci memory mapping */
+ pc_pci_as_mapping_init(OBJECT(f), f->system_memory,
+ f->pci_address_space);
+
+ memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
+ f->pci_address_space, 0xa0000, 0x20000);
+ memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
+ &f->smram_region, 1);
+ memory_region_set_enabled(&f->smram_region, false);
+ init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
+ &f->pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
+ for (i = 0; i < 12; ++i) {
+ init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
+ &f->pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE,
+ PAM_EXPAN_SIZE);
+ }
+
+ /* Xen supports additional interrupt routes from the PCI devices to
+ * the IOAPIC: the four pins of each PCI device on the bus are also
+ * connected to the IOAPIC directly.
+ * These additional routes can be discovered through ACPI. */
+ piix3 = DO_UPCAST(PIIX3State, dev,
+ pci_create_simple_multifunction(b, -1, true, "PIIX3-xen"));
+ pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
+ piix3, XEN_PIIX_NUM_PIRQS);
+ piix3->pic = pic;
+ *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
+
+ *piix3_devfn = piix3->dev.devfn;
+
+ ram_size = ram_size / 8 / 1024 / 1024;
+ if (ram_size > 255) {
+ ram_size = 255;
+ }
+ d->config[0x57] = ram_size;
+
+ i440fx_update_memory_mappings(f);
+
+ return b;
+}
+
PCIBus *find_i440fx(void)
{
PCIHostState *s = OBJECT_CHECK(PCIHostState,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1c0c382..51656d9 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -239,6 +239,16 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
MemoryRegion *pci_memory,
MemoryRegion *ram_memory);
+PCIBus *xen_igd_i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
+ ISABus **isa_bus, qemu_irq *pic,
+ MemoryRegion *address_space_mem,
+ MemoryRegion *address_space_io,
+ ram_addr_t ram_size,
+ ram_addr_t below_4g_mem_size,
+ ram_addr_t above_4g_mem_size,
+ MemoryRegion *pci_memory,
+ MemoryRegion *ram_memory);
+
PCIBus *find_i440fx(void);
/* piix4.c */
extern PCIDevice *piix4_dev;
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 3/4] xen:hw:pci-host:piix: introduce xen_igd_i440fx_init
2014-07-24 11:30 ` [Qemu-devel] [PATCH 3/4] xen:hw:pci-host:piix: introduce xen_igd_i440fx_init Tiejun Chen
@ 2014-07-29 11:19 ` Michael S. Tsirkin
2014-07-30 8:24 ` Chen, Tiejun
0 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2014-07-29 11:19 UTC (permalink / raw)
To: Tiejun Chen; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
On Thu, Jul 24, 2014 at 07:30:28PM +0800, Tiejun Chen wrote:
> This is almost same as an original i440fx_init but just
> work with that xen igd host bridge to passthrough.
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
> hw/pci-host/piix.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 10 +++++++
> 2 files changed, 89 insertions(+)
>
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 9feddf5..7ef08d7 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -407,6 +407,85 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> return b;
> }
>
> +PCIBus *xen_igd_i440fx_init(PCII440FXState **pi440fx_state,
> + int *piix3_devfn,
> + ISABus **isa_bus, qemu_irq *pic,
> + MemoryRegion *address_space_mem,
> + MemoryRegion *address_space_io,
> + ram_addr_t ram_size,
> + ram_addr_t below_4g_mem_size,
> + ram_addr_t above_4g_mem_size,
> + MemoryRegion *pci_address_space,
> + MemoryRegion *ram_memory)
> +{
> + DeviceState *dev;
> + PCIBus *b;
> + PCIDevice *d;
> + PCIHostState *s;
> + PIIX3State *piix3;
> + PCII440FXState *f;
> + unsigned i;
> + I440FXState *i440fx;
> +
> + dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
> + s = PCI_HOST_BRIDGE(dev);
> + b = pci_bus_new(dev, NULL, pci_address_space,
> + address_space_io, 0, TYPE_PCI_BUS);
> + s->bus = b;
> + object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
> + qdev_init_nofail(dev);
> +
> + d = pci_create_simple(b, 0, TYPE_I440FX_XEN_PCI_DEVICE);
> + *pi440fx_state = I440FX_XEN_PCI_DEVICE(d);
> + f = *pi440fx_state;
> + f->system_memory = address_space_mem;
> + f->pci_address_space = pci_address_space;
> + f->ram_memory = ram_memory;
> +
> + i440fx = I440FX_PCI_HOST_BRIDGE(dev);
> + i440fx->pci_info.w32.begin = below_4g_mem_size;
> +
> + /* setup pci memory mapping */
> + pc_pci_as_mapping_init(OBJECT(f), f->system_memory,
> + f->pci_address_space);
> +
> + memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
> + f->pci_address_space, 0xa0000, 0x20000);
> + memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
> + &f->smram_region, 1);
> + memory_region_set_enabled(&f->smram_region, false);
> + init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
> + &f->pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
> + for (i = 0; i < 12; ++i) {
> + init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
> + &f->pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE,
> + PAM_EXPAN_SIZE);
> + }
> +
> + /* Xen supports additional interrupt routes from the PCI devices to
> + * the IOAPIC: the four pins of each PCI device on the bus are also
> + * connected to the IOAPIC directly.
> + * These additional routes can be discovered through ACPI. */
> + piix3 = DO_UPCAST(PIIX3State, dev,
> + pci_create_simple_multifunction(b, -1, true, "PIIX3-xen"));
> + pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
> + piix3, XEN_PIIX_NUM_PIRQS);
> + piix3->pic = pic;
> + *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
> +
> + *piix3_devfn = piix3->dev.devfn;
> +
> + ram_size = ram_size / 8 / 1024 / 1024;
> + if (ram_size > 255) {
> + ram_size = 255;
> + }
> + d->config[0x57] = ram_size;
> +
> + i440fx_update_memory_mappings(f);
> +
> + return b;
> +}
Too much copy-paste. Please refactor to avoid code duplication.
Is the only difference here the use of TYPE_I440FX_XEN_PCI_DEVICE?
Then you can pass type in as a parameter to a common static sub-function.
> +
> PCIBus *find_i440fx(void)
> {
> PCIHostState *s = OBJECT_CHECK(PCIHostState,
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 1c0c382..51656d9 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -239,6 +239,16 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
> MemoryRegion *pci_memory,
> MemoryRegion *ram_memory);
>
> +PCIBus *xen_igd_i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
> + ISABus **isa_bus, qemu_irq *pic,
> + MemoryRegion *address_space_mem,
> + MemoryRegion *address_space_io,
> + ram_addr_t ram_size,
> + ram_addr_t below_4g_mem_size,
> + ram_addr_t above_4g_mem_size,
> + MemoryRegion *pci_memory,
> + MemoryRegion *ram_memory);
> +
> PCIBus *find_i440fx(void);
> /* piix4.c */
> extern PCIDevice *piix4_dev;
> --
> 1.9.1
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 3/4] xen:hw:pci-host:piix: introduce xen_igd_i440fx_init
2014-07-29 11:19 ` Michael S. Tsirkin
@ 2014-07-30 8:24 ` Chen, Tiejun
2014-07-30 17:46 ` Michael S. Tsirkin
0 siblings, 1 reply; 18+ messages in thread
From: Chen, Tiejun @ 2014-07-30 8:24 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
On 2014/7/29 19:19, Michael S. Tsirkin wrote:
> On Thu, Jul 24, 2014 at 07:30:28PM +0800, Tiejun Chen wrote:
>> This is almost same as an original i440fx_init but just
>> work with that xen igd host bridge to passthrough.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>> hw/pci-host/piix.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> include/hw/i386/pc.h | 10 +++++++
>> 2 files changed, 89 insertions(+)
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index 9feddf5..7ef08d7 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -407,6 +407,85 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
>> return b;
>> }
>>
>> +PCIBus *xen_igd_i440fx_init(PCII440FXState **pi440fx_state,
>> + int *piix3_devfn,
>> + ISABus **isa_bus, qemu_irq *pic,
>> + MemoryRegion *address_space_mem,
>> + MemoryRegion *address_space_io,
>> + ram_addr_t ram_size,
>> + ram_addr_t below_4g_mem_size,
>> + ram_addr_t above_4g_mem_size,
>> + MemoryRegion *pci_address_space,
>> + MemoryRegion *ram_memory)
>> +{
>> + DeviceState *dev;
>> + PCIBus *b;
>> + PCIDevice *d;
>> + PCIHostState *s;
>> + PIIX3State *piix3;
>> + PCII440FXState *f;
>> + unsigned i;
>> + I440FXState *i440fx;
>> +
>> + dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
>> + s = PCI_HOST_BRIDGE(dev);
>> + b = pci_bus_new(dev, NULL, pci_address_space,
>> + address_space_io, 0, TYPE_PCI_BUS);
>> + s->bus = b;
>> + object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
>> + qdev_init_nofail(dev);
>> +
>> + d = pci_create_simple(b, 0, TYPE_I440FX_XEN_PCI_DEVICE);
>> + *pi440fx_state = I440FX_XEN_PCI_DEVICE(d);
>> + f = *pi440fx_state;
>> + f->system_memory = address_space_mem;
>> + f->pci_address_space = pci_address_space;
>> + f->ram_memory = ram_memory;
>> +
>> + i440fx = I440FX_PCI_HOST_BRIDGE(dev);
>> + i440fx->pci_info.w32.begin = below_4g_mem_size;
>> +
>> + /* setup pci memory mapping */
>> + pc_pci_as_mapping_init(OBJECT(f), f->system_memory,
>> + f->pci_address_space);
>> +
>> + memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
>> + f->pci_address_space, 0xa0000, 0x20000);
>> + memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
>> + &f->smram_region, 1);
>> + memory_region_set_enabled(&f->smram_region, false);
>> + init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
>> + &f->pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
>> + for (i = 0; i < 12; ++i) {
>> + init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
>> + &f->pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE,
>> + PAM_EXPAN_SIZE);
>> + }
>> +
>> + /* Xen supports additional interrupt routes from the PCI devices to
>> + * the IOAPIC: the four pins of each PCI device on the bus are also
>> + * connected to the IOAPIC directly.
>> + * These additional routes can be discovered through ACPI. */
>> + piix3 = DO_UPCAST(PIIX3State, dev,
>> + pci_create_simple_multifunction(b, -1, true, "PIIX3-xen"));
>> + pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
>> + piix3, XEN_PIIX_NUM_PIRQS);
>> + piix3->pic = pic;
>> + *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
>> +
>> + *piix3_devfn = piix3->dev.devfn;
>> +
>> + ram_size = ram_size / 8 / 1024 / 1024;
>> + if (ram_size > 255) {
>> + ram_size = 255;
>> + }
>> + d->config[0x57] = ram_size;
>> +
>> + i440fx_update_memory_mappings(f);
>> +
>> + return b;
>> +}
>
> Too much copy-paste. Please refactor to avoid code duplication.
Okay.
> Is the only difference here the use of TYPE_I440FX_XEN_PCI_DEVICE?
Yes.
> Then you can pass type in as a parameter to a common static sub-function.
I'm fine but as I remember Paolo don't like we intervene a common
function. And we'll introduce something specific to IGD, like that faked
PCIe device.
Thanks
Tiejun
>
>> +
>> PCIBus *find_i440fx(void)
>> {
>> PCIHostState *s = OBJECT_CHECK(PCIHostState,
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 1c0c382..51656d9 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -239,6 +239,16 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
>> MemoryRegion *pci_memory,
>> MemoryRegion *ram_memory);
>>
>> +PCIBus *xen_igd_i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
>> + ISABus **isa_bus, qemu_irq *pic,
>> + MemoryRegion *address_space_mem,
>> + MemoryRegion *address_space_io,
>> + ram_addr_t ram_size,
>> + ram_addr_t below_4g_mem_size,
>> + ram_addr_t above_4g_mem_size,
>> + MemoryRegion *pci_memory,
>> + MemoryRegion *ram_memory);
>> +
>> PCIBus *find_i440fx(void);
>> /* piix4.c */
>> extern PCIDevice *piix4_dev;
>> --
>> 1.9.1
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 3/4] xen:hw:pci-host:piix: introduce xen_igd_i440fx_init
2014-07-30 8:24 ` Chen, Tiejun
@ 2014-07-30 17:46 ` Michael S. Tsirkin
0 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2014-07-30 17:46 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
On Wed, Jul 30, 2014 at 04:24:25PM +0800, Chen, Tiejun wrote:
> On 2014/7/29 19:19, Michael S. Tsirkin wrote:
> >On Thu, Jul 24, 2014 at 07:30:28PM +0800, Tiejun Chen wrote:
> >>This is almost same as an original i440fx_init but just
> >>work with that xen igd host bridge to passthrough.
> >>
> >>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>---
> >> hw/pci-host/piix.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> include/hw/i386/pc.h | 10 +++++++
> >> 2 files changed, 89 insertions(+)
> >>
> >>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>index 9feddf5..7ef08d7 100644
> >>--- a/hw/pci-host/piix.c
> >>+++ b/hw/pci-host/piix.c
> >>@@ -407,6 +407,85 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> >> return b;
> >> }
> >>
> >>+PCIBus *xen_igd_i440fx_init(PCII440FXState **pi440fx_state,
> >>+ int *piix3_devfn,
> >>+ ISABus **isa_bus, qemu_irq *pic,
> >>+ MemoryRegion *address_space_mem,
> >>+ MemoryRegion *address_space_io,
> >>+ ram_addr_t ram_size,
> >>+ ram_addr_t below_4g_mem_size,
> >>+ ram_addr_t above_4g_mem_size,
> >>+ MemoryRegion *pci_address_space,
> >>+ MemoryRegion *ram_memory)
> >>+{
> >>+ DeviceState *dev;
> >>+ PCIBus *b;
> >>+ PCIDevice *d;
> >>+ PCIHostState *s;
> >>+ PIIX3State *piix3;
> >>+ PCII440FXState *f;
> >>+ unsigned i;
> >>+ I440FXState *i440fx;
> >>+
> >>+ dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
> >>+ s = PCI_HOST_BRIDGE(dev);
> >>+ b = pci_bus_new(dev, NULL, pci_address_space,
> >>+ address_space_io, 0, TYPE_PCI_BUS);
> >>+ s->bus = b;
> >>+ object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
> >>+ qdev_init_nofail(dev);
> >>+
> >>+ d = pci_create_simple(b, 0, TYPE_I440FX_XEN_PCI_DEVICE);
> >>+ *pi440fx_state = I440FX_XEN_PCI_DEVICE(d);
> >>+ f = *pi440fx_state;
> >>+ f->system_memory = address_space_mem;
> >>+ f->pci_address_space = pci_address_space;
> >>+ f->ram_memory = ram_memory;
> >>+
> >>+ i440fx = I440FX_PCI_HOST_BRIDGE(dev);
> >>+ i440fx->pci_info.w32.begin = below_4g_mem_size;
> >>+
> >>+ /* setup pci memory mapping */
> >>+ pc_pci_as_mapping_init(OBJECT(f), f->system_memory,
> >>+ f->pci_address_space);
> >>+
> >>+ memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
> >>+ f->pci_address_space, 0xa0000, 0x20000);
> >>+ memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
> >>+ &f->smram_region, 1);
> >>+ memory_region_set_enabled(&f->smram_region, false);
> >>+ init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
> >>+ &f->pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
> >>+ for (i = 0; i < 12; ++i) {
> >>+ init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
> >>+ &f->pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE,
> >>+ PAM_EXPAN_SIZE);
> >>+ }
> >>+
> >>+ /* Xen supports additional interrupt routes from the PCI devices to
> >>+ * the IOAPIC: the four pins of each PCI device on the bus are also
> >>+ * connected to the IOAPIC directly.
> >>+ * These additional routes can be discovered through ACPI. */
> >>+ piix3 = DO_UPCAST(PIIX3State, dev,
> >>+ pci_create_simple_multifunction(b, -1, true, "PIIX3-xen"));
> >>+ pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
> >>+ piix3, XEN_PIIX_NUM_PIRQS);
> >>+ piix3->pic = pic;
> >>+ *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
> >>+
> >>+ *piix3_devfn = piix3->dev.devfn;
> >>+
> >>+ ram_size = ram_size / 8 / 1024 / 1024;
> >>+ if (ram_size > 255) {
> >>+ ram_size = 255;
> >>+ }
> >>+ d->config[0x57] = ram_size;
> >>+
> >>+ i440fx_update_memory_mappings(f);
> >>+
> >>+ return b;
> >>+}
> >
> >Too much copy-paste. Please refactor to avoid code duplication.
>
> Okay.
>
> >Is the only difference here the use of TYPE_I440FX_XEN_PCI_DEVICE?
>
> Yes.
>
> >Then you can pass type in as a parameter to a common static sub-function.
>
> I'm fine but as I remember Paolo don't like we intervene a common function.
> And we'll introduce something specific to IGD, like that faked PCIe device.
>
> Thanks
> Tiejun
Exactly, so in this way, there is no "if (xen-igd-pt)" in common code.
> >
> >>+
> >> PCIBus *find_i440fx(void)
> >> {
> >> PCIHostState *s = OBJECT_CHECK(PCIHostState,
> >>diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >>index 1c0c382..51656d9 100644
> >>--- a/include/hw/i386/pc.h
> >>+++ b/include/hw/i386/pc.h
> >>@@ -239,6 +239,16 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
> >> MemoryRegion *pci_memory,
> >> MemoryRegion *ram_memory);
> >>
> >>+PCIBus *xen_igd_i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
> >>+ ISABus **isa_bus, qemu_irq *pic,
> >>+ MemoryRegion *address_space_mem,
> >>+ MemoryRegion *address_space_io,
> >>+ ram_addr_t ram_size,
> >>+ ram_addr_t below_4g_mem_size,
> >>+ ram_addr_t above_4g_mem_size,
> >>+ MemoryRegion *pci_memory,
> >>+ MemoryRegion *ram_memory);
> >>+
> >> PCIBus *find_i440fx(void);
> >> /* piix4.c */
> >> extern PCIDevice *piix4_dev;
> >>--
> >>1.9.1
> >
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 4/4] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough
2014-07-24 11:30 [Qemu-devel] [PATCH 0/4] xen:passthrough: introduce a separate machine to igd passthrough Tiejun Chen
` (2 preceding siblings ...)
2014-07-24 11:30 ` [Qemu-devel] [PATCH 3/4] xen:hw:pci-host:piix: introduce xen_igd_i440fx_init Tiejun Chen
@ 2014-07-24 11:30 ` Tiejun Chen
2014-07-29 11:29 ` Michael S. Tsirkin
2014-07-29 5:45 ` [Qemu-devel] [PATCH 0/4] xen:passthrough: introduce a separate machine to igd passthrough Chen, Tiejun
4 siblings, 1 reply; 18+ messages in thread
From: Tiejun Chen @ 2014-07-24 11:30 UTC (permalink / raw)
To: mst, stefano.stabellini, pbonzini; +Cc: qemu-devel, xen-devel
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;
+ isa_bus = isa_bus_new(NULL, system_io);
+ no_hpet = 1;
+ }
+}
+
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough
2014-07-24 11:30 ` [Qemu-devel] [PATCH 4/4] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough Tiejun Chen
@ 2014-07-29 11:29 ` Michael S. Tsirkin
2014-07-30 8:31 ` Chen, Tiejun
0 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2014-07-29 11:29 UTC (permalink / raw)
To: Tiejun Chen; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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?
> + 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.
> +}
> +
> 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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough
2014-07-29 11:29 ` Michael S. Tsirkin
@ 2014-07-30 8:31 ` Chen, Tiejun
2014-07-30 17:47 ` Michael S. Tsirkin
0 siblings, 1 reply; 18+ messages in thread
From: Chen, Tiejun @ 2014-07-30 8:31 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough
2014-07-30 8:31 ` Chen, Tiejun
@ 2014-07-30 17:47 ` Michael S. Tsirkin
0 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2014-07-30 17:47 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
On Wed, Jul 30, 2014 at 04:31:24PM +0800, Chen, Tiejun wrote:
> 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().
Except you broke that, and here you copy-pasted the bugs.
> >
> >>+ 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
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/4] xen:passthrough: introduce a separate machine to igd passthrough
2014-07-24 11:30 [Qemu-devel] [PATCH 0/4] xen:passthrough: introduce a separate machine to igd passthrough Tiejun Chen
` (3 preceding siblings ...)
2014-07-24 11:30 ` [Qemu-devel] [PATCH 4/4] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough Tiejun Chen
@ 2014-07-29 5:45 ` Chen, Tiejun
4 siblings, 0 replies; 18+ messages in thread
From: Chen, Tiejun @ 2014-07-29 5:45 UTC (permalink / raw)
To: mst, stefano.stabellini, pbonzini; +Cc: qemu-devel, xen-devel
Michael, Paolo and Stefano,
Any comments?
Thanks
Tiejun
On 2014/7/24 19:30, Tiejun Chen wrote:
> As we discussed currently we have to introduce a separate machine
> to work out igd passthrough.
>
> ----------------------------------------------------------------
> Tiejun Chen (4):
> hw:i386:pc_piix: split pc_init1()
> xen:hw:pci-host:piix: create host bridge to passthrough
> xen:hw:pci-host:piix: introduce xen_igd_i440fx_init
> xen:hw:i386:pc_piix: introduce new machine for IGD passthrough
>
> hw/i386/pc_piix.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------
> hw/pci-host/piix.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 10 ++++++++++
> 3 files changed, 289 insertions(+), 23 deletions(-)
>
> Thanks
> Tiejun
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread