On Fri, Jun 11, 2010 at 12:08:29PM +0100, Stefano Stabellini wrote:
> On Tue, 8 Jun 2010, Kay, Allen M wrote:
> > Isaku,
> >
> > Thanks for the feedback.
> >
> > > pci_{read, write}_block() would be better than
> > > switch(len) case 1: case 2: case4:...
> >
> > Done!
> >
> > > Is it really necessary to move PCIBus and PCIBridge to the header file?
> > > Doesn't pci_bridge_init() work?
> >
> > I changed to code to utilize pci_bridge_init(). However, I still need to
> > move
> > PCIBus and PCIBridge defines to the header file. The alternative is to
> > pollute
> > pc.c with graphics passthrough specific code.
> >
> > > Overriding pci config read/write methods of i440fx would be much cleaner
> > > than hooking pci_data_read/write. (pass igd_pci_read/write to
> > > pci_register_device() in i440fx_init() in hw/piix_pci.c instead of NULL)
> >
> > Doing this resulted in a lot of duplicated code and also force code path to
> > change
> > even when IGD passthrough is not used. To do it correctly, I also need to
> > put in IGD or PASSTHROGH awareness in piix_pci.c. For now, I'm going to
> > keep
> > the original patch.
> >
>
> I see.
>
> Even though the patch is an improvement over what we currently have in
> qemu-xen, it is still not acceptable in upstream qemu as it is.
> Considering that we are pretty close to have the merge complete, I think
> it worth trying to come up with something cleaner.
>
> Isaku, you are the latest one to touch the pci_host code in qemu, do you
> have any other suggestion?
How about the following patch which is on top of the v2 patch?
I did only compile test, but I think it's enough to show my idea.
For PCI bridge, I have a patches to clean up/enhance the implementation and
I'm planning to push to the qemu upstream soon.
>From 9a22525e523e834ab3822d14e715388c19a7d02e Mon Sep 17 00:00:00 2001
Message-Id:
<9a22525e523e834ab3822d14e715388c19a7d02e.1276485897.git.yamahata@xxxxxxxxxxxxx>
In-Reply-To: <cover.1276485897.git.yamahata@xxxxxxxxxxxxx>
References: <cover.1276485897.git.yamahata@xxxxxxxxxxxxx>
From: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
Date: Mon, 14 Jun 2010 12:24:31 +0900
Subject: [PATCH] pci passthrough: some clean up and unexport PCIBus and
PCIBridge.
some clean up and unexport PCIBus and PCIBridge.
Signed-off-by: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
---
hw/pass-through.h | 10 ++++++++--
hw/pci.c | 29 +++++++++++++++++++++++------
hw/pci.h | 23 -----------------------
hw/piix_pci.c | 4 ++--
hw/pt-graphics.c | 32 +++++++++++++++++++-------------
5 files changed, 52 insertions(+), 46 deletions(-)
diff --git a/hw/pass-through.h b/hw/pass-through.h
index 242d079..adc9f58 100644
--- a/hw/pass-through.h
+++ b/hw/pass-through.h
@@ -409,13 +409,19 @@ uint8_t pci_intx(struct pt_dev *ptdev);
u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len);
int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int len);
void intel_pch_init(PCIBus *bus);
-int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
-int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len);
int register_vga_regions(struct pt_dev *real_device);
int unregister_vga_regions(struct pt_dev *real_device);
int setup_vga_pt(struct pt_dev *real_device);
PCIBus *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
uint16_t did, const char *name, uint16_t revision);
+#ifdef CONFIG_PASSTHROUGH
+void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
+uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, int len);
+#else
+#define igd_pci_write(pci_dev, config_addr, val, len) NULL
+#define igd_pci_read(pci_dev, config_addr, val, len) NULL
+#endif
+
#endif /* __PASSTHROUGH_H__ */
diff --git a/hw/pci.c b/hw/pci.c
index a5cb378..b6400f3 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -39,6 +39,24 @@ extern int igd_passthru;
//#define DEBUG_PCI
+struct PCIBus {
+ int bus_num;
+ int devfn_min;
+ pci_set_irq_fn set_irq;
+ pci_map_irq_fn map_irq;
+ uint32_t config_reg; /* XXX: suppress */
+ /* low level pic */
+ SetIRQFunc *low_set_irq;
+ qemu_irq *irq_opaque;
+ PCIDevice *devices[256];
+ PCIDevice *parent_dev;
+ PCIBus *next;
+ /* The bus IRQ state is the logical OR of the connected devices.
+ Keep a count of the number of devices with raised IRQs. */
+ int nirq;
+ int irq_count[];
+};
+
static void pci_update_mappings(PCIDevice *d);
static void pci_set_irq(void *opaque, int irq_num, int level);
@@ -573,9 +591,6 @@ void pci_data_write(void *opaque, uint32_t addr, uint32_t
val, int len)
pci_dev->name, config_addr, val, len);
#endif
-#ifdef CONFIG_PASSTHROUGH
- if (igd_pci_write(pci_dev, config_addr, val, len) == 0)
-#endif
pci_dev->config_write(pci_dev, config_addr, val, len);
}
@@ -610,9 +625,6 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
}
config_addr = addr & 0xff;
-#ifdef CONFIG_PASSTHROUGH
- if ( igd_pci_read(pci_dev, config_addr, &val, len) == 0)
-#endif
val = pci_dev->config_read(pci_dev, config_addr, len);
done_config_read:
@@ -860,6 +872,11 @@ void pci_unplug_netifs(void)
}
}
+typedef struct {
+ PCIDevice dev;
+ PCIBus *bus;
+} PCIBridge;
+
void pci_bridge_write_config(PCIDevice *d,
uint32_t address, uint32_t val, int len)
{
diff --git a/hw/pci.h b/hw/pci.h
index 7a44035..8abbad7 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -210,29 +210,6 @@ struct PCIDevice {
typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
-struct PCIBus {
- int bus_num;
- int devfn_min;
- pci_set_irq_fn set_irq;
- pci_map_irq_fn map_irq;
- uint32_t config_reg; /* XXX: suppress */
- /* low level pic */
- SetIRQFunc *low_set_irq;
- qemu_irq *irq_opaque;
- PCIDevice *devices[256];
- PCIDevice *parent_dev;
- PCIBus *next;
- /* The bus IRQ state is the logical OR of the connected devices.
- Keep a count of the number of devices with raised IRQs. */
- int nirq;
- int irq_count[];
-};
-
-typedef struct {
- PCIDevice dev;
- PCIBus *bus;
-} PCIBridge;
-
extern char direct_pci_str[];
extern int direct_pci_msitranslate;
extern int direct_pci_power_mgmt;
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 49d72b2..1bfe0fb 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -25,7 +25,7 @@
#include "hw.h"
#include "pc.h"
#include "pci.h"
-
+#include "pass-through.h"
static void i440fx_set_irq(qemu_irq *pic, int irq_num, int level);
static void piix3_write_config(PCIDevice *d,
@@ -207,7 +207,7 @@ PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq
*pic)
register_ioport_read(0xcfc, 4, 4, pci_host_data_readl, s);
d = pci_register_device(b, "i440FX", sizeof(PCIDevice), 0,
- NULL, NULL);
+ igd_pci_read, igd_pci_write);
pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_INTEL);
pci_config_set_device_id(d->config, PCI_DEVICE_ID_INTEL_82441);
diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c
index 4923715..32c174d 100644
--- a/hw/pt-graphics.c
+++ b/hw/pt-graphics.c
@@ -8,6 +8,7 @@
#include <unistd.h>
#include <sys/ioctl.h>
+#include <assert.h>
extern int gfx_passthru;
extern int igd_passthru;
@@ -34,10 +35,13 @@ void intel_pch_init(PCIBus *bus)
pch_map_irq, "intel_bridge_1f");
}
-int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
+void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
{
- if ( !igd_passthru || (pci_dev->devfn != 0x00 ) )
- return 0;
+ assert(pci_dev->devfn == 0x00);
+ if ( !igd_passthru ) {
+ pci_default_write_config(pci_dev, config_addr, val, len);
+ return;
+ }
switch (config_addr)
{
@@ -48,15 +52,18 @@ int igd_pci_write(PCIDevice *pci_dev, int config_addr,
uint32_t val, int len)
PCI_FUNC(pci_dev->devfn), config_addr, len, val);
break;
default:
- pci_dev->config_write(pci_dev, config_addr, val, len);
+ pci_default_write_config(pci_dev, config_addr, val, len);
}
- return 1;
}
-int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
+uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, int len)
{
- if ( !igd_passthru || (pci_dev->devfn != 0) )
- return 0;
+ uint32_t val;
+
+ assert(pci_dev->devfn == 0x00);
+ if ( !igd_passthru ) {
+ return pci_default_read_config(pci_dev, config_addr, len);
+ }
switch (config_addr)
{
@@ -68,17 +75,16 @@ int igd_pci_read(PCIDevice *pci_dev, int config_addr,
uint32_t *val, int len)
case 0x58: /* SNB: PAVPC Offset */
case 0xa4: /* SNB: graphics base of stolen memory */
case 0xa8: /* SNB: base of GTT stolen memory */
- *val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
+ val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
0, config_addr, len);
PT_LOG("pci_config_read: %x:%x.%x: addr=%x len=%x val=%x\n",
pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
- PCI_FUNC(pci_dev->devfn), config_addr, len, *val);
-
+ PCI_FUNC(pci_dev->devfn), config_addr, len, val);
break;
default:
- *val = pci_dev->config_read(pci_dev, config_addr, len);
+ val = pci_default_read_config(pci_dev, config_addr, len);
}
- return 1;
+ return val;
}
/*
--
1.6.6.1
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home