[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v2][PATCH 8/8] xen, gfx passthrough: add opregion mapp
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [v2][PATCH 8/8] xen, gfx passthrough: add opregion mapping |
Date: |
Mon, 19 May 2014 12:53:39 +0100 |
User-agent: |
Alpine 2.02 (DEB 1266 2009-07-14) |
On Fri, 16 May 2014, 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 <address@hidden>
>
> Signed-off-by: Yang Zhang <address@hidden>
> Signed-off-by: Tiejun Chen <address@hidden>
> Cc: Jean Guyader <address@hidden>
> ---
> 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 | 45 ++++++++++++++++++++++++++++++++++++++++++-
> hw/xen/xen_pt_graphics.c | 47
> +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 94 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
>
> 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..cf36a40 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,
> },
If I am not mistaken, in the original patch to qemu-xen-traditional, we
were not adding an Intel IGD Opregion group. Instead we were changing
the size of the header Type0 reg group:
+static uint8_t pt_reg_grp_header0_size_init(struct pt_dev *ptdev,
+ struct pt_reg_grp_info_tbl *grp_reg, uint32_t base_offset)
+{
+ /*
+ ** 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 (igd_passthru)
+ return 0xFF;
+ return grp_reg->grp_size;
+}
Here instead we are adding the new group and forcing the offset to be
PCI_INTEL_OPREGION, below. It looks functionally equivalent, but nicer.
But wouldn't it be even better to have find_cap_offset return the right
offset for Intel IGD Opregion group? Why do we need to manually set
reg_grp_offset to PCI_INTEL_OPREGION?
> @@ -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,10 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
> }
> }
>
> + 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 066bc4d..b25ecae 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -6,6 +6,8 @@
> #include "hw/xen/xen_backend.h"
> #include "hw/pci/pci_bus.h"
>
> +static int igd_guest_opregion;
> +
> static int is_vga_passthrough(XenHostPCIDevice *dev)
> {
> return (xen_has_gfx_passthru
> @@ -386,3 +388,48 @@ 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) {
> + 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)
> +{
> + uint32_t host_opregion = 0;
> + 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 *)&host_opregion, 4);
> + igd_guest_opregion = (val & ~0xfff) | (host_opregion & 0xfff);
> +
> + ret = xc_domain_memory_mapping(xen_xc, xen_domid,
> + igd_guest_opregion >> XC_PAGE_SHIFT,
> + host_opregion >> XC_PAGE_SHIFT,
> + 2,
> + DPCI_ADD_MAPPING);
> +
> + if (ret != 0) {
> + XEN_PT_ERR(&s->dev, "Error: Can't map opregion\n");
> + igd_guest_opregion = 0;
> + return;
> + }
> +
> + XEN_PT_LOG(&s->dev, "Map OpRegion: %x -> %x\n", host_opregion,
> + igd_guest_opregion);
> +}
> --
> 1.9.1
>
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 5/8] xen, gfx passthrough: create intel isa bridge, (continued)
- [Qemu-devel] [v2][PATCH 6/8] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Tiejun Chen, 2014/05/16
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 6/8] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Konrad Rzeszutek Wilk, 2014/05/16
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 6/8] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Zhang, Yang Z, 2014/05/18
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 6/8] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Konrad Rzeszutek Wilk, 2014/05/19
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 6/8] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Chen, Tiejun, 2014/05/20
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 6/8] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Konrad Rzeszutek Wilk, 2014/05/20
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 6/8] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Chen, Tiejun, 2014/05/19
[Qemu-devel] [v2][PATCH 8/8] xen, gfx passthrough: add opregion mapping, Tiejun Chen, 2014/05/16
- Re: [Qemu-devel] [v2][PATCH 8/8] xen, gfx passthrough: add opregion mapping,
Stefano Stabellini <=
- Re: [Qemu-devel] [v2][PATCH 8/8] xen, gfx passthrough: add opregion mapping, Chen, Tiejun, 2014/05/20
- Re: [Qemu-devel] [v2][PATCH 8/8] xen, gfx passthrough: add opregion mapping, Stefano Stabellini, 2014/05/20
- Re: [Qemu-devel] [v2][PATCH 8/8] xen, gfx passthrough: add opregion mapping, Chen, Tiejun, 2014/05/20
[Qemu-devel] [v2][PATCH 7/8] xen, gfx passthrough: create host bridge to passthrough, Tiejun Chen, 2014/05/16