|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [v5][PATCH 5/5] xen, gfx passthrough: add opregion mapping
On Wed, Jun 25, 2014 at 10:17:21AM +0800, Tiejun Chen wrote:
> The OpRegion shouldn't be mapped 1:1 because the address in the host
> can't be used in the guest directly.
>
> This patch traps read and write access to the opregion of the Intel
> GPU config space (offset 0xfc).
>
> The original patch is from Jean Guyader <jean.guyader@xxxxxxxxxxxxx>
>
> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
> Cc: Jean Guyader <jean.guyader@xxxxxxxxxxxxx>
> ---
> v5:
>
> * Nothing is changed.
>
> v4:
>
> * Nothing is changed.
>
> v3:
>
> * Fix some typos.
> * Add more comments to make that readable.
> * To unmap igd_opregion when call xen_pt_unregister_vga_regions().
> * Improve some return paths.
> * We need to map 3 pages for opregion as hvmloader set.
> * Force to convert igd_guest/host_opoegion as an unsigned long type
> while calling xc_domain_memory_mapping().
>
> v2:
>
> * We should return zero as an invalid address value while calling
> igd_read_opregion().
>
> hw/xen/xen_pt.h | 4 ++-
> hw/xen/xen_pt_config_init.c | 50 ++++++++++++++++++++++++++++++++++-
> hw/xen/xen_pt_graphics.c | 64
> +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 116 insertions(+), 2 deletions(-)
>
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index 507165c..25147cf 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -63,7 +63,7 @@ typedef int (*xen_pt_conf_byte_read)
> #define XEN_PT_BAR_UNMAPPED (-1)
>
> #define PCI_CAP_MAX 48
> -
> +#define PCI_INTEL_OPREGION 0xfc
>
XEN_.... please
PCI_CAP_MAX should be fixed too.
> typedef enum {
> XEN_PT_GRP_TYPE_HARDWIRED = 0, /* 0 Hardwired reg group */
> @@ -306,5 +306,7 @@ int pci_create_pch(PCIBus *bus);
> void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
> uint32_t val, int len);
> uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
> +uint32_t igd_read_opregion(XenPCIPassthroughState *s);
> +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
>
> #endif /* !XEN_PT_H */
> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
> index de9a20f..6eaaa9a 100644
> --- a/hw/xen/xen_pt_config_init.c
> +++ b/hw/xen/xen_pt_config_init.c
> @@ -575,6 +575,22 @@ static int
> xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s,
> return 0;
> }
>
> +static int xen_pt_intel_opregion_read(XenPCIPassthroughState *s,
> + XenPTReg *cfg_entry,
> + uint32_t *value, uint32_t valid_mask)
> +{
> + *value = igd_read_opregion(s);
> + return 0;
> +}
> +
> +static int xen_pt_intel_opregion_write(XenPCIPassthroughState *s,
> + XenPTReg *cfg_entry, uint32_t *value,
> + uint32_t dev_value, uint32_t
> valid_mask)
> +{
> + igd_write_opregion(s, *value);
> + return 0;
> +}
> +
> /* Header Type0 reg static information table */
> static XenPTRegInfo xen_pt_emu_reg_header0[] = {
> /* Vendor ID reg */
> @@ -1440,6 +1456,20 @@ static XenPTRegInfo xen_pt_emu_reg_msix[] = {
> },
> };
>
> +static XenPTRegInfo xen_pt_emu_reg_igd_opregion[] = {
> + /* Intel IGFX OpRegion reg */
> + {
> + .offset = 0x0,
> + .size = 4,
> + .init_val = 0,
> + .no_wb = 1,
> + .u.dw.read = xen_pt_intel_opregion_read,
> + .u.dw.write = xen_pt_intel_opregion_write,
> + },
> + {
> + .size = 0,
> + },
> +};
>
> /****************************
> * Capabilities
> @@ -1677,6 +1707,14 @@ static const XenPTRegGroupInfo xen_pt_emu_reg_grps[] =
> {
> .size_init = xen_pt_msix_size_init,
> .emu_regs = xen_pt_emu_reg_msix,
> },
> + /* Intel IGD Opregion group */
> + {
> + .grp_id = PCI_INTEL_OPREGION,
> + .grp_type = XEN_PT_GRP_TYPE_EMU,
> + .grp_size = 0x4,
> + .size_init = xen_pt_reg_grp_size_init,
> + .emu_regs = xen_pt_emu_reg_igd_opregion,
> + },
> {
> .grp_size = 0,
> },
> @@ -1806,7 +1844,8 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
> uint32_t reg_grp_offset = 0;
> XenPTRegGroup *reg_grp_entry = NULL;
>
> - if (xen_pt_emu_reg_grps[i].grp_id != 0xFF) {
> + if (xen_pt_emu_reg_grps[i].grp_id != 0xFF
> + && xen_pt_emu_reg_grps[i].grp_id != PCI_INTEL_OPREGION) {
> if (xen_pt_hide_dev_cap(&s->real_device,
> xen_pt_emu_reg_grps[i].grp_id)) {
> continue;
> @@ -1819,6 +1858,15 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
> }
> }
>
> + /*
> + * By default we will trap up to 0x40 in the cfg space.
> + * If an intel device is pass through we need to trap 0xfc,
> + * therefore the size should be 0xff.
> + */
> + if (xen_pt_emu_reg_grps[i].grp_id == PCI_INTEL_OPREGION) {
> + reg_grp_offset = PCI_INTEL_OPREGION;
> + }
> +
> reg_grp_entry = g_new0(XenPTRegGroup, 1);
> QLIST_INIT(®_grp_entry->reg_tbl_list);
> QLIST_INSERT_HEAD(&s->reg_grps, reg_grp_entry, entries);
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> index f3fbfed..fa341ad 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -6,6 +6,9 @@
> #include "hw/xen/xen_backend.h"
> #include "hw/pci/pci_bus.h"
>
> +static unsigned long igd_guest_opregion;
> +static unsigned long igd_host_opregion;
> +
> static int is_vga_passthrough(XenHostPCIDevice *dev)
> {
> return (xen_has_gfx_passthru
> @@ -88,6 +91,7 @@ int xen_pt_register_vga_regions(XenHostPCIDevice *dev)
> int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
> {
> int i = 0;
> + int ret = 0;
>
> if (!is_vga_passthrough(dev)) {
> return 0;
> @@ -114,6 +118,17 @@ int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
> }
> }
>
> + if (igd_guest_opregion) {
> + ret = xc_domain_memory_mapping(xen_xc, xen_domid,
> + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
> + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
don't spread casts all around.
Should be a last resort.
> + 3,
> + DPCI_REMOVE_MAPPING);
> + if (ret) {
> + return ret;
> + }
> + }
> +
> return 0;
> }
>
> @@ -447,3 +462,52 @@ err_out:
> XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
> return -1;
> }
> +
> +uint32_t igd_read_opregion(XenPCIPassthroughState *s)
> +{
> + uint32_t val = 0;
> +
> + if (igd_guest_opregion == 0) {
!igd_guest_opregion is shorter and does the same,
> + return val;
> + }
> +
> + val = igd_guest_opregion;
> +
> + XEN_PT_LOG(&s->dev, "Read opregion val=%x\n", val);
> + return val;
> +}
> +
> +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val)
> +{
> + int ret;
> +
> + if (igd_guest_opregion) {
> + XEN_PT_LOG(&s->dev, "opregion register already been set, ignoring
> %x\n",
> + val);
> + return;
> + }
> +
> + xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION,
> + (uint8_t *)&igd_host_opregion, 4);
> + igd_guest_opregion = (unsigned long)(val & ~0xfff)
> + | (igd_host_opregion & 0xfff);
> +
Clearly broken on BE.
Maybe not important here but writing clean code is
just as easy.
uint8_t igd_host_opregion[4];
...
xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION,
igd_host_opregion, sizeof igd_host_opregion);
igd_guest_opregion = (val & ~0xfff) |
(pci_get_word(igd_host_opregion) & 0xfff);
0xfff should be a macro too to avoid duplication.
> + ret = xc_domain_memory_mapping(xen_xc, xen_domid,
> + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
> + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
> + 3,
> + DPCI_ADD_MAPPING);
> +
> + if (ret) {
> + XEN_PT_ERR(&s->dev, "[%d]:Can't map IGD host opregion:0x%lx to"
> + " guest opregion:0x%lx.\n", ret,
> + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
> + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT));
> + igd_guest_opregion = 0;
> + return;
> + }
> +
> + XEN_PT_LOG(&s->dev, "Map OpRegion: 0x%lx -> 0x%lx\n",
> + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
> + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT));
> +}
> --
> 1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |