[PATCH][intel_agp] fix name for G35 chipset

[Zhenyu Wang]

Change origin chipset name i965G_1 to market name G35.

Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
---
 drivers/char/agp/intel-agp.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index d879619..7f8b06c 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -12,8 +12,8 @@
 
 #define PCI_DEVICE_ID_INTEL_82946GZ_HB  0x2970
 #define PCI_DEVICE_ID_INTEL_82946GZ_IG  0x2972
-#define PCI_DEVICE_ID_INTEL_82965G_1_HB 0x2980
-#define PCI_DEVICE_ID_INTEL_82965G_1_IG 0x2982
+#define PCI_DEVICE_ID_INTEL_82G35_HB0x2980
+#define PCI_DEVICE_ID_INTEL_82G35_IG0x2982
 #define PCI_DEVICE_ID_INTEL_82965Q_HB   0x2990
 #define PCI_DEVICE_ID_INTEL_82965Q_IG   0x2992
 #define PCI_DEVICE_ID_INTEL_82965G_HB   0x29A0
@@ -32,7 +32,7 @@
 #define PCI_DEVICE_ID_INTEL_Q33_IG  0x29D2
 
 #define IS_I965 (agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82946GZ_HB || \
- agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82965G_1_HB || 
\
+ agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82G35_HB || \
  agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82965Q_HB || \
  agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82965G_HB || \
  agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82965GM_HB || \
@@ -1866,7 +1866,7 @@ static const struct intel_driver_description {
NULL, intel_915_driver },
{ PCI_DEVICE_ID_INTEL_82946GZ_HB, PCI_DEVICE_ID_INTEL_82946GZ_IG, 0, 
946GZ,
NULL, intel_i965_driver },
-   { PCI_DEVICE_ID_INTEL_82965G_1_HB, PCI_DEVICE_ID_INTEL_82965G_1_IG, 0, 
965G,
+   { PCI_DEVICE_ID_INTEL_82G35_HB, PCI_DEVICE_ID_INTEL_82G35_IG, 0, G35,
NULL, intel_i965_driver },
{ PCI_DEVICE_ID_INTEL_82965Q_HB, PCI_DEVICE_ID_INTEL_82965Q_IG, 0, 
965Q,
NULL, intel_i965_driver },
@@ -2065,7 +2065,7 @@ static struct pci_device_id agp_intel_pci_table[] = {
ID(PCI_DEVICE_ID_INTEL_82945GM_HB),
ID(PCI_DEVICE_ID_INTEL_82945GME_HB),
ID(PCI_DEVICE_ID_INTEL_82946GZ_HB),
-   ID(PCI_DEVICE_ID_INTEL_82965G_1_HB),
+   ID(PCI_DEVICE_ID_INTEL_82G35_HB),
ID(PCI_DEVICE_ID_INTEL_82965Q_HB),
ID(PCI_DEVICE_ID_INTEL_82965G_HB),
ID(PCI_DEVICE_ID_INTEL_82965GM_HB),
-- 
1.5.3.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH][AGP] intel_agp: add new chipset ids

[Zhenyu Wang]

Dave,

This one adds new pci ids for Intel intergrated graphics
chipset, with gtt table access change on it and new gtt table
size definition.

Thanks.

Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
---
 drivers/char/agp/agp.h   |3 +++
 drivers/char/agp/intel-agp.c |   31 ++-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/agp.h b/drivers/char/agp/agp.h
index b83824c..d132914 100644
--- a/drivers/char/agp/agp.h
+++ b/drivers/char/agp/agp.h
@@ -235,6 +235,9 @@ struct agp_bridge_data {
 #define I965_PGETBL_SIZE_512KB (0  1)
 #define I965_PGETBL_SIZE_256KB (1  1)
 #define I965_PGETBL_SIZE_128KB (2  1)
+#define I965_PGETBL_SIZE_1MB   (3  1)
+#define I965_PGETBL_SIZE_2MB   (4  1)
+#define I965_PGETBL_SIZE_1_5MB (5  1)
 #define G33_PGETBL_SIZE_MASK(3  8)
 #define G33_PGETBL_SIZE_1M  (1  8)
 #define G33_PGETBL_SIZE_2M  (2  8)
diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index d879619..091e765 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -30,13 +30,16 @@
 #define PCI_DEVICE_ID_INTEL_Q35_IG  0x29B2
 #define PCI_DEVICE_ID_INTEL_Q33_HB  0x29D0
 #define PCI_DEVICE_ID_INTEL_Q33_IG  0x29D2
+#define PCI_DEVICE_ID_INTEL_IGD_HB  0x2A40
+#define PCI_DEVICE_ID_INTEL_IGD_IG  0x2A42
 
 #define IS_I965 (agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82946GZ_HB || \
  agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82965G_1_HB || 
\
  agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82965Q_HB || \
  agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82965G_HB || \
  agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82965GM_HB || \
- agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82965GME_HB)
+ agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_82965GME_HB || 
\
+ agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_IGD_HB)
 
 #define IS_G33 (agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_G33_HB || \
agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_Q35_HB || \
@@ -456,6 +459,15 @@ static void intel_i830_init_gtt_entries(void)
case I965_PGETBL_SIZE_512KB:
size = 512;
break;
+   case I965_PGETBL_SIZE_1MB:
+   size = 1024;
+   break;
+   case I965_PGETBL_SIZE_2MB:
+   size = 2048;
+   break;
+   case I965_PGETBL_SIZE_1_5MB:
+   size = 1024 + 512;
+   break;
default:
printk(KERN_INFO PFX Unknown page table size, 
   assuming 512KB\n);
@@ -981,6 +993,7 @@ static int intel_i965_create_gatt_table(struct 
agp_bridge_data *bridge)
struct aper_size_info_fixed *size;
int num_entries;
u32 temp;
+   int gtt_offset, gtt_size;
 
size = agp_bridge-current_size;
page_order = size-page_order;
@@ -990,13 +1003,18 @@ static int intel_i965_create_gatt_table(struct 
agp_bridge_data *bridge)
pci_read_config_dword(intel_private.pcidev, I915_MMADDR, temp);
 
temp = 0xfff0;
-   intel_private.gtt = ioremap((temp + (512 * 1024)) , 512 * 1024);
 
-   if (!intel_private.gtt)
-   return -ENOMEM;
+   if (agp_bridge-dev-device == PCI_DEVICE_ID_INTEL_IGD_HB)
+  gtt_offset = gtt_size = MB(2);
+   else
+  gtt_offset = gtt_size = KB(512);
+
+   intel_private.gtt = ioremap((temp + gtt_offset) , gtt_size);
 
+   if (!intel_private.gtt)
+  return -ENOMEM;
 
-   intel_private.registers = ioremap(temp,128 * 4096);
+   intel_private.registers = ioremap(temp, 128 * 4096);
if (!intel_private.registers) {
iounmap(intel_private.gtt);
return -ENOMEM;
@@ -1884,6 +1902,8 @@ static const struct intel_driver_description {
NULL, intel_g33_driver },
{ PCI_DEVICE_ID_INTEL_Q33_HB, PCI_DEVICE_ID_INTEL_Q33_IG, 0, Q33,
NULL, intel_g33_driver },
+   { PCI_DEVICE_ID_INTEL_IGD_HB, PCI_DEVICE_ID_INTEL_IGD_IG, 0,
+   Intel Integrated Graphics Device, NULL, intel_i965_driver },
{ 0, 0, 0, NULL, NULL, NULL }
 };
 
@@ -2073,6 +2093,7 @@ static struct pci_device_id agp_intel_pci_table[] = {
ID(PCI_DEVICE_ID_INTEL_G33_HB),
ID(PCI_DEVICE_ID_INTEL_Q35_HB),
ID(PCI_DEVICE_ID_INTEL_Q33_HB),
+   ID(PCI_DEVICE_ID_INTEL_IGD_HB),
{ }
 };
 
-- 
1.5.3.7

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: X-freeze after clflush changes [Was: 2.6.23-rc6-mm1]

[Zhenyu Wang]

On 2007.09.20 17:33:45 +, Dave Airlie wrote:
  Maybe you are rather interested in these dmesg lines:
  Linux agpgart interface v0.102
  agpgart: suspend/resume problematic: resume with 3D/DRI active may lockup 
  X.Org
  on some chipset/BIOS combos (see DEBUG_AGP_PM in intel-agp.c)
  agpgart: Detected an Intel G33 Chipset.
  agpgart: Detected 8192K stolen memory.
  agpgart: AGP aperture is 256M @ 0xd000
  [drm] Initialized drm 1.1.0 20060810
  ACPI: PCI Interrupt :00:02.0[A] - GSI 16 (level, low) - IRQ 16
  [drm] Initialized i915 1.6.0 20060119 on minor 0
  ...
  set status page addr 0x00033000
  agpgart: pg_start == 0x05ff,intel_private.gtt_entries == 0x0800
  agpgart: Trying to insert into local/stolen memory
 
  So the problem is, that X passes too low start.
 
  The X log:
  http://www.fi.muni.cz/~xslaby/sklad/Xorg.0.log.old

Could you try current xf86-video-intel driver? just do
git clone git://anongit.freedesktop.org/git/xorg/driver/xf86-video-intel

My G33 was just verified broken today... so I'll try to reproduce it on
another one tomorrow.

 
 I've cc'd Zhenyu who might be able to shed some light on this? can you
 try 2.6.23-rc7 as maybe the G33 support still needs some work.. or
 maybe I'm missing a patch in the drm..
 

yep, should try 2.6.23-rc7 first, and it seems not drm relate.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC][PATCH 0/4] enabling graphics memory dma remapping

[Zhenyu Wang]

Intel IOMMU (a.k.a VT-d) is under rapid deployment on desktop
and mobile platforms. As platform provides multiple dma remap
engines for devices like those lives on south bridge (net, sound,
etc.), and we also have one engine specific to graphics device.

If this engine is functioning, the access to graphics memory
routine is to first look up in GART table, which return virtual
dma address that will further be route to graphics DMAR engine,
which then looking up DMAR table to get real physical address.

Current intel iommu function in kernel is providing a graphics
workaround kernel config (CONFIG_DMAR_GFX_WA) to make a 1:1 mapping
initialization for graphics device, so what we fill in GART table
got from agpgart page alloc is identical to virtual dma address in
DMAR table. No change needs to be made for graphics driver.

Following patches try to add dma remapping function to agpgart
module, so we won't depend on intel iommu graphics workaround. 
As DMAR is only available on x86_64 now, so these patches can only
work for x86_64 system.

Here're three patches below:

- intel_iommu-explicit-export-current-graphics-dmar-status.patch
  
  This exports current status of graphics dma remap engine, which
  depends on current platform iommu support, kernel config or runtime
  config. This info will be used in agp module for detect whether
  we should do remapping or not.

- AGP-Add-generic-support-for-graphics-dma-remapping.patch

  This adds new hooks into agp bridge driver to do map/unmap when
  bind/unbind memory into GART table. The aim is to provide generic
  interfaces to let driver implement hardware specific operations.

- AGP-intel_agp-add-support-for-graphics-dma-remapping.patch

  This implements graphics dma remapping support in intel_agp module,
  current only desktop chipset G33 series can use it.

Thanks.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

[PATCH] [intel_iommu] explicit export current graphics dmar status

To make it possbile to tell other modules about curent
graphics dmar engine status, that could decide if graphics
driver should remap physical address to dma address.

Also this one trys to make dmar_disabled really present
current status of DMAR, which would be used for completely
express graphics dmar engine is active or not.

Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
---
 drivers/pci/intel-iommu.c |   18 --
 include/linux/dmar.h  |6 ++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index e079a52..81a0abc 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -53,7 +53,7 @@
 static void domain_remove_dev_info(struct dmar_domain *domain);
 
 static int dmar_disabled;
-static int __initdata dmar_map_gfx = 1;
+static int dmar_map_gfx = 1;
 static int dmar_forcedac;
 
 #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
@@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
 }
 __setup(intel_iommu=, intel_iommu_setup);
 
+int intel_iommu_gfx_remapping(void)
+{
+#ifndef CONFIG_DMAR_GFX_WA
+   if (!dmar_disabled  iommu_detected  dmar_map_gfx)
+   return 1;
+#endif
+   return 0;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
+
 static struct kmem_cache *iommu_domain_cache;
 static struct kmem_cache *iommu_devinfo_cache;
 static struct kmem_cache *iommu_iova_cache;
@@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
 
 void __init detect_intel_iommu(void)
 {
-   if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
+   if (dmar_disabled)
+   return;
+   if (swiotlb || no_iommu || iommu_detected) {
+   dmar_disabled = 1;
return;
+   }
if (early_dmar_detect()) {
iommu_detected = 1;
}
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index ffb6439..8ae435e 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
 extern int dmar_table_init(void);
 extern int early_dmar_detect(void);
 
+extern int intel_iommu_gfx_remapping(void);
+
 extern struct list_head dmar_drhd_units;
 extern struct list_head dmar_rmrr_units;
 
@@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
 {
return -ENODEV;
 }
+static inline int intel_iommu_gfx_remapping(void)
+{
+   return 0;
+}
 
 #endif /* !CONFIG_DMAR */
 #endif /* __DMAR_H__ */
-- 
1.5.3.5

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC][PATCH 2/4][AGP] Add generic support for graphics dma remapping

[Zhenyu Wang]

[PATCH] [AGP] Add generic support for graphics dma remapping

New driver hooks for support graphics memory dma remapping
are introduced in this patch. It makes generic code can
tell if current device needs dma remapping, then call driver
provided interfaces for mapping and unmapping. Change has
also been made to handle scratch_page in remapping case.

Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
---
 drivers/char/agp/agp.h  |   14 ++
 drivers/char/agp/backend.c  |   21 -
 drivers/char/agp/generic.c  |   14 ++
 include/linux/agp_backend.h |7 +++
 4 files changed, 55 insertions(+), 1 deletions(-)

diff --git a/drivers/char/agp/agp.h b/drivers/char/agp/agp.h
index b83824c..7806da1 100644
--- a/drivers/char/agp/agp.h
+++ b/drivers/char/agp/agp.h
@@ -118,6 +118,18 @@ struct agp_bridge_driver {
void *(*agp_alloc_page)(struct agp_bridge_data *);
void (*agp_destroy_page)(void *, int flags);
 int (*agp_type_to_mask_type) (struct agp_bridge_data *, int);
+
+   /* Tell current graphics dma remapping engine status, there might
+* be driver or platform specific way to detect. */
+   int (*agp_require_remapping)(void);
+
+   /* This can support single page remapping via void *virt for like
+* scratch_page, or normal bunch of mem page range via struct
+* agp_memory *mem.
+*/
+   int (*agp_map_page)(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single);
+
+   void (*agp_unmap_page)(struct agp_memory *mem, dma_addr_t ret, int 
is_single);
 };
 
 struct agp_bridge_data {
@@ -132,6 +144,8 @@ struct agp_bridge_data {
u32 *gatt_table_real;
unsigned long scratch_page;
unsigned long scratch_page_real;
+   dma_addr_t scratch_page_dma;
+   u8 scratch_page_is_mapped;
unsigned long gart_bus_addr;
unsigned long gatt_bus_addr;
u32 mode;
diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
index 832ded2..cc6d648 100644
--- a/drivers/char/agp/backend.c
+++ b/drivers/char/agp/backend.c
@@ -151,7 +151,20 @@ static int agp_backend_initialize(struct agp_bridge_data 
*bridge)
 
bridge-scratch_page_real = virt_to_gart(addr);
bridge-scratch_page =
-   bridge-driver-mask_memory(bridge, 
bridge-scratch_page_real, 0);
+   bridge-driver-mask_memory(bridge, 
bridge-scratch_page_real, 0);
+
+   if (bridge-driver-agp_require_remapping 
+   bridge-driver-agp_require_remapping()) {
+   if (bridge-driver-agp_map_page(NULL, addr, 
bridge-scratch_page_dma, 1)) {
+   printk(KERN_ERR PFX unable to map scratch 
page\n);
+   rc = -ENOMEM;
+   goto err_out;
+   }
+   bridge-scratch_page_is_mapped = TRUE;
+   bridge-scratch_page =
+   bridge-driver-mask_memory(bridge,
+   bridge-scratch_page_dma, 0);
+   }
}
 
size_value = bridge-driver-fetch_size();
@@ -189,6 +202,9 @@ static int agp_backend_initialize(struct agp_bridge_data 
*bridge)
 
 err_out:
if (bridge-driver-needs_scratch_page) {
+   if (bridge-scratch_page_is_mapped)
+   bridge-driver-agp_unmap_page(NULL, 
bridge-scratch_page_dma, 1);
+

bridge-driver-agp_destroy_page(gart_to_virt(bridge-scratch_page_real),
 AGP_PAGE_DESTROY_UNMAP);
flush_agp_mappings();
@@ -217,6 +233,9 @@ static void agp_backend_cleanup(struct agp_bridge_data 
*bridge)
 
if (bridge-driver-agp_destroy_page 
bridge-driver-needs_scratch_page) {
+   if (bridge-scratch_page_is_mapped)
+   bridge-driver-agp_unmap_page(NULL, 
bridge-scratch_page_dma, 1);
+

bridge-driver-agp_destroy_page(gart_to_virt(bridge-scratch_page_real),
 AGP_PAGE_DESTROY_UNMAP);
flush_agp_mappings();
diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c
index 64b2f6d..8966c94 100644
--- a/drivers/char/agp/generic.c
+++ b/drivers/char/agp/generic.c
@@ -415,6 +415,14 @@ int agp_bind_memory(struct agp_memory *curr, off_t 
pg_start)
curr-bridge-driver-cache_flush();
curr-is_flushed = TRUE;
}
+
+   if (curr-bridge-driver-agp_require_remapping 
+   curr-bridge-driver-agp_require_remapping()) {
+   ret_val = curr-bridge-driver-agp_map_page(curr, NULL, NULL, 
0);
+   if (ret_val)
+   return ret_val;
+   }
+
ret_val = curr-bridge-driver-insert_memory(curr, pg_start, 
curr-type);
 
if (ret_val != 0)
@@ -454,6

[RFC][PATCH 3/4][AGP] intel_agp: add support for graphics dma remapping on G33

[Zhenyu Wang]

[PATCH] [AGP] intel_agp: add support for graphics dma remapping on G33

When graphics dma remapping engine is active, we must fill
gart table with dma address from dmar engine, as now graphics
device access to graphics memory must go through dma remapping
table to get real physical address.

Add support on G33 chipset, which has graphics device specific
dmar engine avaiable.

Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
---
 drivers/char/agp/Kconfig |   12 +++-
 drivers/char/agp/intel-agp.c |  134 +-
 2 files changed, 141 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/Kconfig b/drivers/char/agp/Kconfig
index ccb1fa8..e60c2b0 100644
--- a/drivers/char/agp/Kconfig
+++ b/drivers/char/agp/Kconfig
@@ -74,8 +74,16 @@ config AGP_INTEL
  on Intel 440LX/BX/GX, 815, 820, 830, 840, 845, 850, 860, 875,
  E7205 and E7505 chipsets and full support for the 810, 815, 830M,
  845G, 852GM, 855GM, 865G and I915 integrated graphics chipsets.
-
-
+   
+config AGP_INTEL_DMAR
+   bool
+   depends on AGP_INTEL  DMAR  !DMAR_GFX_WA
+   default y
+   help
+ This option will enable graphics address remapping with Intel
+ DMAR engine aka VT-d if you don't select graphics workaround on
+ Intel DMAR. The graphics address filled to gart table will be
+ changed by remapping within graphics DMAR engine for domain.
 
 config AGP_NVIDIA
tristate NVIDIA nForce/nForce2 chipset support
diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index d879619..7424667 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -9,6 +9,9 @@
 #include linux/pagemap.h
 #include linux/agp_backend.h
 #include agp.h
+#ifdef CONFIG_AGP_INTEL_DMAR
+#include linux/dmar.h
+#endif
 
 #define PCI_DEVICE_ID_INTEL_82946GZ_HB  0x2970
 #define PCI_DEVICE_ID_INTEL_82946GZ_IG  0x2972
@@ -115,6 +118,97 @@ static struct _intel_private {
int gtt_entries;/* i830+ */
 } intel_private;
 
+static int intel_agp_require_remapping(void)
+{
+#ifdef CONFIG_AGP_INTEL_DMAR
+   return intel_iommu_gfx_remapping();
+#else
+   return 0;
+#endif
+}
+
+#ifdef CONFIG_AGP_INTEL_DMAR
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single)
+{
+   int i;
+   struct scatterlist *sg;
+
+   if (is_single) {
+   *ret = pci_map_single(intel_private.pcidev, virt, PAGE_SIZE,
+   PCI_DMA_BIDIRECTIONAL);
+   if (pci_dma_mapping_error(*ret))
+   return -EINVAL;
+   return 0;
+   }
+
+   DBG(try mapping %lu pages\n, (unsigned long)mem-page_count);
+
+   if ((mem-page_count * sizeof(*mem-sg_list))  2*PAGE_SIZE)
+   mem-sg_list = kcalloc(mem-page_count, sizeof(*mem-sg_list), 
GFP_KERNEL);
+
+   if (mem-sg_list == NULL) {
+   mem-sg_list = vmalloc(mem-page_count * sizeof(*mem-sg_list));
+   mem-sg_vmalloc_flag = 1;
+   }
+
+   if (!mem-sg_list) {
+   mem-sg_vmalloc_flag = 0;
+   return -ENOMEM;
+   }
+   sg_init_table(mem-sg_list, mem-page_count);
+
+   sg = mem-sg_list;
+   for (i = 0 ; i  mem-page_count; i++, sg = sg_next(sg))
+   sg_set_buf(sg, gart_to_virt(mem-memory[i]), PAGE_SIZE);
+
+   mem-num_sg = pci_map_sg(intel_private.pcidev, mem-sg_list,
+   mem-page_count, PCI_DMA_BIDIRECTIONAL);
+   if (mem-num_sg == 0) {
+   if (mem-sg_vmalloc_flag)
+   vfree(mem-sg_list);
+   else
+   kfree(mem-sg_list);
+   mem-sg_list = NULL;
+   mem-sg_vmalloc_flag = 0;
+   return -ENOMEM;
+   }
+   mem-is_mapped = TRUE;
+   return 0;
+}
+
+static void intel_agp_unmap_pages(struct agp_memory *mem, dma_addr_t addr, int 
is_single)
+{
+   if (is_single) {
+   pci_unmap_single(intel_private.pcidev, addr, PAGE_SIZE,
+   PCI_DMA_BIDIRECTIONAL);
+   return;
+   }
+   DBG(try unmapping %lu pages\n, (unsigned long)mem-page_count);
+
+   if (!mem-is_mapped)
+   return;
+
+   if (mem-num_sg)
+   pci_unmap_sg(intel_private.pcidev, mem-sg_list, 
mem-page_count,
+   PCI_DMA_BIDIRECTIONAL);
+   if (mem-sg_vmalloc_flag)
+   vfree(mem-sg_list);
+   else
+   kfree(mem-sg_list);
+   mem-sg_vmalloc_flag = 0;
+   mem-sg_list = NULL;
+   mem-num_sg = 0;
+}
+#else
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single)
+{
+return 0;
+}
+static void intel_agp_unmap_pages(struct agp_memory *mem, dma_addr_t addr, int 
is_single)
+{
+}
+#endif
+
 static int intel_i810_fetch_size(void)
 {
u32 smram_miscc;
@@ -847,9

[agp-mm][PATCH 0/4] enabling graphics memory dma remapping

[Zhenyu Wang]

On 2007.12.18 13:08:09 +, Zhenyu Wang wrote:
 
 Here're three patches below:
 
 - intel_iommu-explicit-export-current-graphics-dmar-status.patch
   
   This exports current status of graphics dma remap engine, which
   depends on current platform iommu support, kernel config or runtime
   config. This info will be used in agp module for detect whether
   we should do remapping or not.
 
 - AGP-Add-generic-support-for-graphics-dma-remapping.patch
 
   This adds new hooks into agp bridge driver to do map/unmap when
   bind/unbind memory into GART table. The aim is to provide generic
   interfaces to let driver implement hardware specific operations.
 
 - AGP-intel_agp-add-support-for-graphics-dma-remapping.patch
 
   This implements graphics dma remapping support in intel_agp module,
   current only desktop chipset G33 series can use it.

Above patch series are against master branch, following threads try to
rebase them to Airlie's agp-mm tree for -mm inclusion.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

[agp-mm] [intel_iommu] explicit export current graphics dmar status

To make it possbile to tell other modules about curent
graphics dmar engine status, that could decide if graphics
driver should remap physical address to dma address.

Also this one trys to make dmar_disabled really present
current status of DMAR, which would be used for completely
express graphics dmar engine is active or not.

Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
---
 drivers/pci/intel-iommu.c |   18 --
 include/linux/dmar.h  |6 ++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index e079a52..81a0abc 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -53,7 +53,7 @@
 static void domain_remove_dev_info(struct dmar_domain *domain);
 
 static int dmar_disabled;
-static int __initdata dmar_map_gfx = 1;
+static int dmar_map_gfx = 1;
 static int dmar_forcedac;
 
 #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
@@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
 }
 __setup(intel_iommu=, intel_iommu_setup);
 
+int intel_iommu_gfx_remapping(void)
+{
+#ifndef CONFIG_DMAR_GFX_WA
+   if (!dmar_disabled  iommu_detected  dmar_map_gfx)
+   return 1;
+#endif
+   return 0;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
+
 static struct kmem_cache *iommu_domain_cache;
 static struct kmem_cache *iommu_devinfo_cache;
 static struct kmem_cache *iommu_iova_cache;
@@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
 
 void __init detect_intel_iommu(void)
 {
-   if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
+   if (dmar_disabled)
+   return;
+   if (swiotlb || no_iommu || iommu_detected) {
+   dmar_disabled = 1;
return;
+   }
if (early_dmar_detect()) {
iommu_detected = 1;
}
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index ffb6439..8ae435e 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
 extern int dmar_table_init(void);
 extern int early_dmar_detect(void);
 
+extern int intel_iommu_gfx_remapping(void);
+
 extern struct list_head dmar_drhd_units;
 extern struct list_head dmar_rmrr_units;
 
@@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
 {
return -ENODEV;
 }
+static inline int intel_iommu_gfx_remapping(void)
+{
+   return 0;
+}
 
 #endif /* !CONFIG_DMAR */
 #endif /* __DMAR_H__ */
-- 
1.5.3.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[agp-mm][PATCH 2/4][AGP] Add generic support for graphics dma remapping

[Zhenyu Wang]

[agp-mm] [AGP] Add generic support for graphics dma remapping

New driver hooks for support graphics memory dma remapping
are introduced in this patch. It makes generic code can
tell if current device needs dma remapping, then call driver
provided interfaces for mapping and unmapping. Change has
also been made to handle scratch_page in remapping case.

Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
---
 drivers/char/agp/agp.h  |   13 +
 drivers/char/agp/backend.c  |   21 -
 drivers/char/agp/generic.c  |   14 ++
 include/linux/agp_backend.h |7 +++
 4 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/drivers/char/agp/agp.h b/drivers/char/agp/agp.h
index 9ec9374..2c78a28 100644
--- a/drivers/char/agp/agp.h
+++ b/drivers/char/agp/agp.h
@@ -119,6 +119,17 @@ struct agp_bridge_driver {
void (*agp_destroy_page)(void *, int flags);
int (*agp_type_to_mask_type) (struct agp_bridge_data *, int);
void (*chipset_flush)(struct agp_bridge_data *);
+   /* Tell current graphics dma remapping engine status, there might
+* be driver or platform specific way to detect. */
+   int (*agp_require_remapping)(void);
+
+   /* This can support single page remapping via void *virt for like
+* scratch_page, or normal bunch of mem page range via struct
+* agp_memory *mem.
+*/
+   int (*agp_map_page)(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single);
+
+   void (*agp_unmap_page)(struct agp_memory *mem, dma_addr_t ret, int 
is_single);
 };
 
 struct agp_bridge_data {
@@ -133,6 +144,8 @@ struct agp_bridge_data {
u32 *gatt_table_real;
unsigned long scratch_page;
unsigned long scratch_page_real;
+   dma_addr_t scratch_page_dma;
+   u8 scratch_page_is_mapped;
unsigned long gart_bus_addr;
unsigned long gatt_bus_addr;
u32 mode;
diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
index f9c180c..7898051 100644
--- a/drivers/char/agp/backend.c
+++ b/drivers/char/agp/backend.c
@@ -151,7 +151,20 @@ static int agp_backend_initialize(struct agp_bridge_data 
*bridge)
 
bridge-scratch_page_real = virt_to_gart(addr);
bridge-scratch_page =
-   bridge-driver-mask_memory(bridge, 
bridge-scratch_page_real, 0);
+   bridge-driver-mask_memory(bridge, 
bridge-scratch_page_real, 0);
+
+   if (bridge-driver-agp_require_remapping 
+   bridge-driver-agp_require_remapping()) {
+   if (bridge-driver-agp_map_page(NULL, addr, 
bridge-scratch_page_dma, 1)) {
+   printk(KERN_ERR PFX unable to map scratch 
page\n);
+   rc = -ENOMEM;
+   goto err_out;
+   }
+   bridge-scratch_page_is_mapped = TRUE;
+   bridge-scratch_page =
+   bridge-driver-mask_memory(bridge,
+   bridge-scratch_page_dma, 0);
+   }
}
 
size_value = bridge-driver-fetch_size();
@@ -189,6 +202,9 @@ static int agp_backend_initialize(struct agp_bridge_data 
*bridge)
 
 err_out:
if (bridge-driver-needs_scratch_page) {
+   if (bridge-scratch_page_is_mapped)
+   bridge-driver-agp_unmap_page(NULL, 
bridge-scratch_page_dma, 1);
+

bridge-driver-agp_destroy_page(gart_to_virt(bridge-scratch_page_real),
 AGP_PAGE_DESTROY_UNMAP);
flush_agp_mappings();
@@ -217,6 +233,9 @@ static void agp_backend_cleanup(struct agp_bridge_data 
*bridge)
 
if (bridge-driver-agp_destroy_page 
bridge-driver-needs_scratch_page) {
+   if (bridge-scratch_page_is_mapped)
+   bridge-driver-agp_unmap_page(NULL, 
bridge-scratch_page_dma, 1);
+

bridge-driver-agp_destroy_page(gart_to_virt(bridge-scratch_page_real),
 AGP_PAGE_DESTROY_UNMAP);
flush_agp_mappings();
diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c
index 8c67b4f..d2a502c 100644
--- a/drivers/char/agp/generic.c
+++ b/drivers/char/agp/generic.c
@@ -422,6 +422,14 @@ int agp_bind_memory(struct agp_memory *curr, off_t 
pg_start)
curr-bridge-driver-cache_flush();
curr-is_flushed = TRUE;
}
+
+   if (curr-bridge-driver-agp_require_remapping 
+   curr-bridge-driver-agp_require_remapping()) {
+   ret_val = curr-bridge-driver-agp_map_page(curr, NULL, NULL, 
0);
+   if (ret_val)
+   return ret_val;
+   }
+
ret_val = curr-bridge-driver-insert_memory(curr, pg_start, 
curr-type);
 
if (ret_val != 0)
@@ -461,6

[agp-mm][PATCH 3/4][AGP] intel_agp: add support for graphics dma remapping on G33

[Zhenyu Wang]

[agp-mm] [AGP] intel_agp: add support for graphics dma remapping on G33

When graphics dma remapping engine is active, we must fill
gart table with dma address from dmar engine, as now graphics
device access to graphics memory must go through dma remapping
table to get real physical address.

Add support on G33 chipset, which has graphics device specific
dmar engine avaiable.

Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
---
 drivers/char/agp/Kconfig |   12 +++-
 drivers/char/agp/intel-agp.c |  134 +-
 2 files changed, 141 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/Kconfig b/drivers/char/agp/Kconfig
index ccb1fa8..e60c2b0 100644
--- a/drivers/char/agp/Kconfig
+++ b/drivers/char/agp/Kconfig
@@ -74,8 +74,16 @@ config AGP_INTEL
  on Intel 440LX/BX/GX, 815, 820, 830, 840, 845, 850, 860, 875,
  E7205 and E7505 chipsets and full support for the 810, 815, 830M,
  845G, 852GM, 855GM, 865G and I915 integrated graphics chipsets.
-
-
+   
+config AGP_INTEL_DMAR
+   bool
+   depends on AGP_INTEL  DMAR  !DMAR_GFX_WA
+   default y
+   help
+ This option will enable graphics address remapping with Intel
+ DMAR engine aka VT-d if you don't select graphics workaround on
+ Intel DMAR. The graphics address filled to gart table will be
+ changed by remapping within graphics DMAR engine for domain.
 
 config AGP_NVIDIA
tristate NVIDIA nForce/nForce2 chipset support
diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index f443682..31a939d 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -9,6 +9,9 @@
 #include linux/pagemap.h
 #include linux/agp_backend.h
 #include agp.h
+#ifdef CONFIG_AGP_INTEL_DMAR
+#include linux/dmar.h
+#endif
 
 #define PCI_DEVICE_ID_INTEL_82946GZ_HB  0x2970
 #define PCI_DEVICE_ID_INTEL_82946GZ_IG  0x2972
@@ -123,6 +126,97 @@ static struct _intel_private {
struct resource ifp_resource;
 } intel_private;
 
+static int intel_agp_require_remapping(void)
+{
+#ifdef CONFIG_AGP_INTEL_DMAR
+   return intel_iommu_gfx_remapping();
+#else
+   return 0;
+#endif
+}
+
+#ifdef CONFIG_AGP_INTEL_DMAR
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single)
+{
+   int i;
+   struct scatterlist *sg;
+
+   if (is_single) {
+   *ret = pci_map_single(intel_private.pcidev, virt, PAGE_SIZE,
+   PCI_DMA_BIDIRECTIONAL);
+   if (pci_dma_mapping_error(*ret))
+   return -EINVAL;
+   return 0;
+   }
+
+   DBG(try mapping %lu pages\n, (unsigned long)mem-page_count);
+
+   if ((mem-page_count * sizeof(*mem-sg_list))  2*PAGE_SIZE)
+   mem-sg_list = kcalloc(mem-page_count, sizeof(*mem-sg_list), 
GFP_KERNEL);
+
+   if (mem-sg_list == NULL) {
+   mem-sg_list = vmalloc(mem-page_count * sizeof(*mem-sg_list));
+   mem-sg_vmalloc_flag = 1;
+   }
+
+   if (!mem-sg_list) {
+   mem-sg_vmalloc_flag = 0;
+   return -ENOMEM;
+   }
+   sg_init_table(mem-sg_list, mem-page_count);
+
+   sg = mem-sg_list;
+   for (i = 0 ; i  mem-page_count; i++, sg = sg_next(sg))
+   sg_set_buf(sg, gart_to_virt(mem-memory[i]), PAGE_SIZE);
+
+   mem-num_sg = pci_map_sg(intel_private.pcidev, mem-sg_list,
+   mem-page_count, PCI_DMA_BIDIRECTIONAL);
+   if (mem-num_sg == 0) {
+   if (mem-sg_vmalloc_flag)
+   vfree(mem-sg_list);
+   else
+   kfree(mem-sg_list);
+   mem-sg_list = NULL;
+   mem-sg_vmalloc_flag = 0;
+   return -ENOMEM;
+   }
+   mem-is_mapped = TRUE;
+   return 0;
+}
+
+static void intel_agp_unmap_pages(struct agp_memory *mem, dma_addr_t addr, int 
is_single)
+{
+   if (is_single) {
+   pci_unmap_single(intel_private.pcidev, addr, PAGE_SIZE,
+   PCI_DMA_BIDIRECTIONAL);
+   return;
+   }
+   DBG(try unmapping %lu pages\n, (unsigned long)mem-page_count);
+
+   if (!mem-is_mapped)
+   return;
+
+   if (mem-num_sg)
+   pci_unmap_sg(intel_private.pcidev, mem-sg_list, 
mem-page_count,
+   PCI_DMA_BIDIRECTIONAL);
+   if (mem-sg_vmalloc_flag)
+   vfree(mem-sg_list);
+   else
+   kfree(mem-sg_list);
+   mem-sg_vmalloc_flag = 0;
+   mem-sg_list = NULL;
+   mem-num_sg = 0;
+}
+#else
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single)
+{
+return 0;
+}
+static void intel_agp_unmap_pages(struct agp_memory *mem, dma_addr_t addr, int 
is_single)
+{
+}
+#endif
+
 static int intel_i810_fetch_size(void)
 {
u32 smram_miscc;
@@ -991,9 +1085,40 @@ static int

Re: linux-2.6.24-rcX regression / xserver-xorg-video-intel / Q35

[Zhenyu Wang]

On 2007.12.22 01:25:16 +0100, Harald Welte wrote:
 
 I'm running an Intel DQ35JO mainboard (Q35 chipset, Q6600 CPU) and I am
 observing a regression with linux-2.6.24-rc1 through -rc6 (linux-2.6.git as
 of today, ea67db4cdbbf7f4e74150e71da0984e25121f500).
 
 The last working version is 2.6.24-rc1.
 
 The system is running debian unstable (current) using
 xserver-xorg-video-intel 2.2.0-1
 
 So what is the actual problem:
 It seems to be related to the way how the iommu/gart is used for memory
 allocation of the framebuffer memory.

There's no behavior change in intel agp module between .24-rc1 to rc6.
IOMMU shouldn't matter here, if you build x86_64 system and with
CONFIG_DMAR on, you should be already having CONFIG_DMAR_GFX_WA for you.

 
 Xorg starts just as it should, but the lower part of the screen is
 completely gobbled.  I suppose the lower part of the screen is actually
 showing off-screen memory at some completely differnt location.

Do you have other changes except kernel? like other xorg packages, bios?

 
 Interestingly, the mouse cursor is superimposed on top of the garbage
 (and it is not distorted).

yeah, hardware cursor uses another plane than scan buffer, it's
seperate.

Could you fire a bug to https://bugs.freedesktop.org with versions and
logs? Thanks.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: 2.6.24-rc6-git7: Reported regressions from 2.6.23

[Zhenyu Wang]

On 2008.01.01 22:02:57 +, Rafael J. Wysocki wrote:
 Subject   : linux-2.6.24-rcX regression / 
 xserver-xorg-video-intel / Q35
 Submitter : Harald Welte [EMAIL PROTECTED]
 Date  : 2007-12-22 04:37
 References: http://lkml.org/lkml/2007/12/21/269
 http://bugzilla.kernel.org/show_bug.cgi?id=9618
 Handled-By: Zhenyu Wang [EMAIL PROTECTED]
 Patch : 

This is resolved by turning on CONFIG_DMAR_GFX_WA.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

On 2007.12.19 13:26:08 +, Zhenyu Wang wrote:
 
 [agp-mm] [intel_iommu] explicit export current graphics dmar status
 
 To make it possbile to tell other modules about curent
 graphics dmar engine status, that could decide if graphics
 driver should remap physical address to dma address.
 
 Also this one trys to make dmar_disabled really present
 current status of DMAR, which would be used for completely
 express graphics dmar engine is active or not.
 
 Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
 ---
  drivers/pci/intel-iommu.c |   18 --
  include/linux/dmar.h  |6 ++
  2 files changed, 22 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
 index e079a52..81a0abc 100644
 --- a/drivers/pci/intel-iommu.c
 +++ b/drivers/pci/intel-iommu.c
 @@ -53,7 +53,7 @@
  static void domain_remove_dev_info(struct dmar_domain *domain);
  
  static int dmar_disabled;
 -static int __initdata dmar_map_gfx = 1;
 +static int dmar_map_gfx = 1;
  static int dmar_forcedac;
  
  #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
 @@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
  }
  __setup(intel_iommu=, intel_iommu_setup);
  
 +int intel_iommu_gfx_remapping(void)
 +{
 +#ifndef CONFIG_DMAR_GFX_WA
 + if (!dmar_disabled  iommu_detected  dmar_map_gfx)
 + return 1;
 +#endif
 + return 0;
 +}
 +EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
 +
  static struct kmem_cache *iommu_domain_cache;
  static struct kmem_cache *iommu_devinfo_cache;
  static struct kmem_cache *iommu_iova_cache;
 @@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
  
  void __init detect_intel_iommu(void)
  {
 - if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
 + if (dmar_disabled)
 + return;
 + if (swiotlb || no_iommu || iommu_detected) {
 + dmar_disabled = 1;
   return;
 + }
   if (early_dmar_detect()) {
   iommu_detected = 1;
   }
 diff --git a/include/linux/dmar.h b/include/linux/dmar.h
 index ffb6439..8ae435e 100644
 --- a/include/linux/dmar.h
 +++ b/include/linux/dmar.h
 @@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
  extern int dmar_table_init(void);
  extern int early_dmar_detect(void);
  
 +extern int intel_iommu_gfx_remapping(void);
 +
  extern struct list_head dmar_drhd_units;
  extern struct list_head dmar_rmrr_units;
  
 @@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
  {
   return -ENODEV;
  }
 +static inline int intel_iommu_gfx_remapping(void)
 +{
 + return 0;
 +}
  
  #endif /* !CONFIG_DMAR */
  #endif /* __DMAR_H__ */
 -- 
 1.5.3.4
 

Any comments to this one? Is it ok to push this iommu patch with
agp dma remapping patches to next -mm?

Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: X-freeze after clflush changes [Was: 2.6.23-rc6-mm1]

[Zhenyu Wang]

On 2007.09.21 00:10:26 +, Jiri Slaby wrote:
  Could you try current xf86-video-intel driver? just do
  git clone git://anongit.freedesktop.org/git/xorg/driver/xf86-video-intel
 
 It works! 

yep, I also pushed a fix for G33 in xf86-video-intel when fixing the intel agp.
So for G33 user, you should upgrade both to be able to work correctly.

 3d problem, but it has maybe nothing to do with kernel:
 $ glxinfo
 name of display: :0.0
 Unrecognized deviceID 29c2
 X Error of failed request:  GLXBadContext
 ...

It looks you have an old version of mesa, that i915 dri driver doesn't know
your chipset. Try mesa-7.X.

I have also seen X exit broken with 2.6.23-rc6-mm1, will follow this thread
and try Dave's patch.

Thanks for testing!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [AGPGART] intel_agp: fix stolen mem range on G33

[Zhenyu Wang]

Subject: [PATCH] [AGPGART] intel_agp: fix stolen mem range on G33

G33 GTT stolen memory is below graphics data
stolen memory and be seperate, so don't subtract
it in stolen mem counting.

Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
---
 drivers/char/agp/intel-agp.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index 2c9ca2c..20d50d0 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -506,6 +506,11 @@ static void intel_i830_init_gtt_entries(void)
break;
}
} else {
+   /* G33's GTT stolen memory is seperate from gfx data
+* stolen memory.
+*/
+   if (IS_G33)
+   size = 0;
switch (gmch_ctrl  I830_GMCH_GMS_MASK) {
case I855_GMCH_GMS_STOLEN_1M:
gtt_entries = MB(1) - KB(size);
-- 
1.5.2.3
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [AGPGART] intel_agp: fix GTT map size on G33

[Zhenyu Wang]

Subject: [PATCH] [AGPGART] intel_agp: fix GTT map size on G33

G33 has 1MB GTT table range. Fix GTT mapping
in case like 512MB aperture size.

Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
---
 drivers/char/agp/intel-agp.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index 20d50d0..6a5c555 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -919,6 +919,7 @@ static int intel_i915_create_gatt_table(struct 
agp_bridge_data *bridge)
struct aper_size_info_fixed *size;
int num_entries;
u32 temp, temp2;
+   int gtt_map_size = 256 * 1024;
 
size = agp_bridge-current_size;
page_order = size-page_order;
@@ -928,7 +929,9 @@ static int intel_i915_create_gatt_table(struct 
agp_bridge_data *bridge)
pci_read_config_dword(intel_private.pcidev, I915_MMADDR, temp);
pci_read_config_dword(intel_private.pcidev, I915_PTEADDR,temp2);
 
-   intel_private.gtt = ioremap(temp2, 256 * 1024);
+   if (IS_G33)
+   gtt_map_size = 1024 * 1024; /* 1M on G33 */
+   intel_private.gtt = ioremap(temp2, gtt_map_size);
if (!intel_private.gtt)
return -ENOMEM;
 
-- 
1.5.2.3
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

Mark, sorry for missing this for long time...

On 2008.01.08 12:44:20 -0800, mark gross wrote:
   
   [agp-mm] [intel_iommu] explicit export current graphics dmar status
   
   To make it possbile to tell other modules about curent
   graphics dmar engine status, that could decide if graphics
   driver should remap physical address to dma address.
   
   Also this one trys to make dmar_disabled really present
   current status of DMAR, which would be used for completely
   express graphics dmar engine is active or not.
 
 do you think this exporting will be needed?
 If so why and when would someone use it?

This is used for our graphics driver module to know if we have to
do dma remapping in iommu case, both in kernel config and kernel
boot time param config. 

The simplest case is that DMAR_GFX_WA is on, which no dma mapping
will act in intel_agp.

If no DMAR_GFX_WA, we still have boot param to turn off whole intel
iommu (intel_iommu=off), just turn off graphics remap engine
(intel_iommu=igfx_off). So this exported interface is used to know
that in runtime.

 
   
   Signed-off-by: Zhenyu Wang [EMAIL PROTECTED]
   ---
drivers/pci/intel-iommu.c |   18 --
include/linux/dmar.h  |6 ++
2 files changed, 22 insertions(+), 2 deletions(-)
   
   diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
   index e079a52..81a0abc 100644
   --- a/drivers/pci/intel-iommu.c
   +++ b/drivers/pci/intel-iommu.c
   @@ -53,7 +53,7 @@
static void domain_remove_dev_info(struct dmar_domain *domain);

static int dmar_disabled;
   -static int __initdata dmar_map_gfx = 1;
   +static int dmar_map_gfx = 1;
static int dmar_forcedac;

#define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
   @@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
}
__setup(intel_iommu=, intel_iommu_setup);

   +int intel_iommu_gfx_remapping(void)
   +{
   +#ifndef CONFIG_DMAR_GFX_WA
   + if (!dmar_disabled  iommu_detected  dmar_map_gfx)
   + return 1;
   +#endif
   + return 0;
   +}
   +EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
   +
static struct kmem_cache *iommu_domain_cache;
static struct kmem_cache *iommu_devinfo_cache;
static struct kmem_cache *iommu_iova_cache;
   @@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)

void __init detect_intel_iommu(void)
{
   - if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
   + if (dmar_disabled)
   + return;
   + if (swiotlb || no_iommu || iommu_detected) {
   + dmar_disabled = 1;
 return;
 
 Why the bloat?  This block of 7 lines looks like it does the same as the
 2 you replaced.

No, dmar_disabled is not set in origin, which can't tell if it's turned
off with (intel_iommu=off) later.

 
   + }
 if (early_dmar_detect()) {
 iommu_detected = 1;
 }
   diff --git a/include/linux/dmar.h b/include/linux/dmar.h
   index ffb6439..8ae435e 100644
   --- a/include/linux/dmar.h
   +++ b/include/linux/dmar.h
   @@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
extern int dmar_table_init(void);
extern int early_dmar_detect(void);

   +extern int intel_iommu_gfx_remapping(void);
   +
extern struct list_head dmar_drhd_units;
extern struct list_head dmar_rmrr_units;

   @@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
{
 return -ENODEV;
}
   +static inline int intel_iommu_gfx_remapping(void)
   +{
   + return 0;
   +}
 
 Um this function is silly lets not post it.
 

#endif /* !CONFIG_DMAR */
#endif /* __DMAR_H__ */
   -- 
   1.5.3.4
   
  
  Any comments to this one? Is it ok to push this iommu patch with
  agp dma remapping patches to next -mm?
  
 
 Its not ready for posting.
 
 --mgross
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

On 2008.01.25 10:08:20 -0800, mark gross wrote:
  This is used for our graphics driver module to know if we have to
  do dma remapping in iommu case, both in kernel config and kernel
  boot time param config. 
 
 ok. How soon do you need this export?
 

As graphics part of these patches depend on this one, it should go
first, or along with agpgart patches, that should first be in Airlie's
agp-mm queue.

 send me your current patch and eta for the graphics module patches so I
 can better coordinate with you.

You can find my current patches against agp-mm tree (so equally against
current -mm kernel) here, http://people.freedesktop.org/~zhen/, fixed
with some warnings from checkpatch.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[AGPGART] intel_agp: fix ids for 945GME, 965GME

[Zhenyu Wang]

Subject: [PATCH] [AGPGART] intel_agp: fix ids for 945GME and 965GME

These chipsets actually have new host bridge ids. This should fix the gart
init problem on them. Please apply, thanks.

Signed-off-by: Wang Zhenyu [EMAIL PROTECTED]
---
 drivers/char/agp/intel-agp.c |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index a124060..86a7e11 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -20,7 +20,9 @@
 #define PCI_DEVICE_ID_INTEL_82965G_IG   0x29A2
 #define PCI_DEVICE_ID_INTEL_82965GM_HB  0x2A00
 #define PCI_DEVICE_ID_INTEL_82965GM_IG  0x2A02
+#define PCI_DEVICE_ID_INTEL_82965GME_HB 0x2A10
 #define PCI_DEVICE_ID_INTEL_82965GME_IG 0x2A12
+#define PCI_DEVICE_ID_INTEL_82945GME_HB 0x27AC
 #define PCI_DEVICE_ID_INTEL_82945GME_IG 0x27AE
 #define PCI_DEVICE_ID_INTEL_G33_HB  0x29C0
 #define PCI_DEVICE_ID_INTEL_G33_IG  0x29C2
@@ -1848,9 +1850,9 @@ static const struct intel_driver_description {
NULL, intel_915_driver },
{ PCI_DEVICE_ID_INTEL_82945G_HB, PCI_DEVICE_ID_INTEL_82945G_IG, 0, 
945G,
NULL, intel_915_driver },
-   { PCI_DEVICE_ID_INTEL_82945GM_HB, PCI_DEVICE_ID_INTEL_82945GM_IG, 1, 
945GM,
+   { PCI_DEVICE_ID_INTEL_82945GM_HB, PCI_DEVICE_ID_INTEL_82945GM_IG, 0, 
945GM,
NULL, intel_915_driver },
-   { PCI_DEVICE_ID_INTEL_82945GM_HB, PCI_DEVICE_ID_INTEL_82945GME_IG, 0, 
945GME,
+   { PCI_DEVICE_ID_INTEL_82945GME_HB, PCI_DEVICE_ID_INTEL_82945GME_IG, 0, 
945GME,
NULL, intel_915_driver },
{ PCI_DEVICE_ID_INTEL_82946GZ_HB, PCI_DEVICE_ID_INTEL_82946GZ_IG, 0, 
946GZ,
NULL, intel_i965_driver },
@@ -1860,9 +1862,9 @@ static const struct intel_driver_description {
NULL, intel_i965_driver },
{ PCI_DEVICE_ID_INTEL_82965G_HB, PCI_DEVICE_ID_INTEL_82965G_IG, 0, 
965G,
NULL, intel_i965_driver },
-   { PCI_DEVICE_ID_INTEL_82965GM_HB, PCI_DEVICE_ID_INTEL_82965GM_IG, 1, 
965GM,
+   { PCI_DEVICE_ID_INTEL_82965GM_HB, PCI_DEVICE_ID_INTEL_82965GM_IG, 0, 
965GM,
NULL, intel_i965_driver },
-   { PCI_DEVICE_ID_INTEL_82965GM_HB, PCI_DEVICE_ID_INTEL_82965GME_IG, 0, 
965GME/GLE,
+   { PCI_DEVICE_ID_INTEL_82965GME_HB, PCI_DEVICE_ID_INTEL_82965GME_IG, 0, 
965GME/GLE,
NULL, intel_i965_driver },
{ PCI_DEVICE_ID_INTEL_7505_0, 0, 0, E7505, intel_7505_driver, NULL },
{ PCI_DEVICE_ID_INTEL_7205_0, 0, 0, E7205, intel_7505_driver, NULL },
-- 
1.5.2.3
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [Intel-gfx] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type

[Zhenyu Wang]

On 2014.06.19 17:53:51 +0800, Tiejun Chen wrote:
 Originally the reason to probe ISA bridge instead of Dev31:Fun0
 is to make graphics device passthrough work easy for VMM, that
 only need to expose ISA bridge to let driver know the real
 hardware underneath. This is a requirement from virtualization
 team. Especially in that virtualized environments, XEN, there
 is irrelevant ISA bridge in the system with that legacy qemu
 version specific to xen, qemu-xen-traditional. So to work
 reliably, we should scan through all the ISA bridge devices
 and check for the first match, instead of only checking the
 first one.
 
 But actually, qemu-xen-traditional, is always enumerated with
 Dev31:Fun0, 00:1f.0 as follows:
 
 hw/pt-graphics.c:
 
 intel_pch_init()
 |
 + pci_isa_bridge_init(bus, PCI_DEVFN(0x1f, 0), ...);
 
 so this mean that isa bridge is still represented with Dev31:Func0
 like the native OS. Furthermore, currently we're pushing VGA
 passthrough support into qemu upstream, and with some discussion,
 we wouldn't set the bridge class type and just expose this devfn.
 
 So we just go back to check devfn to make life normal.
 
 Signed-off-by: Tiejun Chen tiejun.c...@intel.com

This was added historically when supporting graphics device passthrough.
Looks qemu upstream can't accept multiple ISA bridge and our PCH is always
on device 31: func0 as far as I know. Looks good to me.

Reviewed-by: Zhenyu Wang zhen...@linux.intel.com 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: Digital signature

Re: [RFC 0/6] Non perf based Gen Graphics OA unit driver

[Zhenyu Wang]

On 2015.09.29 15:39:03 +0100, Robert Bragg wrote:
> 
> - Logistically it might be more practical to contain this to the
>   graphics stack.
> 
> It seems fair to consider that if we can't see a very compelling
> benefit to building on perf, then containing this work to
> drivers/gpu/drm/i915 may simplify the review process as well as
> future maintenance and development.
> 

I think even we all initially like to go with perf but it appears later
that we might need to stick this more close with i915 driver. Also think
about to enable global profiling for all graphics clients, extending or
enabling it within i915 specific interface seems more feasible instead of
trying to create another PMU driver like previous implementation attempt
to suit the need for different gfx perf data definition.

Robert, thanks for send and elaborate on this.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: Digital signature

Re: [igvt-g-dev] [PATCH] drm/i915/gvt: Fix kmem_cache_create() name

[Zhenyu Wang]

On 2017.01.24 13:15:43 -0700, Alex Williamson wrote:
> According to kmem_cache_sanity_check(), spaces are not allowed in the
> name of a cache and results in a kernel oops with CONFIG_DEBUG_VM.
> Convert to underscores.
> 
> Signed-off-by: Alex Williamson 
> ---

Will send to 4.10 fixes. Thanks!

>  drivers/gpu/drm/i915/gvt/execlist.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/execlist.c 
> b/drivers/gpu/drm/i915/gvt/execlist.c
> index f32bb6f..d03b229 100644
> --- a/drivers/gpu/drm/i915/gvt/execlist.c
> +++ b/drivers/gpu/drm/i915/gvt/execlist.c
> @@ -826,7 +826,7 @@ int intel_vgpu_init_execlist(struct intel_vgpu *vgpu)
>   INIT_LIST_HEAD(>workload_q_head[i]);
>   }
>  
> - vgpu->workloads = kmem_cache_create("gvt-g vgpu workload",
> + vgpu->workloads = kmem_cache_create("gvt-g_vgpu_workload",
>   sizeof(struct intel_vgpu_workload), 0,
>   SLAB_HWCACHE_ALIGN,
>   NULL);
> 
> ___
> igvt-g-dev mailing list
> igvt-g-...@lists.01.org
> https://lists.01.org/mailman/listinfo/igvt-g-dev

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [igvt-g-dev] [PATCH] drm/i915/gvt/kvmgt: mdev ABI is available_instances, not available_instance

[Zhenyu Wang]

On 2017.01.24 12:53:45 -0700, Alex Williamson wrote:
> Per the ABI specification[1], each mdev_supported_types entry should
> have an available_instances, with an "s", not available_instance.
> 
> [1] Documentation/ABI/testing/sysfs-bus-vfio-mdev
> 
> Signed-off-by: Alex Williamson 
> ---
> 
> This should really be fixed before initial release in v4.10

Will queue this up for 4.10 fixes. Thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH -next] drm/i915/gvt: fix return value check

[Zhenyu Wang]

On 2016.10.19 16:18:03 +, Wei Yongjun wrote:
> From: Wei Yongjun 
> 
> In case of error, the function i915_gem_object_create() returns
> ERR_PTR() not NULL. The NULL test in the return value check should
> be replaced with IS_ERR().
> 
> Signed-off-by: Wei Yongjun 

Hi, Yongjun, we've already had this fixed in our queue for next pull request,
will send very soon.

Thanks.

> ---
>  drivers/gpu/drm/i915/gvt/cmd_parser.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c 
> b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index 5808ee7..6abb2a6 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -1640,8 +1640,8 @@ static int perform_bb_shadow(struct parser_exec_state 
> *s)
>  
>   entry_obj->obj = i915_gem_object_create(&(s->vgpu->gvt->dev_priv->drm),
>   round_up(bb_size, PAGE_SIZE));
> - if (entry_obj->obj == NULL)
> - return -ENOMEM;
> + if (IS_ERR(entry_obj->obj))
> + return PTR_ERR(entry_obj->obj);
>   entry_obj->len = bb_size;
>   INIT_LIST_HEAD(_obj->list);
>  
> @@ -2712,8 +2712,8 @@ static int shadow_indirect_ctx(struct 
> intel_shadow_wa_ctx *wa_ctx)
>  
>   wa_ctx->indirect_ctx.obj = i915_gem_object_create(dev,
>   round_up(ctx_size + CACHELINE_BYTES, PAGE_SIZE));
> - if (wa_ctx->indirect_ctx.obj == NULL)
> - return -ENOMEM;
> + if (IS_ERR(wa_ctx->indirect_ctx.obj))
> + return PTR_ERR(wa_ctx->indirect_ctx.obj);
>  
>   ret = i915_gem_object_get_pages(wa_ctx->indirect_ctx.obj);
>   if (ret)
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH 2/2] drm/i915/gvt: fix compilation

[Zhenyu Wang]

On 2016.10.21 17:25:50 +0200, Arnd Bergmann wrote:
> Two functions in the newly added gvt render code are obviously
> broken, as they reference a variable without initialization and
> don't reference another variable at all:
> 
> drivers/gpu/drm/i915/gvt/render.c: In function 
> ???intel_gvt_load_render_mmio???:
> drivers/gpu/drm/i915/gvt/render.c:148:13: error: ???offset.reg??? may be used 
> uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/gpu/drm/i915/gvt/render.c: In function 
> ???intel_gvt_restore_render_mmio???:
> drivers/gpu/drm/i915/gvt/render.c:185:13: error: ???offset.reg??? may be used 
> uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This is probably not a correct fix, but it gets us a clean build
> by removing the unused arrays and initializing the offset variable
> to something that potentially might be correct.
> 
> Fixes: 178657139307 ("drm/i915/gvt: vGPU context switch")
> Signed-off-by: Arnd Bergmann 
> ---

I think the correct fix is like

diff --git a/drivers/gpu/drm/i915/gvt/render.c 
b/drivers/gpu/drm/i915/gvt/render.c
index feebb65..cc23c3f 100644
--- a/drivers/gpu/drm/i915/gvt/render.c
+++ b/drivers/gpu/drm/i915/gvt/render.c
@@ -162,6 +162,7 @@ static void load_mocs(struct intel_vgpu *vgpu, int ring_id)
if (!IS_SKYLAKE(dev_priv))
return;
 
+   offset.reg = regs[ring_id];
for (i = 0; i < 64; i++) {
gen9_render_mocs[ring_id][i] = I915_READ(offset);
I915_WRITE(offset, vgpu_vreg(vgpu, offset));
@@ -199,6 +200,7 @@ static void restore_mocs(struct intel_vgpu *vgpu, int 
ring_id)
if (!IS_SKYLAKE(dev_priv))
return;
 
+   offset.reg = regs[ring_id];
for (i = 0; i < 64; i++) {
vgpu_vreg(vgpu, offset) = I915_READ(offset);
I915_WRITE(offset, gen9_render_mocs[ring_id][i]);

Thanks for pointing this out, it's a mistake during our code preparation for 
upstream.
I'll queue this up.

>  drivers/gpu/drm/i915/gvt/render.c | 25 +++--
>  1 file changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/render.c 
> b/drivers/gpu/drm/i915/gvt/render.c
> index feebb65ba641..79e112288065 100644
> --- a/drivers/gpu/drm/i915/gvt/render.c
> +++ b/drivers/gpu/drm/i915/gvt/render.c
> @@ -147,29 +147,20 @@ static void load_mocs(struct intel_vgpu *vgpu, int 
> ring_id)
>  {
>   struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
>   i915_reg_t offset, l3_offset;
> - u32 regs[] = {
> - [RCS] = 0xc800,
> - [VCS] = 0xc900,
> - [VCS2] = 0xca00,
> - [BCS] = 0xcc00,
> - [VECS] = 0xcb00,
> - };
>   int i;
>  
> - if (WARN_ON(ring_id >= ARRAY_SIZE(regs)))
> - return;
> -
>   if (!IS_SKYLAKE(dev_priv))
>   return;
>  
>   for (i = 0; i < 64; i++) {
> + offset.reg = i * 4;
>   gen9_render_mocs[ring_id][i] = I915_READ(offset);
>   I915_WRITE(offset, vgpu_vreg(vgpu, offset));
>   POSTING_READ(offset);
> - offset.reg += 4;
>   }
>  
>   if (ring_id == RCS) {
> + offset.reg = 64 * 4;
>   l3_offset.reg = 0xb020;
>   for (i = 0; i < 32; i++) {
>   gen9_render_mocs_L3[i] = I915_READ(l3_offset);
> @@ -184,26 +175,16 @@ static void restore_mocs(struct intel_vgpu *vgpu, int 
> ring_id)
>  {
>   struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
>   i915_reg_t offset, l3_offset;
> - u32 regs[] = {
> - [RCS] = 0xc800,
> - [VCS] = 0xc900,
> - [VCS2] = 0xca00,
> - [BCS] = 0xcc00,
> - [VECS] = 0xcb00,
> - };
>   int i;
>  
> - if (WARN_ON(ring_id >= ARRAY_SIZE(regs)))
> - return;
> -
>   if (!IS_SKYLAKE(dev_priv))
>   return;
>  
>   for (i = 0; i < 64; i++) {
> + offset.reg = i * 4;
>   vgpu_vreg(vgpu, offset) = I915_READ(offset);
>   I915_WRITE(offset, gen9_render_mocs[ring_id][i]);
>   POSTING_READ(offset);
> - offset.reg += 4;
>   }
>  
>   if (ring_id == RCS) {
> -- 
> 2.9.0
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH 1/2] drm/i915/gvt: add ACPI and 64BIT dependencies

[Zhenyu Wang]

On 2016.10.21 17:25:49 +0200, Arnd Bergmann wrote:
> The newly added gvt code produces lots of serious warnings and errors
> when either built on 32-bit x86, or built with ACPI disabled, e.g.
> 
> drivers/gpu/drm/i915/gvt/gtt.c: In function ???read_pte64???:
> drivers/gpu/drm/i915/gvt/gtt.c:277:2: error: left shift count >= width of 
> type [-Werror]
> drivers/gpu/drm/i915/gvt/gtt.c: In function ???gen8_gtt_get_pfn???:
> drivers/gpu/drm/i915/gvt/gtt.c:360:3: error: left shift count >= width of 
> type [-Werror]
> drivers/gpu/drm/i915/gvt/opregion.c: In function 
> ???intel_gvt_init_opregion???:
> drivers/gpu/drm/i915/gvt/opregion.c:183:2: error: implicit declaration of 
> function ???acpi_os_ioremap??? [-Werror=implicit-function-declaration]
> 
> This avoids the problems by simply disallowing those configurations
> in Kconfig. I'm sure it's possible to make the code more portable
> and support building GVT without those options, but it might not be
> useful to do so.
> 
> Fixes: 4d60c5fd3f87 ("drm/i915/gvt: vGPU PCI configuration space 
> virtualization")
> Signed-off-by: Arnd Bergmann 
> ---
> If the code is meant to work on 32-bit and non-ACPI kernels, please
> treat this as a bug report and disregard the patch.
> ---

Thanks, Arnd. We have to depend on 64bit now and not require for ACPI,
as we used one acpi function for opregion mem map which is not necessary,
so I queued one 64bit dependence and another to remove acpi dependence for 
Daniel.

>  drivers/gpu/drm/i915/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 6d4194288d11..1b9308284dde 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -84,6 +84,7 @@ config DRM_I915_USERPTR
>  config DRM_I915_GVT
>  bool "Enable Intel GVT-g graphics virtualization host support"
>  depends on DRM_I915
> + depends on 64BIT && ACPI
>  default n
>  help
> Choose this option if you want to enable Intel GVT-g graphics
> -- 
> 2.9.0
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [Intel-gfx] linux-next: Tree for Oct 20 (gpu/drm/i915)

[Zhenyu Wang]

On 2016.10.20 21:25:03 +0300, Jani Nikula wrote:
> On Thu, 20 Oct 2016, Daniel Vetter  wrote:
> > On Thu, Oct 20, 2016 at 7:37 PM, Randy Dunlap  wrote:
> >> On 10/19/16 20:20, Stephen Rothwell wrote:
> >>> Hi all,
> >>>
> >>> Changes since 20161019:
> >>>
> >>
> >> on i386: when CONFIG_ACPI is not enabled:
> >
> > Adding Zhenyu. Might be good to have a fix just for this that I
> > directly pick up, since I want to tag the first 4.10 pull for Dave
> > Airlie this w/e.
> 
> How about just this?
>

I'd like to not depend on acpi function any more, so just change that
for memremap. GVT-g driver currently only supports 64BIT kernel so will
fix that dependence. I'll send fix pull for Daniel later.

thanks

> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 6aedc96aa412..94914381e8ef 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -85,7 +85,7 @@ config DRM_I915_USERPTR
>  
>  config DRM_I915_GVT
>  bool "Enable Intel GVT-g graphics virtualization host support"
> -depends on DRM_I915
> +depends on DRM_I915 && ACPI
>  default n
>  help
> Choose this option if you want to enable Intel GVT-g graphics
> 
> 
> 
> > -Daniel
> >
> >> ../drivers/gpu/drm/i915/gvt/opregion.c: In function 
> >> 'intel_gvt_init_opregion':
> >> ../drivers/gpu/drm/i915/gvt/opregion.c:183:2: error: implicit declaration 
> >> of function 'acpi_os_ioremap' [-Werror=implicit-function-declaration]
> >>   gvt->opregion.opregion_va = acpi_os_ioremap(gvt->opregion.opregion_pa,
> >>   ^
> >> ../drivers/gpu/drm/i915/gvt/opregion.c:183:28: warning: assignment makes 
> >> pointer from integer without a cast [enabled by default]
> >>   gvt->opregion.opregion_va = acpi_os_ioremap(gvt->opregion.opregion_pa,
> >> ^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c: In function 'read_pte64':
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:277:2: warning: left shift count >= 
> >> width of type [enabled by default]
> >>   pte |= ioread32(addr + 4) << 32;
> >>   ^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c: In function 'gen8_gtt_get_pfn':
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:360:3: warning: left shift count >= 
> >> width of type [enabled by default]
> >>pfn = (e->val64 & ADDR_4K_MASK) >> 12;
> >>^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c: In function 'gen8_gtt_set_pfn':
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:373:3: warning: left shift count >= 
> >> width of type [enabled by default]
> >>e->val64 &= ~ADDR_4K_MASK;
> >>^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:374:3: warning: left shift count >= 
> >> width of type [enabled by default]
> >>pfn &= (ADDR_4K_MASK >> 12);
> >>^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c: In function 'gen8_gma_to_pml4_index':
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:436:1: warning: right shift count >= 
> >> width of type [enabled by default]
> >>  DEFINE_PPGTT_GMA_TO_INDEX(gen8, pml4, (gma >> 39 & 0x1ff));
> >>  ^
> >>   CC  drivers/gpu/drm/radeon/si_smc.o
> >> In file included from ../drivers/gpu/drm/i915/i915_drv.h:46:0,
> >>  from ../drivers/gpu/drm/i915/gvt/gtt.c:36:
> >> ../drivers/gpu/drm/i915/gvt/gtt.c: In function 
> >> 'intel_gvt_create_scratch_page':
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:1945:47: warning: cast from pointer to 
> >> integer of different size [-Wpointer-to-int-cast]
> >>gvt_err("fail to translate vaddr:0x%llx\n", (u64)vaddr);
> >>^
> >> ../include/drm/drmP.h:201:43: note: in definition of macro 'DRM_ERROR'
> >>   drm_printk(KERN_ERR, DRM_UT_NONE, fmt, ##__VA_ARGS__)
> >>^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:1945:3: note: in expansion of macro 
> >> 'gvt_err'
> >>gvt_err("fail to translate vaddr:0x%llx\n", (u64)vaddr);
> >>^
> >>
> >>
> >>
> >> --
> >> ~Randy
> >> ___
> >> Intel-gfx mailing list
> >> intel-...@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: linux-next: build warnings after merge of the drm-intel tree

[Zhenyu Wang]

On 2016.10.19 10:57:53 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the drm-intel tree, today's linux-next build (x86_64
> allmodconfig) produced these warnings:
> 
> drivers/gpu/drm/i915/gvt/execlist.c: In function 
> 'release_shadow_batch_buffer':
> drivers/gpu/drm/i915/gvt/execlist.c:501:4: warning: 
> 'drm_gem_object_unreference' is deprecated [-Wdeprecated-declarations]
> drm_gem_object_unreference(&(entry_obj->obj->base));
> ^
> In file included from drivers/gpu/drm/i915/gvt/execlist.c:35:0:
> drivers/gpu/drm/i915/i915_drv.h:2344:13: note: declared here
>  extern void drm_gem_object_unreference(struct drm_gem_object *);
>  ^
> drivers/gpu/drm/i915/gvt/execlist.c: In function 'release_shadow_wa_ctx':
> drivers/gpu/drm/i915/gvt/execlist.c:514:2: warning: 
> 'drm_gem_object_unreference' is deprecated [-Wdeprecated-declarations]
>   drm_gem_object_unreference(&(wa_ctx->indirect_ctx.obj->base));
>   ^
> In file included from drivers/gpu/drm/i915/gvt/execlist.c:35:0:
> drivers/gpu/drm/i915/i915_drv.h:2344:13: note: declared here
>  extern void drm_gem_object_unreference(struct drm_gem_object *);
>  ^
> 
> Introduced by commit
> 
>   be1da7070aea ("drm/i915/gvt: vGPU command scanner")
> 
> interacting with commit
> 
>   f8c417cdb1b8 ("drm/i915: Rename drm_gem_object_unreference in preparation 
> for lockless free")
> 
> from Linus' tree.
> 

Sorry for this, we will fix that in next pull request to replace old interface.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH v14 00/22] Add Mediated device support

[Zhenyu Wang]

On 2016.11.17 16:51:45 -0700, Alex Williamson wrote:
> On Thu, 17 Nov 2016 23:29:38 +
> "Tian, Kevin"  wrote:
> 
> > > From: Alex Williamson [mailto:alex.william...@redhat.com]
> > > Sent: Friday, November 18, 2016 5:25 AM
> > > 
> > > On Thu, 17 Nov 2016 02:16:12 +0530
> > > Kirti Wankhede  wrote:  
> > > >
> > > >  Documentation/ABI/testing/sysfs-bus-vfio-mdev |  111 ++
> > > >  Documentation/vfio-mediated-device.txt|  399 +++
> > > >  MAINTAINERS   |9 +
> > > >  drivers/vfio/Kconfig  |1 +
> > > >  drivers/vfio/Makefile |1 +
> > > >  drivers/vfio/mdev/Kconfig |   17 +
> > > >  drivers/vfio/mdev/Makefile|5 +
> > > >  drivers/vfio/mdev/mdev_core.c |  385 +++
> > > >  drivers/vfio/mdev/mdev_driver.c   |  119 ++
> > > >  drivers/vfio/mdev/mdev_private.h  |   41 +
> > > >  drivers/vfio/mdev/mdev_sysfs.c|  286 +
> > > >  drivers/vfio/mdev/vfio_mdev.c |  180 +++
> > > >  drivers/vfio/pci/vfio_pci.c   |   83 +-
> > > >  drivers/vfio/platform/vfio_platform_common.c  |   31 +-
> > > >  drivers/vfio/vfio.c   |  340 +-
> > > >  drivers/vfio/vfio_iommu_type1.c   |  872 +++---
> > > >  include/linux/mdev.h  |  177 +++
> > > >  include/linux/vfio.h  |   32 +-
> > > >  include/uapi/linux/vfio.h |   10 +
> > > >  samples/vfio-mdev/Makefile|   13 +
> > > >  samples/vfio-mdev/mtty.c  | 1503  
> > > +  
> > > >  21 files changed, 4358 insertions(+), 257 deletions(-)
> > > >  create mode 100644 Documentation/ABI/testing/sysfs-bus-vfio-mdev
> > > >  create mode 100644 Documentation/vfio-mediated-device.txt
> > > >  create mode 100644 drivers/vfio/mdev/Kconfig
> > > >  create mode 100644 drivers/vfio/mdev/Makefile
> > > >  create mode 100644 drivers/vfio/mdev/mdev_core.c
> > > >  create mode 100644 drivers/vfio/mdev/mdev_driver.c
> > > >  create mode 100644 drivers/vfio/mdev/mdev_private.h
> > > >  create mode 100644 drivers/vfio/mdev/mdev_sysfs.c
> > > >  create mode 100644 drivers/vfio/mdev/vfio_mdev.c
> > > >  create mode 100644 include/linux/mdev.h
> > > >  create mode 100644 samples/vfio-mdev/Makefile
> > > >  create mode 100644 samples/vfio-mdev/mtty.c  
> > > 
> > > As discussed, I dropped patch 12, updated the documentation, and added
> > > 'retries' initialization.  This is now applied to my next branch for
> > > v4.10.  Thanks to the reviewers and Kirti and Neo for your hard work!
> > > Thanks,
> > >   
> > 
> > That's a great news! Alex, do you have an idea when this series may
> > hit linux-next? :-)
> 
> Whenever there's a new build, hopefully within the next 24hrs, but I
> don't really know the schedule.  Thanks,
> 

Alex, could you do a pull request of mdev for Daniel's drm-intel tree?
We need to send KVMGT mdev support pull base on that.

Thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

[RESEND][GIT PULL] i915/gvt KVMGT tree for 4.10

[Zhenyu Wang]

Hi, Linus

This is KVMGT pull for 4.10 as explained by Daniel. The last minute
rebase is to appease git pull-request since the diffstat was obvious
bonghits and somehow included vfio/mdev again despite that was merged
already.

Thanks.

The following changes since commit 9439b3710df688d853eb6cb4851256f2c92b1797:

  Merge tag 'drm-for-v4.10' of git://people.freedesktop.org/~airlied/linux 
(2016-12-13 09:35:09 -0800)

are available in the git repository at:

  https://github.com/01org/gvt-linux.git tags/kvmgt-vfio-mdev-for-v4.10-rc1

for you to fetch changes up to 659643f7d81432189c2c87230e2feee4c75c14c1:

  drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT (2016-12-16 16:55:26 +0800)


kvmgt-vfio-mdev-for-v4.10-rc1

This is KVMGT support depending on VFIO/mdev framework.


Jike Song (3):
  drm/i915/gvt/kvmgt: replace kmalloc() by kzalloc()
  drm/i915/gvt/kvmgt: read/write GPA via KVM API
  drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT

 drivers/gpu/drm/i915/Kconfig  |   1 +
 drivers/gpu/drm/i915/gvt/Makefile |   2 -
 drivers/gpu/drm/i915/gvt/gvt.h|   6 +-
 drivers/gpu/drm/i915/gvt/kvmgt.c  | 975 +++---
 4 files changed, 923 insertions(+), 61 deletions(-)

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915/gvt: fix deadlock in dispatch_workload()'s error path

[Zhenyu Wang]

On 2016.12.04 23:57:18 +, Eric Engestrom wrote:
> 90d27a1 moved the lock before this error path but forgot to add an
> unlock here.
> 
> Fixes: 90d27a1b180e51ef0713 ("drm/i915/gvt: fix deadlock in workload_thread")
> Cc: Pei Zhang <pei.zh...@intel.com>
> Cc: Zhenyu Wang <zhen...@linux.intel.com>
> Signed-off-by: Eric Engestrom <e...@engestrom.ch>
> ---

Hi, this has been fixed on 
https://cgit.freedesktop.org/drm/drm-intel/commit/?h=drm-intel-next-fixes=53d6f812c0dbf1c9cad89b1c2118e61c13ca9677

Thanks!

>  drivers/gpu/drm/i915/gvt/scheduler.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
> b/drivers/gpu/drm/i915/gvt/scheduler.c
> index f898df3..cd13c4b 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -177,6 +177,7 @@ static int dispatch_workload(struct intel_vgpu_workload 
> *workload)
>   rq = i915_gem_request_alloc(dev_priv->engine[ring_id], shadow_ctx);
>   if (IS_ERR(rq)) {
>   gvt_err("fail to allocate gem request\n");
> + mutex_unlock(_priv->drm.struct_mutex);
>   workload->status = PTR_ERR(rq);
>   return workload->status;
>   }
> -- 
> Cheers,
>   Eric
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: linux-next: build failure after merge of the drm-intel-fixes tree

[Zhenyu Wang]

On 2017.01.02 21:48:57 -0700, Alex Williamson wrote:
> > Alex, I liked to have kvmgt related mdev interface change be merged through
> > vfio tree, but wasn't awared one of Jike's fix had conflict. Could you apply
> > below fix in your tree? I think in general for possible interface change in
> > future we still need a pull request for i915 to resolve dependence earlier.
> 
> Hi Zhenyu,
> 
> Hopefully this abstraction will help to isolate vendor drivers from
> mdev API changes in the future.  I can certainly roll this patch into
> the original to maintain bisectability.  I want to get these changes in
> for rc3, will a pull request for the i915 changes be sent this week?

Send to Jani who is managing i915 fixes pull.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: linux-next: build failure after merge of the drm-intel-fixes tree

[Zhenyu Wang]

On 2017.01.03 10:42:39 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the drm-intel-fixes tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> drivers/gpu/drm/i915/gvt/kvmgt.c: In function 'intel_vgpu_open':
> drivers/gpu/drm/i915/gvt/kvmgt.c:511:32: error: dereferencing pointer to 
> incomplete type 'struct mdev_device'
>   vfio_unregister_notifier(>dev, VFIO_GROUP_NOTIFY,
> ^   
> 
> Caused by commit
> 
>   99e3123e3d72 ("vfio-mdev: Make mdev_device private and abstract interfaces")
> 
> from the vfio-fixes tree interacting with commit
> 
>   364fb6b789ff ("drm/i915/gvt/kvmgt: prevent double-release of vgpu")
> 
> from the drm-intel-fixes tree.

Alex, I liked to have kvmgt related mdev interface change be merged through
vfio tree, but wasn't awared one of Jike's fix had conflict. Could you apply
below fix in your tree? I think in general for possible interface change in
future we still need a pull request for i915 to resolve dependence earlier.

Thanks.

> 
> I applied this merge fix patch:
> 
> From: Stephen Rothwell 
> Date: Tue, 3 Jan 2017 10:38:48 +1100
> Subject: [PATCH] vfio-mdev: fixup for "Make mdev_device private and abstract 
> interfaces"
> 
> Signed-off-by: Stephen Rothwell 
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index c24b665e007b..faaae07ae487 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -508,7 +508,7 @@ static int intel_vgpu_open(struct mdev_device *mdev)
>   return ret;
>  
>  undo_group:
> - vfio_unregister_notifier(>dev, VFIO_GROUP_NOTIFY,
> + vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
>   >vdev.group_notifier);
>  
>  undo_iommu:
> -- 
> 2.10.2
> 
> -- 
> Cheers,
> Stephen Rothwell

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [igvt-g-dev] [PATCH 1/1] drm/i915/gvt: verify functions types in new_mmio_info()

[Zhenyu Wang]

On 2016.12.26 14:52:23 +0100, Nicolas Iooss wrote:
> The current prototype of new_mmio_info() uses void* for parameters read
> and write, which are functions with precise calling conventions
> (argument types and return type). Write down these conventions in
> new_mmio_info() definition.
> 
> This has been reported by the following warnings when clang is used to
> build the kernel:
> 
> drivers/gpu/drm/i915/gvt/handlers.c:124:21: error: pointer type
> mismatch ('void *' and 'int (*)(struct intel_vgpu *, unsigned int,
> void *, unsigned int)') [-Werror,-Wpointer-type-mismatch]
> info->read = read ? read : intel_vgpu_default_mmio_read;
>   ^    
> drivers/gpu/drm/i915/gvt/handlers.c:125:23: error: pointer type
> mismatch ('void *' and 'int (*)(struct intel_vgpu *, unsigned int,
> void *, unsigned int)') [-Werror,-Wpointer-type-mismatch]
> info->write = write ? write : intel_vgpu_default_mmio_write;
> ^ ~   ~
> 
> This allows the compiler to detect that sbi_ctl_mmio_write() returns a
> "bool" value instead of an expected "int" one. Fix this.
>

Looks good to me. Will queue this up. Thanks

> Signed-off-by: Nicolas Iooss 
> ---
>  drivers/gpu/drm/i915/gvt/handlers.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index 522809710312..052e57124c0a 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -93,7 +93,8 @@ static void write_vreg(struct intel_vgpu *vgpu, unsigned 
> int offset,
>  static int new_mmio_info(struct intel_gvt *gvt,
>   u32 offset, u32 flags, u32 size,
>   u32 addr_mask, u32 ro_mask, u32 device,
> - void *read, void *write)
> + int (*read)(struct intel_vgpu *, unsigned int, void *, unsigned 
> int),
> + int (*write)(struct intel_vgpu *, unsigned int, void *, 
> unsigned int))
>  {
>   struct intel_gvt_mmio_info *info, *p;
>   u32 start, end, i;
> @@ -974,7 +975,7 @@ static int sbi_data_mmio_read(struct intel_vgpu *vgpu, 
> unsigned int offset,
>   return 0;
>  }
>  
> -static bool sbi_ctl_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
> +static int sbi_ctl_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
>   void *p_data, unsigned int bytes)
>  {
>   u32 data;
> -- 
> 2.11.0
> 
> ___
> igvt-g-dev mailing list
> igvt-g-...@lists.01.org
> https://lists.01.org/mailman/listinfo/igvt-g-dev

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915/kvmgt: avoid dereferencing a potentially null info pointer

[Zhenyu Wang]

On 2017.03.23 14:43:44 +0100, Frans Klaver wrote:
> On Thu, Mar 23, 2017 at 1:22 PM, Colin King  wrote:
> > From: Colin Ian King 
> >
> > info is being checked to see if it is a null pointer, however, vpgu is
> > dereferencing info before this check, leading to a potential null
> > pointer dereference.  If info is null, then the error message being
> > printed by macro gvt_vgpu_err and this requires vpgu to exist. We can
> > use a null vpgu as the macro has a sanity check to see if vpgu is null,
> > so this is OK.
> >
> > Detected with CoverityScan, CID#1420672 ("Dereference nefore null check")
> 
> s,nefore,before,
> 
> 
> >
> > Fixes: 695fbc08d80f ("drm/i915/gvt: replace the gvt_err with gvt_vgpu_err")
> > Signed-off-by: Colin Ian King 
> > ---
> >  drivers/gpu/drm/i915/gvt/kvmgt.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> > b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 1ea3eb270de8..f8619a772c44 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1339,9 +1339,9 @@ static int kvmgt_guest_init(struct mdev_device *mdev)
> >
> >  static bool kvmgt_guest_exit(struct kvmgt_guest_info *info)
> >  {
> > -   struct intel_vgpu *vgpu = info->vgpu;
> > -
> > if (!info) {
> > +   struct intel_vgpu *vgpu = NULL;
> > +
> > gvt_vgpu_err("kvmgt_guest_info invalid\n");
> > return false;
> > }
> 
> Does this mean the original gvt_err() macro is no longer there?
> 
> And apparently gvt_vgpu_err is a macro that depends on the specifics
> of its environment? Yikes.
> 

The null check is redundant there, we can just remove that block and extra
vgpu variable.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] kvmgt: Hold struct kvm reference

[Zhenyu Wang]

On 2017.03.20 14:58:06 +0800, Jike Song wrote:
> On 03/20/2017 10:38 AM, Alex Williamson wrote:
> > The kvmgt code keeps a pointer to the struct kvm associated with the
> > device, but doesn't actually hold a reference to it.  If we do unclean
> > shutdown testing (ie. killing the user process), then we can see the
> > kvm association to the device unset, which causes kvmgt to trigger a
> > device release via a work queue.  Naturally we cannot guarantee that
> > the cached struct kvm pointer is still valid at this point without
> > holding a reference.  The observed failure in this case is a stuck
> > cpu trying to acquire the spinlock from the invalid reference, but
> > other failure modes are clearly possible.  Hold a reference to avoid
> > this.
> > 
> > Signed-off-by: Alex Williamson <alex.william...@redhat.com>
> > Cc: sta...@vger.kernel.org #v4.10
> > Cc: Jike Song <jike.s...@intel.com>
> > Cc: Paolo Bonzini <pbonz...@redhat.com>
> > Cc: Zhenyu Wang <zhen...@linux.intel.com>
> > Cc: Zhi Wang <zhi.a.w...@intel.com>
> > ---
> 
> Reviewed-by: Jike Song <jike.s...@intel.com>
> 
> Thanks for the fix!
> 

queued in fixes tree, thanks!

> 
> >  drivers/gpu/drm/i915/gvt/kvmgt.c |2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> > b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 84d801638ede..142b8bd4ba6b 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1324,6 +1324,7 @@ static int kvmgt_guest_init(struct mdev_device *mdev)
> > vgpu->handle = (unsigned long)info;
> > info->vgpu = vgpu;
> > info->kvm = kvm;
> > +   kvm_get_kvm(info->kvm);
> >  
> > kvmgt_protect_table_init(info);
> > gvt_cache_init(vgpu);
> > @@ -1343,6 +1344,7 @@ static bool kvmgt_guest_exit(struct kvmgt_guest_info 
> > *info)
> > }
> >  
> > kvm_page_track_unregister_notifier(info->kvm, >track_node);
> > +   kvm_put_kvm(info->kvm);
> > kvmgt_protect_table_destroy(info);
> > gvt_cache_destroy(info->vgpu);
> > vfree(info);
> > 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915: disable KASAN for handlers

[Zhenyu Wang]

On 2017.03.30 11:46:27 +0200, Jiri Slaby wrote:
> Handlers are currently the only blocker to compile the kernel with gcc 7
> and KASAN+use-after-scope enabled:
> drivers/gpu/drm/i915/gvt/handlers.c:2200:1: error: the frame size of 43760 
> bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> drivers/gpu/drm/i915/gvt/handlers.c:2402:1: error: the frame size of 9400 
> bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> drivers/gpu/drm/i915/gvt/handlers.c:2628:1: error: the frame size of 11256 
> bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> 
> It is due to many expansions of MMIO_* macros in init_generic_mmio_info.
> INTEL_GVT_MMIO_OFFSET generates for each such line a __reg and an
> offset. There are too many for KASAN to keep up.
> 
> So disable KASAN for this file.
> 
> Signed-off-by: Jiri Slaby <jsl...@suse.cz>
> Cc: Martin Liska <mli...@suse.cz>
> Cc: Zhenyu Wang <zhen...@linux.intel.com>
> Cc: Zhi Wang <zhi.a.w...@intel.com>
> Cc: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Jani Nikula <jani.nik...@linux.intel.com>
> Cc: David Airlie <airl...@linux.ie>
> Cc: intel-gvt-...@lists.freedesktop.org
> Cc: intel-...@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org
> ---
>  drivers/gpu/drm/i915/gvt/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile 
> b/drivers/gpu/drm/i915/gvt/Makefile
> index b123c20e2097..942f1849d194 100644
> --- a/drivers/gpu/drm/i915/gvt/Makefile
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -6,3 +6,5 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o 
> trace_points.o firmware.o \
>  ccflags-y+= -I$(src) -I$(src)/$(GVT_DIR) -Wall
>  i915-y   += $(addprefix $(GVT_DIR)/, 
> $(GVT_SOURCE))
>  obj-$(CONFIG_DRM_I915_GVT_KVMGT) += $(GVT_DIR)/kvmgt.o
> +
> +KASAN_SANITIZE_handlers.o := n
> -- 
> 2.12.2
> 

Applied this, we'd better cleanup legacy usage to current i915 mmio reg define. 
Thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH v10] vfio: ABI for mdev display dma-buf operation

[Zhenyu Wang]

On 2017.07.11 11:12:36 +0200, Daniel Vetter wrote:
> On Tue, Jul 11, 2017 at 08:14:08AM +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > > +struct vfio_device_query_gfx_plane {
> > > > +   __u32 argsz;
> > > > +   __u32 flags;
> > > > +   struct vfio_device_gfx_plane_info plane_info;
> > > > +   __u32 plane_type;
> > > > +   __s32 fd; /* dma-buf fd */
> > > > +   __u32 plane_id;
> > > > +};
> > > > +
> > > 
> > > It would be better to have comment here about what are expected
> > > values
> > > for plane_type and plane_id.
> > 
> > plane_type is DRM_PLANE_TYPE_*.
> > 
> > yes, a comment saying so would be good, same for drm_format which is
> > DRM_FORMAT_*.  While looking at these two: renaming plane_type to
> > drm_plane_type (for consistency) is probably a good idea too.

For drm universal plane, this is not in drm uapi, but uabi. I think we
can align with drm plane definition for sure, but not need to pull in
drm header for that enum type.

> > 
> > plane_id needs a specification.
> 
> Why do you need plane_type? With universal planes the plane_id along is
> sufficient to identify a plane on a given drm device instance. I'd just
> remove it.

This interface is to get vGPU display plane info, there's no normal
drm kms client involved, but vGPU device model trys to expose guest
planes for display. We need to ask for what type of plane required on
target vGPU. I think plane_id here doesn't mean like in drm kms,
but I'm not sure about plane_id here without details, what's the
purpose, etc.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915/gvt: fix typo: "supporte" -> "support"

[Zhenyu Wang]

On 2017.04.25 10:05:12 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> trivial fix to typo in WARN_ONCE message
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/gpu/drm/i915/gvt/handlers.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index 0ad1a508e2af..c995e540ff96 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -1244,7 +1244,7 @@ static int dma_ctrl_write(struct intel_vgpu *vgpu, 
> unsigned int offset,
>   mode = vgpu_vreg(vgpu, offset);
>  
>   if (GFX_MODE_BIT_SET_IN_MASK(mode, START_DMA)) {
> - WARN_ONCE(1, "VM(%d): iGVT-g doesn't supporte GuC\n",
> + WARN_ONCE(1, "VM(%d): iGVT-g doesn't support GuC\n",
>   vgpu->id);
>   return 0;
>   }
> -- 

applied, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH v6 3/6] drm/i915/gvt: Frame buffer decoder support for GVT-g

[Zhenyu Wang]

On 2017.05.27 16:38:49 +0800, Xiaoguang Chen wrote:
> decode frambuffer attributes of primary, cursor and sprite plane
> 
> Signed-off-by: Xiaoguang Chen 
> ---
>  drivers/gpu/drm/i915/gvt/Makefile |   3 +-
>  drivers/gpu/drm/i915/gvt/display.c|   2 +-
>  drivers/gpu/drm/i915/gvt/display.h|   2 +
>  drivers/gpu/drm/i915/gvt/fb_decoder.c | 479 
> ++
>  drivers/gpu/drm/i915/gvt/fb_decoder.h | 166 
>  drivers/gpu/drm/i915/gvt/gvt.h|   1 +
>  6 files changed, 651 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.c
>  create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.h
> 
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile 
> b/drivers/gpu/drm/i915/gvt/Makefile
> index b123c20..192ca26 100644
> --- a/drivers/gpu/drm/i915/gvt/Makefile
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -1,7 +1,8 @@
>  GVT_DIR := gvt
>  GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o 
> firmware.o \
>   interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
> - execlist.o scheduler.o sched_policy.o render.o cmd_parser.o
> + execlist.o scheduler.o sched_policy.o render.o cmd_parser.o \
> + fb_decoder.o
>  
>  ccflags-y+= -I$(src) -I$(src)/$(GVT_DIR) -Wall
>  i915-y   += $(addprefix $(GVT_DIR)/, 
> $(GVT_SOURCE))
> diff --git a/drivers/gpu/drm/i915/gvt/display.c 
> b/drivers/gpu/drm/i915/gvt/display.c
> index e0261fc..f5f63c5 100644
> --- a/drivers/gpu/drm/i915/gvt/display.c
> +++ b/drivers/gpu/drm/i915/gvt/display.c
> @@ -67,7 +67,7 @@ static int edp_pipe_is_enabled(struct intel_vgpu *vgpu)
>   return 1;
>  }
>  
> -static int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe)
> +int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe)
>  {
>   struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
>  
> diff --git a/drivers/gpu/drm/i915/gvt/display.h 
> b/drivers/gpu/drm/i915/gvt/display.h
> index d73de22..b46b868 100644
> --- a/drivers/gpu/drm/i915/gvt/display.h
> +++ b/drivers/gpu/drm/i915/gvt/display.h
> @@ -179,4 +179,6 @@ int intel_vgpu_init_display(struct intel_vgpu *vgpu, u64 
> resolution);
>  void intel_vgpu_reset_display(struct intel_vgpu *vgpu);
>  void intel_vgpu_clean_display(struct intel_vgpu *vgpu);
>  
> +int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe);
> +
>  #endif
> diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c 
> b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> new file mode 100644
> index 000..d4404fd
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> @@ -0,0 +1,479 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
> THE
> + * SOFTWARE.
> + *
> + * Authors:
> + *Kevin Tian 
> + *
> + * Contributors:
> + *Bing Niu 
> + *Xu Han 
> + *Ping Gao 
> + *Xiaoguang Chen 
> + *Yang Liu 
> + *
> + */
> +
> +#include 
> +#include "i915_drv.h"
> +#include "gvt.h"
> +
> +/* The below definitions are required by guest. */
> +// [63:0] x:R:G:B 16:16:16:16 little endian
> +#define DRM_FORMAT_XRGB161616_GVT  fourcc_code('X', 'R', '4', '8')
> +// [63:0] x:B:G:R 16:16:16:16 little endian
> +#define DRM_FORMAT_XBGR161616_GVT  fourcc_code('X', 'B', '4', '8')
> +

Should be added to drm_fourcc?

> +#define FORMAT_NUM   16

PRIMARY_FORMAT_NUM

> +struct pixel_format {
> + int drm_format; /* Pixel format in DRM definition */
> + int bpp;/* Bits per pixel, 0 indicates invalid */
> + char *desc; /* The description */
> +};
> +
> +/* non-supported format has bpp default to 0 */
> +static struct pixel_format 

Re: [PATCH] drm/i915/gvt: remove redundant -Wall

[Zhenyu Wang]

On 2017.05.21 00:15:27 -0700, Nick Desaulniers wrote:
> This flag is already set in the top level Makefile of the kernel.
> 
> Also, by having set CONFIG_DRM_I915_GVT, thereby appending -Wall to
> ccflags, you undo all the -Wno-* cflags previously set in the Make
> variable KBUILD_CFLAGS.
> 
> For example:
> 
> cc foo.c -Wall -Wno-format -Wall
> 
> resets -Wformat.
> 
> Signed-off-by: Nick Desaulniers 
> ---
> I verified that -Wall was redundant by compiling with V=1:
> make V=1 -j4 drivers/gpu/drm/i915/i915_gem_request.o
> 
>  drivers/gpu/drm/i915/gvt/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile 
> b/drivers/gpu/drm/i915/gvt/Makefile
> index b123c20e2097..f5486cb94818 100644
> --- a/drivers/gpu/drm/i915/gvt/Makefile
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -3,6 +3,6 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o 
> trace_points.o firmware.o \
>   interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
>   execlist.o scheduler.o sched_policy.o render.o cmd_parser.o
>  
> -ccflags-y+= -I$(src) -I$(src)/$(GVT_DIR) -Wall
> +ccflags-y+= -I$(src) -I$(src)/$(GVT_DIR)
>  i915-y   += $(addprefix $(GVT_DIR)/, 
> $(GVT_SOURCE))
>  obj-$(CONFIG_DRM_I915_GVT_KVMGT) += $(GVT_DIR)/kvmgt.o
> -- 

Pushed, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH v6 2/6] drm/i915/gvt: OpRegion support for GVT-g

[Zhenyu Wang]

On 2017.05.27 16:38:48 +0800, Xiaoguang Chen wrote:
> OpRegion is needed to support display related operation for
> intel vgpu.
> 
> A vfio device region is added to intel vgpu to deliver the
> host OpRegion information to user space so user space can
> construct the OpRegion for vgpu.
> 
> Signed-off-by: Bing Niu 
> Signed-off-by: Xiaoguang Chen 
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c| 97 
> +
>  drivers/gpu/drm/i915/gvt/opregion.c |  8 ++-
>  2 files changed, 104 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 3c6a02b..389f072 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -53,6 +53,8 @@ static const struct intel_gvt_ops *intel_gvt_ops;
>  #define VFIO_PCI_INDEX_TO_OFFSET(index) ((u64)(index) << 
> VFIO_PCI_OFFSET_SHIFT)
>  #define VFIO_PCI_OFFSET_MASK(((u64)(1) << VFIO_PCI_OFFSET_SHIFT) - 1)
>  
> +#define OPREGION_SIGNATURE "IntelGraphicsMem"
> +
>  struct vfio_region;
>  struct intel_vgpu_regops {
>   size_t (*rw)(struct intel_vgpu *vgpu, char *buf,
> @@ -436,6 +438,92 @@ static void kvmgt_protect_table_del(struct 
> kvmgt_guest_info *info,
>   }
>  }
>  
> +static size_t intel_vgpu_reg_rw_opregion(struct intel_vgpu *vgpu, char *buf,
> + size_t count, loff_t *ppos, bool iswrite)
> +{
> + unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) -
> + VFIO_PCI_NUM_REGIONS;
> + void *base = vgpu->vdev.region[i].data;
> + loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> +
> + if (pos >= vgpu->vdev.region[i].size || iswrite) {
> + gvt_vgpu_err("invalid op or offset for Intel vgpu OpRegion\n");
> + return -EINVAL;
> + }
> + count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
> + memcpy(buf, base + pos, count);
> +
> + return count;
> +}
> +
> +static void intel_vgpu_reg_release_opregion(struct intel_vgpu *vgpu,
> + struct vfio_region *region)
> +{
> + memunmap(region->data);
> +}
> +
> +static const struct intel_vgpu_regops intel_vgpu_regops_opregion = {
> + .rw = intel_vgpu_reg_rw_opregion,
> + .release = intel_vgpu_reg_release_opregion,
> +};
> +
> +static int intel_vgpu_register_reg(struct intel_vgpu *vgpu,
> + unsigned int type, unsigned int subtype,
> + const struct intel_vgpu_regops *ops,
> + size_t size, u32 flags, void *data)
> +{
> + struct vfio_region *region;
> +
> + region = krealloc(vgpu->vdev.region,
> + (vgpu->vdev.num_regions + 1) * sizeof(*region),
> + GFP_KERNEL);
> + if (!region)
> + return -ENOMEM;
> +
> + vgpu->vdev.region = region;
> + vgpu->vdev.region[vgpu->vdev.num_regions].type = type;
> + vgpu->vdev.region[vgpu->vdev.num_regions].subtype = subtype;
> + vgpu->vdev.region[vgpu->vdev.num_regions].ops = ops;
> + vgpu->vdev.region[vgpu->vdev.num_regions].size = size;
> + vgpu->vdev.region[vgpu->vdev.num_regions].flags = flags;
> + vgpu->vdev.region[vgpu->vdev.num_regions].data = data;
> + vgpu->vdev.num_regions++;
> +
> + return 0;
> +}
> +
> +static int intel_vgpu_reg_init_opregion(struct intel_vgpu *vgpu)
> +{
> + unsigned int addr;
> + void *base;
> + int ret;
> +
> + addr = vgpu->gvt->opregion.opregion_pa;
> + if (!addr || !(~addr))
> + return -ENODEV;
> +
> + base = memremap(addr, OPREGION_SIZE, MEMREMAP_WB);
> + if (!base)
> + return -ENOMEM;
> +
> + if (memcmp(base, OPREGION_SIGNATURE, 16)) {
> + memunmap(base);
> + return -EINVAL;
> + }
> +
> + ret = intel_vgpu_register_reg(vgpu,
> + PCI_VENDOR_ID_INTEL | VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
> + VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION,
> + _vgpu_regops_opregion, OPREGION_SIZE,
> + VFIO_REGION_INFO_FLAG_READ, base);
> + if (ret) {
> + memunmap(base);
> + return ret;
> + }
> +
> + return ret;
> +}
> +
>  static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
>  {
>   struct intel_vgpu *vgpu = NULL;
> @@ -467,6 +555,15 @@ static int intel_vgpu_create(struct kobject *kobj, 
> struct mdev_device *mdev)
>   vgpu->vdev.mdev = mdev;
>   mdev_set_drvdata(mdev, vgpu);
>  
> + ret = intel_vgpu_reg_init_opregion(vgpu);
> + if (ret) {
> + gvt_vgpu_err("create OpRegion failed\n");
> + goto out;
> + }

Still need to handle error path for created vgpu.

> +
> + gvt_dbg_core("create OpRegion succeeded for mdev:%s\n",
> + dev_name(mdev_dev(mdev)));
> +
>   gvt_dbg_core("intel_vgpu_create succeeded for mdev: %s\n",
>dev_name(mdev_dev(mdev)));
>   ret = 0;
> diff 

Re: [PATCH v6 2/6] drm/i915/gvt: OpRegion support for GVT-g

[Zhenyu Wang]

On 2017.05.31 06:22:28 +, Chen, Xiaoguang wrote:
> >> @@ -467,6 +555,15 @@ static int intel_vgpu_create(struct kobject *kobj,
> >struct mdev_device *mdev)
> >>vgpu->vdev.mdev = mdev;
> >>mdev_set_drvdata(mdev, vgpu);
> >>
> >> +  ret = intel_vgpu_reg_init_opregion(vgpu);
> >> +  if (ret) {
> >> +  gvt_vgpu_err("create OpRegion failed\n");
> >> +  goto out;
> >> +  }
> >
> >Still need to handle error path for created vgpu.
> Just checked the code, if initialize the opregion failed we should first 
> release vfio/mdev releated work(maybe call intel_vgpu_release function)  and 
> then destroy the vgpu. Will update in the next version.
> 

Better to init opregion inside of create vgpu and do proper error
handling there too.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH v8 3/6] drm/i915/gvt: Frame buffer decoder support for GVT-g

[Zhenyu Wang]

On 2017.06.09 14:50:39 +0800, Xiaoguang Chen wrote:
> decode frambuffer attributes of primary, cursor and sprite plane
> 
> Signed-off-by: Xiaoguang Chen 

...

> +/**
> + * intel_vgpu_decode_primary_plane - Decode primary plane
> + * @vgpu: input vgpu
> + * @plane: primary plane to save decoded info
> + * This function is called for decoding plane
> + *
> + * Returns:
> + * 0 on success, non-zero if failed.
> + */
> +int intel_vgpu_decode_primary_plane(struct intel_vgpu *vgpu,
> + struct intel_vgpu_primary_plane_format *plane)
> +{
> + u32 val, fmt;
> + struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> + int pipe;
> +
> + pipe = get_active_pipe(vgpu);
> + if (pipe >= I915_MAX_PIPES)
> + return -ENODEV;
> +
> + val = vgpu_vreg(vgpu, DSPCNTR(pipe));
> + plane->enabled = !!(val & DISPLAY_PLANE_ENABLE);
> + if (!plane->enabled)
> + return -ENODEV;
> +
> + if (IS_SKYLAKE(dev_priv)) {
> + plane->tiled = (val & PLANE_CTL_TILED_MASK) >>
> + _PLANE_CTL_TILED_SHIFT;
> + fmt = skl_format_to_drm(
> + val & PLANE_CTL_FORMAT_MASK,
> + val & PLANE_CTL_ORDER_RGBX,
> + val & PLANE_CTL_ALPHA_MASK,
> + val & PLANE_CTL_YUV422_ORDER_MASK);
> + plane->bpp = skl_pixel_formats[fmt].bpp;
> + plane->drm_format = skl_pixel_formats[fmt].drm_format;
> + } else {
> + plane->tiled = !!(val & DISPPLANE_TILED);
> + fmt = (val & DISPPLANE_PIXFORMAT_MASK) >> _PRI_PLANE_FMT_SHIFT;
> + plane->bpp = bdw_pixel_formats[fmt].bpp;
> + plane->drm_format = bdw_pixel_formats[fmt].drm_format;
> + }
> +
> + if (!skl_pixel_formats[fmt].bpp && !bdw_pixel_formats[fmt].bpp) {
> + gvt_vgpu_err("Non-supported pixel format (0x%x)\n", fmt);
> + return -EINVAL;
> + }

Is this correct? shouldn't be plane->bpp as last time comment?

> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 55e3010..400759f 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -161,6 +161,12 @@ extern "C" {
>  #define DRM_FORMAT_YUV444fourcc_code('Y', 'U', '2', '4') /* 
> non-subsampled Cb (1) and Cr (2) planes */
>  #define DRM_FORMAT_YVU444fourcc_code('Y', 'V', '2', '4') /* 
> non-subsampled Cr (1) and Cb (2) planes */
>  
> +/*
> + * Intel GVT-g plane format definition
> + */
> +#define DRM_FORMAT_XRGB161616_GVT  fourcc_code('X', 'R', '4', '8') /* [63:0] 
> x:R:G:B 16:16:16:16 little endian */
> +#define DRM_FORMAT_XBGR161616_GVT  fourcc_code('X', 'B', '4', '8') /* [63:0] 
> x:B:G:R 16:16:16:16 little endian */
> +
>  

This should be a seperate patch and not need GVT postfix for format definition.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH][drm-next] drm/i915/gvt: ensure -ve return value is handled correctly

[Zhenyu Wang]

On 2017.09.19 16:55:34 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> An earlier fix changed the return type from find_bb_size however the
> integer return is being assigned to a unsigned int so the -ve error
> check will never be detected. Make bb_size an int to fix this.
> 
> Detected by CoverityScan CID#1456886 ("Unsigned compared against 0")
> 
> Fixes: 1e3197d6ad73 ("drm/i915/gvt: Refine error handling for 
> perform_bb_shadow")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/gpu/drm/i915/gvt/cmd_parser.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c 
> b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index 2c0ccbb817dc..f41cbf664b69 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -1628,7 +1628,7 @@ static int perform_bb_shadow(struct parser_exec_state 
> *s)
>   struct intel_shadow_bb_entry *entry_obj;
>   struct intel_vgpu *vgpu = s->vgpu;
>   unsigned long gma = 0;
> - uint32_t bb_size;
> + int bb_size;
>   void *dst = NULL;
>   int ret = 0;
>  

Applied this, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH][drm-next] drm/i915/gvt: ensure -ve return value is handled correctly

[Zhenyu Wang]

On 2017.09.19 19:35:23 -0700, Joe Perches wrote:
> On Wed, 2017-09-20 at 05:46 +0800, Zhenyu Wang wrote:
> > On 2017.09.19 16:55:34 +0100, Colin King wrote:
> > > From: Colin Ian King <colin.k...@canonical.com>
> > > 
> > > An earlier fix changed the return type from find_bb_size however the
> > > integer return is being assigned to a unsigned int so the -ve error
> > > check will never be detected. Make bb_size an int to fix this.
> > > 
> > > Detected by CoverityScan CID#1456886 ("Unsigned compared against 0")
> > > 
> > > Fixes: 1e3197d6ad73 ("drm/i915/gvt: Refine error handling for 
> > > perform_bb_shadow")
> > > Signed-off-by: Colin Ian King <colin.k...@canonical.com>
> > > ---
> > >  drivers/gpu/drm/i915/gvt/cmd_parser.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c 
> > > b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > index 2c0ccbb817dc..f41cbf664b69 100644
> > > --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > @@ -1628,7 +1628,7 @@ static int perform_bb_shadow(struct 
> > > parser_exec_state *s)
> > >   struct intel_shadow_bb_entry *entry_obj;
> > >   struct intel_vgpu *vgpu = s->vgpu;
> > >   unsigned long gma = 0;
> > > - uint32_t bb_size;
> > > + int bb_size;
> > >   void *dst = NULL;
> > >   int ret = 0;
> > >  
> > 
> > Applied this, thanks!
> 
> Is it possible for bb_size to be both >= 2g and valid?

Never be possible in practise and if really that big I think something
is already insane indeed.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH v18 0/6] drm/i915/gvt: Dma-buf support for GVT-g

[Zhenyu Wang]

On 2017.11.15 11:49:00 +0100, Gerd Hoffmann wrote:
> On Wed, Nov 15, 2017 at 05:11:49PM +0800, Tina Zhang wrote:
> > v17->v18:
> > 1) unmap vgpu's opregion when destroying vgpu.
> > 2) update comments for VFIO_DEVICE_GET_GFX_DMABUF. (Alex)
> 
> > This patch set adds the dma-buf support for intel GVT-g.
> > 
> > dma-buf is an uniform mechanism to share DMA buffers across different
> > devices and subsystems. dma-buf for intel GVT-g is mainly used to share
> > the vgpu's framebuffer to userspace to leverage userspace graphics stacks
> > to render the framebuffer to the display monitor.
> > 
> > The main idea is that we create a gem object and set vgpu's framebuffer as
> > its backing storage. Then, export a dma-buf associated with this gem object.
> > With the fd of this dma-buf, userspace can directly handle this buffer.
> > 
> > This patch set can be tried with the following example:
> > git://git.kraxel.org/qemu  branch: work/intel-vgpu
> > 
> > A topic branch with the latest patch set is:
> > https://github.com/intel/gvt-linux.git   branch: topic/dmabuf
> 
> Tested-by: Gerd Hoffmann 
> 

After debugging with Tina on one left race that fixed by
https://lists.freedesktop.org/archives/intel-gvt-dev/2017-November/002505.html

I still need below qemu fix for proper cursor handling, otherwise qemu
just crashed when I click in my terminal program which hides cursor then.

diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index e500ec2cb1..d9a044b080 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -169,8 +169,9 @@ static void vfio_display_dmabuf_update(void *opaque)
 cursor = vfio_display_get_dmabuf(vdev, DRM_PLANE_TYPE_CURSOR);
 if (vdev->cursor != cursor) {
 vdev->cursor = cursor;
-dpy_gl_cursor_dmabuf(vdev->display_con,
- >buf);
+if (cursor)
+dpy_gl_cursor_dmabuf(vdev->display_con,
+ >buf);
 free_bufs = true;
 }
 if (cursor != NULL) {

And with these it seems pretty fine now that I'll queue them up for -next pull.

thanks

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] vfio: Simplify capability helper

[Zhenyu Wang]

On 2017.12.13 12:13:34 +1100, Alexey Kardashevskiy wrote:
> On 13/12/17 06:59, Alex Williamson wrote:
> > The vfio_info_add_capability() helper requires the caller to pass a
> > capability ID, which it then uses to fill in header fields, assuming
> > hard coded versions.  This makes for an awkward and rigid interface.
> > The only thing we want this helper to do is allocate sufficient
> > space in the caps buffer and chain this capability into the list.
> > Reduce it to that simple task.
> > 
> > Signed-off-by: Alex Williamson <alex.william...@redhat.com>
> 
> 
> Makes more sense now, thanks. I'll repost mine on top of this.
> 
> 
> Reviewed-by: Alexey Kardashevskiy <a...@ozlabs.ru>
> 
>

Looks good for KVMGT part.

Acked-by: Zhenyu Wang <zhen...@linux.intel.com>

> Below one observation, unrelated to this patch.
> 
> > ---
> >  drivers/gpu/drm/i915/gvt/kvmgt.c |   15 +++
> >  drivers/vfio/pci/vfio_pci.c  |   14 ++
> >  drivers/vfio/vfio.c  |   52 
> > +++---
> >  include/linux/vfio.h |3 +-
> >  4 files changed, 24 insertions(+), 60 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> > b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 96060920a6fe..0a7d084da1a2 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1012,6 +1012,8 @@ static long intel_vgpu_ioctl(struct mdev_device 
> > *mdev, unsigned int cmd,
> > if (!sparse)
> > return -ENOMEM;
> >  
> > +   sparse->header.id = VFIO_REGION_INFO_CAP_SPARSE_MMAP;
> > +   sparse->header.version = 1;
> > sparse->nr_areas = nr_areas;
> > cap_type_id = VFIO_REGION_INFO_CAP_SPARSE_MMAP;
> 
> 
> @cap_type_id is initialized in just one of many cases of switch
> (info.index) and after the entire switch, there is switch (cap_type_id). I
> wonder why compiler missed "potentially uninitialized variable here,
> although there is no bug - @cap_type_id is in sync with @spapse. It would
> make it cleaner imho just to have vfio_info_add_capability() next to the
> header initialization.
> 

yeah, we could clean that up, thanks for pointing out.

> 
> 
> > sparse->areas[0].offset =
> > @@ -1033,7 +1035,9 @@ static long intel_vgpu_ioctl(struct mdev_device 
> > *mdev, unsigned int cmd,
> > break;
> > default:
> > {
> > -   struct vfio_region_info_cap_type cap_type;
> > +   struct vfio_region_info_cap_type cap_type = {
> > +   .header.id = VFIO_REGION_INFO_CAP_TYPE,
> > +   .header.version = 1 };
> >  
> > if (info.index >= VFIO_PCI_NUM_REGIONS +
> > vgpu->vdev.num_regions)
> > @@ -1050,8 +1054,8 @@ static long intel_vgpu_ioctl(struct mdev_device 
> > *mdev, unsigned int cmd,
> > cap_type.subtype = vgpu->vdev.region[i].subtype;
> >  
> > ret = vfio_info_add_capability(,
> > -   VFIO_REGION_INFO_CAP_TYPE,
> > -   _type);
> > +   _type.header,
> > +   sizeof(cap_type));
> > if (ret)
> > return ret;
> > }
> > @@ -1061,8 +1065,9 @@ static long intel_vgpu_ioctl(struct mdev_device 
> > *mdev, unsigned int cmd,
> > switch (cap_type_id) {
> > case VFIO_REGION_INFO_CAP_SPARSE_MMAP:
> > ret = vfio_info_add_capability(,
> > -   VFIO_REGION_INFO_CAP_SPARSE_MMAP,
> > -   sparse);
> > +   >header, sizeof(*sparse) +
> > +   (sparse->nr_areas *
> > +   sizeof(*sparse->areas)));
> > kfree(sparse);
> > if (ret)
> > return ret;
> > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> > index f041b1a6cf66..a

Re: [PATCH v18 5/6] vfio: ABI for mdev display dma-buf operation

[Zhenyu Wang]

On 2017.11.15 11:48:42 -0700, Alex Williamson wrote:
> On Wed, 15 Nov 2017 17:11:54 +0800
> Tina Zhang  wrote:
> 
> > Add VFIO_DEVICE_QUERY_GFX_PLANE ioctl command to let user query and get
> > a plane and its information. So far, two types of buffers are supported:
> > buffers based on dma-buf and buffers based on region.
> > 
> > This ioctl can be invoked with:
> > 1) Either DMABUF or REGION flag. Vendor driver returns a plane_info
> > successfully only when the specific kind of buffer is supported.
> > 2) Flag PROBE. And at the same time either DMABUF or REGION must be set,
> > so that vendor driver returns success only when the specific kind of
> > buffer is supported.
> > 
> > Add VFIO_DEVICE_GET_GFX_DMABUF ioctl command to let user get a specific
> > dma-buf fd of an exposed MDEV buffer provided by dmabuf_id which was
> > returned in VFIO_DEVICE_QUERY_GFX_PLANE ioctl command.
> > 
> > The life cycle of an exposed MDEV buffer is handled by userspace and
> > tracked by kernel space. The returned dmabuf_id in struct vfio_device_
> > query_gfx_plane can be a new id of a new exposed buffer or an old id of
> > a re-exported buffer. Host user can check the value of dmabuf_id to see
> > if it needs to create new resources according to the new exposed buffer
> > or just re-use the existing resource related to the old buffer.
> > 
> > v18:
> > - update comments for VFIO_DEVICE_GET_GFX_DMABUF. (Alex)
> > 
> > v17:
> > - modify VFIO_DEVICE_GET_GFX_DMABUF interface. (Alex)
> > 
> > v16:
> > - add x_hot and y_hot fields. (Gerd)
> > - add comments for VFIO_DEVICE_GET_GFX_DMABUF. (Alex)
> > - rebase to 4.14.0-rc6.
> > 
> > v15:
> > - add a ioctl to get a dmabuf for a given dmabuf id. (Gerd)
> > 
> > v14:
> > - add PROBE, DMABUF and REGION flags. (Alex)
> > 
> > v12:
> > - add drm_format_mod back. (Gerd and Zhenyu)
> > - add region_index. (Gerd)
> > 
> > v11:
> > - rename plane_type to drm_plane_type. (Gerd)
> > - move fields of vfio_device_query_gfx_plane to vfio_device_gfx_plane_info.
> >   (Gerd)
> > - remove drm_format_mod, start fields. (Daniel)
> > - remove plane_id.
> > 
> > v10:
> > - refine the ABI API VFIO_DEVICE_QUERY_GFX_PLANE. (Alex) (Gerd)
> > 
> > v3:
> > - add a field gvt_plane_info in the drm_i915_gem_obj structure to save
> >   the decoded plane information to avoid look up while need the plane
> >   info. (Gerd)
> > 
> > Signed-off-by: Tina Zhang 
> > Cc: Gerd Hoffmann 
> > Cc: Alex Williamson 
> > Cc: Daniel Vetter 
> > ---
> >  include/uapi/linux/vfio.h | 62 
> > +++
> >  1 file changed, 62 insertions(+)
> > 
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index ae46105..5c1cca2 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -502,6 +502,68 @@ struct vfio_pci_hot_reset {
> >  
> >  #define VFIO_DEVICE_PCI_HOT_RESET  _IO(VFIO_TYPE, VFIO_BASE + 13)
> >  
> > +/**
> > + * VFIO_DEVICE_QUERY_GFX_PLANE - _IOW(VFIO_TYPE, VFIO_BASE + 14,
> > + *struct vfio_device_query_gfx_plane)
> > + *
> > + * Set the drm_plane_type and flags, then retrieve the gfx plane info.
> > + *
> > + * flags supported:
> > + * - VFIO_GFX_PLANE_TYPE_PROBE and VFIO_GFX_PLANE_TYPE_DMABUF are set
> > + *   to ask if the mdev supports dma-buf. 0 on support, -EINVAL on no
> > + *   support for dma-buf.
> > + * - VFIO_GFX_PLANE_TYPE_PROBE and VFIO_GFX_PLANE_TYPE_REGION are set
> > + *   to ask if the mdev supports region. 0 on support, -EINVAL on no
> > + *   support for region.
> > + * - VFIO_GFX_PLANE_TYPE_DMABUF or VFIO_GFX_PLANE_TYPE_REGION is set
> > + *   with each call to query the plane info.
> > + * - Others are invalid and return -EINVAL.
> > + *
> > + * Note:
> > + * 1. Plane could be disabled by guest. In that case, success will be
> > + *returned with zero-initialized drm_format, size, width and height
> > + *fields.
> > + * 2. x_hot/y_hot is set to 0x if no hotspot information available
> > + *
> > + * Return: 0 on success, -errno on other failure.
> > + */
> > +struct vfio_device_gfx_plane_info {
> > +   __u32 argsz;
> > +   __u32 flags;
> > +#define VFIO_GFX_PLANE_TYPE_PROBE (1 << 0)
> > +#define VFIO_GFX_PLANE_TYPE_DMABUF (1 << 1)
> > +#define VFIO_GFX_PLANE_TYPE_REGION (1 << 2)
> > +   /* in */
> > +   __u32 drm_plane_type;   /* type of plane: DRM_PLANE_TYPE_* */
> > +   /* out */
> > +   __u32 drm_format;   /* drm format of plane */
> > +   __u64 drm_format_mod;   /* tiled mode */
> > +   __u32 width;/* width of plane */
> > +   __u32 height;   /* height of plane */
> > +   __u32 stride;   /* stride of plane */
> > +   __u32 size; /* size of plane in bytes, align on page*/
> > +   __u32 x_pos;/* horizontal position of cursor plane */
> > +   __u32 y_pos;/* vertical position of cursor plane*/
> > +   __u32 x_hot;/* 

Re: linux-next: Signed-off-by missing for commits in the drm tree

[Zhenyu Wang]

On 2017.12.04 10:57:35 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> There is a series of commits
> 
>   54cff6479fd8 - c1802534e5a6
> (not all in that range)
> 
> that are missing a Signed-off-by from their committer.
> 

They were originally committed by Zhi himself, but I had to rebase
onto intel -next before they're sent for pull and missed my signed-off..
Really sorry for that! I think maybe we could still be able to fix that
before next kernel merge.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915/gvt/fb_decoder: Fix out-of-bounds read

[Zhenyu Wang]

On 2017.12.09 00:37:59 -0600, Gustavo A. R. Silva wrote:
> In case function skl_format_to_drm returns -EINVAL, fmt turns into a huge
> number as fmt is of type u32, hence there is an out-of-bounds read when
> using fmt as an index for array skl_pixel_formats at line 225:
> plane->bpp = skl_pixel_formats[fmt].bpp;
> 
> Fix this by comparing the value returned by function skl_format_to_drm
> against the size of array skl_pixel_formats, so in case it is greater than
> or equal to the number of items contained in skl_pixel_formats, print an
> error message and return -EINVAL.
> 
> Addresses-Coverity-ID: 1462495
> Addresses-Coverity-ID: 1462502 ("Out-of-bounds read")
> Fixes: 9f31d1063b43 ("drm/i915/gvt: Add framebuffer decoder support")
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/gpu/drm/i915/gvt/fb_decoder.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c 
> b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> index 72f4217..aed578b 100644
> --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> @@ -222,6 +222,12 @@ int intel_vgpu_decode_primary_plane(struct intel_vgpu 
> *vgpu,
>   val & PLANE_CTL_ORDER_RGBX,
>   val & PLANE_CTL_ALPHA_MASK,
>   val & PLANE_CTL_YUV422_ORDER_MASK);
> +
> + if (fmt >= ARRAY_SIZE(skl_pixel_formats)) {
> + gvt_vgpu_err("Out-of-bounds pixel format index\n");
> + return -EINVAL;
> + }
> +
>   plane->bpp = skl_pixel_formats[fmt].bpp;
>   plane->drm_format = skl_pixel_formats[fmt].drm_format;
>   } else {
> -- 

Applied this, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH v4 1/2] vfio/mdev: Check globally for duplicate devices

[Zhenyu Wang]

On 2018.05.22 09:53:37 -0600, Alex Williamson wrote:
> [Cc +GVT-g maintainers/lists]
> 
> On Tue, 22 May 2018 10:13:46 +0200
> Cornelia Huck <coh...@redhat.com> wrote:
> 
> > On Fri, 18 May 2018 13:10:25 -0600
> > Alex Williamson <alex.william...@redhat.com> wrote:
> > 
> > > When we create an mdev device, we check for duplicates against the
> > > parent device and return -EEXIST if found, but the mdev device
> > > namespace is global since we'll link all devices from the bus.  We do
> > > catch this later in sysfs_do_create_link_sd() to return -EEXIST, but
> > > with it comes a kernel warning and stack trace for trying to create
> > > duplicate sysfs links, which makes it an undesirable response.
> > > 
> > > Therefore we should really be looking for duplicates across all mdev
> > > parent devices, or as implemented here, against our mdev device list.
> > > Using mdev_list to prevent duplicates means that we can remove
> > > mdev_parent.lock, but in order not to serialize mdev device creation
> > > and removal globally, we add mdev_device.active which allows UUIDs to
> > > be reserved such that we can drop the mdev_list_lock before the mdev
> > > device is fully in place.
> > > 
> > > Two behavioral notes; first, mdev_parent.lock had the side-effect of
> > > serializing mdev create and remove ops per parent device.  This was
> > > an implementation detail, not an intentional guarantee provided to
> > > the mdev vendor drivers.  Vendor drivers can trivially provide this
> > > serialization internally if necessary.  Second, review comments note
> > > the new -EAGAIN behavior when the device, and in particular the remove
> > > attribute, becomes visible in sysfs.  If a remove is triggered prior
> > > to completion of mdev_device_create() the user will see a -EAGAIN
> > > error.  While the errno is different, receiving an error during this
> > > period is not, the previous implementation returned -ENODEV for the
> > > same condition.  Furthermore, the consistency to the user is improved
> > > in the case where mdev_device_remove_ops() returns error.  Previously
> > > concurrent calls to mdev_device_remove() could see the device
> > > disappear with -ENODEV and return in the case of error.  Now a user
> > > would see -EAGAIN while the device is in this transitory state.
> > > 
> > > Signed-off-by: Alex Williamson <alex.william...@redhat.com>
> > > ---
> > >  Documentation/vfio-mediated-device.txt |5 ++
> > >  drivers/vfio/mdev/mdev_core.c  |  102 
> > > +++-
> > >  drivers/vfio/mdev/mdev_private.h   |2 -
> > >  3 files changed, 42 insertions(+), 67 deletions(-)  
> > 
> > Reviewed-by: Cornelia Huck <coh...@redhat.com>
> > 
> > I think it is better to deal with any possible vendor driver
> > implications on top of this (I still believe that vfio-ccw is fine).
> 
> Thanks Cornelia.  So if vfio-ccw is fine, presumably NVIDIA is fine,
> then this leaves GVT-g to see if there's any fallout.  Zhenyu & Zhi,
> I've linked the series under discussion here below[1].  The question to
> you is the first of the two behavioral notes listed above, does GVT-g
> have any dependency on the mdev core providing serialization per mdev
> parent device for the create and remove callbacks within the
> mdev_parent_ops?  This was never an intended feature of the
> implementation and as noted it should be trivial for for an mdev vendor
> driver to provide equivalent course grained serialization if
> necessary.  Of course it would be better to implement that sooner
> rather than later if required.
> 
> I see that __intel_gvt_create_vgpu() makes use of gvt->lock, which
> would seem to already provide this level of per-parent locking. The
> remove path makes use of this same lock, so I think we're ok, but
> looking for an explicit ack so there are no surprises.  I'd like
> to queue this series for v4.18.  Thanks,
> 

yeah, we don't expect mdev core for parent serialization for create and
remove of mdev device. Series look good to me.

Acked-by: Zhenyu Wang <zhen...@linux.intel.com>


> Alex
> 
> [1] https://lkml.org/lkml/2018/5/18/1035

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH][drm-next] drm/i915/gvt: fix spelling mistake: "destoried" -> "destroyed"

[Zhenyu Wang]

On 2018.03.12 12:43:58 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Trivial fix to spelling mistake in gvt_err error message text.
> 
> Signed-off-by: Colin Ian King 
> ---

Thanks Colin, will pick up.

>  drivers/gpu/drm/i915/gvt/gtt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 0a100a288e6d..8eb76becd676 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -2046,7 +2046,7 @@ static void intel_vgpu_destroy_all_ppgtt_mm(struct 
> intel_vgpu *vgpu)
>   }
>  
>   if (GEM_WARN_ON(!list_empty(>gtt.ppgtt_mm_list_head)))
> - gvt_err("vgpu ppgtt mm is not fully destoried\n");
> + gvt_err("vgpu ppgtt mm is not fully destroyed\n");
>  
>   if (GEM_WARN_ON(!radix_tree_empty(>gtt.spt_tree))) {
>   gvt_err("Why we still has spt not freed?\n");
> -- 
> 2.15.1
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915/gvt/scheduler: Remove unnecessary NULL checks in sr_oa_regs

[Zhenyu Wang]

On 2018.03.22 21:31:33 +, Chris Wilson wrote:
> Quoting Gustavo A. R. Silva (2018-03-22 18:21:54)
> > The checks are misleading and not required [1].
> > 
> > [1] https://lkml.org/lkml/2018/3/19/1792
> > 
> > Addresses-Coverity-ID: 1466017
> > Cc: Chris Wilson 
> > Signed-off-by: Gustavo A. R. Silva 
> Reviewed-by: Chris Wilson 
> 

Looks good to me, I will pick this up, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH 1/2] vfio: add edid api for display (vgpu) devices.

[Zhenyu Wang]

On 2018.09.14 14:25:52 +0200, Gerd Hoffmann wrote:
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 1aa7b82e81..901f279033 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -200,12 +200,25 @@ struct vfio_device_info {
>  #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2)  /* vfio-platform device */
>  #define VFIO_DEVICE_FLAGS_AMBA  (1 << 3) /* vfio-amba device */
>  #define VFIO_DEVICE_FLAGS_CCW(1 << 4)/* vfio-ccw device */
> +#define VFIO_DEVICE_FLAGS_CAPS   (1 << 5)/* cap_offset present */
>   __u32   num_regions;/* Max region index + 1 */
>   __u32   num_irqs;   /* Max IRQ index + 1 */
> + __u32   cap_offset; /* Offset within info struct of first cap */
>  };
>  #define VFIO_DEVICE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 7)
>  
>  /*
> + * FIXME: docs ...
> + */
> +#define VFIO_DEVICE_INFO_CAP_EDID1
> +
> +struct vfio_device_info_edid_cap {
> + struct vfio_info_cap_header header;
> + __u32   max_x; /* Max display height (zero == no limit) */
> + __u32   max_y; /* Max display height (zero == no limit) */
> +};

As current virtual display for Intel vGPU is still emulating against real HW
pipeline with same limitations, asked display developers that whether or not
specific mode can work might still depend on current or future HW behavior.
So could we add some hints on what kind of edid mode vfio device can operate?
Some may support arbitrary modes, but some may only support standard modes.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH 1/2] vfio: add edid api for display (vgpu) devices.

[Zhenyu Wang]

On 2018.09.17 10:50:33 +0200, Gerd Hoffmann wrote:
> > > +#define VFIO_DEVICE_INFO_CAP_EDID1
> > > +
> > > +struct vfio_device_info_edid_cap {
> > > + struct vfio_info_cap_header header;
> > > + __u32   max_x; /* Max display height (zero == no limit) */
> > > + __u32   max_y; /* Max display height (zero == no limit) */
> > > +};
> > 
> > As current virtual display for Intel vGPU is still emulating against real HW
> > pipeline with same limitations, asked display developers that whether or not
> > specific mode can work might still depend on current or future HW behavior.
> > So could we add some hints on what kind of edid mode vfio device can 
> > operate?
> > Some may support arbitrary modes, but some may only support standard modes.
> 
> What kind of restrictions do we have here?  Really to a fixed list of
> standard modes?

Restriction is still with HW differences, e.g for skl/kbl with ddi wrpll
within min/max clock range which may generate any required frequency, but
I've been told for bxt there're some gaps in clock range that could be
generated.

> 
> Some testing (kaby lake) indicates y axis has no restrictions and x axis
> gets rounded up to the next multiple of 8 pixels (32 bytes), maybe to
> align scanlines with cachelines?

That should be plane stride requirement I think.

> 
> Oh, and btw:  Seems the resolution restriction (to 1024x768 for the
> smallest vgpu type) seems to not be enforced.  Intentional?
> 

hmm, what do you mean here? Not enforce to have only one mode for vgpu type?
Or can't change to other mode?

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

[AGPGART] intel_agp: fix ids for 945GME, 965GME

[Zhenyu Wang]

Subject: [PATCH] [AGPGART] intel_agp: fix ids for 945GME and 965GME

These chipsets actually have new host bridge ids. This should fix the gart
init problem on them. Please apply, thanks.

Signed-off-by: Wang Zhenyu <[EMAIL PROTECTED]>
---
 drivers/char/agp/intel-agp.c |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index a124060..86a7e11 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -20,7 +20,9 @@
 #define PCI_DEVICE_ID_INTEL_82965G_IG   0x29A2
 #define PCI_DEVICE_ID_INTEL_82965GM_HB  0x2A00
 #define PCI_DEVICE_ID_INTEL_82965GM_IG  0x2A02
+#define PCI_DEVICE_ID_INTEL_82965GME_HB 0x2A10
 #define PCI_DEVICE_ID_INTEL_82965GME_IG 0x2A12
+#define PCI_DEVICE_ID_INTEL_82945GME_HB 0x27AC
 #define PCI_DEVICE_ID_INTEL_82945GME_IG 0x27AE
 #define PCI_DEVICE_ID_INTEL_G33_HB  0x29C0
 #define PCI_DEVICE_ID_INTEL_G33_IG  0x29C2
@@ -1848,9 +1850,9 @@ static const struct intel_driver_description {
NULL, _915_driver },
{ PCI_DEVICE_ID_INTEL_82945G_HB, PCI_DEVICE_ID_INTEL_82945G_IG, 0, 
"945G",
NULL, _915_driver },
-   { PCI_DEVICE_ID_INTEL_82945GM_HB, PCI_DEVICE_ID_INTEL_82945GM_IG, 1, 
"945GM",
+   { PCI_DEVICE_ID_INTEL_82945GM_HB, PCI_DEVICE_ID_INTEL_82945GM_IG, 0, 
"945GM",
NULL, _915_driver },
-   { PCI_DEVICE_ID_INTEL_82945GM_HB, PCI_DEVICE_ID_INTEL_82945GME_IG, 0, 
"945GME",
+   { PCI_DEVICE_ID_INTEL_82945GME_HB, PCI_DEVICE_ID_INTEL_82945GME_IG, 0, 
"945GME",
NULL, _915_driver },
{ PCI_DEVICE_ID_INTEL_82946GZ_HB, PCI_DEVICE_ID_INTEL_82946GZ_IG, 0, 
"946GZ",
NULL, _i965_driver },
@@ -1860,9 +1862,9 @@ static const struct intel_driver_description {
NULL, _i965_driver },
{ PCI_DEVICE_ID_INTEL_82965G_HB, PCI_DEVICE_ID_INTEL_82965G_IG, 0, 
"965G",
NULL, _i965_driver },
-   { PCI_DEVICE_ID_INTEL_82965GM_HB, PCI_DEVICE_ID_INTEL_82965GM_IG, 1, 
"965GM",
+   { PCI_DEVICE_ID_INTEL_82965GM_HB, PCI_DEVICE_ID_INTEL_82965GM_IG, 0, 
"965GM",
NULL, _i965_driver },
-   { PCI_DEVICE_ID_INTEL_82965GM_HB, PCI_DEVICE_ID_INTEL_82965GME_IG, 0, 
"965GME/GLE",
+   { PCI_DEVICE_ID_INTEL_82965GME_HB, PCI_DEVICE_ID_INTEL_82965GME_IG, 0, 
"965GME/GLE",
NULL, _i965_driver },
{ PCI_DEVICE_ID_INTEL_7505_0, 0, 0, "E7505", _7505_driver, NULL },
{ PCI_DEVICE_ID_INTEL_7205_0, 0, 0, "E7205", _7505_driver, NULL },
-- 
1.5.2.3
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [AGPGART] intel_agp: fix stolen mem range on G33

[Zhenyu Wang]

Subject: [PATCH] [AGPGART] intel_agp: fix stolen mem range on G33

G33 GTT stolen memory is below graphics data
stolen memory and be seperate, so don't subtract
it in stolen mem counting.

Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
---
 drivers/char/agp/intel-agp.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index 2c9ca2c..20d50d0 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -506,6 +506,11 @@ static void intel_i830_init_gtt_entries(void)
break;
}
} else {
+   /* G33's GTT stolen memory is seperate from gfx data
+* stolen memory.
+*/
+   if (IS_G33)
+   size = 0;
switch (gmch_ctrl & I830_GMCH_GMS_MASK) {
case I855_GMCH_GMS_STOLEN_1M:
gtt_entries = MB(1) - KB(size);
-- 
1.5.2.3
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [AGPGART] intel_agp: fix GTT map size on G33

[Zhenyu Wang]

Subject: [PATCH] [AGPGART] intel_agp: fix GTT map size on G33

G33 has 1MB GTT table range. Fix GTT mapping
in case like 512MB aperture size.

Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
---
 drivers/char/agp/intel-agp.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index 20d50d0..6a5c555 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -919,6 +919,7 @@ static int intel_i915_create_gatt_table(struct 
agp_bridge_data *bridge)
struct aper_size_info_fixed *size;
int num_entries;
u32 temp, temp2;
+   int gtt_map_size = 256 * 1024;
 
size = agp_bridge->current_size;
page_order = size->page_order;
@@ -928,7 +929,9 @@ static int intel_i915_create_gatt_table(struct 
agp_bridge_data *bridge)
pci_read_config_dword(intel_private.pcidev, I915_MMADDR, );
pci_read_config_dword(intel_private.pcidev, I915_PTEADDR,);
 
-   intel_private.gtt = ioremap(temp2, 256 * 1024);
+   if (IS_G33)
+   gtt_map_size = 1024 * 1024; /* 1M on G33 */
+   intel_private.gtt = ioremap(temp2, gtt_map_size);
if (!intel_private.gtt)
return -ENOMEM;
 
-- 
1.5.2.3
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [AGPGART] intel_agp: fix stolen mem range on G33

[Zhenyu Wang]

On 2007.09.05 08:19:37 +, Randy Dunlap wrote:
> *** Remember to use Documentation/SubmitChecklist when testing your code ***

Thanks Randy. Here's updated patch with typo and tab style
fixed. I misused x style.

Subject: [PATCH] [AGPGART] intel_agp: fix stolen mem range on G33

G33 GTT stolen memory is below graphics data
stolen memory and be seperate, so don't subtract
it in stolen mem counting.

Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
---
 drivers/char/agp/intel-agp.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index 2c9ca2c..581f922 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -506,6 +506,11 @@ static void intel_i830_init_gtt_entries(void)
break;
}
} else {
+   /* G33's GTT stolen memory is separate from gfx data
+* stolen memory.
+*/
+   if (IS_G33)
+   size = 0;
switch (gmch_ctrl & I830_GMCH_GMS_MASK) {
case I855_GMCH_GMS_STOLEN_1M:
gtt_entries = MB(1) - KB(size);
-- 
1.5.2.3
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: X-freeze after clflush changes [Was: 2.6.23-rc6-mm1]

[Zhenyu Wang]

On 2007.09.20 17:33:45 +, Dave Airlie wrote:
> > Maybe you are rather interested in these dmesg lines:
> > Linux agpgart interface v0.102
> > agpgart: suspend/resume problematic: resume with 3D/DRI active may lockup 
> > X.Org
> > on some chipset/BIOS combos (see DEBUG_AGP_PM in intel-agp.c)
> > agpgart: Detected an Intel G33 Chipset.
> > agpgart: Detected 8192K stolen memory.
> > agpgart: AGP aperture is 256M @ 0xd000
> > [drm] Initialized drm 1.1.0 20060810
> > ACPI: PCI Interrupt :00:02.0[A] -> GSI 16 (level, low) -> IRQ 16
> > [drm] Initialized i915 1.6.0 20060119 on minor 0
> > ...
> > set status page addr 0x00033000
> > agpgart: pg_start == 0x05ff,intel_private.gtt_entries == 0x0800
> > agpgart: Trying to insert into local/stolen memory
> >
> > So the problem is, that X passes too low start.
> >
> > The X log:
> > http://www.fi.muni.cz/~xslaby/sklad/Xorg.0.log.old

Could you try current xf86-video-intel driver? just do
git clone git://anongit.freedesktop.org/git/xorg/driver/xf86-video-intel

My G33 was just verified broken today... so I'll try to reproduce it on
another one tomorrow.

> 
> I've cc'd Zhenyu who might be able to shed some light on this? can you
> try 2.6.23-rc7 as maybe the G33 support still needs some work.. or
> maybe I'm missing a patch in the drm..
> 

yep, should try 2.6.23-rc7 first, and it seems not drm relate.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: X-freeze after clflush changes [Was: 2.6.23-rc6-mm1]

[Zhenyu Wang]

On 2007.09.21 00:10:26 +, Jiri Slaby wrote:
> > Could you try current xf86-video-intel driver? just do
> > git clone git://anongit.freedesktop.org/git/xorg/driver/xf86-video-intel
> 
> It works! 

yep, I also pushed a fix for G33 in xf86-video-intel when fixing the intel agp.
So for G33 user, you should upgrade both to be able to work correctly.

> 3d problem, but it has maybe nothing to do with kernel:
> $ glxinfo
> name of display: :0.0
> Unrecognized deviceID 29c2
> X Error of failed request:  GLXBadContext
> ...

It looks you have an old version of mesa, that i915 dri driver doesn't know
your chipset. Try mesa-7.X.

I have also seen X exit broken with 2.6.23-rc6-mm1, will follow this thread
and try Dave's patch.

Thanks for testing!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [Intel-gfx] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type

[Zhenyu Wang]

On 2014.06.19 17:53:51 +0800, Tiejun Chen wrote:
> Originally the reason to probe ISA bridge instead of Dev31:Fun0
> is to make graphics device passthrough work easy for VMM, that
> only need to expose ISA bridge to let driver know the real
> hardware underneath. This is a requirement from virtualization
> team. Especially in that virtualized environments, XEN, there
> is irrelevant ISA bridge in the system with that legacy qemu
> version specific to xen, qemu-xen-traditional. So to work
> reliably, we should scan through all the ISA bridge devices
> and check for the first match, instead of only checking the
> first one.
> 
> But actually, qemu-xen-traditional, is always enumerated with
> Dev31:Fun0, 00:1f.0 as follows:
> 
> hw/pt-graphics.c:
> 
> intel_pch_init()
> |
> + pci_isa_bridge_init(bus, PCI_DEVFN(0x1f, 0), ...);
> 
> so this mean that isa bridge is still represented with Dev31:Func0
> like the native OS. Furthermore, currently we're pushing VGA
> passthrough support into qemu upstream, and with some discussion,
> we wouldn't set the bridge class type and just expose this devfn.
> 
> So we just go back to check devfn to make life normal.
> 
> Signed-off-by: Tiejun Chen 

This was added historically when supporting graphics device passthrough.
Looks qemu upstream can't accept multiple ISA bridge and our PCH is always
on device 31: func0 as far as I know. Looks good to me.

Reviewed-by: Zhenyu Wang  

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: Digital signature

Re: [PATCH][drm-next] drm/i915/gvt: fix spelling mistake: "destoried" -> "destroyed"

[Zhenyu Wang]

On 2018.03.12 12:43:58 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Trivial fix to spelling mistake in gvt_err error message text.
> 
> Signed-off-by: Colin Ian King 
> ---

Thanks Colin, will pick up.

>  drivers/gpu/drm/i915/gvt/gtt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 0a100a288e6d..8eb76becd676 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -2046,7 +2046,7 @@ static void intel_vgpu_destroy_all_ppgtt_mm(struct 
> intel_vgpu *vgpu)
>   }
>  
>   if (GEM_WARN_ON(!list_empty(>gtt.ppgtt_mm_list_head)))
> - gvt_err("vgpu ppgtt mm is not fully destoried\n");
> + gvt_err("vgpu ppgtt mm is not fully destroyed\n");
>  
>   if (GEM_WARN_ON(!radix_tree_empty(>gtt.spt_tree))) {
>   gvt_err("Why we still has spt not freed?\n");
> -- 
> 2.15.1
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915/gvt/scheduler: Remove unnecessary NULL checks in sr_oa_regs

[Zhenyu Wang]

On 2018.03.22 21:31:33 +, Chris Wilson wrote:
> Quoting Gustavo A. R. Silva (2018-03-22 18:21:54)
> > The checks are misleading and not required [1].
> > 
> > [1] https://lkml.org/lkml/2018/3/19/1792
> > 
> > Addresses-Coverity-ID: 1466017
> > Cc: Chris Wilson 
> > Signed-off-by: Gustavo A. R. Silva 
> Reviewed-by: Chris Wilson 
> 

Looks good to me, I will pick this up, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH][drm-next] drm/i915/gvt: ensure -ve return value is handled correctly

[Zhenyu Wang]

On 2017.09.19 16:55:34 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> An earlier fix changed the return type from find_bb_size however the
> integer return is being assigned to a unsigned int so the -ve error
> check will never be detected. Make bb_size an int to fix this.
> 
> Detected by CoverityScan CID#1456886 ("Unsigned compared against 0")
> 
> Fixes: 1e3197d6ad73 ("drm/i915/gvt: Refine error handling for 
> perform_bb_shadow")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/gpu/drm/i915/gvt/cmd_parser.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c 
> b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index 2c0ccbb817dc..f41cbf664b69 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -1628,7 +1628,7 @@ static int perform_bb_shadow(struct parser_exec_state 
> *s)
>   struct intel_shadow_bb_entry *entry_obj;
>   struct intel_vgpu *vgpu = s->vgpu;
>   unsigned long gma = 0;
> - uint32_t bb_size;
> + int bb_size;
>   void *dst = NULL;
>   int ret = 0;
>  

Applied this, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH][drm-next] drm/i915/gvt: ensure -ve return value is handled correctly

[Zhenyu Wang]

On 2017.09.19 19:35:23 -0700, Joe Perches wrote:
> On Wed, 2017-09-20 at 05:46 +0800, Zhenyu Wang wrote:
> > On 2017.09.19 16:55:34 +0100, Colin King wrote:
> > > From: Colin Ian King 
> > > 
> > > An earlier fix changed the return type from find_bb_size however the
> > > integer return is being assigned to a unsigned int so the -ve error
> > > check will never be detected. Make bb_size an int to fix this.
> > > 
> > > Detected by CoverityScan CID#1456886 ("Unsigned compared against 0")
> > > 
> > > Fixes: 1e3197d6ad73 ("drm/i915/gvt: Refine error handling for 
> > > perform_bb_shadow")
> > > Signed-off-by: Colin Ian King 
> > > ---
> > >  drivers/gpu/drm/i915/gvt/cmd_parser.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c 
> > > b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > index 2c0ccbb817dc..f41cbf664b69 100644
> > > --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > @@ -1628,7 +1628,7 @@ static int perform_bb_shadow(struct 
> > > parser_exec_state *s)
> > >   struct intel_shadow_bb_entry *entry_obj;
> > >   struct intel_vgpu *vgpu = s->vgpu;
> > >   unsigned long gma = 0;
> > > - uint32_t bb_size;
> > > + int bb_size;
> > >   void *dst = NULL;
> > >   int ret = 0;
> > >  
> > 
> > Applied this, thanks!
> 
> Is it possible for bb_size to be both >= 2g and valid?

Never be possible in practise and if really that big I think something
is already insane indeed.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH v10] vfio: ABI for mdev display dma-buf operation

[Zhenyu Wang]

On 2017.07.11 11:12:36 +0200, Daniel Vetter wrote:
> On Tue, Jul 11, 2017 at 08:14:08AM +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > > +struct vfio_device_query_gfx_plane {
> > > > +   __u32 argsz;
> > > > +   __u32 flags;
> > > > +   struct vfio_device_gfx_plane_info plane_info;
> > > > +   __u32 plane_type;
> > > > +   __s32 fd; /* dma-buf fd */
> > > > +   __u32 plane_id;
> > > > +};
> > > > +
> > > 
> > > It would be better to have comment here about what are expected
> > > values
> > > for plane_type and plane_id.
> > 
> > plane_type is DRM_PLANE_TYPE_*.
> > 
> > yes, a comment saying so would be good, same for drm_format which is
> > DRM_FORMAT_*.  While looking at these two: renaming plane_type to
> > drm_plane_type (for consistency) is probably a good idea too.

For drm universal plane, this is not in drm uapi, but uabi. I think we
can align with drm plane definition for sure, but not need to pull in
drm header for that enum type.

> > 
> > plane_id needs a specification.
> 
> Why do you need plane_type? With universal planes the plane_id along is
> sufficient to identify a plane on a given drm device instance. I'd just
> remove it.

This interface is to get vGPU display plane info, there's no normal
drm kms client involved, but vGPU device model trys to expose guest
planes for display. We need to ask for what type of plane required on
target vGPU. I think plane_id here doesn't mean like in drm kms,
but I'm not sure about plane_id here without details, what's the
purpose, etc.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH v18 0/6] drm/i915/gvt: Dma-buf support for GVT-g

[Zhenyu Wang]

On 2017.11.15 11:49:00 +0100, Gerd Hoffmann wrote:
> On Wed, Nov 15, 2017 at 05:11:49PM +0800, Tina Zhang wrote:
> > v17->v18:
> > 1) unmap vgpu's opregion when destroying vgpu.
> > 2) update comments for VFIO_DEVICE_GET_GFX_DMABUF. (Alex)
> 
> > This patch set adds the dma-buf support for intel GVT-g.
> > 
> > dma-buf is an uniform mechanism to share DMA buffers across different
> > devices and subsystems. dma-buf for intel GVT-g is mainly used to share
> > the vgpu's framebuffer to userspace to leverage userspace graphics stacks
> > to render the framebuffer to the display monitor.
> > 
> > The main idea is that we create a gem object and set vgpu's framebuffer as
> > its backing storage. Then, export a dma-buf associated with this gem object.
> > With the fd of this dma-buf, userspace can directly handle this buffer.
> > 
> > This patch set can be tried with the following example:
> > git://git.kraxel.org/qemu  branch: work/intel-vgpu
> > 
> > A topic branch with the latest patch set is:
> > https://github.com/intel/gvt-linux.git   branch: topic/dmabuf
> 
> Tested-by: Gerd Hoffmann 
> 

After debugging with Tina on one left race that fixed by
https://lists.freedesktop.org/archives/intel-gvt-dev/2017-November/002505.html

I still need below qemu fix for proper cursor handling, otherwise qemu
just crashed when I click in my terminal program which hides cursor then.

diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index e500ec2cb1..d9a044b080 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -169,8 +169,9 @@ static void vfio_display_dmabuf_update(void *opaque)
 cursor = vfio_display_get_dmabuf(vdev, DRM_PLANE_TYPE_CURSOR);
 if (vdev->cursor != cursor) {
 vdev->cursor = cursor;
-dpy_gl_cursor_dmabuf(vdev->display_con,
- >buf);
+if (cursor)
+dpy_gl_cursor_dmabuf(vdev->display_con,
+ >buf);
 free_bufs = true;
 }
 if (cursor != NULL) {

And with these it seems pretty fine now that I'll queue them up for -next pull.

thanks

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: linux-next: Signed-off-by missing for commits in the drm tree

[Zhenyu Wang]

On 2017.12.04 10:57:35 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> There is a series of commits
> 
>   54cff6479fd8 - c1802534e5a6
> (not all in that range)
> 
> that are missing a Signed-off-by from their committer.
> 

They were originally committed by Zhi himself, but I had to rebase
onto intel -next before they're sent for pull and missed my signed-off..
Really sorry for that! I think maybe we could still be able to fix that
before next kernel merge.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915/gvt/fb_decoder: Fix out-of-bounds read

[Zhenyu Wang]

On 2017.12.09 00:37:59 -0600, Gustavo A. R. Silva wrote:
> In case function skl_format_to_drm returns -EINVAL, fmt turns into a huge
> number as fmt is of type u32, hence there is an out-of-bounds read when
> using fmt as an index for array skl_pixel_formats at line 225:
> plane->bpp = skl_pixel_formats[fmt].bpp;
> 
> Fix this by comparing the value returned by function skl_format_to_drm
> against the size of array skl_pixel_formats, so in case it is greater than
> or equal to the number of items contained in skl_pixel_formats, print an
> error message and return -EINVAL.
> 
> Addresses-Coverity-ID: 1462495
> Addresses-Coverity-ID: 1462502 ("Out-of-bounds read")
> Fixes: 9f31d1063b43 ("drm/i915/gvt: Add framebuffer decoder support")
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/gpu/drm/i915/gvt/fb_decoder.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c 
> b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> index 72f4217..aed578b 100644
> --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> @@ -222,6 +222,12 @@ int intel_vgpu_decode_primary_plane(struct intel_vgpu 
> *vgpu,
>   val & PLANE_CTL_ORDER_RGBX,
>   val & PLANE_CTL_ALPHA_MASK,
>   val & PLANE_CTL_YUV422_ORDER_MASK);
> +
> + if (fmt >= ARRAY_SIZE(skl_pixel_formats)) {
> + gvt_vgpu_err("Out-of-bounds pixel format index\n");
> + return -EINVAL;
> + }
> +
>   plane->bpp = skl_pixel_formats[fmt].bpp;
>   plane->drm_format = skl_pixel_formats[fmt].drm_format;
>   } else {
> -- 

Applied this, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] vfio: Simplify capability helper

[Zhenyu Wang]

On 2017.12.13 12:13:34 +1100, Alexey Kardashevskiy wrote:
> On 13/12/17 06:59, Alex Williamson wrote:
> > The vfio_info_add_capability() helper requires the caller to pass a
> > capability ID, which it then uses to fill in header fields, assuming
> > hard coded versions.  This makes for an awkward and rigid interface.
> > The only thing we want this helper to do is allocate sufficient
> > space in the caps buffer and chain this capability into the list.
> > Reduce it to that simple task.
> > 
> > Signed-off-by: Alex Williamson 
> 
> 
> Makes more sense now, thanks. I'll repost mine on top of this.
> 
> 
> Reviewed-by: Alexey Kardashevskiy 
> 
>

Looks good for KVMGT part.

Acked-by: Zhenyu Wang 

> Below one observation, unrelated to this patch.
> 
> > ---
> >  drivers/gpu/drm/i915/gvt/kvmgt.c |   15 +++
> >  drivers/vfio/pci/vfio_pci.c  |   14 ++
> >  drivers/vfio/vfio.c  |   52 
> > +++---
> >  include/linux/vfio.h |3 +-
> >  4 files changed, 24 insertions(+), 60 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> > b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 96060920a6fe..0a7d084da1a2 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1012,6 +1012,8 @@ static long intel_vgpu_ioctl(struct mdev_device 
> > *mdev, unsigned int cmd,
> > if (!sparse)
> > return -ENOMEM;
> >  
> > +   sparse->header.id = VFIO_REGION_INFO_CAP_SPARSE_MMAP;
> > +   sparse->header.version = 1;
> > sparse->nr_areas = nr_areas;
> > cap_type_id = VFIO_REGION_INFO_CAP_SPARSE_MMAP;
> 
> 
> @cap_type_id is initialized in just one of many cases of switch
> (info.index) and after the entire switch, there is switch (cap_type_id). I
> wonder why compiler missed "potentially uninitialized variable here,
> although there is no bug - @cap_type_id is in sync with @spapse. It would
> make it cleaner imho just to have vfio_info_add_capability() next to the
> header initialization.
> 

yeah, we could clean that up, thanks for pointing out.

> 
> 
> > sparse->areas[0].offset =
> > @@ -1033,7 +1035,9 @@ static long intel_vgpu_ioctl(struct mdev_device 
> > *mdev, unsigned int cmd,
> > break;
> > default:
> > {
> > -   struct vfio_region_info_cap_type cap_type;
> > +   struct vfio_region_info_cap_type cap_type = {
> > +   .header.id = VFIO_REGION_INFO_CAP_TYPE,
> > +   .header.version = 1 };
> >  
> > if (info.index >= VFIO_PCI_NUM_REGIONS +
> > vgpu->vdev.num_regions)
> > @@ -1050,8 +1054,8 @@ static long intel_vgpu_ioctl(struct mdev_device 
> > *mdev, unsigned int cmd,
> > cap_type.subtype = vgpu->vdev.region[i].subtype;
> >  
> > ret = vfio_info_add_capability(,
> > -   VFIO_REGION_INFO_CAP_TYPE,
> > -   _type);
> > +   _type.header,
> > +   sizeof(cap_type));
> > if (ret)
> > return ret;
> > }
> > @@ -1061,8 +1065,9 @@ static long intel_vgpu_ioctl(struct mdev_device 
> > *mdev, unsigned int cmd,
> > switch (cap_type_id) {
> > case VFIO_REGION_INFO_CAP_SPARSE_MMAP:
> > ret = vfio_info_add_capability(,
> > -   VFIO_REGION_INFO_CAP_SPARSE_MMAP,
> > -   sparse);
> > +   >header, sizeof(*sparse) +
> > +   (sparse->nr_areas *
> > +   sizeof(*sparse->areas)));
> > kfree(sparse);
> > if (ret)
> > return ret;
> > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> > index f041b1a6cf66..a73e40983880 100644
> > --- a/drivers/vfio/pci/vfio_pci.c
> > +++ b/dr

Re: [PATCH] drm/i915/gvt: return error when failing to take the module reference

[Zhenyu Wang]

On 2020.11.12 21:22:32 +0800, Xiongfeng Wang wrote:
> When we fail to take the module reference, we go to the 'undo*' branch and
> return. But the returned variable 'ret' has been set as zero by the
> above code. Change 'ret' to '-ENODEV' in this situation.
> 
> Fixes: 9bdb073464d6 ("drm/i915/gvt: Change KVMGT as self load module")
> Reported-by: Hulk Robot 
> Signed-off-by: Xiongfeng Wang 
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index ad8a9df..778eb8c 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -829,8 +829,10 @@ static int intel_vgpu_open(struct mdev_device *mdev)
>   /* Take a module reference as mdev core doesn't take
>* a reference for vendor driver.
>*/
> - if (!try_module_get(THIS_MODULE))
> + if (!try_module_get(THIS_MODULE)) {
> + ret = -ENODEV;
>   goto undo_group;
> + }
>  
>   ret = kvmgt_guest_init(mdev);
>   if (ret)
> -- 

Thanks for the fix!

Reviewed-by: Zhenyu Wang 

-- 

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915/gvt: replace idr_init() by idr_init_base()

[Zhenyu Wang]

On 2020.11.04 17:45:32 +0530, Deepak R Varma wrote:
> idr_init() uses base 0 which is an invalid identifier. The new function
> idr_init_base allows IDR to set the ID lookup from base 1. This avoids
> all lookups that otherwise starts from 0 since 0 is always unused.
> 
> References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
> 
> Signed-off-by: Deepak R Varma 
> ---
>  drivers/gpu/drm/i915/gvt/gvt.c  | 2 +-
>  drivers/gpu/drm/i915/gvt/vgpu.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index c7c561237883..45b492edbb19 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -312,7 +312,7 @@ int intel_gvt_init_device(struct drm_i915_private *i915)
>  
>   gvt_dbg_core("init gvt device\n");
>  
> - idr_init(>vgpu_idr);
> + idr_init_base(>vgpu_idr, 1);
>   spin_lock_init(>scheduler.mmio_context_lock);
>   mutex_init(>lock);
>   mutex_init(>sched_lock);
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index f6d7e33c7099..1c8e63f84134 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -393,7 +393,7 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct 
> intel_gvt *gvt,
>   mutex_init(>dmabuf_lock);
>   INIT_LIST_HEAD(>dmabuf_obj_list_head);
>   INIT_RADIX_TREE(>page_track_tree, GFP_KERNEL);
> - idr_init(>object_idr);
> + idr_init_base(>object_idr, 1);
>   intel_vgpu_init_cfg_space(vgpu, param->primary);
>   vgpu->d3_entered = false;
>  
> -- 

Looks good to me. Thanks!

Reviewed-by: Zhenyu Wang 

-- 

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH v5 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace

[Zhenyu Wang]

On 2019.08.16 10:35:26 +0800, Tina Zhang wrote:
> Deliver the display refresh events to the user land. Userspace can use
> the irq mask/unmask mechanism to disable or enable the event delivery.
> 
> As we know, delivering refresh event at each vblank safely avoids
> tearing and unexpected event overwhelming, but there are still spaces
> to optimize.
> 
> For handling the normal case, deliver the page flip refresh
> event at each vblank, in other words, bounded by vblanks. Skipping some
> events bring performance enhancement while not hurting user experience.
> 
> For single framebuffer case, deliver the refresh events to userspace at
> all vblanks. This heuristic at each vblank leverages pageflip_count
> incresements to determine if there is no page flip happens after a certain
> period and so that the case is regarded as single framebuffer one.
> Although this heuristic makes incorrect decision sometimes and it depends
> on guest behavior, for example, when no cursor movements happen, the
> user experience does not harm and front buffer is still correctly acquired.
> Meanwhile, in actual single framebuffer case, the user experience is
> enhanced compared with page flip events only.
> 
> Addtionally, to mitigate the events delivering footprints, one eventfd and
> 8 byte eventfd counter partition are leveraged.
> 
> v2:
> - Support vfio_irq_info_cap_display_plane_events. (Tina)
> 
> Signed-off-by: Tina Zhang 
> Signed-off-by: Kechen Lu 
> ---
>  drivers/gpu/drm/i915/gvt/display.c |  22 
>  drivers/gpu/drm/i915/gvt/gvt.h |   2 +
>  drivers/gpu/drm/i915/gvt/kvmgt.c   | 159 +++--
>  3 files changed, 174 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/display.c 
> b/drivers/gpu/drm/i915/gvt/display.c
> index 1a0a4ae4826e..616285e4a014 100644
> --- a/drivers/gpu/drm/i915/gvt/display.c
> +++ b/drivers/gpu/drm/i915/gvt/display.c
> @@ -34,6 +34,8 @@
>  
>  #include "i915_drv.h"
>  #include "gvt.h"
> +#include 
> +#include 
>  
>  static int get_edp_pipe(struct intel_vgpu *vgpu)
>  {
> @@ -387,6 +389,8 @@ void intel_gvt_check_vblank_emulation(struct intel_gvt 
> *gvt)
>   mutex_unlock(>lock);
>  }
>  
> +#define PAGEFLIP_DELAY_THR 10
> +
>  static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
>  {
>   struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> @@ -396,7 +400,10 @@ static void emulate_vblank_on_pipe(struct intel_vgpu 
> *vgpu, int pipe)
>   [PIPE_B] = PIPE_B_VBLANK,
>   [PIPE_C] = PIPE_C_VBLANK,
>   };
> + int pri_flip_event = SKL_FLIP_EVENT(pipe, PLANE_PRIMARY);
>   int event;
> + u64 eventfd_signal_val = 0;
> + static int no_pageflip_count;
>  
>   if (pipe < PIPE_A || pipe > PIPE_C)
>   return;
> @@ -407,11 +414,26 @@ static void emulate_vblank_on_pipe(struct intel_vgpu 
> *vgpu, int pipe)
>   if (!pipe_is_enabled(vgpu, pipe))
>   continue;
>  
> + if (event == pri_flip_event)
> + eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
> +
>   intel_vgpu_trigger_virtual_event(vgpu, event);
>   }
>  
> + if (eventfd_signal_val)
> + no_pageflip_count = 0;
> + else if (!eventfd_signal_val && no_pageflip_count > PAGEFLIP_DELAY_THR)

extra !eventfd_signal_val

> + eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
> + else
> + no_pageflip_count++;

no_pageflip_count should be per-vgpu instead of static.

> +
> + if (vgpu->vdev.vblank_trigger && !vgpu->vdev.display_event_mask &&
> + eventfd_signal_val)
> + eventfd_signal(vgpu->vdev.vblank_trigger, eventfd_signal_val);
> +
>   if (pipe_is_enabled(vgpu, pipe)) {
>   vgpu_vreg_t(vgpu, PIPE_FRMCOUNT_G4X(pipe))++;
> +

extra line

>   intel_vgpu_trigger_virtual_event(vgpu, vblank_event[pipe]);
>   }
>  }
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index cd29ea28d7ed..6c8ed030c30b 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -205,6 +205,8 @@ struct intel_vgpu {
>   int num_irqs;
>   struct eventfd_ctx *intx_trigger;
>   struct eventfd_ctx *msi_trigger;
> + struct eventfd_ctx *vblank_trigger;
> + u32 display_event_mask;
>  
>   /*
>* Two caches are used to avoid mapping duplicated pages (eg.
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index fd1633342e53..9ace1f4ff9eb 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1250,6 +1250,8 @@ static int intel_vgpu_get_irq_count(struct intel_vgpu 
> *vgpu, int type)
>  {
>   if (type == VFIO_PCI_INTX_IRQ_INDEX || type == VFIO_PCI_MSI_IRQ_INDEX)
>   return 1;
> + else if (type < VFIO_PCI_NUM_IRQS + vgpu->vdev.num_irqs)
> +   

Re: 2.6.24-rc6-git7: Reported regressions from 2.6.23

[Zhenyu Wang]

On 2008.01.01 22:02:57 +, Rafael J. Wysocki wrote:
> Subject   : linux-2.6.24-rcX regression / 
> xserver-xorg-video-intel / Q35
> Submitter : Harald Welte <[EMAIL PROTECTED]>
> Date  : 2007-12-22 04:37
> References: http://lkml.org/lkml/2007/12/21/269
> http://bugzilla.kernel.org/show_bug.cgi?id=9618
> Handled-By: Zhenyu Wang <[EMAIL PROTECTED]>
> Patch : 

This is resolved by turning on CONFIG_DMAR_GFX_WA.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

On 2007.12.19 13:26:08 +, Zhenyu Wang wrote:
> 
> [agp-mm] [intel_iommu] explicit export current graphics dmar status
> 
> To make it possbile to tell other modules about curent
> graphics dmar engine status, that could decide if graphics
> driver should remap physical address to dma address.
> 
> Also this one trys to make dmar_disabled really present
> current status of DMAR, which would be used for completely
> express graphics dmar engine is active or not.
> 
> Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
> ---
>  drivers/pci/intel-iommu.c |   18 --
>  include/linux/dmar.h  |6 ++
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index e079a52..81a0abc 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -53,7 +53,7 @@
>  static void domain_remove_dev_info(struct dmar_domain *domain);
>  
>  static int dmar_disabled;
> -static int __initdata dmar_map_gfx = 1;
> +static int dmar_map_gfx = 1;
>  static int dmar_forcedac;
>  
>  #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
> @@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
>  }
>  __setup("intel_iommu=", intel_iommu_setup);
>  
> +int intel_iommu_gfx_remapping(void)
> +{
> +#ifndef CONFIG_DMAR_GFX_WA
> + if (!dmar_disabled && iommu_detected && dmar_map_gfx)
> + return 1;
> +#endif
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
> +
>  static struct kmem_cache *iommu_domain_cache;
>  static struct kmem_cache *iommu_devinfo_cache;
>  static struct kmem_cache *iommu_iova_cache;
> @@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
>  
>  void __init detect_intel_iommu(void)
>  {
> - if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
> + if (dmar_disabled)
> + return;
> + if (swiotlb || no_iommu || iommu_detected) {
> + dmar_disabled = 1;
>   return;
> + }
>   if (early_dmar_detect()) {
>   iommu_detected = 1;
>   }
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index ffb6439..8ae435e 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
>  extern int dmar_table_init(void);
>  extern int early_dmar_detect(void);
>  
> +extern int intel_iommu_gfx_remapping(void);
> +
>  extern struct list_head dmar_drhd_units;
>  extern struct list_head dmar_rmrr_units;
>  
> @@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
>  {
>   return -ENODEV;
>  }
> +static inline int intel_iommu_gfx_remapping(void)
> +{
> + return 0;
> +}
>  
>  #endif /* !CONFIG_DMAR */
>  #endif /* __DMAR_H__ */
> -- 
> 1.5.3.4
> 

Any comments to this one? Is it ok to push this iommu patch with
agp dma remapping patches to next -mm?

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH][intel_agp] fix name for G35 chipset

[Zhenyu Wang]

Change origin chipset name i965G_1 to market name G35.

Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
---
 drivers/char/agp/intel-agp.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index d879619..7f8b06c 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -12,8 +12,8 @@
 
 #define PCI_DEVICE_ID_INTEL_82946GZ_HB  0x2970
 #define PCI_DEVICE_ID_INTEL_82946GZ_IG  0x2972
-#define PCI_DEVICE_ID_INTEL_82965G_1_HB 0x2980
-#define PCI_DEVICE_ID_INTEL_82965G_1_IG 0x2982
+#define PCI_DEVICE_ID_INTEL_82G35_HB0x2980
+#define PCI_DEVICE_ID_INTEL_82G35_IG0x2982
 #define PCI_DEVICE_ID_INTEL_82965Q_HB   0x2990
 #define PCI_DEVICE_ID_INTEL_82965Q_IG   0x2992
 #define PCI_DEVICE_ID_INTEL_82965G_HB   0x29A0
@@ -32,7 +32,7 @@
 #define PCI_DEVICE_ID_INTEL_Q33_IG  0x29D2
 
 #define IS_I965 (agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82946GZ_HB || \
- agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965G_1_HB || 
\
+ agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82G35_HB || \
  agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965Q_HB || \
  agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965G_HB || \
  agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965GM_HB || \
@@ -1866,7 +1866,7 @@ static const struct intel_driver_description {
NULL, _915_driver },
{ PCI_DEVICE_ID_INTEL_82946GZ_HB, PCI_DEVICE_ID_INTEL_82946GZ_IG, 0, 
"946GZ",
NULL, _i965_driver },
-   { PCI_DEVICE_ID_INTEL_82965G_1_HB, PCI_DEVICE_ID_INTEL_82965G_1_IG, 0, 
"965G",
+   { PCI_DEVICE_ID_INTEL_82G35_HB, PCI_DEVICE_ID_INTEL_82G35_IG, 0, "G35",
NULL, _i965_driver },
{ PCI_DEVICE_ID_INTEL_82965Q_HB, PCI_DEVICE_ID_INTEL_82965Q_IG, 0, 
"965Q",
NULL, _i965_driver },
@@ -2065,7 +2065,7 @@ static struct pci_device_id agp_intel_pci_table[] = {
ID(PCI_DEVICE_ID_INTEL_82945GM_HB),
ID(PCI_DEVICE_ID_INTEL_82945GME_HB),
ID(PCI_DEVICE_ID_INTEL_82946GZ_HB),
-   ID(PCI_DEVICE_ID_INTEL_82965G_1_HB),
+   ID(PCI_DEVICE_ID_INTEL_82G35_HB),
ID(PCI_DEVICE_ID_INTEL_82965Q_HB),
ID(PCI_DEVICE_ID_INTEL_82965G_HB),
ID(PCI_DEVICE_ID_INTEL_82965GM_HB),
-- 
1.5.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH][AGP] intel_agp: add new chipset ids

[Zhenyu Wang]

Dave,

This one adds new pci ids for Intel intergrated graphics
chipset, with gtt table access change on it and new gtt table
size definition.

Thanks.

Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
---
 drivers/char/agp/agp.h   |3 +++
 drivers/char/agp/intel-agp.c |   31 ++-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/agp.h b/drivers/char/agp/agp.h
index b83824c..d132914 100644
--- a/drivers/char/agp/agp.h
+++ b/drivers/char/agp/agp.h
@@ -235,6 +235,9 @@ struct agp_bridge_data {
 #define I965_PGETBL_SIZE_512KB (0 << 1)
 #define I965_PGETBL_SIZE_256KB (1 << 1)
 #define I965_PGETBL_SIZE_128KB (2 << 1)
+#define I965_PGETBL_SIZE_1MB   (3 << 1)
+#define I965_PGETBL_SIZE_2MB   (4 << 1)
+#define I965_PGETBL_SIZE_1_5MB (5 << 1)
 #define G33_PGETBL_SIZE_MASK(3 << 8)
 #define G33_PGETBL_SIZE_1M  (1 << 8)
 #define G33_PGETBL_SIZE_2M  (2 << 8)
diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index d879619..091e765 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -30,13 +30,16 @@
 #define PCI_DEVICE_ID_INTEL_Q35_IG  0x29B2
 #define PCI_DEVICE_ID_INTEL_Q33_HB  0x29D0
 #define PCI_DEVICE_ID_INTEL_Q33_IG  0x29D2
+#define PCI_DEVICE_ID_INTEL_IGD_HB  0x2A40
+#define PCI_DEVICE_ID_INTEL_IGD_IG  0x2A42
 
 #define IS_I965 (agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82946GZ_HB || \
  agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965G_1_HB || 
\
  agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965Q_HB || \
  agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965G_HB || \
  agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965GM_HB || \
- agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965GME_HB)
+ agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965GME_HB || 
\
+ agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_IGD_HB)
 
 #define IS_G33 (agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_G33_HB || \
agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_Q35_HB || \
@@ -456,6 +459,15 @@ static void intel_i830_init_gtt_entries(void)
case I965_PGETBL_SIZE_512KB:
size = 512;
break;
+   case I965_PGETBL_SIZE_1MB:
+   size = 1024;
+   break;
+   case I965_PGETBL_SIZE_2MB:
+   size = 2048;
+   break;
+   case I965_PGETBL_SIZE_1_5MB:
+   size = 1024 + 512;
+   break;
default:
printk(KERN_INFO PFX "Unknown page table size, "
   "assuming 512KB\n");
@@ -981,6 +993,7 @@ static int intel_i965_create_gatt_table(struct 
agp_bridge_data *bridge)
struct aper_size_info_fixed *size;
int num_entries;
u32 temp;
+   int gtt_offset, gtt_size;
 
size = agp_bridge->current_size;
page_order = size->page_order;
@@ -990,13 +1003,18 @@ static int intel_i965_create_gatt_table(struct 
agp_bridge_data *bridge)
pci_read_config_dword(intel_private.pcidev, I915_MMADDR, );
 
temp &= 0xfff0;
-   intel_private.gtt = ioremap((temp + (512 * 1024)) , 512 * 1024);
 
-   if (!intel_private.gtt)
-   return -ENOMEM;
+   if (agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_IGD_HB)
+  gtt_offset = gtt_size = MB(2);
+   else
+  gtt_offset = gtt_size = KB(512);
+
+   intel_private.gtt = ioremap((temp + gtt_offset) , gtt_size);
 
+   if (!intel_private.gtt)
+  return -ENOMEM;
 
-   intel_private.registers = ioremap(temp,128 * 4096);
+   intel_private.registers = ioremap(temp, 128 * 4096);
if (!intel_private.registers) {
iounmap(intel_private.gtt);
return -ENOMEM;
@@ -1884,6 +1902,8 @@ static const struct intel_driver_description {
NULL, _g33_driver },
{ PCI_DEVICE_ID_INTEL_Q33_HB, PCI_DEVICE_ID_INTEL_Q33_IG, 0, "Q33",
NULL, _g33_driver },
+   { PCI_DEVICE_ID_INTEL_IGD_HB, PCI_DEVICE_ID_INTEL_IGD_IG, 0,
+   "Intel Integrated Graphics Device", NULL, _i965_driver },
{ 0, 0, 0, NULL, NULL, NULL }
 };
 
@@ -2073,6 +2093,7 @@ static struct pci_device_id agp_intel_pci_table[] = {
ID(PCI_DEVICE_ID_INTEL_G33_HB),
ID(PCI_DEVICE_ID_INTEL_Q35_HB),
ID(PCI_DEVICE_ID_INTEL_Q33_HB),
+   ID(PCI_DEVICE_ID_INTEL_IGD_HB),
{ }
 };
 
-- 
1.5.3.7

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

Mark, sorry for missing this for long time...

On 2008.01.08 12:44:20 -0800, mark gross wrote:
> > > 
> > > [agp-mm] [intel_iommu] explicit export current graphics dmar status
> > > 
> > > To make it possbile to tell other modules about curent
> > > graphics dmar engine status, that could decide if graphics
> > > driver should remap physical address to dma address.
> > > 
> > > Also this one trys to make dmar_disabled really present
> > > current status of DMAR, which would be used for completely
> > > express graphics dmar engine is active or not.
> 
> do you think this exporting will be needed?
> If so why and when would someone use it?

This is used for our graphics driver module to know if we have to
do dma remapping in iommu case, both in kernel config and kernel
boot time param config. 

The simplest case is that DMAR_GFX_WA is on, which no dma mapping
will act in intel_agp.

If no DMAR_GFX_WA, we still have boot param to turn off whole intel
iommu (intel_iommu=off), just turn off graphics remap engine
(intel_iommu=igfx_off). So this exported interface is used to know
that in runtime.

> 
> > > 
> > > Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
> > > ---
> > >  drivers/pci/intel-iommu.c |   18 --
> > >  include/linux/dmar.h  |6 ++
> > >  2 files changed, 22 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> > > index e079a52..81a0abc 100644
> > > --- a/drivers/pci/intel-iommu.c
> > > +++ b/drivers/pci/intel-iommu.c
> > > @@ -53,7 +53,7 @@
> > >  static void domain_remove_dev_info(struct dmar_domain *domain);
> > >  
> > >  static int dmar_disabled;
> > > -static int __initdata dmar_map_gfx = 1;
> > > +static int dmar_map_gfx = 1;
> > >  static int dmar_forcedac;
> > >  
> > >  #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
> > > @@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
> > >  }
> > >  __setup("intel_iommu=", intel_iommu_setup);
> > >  
> > > +int intel_iommu_gfx_remapping(void)
> > > +{
> > > +#ifndef CONFIG_DMAR_GFX_WA
> > > + if (!dmar_disabled && iommu_detected && dmar_map_gfx)
> > > + return 1;
> > > +#endif
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
> > > +
> > >  static struct kmem_cache *iommu_domain_cache;
> > >  static struct kmem_cache *iommu_devinfo_cache;
> > >  static struct kmem_cache *iommu_iova_cache;
> > > @@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
> > >  
> > >  void __init detect_intel_iommu(void)
> > >  {
> > > - if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
> > > + if (dmar_disabled)
> > > + return;
> > > + if (swiotlb || no_iommu || iommu_detected) {
> > > + dmar_disabled = 1;
> > >   return;
> 
> Why the bloat?  This block of 7 lines looks like it does the same as the
> 2 you replaced.

No, dmar_disabled is not set in origin, which can't tell if it's turned
off with (intel_iommu=off) later.

> 
> > > + }
> > >   if (early_dmar_detect()) {
> > >   iommu_detected = 1;
> > >   }
> > > diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> > > index ffb6439..8ae435e 100644
> > > --- a/include/linux/dmar.h
> > > +++ b/include/linux/dmar.h
> > > @@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
> > >  extern int dmar_table_init(void);
> > >  extern int early_dmar_detect(void);
> > >  
> > > +extern int intel_iommu_gfx_remapping(void);
> > > +
> > >  extern struct list_head dmar_drhd_units;
> > >  extern struct list_head dmar_rmrr_units;
> > >  
> > > @@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
> > >  {
> > >   return -ENODEV;
> > >  }
> > > +static inline int intel_iommu_gfx_remapping(void)
> > > +{
> > > + return 0;
> > > +}
> 
> Um this function is silly lets not post it.
> 
> > >  
> > >  #endif /* !CONFIG_DMAR */
> > >  #endif /* __DMAR_H__ */
> > > -- 
> > > 1.5.3.4
> > > 
> > 
> > Any comments to this one? Is it ok to push this iommu patch with
> > agp dma remapping patches to next -mm?
> > 
> 
> Its not ready for posting.
> 
> --mgross
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-2.6.24-rcX regression / xserver-xorg-video-intel / Q35

[Zhenyu Wang]

On 2007.12.22 01:25:16 +0100, Harald Welte wrote:
> 
> I'm running an Intel DQ35JO mainboard (Q35 chipset, Q6600 CPU) and I am
> observing a regression with linux-2.6.24-rc1 through -rc6 (linux-2.6.git as
> of today, ea67db4cdbbf7f4e74150e71da0984e25121f500).
> 
> The last working version is 2.6.24-rc1.
> 
> The system is running debian unstable (current) using
> xserver-xorg-video-intel 2.2.0-1
> 
> So what is the actual problem:
> It seems to be related to the way how the iommu/gart is used for memory
> allocation of the framebuffer memory.

There's no behavior change in intel agp module between .24-rc1 to rc6.
IOMMU shouldn't matter here, if you build x86_64 system and with
CONFIG_DMAR on, you should be already having CONFIG_DMAR_GFX_WA for you.

> 
> Xorg starts just as it should, but the lower part of the screen is
> completely gobbled.  I suppose the lower part of the screen is actually
> showing off-screen memory at some completely differnt location.

Do you have other changes except kernel? like other xorg packages, bios?

> 
> Interestingly, the mouse cursor is superimposed on top of the garbage
> (and it is not distorted).

yeah, hardware cursor uses another plane than scan buffer, it's
seperate.

Could you fire a bug to https://bugs.freedesktop.org with versions and
logs? Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

On 2008.01.25 10:08:20 -0800, mark gross wrote:
> > This is used for our graphics driver module to know if we have to
> > do dma remapping in iommu case, both in kernel config and kernel
> > boot time param config. 
> 
> ok. How soon do you need this export?
> 

As graphics part of these patches depend on this one, it should go
first, or along with agpgart patches, that should first be in Airlie's
agp-mm queue.

> send me your current patch and eta for the graphics module patches so I
> can better coordinate with you.

You can find my current patches against agp-mm tree (so equally against
current -mm kernel) here, http://people.freedesktop.org/~zhen/, fixed
with some warnings from checkpatch.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC][PATCH 0/4] enabling graphics memory dma remapping

[Zhenyu Wang]

Intel IOMMU (a.k.a VT-d) is under rapid deployment on desktop
and mobile platforms. As platform provides multiple dma remap
engines for devices like those lives on south bridge (net, sound,
etc.), and we also have one engine specific to graphics device.

If this engine is functioning, the access to graphics memory
routine is to first look up in GART table, which return virtual
dma address that will further be route to graphics DMAR engine,
which then looking up DMAR table to get real physical address.

Current intel iommu function in kernel is providing a graphics
workaround kernel config (CONFIG_DMAR_GFX_WA) to make a 1:1 mapping
initialization for graphics device, so what we fill in GART table
got from agpgart page alloc is identical to virtual dma address in
DMAR table. No change needs to be made for graphics driver.

Following patches try to add dma remapping function to agpgart
module, so we won't depend on intel iommu graphics workaround. 
As DMAR is only available on x86_64 now, so these patches can only
work for x86_64 system.

Here're three patches below:

- intel_iommu-explicit-export-current-graphics-dmar-status.patch
  
  This exports current status of graphics dma remap engine, which
  depends on current platform iommu support, kernel config or runtime
  config. This info will be used in agp module for detect whether
  we should do remapping or not.

- AGP-Add-generic-support-for-graphics-dma-remapping.patch

  This adds new hooks into agp bridge driver to do map/unmap when
  bind/unbind memory into GART table. The aim is to provide generic
  interfaces to let driver implement hardware specific operations.

- AGP-intel_agp-add-support-for-graphics-dma-remapping.patch

  This implements graphics dma remapping support in intel_agp module,
  current only desktop chipset G33 series can use it.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

[PATCH] [intel_iommu] explicit export current graphics dmar status

To make it possbile to tell other modules about curent
graphics dmar engine status, that could decide if graphics
driver should remap physical address to dma address.

Also this one trys to make dmar_disabled really present
current status of DMAR, which would be used for completely
express graphics dmar engine is active or not.

Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
---
 drivers/pci/intel-iommu.c |   18 --
 include/linux/dmar.h  |6 ++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index e079a52..81a0abc 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -53,7 +53,7 @@
 static void domain_remove_dev_info(struct dmar_domain *domain);
 
 static int dmar_disabled;
-static int __initdata dmar_map_gfx = 1;
+static int dmar_map_gfx = 1;
 static int dmar_forcedac;
 
 #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
@@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
 }
 __setup("intel_iommu=", intel_iommu_setup);
 
+int intel_iommu_gfx_remapping(void)
+{
+#ifndef CONFIG_DMAR_GFX_WA
+   if (!dmar_disabled && iommu_detected && dmar_map_gfx)
+   return 1;
+#endif
+   return 0;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
+
 static struct kmem_cache *iommu_domain_cache;
 static struct kmem_cache *iommu_devinfo_cache;
 static struct kmem_cache *iommu_iova_cache;
@@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
 
 void __init detect_intel_iommu(void)
 {
-   if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
+   if (dmar_disabled)
+   return;
+   if (swiotlb || no_iommu || iommu_detected) {
+   dmar_disabled = 1;
return;
+   }
if (early_dmar_detect()) {
iommu_detected = 1;
}
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index ffb6439..8ae435e 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
 extern int dmar_table_init(void);
 extern int early_dmar_detect(void);
 
+extern int intel_iommu_gfx_remapping(void);
+
 extern struct list_head dmar_drhd_units;
 extern struct list_head dmar_rmrr_units;
 
@@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
 {
return -ENODEV;
 }
+static inline int intel_iommu_gfx_remapping(void)
+{
+   return 0;
+}
 
 #endif /* !CONFIG_DMAR */
 #endif /* __DMAR_H__ */
-- 
1.5.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC][PATCH 2/4][AGP] Add generic support for graphics dma remapping

[Zhenyu Wang]

[PATCH] [AGP] Add generic support for graphics dma remapping

New driver hooks for support graphics memory dma remapping
are introduced in this patch. It makes generic code can
tell if current device needs dma remapping, then call driver
provided interfaces for mapping and unmapping. Change has
also been made to handle scratch_page in remapping case.

Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
---
 drivers/char/agp/agp.h  |   14 ++
 drivers/char/agp/backend.c  |   21 -
 drivers/char/agp/generic.c  |   14 ++
 include/linux/agp_backend.h |7 +++
 4 files changed, 55 insertions(+), 1 deletions(-)

diff --git a/drivers/char/agp/agp.h b/drivers/char/agp/agp.h
index b83824c..7806da1 100644
--- a/drivers/char/agp/agp.h
+++ b/drivers/char/agp/agp.h
@@ -118,6 +118,18 @@ struct agp_bridge_driver {
void *(*agp_alloc_page)(struct agp_bridge_data *);
void (*agp_destroy_page)(void *, int flags);
 int (*agp_type_to_mask_type) (struct agp_bridge_data *, int);
+
+   /* Tell current graphics dma remapping engine status, there might
+* be driver or platform specific way to detect. */
+   int (*agp_require_remapping)(void);
+
+   /* This can support single page remapping via void *virt for like
+* scratch_page, or normal bunch of mem page range via struct
+* agp_memory *mem.
+*/
+   int (*agp_map_page)(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single);
+
+   void (*agp_unmap_page)(struct agp_memory *mem, dma_addr_t ret, int 
is_single);
 };
 
 struct agp_bridge_data {
@@ -132,6 +144,8 @@ struct agp_bridge_data {
u32 *gatt_table_real;
unsigned long scratch_page;
unsigned long scratch_page_real;
+   dma_addr_t scratch_page_dma;
+   u8 scratch_page_is_mapped;
unsigned long gart_bus_addr;
unsigned long gatt_bus_addr;
u32 mode;
diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
index 832ded2..cc6d648 100644
--- a/drivers/char/agp/backend.c
+++ b/drivers/char/agp/backend.c
@@ -151,7 +151,20 @@ static int agp_backend_initialize(struct agp_bridge_data 
*bridge)
 
bridge->scratch_page_real = virt_to_gart(addr);
bridge->scratch_page =
-   bridge->driver->mask_memory(bridge, 
bridge->scratch_page_real, 0);
+   bridge->driver->mask_memory(bridge, 
bridge->scratch_page_real, 0);
+
+   if (bridge->driver->agp_require_remapping &&
+   bridge->driver->agp_require_remapping()) {
+   if (bridge->driver->agp_map_page(NULL, addr, 
>scratch_page_dma, 1)) {
+   printk(KERN_ERR PFX "unable to map scratch 
page\n");
+   rc = -ENOMEM;
+   goto err_out;
+   }
+   bridge->scratch_page_is_mapped = TRUE;
+   bridge->scratch_page =
+   bridge->driver->mask_memory(bridge,
+   bridge->scratch_page_dma, 0);
+   }
}
 
size_value = bridge->driver->fetch_size();
@@ -189,6 +202,9 @@ static int agp_backend_initialize(struct agp_bridge_data 
*bridge)
 
 err_out:
if (bridge->driver->needs_scratch_page) {
+   if (bridge->scratch_page_is_mapped)
+   bridge->driver->agp_unmap_page(NULL, 
bridge->scratch_page_dma, 1);
+

bridge->driver->agp_destroy_page(gart_to_virt(bridge->scratch_page_real),
 AGP_PAGE_DESTROY_UNMAP);
flush_agp_mappings();
@@ -217,6 +233,9 @@ static void agp_backend_cleanup(struct agp_bridge_data 
*bridge)
 
if (bridge->driver->agp_destroy_page &&
bridge->driver->needs_scratch_page) {
+   if (bridge->scratch_page_is_mapped)
+   bridge->driver->agp_unmap_page(NULL, 
bridge->scratch_page_dma, 1);
+

bridge->driver->agp_destroy_page(gart_to_virt(bridge->scratch_page_real),
 AGP_PAGE_DESTROY_UNMAP);
flush_agp_mappings();
diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c
index 64b2f6d..8966c94 100644
--- a/drivers/char/agp/generic.c
+++ b/drivers/char/agp/generic.c
@@ -415,6 +415,14 @@ int agp_bind_memory(struct agp_memory *curr, off_t 
pg_start)
curr->bridge->driver->cache_flush();
curr->is_flushed = TRUE;
}
+
+   if (curr->bridge->driver->agp_require_remapping &&
+   curr->bridge->driver->agp_require_remapping()) {
+   ret_val = c

[RFC][PATCH 3/4][AGP] intel_agp: add support for graphics dma remapping on G33

[Zhenyu Wang]

[PATCH] [AGP] intel_agp: add support for graphics dma remapping on G33

When graphics dma remapping engine is active, we must fill
gart table with dma address from dmar engine, as now graphics
device access to graphics memory must go through dma remapping
table to get real physical address.

Add support on G33 chipset, which has graphics device specific
dmar engine avaiable.

Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
---
 drivers/char/agp/Kconfig |   12 +++-
 drivers/char/agp/intel-agp.c |  134 +-
 2 files changed, 141 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/Kconfig b/drivers/char/agp/Kconfig
index ccb1fa8..e60c2b0 100644
--- a/drivers/char/agp/Kconfig
+++ b/drivers/char/agp/Kconfig
@@ -74,8 +74,16 @@ config AGP_INTEL
  on Intel 440LX/BX/GX, 815, 820, 830, 840, 845, 850, 860, 875,
  E7205 and E7505 chipsets and full support for the 810, 815, 830M,
  845G, 852GM, 855GM, 865G and I915 integrated graphics chipsets.
-
-
+   
+config AGP_INTEL_DMAR
+   bool
+   depends on AGP_INTEL && DMAR && !DMAR_GFX_WA
+   default y
+   help
+ This option will enable graphics address remapping with Intel
+ DMAR engine aka VT-d if you don't select graphics workaround on
+ Intel DMAR. The graphics address filled to gart table will be
+ changed by remapping within graphics DMAR engine for domain.
 
 config AGP_NVIDIA
tristate "NVIDIA nForce/nForce2 chipset support"
diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index d879619..7424667 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -9,6 +9,9 @@
 #include 
 #include 
 #include "agp.h"
+#ifdef CONFIG_AGP_INTEL_DMAR
+#include 
+#endif
 
 #define PCI_DEVICE_ID_INTEL_82946GZ_HB  0x2970
 #define PCI_DEVICE_ID_INTEL_82946GZ_IG  0x2972
@@ -115,6 +118,97 @@ static struct _intel_private {
int gtt_entries;/* i830+ */
 } intel_private;
 
+static int intel_agp_require_remapping(void)
+{
+#ifdef CONFIG_AGP_INTEL_DMAR
+   return intel_iommu_gfx_remapping();
+#else
+   return 0;
+#endif
+}
+
+#ifdef CONFIG_AGP_INTEL_DMAR
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single)
+{
+   int i;
+   struct scatterlist *sg;
+
+   if (is_single) {
+   *ret = pci_map_single(intel_private.pcidev, virt, PAGE_SIZE,
+   PCI_DMA_BIDIRECTIONAL);
+   if (pci_dma_mapping_error(*ret))
+   return -EINVAL;
+   return 0;
+   }
+
+   DBG("try mapping %lu pages\n", (unsigned long)mem->page_count);
+
+   if ((mem->page_count * sizeof(*mem->sg_list)) < 2*PAGE_SIZE)
+   mem->sg_list = kcalloc(mem->page_count, sizeof(*mem->sg_list), 
GFP_KERNEL);
+
+   if (mem->sg_list == NULL) {
+   mem->sg_list = vmalloc(mem->page_count * sizeof(*mem->sg_list));
+   mem->sg_vmalloc_flag = 1;
+   }
+
+   if (!mem->sg_list) {
+   mem->sg_vmalloc_flag = 0;
+   return -ENOMEM;
+   }
+   sg_init_table(mem->sg_list, mem->page_count);
+
+   sg = mem->sg_list;
+   for (i = 0 ; i < mem->page_count; i++, sg = sg_next(sg))
+   sg_set_buf(sg, gart_to_virt(mem->memory[i]), PAGE_SIZE);
+
+   mem->num_sg = pci_map_sg(intel_private.pcidev, mem->sg_list,
+   mem->page_count, PCI_DMA_BIDIRECTIONAL);
+   if (mem->num_sg == 0) {
+   if (mem->sg_vmalloc_flag)
+   vfree(mem->sg_list);
+   else
+   kfree(mem->sg_list);
+   mem->sg_list = NULL;
+   mem->sg_vmalloc_flag = 0;
+   return -ENOMEM;
+   }
+   mem->is_mapped = TRUE;
+   return 0;
+}
+
+static void intel_agp_unmap_pages(struct agp_memory *mem, dma_addr_t addr, int 
is_single)
+{
+   if (is_single) {
+   pci_unmap_single(intel_private.pcidev, addr, PAGE_SIZE,
+   PCI_DMA_BIDIRECTIONAL);
+   return;
+   }
+   DBG("try unmapping %lu pages\n", (unsigned long)mem->page_count);
+
+   if (!mem->is_mapped)
+   return;
+
+   if (mem->num_sg)
+   pci_unmap_sg(intel_private.pcidev, mem->sg_list, 
mem->page_count,
+   PCI_DMA_BIDIRECTIONAL);
+   if (mem->sg_vmalloc_flag)
+   vfree(mem->sg_list);
+   else
+   kfree(mem->sg_list);
+   mem->sg_vmalloc_flag = 0;
+   mem->sg_list = NULL;
+   mem->num_sg = 0;
+}
+#else
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single)
+{
+re

[agp-mm][PATCH 0/4] enabling graphics memory dma remapping

[Zhenyu Wang]

On 2007.12.18 13:08:09 +, Zhenyu Wang wrote:
> 
> Here're three patches below:
> 
> - intel_iommu-explicit-export-current-graphics-dmar-status.patch
>   
>   This exports current status of graphics dma remap engine, which
>   depends on current platform iommu support, kernel config or runtime
>   config. This info will be used in agp module for detect whether
>   we should do remapping or not.
> 
> - AGP-Add-generic-support-for-graphics-dma-remapping.patch
> 
>   This adds new hooks into agp bridge driver to do map/unmap when
>   bind/unbind memory into GART table. The aim is to provide generic
>   interfaces to let driver implement hardware specific operations.
> 
> - AGP-intel_agp-add-support-for-graphics-dma-remapping.patch
> 
>   This implements graphics dma remapping support in intel_agp module,
>   current only desktop chipset G33 series can use it.

Above patch series are against master branch, following threads try to
rebase them to Airlie's agp-mm tree for -mm inclusion.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

[agp-mm] [intel_iommu] explicit export current graphics dmar status

To make it possbile to tell other modules about curent
graphics dmar engine status, that could decide if graphics
driver should remap physical address to dma address.

Also this one trys to make dmar_disabled really present
current status of DMAR, which would be used for completely
express graphics dmar engine is active or not.

Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
---
 drivers/pci/intel-iommu.c |   18 --
 include/linux/dmar.h  |6 ++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index e079a52..81a0abc 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -53,7 +53,7 @@
 static void domain_remove_dev_info(struct dmar_domain *domain);
 
 static int dmar_disabled;
-static int __initdata dmar_map_gfx = 1;
+static int dmar_map_gfx = 1;
 static int dmar_forcedac;
 
 #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
@@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
 }
 __setup("intel_iommu=", intel_iommu_setup);
 
+int intel_iommu_gfx_remapping(void)
+{
+#ifndef CONFIG_DMAR_GFX_WA
+   if (!dmar_disabled && iommu_detected && dmar_map_gfx)
+   return 1;
+#endif
+   return 0;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
+
 static struct kmem_cache *iommu_domain_cache;
 static struct kmem_cache *iommu_devinfo_cache;
 static struct kmem_cache *iommu_iova_cache;
@@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
 
 void __init detect_intel_iommu(void)
 {
-   if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
+   if (dmar_disabled)
+   return;
+   if (swiotlb || no_iommu || iommu_detected) {
+   dmar_disabled = 1;
return;
+   }
if (early_dmar_detect()) {
iommu_detected = 1;
}
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index ffb6439..8ae435e 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
 extern int dmar_table_init(void);
 extern int early_dmar_detect(void);
 
+extern int intel_iommu_gfx_remapping(void);
+
 extern struct list_head dmar_drhd_units;
 extern struct list_head dmar_rmrr_units;
 
@@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
 {
return -ENODEV;
 }
+static inline int intel_iommu_gfx_remapping(void)
+{
+   return 0;
+}
 
 #endif /* !CONFIG_DMAR */
 #endif /* __DMAR_H__ */
-- 
1.5.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[agp-mm][PATCH 2/4][AGP] Add generic support for graphics dma remapping

[Zhenyu Wang]

[agp-mm] [AGP] Add generic support for graphics dma remapping

New driver hooks for support graphics memory dma remapping
are introduced in this patch. It makes generic code can
tell if current device needs dma remapping, then call driver
provided interfaces for mapping and unmapping. Change has
also been made to handle scratch_page in remapping case.

Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
---
 drivers/char/agp/agp.h  |   13 +
 drivers/char/agp/backend.c  |   21 -
 drivers/char/agp/generic.c  |   14 ++
 include/linux/agp_backend.h |7 +++
 4 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/drivers/char/agp/agp.h b/drivers/char/agp/agp.h
index 9ec9374..2c78a28 100644
--- a/drivers/char/agp/agp.h
+++ b/drivers/char/agp/agp.h
@@ -119,6 +119,17 @@ struct agp_bridge_driver {
void (*agp_destroy_page)(void *, int flags);
int (*agp_type_to_mask_type) (struct agp_bridge_data *, int);
void (*chipset_flush)(struct agp_bridge_data *);
+   /* Tell current graphics dma remapping engine status, there might
+* be driver or platform specific way to detect. */
+   int (*agp_require_remapping)(void);
+
+   /* This can support single page remapping via void *virt for like
+* scratch_page, or normal bunch of mem page range via struct
+* agp_memory *mem.
+*/
+   int (*agp_map_page)(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single);
+
+   void (*agp_unmap_page)(struct agp_memory *mem, dma_addr_t ret, int 
is_single);
 };
 
 struct agp_bridge_data {
@@ -133,6 +144,8 @@ struct agp_bridge_data {
u32 *gatt_table_real;
unsigned long scratch_page;
unsigned long scratch_page_real;
+   dma_addr_t scratch_page_dma;
+   u8 scratch_page_is_mapped;
unsigned long gart_bus_addr;
unsigned long gatt_bus_addr;
u32 mode;
diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
index f9c180c..7898051 100644
--- a/drivers/char/agp/backend.c
+++ b/drivers/char/agp/backend.c
@@ -151,7 +151,20 @@ static int agp_backend_initialize(struct agp_bridge_data 
*bridge)
 
bridge->scratch_page_real = virt_to_gart(addr);
bridge->scratch_page =
-   bridge->driver->mask_memory(bridge, 
bridge->scratch_page_real, 0);
+   bridge->driver->mask_memory(bridge, 
bridge->scratch_page_real, 0);
+
+   if (bridge->driver->agp_require_remapping &&
+   bridge->driver->agp_require_remapping()) {
+   if (bridge->driver->agp_map_page(NULL, addr, 
>scratch_page_dma, 1)) {
+   printk(KERN_ERR PFX "unable to map scratch 
page\n");
+   rc = -ENOMEM;
+   goto err_out;
+   }
+   bridge->scratch_page_is_mapped = TRUE;
+   bridge->scratch_page =
+   bridge->driver->mask_memory(bridge,
+   bridge->scratch_page_dma, 0);
+   }
}
 
size_value = bridge->driver->fetch_size();
@@ -189,6 +202,9 @@ static int agp_backend_initialize(struct agp_bridge_data 
*bridge)
 
 err_out:
if (bridge->driver->needs_scratch_page) {
+   if (bridge->scratch_page_is_mapped)
+   bridge->driver->agp_unmap_page(NULL, 
bridge->scratch_page_dma, 1);
+

bridge->driver->agp_destroy_page(gart_to_virt(bridge->scratch_page_real),
 AGP_PAGE_DESTROY_UNMAP);
flush_agp_mappings();
@@ -217,6 +233,9 @@ static void agp_backend_cleanup(struct agp_bridge_data 
*bridge)
 
if (bridge->driver->agp_destroy_page &&
bridge->driver->needs_scratch_page) {
+   if (bridge->scratch_page_is_mapped)
+   bridge->driver->agp_unmap_page(NULL, 
bridge->scratch_page_dma, 1);
+

bridge->driver->agp_destroy_page(gart_to_virt(bridge->scratch_page_real),
 AGP_PAGE_DESTROY_UNMAP);
flush_agp_mappings();
diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c
index 8c67b4f..d2a502c 100644
--- a/drivers/char/agp/generic.c
+++ b/drivers/char/agp/generic.c
@@ -422,6 +422,14 @@ int agp_bind_memory(struct agp_memory *curr, off_t 
pg_start)
curr->bridge->driver->cache_flush();
curr->is_flushed = TRUE;
}
+
+   if (curr->bridge->driver->agp_require_remapping &&
+   curr->bridge->driver->agp_require_remapping()) {
+   ret_val = c

[agp-mm][PATCH 3/4][AGP] intel_agp: add support for graphics dma remapping on G33

[Zhenyu Wang]

[agp-mm] [AGP] intel_agp: add support for graphics dma remapping on G33

When graphics dma remapping engine is active, we must fill
gart table with dma address from dmar engine, as now graphics
device access to graphics memory must go through dma remapping
table to get real physical address.

Add support on G33 chipset, which has graphics device specific
dmar engine avaiable.

Signed-off-by: Zhenyu Wang <[EMAIL PROTECTED]>
---
 drivers/char/agp/Kconfig |   12 +++-
 drivers/char/agp/intel-agp.c |  134 +-
 2 files changed, 141 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/Kconfig b/drivers/char/agp/Kconfig
index ccb1fa8..e60c2b0 100644
--- a/drivers/char/agp/Kconfig
+++ b/drivers/char/agp/Kconfig
@@ -74,8 +74,16 @@ config AGP_INTEL
  on Intel 440LX/BX/GX, 815, 820, 830, 840, 845, 850, 860, 875,
  E7205 and E7505 chipsets and full support for the 810, 815, 830M,
  845G, 852GM, 855GM, 865G and I915 integrated graphics chipsets.
-
-
+   
+config AGP_INTEL_DMAR
+   bool
+   depends on AGP_INTEL && DMAR && !DMAR_GFX_WA
+   default y
+   help
+ This option will enable graphics address remapping with Intel
+ DMAR engine aka VT-d if you don't select graphics workaround on
+ Intel DMAR. The graphics address filled to gart table will be
+ changed by remapping within graphics DMAR engine for domain.
 
 config AGP_NVIDIA
tristate "NVIDIA nForce/nForce2 chipset support"
diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index f443682..31a939d 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -9,6 +9,9 @@
 #include 
 #include 
 #include "agp.h"
+#ifdef CONFIG_AGP_INTEL_DMAR
+#include 
+#endif
 
 #define PCI_DEVICE_ID_INTEL_82946GZ_HB  0x2970
 #define PCI_DEVICE_ID_INTEL_82946GZ_IG  0x2972
@@ -123,6 +126,97 @@ static struct _intel_private {
struct resource ifp_resource;
 } intel_private;
 
+static int intel_agp_require_remapping(void)
+{
+#ifdef CONFIG_AGP_INTEL_DMAR
+   return intel_iommu_gfx_remapping();
+#else
+   return 0;
+#endif
+}
+
+#ifdef CONFIG_AGP_INTEL_DMAR
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single)
+{
+   int i;
+   struct scatterlist *sg;
+
+   if (is_single) {
+   *ret = pci_map_single(intel_private.pcidev, virt, PAGE_SIZE,
+   PCI_DMA_BIDIRECTIONAL);
+   if (pci_dma_mapping_error(*ret))
+   return -EINVAL;
+   return 0;
+   }
+
+   DBG("try mapping %lu pages\n", (unsigned long)mem->page_count);
+
+   if ((mem->page_count * sizeof(*mem->sg_list)) < 2*PAGE_SIZE)
+   mem->sg_list = kcalloc(mem->page_count, sizeof(*mem->sg_list), 
GFP_KERNEL);
+
+   if (mem->sg_list == NULL) {
+   mem->sg_list = vmalloc(mem->page_count * sizeof(*mem->sg_list));
+   mem->sg_vmalloc_flag = 1;
+   }
+
+   if (!mem->sg_list) {
+   mem->sg_vmalloc_flag = 0;
+   return -ENOMEM;
+   }
+   sg_init_table(mem->sg_list, mem->page_count);
+
+   sg = mem->sg_list;
+   for (i = 0 ; i < mem->page_count; i++, sg = sg_next(sg))
+   sg_set_buf(sg, gart_to_virt(mem->memory[i]), PAGE_SIZE);
+
+   mem->num_sg = pci_map_sg(intel_private.pcidev, mem->sg_list,
+   mem->page_count, PCI_DMA_BIDIRECTIONAL);
+   if (mem->num_sg == 0) {
+   if (mem->sg_vmalloc_flag)
+   vfree(mem->sg_list);
+   else
+   kfree(mem->sg_list);
+   mem->sg_list = NULL;
+   mem->sg_vmalloc_flag = 0;
+   return -ENOMEM;
+   }
+   mem->is_mapped = TRUE;
+   return 0;
+}
+
+static void intel_agp_unmap_pages(struct agp_memory *mem, dma_addr_t addr, int 
is_single)
+{
+   if (is_single) {
+   pci_unmap_single(intel_private.pcidev, addr, PAGE_SIZE,
+   PCI_DMA_BIDIRECTIONAL);
+   return;
+   }
+   DBG("try unmapping %lu pages\n", (unsigned long)mem->page_count);
+
+   if (!mem->is_mapped)
+   return;
+
+   if (mem->num_sg)
+   pci_unmap_sg(intel_private.pcidev, mem->sg_list, 
mem->page_count,
+   PCI_DMA_BIDIRECTIONAL);
+   if (mem->sg_vmalloc_flag)
+   vfree(mem->sg_list);
+   else
+   kfree(mem->sg_list);
+   mem->sg_vmalloc_flag = 0;
+   mem->sg_list = NULL;
+   mem->num_sg = 0;
+}
+#else
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t 
*ret, int is_single)
+{
+return 0;
+}
+static v

Re: [PATCH] drm/i915/gvt: fix uninitialized return in intel_gvt_update_reg_whitelist()

[Zhenyu Wang]

On 2021.01.25 09:44:53 +, Chris Wilson wrote:
> Quoting Dan Carpenter (2021-01-25 08:48:30)
> > Smatch found an uninitialized variable bug in this code:
> > 
> > drivers/gpu/drm/i915/gvt/cmd_parser.c:3191 
> > intel_gvt_update_reg_whitelist()
> > error: uninitialized symbol 'ret'.
> > 
> > The first thing that Smatch complains about is that "ret" isn't set if
> > we don't enter the "for_each_engine(engine, _priv->gt, id) {" loop.
> > Presumably we always have at least one engine so that's a false
> > positive.
> > 
> > But it's definitely a bug to not set "ret" if i915_gem_object_pin_map()
> > fails.
> 
> True.
>  
> > Let's fix the bug and silence the false positive.
> > 
> > Fixes: 493f30cd086e ("drm/i915/gvt: parse init context to update cmd 
> > accessible reg whitelist")
> > Signed-off-by: Dan Carpenter 
> Reviewed-by: Chris Wilson 

Thanks, Dan & Chris! Queued this up.


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915/gvt: Use ARRAY_SIZE for vgpu_types

[Zhenyu Wang]

On 2020.05.18 22:00:52 +0100, Chris Wilson wrote:
> Quoting Aishwarya Ramakrishnan (2020-05-18 16:03:36)
> > Prefer ARRAY_SIZE instead of using sizeof
> > 
> > Fixes coccicheck warning: Use ARRAY_SIZE
> > 
> > Signed-off-by: Aishwarya Ramakrishnan 
> Reviewed-by: Chris Wilson 

Applied, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915/gvt: print actionable error message when gm runs out

[Zhenyu Wang]

On 2020.06.03 14:33:21 +0200, Julian Stecklina wrote:
> When a user tries to allocate too many or too big vGPUs and runs out
> of graphics memory, the resulting error message is not actionable and
> looks like an internal error.
> 
> Change the error message to clearly point out what actions a user can
> take to resolve this situation.
> 
> Cc: Thomas Prescher 
> Cc: Zhenyu Wang 
> Signed-off-by: Julian Stecklina 
> ---
>  drivers/gpu/drm/i915/gvt/aperture_gm.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/aperture_gm.c 
> b/drivers/gpu/drm/i915/gvt/aperture_gm.c
> index 0d6d598713082..5c5c8e871dae2 100644
> --- a/drivers/gpu/drm/i915/gvt/aperture_gm.c
> +++ b/drivers/gpu/drm/i915/gvt/aperture_gm.c
> @@ -69,9 +69,12 @@ static int alloc_gm(struct intel_vgpu *vgpu, bool high_gm)
> start, end, flags);
>   mmio_hw_access_post(gt);
>   mutex_unlock(>ggtt->vm.mutex);
> - if (ret)
> - gvt_err("fail to alloc %s gm space from host\n",
> - high_gm ? "high" : "low");
> + if (ret) {
> + gvt_err("vgpu%d: failed to allocate %s gm space from host\n",
> + vgpu->id, high_gm ? "high" : "low");
> + gvt_err("vgpu%d: destroying vGPUs, decreasing vGPU memory size 
> or increasing GPU aperture size may resolve this\n",
> + vgpu->id);

Currently we can't decrease vGPU mem size as defined by mdev type,
so actually you may try different vGPU type. And aperture size is
also handled for supported vGPU mdev types, so assume user should
already be awared of that too. I just don't want us to be too chatty. :)

> + }
>  
>   return ret;
>  }
> -- 
> 2.26.2
> 
> ___
> intel-gvt-dev mailing list
> intel-gvt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH v18 5/6] vfio: ABI for mdev display dma-buf operation

[Zhenyu Wang]

On 2017.11.15 11:48:42 -0700, Alex Williamson wrote:
> On Wed, 15 Nov 2017 17:11:54 +0800
> Tina Zhang  wrote:
> 
> > Add VFIO_DEVICE_QUERY_GFX_PLANE ioctl command to let user query and get
> > a plane and its information. So far, two types of buffers are supported:
> > buffers based on dma-buf and buffers based on region.
> > 
> > This ioctl can be invoked with:
> > 1) Either DMABUF or REGION flag. Vendor driver returns a plane_info
> > successfully only when the specific kind of buffer is supported.
> > 2) Flag PROBE. And at the same time either DMABUF or REGION must be set,
> > so that vendor driver returns success only when the specific kind of
> > buffer is supported.
> > 
> > Add VFIO_DEVICE_GET_GFX_DMABUF ioctl command to let user get a specific
> > dma-buf fd of an exposed MDEV buffer provided by dmabuf_id which was
> > returned in VFIO_DEVICE_QUERY_GFX_PLANE ioctl command.
> > 
> > The life cycle of an exposed MDEV buffer is handled by userspace and
> > tracked by kernel space. The returned dmabuf_id in struct vfio_device_
> > query_gfx_plane can be a new id of a new exposed buffer or an old id of
> > a re-exported buffer. Host user can check the value of dmabuf_id to see
> > if it needs to create new resources according to the new exposed buffer
> > or just re-use the existing resource related to the old buffer.
> > 
> > v18:
> > - update comments for VFIO_DEVICE_GET_GFX_DMABUF. (Alex)
> > 
> > v17:
> > - modify VFIO_DEVICE_GET_GFX_DMABUF interface. (Alex)
> > 
> > v16:
> > - add x_hot and y_hot fields. (Gerd)
> > - add comments for VFIO_DEVICE_GET_GFX_DMABUF. (Alex)
> > - rebase to 4.14.0-rc6.
> > 
> > v15:
> > - add a ioctl to get a dmabuf for a given dmabuf id. (Gerd)
> > 
> > v14:
> > - add PROBE, DMABUF and REGION flags. (Alex)
> > 
> > v12:
> > - add drm_format_mod back. (Gerd and Zhenyu)
> > - add region_index. (Gerd)
> > 
> > v11:
> > - rename plane_type to drm_plane_type. (Gerd)
> > - move fields of vfio_device_query_gfx_plane to vfio_device_gfx_plane_info.
> >   (Gerd)
> > - remove drm_format_mod, start fields. (Daniel)
> > - remove plane_id.
> > 
> > v10:
> > - refine the ABI API VFIO_DEVICE_QUERY_GFX_PLANE. (Alex) (Gerd)
> > 
> > v3:
> > - add a field gvt_plane_info in the drm_i915_gem_obj structure to save
> >   the decoded plane information to avoid look up while need the plane
> >   info. (Gerd)
> > 
> > Signed-off-by: Tina Zhang 
> > Cc: Gerd Hoffmann 
> > Cc: Alex Williamson 
> > Cc: Daniel Vetter 
> > ---
> >  include/uapi/linux/vfio.h | 62 
> > +++
> >  1 file changed, 62 insertions(+)
> > 
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index ae46105..5c1cca2 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -502,6 +502,68 @@ struct vfio_pci_hot_reset {
> >  
> >  #define VFIO_DEVICE_PCI_HOT_RESET  _IO(VFIO_TYPE, VFIO_BASE + 13)
> >  
> > +/**
> > + * VFIO_DEVICE_QUERY_GFX_PLANE - _IOW(VFIO_TYPE, VFIO_BASE + 14,
> > + *struct vfio_device_query_gfx_plane)
> > + *
> > + * Set the drm_plane_type and flags, then retrieve the gfx plane info.
> > + *
> > + * flags supported:
> > + * - VFIO_GFX_PLANE_TYPE_PROBE and VFIO_GFX_PLANE_TYPE_DMABUF are set
> > + *   to ask if the mdev supports dma-buf. 0 on support, -EINVAL on no
> > + *   support for dma-buf.
> > + * - VFIO_GFX_PLANE_TYPE_PROBE and VFIO_GFX_PLANE_TYPE_REGION are set
> > + *   to ask if the mdev supports region. 0 on support, -EINVAL on no
> > + *   support for region.
> > + * - VFIO_GFX_PLANE_TYPE_DMABUF or VFIO_GFX_PLANE_TYPE_REGION is set
> > + *   with each call to query the plane info.
> > + * - Others are invalid and return -EINVAL.
> > + *
> > + * Note:
> > + * 1. Plane could be disabled by guest. In that case, success will be
> > + *returned with zero-initialized drm_format, size, width and height
> > + *fields.
> > + * 2. x_hot/y_hot is set to 0x if no hotspot information available
> > + *
> > + * Return: 0 on success, -errno on other failure.
> > + */
> > +struct vfio_device_gfx_plane_info {
> > +   __u32 argsz;
> > +   __u32 flags;
> > +#define VFIO_GFX_PLANE_TYPE_PROBE (1 << 0)
> > +#define VFIO_GFX_PLANE_TYPE_DMABUF (1 << 1)
> > +#define VFIO_GFX_PLANE_TYPE_REGION (1 << 2)
> > +   /* in */
> > +   __u32 drm_plane_type;   /* type of plane: DRM_PLANE_TYPE_* */
> > +   /* out */
> > +   __u32 drm_format;   /* drm format of plane */
> > +   __u64 drm_format_mod;   /* tiled mode */
> > +   __u32 width;/* width of plane */
> > +   __u32 height;   /* height of plane */
> > +   __u32 stride;   /* stride of plane */
> > +   __u32 size; /* size of plane in bytes, align on page*/
> > +   __u32 x_pos;/* horizontal position of cursor plane */
> > +   __u32 y_pos;/* vertical position of cursor plane*/
> > +   __u32 x_hot;/* horizontal position of cursor hotspot */
> > +   __u32 y_hot;/* vertical position of cursor hotspot */
> > +   union {

Re: [RFC PATCH v4 1/6] vfio: Define device specific irq type capability

[Zhenyu Wang]

On 2019.07.18 23:56:35 +0800, Kechen Lu wrote:
> From: Tina Zhang 
> 
> Cap the number of irqs with fixed indexes and use capability chains
> to chain device specific irqs.
> 
> Signed-off-by: Tina Zhang 
> Signed-off-by: Eric Auger 
> ---
>  include/uapi/linux/vfio.h | 19 ++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 8f10748dac79..be6adab4f759 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -448,11 +448,27 @@ struct vfio_irq_info {
>  #define VFIO_IRQ_INFO_MASKABLE   (1 << 1)
>  #define VFIO_IRQ_INFO_AUTOMASKED (1 << 2)
>  #define VFIO_IRQ_INFO_NORESIZE   (1 << 3)
> +#define VFIO_IRQ_INFO_FLAG_CAPS  (1 << 4) /* Info supports caps 
> */
>   __u32   index;  /* IRQ index */
>   __u32   count;  /* Number of IRQs within this index */
> + __u32   cap_offset; /* Offset within info struct of first cap */

This still breaks ABI as argsz would be updated with this new field, so it would
cause compat issue. I think my last suggestion was to assume cap list starts 
after
vfio_irq_info.

>  };
>  #define VFIO_DEVICE_GET_IRQ_INFO _IO(VFIO_TYPE, VFIO_BASE + 9)
>  
> +/*
> + * The irq type capability allows irqs unique to a specific device or
> + * class of devices to be exposed.
> + *
> + * The structures below define version 1 of this capability.
> + */
> +#define VFIO_IRQ_INFO_CAP_TYPE  3
> +
> +struct vfio_irq_info_cap_type {
> + struct vfio_info_cap_header header;
> + __u32 type; /* global per bus driver */
> + __u32 subtype;  /* type specific */
> +};
> +
>  /**
>   * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct 
> vfio_irq_set)
>   *
> @@ -554,7 +570,8 @@ enum {
>   VFIO_PCI_MSIX_IRQ_INDEX,
>   VFIO_PCI_ERR_IRQ_INDEX,
>   VFIO_PCI_REQ_IRQ_INDEX,
> - VFIO_PCI_NUM_IRQS
> + VFIO_PCI_NUM_IRQS = 5   /* Fixed user ABI, IRQ indexes >=5 use   */
> + /* device specific cap to define content */
>  };
>  
>  /*
> -- 
> 2.17.1
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [RFC PATCH v4 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace

[Zhenyu Wang]

On 2019.07.18 23:56:38 +0800, Kechen Lu wrote:
> From: Tina Zhang 
> 
> Deliver the display refresh events to the user land. Userspace can use
> the irq mask/unmask mechanism to disable or enable the event delivery.
> 
> As we know, delivering refresh event at each vblank safely avoids
> tearing and unexpected event overwhelming, but there are still spaces
> to optimize.
> 
> For handling the normal case, deliver the page flip refresh
> event at each vblank, in other words, bounded by vblanks. Skipping some
> events bring performance enhancement while not hurting user experience.
> 
> For single framebuffer case, deliver the refresh events to userspace at
> all vblanks. This heuristic at each vblank leverages pageflip_count
> incresements to determine if there is no page flip happens after a certain
> period and so that the case is regarded as single framebuffer one.
> Although this heuristic makes incorrect decision sometimes and it depends
> on guest behavior, for example, when no cursor movements happen, the
> user experience does not harm and front buffer is still correctly acquired.
> Meanwhile, in actual single framebuffer case, the user experience is
> enhanced compared with page flip events only.
> 
> Addtionally, to mitigate the events delivering footprints, one eventfd and
> 8 byte eventfd counter partition are leveraged.
> 
> Signed-off-by: Tina Zhang 
> Signed-off-by: Kechen Lu 
> ---
>  drivers/gpu/drm/i915/gvt/display.c |  21 
>  drivers/gpu/drm/i915/gvt/gvt.h |   7 ++
>  drivers/gpu/drm/i915/gvt/kvmgt.c   | 154 +++--
>  3 files changed, 173 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/display.c 
> b/drivers/gpu/drm/i915/gvt/display.c
> index 1a0a4ae4826e..036db8199983 100644
> --- a/drivers/gpu/drm/i915/gvt/display.c
> +++ b/drivers/gpu/drm/i915/gvt/display.c
> @@ -387,6 +387,8 @@ void intel_gvt_check_vblank_emulation(struct intel_gvt 
> *gvt)
>   mutex_unlock(>lock);
>  }
>  
> +#define PAGEFLIP_INC_COUNT 5
> +
>  static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
>  {
>   struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> @@ -396,7 +398,10 @@ static void emulate_vblank_on_pipe(struct intel_vgpu 
> *vgpu, int pipe)
>   [PIPE_B] = PIPE_B_VBLANK,
>   [PIPE_C] = PIPE_C_VBLANK,
>   };
> + int pri_flip_event = SKL_FLIP_EVENT(pipe, PLANE_PRIMARY);
>   int event;
> + u64 eventfd_signal_val = 0;
> + static int pageflip_count;
>  
>   if (pipe < PIPE_A || pipe > PIPE_C)
>   return;
> @@ -407,11 +412,27 @@ static void emulate_vblank_on_pipe(struct intel_vgpu 
> *vgpu, int pipe)
>   if (!pipe_is_enabled(vgpu, pipe))
>   continue;
>  
> + if (event == pri_flip_event) {
> + eventfd_signal_val += DISPLAY_PRI_REFRESH_EVENT_INC;
> + pageflip_count += PAGEFLIP_INC_COUNT;
> + }
> +
>   intel_vgpu_trigger_virtual_event(vgpu, event);
>   }
>  
> + if (--pageflip_count < 0) {
> + eventfd_signal_val += DISPLAY_PRI_REFRESH_EVENT_INC;
> + pageflip_count = 0;
> + }

If pageflip_count has been increased to a big number from page flip
event for some time, then if guest switch for single buffer render, it
would take 5x vblank time to send refresh then..

> +
> + if (vgpu->vdev.vblank_trigger && !(vgpu->vdev.display_event_mask
> + & (DISPLAY_PRI_REFRESH_EVENT | DISPLAY_CUR_REFRESH_EVENT)) &&
> + eventfd_signal_val)
> + eventfd_signal(vgpu->vdev.vblank_trigger, eventfd_signal_val);
> +
>   if (pipe_is_enabled(vgpu, pipe)) {
>   vgpu_vreg_t(vgpu, PIPE_FRMCOUNT_G4X(pipe))++;
> +
>   intel_vgpu_trigger_virtual_event(vgpu, vblank_event[pipe]);
>   }
>  }
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 64d1c1aaa42a..b654b6fa0663 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -165,6 +165,11 @@ struct intel_vgpu_submission {
>   bool active;
>  };
>  
> +#define DISPLAY_PRI_REFRESH_EVENT(1 << 0)
> +#define DISPLAY_PRI_REFRESH_EVENT_INC(1UL << 56)
> +#define DISPLAY_CUR_REFRESH_EVENT(1 << 1)
> +#define DISPLAY_CUR_REFRESH_EVENT_INC(1UL << 48)
> +

As this is for eventfd interface definition, need to put in vfio header instead 
of gvt's,
as this is userspace API. And better reorder for different usage on irq masking 
and eventfd value.

For eventfd value, this looks like counter for each plane? Or do we just need a 
flag?

>  struct intel_vgpu {
>   struct intel_gvt *gvt;
>   struct mutex vgpu_lock;
> @@ -205,6 +210,8 @@ struct intel_vgpu {
>   int num_irqs;
>   struct eventfd_ctx *intx_trigger;
>   struct eventfd_ctx *msi_trigger;
> + struct eventfd_ctx *vblank_trigger;
> + u32 

Re: [RFC PATCH v4 6/6] drm/i915/gvt: Add cursor plane reg update trap emulation handler

[Zhenyu Wang]

On 2019.07.18 23:56:40 +0800, Kechen Lu wrote:
> This patch adds the cursor plane CURBASE reg update trap handler
> in order to :
> 
> - Deliver the cursor refresh event at each vblank emulation,
> the flip_done_event bit check is supposed to do here. If cursor
> plane updates happen, deliver the cursor refresh events.
> 
> - Support the sync and async cursor plane updates and
> corresponding cursor plane flip interrupts reporting.
> 
> Signed-off-by: Kechen Lu 
> ---
>  drivers/gpu/drm/i915/gvt/display.c   | 11 +++
>  drivers/gpu/drm/i915/gvt/handlers.c  | 27 ---
>  drivers/gpu/drm/i915/gvt/interrupt.c |  7 +++
>  drivers/gpu/drm/i915/gvt/interrupt.h |  3 +++
>  4 files changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/display.c 
> b/drivers/gpu/drm/i915/gvt/display.c
> index df52e4b4c1b0..a0accc51d44f 100644
> --- a/drivers/gpu/drm/i915/gvt/display.c
> +++ b/drivers/gpu/drm/i915/gvt/display.c
> @@ -399,6 +399,7 @@ static void emulate_vblank_on_pipe(struct intel_vgpu 
> *vgpu, int pipe)
>   [PIPE_C] = PIPE_C_VBLANK,
>   };
>   int pri_flip_event = SKL_FLIP_EVENT(pipe, PLANE_PRIMARY);
> + int cur_flip_event = CURSOR_A_FLIP_DONE + pipe;
>   int event;
>   u64 eventfd_signal_val = 0;
>   static int pageflip_count;
> @@ -417,6 +418,11 @@ static void emulate_vblank_on_pipe(struct intel_vgpu 
> *vgpu, int pipe)
>   pageflip_count += PAGEFLIP_INC_COUNT;
>   }
>  
> + if (event == cur_flip_event) {
> + eventfd_signal_val += DISPLAY_CUR_REFRESH_EVENT_INC;
> + pageflip_count += PAGEFLIP_INC_COUNT;
> + }
> +
>   intel_vgpu_trigger_virtual_event(vgpu, event);
>   }
>  
> @@ -430,6 +436,11 @@ static void emulate_vblank_on_pipe(struct intel_vgpu 
> *vgpu, int pipe)
>   eventfd_signal_val += DISPLAY_PRI_REFRESH_EVENT_INC;
>   pageflip_count += PAGEFLIP_INC_COUNT;
>   }
> +
> + if (event == PLANE_CURSOR) {
> + eventfd_signal_val += DISPLAY_CUR_REFRESH_EVENT_INC;
> + pageflip_count += PAGEFLIP_INC_COUNT;
> + }
>   }
>  
>   if (--pageflip_count < 0) {
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index 6ad29c4f08e5..821ff88977d8 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -767,6 +767,27 @@ static int pri_surf_mmio_write(struct intel_vgpu *vgpu, 
> unsigned int offset,
>   return 0;
>  }
>  
> +#define CURBASE_TO_PIPE(reg) \
> + calc_index(offset, _CURABASE, _CURBBASE, 0, CURBASE(PIPE_C))
> +
> +static int cur_surf_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
> + void *p_data, unsigned int bytes)
> +{
> + struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> + u32 pipe = CURBASE_TO_PIPE(offset);
> + int event = CURSOR_A_FLIP_DONE + pipe;
> +
> + write_vreg(vgpu, offset, p_data, bytes);
> +
> + if (vgpu_vreg_t(vgpu, CURCNTR(pipe)) & PLANE_CTL_ASYNC_FLIP) {
> + intel_vgpu_trigger_virtual_event(vgpu, event);
> + set_bit(PLANE_CURSOR, vgpu->display.async_flip_event[pipe]);
> + } else
> + set_bit(event, vgpu->irq.flip_done_event[pipe]);
> +
> + return 0;
> +}
> +
>  #define SPRSURF_TO_PIPE(offset) \
>   calc_index(offset, _SPRA_SURF, _SPRB_SURF, 0, SPRSURF(PIPE_C))
>  
> @@ -1955,9 +1976,9 @@ static int init_generic_mmio_info(struct intel_gvt *gvt)
>   MMIO_D(CURPOS(PIPE_B), D_ALL);
>   MMIO_D(CURPOS(PIPE_C), D_ALL);
>  
> - MMIO_D(CURBASE(PIPE_A), D_ALL);
> - MMIO_D(CURBASE(PIPE_B), D_ALL);
> - MMIO_D(CURBASE(PIPE_C), D_ALL);
> + MMIO_DH(CURBASE(PIPE_A), D_ALL, NULL, cur_surf_mmio_write);
> + MMIO_DH(CURBASE(PIPE_B), D_ALL, NULL, cur_surf_mmio_write);
> + MMIO_DH(CURBASE(PIPE_C), D_ALL, NULL, cur_surf_mmio_write);

I think we should also track cursor pos change right?

>  
>   MMIO_D(CUR_FBC_CTL(PIPE_A), D_ALL);
>   MMIO_D(CUR_FBC_CTL(PIPE_B), D_ALL);
> diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c 
> b/drivers/gpu/drm/i915/gvt/interrupt.c
> index 951681813230..9c2b9d2e1529 100644
> --- a/drivers/gpu/drm/i915/gvt/interrupt.c
> +++ b/drivers/gpu/drm/i915/gvt/interrupt.c
> @@ -113,6 +113,9 @@ static const char * const irq_name[INTEL_GVT_EVENT_MAX] = 
> {
>   [SPRITE_A_FLIP_DONE] = "Sprite Plane A flip done",
>   [SPRITE_B_FLIP_DONE] = "Sprite Plane B flip done",
>   [SPRITE_C_FLIP_DONE] = "Sprite Plane C flip done",
> + [CURSOR_A_FLIP_DONE] = "Cursor Plane A flip done",
> + [CURSOR_B_FLIP_DONE] = "Cursor Plane B flip done",
> + [CURSOR_C_FLIP_DONE] = "Cursor Plane C flip done",
>  
>   [PCU_THERMAL] = "PCU Thermal Event",
>   [PCU_PCODE2DRIVER_MAILBOX] = "PCU pcode2driver mailbox event",
> @@ -593,6 +596,10 @@ 

Re: [RFC PATCH v4 1/6] vfio: Define device specific irq type capability

[Zhenyu Wang]

On 2019.07.19 09:02:33 +, Lu, Kechen wrote:
> Hi,
> 
> > -Original Message-
> > From: Zhenyu Wang [mailto:zhen...@linux.intel.com]
> > Sent: Friday, July 19, 2019 2:06 PM
> > To: Lu, Kechen 
> > Cc: intel-gvt-...@lists.freedesktop.org; k...@vger.kernel.org; linux- 
> > ker...@vger.kernel.org; Zhang, Tina ; 
> > kra...@redhat.com; zhen...@linux.intel.com; Lv, Zhiyuan 
> > ; Wang, Zhi A ; Tian, 
> > Kevin ; Yuan, Hang ; 
> > alex.william...@redhat.com; Eric Auger 
> > Subject: Re: [RFC PATCH v4 1/6] vfio: Define device specific irq type 
> > capability
> > 
> > On 2019.07.18 23:56:35 +0800, Kechen Lu wrote:
> > > From: Tina Zhang 
> > >
> > > Cap the number of irqs with fixed indexes and use capability chains 
> > > to chain device specific irqs.
> > >
> > > Signed-off-by: Tina Zhang 
> > > Signed-off-by: Eric Auger 
> > > ---
> > >  include/uapi/linux/vfio.h | 19 ++-
> > >  1 file changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h 
> > > index 8f10748dac79..be6adab4f759 100644
> > > --- a/include/uapi/linux/vfio.h
> > > +++ b/include/uapi/linux/vfio.h
> > > @@ -448,11 +448,27 @@ struct vfio_irq_info {
> > >  #define VFIO_IRQ_INFO_MASKABLE   (1 << 1)
> > >  #define VFIO_IRQ_INFO_AUTOMASKED (1 << 2)
> > >  #define VFIO_IRQ_INFO_NORESIZE   (1 << 3)
> > > +#define VFIO_IRQ_INFO_FLAG_CAPS  (1 << 4) /* Info supports caps
> > */
> > >   __u32   index;  /* IRQ index */
> > >   __u32   count;  /* Number of IRQs within this index */
> > > + __u32   cap_offset; /* Offset within info struct of first cap */
> > 
> > This still breaks ABI as argsz would be updated with this new field, 
> > so it would cause compat issue. I think my last suggestion was to 
> > assume cap list starts after vfio_irq_info.
> >
>  
> In the common practice, the general logic is first use the "count" as the 
> "minsz" boundary to perform copy from user, and then perform following logic, 
> so that the incompatibility issue would not happen. BTW, this patch has been 
> double checked by Eric Auger before included in his patch-set. 
> 

yeah, sorry I was thinking vfio might fail in that case but it seems
current code assume argsz should be larger than minsz for count here,
so that's fine.

> 
> > >  };
> > >  #define VFIO_DEVICE_GET_IRQ_INFO _IO(VFIO_TYPE, VFIO_BASE + 9)
> > >
> > > +/*
> > > + * The irq type capability allows irqs unique to a specific device 
> > > +or
> > > + * class of devices to be exposed.
> > > + *
> > > + * The structures below define version 1 of this capability.
> > > + */
> > > +#define VFIO_IRQ_INFO_CAP_TYPE  3
> > > +
> > > +struct vfio_irq_info_cap_type {
> > > + struct vfio_info_cap_header header;
> > > + __u32 type; /* global per bus driver */
> > > + __u32 subtype;  /* type specific */ };
> > > +
> > >  /**
> > >   * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct
> > vfio_irq_set)
> > >   *
> > > @@ -554,7 +570,8 @@ enum {
> > >   VFIO_PCI_MSIX_IRQ_INDEX,
> > >   VFIO_PCI_ERR_IRQ_INDEX,
> > >   VFIO_PCI_REQ_IRQ_INDEX,
> > > - VFIO_PCI_NUM_IRQS
> > > + VFIO_PCI_NUM_IRQS = 5   /* Fixed user ABI, IRQ indexes >=5 use
> > */
> > > + /* device specific cap to define content */
> > >  };
> > >
> > >  /*
> > > --
> > > 2.17.1
> > >
> > 
> > --
> > Open Source Technology Center, Intel ltd.
> > 
> > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [PATCH 1/2] vfio: ABI for setting mdev display flip eventfd

[Zhenyu Wang]

On 2019.05.27 16:43:11 +0800, Tina Zhang wrote:
> Add VFIO_DEVICE_SET_GFX_FLIP_EVENTFD ioctl command to set eventfd
> based signaling mechanism to deliver vGPU framebuffer page flip
> event to userspace.
>

Should we add probe to see if driver can support gfx flip event?

> Signed-off-by: Tina Zhang 
> ---
>  include/uapi/linux/vfio.h | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 02bb7ad6e986..27300597717f 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -696,6 +696,18 @@ struct vfio_device_ioeventfd {
>  
>  #define VFIO_DEVICE_IOEVENTFD_IO(VFIO_TYPE, VFIO_BASE + 16)
>  
> +/**
> + * VFIO_DEVICE_SET_GFX_FLIP_EVENTFD - _IOW(VFIO_TYPE, VFIO_BASE + 17, __s32)
> + *
> + * Set eventfd based signaling mechanism to deliver vGPU framebuffer page
> + * flip event to userspace. A value of -1 is used to stop the page flip
> + * delivering.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +
> +#define VFIO_DEVICE_SET_GFX_FLIP_EVENTFD _IO(VFIO_TYPE, VFIO_BASE + 17)
> +
>  /*  API for Type1 VFIO IOMMU  */
>  
>  /**
> -- 
> 2.17.1
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature