[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [v2][PATCH 2/5] hw:pci-host:piix: split i440fx_init
On 2014/7/31 23:44, Michael S. Tsirkin wrote:
On Thu, Jul 31, 2014 at 06:12:32PM +0800, Chen, Tiejun wrote:
On 2014/7/31 17:53, Michael S. Tsirkin wrote:
On Thu, Jul 31, 2014 at 05:26:41PM +0800, Chen, Tiejun wrote:
On 2014/7/31 17:10, Michael S. Tsirkin wrote:
On Thu, Jul 31, 2014 at 02:31:36PM +0800, Tiejun Chen wrote:
We'd like to split i440fx_init and then we can share something
with other stuff.
Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
I think this is too much work for very little benefit.
Just pass const char *type to i440fx_init.
You know we will introduce that faked PCIe device represented that PCH
later,
Later when? On top of this patch series? Would like to see it all
before applying this ...
I will send this with other IGD stuff after this series is fine to you since
its just creating a simple PCIe device.
I think you should know this whole story since as you guys discussed we
don't fix that PCH at 1f.0.
And this works with all legacy drivers and with windows
drivers?
If we want to support these drivers, we still need to fix PCH at 1f.0.
I think we can discuss this in detail when I send patch to address this
point later. But now I don't think that block this patches series since
this is about how to do *inside* that creating PCIe device subfunction.
This is also why I try to the whole IGD patches, respectively since they
always bring more comments or discussion, then it will be difficult to
cover them at the same time by myself.
I hope I can do this step by step address all your comments until you
guys expect :)
Thanks
Tiejun
So it may be like this,
static int create_pseudo_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
{
struct PCIDevice *dev;
char rid;
/* We havt to use a simple PCI device to fake this ISA bridge
* to avoid making some confusion to BIOS and ACPI.
*/
dev = pci_create(bus, -1, "pseudo-intel-pch-isa-bridge");
qdev_init_nofail(&dev->qdev);
pci_config_set_vendor_id(dev->config, XEN_SUBSYSTEM_ID));
^
I don't remember this exactly.
pci_config_set_device_id(dev->config, hdev->device_id);
This is a hack anyway, how about reverse-decoding
required device id from the supplied card instead of
poking at the host?
You can do it when card is initialized.
return 0;
}
so how to distinguish them? Are you saying I should check the type
like this?
if(Xen-Type)
{}
else
{}
No! Put the code in init function for the respective class,
pass type as an argument:
If you mean we don't introduce any "if/else", I still don't understand how
to insert such that function above, could you show this exactly?
Tiejun
Stick it in the constructor for your xen pt device.
----
i440fx: make types configurable at run-time
Xen wants to supply a different pci and host devices,
inheriting i440fx devices. Make types configurable.
Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
-->
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index be8fdfe..86f295a 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -230,7 +230,11 @@ extern int no_hpet;
struct PCII440FXState;
typedef struct PCII440FXState PCII440FXState;
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
+#define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
+#define TYPE_I440FX_PCI_DEVICE "i440FX"
+
+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
+ PCII440FXState **pi440fx_state, int *piix_devfn,
ISABus **isa_bus, qemu_irq *pic,
MemoryRegion *address_space_mem,
MemoryRegion *address_space_io,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 31125b7..e0979cd 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -194,7 +194,9 @@ static void pc_init1(MachineState *machine,
}
if (pci_enabled) {
- pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
+ pci_bus = i440fx_init(TYPE_I440FX_PCI_HOST_BRIDGE,
+ TYPE_I440FX_PCI_DEVICE,
+ &i440fx_state, &piix3_devfn, &isa_bus, gsi,
system_memory, system_io, machine->ram_size,
below_4g_mem_size,
above_4g_mem_size,
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index e0e0946..0cd82b8 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -40,7 +40,6 @@
* http://download.intel.com/design/chipsets/datashts/29054901.pdf
*/
-#define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
#define I440FX_PCI_HOST_BRIDGE(obj) \
OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_PCI_HOST_BRIDGE)
@@ -91,7 +90,6 @@ typedef struct PIIX3State {
MemoryRegion rcr_mem;
} PIIX3State;
-#define TYPE_I440FX_PCI_DEVICE "i440FX"
#define I440FX_PCI_DEVICE(obj) \
OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
@@ -305,7 +303,8 @@ static int i440fx_initfn(PCIDevice *dev)
return 0;
}
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
+ PCII440FXState **pi440fx_state,
int *piix3_devfn,
ISABus **isa_bus, qemu_irq *pic,
MemoryRegion *address_space_mem,
@@ -325,7 +324,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
unsigned i;
I440FXState *i440fx;
- dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
+ dev = qdev_create(NULL, host_type);
s = PCI_HOST_BRIDGE(dev);
b = pci_bus_new(dev, NULL, pci_address_space,
address_space_io, 0, TYPE_PCI_BUS);
@@ -333,7 +332,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev),
NULL);
qdev_init_nofail(dev);
- d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
+ d = pci_create_simple(b, 0, pci_type);
*pi440fx_state = I440FX_PCI_DEVICE(d);
f = *pi440fx_state;
f->system_memory = address_space_mem;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
