Attached.
On Tue, Jun 15, 2010 at 05:58:57PM -0700, Kay, Allen M wrote:
> Isaku,
>
> I cannot apply the patch below because format problems. Can you resend it as
> a attachment?
>
> Allen
>
> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
> [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Isaku Yamahata
> Sent: Sunday, June 13, 2010 8:30 PM
> To: Stefano Stabellini
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx; Kay, Allen M; Han, Weidong; Jean Guyader;
> Ian Pratt; Ross Philipson
> Subject: Re: [Xen-devel] [PATCH][VTD] enable integrated graphics passthrough
> for Calpella and Sandybridge
>
> 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
>
--
yamahata
0002-pci-passthrough-some-clean-up-and-unexport-PCIBus-an.patch
Description: Text document
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home