Re: [Intel-gfx] [PATCH 1/4] drm/i915/gvt: remove unused to_gvt() and reduce includes

[Wang, Zhi A]

I will try to ping Zhenyu to pick it up from GVT-g. 

Thanks,
Zhi.

-Original Message-
From: Nikula, Jani  
Sent: Wednesday, October 4, 2023 7:33 PM
To: Wang, Zhi A ; intel-gvt-...@lists.freedesktop.org; 
Zhenyu Wang 
Cc: intel-gfx@lists.freedesktop.org
Subject: RE: [PATCH 1/4] drm/i915/gvt: remove unused to_gvt() and reduce 
includes

On Wed, 04 Oct 2023, "Wang, Zhi A"  wrote:
> Singed-off-by: Zhi Wang 

Mmh, sorry, what does that mean here? Are you picking them up via gvt?

BR,
Jani.

>
> Thanks,
> Zhi.
>
> -Original Message-
> From: Nikula, Jani 
> Sent: Wednesday, October 4, 2023 3:54 PM
> To: intel-gvt-...@lists.freedesktop.org; Zhenyu Wang 
> ; Wang, Zhi A 
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 1/4] drm/i915/gvt: remove unused to_gvt() and 
> reduce includes
>
> On Tue, 26 Sep 2023, Jani Nikula  wrote:
>> gvt.h has no need to include i915_drv.h once the unused to_gvt() has 
>> been removed.
>>
>> Signed-off-by: Jani Nikula 
>
> Zhenyu, Zhi, ping?
>
> BR,
> Jani.
>
>
>
>> ---
>>  drivers/gpu/drm/i915/gvt/gvt.h | 7 +--
>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h 
>> b/drivers/gpu/drm/i915/gvt/gvt.h index 53a0a42a50db..3a0624fe63bf
>> 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.h
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
>> @@ -39,7 +39,7 @@
>>
>>  #include 
>>
>> -#include "i915_drv.h"
>> +#include "gt/intel_gt.h"
>>  #include "intel_gvt.h"
>>
>>  #include "debug.h"
>> @@ -368,11 +368,6 @@ struct intel_gvt {
>>   struct dentry *debugfs_root;
>>  };
>>
>> -static inline struct intel_gvt *to_gvt(struct drm_i915_private 
>> *i915) -{
>> - return i915->gvt;
>> -}
>> -
>>  enum {
>>   /* Scheduling trigger by timer */
>>   INTEL_GVT_REQUEST_SCHED = 0,
>
> --
> Jani Nikula, Intel

--
Jani Nikula, Intel

Re: [Intel-gfx] [PATCH 1/4] drm/i915/gvt: remove unused to_gvt() and reduce includes

[Wang, Zhi A]

Singed-off-by: Zhi Wang 

Thanks,
Zhi.

-Original Message-
From: Nikula, Jani  
Sent: Wednesday, October 4, 2023 3:54 PM
To: intel-gvt-...@lists.freedesktop.org; Zhenyu Wang ; 
Wang, Zhi A 
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/4] drm/i915/gvt: remove unused to_gvt() and reduce 
includes

On Tue, 26 Sep 2023, Jani Nikula  wrote:
> gvt.h has no need to include i915_drv.h once the unused to_gvt() has 
> been removed.
>
> Signed-off-by: Jani Nikula 

Zhenyu, Zhi, ping?

BR,
Jani.



> ---
>  drivers/gpu/drm/i915/gvt/gvt.h | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h 
> b/drivers/gpu/drm/i915/gvt/gvt.h index 53a0a42a50db..3a0624fe63bf 
> 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -39,7 +39,7 @@
>  
>  #include 
>  
> -#include "i915_drv.h"
> +#include "gt/intel_gt.h"
>  #include "intel_gvt.h"
>  
>  #include "debug.h"
> @@ -368,11 +368,6 @@ struct intel_gvt {
>   struct dentry *debugfs_root;
>  };
>  
> -static inline struct intel_gvt *to_gvt(struct drm_i915_private *i915) 
> -{
> - return i915->gvt;
> -}
> -
>  enum {
>   /* Scheduling trigger by timer */
>   INTEL_GVT_REQUEST_SCHED = 0,

--
Jani Nikula, Intel

Re: [Intel-gfx] [PATCH v4 29/29] drm/i915/gvt: Drop final dependencies on KVM internal details

[Wang, Zhi A]

On 7/29/2023 4:35 AM, Sean Christopherson wrote:

Open code gpa_to_gfn() in kvmgt_page_track_write() and drop KVMGT's
dependency on kvm_host.h, i.e. include only kvm_page_track.h.  KVMGT
assumes "gfn == gpa >> PAGE_SHIFT" all over the place, including a few
lines below in the same function with the same gpa, i.e. there's no
reason to use KVM's helper for this one case.

No functional change intended.

Reviewed-by: Yan Zhao 
Tested-by: Yongwei Ma 
Signed-off-by: Sean Christopherson 
---
  drivers/gpu/drm/i915/gvt/gvt.h   | 3 ++-
  drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
  2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 2d65800d8e93..53a0a42a50db 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -34,10 +34,11 @@
  #define _GVT_H_
  
  #include 

-#include 
  #include 
  #include 
  
+#include 

+
  #include "i915_drv.h"
  #include "intel_gvt.h"
  
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c

index eb50997dd369..aaed3969f204 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1585,7 +1585,7 @@ static void kvmgt_page_track_write(gpa_t gpa, const u8 
*val, int len,
  
  	mutex_lock(>vgpu_lock);
  
-	if (kvmgt_gfn_is_write_protected(info, gpa_to_gfn(gpa)))

+   if (kvmgt_gfn_is_write_protected(info, gpa >> PAGE_SHIFT))
intel_vgpu_page_track_handler(info, gpa,
 (void *)val, len);
  

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH v4 19/29] drm/i915/gvt: switch from ->track_flush_slot() to ->track_remove_region()

[Wang, Zhi A]

On 7/29/2023 4:35 AM, Sean Christopherson wrote:

From: Yan Zhao 

Switch from the poorly named and flawed ->track_flush_slot() to the newly
introduced ->track_remove_region().  From KVMGT's perspective, the two
hooks are functionally equivalent, the only difference being that
->track_remove_region() is called only when KVM is 100% certain the
memory region will be removed, i.e. is invoked slightly later in KVM's
memslot modification flow.

Cc: Zhenyu Wang 
Suggested-by: Sean Christopherson 
Signed-off-by: Yan Zhao 
[sean: handle name change, massage changelog, rebase]
Tested-by: Yan Zhao 
Tested-by: Yongwei Ma 
Signed-off-by: Sean Christopherson 
---
  drivers/gpu/drm/i915/gvt/kvmgt.c | 21 +
  1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 3ea3cb9eb599..3f2327455d85 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -108,9 +108,8 @@ struct gvt_dma {
  
  static void kvmgt_page_track_write(gpa_t gpa, const u8 *val, int len,

   struct kvm_page_track_notifier_node *node);
-static void kvmgt_page_track_flush_slot(struct kvm *kvm,
-   struct kvm_memory_slot *slot,
-   struct kvm_page_track_notifier_node *node);
+static void kvmgt_page_track_remove_region(gfn_t gfn, unsigned long nr_pages,
+  struct kvm_page_track_notifier_node 
*node);
  
  static ssize_t intel_vgpu_show_description(struct mdev_type *mtype, char *buf)

  {
@@ -666,7 +665,7 @@ static int intel_vgpu_open_device(struct vfio_device 
*vfio_dev)
return -EEXIST;
  
  	vgpu->track_node.track_write = kvmgt_page_track_write;

-   vgpu->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
+   vgpu->track_node.track_remove_region = kvmgt_page_track_remove_region;
kvm_get_kvm(vgpu->vfio_device.kvm);
kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
 >track_node);
@@ -1617,22 +1616,20 @@ static void kvmgt_page_track_write(gpa_t gpa, const u8 
*val, int len,
mutex_unlock(>vgpu_lock);
  }
  
-static void kvmgt_page_track_flush_slot(struct kvm *kvm,

-   struct kvm_memory_slot *slot,
-   struct kvm_page_track_notifier_node *node)
+static void kvmgt_page_track_remove_region(gfn_t gfn, unsigned long nr_pages,
+  struct kvm_page_track_notifier_node 
*node)
  {
unsigned long i;
-   gfn_t gfn;
struct intel_vgpu *info =
container_of(node, struct intel_vgpu, track_node);
  
  	mutex_lock(>vgpu_lock);
  
-	for (i = 0; i < slot->npages; i++) {

-   gfn = slot->base_gfn + i;
-   if (kvmgt_gfn_is_write_protected(info, gfn))
-   kvmgt_protect_table_del(info, gfn);
+   for (i = 0; i < nr_pages; i++) {
+   if (kvmgt_gfn_is_write_protected(info, gfn + i))
+   kvmgt_protect_table_del(info, gfn + i);
}
+
mutex_unlock(>vgpu_lock);
  }
  

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH v4 17/29] drm/i915/gvt: Don't bother removing write-protection on to-be-deleted slot

[Wang, Zhi A]

On 7/29/2023 4:35 AM, Sean Christopherson wrote:

When handling a slot "flush", don't call back into KVM to drop write
protection for gfns in the slot.  Now that KVM rejects attempts to move
memory slots while KVMGT is attached, the only time a slot is "flushed"
is when it's being removed, i.e. the memslot and all its write-tracking
metadata is about to be deleted.

Reviewed-by: Yan Zhao 
Tested-by: Yongwei Ma 
Signed-off-by: Sean Christopherson 
---
  drivers/gpu/drm/i915/gvt/kvmgt.c | 8 +---
  1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index e9276500435d..3ea3cb9eb599 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1630,14 +1630,8 @@ static void kvmgt_page_track_flush_slot(struct kvm *kvm,
  
  	for (i = 0; i < slot->npages; i++) {

gfn = slot->base_gfn + i;
-   if (kvmgt_gfn_is_write_protected(info, gfn)) {
-   write_lock(>mmu_lock);
-   kvm_slot_page_track_remove_page(kvm, slot, gfn,
-   KVM_PAGE_TRACK_WRITE);
-   write_unlock(>mmu_lock);
-
+   if (kvmgt_gfn_is_write_protected(info, gfn))
kvmgt_protect_table_del(info, gfn);
-   }
}
mutex_unlock(>vgpu_lock);
  }

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH v4 15/29] KVM: drm/i915/gvt: Drop @vcpu from KVM's ->track_write() hook

[Wang, Zhi A]

On 7/29/2023 4:35 AM, Sean Christopherson wrote:

Drop @vcpu from KVM's ->track_write() hook provided for external users of
the page-track APIs now that KVM itself doesn't use the page-track
mechanism.

Reviewed-by: Yan Zhao 
Tested-by: Yongwei Ma 
Signed-off-by: Sean Christopherson 
---
  arch/x86/include/asm/kvm_page_track.h |  5 ++---
  arch/x86/kvm/mmu/page_track.c |  2 +-
  drivers/gpu/drm/i915/gvt/kvmgt.c  | 10 --
  3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/kvm_page_track.h 
b/arch/x86/include/asm/kvm_page_track.h
index eb186bc57f6a..8c4d216e3b2b 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -26,14 +26,13 @@ struct kvm_page_track_notifier_node {
 * It is called when guest is writing the write-tracked page
 * and write emulation is finished at that time.
 *
-* @vcpu: the vcpu where the write access happened.
 * @gpa: the physical address written by guest.
 * @new: the data was written to the address.
 * @bytes: the written length.
 * @node: this node
 */
-   void (*track_write)(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
-   int bytes, struct kvm_page_track_notifier_node 
*node);
+   void (*track_write)(gpa_t gpa, const u8 *new, int bytes,
+   struct kvm_page_track_notifier_node *node);
/*
 * It is called when memory slot is being moved or removed
 * users can drop write-protection for the pages in that memory slot
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index 23088c90d2fd..891e5cc52b45 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -272,7 +272,7 @@ void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, 
const u8 *new,
hlist_for_each_entry_srcu(n, >track_notifier_list, node,
srcu_read_lock_held(>track_srcu))
if (n->track_write)
-   n->track_write(vcpu, gpa, new, bytes, n);
+   n->track_write(gpa, new, bytes, n);
srcu_read_unlock(>track_srcu, idx);
  
  	kvm_mmu_track_write(vcpu, gpa, new, bytes);

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 034be0655daa..e9276500435d 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -106,9 +106,8 @@ struct gvt_dma {
  #define vfio_dev_to_vgpu(vfio_dev) \
container_of((vfio_dev), struct intel_vgpu, vfio_device)
  
-static void kvmgt_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa,

-   const u8 *val, int len,
-   struct kvm_page_track_notifier_node *node);
+static void kvmgt_page_track_write(gpa_t gpa, const u8 *val, int len,
+  struct kvm_page_track_notifier_node *node);
  static void kvmgt_page_track_flush_slot(struct kvm *kvm,
struct kvm_memory_slot *slot,
struct kvm_page_track_notifier_node *node);
@@ -1603,9 +1602,8 @@ int intel_gvt_page_track_remove(struct intel_vgpu *info, 
u64 gfn)
return 0;
  }
  
-static void kvmgt_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa,

-   const u8 *val, int len,
-   struct kvm_page_track_notifier_node *node)
+static void kvmgt_page_track_write(gpa_t gpa, const u8 *val, int len,
+  struct kvm_page_track_notifier_node *node)
  {
struct intel_vgpu *info =
container_of(node, struct intel_vgpu, track_node);

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH v4 11/29] drm/i915/gvt: Protect gfn hash table with vgpu_lock

[Wang, Zhi A]

On 7/29/2023 4:35 AM, Sean Christopherson wrote:

Use vgpu_lock instead of KVM's mmu_lock to protect accesses to the hash
table used to track which gfns are write-protected when shadowing the
guest's GTT, and hoist the acquisition of vgpu_lock from
intel_vgpu_page_track_handler() out to its sole caller,
kvmgt_page_track_write().

This fixes a bug where kvmgt_page_track_write(), which doesn't hold
kvm->mmu_lock, could race with intel_gvt_page_track_remove() and trigger
a use-after-free.

Fixing kvmgt_page_track_write() by taking kvm->mmu_lock is not an option
as mmu_lock is a r/w spinlock, and intel_vgpu_page_track_handler() might
sleep when acquiring vgpu->cache_lock deep down the callstack:

   intel_vgpu_page_track_handler()
   |
   |->  page_track->handler / ppgtt_write_protection_handler()
|
|-> ppgtt_handle_guest_write_page_table_bytes()
|
|->  ppgtt_handle_guest_write_page_table()
 |
 |-> ppgtt_handle_guest_entry_removal()
 |
 |-> ppgtt_invalidate_pte()
 |
 |-> intel_gvt_dma_unmap_guest_page()
 |
 |-> mutex_lock(>cache_lock);

Reviewed-by: Yan Zhao 
Tested-by: Yongwei Ma 
Signed-off-by: Sean Christopherson 
---
  drivers/gpu/drm/i915/gvt/kvmgt.c  | 55 +++
  drivers/gpu/drm/i915/gvt/page_track.c | 10 +
  2 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 6f52886c4051..034be0655daa 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -352,6 +352,8 @@ __kvmgt_protect_table_find(struct intel_vgpu *info, gfn_t 
gfn)
  {
struct kvmgt_pgfn *p, *res = NULL;
  
+	lockdep_assert_held(>vgpu_lock);

+
hash_for_each_possible(info->ptable, p, hnode, gfn) {
if (gfn == p->gfn) {
res = p;
@@ -1553,6 +1555,9 @@ int intel_gvt_page_track_add(struct intel_vgpu *info, u64 
gfn)
if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, info->status))
return -ESRCH;
  
+	if (kvmgt_gfn_is_write_protected(info, gfn))

+   return 0;
+
idx = srcu_read_lock(>srcu);
slot = gfn_to_memslot(kvm, gfn);
if (!slot) {
@@ -1561,16 +1566,12 @@ int intel_gvt_page_track_add(struct intel_vgpu *info, 
u64 gfn)
}
  
  	write_lock(>mmu_lock);

-
-   if (kvmgt_gfn_is_write_protected(info, gfn))
-   goto out;
-
kvm_slot_page_track_add_page(kvm, slot, gfn, KVM_PAGE_TRACK_WRITE);
+   write_unlock(>mmu_lock);
+
+   srcu_read_unlock(>srcu, idx);
+
kvmgt_protect_table_add(info, gfn);
-
-out:
-   write_unlock(>mmu_lock);
-   srcu_read_unlock(>srcu, idx);
return 0;
  }
  
@@ -1583,24 +1584,22 @@ int intel_gvt_page_track_remove(struct intel_vgpu *info, u64 gfn)

if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, info->status))
return -ESRCH;
  
-	idx = srcu_read_lock(>srcu);

-   slot = gfn_to_memslot(kvm, gfn);
-   if (!slot) {
-   srcu_read_unlock(>srcu, idx);
-   return -EINVAL;
-   }
-
-   write_lock(>mmu_lock);
-
if (!kvmgt_gfn_is_write_protected(info, gfn))
-   goto out;
+   return 0;
  
+	idx = srcu_read_lock(>srcu);

+   slot = gfn_to_memslot(kvm, gfn);
+   if (!slot) {
+   srcu_read_unlock(>srcu, idx);
+   return -EINVAL;
+   }
+
+   write_lock(>mmu_lock);
kvm_slot_page_track_remove_page(kvm, slot, gfn, KVM_PAGE_TRACK_WRITE);
+   write_unlock(>mmu_lock);
+   srcu_read_unlock(>srcu, idx);
+
kvmgt_protect_table_del(info, gfn);
-
-out:
-   write_unlock(>mmu_lock);
-   srcu_read_unlock(>srcu, idx);
return 0;
  }
  
@@ -1611,9 +1610,13 @@ static void kvmgt_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa,

struct intel_vgpu *info =
container_of(node, struct intel_vgpu, track_node);
  
+	mutex_lock(>vgpu_lock);

+
if (kvmgt_gfn_is_write_protected(info, gpa_to_gfn(gpa)))
intel_vgpu_page_track_handler(info, gpa,
 (void *)val, len);
+
+   mutex_unlock(>vgpu_lock);
  }
  
  static void kvmgt_page_track_flush_slot(struct kvm *kvm,

@@ -1625,16 +1628,20 @@ static void kvmgt_page_track_flush_slot(struct kvm *kvm,
struct intel_vgpu *info =
container_of(node, struct intel_vgpu, track_node);
  
-	write_lock(>mmu_lock);

+   mutex_lock(>vgpu_lock);
+
for (i = 0; i < slot->npages; i++) {
gfn = slot->base_gfn + i;
if (kvmgt_gfn_is_write_protected(info, gfn)) {
+   write_lock(>mmu_lock);
kvm_slot_page_track_remove_page(kvm, slot, gfn,

Re: [Intel-gfx] [PATCH v4 10/29] drm/i915/gvt: Drop unused helper intel_vgpu_reset_gtt()

[Wang, Zhi A]

On 7/29/2023 4:35 AM, Sean Christopherson wrote:

Drop intel_vgpu_reset_gtt() as it no longer has any callers.  In addition
to eliminating dead code, this eliminates the last possible scenario where
__kvmgt_protect_table_find() can be reached without holding vgpu_lock.
Requiring vgpu_lock to be held when calling __kvmgt_protect_table_find()
will allow a protecting the gfn hash with vgpu_lock without too much fuss.

No functional change intended.

Fixes: ba25d977571e ("drm/i915/gvt: Do not destroy ppgtt_mm during vGPU 
D3->D0.")
Reviewed-by: Yan Zhao 
Tested-by: Yongwei Ma 
Signed-off-by: Sean Christopherson 
---
  drivers/gpu/drm/i915/gvt/gtt.c | 18 --
  drivers/gpu/drm/i915/gvt/gtt.h |  1 -
  2 files changed, 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index f505be9e647a..c3c623b929ce 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -2817,24 +2817,6 @@ void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu, bool 
invalidate_old)
ggtt_invalidate(gvt->gt);
  }
  
-/**

- * intel_vgpu_reset_gtt - reset the all GTT related status
- * @vgpu: a vGPU
- *
- * This function is called from vfio core to reset reset all
- * GTT related status, including GGTT, PPGTT, scratch page.
- *
- */
-void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu)
-{
-   /* Shadow pages are only created when there is no page
-* table tracking data, so remove page tracking data after
-* removing the shadow pages.
-*/
-   intel_vgpu_destroy_all_ppgtt_mm(vgpu);
-   intel_vgpu_reset_ggtt(vgpu, true);
-}
-
  /**
   * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
   * @gvt: intel gvt device
diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
index a3b0f59ec8bd..4cb183e06e95 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.h
+++ b/drivers/gpu/drm/i915/gvt/gtt.h
@@ -224,7 +224,6 @@ void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu, bool 
invalidate_old);
  void intel_vgpu_invalidate_ppgtt(struct intel_vgpu *vgpu);
  
  int intel_gvt_init_gtt(struct intel_gvt *gvt);

-void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu);
  void intel_gvt_clean_gtt(struct intel_gvt *gvt);
  
  struct intel_vgpu_mm *intel_gvt_find_ppgtt_mm(struct intel_vgpu *vgpu,


Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH v4 09/29] drm/i915/gvt: Use an "unsigned long" to iterate over memslot gfns

[Wang, Zhi A]

On 7/29/2023 4:35 AM, Sean Christopherson wrote:

Use an "unsigned long" instead of an "int" when iterating over the gfns
in a memslot.  The number of pages in the memslot is tracked as an
"unsigned long", e.g. KVMGT could theoretically break if a KVM memslot
larger than 16TiB were deleted (2^32 * 4KiB).

Reviewed-by: Yan Zhao 
Tested-by: Yongwei Ma 
Signed-off-by: Sean Christopherson 
---
  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 97c6d3c53710..6f52886c4051 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1620,7 +1620,7 @@ static void kvmgt_page_track_flush_slot(struct kvm *kvm,
struct kvm_memory_slot *slot,
struct kvm_page_track_notifier_node *node)
  {
-   int i;
+   unsigned long i;
gfn_t gfn;
struct intel_vgpu *info =
container_of(node, struct intel_vgpu, track_node);


Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH v4 08/29] drm/i915/gvt: Don't rely on KVM's gfn_to_pfn() to query possible 2M GTT

[Wang, Zhi A]

On 7/29/2023 4:35 AM, Sean Christopherson wrote:

Now that gvt_pin_guest_page() explicitly verifies the pinned PFN is a
transparent hugepage page, don't use KVM's gfn_to_pfn() to pre-check if a
2MiB GTT entry is possible and instead just try to map the GFN with a 2MiB
entry.  Using KVM to query pfn that is ultimately managed through VFIO is
odd, and KVM's gfn_to_pfn() is not intended for non-KVM consumption; it's
exported only because of KVM vendor modules (x86 and PPC).

Open code the check on 2MiB support instead of keeping
is_2MB_gtt_possible() around for a single line of code.

Move the call to intel_gvt_dma_map_guest_page() for a 4KiB entry into its
case statement, i.e. fork the common path into the 4KiB and 2MiB "direct"
shadow paths.  Keeping the call in the "common" path is arguably more in
the spirit of "one change per patch", but retaining the local "page_size"
variable is silly, i.e. the call site will be changed either way, and
jumping around the no-longer-common code is more subtle and rather odd,
i.e. would just need to be immediately cleaned up.

Drop the error message from gvt_pin_guest_page() when KVMGT attempts to
shadow a 2MiB guest page that isn't backed by a compatible hugepage in the
host.  Dropping the pre-check on a THP makes it much more likely that the
"error" will be encountered in normal operation.

Reviewed-by: Yan Zhao 
Tested-by: Yan Zhao 
Tested-by: Yongwei Ma 
Signed-off-by: Sean Christopherson 
---
  drivers/gpu/drm/i915/gvt/gtt.c   | 49 ++--
  drivers/gpu/drm/i915/gvt/kvmgt.c |  1 -
  2 files changed, 8 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 61e38acee2d5..f505be9e647a 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -1145,36 +1145,6 @@ static inline void ppgtt_generate_shadow_entry(struct 
intel_gvt_gtt_entry *se,
ops->set_pfn(se, s->shadow_page.mfn);
  }
  
-/*

- * Check if can do 2M page
- * @vgpu: target vgpu
- * @entry: target pfn's gtt entry
- *
- * Return 1 if 2MB huge gtt shadowing is possible, 0 if miscondition,
- * negative if found err.
- */
-static int is_2MB_gtt_possible(struct intel_vgpu *vgpu,
-   struct intel_gvt_gtt_entry *entry)
-{
-   const struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
-   kvm_pfn_t pfn;
-   int ret;
-
-   if (!HAS_PAGE_SIZES(vgpu->gvt->gt->i915, I915_GTT_PAGE_SIZE_2M))
-   return 0;
-
-   pfn = gfn_to_pfn(vgpu->vfio_device.kvm, ops->get_pfn(entry));
-   if (is_error_noslot_pfn(pfn))
-   return -EINVAL;
-
-   if (!pfn_valid(pfn))
-   return -EINVAL;
-
-   ret = PageTransHuge(pfn_to_page(pfn));
-   kvm_release_pfn_clean(pfn);
-   return ret;
-}
-
  static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
struct intel_vgpu_ppgtt_spt *spt, unsigned long index,
struct intel_gvt_gtt_entry *se)
@@ -1268,7 +1238,7 @@ static int ppgtt_populate_shadow_entry(struct intel_vgpu 
*vgpu,
  {
const struct intel_gvt_gtt_pte_ops *pte_ops = vgpu->gvt->gtt.pte_ops;
struct intel_gvt_gtt_entry se = *ge;
-   unsigned long gfn, page_size = PAGE_SIZE;
+   unsigned long gfn;
dma_addr_t dma_addr;
int ret;
  
@@ -1283,6 +1253,9 @@ static int ppgtt_populate_shadow_entry(struct intel_vgpu *vgpu,

switch (ge->type) {
case GTT_TYPE_PPGTT_PTE_4K_ENTRY:
gvt_vdbg_mm("shadow 4K gtt entry\n");
+   ret = intel_gvt_dma_map_guest_page(vgpu, gfn, PAGE_SIZE, 
_addr);
+   if (ret)
+   return -ENXIO;
break;
case GTT_TYPE_PPGTT_PTE_64K_ENTRY:
gvt_vdbg_mm("shadow 64K gtt entry\n");
@@ -1294,12 +1267,10 @@ static int ppgtt_populate_shadow_entry(struct 
intel_vgpu *vgpu,
return split_64KB_gtt_entry(vgpu, spt, index, );
case GTT_TYPE_PPGTT_PTE_2M_ENTRY:
gvt_vdbg_mm("shadow 2M gtt entry\n");
-   ret = is_2MB_gtt_possible(vgpu, ge);
-   if (ret == 0)
+   if (!HAS_PAGE_SIZES(vgpu->gvt->gt->i915, I915_GTT_PAGE_SIZE_2M) 
||
+   intel_gvt_dma_map_guest_page(vgpu, gfn,
+I915_GTT_PAGE_SIZE_2M, 
_addr))
return split_2MB_gtt_entry(vgpu, spt, index, );
-   else if (ret < 0)
-   return ret;
-   page_size = I915_GTT_PAGE_SIZE_2M;
break;
case GTT_TYPE_PPGTT_PTE_1G_ENTRY:
gvt_vgpu_err("GVT doesn't support 1GB entry\n");
@@ -1309,11 +1280,7 @@ static int ppgtt_populate_shadow_entry(struct intel_vgpu 
*vgpu,
return -EINVAL;
}
  
-	/* direct shadow */

-   ret = intel_gvt_dma_map_guest_page(vgpu, gfn, page_size, _addr);
-   if (ret)
-   return -ENXIO;
-
+   /* Successfully shadowed a 4K or 2M page 

Re: [Intel-gfx] [PATCH v4 05/29] drm/i915/gvt: Put the page reference obtained by KVM's gfn_to_pfn()

[Wang, Zhi A]

On 7/29/2023 4:35 AM, Sean Christopherson wrote:

Put the struct page reference acquired by gfn_to_pfn(), KVM's API is that
the caller is ultimately responsible for dropping any reference.

Note, kvm_release_pfn_clean() ensures the pfn is actually a refcounted
struct page before trying to put any references.

Fixes: b901b252b6cf ("drm/i915/gvt: Add 2M huge gtt support")
Reviewed-by: Yan Zhao 
Tested-by: Yongwei Ma 
Signed-off-by: Sean Christopherson 
---
  drivers/gpu/drm/i915/gvt/gtt.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index f30922c55a0c..5426a27c1b71 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -1158,6 +1158,7 @@ static int is_2MB_gtt_possible(struct intel_vgpu *vgpu,
  {
const struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
kvm_pfn_t pfn;
+   int ret;
  
  	if (!HAS_PAGE_SIZES(vgpu->gvt->gt->i915, I915_GTT_PAGE_SIZE_2M))

return 0;
@@ -1171,7 +1172,9 @@ static int is_2MB_gtt_possible(struct intel_vgpu *vgpu,
if (!pfn_valid(pfn))
return -EINVAL;
  
-	return PageTransHuge(pfn_to_page(pfn));

+   ret = PageTransHuge(pfn_to_page(pfn));
+   kvm_release_pfn_clean(pfn);
+   return ret;
  }
  
  static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,


Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH v4 03/29] drm/i915/gvt: Verify hugepages are contiguous in physical address space

[Wang, Zhi A]

On 8/1/2023 4:47 AM, Yan Zhao wrote:

Reviewed-by: Yan Zhao 

On Fri, Jul 28, 2023 at 06:35:09PM -0700, Sean Christopherson wrote:

When shadowing a GTT entry with a 2M page, verify that the pfns are
contiguous, not just that the struct page pointers are contiguous.  The
memory map is virtual contiguous if "CONFIG_FLATMEM=y ||
CONFIG_SPARSEMEM_VMEMMAP=y", but not for "CONFIG_SPARSEMEM=y &&
CONFIG_SPARSEMEM_VMEMMAP=n", so theoretically KVMGT could encounter struct
pages that are virtually contiguous, but not physically contiguous.

In practice, this flaw is likely a non-issue as it would cause functional
problems iff a section isn't 2M aligned _and_ is directly adjacent to
another section with discontiguous pfns.

Tested-by: Yongwei Ma 
Signed-off-by: Sean Christopherson 
---
  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 de675d799c7d..429f0f993a13 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -161,7 +161,7 @@ static int gvt_pin_guest_page(struct intel_vgpu *vgpu, 
unsigned long gfn,
  
  		if (npage == 0)

base_page = cur_page;
-   else if (base_page + npage != cur_page) {
+   else if (page_to_pfn(base_page) + npage != 
page_to_pfn(cur_page)) {
gvt_vgpu_err("The pages are not continuous\n");
ret = -EINVAL;
npage++;
--
2.41.0.487.g6d72f3e995-goog


Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH v4 01/29] drm/i915/gvt: Verify pfn is "valid" before dereferencing "struct page"

[Wang, Zhi A]

On 7/29/2023 4:35 AM, Sean Christopherson wrote:

Check that the pfn found by gfn_to_pfn() is actually backed by "struct
page" memory prior to retrieving and dereferencing the page.  KVM
supports backing guest memory with VM_PFNMAP, VM_IO, etc., and so
there is no guarantee the pfn returned by gfn_to_pfn() has an associated
"struct page".

Fixes: b901b252b6cf ("drm/i915/gvt: Add 2M huge gtt support")
Reviewed-by: Yan Zhao 
Tested-by: Yongwei Ma 
Signed-off-by: Sean Christopherson 
---
  drivers/gpu/drm/i915/gvt/gtt.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 4ec85308379a..58b9b316ae46 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -1183,6 +1183,10 @@ static int is_2MB_gtt_possible(struct intel_vgpu *vgpu,
pfn = gfn_to_pfn(vgpu->vfio_device.kvm, ops->get_pfn(entry));
if (is_error_noslot_pfn(pfn))
return -EINVAL;
+
+   if (!pfn_valid(pfn))
+   return -EINVAL;
+
return PageTransHuge(pfn_to_page(pfn));
  }
  


Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH v4 04/29] drm/i915/gvt: Don't try to unpin an empty page range

[Wang, Zhi A]

On 7/29/2023 4:35 AM, Sean Christopherson wrote:

From: Yan Zhao 

Attempt to unpin pages in the error path of gvt_pin_guest_page() if and
only if at least one page was successfully pinned.  Unpinning doesn't
cause functional problems, but vfio_device_container_unpin_pages()
rightfully warns about being asked to unpin zero pages.

Signed-off-by: Yan Zhao 
[sean: write changelog]
Signed-off-by: Sean Christopherson 
---
  drivers/gpu/drm/i915/gvt/kvmgt.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 429f0f993a13..0366a699baf5 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -172,7 +172,8 @@ static int gvt_pin_guest_page(struct intel_vgpu *vgpu, 
unsigned long gfn,
*page = base_page;
return 0;
  err:
-   gvt_unpin_guest_page(vgpu, gfn, npage * PAGE_SIZE);
+   if (npage)
+   gvt_unpin_guest_page(vgpu, gfn, npage * PAGE_SIZE);
return ret;
  }


Reviewed-by: Zhi Wang 

Re: [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gvt: remove unused variable gma_bottom in command parser (rev2)

[Wang, Zhi A]

Thanks so much for the trick. Learned.

-Original Message-
From: Jani Nikula  
Sent: Thursday, June 8, 2023 5:01 PM
To: Wang, Zhi A ; Zhi Wang 
Cc: intel-gfx@lists.freedesktop.org
Subject: RE: [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gvt: remove 
unused variable gma_bottom in command parser (rev2)

On Thu, 08 Jun 2023, "Wang, Zhi A"  wrote:
> Please take the  as my S-O-B since this is a 
> patch about GVT-g. I still don't know why my intel email smtp doesn't 
> work.

Thanks, pushed.

You should be able to configure git to set your author From: in the patch, even 
when sending the patch email with a different From:.

See e.g. [1].

BR,
Jani.


[1] 
https://lore.kernel.org/r/20230606191504.18099-2-ville.syrj...@linux.intel.com


--
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gvt: remove unused variable gma_bottom in command parser (rev2)

[Wang, Zhi A]

Hi Jani:

Please take the  as my S-O-B since this is a patch about 
GVT-g. I still don't know why my intel email smtp doesn't work.

Thanks,
Zhi.

-Original Message-
From: Intel-gfx  On Behalf Of Jani 
Nikula
Sent: Monday, June 5, 2023 10:36 PM
To: Zhi Wang 
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gvt: remove 
unused variable gma_bottom in command parser (rev2)

On Fri, 02 Jun 2023, Jani Nikula  wrote:
> On Wed, 31 May 2023, Patchwork  wrote:
>> == Series Details ==
>>
>> Series: drm/i915/gvt: remove unused variable gma_bottom in command parser 
>> (rev2)
>> URL   : https://patchwork.freedesktop.org/series/118512/
>> State : warning
>>
>> == Summary ==
>>
>> Error: dim checkpatch failed
>> c6878ab01be9 drm/i915/gvt: remove unused variable gma_bottom in 
>> command parser
>> -:63: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address 
>> mismatch: 'From: Zhi Wang ' != 'Signed-off-by: Zhi 
>> Wang '
>
> I can fix this while applying, but please indicate whether you 
> intended to have From: Zhi Wang  or 
> Signed-off-by: Zhi Wang .

Ping. I can't apply this with this warning, and I can't fix Signed-off-by 
unless you tell me what to do.


BR,
Jani.


--
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 14/15] drm/i915/gvt: annotate maybe unused gma_bottom variables

[Wang, Zhi A]

On 5/27/2023 12:38 AM, Jani Nikula wrote:

Prepare for re-enabling -Wunused-but-set-variable.

Lacking a better idea, annotate the gma_bottom variables with
__maybe_unused.

Cc: Zhenyu Wang 
Cc: Zhi Wang 
Cc: intel-gvt-...@lists.freedesktop.org
Signed-off-by: Jani Nikula 

---

Frankly I'm not sure what to do with these. Maybe the variables should
be dropped altogether?


I sent an patch to fix the warnings. You can include that one in the 
series. Zhenyu, can you give an rb?



---
  drivers/gpu/drm/i915/gvt/cmd_parser.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c 
b/drivers/gpu/drm/i915/gvt/cmd_parser.c
index 3c4ae1da0d41..2801e17e5522 100644
--- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
+++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
@@ -2833,7 +2833,7 @@ static int command_scan(struct parser_exec_state *s,
  
  static int scan_workload(struct intel_vgpu_workload *workload)

  {
-   unsigned long gma_head, gma_tail, gma_bottom;
+   unsigned long gma_head, gma_tail, __maybe_unused gma_bottom;
struct parser_exec_state s;
int ret = 0;
  
@@ -2874,7 +2874,7 @@ static int scan_workload(struct intel_vgpu_workload *workload)

  static int scan_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx)
  {
  
-	unsigned long gma_head, gma_tail, gma_bottom, ring_size, ring_tail;

+   unsigned long gma_head, gma_tail, __maybe_unused gma_bottom, ring_size, 
ring_tail;
struct parser_exec_state s;
int ret = 0;
struct intel_vgpu_workload *workload = container_of(wa_ctx,

[Intel-gfx] [PATCH] drm/i915/gvt: remove unused variable gma_bottom in command parser

[Wang, Zhi A]

Remove unused variable gma_bottom in scan_workload() and scan_wa_ctx().
commit be1da7070aea ("drm/i915/gvt: vGPU command scanner") introduces
gma_bottom in several functions to calculate the size of the command
buffer. However, some of them are set but actually unused.

When compiling the code with ccflags -Wunused-but-set-variable, gcc
throws warnings.

Remove unused variables to avoid the gcc warnings. Tested via compiling
the code with ccflags -Wunused-but-set-variable.

Fixes: be1da7070aea ("drm/i915/gvt: vGPU command scanner")
Suggested-by: Jani Nikula 
Cc: Zhenyu Wang 
Cc: intel-gvt-...@lists.freedesktop.org
Signed-off-by: Zhi Wang 
---
 drivers/gpu/drm/i915/gvt/cmd_parser.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c 
b/drivers/gpu/drm/i915/gvt/cmd_parser.c

index 3c4ae1da0d41..05f9348b7a9d 100644
--- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
+++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
@@ -2833,7 +2833,7 @@ static int command_scan(struct parser_exec_state *s,

 static int scan_workload(struct intel_vgpu_workload *workload)
 {
-   unsigned long gma_head, gma_tail, gma_bottom;
+   unsigned long gma_head, gma_tail;
struct parser_exec_state s;
int ret = 0;

@@ -2843,7 +2843,6 @@ static int scan_workload(struct 
intel_vgpu_workload *workload)


gma_head = workload->rb_start + workload->rb_head;
gma_tail = workload->rb_start + workload->rb_tail;
-   gma_bottom = workload->rb_start +  _RING_CTL_BUF_SIZE(workload->rb_ctl);

s.buf_type = RING_BUFFER_INSTRUCTION;
s.buf_addr_type = GTT_BUFFER;
@@ -2874,7 +2873,7 @@ static int scan_workload(struct 
intel_vgpu_workload *workload)

 static int scan_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx)
 {

-   unsigned long gma_head, gma_tail, gma_bottom, ring_size, ring_tail;
+   unsigned long gma_head, gma_tail, ring_size, ring_tail;
struct parser_exec_state s;
int ret = 0;
struct intel_vgpu_workload *workload = container_of(wa_ctx,
@@ -2891,7 +2890,6 @@ static int scan_wa_ctx(struct intel_shadow_wa_ctx 
*wa_ctx)

PAGE_SIZE);
gma_head = wa_ctx->indirect_ctx.guest_gma;
gma_tail = wa_ctx->indirect_ctx.guest_gma + ring_tail;
-   gma_bottom = wa_ctx->indirect_ctx.guest_gma + ring_size;

s.buf_type = RING_BUFFER_INSTRUCTION;
s.buf_addr_type = GTT_BUFFER;
--
2.25.1

Re: [Intel-gfx] [PATCH v3 06/28] drm/i915/gvt: Error out on an attempt to shadowing an unknown GTT entry type

[Wang, Zhi A]

On 5/13/2023 8:35 AM, Sean Christopherson wrote:

Reviewed-by: Zhi Wang 

> Bail from ppgtt_populate_shadow_entry() if an unexpected GTT entry type
> is encountered instead of subtly falling through to the common "direct
> shadow" path.  Eliminating the default/error path's reliance on the common
> handling will allow hoisting intel_gvt_dma_map_guest_page() into the case
> statements so that the 2MiB case can try intel_gvt_dma_map_guest_page()
> and fallback to splitting the entry on failure.
> 
> Signed-off-by: Sean Christopherson 
> ---
>   drivers/gpu/drm/i915/gvt/gtt.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 2aed31b497c9..61e38acee2d5 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1306,6 +1306,7 @@ static int ppgtt_populate_shadow_entry(struct 
> intel_vgpu *vgpu,
>   return -EINVAL;
>   default:
>   GEM_BUG_ON(1);
> + return -EINVAL;
>   }
>   
>   /* direct shadow */

Re: [Intel-gfx] [PATCH v3 05/28] drm/i915/gvt: Explicitly check that vGPU is attached before shadowing

[Wang, Zhi A]

On 5/13/2023 8:35 AM, Sean Christopherson wrote:
> Move the check that a vGPU is attacked from is_2MB_gtt_possible() to its
> sole caller, ppgtt_populate_shadow_entry().  All of the paths in
> ppgtt_populate_shadow_entry() eventually check for attachment by way of
> intel_gvt_dma_map_guest_page(), but explicitly checking can avoid
> unnecessary work and will make it more obvious that a future cleanup of
> is_2MB_gtt_possible() isn't introducing a bug.
> 

It might be better move this check to shadow_ppgtt_mm() which is used
in both shadow page table creation and pinning path so that the path
can bail out even earlier when creating a shadow page table but a vGPU
has not been attached to KVM yet.

> Signed-off-by: Sean Christopherson 
> ---
>   drivers/gpu/drm/i915/gvt/gtt.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 5426a27c1b71..2aed31b497c9 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1163,8 +1163,6 @@ static int is_2MB_gtt_possible(struct intel_vgpu *vgpu,
>   if (!HAS_PAGE_SIZES(vgpu->gvt->gt->i915, I915_GTT_PAGE_SIZE_2M))
>   return 0;
>   
> - if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status))
> - return -EINVAL;
>   pfn = gfn_to_pfn(vgpu->vfio_device.kvm, ops->get_pfn(entry));
>   if (is_error_noslot_pfn(pfn))
>   return -EINVAL;
> @@ -1277,6 +1275,9 @@ static int ppgtt_populate_shadow_entry(struct 
> intel_vgpu *vgpu,
>   if (!pte_ops->test_present(ge))
>   return 0;
>   
> + if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status))
> + return -EINVAL;
> +
>   gfn = pte_ops->get_pfn(ge);
>   
>   switch (ge->type) {

Re: [Intel-gfx] [PATCH v3 02/28] drm/i915/gvt: remove interface intel_gvt_is_valid_gfn

[Wang, Zhi A]

On 5/13/2023 8:35 AM, Sean Christopherson wrote:
> From: Yan Zhao 
> 

Acked-by: Zhi Wang 

This was previously to avoid stepping down to the lower level ASAP
when a guest page is not used but still stays in the GPU page table.
(mostly in windows VM, as this is a behavior of WDDM GPU MM). But
it doesn't totally solve the the trapping flood in practice. I am
totally fine that it is removed. Post shadow in the trapping
path might be extended to handle this problem.

> Currently intel_gvt_is_valid_gfn() is called in two places:
> (1) shadowing guest GGTT entry
> (2) shadowing guest PPGTT leaf entry,
> which was introduced in commit cc753fbe1ac4
> ("drm/i915/gvt: validate gfn before set shadow page entry").
> 
> However, now it's not necessary to call this interface any more, because
> a. GGTT partial write issue has been fixed by
> commit bc0686ff5fad
> ("drm/i915/gvt: support inconsecutive partial gtt entry write")
> commit 510fe10b6180
> ("drm/i915/gvt: fix a bug of partially write ggtt enties")
> b. PPGTT resides in normal guest RAM and we only treat 8-byte writes
> as valid page table writes. Any invalid GPA found is regarded as
> an error, either due to guest misbehavior/attack or bug in host
> shadow code.
> So,rather than do GFN pre-checking and replace invalid GFNs with
> scratch GFN and continue silently, just remove the pre-checking and
> abort PPGTT shadowing on error detected.
> c. GFN validity check is still performed in
> intel_gvt_dma_map_guest_page() --> gvt_pin_guest_page().
> It's more desirable to call VFIO interface to do both validity check
> and mapping.
> Calling intel_gvt_is_valid_gfn() to do GFN validity check from KVM side
> while later mapping the GFN through VFIO interface is unnecessarily
> fragile and confusing for unaware readers.
> 
> Signed-off-by: Yan Zhao 
> [sean: remove now-unused local variables]
> Signed-off-by: Sean Christopherson 
> ---
>   drivers/gpu/drm/i915/gvt/gtt.c | 36 +-
>   1 file changed, 1 insertion(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 58b9b316ae46..f30922c55a0c 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -49,22 +49,6 @@
>   static bool enable_out_of_sync = false;
>   static int preallocated_oos_pages = 8192;
>   
> -static bool intel_gvt_is_valid_gfn(struct intel_vgpu *vgpu, unsigned long 
> gfn)
> -{
> - struct kvm *kvm = vgpu->vfio_device.kvm;
> - int idx;
> - bool ret;
> -
> - if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status))
> - return false;
> -
> - idx = srcu_read_lock(>srcu);
> - ret = kvm_is_visible_gfn(kvm, gfn);
> - srcu_read_unlock(>srcu, idx);
> -
> - return ret;
> -}
> -
>   /*
>* validate a gm address and related range size,
>* translate it to host gm address
> @@ -1333,11 +1317,9 @@ static int ppgtt_populate_shadow_entry(struct 
> intel_vgpu *vgpu,
>   static int ppgtt_populate_spt(struct intel_vgpu_ppgtt_spt *spt)
>   {
>   struct intel_vgpu *vgpu = spt->vgpu;
> - struct intel_gvt *gvt = vgpu->gvt;
> - const struct intel_gvt_gtt_pte_ops *ops = gvt->gtt.pte_ops;
>   struct intel_vgpu_ppgtt_spt *s;
>   struct intel_gvt_gtt_entry se, ge;
> - unsigned long gfn, i;
> + unsigned long i;
>   int ret;
>   
>   trace_spt_change(spt->vgpu->id, "born", spt,
> @@ -1354,13 +1336,6 @@ static int ppgtt_populate_spt(struct 
> intel_vgpu_ppgtt_spt *spt)
>   ppgtt_generate_shadow_entry(, s, );
>   ppgtt_set_shadow_entry(spt, , i);
>   } else {
> - gfn = ops->get_pfn();
> - if (!intel_gvt_is_valid_gfn(vgpu, gfn)) {
> - ops->set_pfn(, gvt->gtt.scratch_mfn);
> - ppgtt_set_shadow_entry(spt, , i);
> - continue;
> - }
> -
>   ret = ppgtt_populate_shadow_entry(vgpu, spt, i, );
>   if (ret)
>   goto fail;
> @@ -2335,14 +2310,6 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu 
> *vgpu, unsigned int off,
>   m.val64 = e.val64;
>   m.type = e.type;
>   
> - /* one PTE update may be issued in multiple writes and the
> -  * first write may not construct a valid gfn
> -  */
> - if (!intel_gvt_is_valid_gfn(vgpu, gfn)) {
> - ops->set_pfn(, gvt->gtt.scratch_mfn);
> - goto out;
> - }
> -
>   ret = intel_gvt_dma_map_guest_page(vgpu, gfn, PAGE_SIZE,
>  _addr);
>   if (ret) {
> @@ -2359,7 +2326,6 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu 
> *vgpu, unsigned int off,
>   ops->clear_present();

Re: [Intel-gfx] [PATCH v3] drm/i915/gvt: fix double free bug in split_2MB_gtt_entry

[Wang, Zhi A]

On 12/19/2022 9:57 AM, Zheng Wang wrote:
> Hi Zhi,
> 
> Thanks again for your reply and clear explaination about the function.
> I still have some doubt about the fix. Here is a invoke chain :
> ppgtt_populate_spt
>->ppgtt_populate_shadow_entry
>  ->split_2MB_gtt_entry
> As far as I'm concerned, when something error happens in DMA mapping,
> which will make intel_gvt_dma_map_guest_page return none-zero code,
> It will invoke ppgtt_invalidate_spt and call ppgtt_free_spt,which will
> finally free spt by kfree. But the caller doesn't notice that and frees
> spt by calling ppgtt_free_spt again. This is a typical UAF/Double Free
> vulnerability. So I think the key point is about how to handle spt properly.
> The handle newly allocated spt (aka sub_spt) is not the root cause of this
> issue. Could you please give me more advice about how to fix this security
> bug? Besides, I'm not sure if there are more similar problems in othe 
> location.
> 
> Best regards,
> Zheng Wang
> 

I think it is a case-by-case thing. For example:

The current scenario in this function looks like below:

caller pass spt a
function
alloc spt b
something error
free spt a
return error

The problem is: the function wrongly frees the spt a instead free what 
it allocates.

A proper fix should be:

caller pass spt a
function
alloc spt b
something error
*free spt b*
return error

Thanks,
Zhi.

Re: [Intel-gfx] [PATCH v3] drm/i915/gvt: fix double free bug in split_2MB_gtt_entry

[Wang, Zhi A]

On 12/15/2022 12:47 PM, Joonas Lahtinen wrote:
> (+ Tvrtko as FYI)
>
> Zhenyu, can you take a look at the patch ASAP.
>
> Regards, Joonas

Thanks so much for the reminding and patch.


Actually I don't think it is proper fix as:

split_2MB_gtt_entry() is going to allocate a new spt structure, which is 
a PTE page to hold

the mapping of the 2MB. It will map the sub 4k pages for DMA addrs, form 
them as PTE

entries, write the entries into the new PTE page,  and then link the 
page to the parent

table entry so that the GPU can reach it.


Now something wrong happens when mapping the sub 4K pages. What we need 
are 1) The

existing mappings of DMA addr need to be un-done and 2) the newly 
allocated spt structure

needs to be freed.  These can be handle by ppgtt_invalidate_spt() which 
will handle the 1)

and 2) based on the type of shadow page table, either recursively or 
not. i.e. in this case,

it's a PTE page.


I guess the code wrongly does 1) 2) on the parent page table when 
something error happens in

DMA mapping . You can fix it by releasing the newly allocated spt in the 
error case and put a

Fixes: b901b252b6cf ("drm/i915/gvt: Add 2M huge gtt support") in the 
patch comment.


BTW: For sending the patches, you can take a look on "git send-email". 
It will promise the correct

format and prevent quite some bumps. For email clients, if you feel mutt 
is hard to ramp up,

you can try the Claws Mail. More information can be found in 
Documentation/process/email-clients.rst


Thanks,

Zhi.

>
> Quoting Dave Airlie (2022-10-27 08:12:31)
>> On Thu, 27 Oct 2022 at 13:26, Zheng Hacker  wrote:
>>> Dave Airlie  于2022年10月27日周四 08:01写道：
 On Fri, 7 Oct 2022 at 11:38, Zheng Wang  wrote:
> If intel_gvt_dma_map_guest_page failed, it will call
> ppgtt_invalidate_spt, which will finally free the spt.
> But the caller does not notice that, it will free spt again in error path.
>
> Fix this by spliting invalidate and free in ppgtt_invalidate_spt.
> Only free spt when in good case.
>
> Reported-by: Zheng Wang 
> Signed-off-by: Zheng Wang 
 Has this landed in a tree yet, since it's a possible CVE, might be
 good to merge it somewhere.

 Dave.

>>> Hi Dave,
>>>
>>> This patched hasn't been merged yet. Could you please help with this?
>> I'll add some more people who can probably look at it.
>>
>> Dave.

Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call

[Wang, Zhi A]

On 10/6/22 18:31, Alex Williamson wrote:
> On Thu, 6 Oct 2022 08:37:09 -0300
> Jason Gunthorpe  wrote:
> 
>> On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
>>> We can't have a .remove callback that does nothing, this breaks
>>> removing the device while it's in use.  Once we have the
>>> vfio_unregister_group_dev() fix below, we'll block until the device is
>>> unused, at which point vgpu->attached becomes false.  Unless I'm
>>> missing something, I think we should also follow-up with a patch to
>>> remove that bogus warn-on branch, right?  Thanks,  
>>
>> Yes, looks right to me.
>>
>> I question all the logical arround attached, where is the locking?
> 
> Zhenyu, Zhi, Kevin,
> 
> Could someone please take a look at use of vgpu->attached in the GVT-g
> driver?  It's use in intel_vgpu_remove() is bogus, the .release
> callback needs to use vfio_unregister_group_dev() to wait for the
> device to be unused.  The WARN_ON/return here breaks all future use of
> the device.  I assume @attached has something to do with the page table
> interface with KVM, but it all looks racy anyway.
> 
Thanks for pointing this out.

It was introduced in the GVT-g refactor patch series and Christoph might
not want to touch the vgpu->released while he needed a new state.

I dig it a bit. vgpu->attached would be used for preventing multiple open
on a single vGPU and indicate the kvm_get_kvm() has been done.
vgpu->released was to prevent the release before close, which is now
handled by the vfio_device_*.

What I would like to do are: 
1) Remove the vgpu->released. 2) Use alock to protect vgpu->attached.

After those were solved, the WARN_ON/return in the intel_vgpu_remove()
should be safely removed as the .release will be called after .close_device
deceases the vfio_device->refcnt to zero.

Thanks,
Zhi.

> Also, whatever purpose vgpu->released served looks unnecessary now.
> Thanks,
> 
> Alex
> 

Re: [Intel-gfx] [PATCH v3 1/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM

[Wang, Zhi A]

On 5/23/22 4:41 PM, Alex Williamson wrote:
> 
> Hi Zhi & Zhenyu,
> 
> Please review gvt changes below, I'd prefer to get your ack included.
> Thanks!
> 
> Alex
> 
> On Thu, 19 May 2022 14:33:11 -0400
> Matthew Rosato  wrote:
> 
>> Rather than relying on a notifier for associating the KVM with
>> the group, let's assume that the association has already been
>> made prior to device_open.  The first time a device is opened
>> associate the group KVM with the device.
>>
>> This fixes a user-triggerable oops in GVT.
>>
>> Reviewed-by: Tony Krowiak 
>> Reviewed-by: Kevin Tian 
>> Reviewed-by: Christoph Hellwig 
>> Signed-off-by: Jason Gunthorpe 
>> Signed-off-by: Matthew Rosato 
>> ---
>>  drivers/gpu/drm/i915/gvt/gtt.c|  4 +-
>>  drivers/gpu/drm/i915/gvt/gvt.h|  3 -
>>  drivers/gpu/drm/i915/gvt/kvmgt.c  | 82 ++
>>  drivers/s390/crypto/vfio_ap_ops.c | 35 ++-
>>  drivers/s390/crypto/vfio_ap_private.h |  3 -
>>  drivers/vfio/vfio.c   | 83 ++-
>>  include/linux/vfio.h  |  6 +-
>>  7 files changed, 57 insertions(+), 159 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
>> index 9c5cc2800975..b4f69364f9a1 100644
>> --- a/drivers/gpu/drm/i915/gvt/gtt.c
>> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
>> @@ -51,7 +51,7 @@ static int preallocated_oos_pages = 8192;
>>  
>>  static bool intel_gvt_is_valid_gfn(struct intel_vgpu *vgpu, unsigned long 
>> gfn)
>>  {
>> -struct kvm *kvm = vgpu->kvm;
>> +struct kvm *kvm = vgpu->vfio_device.kvm;
>>  int idx;
>>  bool ret;
>>  
>> @@ -1185,7 +1185,7 @@ static int is_2MB_gtt_possible(struct intel_vgpu *vgpu,
>>  
>>  if (!vgpu->attached)
>>  return -EINVAL;
>> -pfn = gfn_to_pfn(vgpu->kvm, ops->get_pfn(entry));
>> +pfn = gfn_to_pfn(vgpu->vfio_device.kvm, ops->get_pfn(entry));
>>  if (is_error_noslot_pfn(pfn))
>>  return -EINVAL;
>>  return PageTransHuge(pfn_to_page(pfn));
>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
>> index 2af4c83e733c..aee1a45da74b 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.h
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
>> @@ -227,9 +227,6 @@ struct intel_vgpu {
>>  struct mutex cache_lock;
>>  
>>  struct notifier_block iommu_notifier;
>> -struct notifier_block group_notifier;
>> -struct kvm *kvm;
>> -struct work_struct release_work;
>>  atomic_t released;
>>  
>>  struct kvm_page_track_notifier_node track_node;
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 7655ffa97d51..e2f6c56ab342 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -228,8 +228,6 @@ static void intel_gvt_cleanup_vgpu_type_groups(struct 
>> intel_gvt *gvt)
>>  }
>>  }
>>  
>> -static void intel_vgpu_release_work(struct work_struct *work);
>> -
>>  static void gvt_unpin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn,
>>  unsigned long size)
>>  {
>> @@ -761,23 +759,6 @@ static int intel_vgpu_iommu_notifier(struct 
>> notifier_block *nb,
>>  return NOTIFY_OK;
>>  }
>>  
>> -static int intel_vgpu_group_notifier(struct notifier_block *nb,
>> - unsigned long action, void *data)
>> -{
>> -struct intel_vgpu *vgpu =
>> -container_of(nb, struct intel_vgpu, group_notifier);
>> -
>> -/* the only action we care about */
>> -if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
>> -vgpu->kvm = data;
>> -
>> -if (!data)
>> -schedule_work(>release_work);
>> -}
>> -
>> -return NOTIFY_OK;
>> -}
>> -
>>  static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu)
>>  {
>>  struct intel_vgpu *itr;
>> @@ -789,7 +770,7 @@ static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu)
>>  if (!itr->attached)
>>  continue;
>>  
>> -if (vgpu->kvm == itr->kvm) {
>> +if (vgpu->vfio_device.kvm == itr->vfio_device.kvm) {
>>  ret = true;
>>  goto out;
>>  }
>> @@ -806,7 +787,6 @@ static int intel_vgpu_open_device(struct vfio_device 
>> *vfio_dev)
>>  int ret;
>>  
>>  vgpu->iommu_notifier.notifier_call = intel_vgpu_iommu_notifier;
>> -vgpu->group_notifier.notifier_call = intel_vgpu_group_notifier;
>>  
>>  events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
>>  ret = vfio_register_notifier(vfio_dev, VFIO_IOMMU_NOTIFY, ,
>> @@ -817,38 +797,32 @@ static int intel_vgpu_open_device(struct vfio_device 
>> *vfio_dev)
>>  goto out;
>>  }
>>  
>> -events = VFIO_GROUP_NOTIFY_SET_KVM;
>> -ret = vfio_register_notifier(vfio_dev, VFIO_GROUP_NOTIFY, ,
>> - >group_notifier);
>> -if (ret != 0) {
>> -gvt_vgpu_err("vfio_register_notifier for group failed: %d\n",
>> -  

Re: [Intel-gfx] [PATCH v4 5/7] drm/i915/gvt: Change from vfio_group_(un)pin_pages to vfio_(un)pin_pages

[Wang, Zhi A]

On 5/6/22 12:08 AM, Jason Gunthorpe wrote:
> Use the existing vfio_device versions of vfio_(un)pin_pages(). There is no
> reason to use a group interface here, kvmgt has easy access to a
> vfio_device.
> 
> Delete kvmgt_vdev::vfio_group since these calls were the last users.
> 
> Reviewed-by: Kevin Tian 
> Reviewed-by: Christoph Hellwig 
> Signed-off-by: Jason Gunthorpe 
> ---
>  drivers/gpu/drm/i915/gvt/gvt.h   |  1 -
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 27 ++-
>  2 files changed, 6 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 5a28ee965b7f3e..2af4c83e733c6c 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -231,7 +231,6 @@ struct intel_vgpu {
>   struct kvm *kvm;
>   struct work_struct release_work;
>   atomic_t released;
> - struct vfio_group *vfio_group;
>  
>   struct kvm_page_track_notifier_node track_node;
>  #define NR_BKT (1 << 18)
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 1cec4f1fdfaced..7655ffa97d5116 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -243,7 +243,7 @@ static void gvt_unpin_guest_page(struct intel_vgpu *vgpu, 
> unsigned long gfn,
>   for (npage = 0; npage < total_pages; npage++) {
>   unsigned long cur_gfn = gfn + npage;
>  
> - ret = vfio_group_unpin_pages(vgpu->vfio_group, _gfn, 1);
> + ret = vfio_unpin_pages(>vfio_device, _gfn, 1);
>   drm_WARN_ON(>drm, ret != 1);
>   }
>  }
> @@ -266,8 +266,8 @@ static int gvt_pin_guest_page(struct intel_vgpu *vgpu, 
> unsigned long gfn,
>   unsigned long cur_gfn = gfn + npage;
>   unsigned long pfn;
>  
> - ret = vfio_group_pin_pages(vgpu->vfio_group, _gfn, 1,
> -IOMMU_READ | IOMMU_WRITE, );
> + ret = vfio_pin_pages(>vfio_device, _gfn, 1,
> +  IOMMU_READ | IOMMU_WRITE, );
>   if (ret != 1) {
>   gvt_vgpu_err("vfio_pin_pages failed for gfn 0x%lx, ret 
> %d\n",
>cur_gfn, ret);
> @@ -804,7 +804,6 @@ static int intel_vgpu_open_device(struct vfio_device 
> *vfio_dev)
>   struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
>   unsigned long events;
>   int ret;
> - struct vfio_group *vfio_group;
>  
>   vgpu->iommu_notifier.notifier_call = intel_vgpu_iommu_notifier;
>   vgpu->group_notifier.notifier_call = intel_vgpu_group_notifier;
> @@ -827,28 +826,19 @@ static int intel_vgpu_open_device(struct vfio_device 
> *vfio_dev)
>   goto undo_iommu;
>   }
>  
> - vfio_group =
> - vfio_group_get_external_user_from_dev(vgpu->vfio_device.dev);
> - if (IS_ERR_OR_NULL(vfio_group)) {
> - ret = !vfio_group ? -EFAULT : PTR_ERR(vfio_group);
> - gvt_vgpu_err("vfio_group_get_external_user_from_dev failed\n");
> - goto undo_register;
> - }
> - vgpu->vfio_group = vfio_group;
> -
>   ret = -EEXIST;
>   if (vgpu->attached)
> - goto undo_group;
> + goto undo_register;
>  
>   ret = -ESRCH;
>   if (!vgpu->kvm || vgpu->kvm->mm != current->mm) {
>   gvt_vgpu_err("KVM is required to use Intel vGPU\n");
> - goto undo_group;
> + goto undo_register;
>   }
>  
>   ret = -EEXIST;
>   if (__kvmgt_vgpu_exist(vgpu))
> - goto undo_group;
> + goto undo_register;
>  
>   vgpu->attached = true;
>   kvm_get_kvm(vgpu->kvm);
> @@ -868,10 +858,6 @@ static int intel_vgpu_open_device(struct vfio_device 
> *vfio_dev)
>   atomic_set(>released, 0);
>   return 0;
>  
> -undo_group:
> - vfio_group_put_external_user(vgpu->vfio_group);
> - vgpu->vfio_group = NULL;
> -
>  undo_register:
>   vfio_unregister_notifier(vfio_dev, VFIO_GROUP_NOTIFY,
>>group_notifier);
> @@ -925,7 +911,6 @@ static void __intel_vgpu_release(struct intel_vgpu *vgpu)
>   gvt_cache_destroy(vgpu);
>  
>   intel_vgpu_release_msi_eventfd_ctx(vgpu);
> - vfio_group_put_external_user(vgpu->vfio_group);
>  
>   vgpu->kvm = NULL;
>   vgpu->attached = false;
> 
Acked-by: Zhi Wang 

[Intel-gfx] [PULL] gvt-next-2022-04-29

[Wang, Zhi A]

Hi folks:

Here is the pull of gvt-next which fixes the compilation error and warnings
for the the GVT-g refactor patches: 

- Fix a compiling warning of non-static function only having one caller.
- Fix a potential NULL pointer reference in the code re-factor.
- Fix a compiling error when CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n

I also tried to apply this pull on the latest drm-inte-next and it succeeded
without error and warnings.

The following changes since commit 5e9ae5c47052e28a31fb4f55a6e735c28d4c3948:

  drm/i915/gvt: Add missing symbol export. (2022-04-26 04:18:43 -0400)

are available in the Git repository at:

  https://github.com/intel/gvt-linux tags/gvt-next-2022-04-29

for you to fetch changes up to 419f8299ddad6070a6c95aaedf78e50265871f36:

  i915/gvt: Fix NULL pointer dereference in init_mmio_block_handlers 
(2022-04-28 17:06:02 -0400)


gvt-next-2022-04-29

Introduce fixes from previous pull.
- Fix a compiling warning of non-static funtion only having one caller.
- Fix a potential NULL pointer reference in the code re-factor.
- Fix a compiling error when CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n


Wan Jiabing (1):
  i915/gvt: Fix NULL pointer dereference in init_mmio_block_handlers

Zhi Wang (2):
  drm/i915/gvt: Make intel_gvt_match_device() static
  drm/i915/gvt: Fix the compiling error when 
CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n

 drivers/gpu/drm/i915/gvt/handlers.c | 4 ++--
 drivers/gpu/drm/i915/intel_gvt.c| 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

Re: [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Fix missing static

[Wang, Zhi A]

On 4/28/22 8:40 PM, De Marchi, Lucas wrote:
> Fix broken build:
> 
>   $ make W=1 drivers/gpu/drm/i915/gvt/handlers.o
> ...
> CC [M]  drivers/gpu/drm/i915/gvt/handlers.o
>   drivers/gpu/drm/i915/gvt/handlers.c:75:6: error: no previous prototype 
> for ‘intel_gvt_match_device’ [-Werror=missing-prototypes]
>  75 | bool intel_gvt_match_device(struct intel_gvt *gvt,
> |  ^~
>   cc1: all warnings being treated as errors
> 
> Commit e0f74ed4634d ("i915/gvt: Separate the MMIO tracking table from
> GVT-g") removed the prototype from the header due to the function being
> used only in this single compilation unit, but forgot to make it static.
> 
> Fixes: e0f74ed4634d ("i915/gvt: Separate the MMIO tracking table from GVT-g")
> Cc: Zhi Wang 
> Cc: Christoph Hellwig 
> Cc: Zhenyu Wang 
> Signed-off-by: Lucas De Marchi 
> ---
>  drivers/gpu/drm/i915/gvt/handlers.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
Hi Lucas:

Thanks so much for the patch. There is a patch to fix
undergoing already. I will take your second patch.
 
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index cf00398c2870..e4358aa01048 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -72,8 +72,8 @@ unsigned long intel_gvt_get_device_type(struct intel_gvt 
> *gvt)
>   return 0;
>  }
>  
> -bool intel_gvt_match_device(struct intel_gvt *gvt,
> - unsigned long device)
> +static bool intel_gvt_match_device(struct intel_gvt *gvt,
> +unsigned long device)
>  {
>   return intel_gvt_get_device_type(gvt) & device;
>  }
> 

Re: [Intel-gfx] [PULL] gvt-next

[Wang, Zhi A]

On 4/26/22 3:53 PM, Jason Gunthorpe wrote:
> On Tue, Apr 26, 2022 at 07:58:59AM +0000, Wang, Zhi A wrote:
>> Hi folks:
>>
>> Here is the pull of gvt-next which fixs the compilation error when i915 debug
>> is open after the GVT-g refactor patches.
>>
>> Thanks so much for the efforts.
>>
>> Thanks,
>> Zhi.
>>
>> The following changes since commit 2917f53113be3b7a0f374e02cebe6d6b749366b5:
>>
>>   vfio/mdev: Remove mdev drvdata (2022-04-21 07:36:56 -0400)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/intel/gvt-linux tags/gvt-next-2022-04-26
>>
>> for you to fetch changes up to 2da299cee780ea797b3f72558687868072cf5eb5:
>>
>>   drm/i915/gvt: Add missing export of symbols. (2022-04-25 18:03:04 -0400)
>>
>> gvt-next-2022-04-26
>>
>> - Add two missing exports of symbols when i915 debug is enabled.
>>
>> Zhi Wang (1):
>>   drm/i915/gvt: Add missing export of symbols.
>>
>>  drivers/gpu/drm/i915/intel_gvt.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 
> This still has another compile problem:
> 
> ERROR: modpost: "intel_runtime_pm_put" [vmlinux] is a static EXPORT_SYMBOL_GPL
> 
> Because:
> 
> #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> void intel_runtime_pm_put(struct intel_runtime_pm *rpm, intel_wakeref_t wref);
> #else
> static inline void
> intel_runtime_pm_put(struct intel_runtime_pm *rpm, intel_wakeref_t wref)
> {
> intel_runtime_pm_put_unchecked(rpm);
> }
> #endif
> 
> Looks like it happens if CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n
> 
> I think you probably want to #ifdef the export in the same way:
> 
> --- a/drivers/gpu/drm/i915/intel_gvt.c
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -309,7 +309,9 @@ EXPORT_SYMBOL_NS_GPL(__intel_context_do_pin, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(__intel_context_do_unpin, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(intel_ring_begin, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(intel_runtime_pm_get, I915_GVT);
> +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
>  EXPORT_SYMBOL_NS_GPL(intel_runtime_pm_put, I915_GVT);
> +#endif
Sigh. That's tricky. Let me prepare another one. 
>  EXPORT_SYMBOL_NS_GPL(intel_runtime_pm_put_unchecked, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(intel_uncore_forcewake_for_reg, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(intel_uncore_forcewake_get, I915_GVT);
> 
> Jason
> 

Re: [Intel-gfx] [PULL] gvt-next

[Wang, Zhi A]

On 4/26/22 8:37 AM, Jani Nikula wrote:
> On Tue, 26 Apr 2022, "Wang, Zhi A"  wrote:
>> Hi folks:
>>
>> Here is the pull of gvt-next which fixs the compilation error when i915 debug
>> is open after the GVT-g refactor patches.
>>
>> Thanks so much for the efforts.
> 
> Pulled, thanks.
> 
> BR,
> Jani.
> 
Thanks, looks good now. :)

thanks,
Zhi.
>>
>> Thanks,
>> Zhi.
>>
>> The following changes since commit 2917f53113be3b7a0f374e02cebe6d6b749366b5:
>>
>>   vfio/mdev: Remove mdev drvdata (2022-04-21 07:36:56 -0400)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/intel/gvt-linux tags/gvt-next-2022-04-26
>>
>> for you to fetch changes up to 2da299cee780ea797b3f72558687868072cf5eb5:
>>
>>   drm/i915/gvt: Add missing export of symbols. (2022-04-25 18:03:04 -0400)
>>
>> 
>> gvt-next-2022-04-26
>>
>> - Add two missing exports of symbols when i915 debug is enabled.
>>
>> 
>> Zhi Wang (1):
>>   drm/i915/gvt: Add missing export of symbols.
>>
>>  drivers/gpu/drm/i915/intel_gvt.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 

Re: [Intel-gfx] [PULL] gvt-next

[Wang, Zhi A]

On 4/26/22 8:37 AM, Jani Nikula wrote:
> On Tue, 26 Apr 2022, "Wang, Zhi A"  wrote:
>> Hi folks:
>>
>> Here is the pull of gvt-next which fixs the compilation error when i915 debug
>> is open after the GVT-g refactor patches.
>>
>> Thanks so much for the efforts.
> 
> Pulled, thanks.
> 
> BR,
> Jani.
> 
>>
I just updated the branch. Can you check a little bit if you got the newest one?
>> Thanks,
>> Zhi.
>>
>> The following changes since commit 2917f53113be3b7a0f374e02cebe6d6b749366b5:
>>
>>   vfio/mdev: Remove mdev drvdata (2022-04-21 07:36:56 -0400)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/intel/gvt-linux tags/gvt-next-2022-04-26
>>
>> for you to fetch changes up to 2da299cee780ea797b3f72558687868072cf5eb5:
>>
>>   drm/i915/gvt: Add missing export of symbols. (2022-04-25 18:03:04 -0400)
>>
>> 
>> gvt-next-2022-04-26
>>
>> - Add two missing exports of symbols when i915 debug is enabled.
>>
>> 
>> Zhi Wang (1):
>>   drm/i915/gvt: Add missing export of symbols.
>>
>>  drivers/gpu/drm/i915/intel_gvt.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 

[Intel-gfx] [PULL v2] gvt-next

[Wang, Zhi A]

Hi folks:

I updated the branch again. Please use this pull. Here is the pull of
gvt-next which fixes the compilation error when i915 debug is open after
the GVT-g refactor patches.

Thanks so much for the efforts.

Thanks,
Zhi.

The following changes since commit 2917f53113be3b7a0f374e02cebe6d6b749366b5:

  vfio/mdev: Remove mdev drvdata (2022-04-21 07:36:56 -0400)

are available in the Git repository at:

  https://github.com/intel/gvt-linux tags/gvt-next-2022-04-26

for you to fetch changes up to 5e9ae5c47052e28a31fb4f55a6e735c28d4c3948:

  drm/i915/gvt: Add missing symbol export. (2022-04-26 04:18:43 -0400)


gvt-next-2022-04-26

- Add two missing exports of symbols when i915 debug is enabled


Zhi Wang (1):
  drm/i915/gvt: Add missing symbol export.

 drivers/gpu/drm/i915/intel_gvt.c | 2 ++
 1 file changed, 2 insertions(+)


gvt-next-2022-04-26

- Add two missing exports of symbols when i915 debug is enabled.


Zhi Wang (1):
  drm/i915/gvt: Add missing export of symbols.

 drivers/gpu/drm/i915/intel_gvt.c | 2 ++
 1 file changed, 2 insertions(+)

[Intel-gfx] [PULL] gvt-next

[Wang, Zhi A]

Hi folks:

Here is the pull of gvt-next which fixs the compilation error when i915 debug
is open after the GVT-g refactor patches.

Thanks so much for the efforts.

Thanks,
Zhi.

The following changes since commit 2917f53113be3b7a0f374e02cebe6d6b749366b5:

  vfio/mdev: Remove mdev drvdata (2022-04-21 07:36:56 -0400)

are available in the Git repository at:

  https://github.com/intel/gvt-linux tags/gvt-next-2022-04-26

for you to fetch changes up to 2da299cee780ea797b3f72558687868072cf5eb5:

  drm/i915/gvt: Add missing export of symbols. (2022-04-25 18:03:04 -0400)


gvt-next-2022-04-26

- Add two missing exports of symbols when i915 debug is enabled.


Zhi Wang (1):
  drm/i915/gvt: Add missing export of symbols.

 drivers/gpu/drm/i915/intel_gvt.c | 2 ++
 1 file changed, 2 insertions(+)

Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing symbol export.

[Wang, Zhi A]

On 4/25/22 10:03 PM, Zhi Wang wrote:
> When CONFIG_DRM_I915_DEBUG_RUNTIME and CONFIG_DRM_I915_DEBUG_PM
> are enabled, two more extra symols in i915 are required to be
> exported.
> 
> Cc: Jani Nikula 
> Signed-off-by: Zhi Wang 
> ---
>  drivers/gpu/drm/i915/intel_gvt.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_gvt.c 
> b/drivers/gpu/drm/i915/intel_gvt.c
> index 7c03d975069e..24bc693439e8 100644
> --- a/drivers/gpu/drm/i915/intel_gvt.c
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -309,6 +309,7 @@ EXPORT_SYMBOL_NS_GPL(__intel_context_do_pin, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(__intel_context_do_unpin, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(intel_ring_begin, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(intel_runtime_pm_get, I915_GVT);
> +EXPORT_SYMBOL_NS_GPL(intel_runtime_pm_put, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(intel_runtime_pm_put_unchecked, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(intel_uncore_forcewake_for_reg, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(intel_uncore_forcewake_get, I915_GVT);
> @@ -316,3 +317,4 @@ EXPORT_SYMBOL_NS_GPL(intel_uncore_forcewake_put, 
> I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(shmem_pin_map, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(shmem_unpin_map, I915_GVT);
>  EXPORT_SYMBOL_NS_GPL(__px_dma, I915_GVT);
> +EXPORT_SYMBOL_NS_GPL(i915_fence_ops, I915_GVT);
> 
Jani:

Can you give this an rb? As I can't give myself a rb.

Re: [Intel-gfx] [PULL v3] gvt-next

[Wang, Zhi A]

On 4/25/22 12:33 PM, Jani Nikula wrote:
> On Mon, 25 Apr 2022, Jani Nikula  wrote:
>> On Thu, 21 Apr 2022, "Wang, Zhi A"  wrote:
>>> Hi folks:
>>>
>>> Here is the PR of gvt-next. Thanks so much for the patience.
>>
>> Thanks, pulled to drm-intel-next, applied the below fix for the silent
>> conflict on top, and pushed out. Should show up in linux-next shortly.
> 
> Aww crap, this breaks debug builds.
> 
> ERROR: modpost: "intel_runtime_pm_put" [drivers/gpu/drm/i915/kvmgt.ko] 
> undefined!
> ERROR: modpost: "i915_fence_ops" [drivers/gpu/drm/i915/kvmgt.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:134: modules-only.symvers] Error 1
> make[1]: *** Deleting file 'modules-only.symvers'
> make: *** [Makefile:1749: modules] Error 2
> 
> The first is triggered with CONFIG_DRM_I915_DEBUG_RUNTIME_PM=y, the
> latter with CONFIG_DRM_I915_DEBUG_GEM=y.
> 
> Please add the proper fix on top of the topic branch, and send an
> additional pull request. Looks like exports will do it.
> 
> On that note, I'm wondering about the use of
> EXPORT_SYMBOL_NS_GPL(). It's between two MIT licensed modules after
> all. Maybe just EXPORT_SYMBOL_NS()?
> 
Will do. Thanks so much for catching this. 
> BR,
> Jani.
> 
> 
>>
>> BR,
>> Jani.
>>
>>>
>>> Mostly it includes the patch bundle of GVT-g re-factor patches for adapting 
>>> the GVT-g with the
>>> new MDEV interfaces:
>>>
>>> - Separating the MMIO table from GVT-g. (Zhi)
>>> - GVT-g re-factor. (Christoph)
>>> - GVT-g mdev API cleanup. (Jason)
>>> - GVT-g trace/makefile cleanup. (Jani)
>>>
>>> Thanks so much for making this happen.
>>>
>>> This PR has been tested as following and no problem shows up:
>>>
>>> $dim update-branches
>>> $dim apply-pull drm-intel-next < this_email.eml
>>>
>>> When merging this pull to drm-intel-next, please include the following code 
>>> in the merge commit:
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c 
>>> b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
>>> index 03a7fcd0f904..72dac1718f3e 100644
>>> --- a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
>>> +++ b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
>>> @@ -3,6 +3,7 @@
>>>   * Copyright © 2020 Intel Corporation
>>>   */
>>>  
>>> +#include "display/intel_dmc_regs.h"
>>>  #include "display/vlv_dsi_pll_regs.h"
>>>  #include "gt/intel_gt_regs.h"
>>>  #include "gvt/gvt.h"
>>>
>>>
>>> The following changes since commit 3123109284176b1532874591f7c81f3837bbdc17:
>>>
>>>   Linux 5.18-rc1 (2022-04-03 14:08:21 -0700)
>>>
>>> are available in the Git repository at:
>>>
>>>   https://github.com/intel/gvt-linux tags/gvt-next-2022-04-21-for-christoph
>>>
>>> for you to fetch changes up to 2917f53113be3b7a0f374e02cebe6d6b749366b5:
>>>
>>>   vfio/mdev: Remove mdev drvdata (2022-04-21 07:36:56 -0400)
>>>
>>> 
>>> gvt-next-2022-04-21-for-christoph
>>>
>>> - Separating the MMIO table from GVT-g. (Zhi)
>>> - GVT-g re-factor. (Christoph)
>>> - GVT-g mdev API cleanup. (Jason)
>>> - GVT-g trace/makefile cleanup. (Jani)
>>>
>>> 
>>> Christoph Hellwig (27):
>>>   drm/i915/gvt: remove module refcounting in 
>>> intel_gvt_{,un}register_hypervisor
>>>   drm/i915/gvt: remove enum hypervisor_type
>>>   drm/i915/gvt: rename intel_vgpu_ops to intel_vgpu_mdev_ops
>>>   drm/i915/gvt: move the gvt code into kvmgt.ko
>>>   drm/i915/gvt: remove intel_gvt_ops
>>>   drm/i915/gvt: remove the map_gfn_to_mfn and set_trap_area ops
>>>   drm/i915/gvt: remove the unused from_virt_to_mfn op
>>>   drm/i915/gvt: merge struct kvmgt_vdev into struct intel_vgpu
>>>   drm/i915/gvt: merge struct kvmgt_guest_info into strut intel_vgpu
>>>   drm/i915/gvt: remove vgpu->handle
>>>   drm/i915/gvt: devirtualize ->{read,write}_gpa
>>>   drm/i915/gvt: devirtualize ->{get,put}_vfio_device
>>>   drm/i915/gvt: devirtualize ->set_edid and ->set_opregion
>>>   drm/i915/gvt: devirtualize ->detach_vgpu
>>>   drm/i915/gvt: devirtualize ->inject_msi
>>>   dr

[Intel-gfx] [PULL v3] gvt-next

[Wang, Zhi A]

Hi folks:

Here is the PR of gvt-next. Thanks so much for the patience.

Mostly it includes the patch bundle of GVT-g re-factor patches for adapting the 
GVT-g with the
new MDEV interfaces:

- Separating the MMIO table from GVT-g. (Zhi)
- GVT-g re-factor. (Christoph)
- GVT-g mdev API cleanup. (Jason)
- GVT-g trace/makefile cleanup. (Jani)

Thanks so much for making this happen.

This PR has been tested as following and no problem shows up:

$dim update-branches
$dim apply-pull drm-intel-next < this_email.eml

When merging this pull to drm-intel-next, please include the following code in 
the merge commit:

diff --git a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c 
b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
index 03a7fcd0f904..72dac1718f3e 100644
--- a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
+++ b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
@@ -3,6 +3,7 @@
  * Copyright © 2020 Intel Corporation
  */
 
+#include "display/intel_dmc_regs.h"
 #include "display/vlv_dsi_pll_regs.h"
 #include "gt/intel_gt_regs.h"
 #include "gvt/gvt.h"


The following changes since commit 3123109284176b1532874591f7c81f3837bbdc17:

  Linux 5.18-rc1 (2022-04-03 14:08:21 -0700)

are available in the Git repository at:

  https://github.com/intel/gvt-linux tags/gvt-next-2022-04-21-for-christoph

for you to fetch changes up to 2917f53113be3b7a0f374e02cebe6d6b749366b5:

  vfio/mdev: Remove mdev drvdata (2022-04-21 07:36:56 -0400)


gvt-next-2022-04-21-for-christoph

- Separating the MMIO table from GVT-g. (Zhi)
- GVT-g re-factor. (Christoph)
- GVT-g mdev API cleanup. (Jason)
- GVT-g trace/makefile cleanup. (Jani)


Christoph Hellwig (27):
  drm/i915/gvt: remove module refcounting in 
intel_gvt_{,un}register_hypervisor
  drm/i915/gvt: remove enum hypervisor_type
  drm/i915/gvt: rename intel_vgpu_ops to intel_vgpu_mdev_ops
  drm/i915/gvt: move the gvt code into kvmgt.ko
  drm/i915/gvt: remove intel_gvt_ops
  drm/i915/gvt: remove the map_gfn_to_mfn and set_trap_area ops
  drm/i915/gvt: remove the unused from_virt_to_mfn op
  drm/i915/gvt: merge struct kvmgt_vdev into struct intel_vgpu
  drm/i915/gvt: merge struct kvmgt_guest_info into strut intel_vgpu
  drm/i915/gvt: remove vgpu->handle
  drm/i915/gvt: devirtualize ->{read,write}_gpa
  drm/i915/gvt: devirtualize ->{get,put}_vfio_device
  drm/i915/gvt: devirtualize ->set_edid and ->set_opregion
  drm/i915/gvt: devirtualize ->detach_vgpu
  drm/i915/gvt: devirtualize ->inject_msi
  drm/i915/gvt: devirtualize ->is_valid_gfn
  drm/i915/gvt: devirtualize ->gfn_to_mfn
  drm/i915/gvt: devirtualize ->{enable,disable}_page_track
  drm/i915/gvt: devirtualize ->dma_{,un}map_guest_page
  drm/i915/gvt: devirtualize dma_pin_guest_page
  drm/i915/gvt: remove struct intel_gvt_mpt
  drm/i915/gvt: remove the extra vfio_device refcounting for dmabufs
  drm/i915/gvt: streamline intel_vgpu_create
  drm/i915/gvt: pass a struct intel_vgpu to the vfio read/write helpers
  drm/i915/gvt: remove kvmgt_guest_{init,exit}
  drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev
  drm/i915/gvt: merge gvt.c into kvmgvt.c

Jani Nikula (2):
  drm/i915/gvt: fix trace TRACE_INCLUDE_PATH
  drm/i915/gvt: better align the Makefile with i915 Makefile

Jason Gunthorpe (5):
  vfio/mdev: Remove vfio_mdev.c
  vfio/mdev: Remove mdev_parent_ops dev_attr_groups
  vfio/mdev: Remove mdev_parent_ops
  vfio/mdev: Use the driver core to create the 'remove' file
  vfio/mdev: Remove mdev drvdata

Zhi Wang (3):
  i915/gvt: Separate the MMIO tracking table from GVT-g
  i915/gvt: Save the initial HW state snapshot in i915
  i915/gvt: Use the initial HW state snapshot saved in i915

 Documentation/driver-api/vfio-mediated-device.rst |   27 +-
 drivers/gpu/drm/i915/Kconfig  |   36 +-
 drivers/gpu/drm/i915/Makefile |8 +-
 drivers/gpu/drm/i915/gvt/Makefile |   30 +-
 drivers/gpu/drm/i915/gvt/cfg_space.c  |   89 +-
 drivers/gpu/drm/i915/gvt/cmd_parser.c |4 +-
 drivers/gpu/drm/i915/gvt/dmabuf.c |   36 +-
 drivers/gpu/drm/i915/gvt/execlist.c   |   12 +-
 drivers/gpu/drm/i915/gvt/firmware.c   |   25 +-
 drivers/gpu/drm/i915/gvt/gtt.c|   55 +-
 drivers/gpu/drm/i915/gvt/gvt.c|  340 --
 drivers/gpu/drm/i915/gvt/gvt.h|  128 +-
 drivers/gpu/drm/i915/gvt/handlers.c   | 1033 +++--
 drivers/gpu/drm/i915/gvt/hypercall.h  |   82 --
 drivers/gpu/drm/i915/gvt/interrupt.c  |   40 +-
 drivers/gpu/drm/i915/gvt/kvmgt.c  | 1097 +
 drivers/gpu/drm/i915/gvt/mmio.c   |4 +-
 

Re: [Intel-gfx] [PULL v2] gvt-next

[Wang, Zhi A]

On 4/21/22 1:14 PM, Jason Gunthorpe wrote:
> On Thu, Apr 21, 2022 at 09:41:30AM +0300, Joonas Lahtinen wrote:
>> + Tvrtko
>>
>> Quoting Christoph Hellwig (2022-04-21 08:47:38)
>>> On Thu, Apr 21, 2022 at 04:57:34AM +, Wang, Zhi A wrote:
>>>> Is it possible that I can send two different pull based on the same branch?
>>>> I was thinking I can remove this line in the original patch and then add a
>>>> small patch to add this line back on the top. Then make two different tags
>>>> before and after that small patch, send one pull with tag that includes 
>>>> that
>>>> small patch to i915 and the other pull with tag that doesn't includes it to
>>>> VFIO?
>>>
>>> Yes, you can do that as long as the small fixup commit is the very last
>>> one.
> 
> Keep in mind when doing this that best practice is for every commit to
> compile.
> 
> So if you add a commit with a new #include to this topic branch that
> commit will not compile.
> 
> Best practice is to fix the compilation breakage in a merge commit,
> either created by you or created by your upstream.
> 
I see. Let me update it. 
> Jason
> 

Re: [Intel-gfx] [PULL v2] gvt-next

[Wang, Zhi A]

On 4/20/22 8:00 PM, Jason Gunthorpe wrote:
> On Wed, Apr 20, 2022 at 02:46:00PM -0300, Jason Gunthorpe wrote:
>> On Wed, Apr 20, 2022 at 11:40:33AM -0600, Alex Williamson wrote:
>>> On Wed, 20 Apr 2022 13:43:51 -0300
>>> Jason Gunthorpe  wrote:
>>>
>>>> On Wed, Apr 20, 2022 at 04:34:47PM +, Wang, Zhi A wrote:
>>>>> Hi folks:
>>>>>
>>>>> Here is the PR of gvt-next. Thanks so much for the patience.
>>>>>
>>>>> Mostly it includes the patch bundle of GVT-g re-factor patches for 
>>>>> adapting the GVT-g with the
>>>>> new MDEV interfaces:
>>>>>
>>>>> - Separating the MMIO table from GVT-g. (Zhi)
>>>>> - GVT-g re-factor. (Christoph)
>>>>> - GVT-g mdev API cleanup. (Jason)
>>>>> - GVT-g trace/makefile cleanup. (Jani)
>>>>>
>>>>> Thanks so much for making this happen.
>>>>>
>>>>> This PR has been tested as following and no problem shows up:
>>>>>
>>>>> $dim update-branches
>>>>> $dim apply-pull drm-intel-next < this_email.eml
>>>>>
>>>>> The following changes since commit 
>>>>> 3123109284176b1532874591f7c81f3837bbdc17:
>>>>>
>>>>>   Linux 5.18-rc1 (2022-04-03 14:08:21 -0700)
>>>>>
>>>>> are available in the Git repository at:
>>>>>
>>>>>   https://github.com/intel/gvt-linux 
>>>>> tags/gvt-next-2022-04-20-for-christoph
>>>>>
>>>>> for you to fetch changes up to ae7875879b7c838bffb42ed6db4658e5c504032e:
>>>>>
>>>>>   vfio/mdev: Remove mdev drvdata (2022-04-20 03:15:58 -0400)  
>>>>
>>>> This looks well constructed now! thanks
>>>>
>>>> Alex you can pull this for VFIO after Jani grab it
>>>>
>>>> I'll respin my vfio_group series on top of this branch
>>>
>>> Sure, so just to confirm tags/gvt-next-2022-04-20-for-christoph is
>>> pruned down to exactly the commits we're looking for now?  Thanks,
>>
>> Yes, the above is correct and the tag points to commit
>> ae7875879b7c838bffb42ed6db4658e5c504032e
>>
>> It is the bare minimum series
> 
> Actually this topic branch doesn't compile:
> 
> ../drivers/gpu/drm/i915/intel_gvt_mmio_table.c:7:10: fatal error: 
> 'display/intel_dmc_regs.h' file not found
> #include "display/intel_dmc_regs.h"
>  ^~
> 1 error generated.
> 
> :( :(
> 
> This is the merge conflict that was mentioned. This topic branch needs
> to delete the above intel_dmc_regs.h include file
> 
> When drm-intel-next merges this PR then need to add it back as part of
> the merge resolution - so explain this in the PR text above and
> include a diff that does it when you send it again. (or do the merge
> yourself as I showed before, it depends on what drm-intel-next wants)
> 
Hi Jason:

Is it possible that I can send two different pull based on the same branch?
I was thinking I can remove this line in the original patch and then add a
small patch to add this line back on the top. Then make two different tags
before and after that small patch, send one pull with tag that includes that
small patch to i915 and the other pull with tag that doesn't includes it to
VFIO?

Thanks,
Zhi.

> Jason
> 

[Intel-gfx] [PULL v2] gvt-next

[Wang, Zhi A]

Hi folks:

Here is the PR of gvt-next. Thanks so much for the patience.

Mostly it includes the patch bundle of GVT-g re-factor patches for adapting the 
GVT-g with the
new MDEV interfaces:

- Separating the MMIO table from GVT-g. (Zhi)
- GVT-g re-factor. (Christoph)
- GVT-g mdev API cleanup. (Jason)
- GVT-g trace/makefile cleanup. (Jani)

Thanks so much for making this happen.

This PR has been tested as following and no problem shows up:

$dim update-branches
$dim apply-pull drm-intel-next < this_email.eml

The following changes since commit 3123109284176b1532874591f7c81f3837bbdc17:

  Linux 5.18-rc1 (2022-04-03 14:08:21 -0700)

are available in the Git repository at:

  https://github.com/intel/gvt-linux tags/gvt-next-2022-04-20-for-christoph

for you to fetch changes up to ae7875879b7c838bffb42ed6db4658e5c504032e:

  vfio/mdev: Remove mdev drvdata (2022-04-20 03:15:58 -0400)


gvt-next-2022-04-22-for-christoph

- Separating the MMIO table from GVT-g. (Zhi)
- GVT-g re-factor. (Christoph)
- GVT-g mdev API cleanup. (Jason)
- GVT-g trace/makefile cleanup. (Jani)


Christoph Hellwig (27):
  drm/i915/gvt: remove module refcounting in 
intel_gvt_{,un}register_hypervisor
  drm/i915/gvt: remove enum hypervisor_type
  drm/i915/gvt: rename intel_vgpu_ops to intel_vgpu_mdev_ops
  drm/i915/gvt: move the gvt code into kvmgt.ko
  drm/i915/gvt: remove intel_gvt_ops
  drm/i915/gvt: remove the map_gfn_to_mfn and set_trap_area ops
  drm/i915/gvt: remove the unused from_virt_to_mfn op
  drm/i915/gvt: merge struct kvmgt_vdev into struct intel_vgpu
  drm/i915/gvt: merge struct kvmgt_guest_info into strut intel_vgpu
  drm/i915/gvt: remove vgpu->handle
  drm/i915/gvt: devirtualize ->{read,write}_gpa
  drm/i915/gvt: devirtualize ->{get,put}_vfio_device
  drm/i915/gvt: devirtualize ->set_edid and ->set_opregion
  drm/i915/gvt: devirtualize ->detach_vgpu
  drm/i915/gvt: devirtualize ->inject_msi
  drm/i915/gvt: devirtualize ->is_valid_gfn
  drm/i915/gvt: devirtualize ->gfn_to_mfn
  drm/i915/gvt: devirtualize ->{enable,disable}_page_track
  drm/i915/gvt: devirtualize ->dma_{,un}map_guest_page
  drm/i915/gvt: devirtualize dma_pin_guest_page
  drm/i915/gvt: remove struct intel_gvt_mpt
  drm/i915/gvt: remove the extra vfio_device refcounting for dmabufs
  drm/i915/gvt: streamline intel_vgpu_create
  drm/i915/gvt: pass a struct intel_vgpu to the vfio read/write helpers
  drm/i915/gvt: remove kvmgt_guest_{init,exit}
  drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev
  drm/i915/gvt: merge gvt.c into kvmgvt.c

Jani Nikula (2):
  drm/i915/gvt: fix trace TRACE_INCLUDE_PATH
  drm/i915/gvt: better align the Makefile with i915 Makefile

Jason Gunthorpe (5):
  vfio/mdev: Remove vfio_mdev.c
  vfio/mdev: Remove mdev_parent_ops dev_attr_groups
  vfio/mdev: Remove mdev_parent_ops
  vfio/mdev: Use the driver core to create the 'remove' file
  vfio/mdev: Remove mdev drvdata

Zhi Wang (3):
  i915/gvt: Separate the MMIO tracking table from GVT-g
  i915/gvt: Save the initial HW state snapshot in i915
  i915/gvt: Use the initial HW state snapshot saved in i915

 Documentation/driver-api/vfio-mediated-device.rst |   27 +-
 drivers/gpu/drm/i915/Kconfig  |   36 +-
 drivers/gpu/drm/i915/Makefile |8 +-
 drivers/gpu/drm/i915/gvt/Makefile |   30 +-
 drivers/gpu/drm/i915/gvt/cfg_space.c  |   89 +-
 drivers/gpu/drm/i915/gvt/cmd_parser.c |4 +-
 drivers/gpu/drm/i915/gvt/dmabuf.c |   36 +-
 drivers/gpu/drm/i915/gvt/execlist.c   |   12 +-
 drivers/gpu/drm/i915/gvt/firmware.c   |   25 +-
 drivers/gpu/drm/i915/gvt/gtt.c|   55 +-
 drivers/gpu/drm/i915/gvt/gvt.c|  340 --
 drivers/gpu/drm/i915/gvt/gvt.h|  128 +-
 drivers/gpu/drm/i915/gvt/handlers.c   | 1033 +++-
 drivers/gpu/drm/i915/gvt/hypercall.h  |   82 --
 drivers/gpu/drm/i915/gvt/interrupt.c  |   40 +-
 drivers/gpu/drm/i915/gvt/kvmgt.c  | 1097 +
 drivers/gpu/drm/i915/gvt/mmio.c   |4 +-
 drivers/gpu/drm/i915/gvt/mmio.h   |1 -
 drivers/gpu/drm/i915/gvt/mpt.h|  400 ---
 drivers/gpu/drm/i915/gvt/opregion.c   |  148 +--
 drivers/gpu/drm/i915/gvt/page_track.c |8 +-
 drivers/gpu/drm/i915/gvt/reg.h|9 +-
 drivers/gpu/drm/i915/gvt/scheduler.c  |   37 +-
 drivers/gpu/drm/i915/gvt/trace.h  |2 +-
 drivers/gpu/drm/i915/gvt/vgpu.c   |   22 +-
 drivers/gpu/drm/i915/i915_driver.c|7 -
 

Re: [Intel-gfx] [PULL] gvt-next

[Wang, Zhi A]

On 4/20/22 3:02 PM, Jason Gunthorpe wrote:
> On Wed, Apr 20, 2022 at 02:41:04PM +0000, Wang, Zhi A wrote:
>> On 4/20/22 12:13 PM, Jason Gunthorpe wrote:
>>> On Wed, Apr 20, 2022 at 08:04:31AM +, Wang, Zhi A wrote:
>>>> Hi folks:
>>>>
>>>> Here is the PR of gvt-next.
>>>>
>>>> Mostly it includes the patch bundle of GVT-g re-factor patches for 
>>>> adapting the GVT-g with the
>>>> new MDEV interfaces:
>>>>
>>>> - Separating the MMIO table from GVT-g. (Zhi)
>>>> - GVT-g re-factor. (Christoph)
>>>> - GVT-g mdev API cleanup. (Jason)
>>>> - GVT-g trace/makefile cleanup. (Jani)
>>>>
>>>> Thanks so much for making this happen.
>>>>
>>>> This PR has been tested as following and no problem shows up:
>>>>
>>>> $dim update-branches
>>>> $dim apply-pull drm-intel-next < this_email.eml
>>>>
>>>> The following changes since commit 
>>>> b39d2c6202426b560641e5800c5523851b5db586:
>>>>
>>>>   drm/i915/fbc: Call intel_fbc_activate() directly from frontbuffer flush 
>>>> (2022-04-13 17:20:49 +0300)
>>>
>>> ??
>>>
>>> Why did you rebase this again? It needs to be on a rc release tag as
>>> you had in your github, not whatever this is.
>>>
>> Hi Jason:
>>
>> Here is what I understand, the pull going to the drm-intel-next
>> needs to be based on drm-intel-next from the branch gvt-next. 
> 
> No, there cannot be two pulls, as I explained when using topic
> branches you must never rebase.
> 
>> The pull going to the VFIO needs to be based on -rc, the
>> topic/for-christoph brnach.
> 
> Yes, so what you need to do is:
> 
> # Get a clean tree on drm-intel-next
> $ git worktree add /tmp/gvt-next
> Preparing worktree (new branch 'gvt-next')
> $ cd /tmp/gvt-next
> $ git reset --hard b39d2c6202426b560641e5800c5523851b5db586  # drm-intel-next 
> commit you tested
> 
> # Merge Christoph's topic:
> $ git fetch https://github.com/intel/gvt-linux.git topic/for-christoph
> $ git merge FETCH_HEAD
> Auto-merging drivers/gpu/drm/i915/Makefile
> Auto-merging drivers/gpu/drm/i915/gvt/handlers.c
> Auto-merging drivers/gpu/drm/i915/i915_driver.c
> Auto-merging drivers/gpu/drm/i915/i915_drv.h
> Merge made by the 'ort' strategy.
> 
Exactly what I did on my branch except that I sent the pull request based on my 
gvt-next. :(
> [..
> Merge branch 'topic/for-christoph' of https://github.com/intel/gvt-linux into 
> gvt-next
> 
> # By Christoph Hellwig (27) and others
> # Via Zhi Wang
> * 'topic/for-christoph' of https://github.com/intel/gvt-linux: (37 commits)
> ]
> 
> And then check it against what you prepared in this PR here:
> 
> $ git diff HEAD 888471711a80b22c53547f3a625f20f487714f28
> [empty]
> 
> *do not rebase a topic branch* this is very important.
> 
> Now - given that we can see there is no merge conflict you don't need
> to do anything! Just send topic/for-christoph, exactly as-it-is to
> drm-intel-next as a PR and that is all.
> 
>> Sorry this is way too complicated to me. Let me prepare the new pull
>> as what you ask. Shall I send the exact same pull to i915 and VFIO ?
> 
> Yes, exact same, this is important.
> 
> You were very close before, the only issue was rebasing
> topic/for-christoph instead of merging.
>
I will send it by the end of the day. Thanks so much for your patience.
 
> Jason
> 

Re: [Intel-gfx] [PULL] gvt-next

[Wang, Zhi A]

On 4/20/22 12:13 PM, Jason Gunthorpe wrote:
> On Wed, Apr 20, 2022 at 08:04:31AM +0000, Wang, Zhi A wrote:
>> Hi folks:
>>
>> Here is the PR of gvt-next.
>>
>> Mostly it includes the patch bundle of GVT-g re-factor patches for adapting 
>> the GVT-g with the
>> new MDEV interfaces:
>>
>> - Separating the MMIO table from GVT-g. (Zhi)
>> - GVT-g re-factor. (Christoph)
>> - GVT-g mdev API cleanup. (Jason)
>> - GVT-g trace/makefile cleanup. (Jani)
>>
>> Thanks so much for making this happen.
>>
>> This PR has been tested as following and no problem shows up:
>>
>> $dim update-branches
>> $dim apply-pull drm-intel-next < this_email.eml
>>
>> The following changes since commit b39d2c6202426b560641e5800c5523851b5db586:
>>
>>   drm/i915/fbc: Call intel_fbc_activate() directly from frontbuffer flush 
>> (2022-04-13 17:20:49 +0300)
> 
> ??
> 
> Why did you rebase this again? It needs to be on a rc release tag as
> you had in your github, not whatever this is.
> 
Hi Jason:

Here is what I understand, the pull going to the drm-intel-next needs to be 
based on drm-intel-next from the branch gvt-next. The pull going to the VFIO 
needs to be based on -rc, the topic/for-christoph brnach. So I did a merge from 
topic/for-christoph to my gvt-next branch and sent this pull, so that our QA 
can test these bundle on the gvt-staging branch.

Sorry this is way too complicated to me. Let me prepare the new pull as what 
you ask. Shall I send the exact same pull to i915 and VFIO ?

Thanks,
Zhi.

> Jason
> 

[Intel-gfx] [PULL] gvt-next

[Wang, Zhi A]

Hi folks:

Here is the PR of gvt-next.

Mostly it includes the patch bundle of GVT-g re-factor patches for adapting the 
GVT-g with the
new MDEV interfaces:

- Separating the MMIO table from GVT-g. (Zhi)
- GVT-g re-factor. (Christoph)
- GVT-g mdev API cleanup. (Jason)
- GVT-g trace/makefile cleanup. (Jani)

Thanks so much for making this happen.

This PR has been tested as following and no problem shows up:

$dim update-branches
$dim apply-pull drm-intel-next < this_email.eml

The following changes since commit b39d2c6202426b560641e5800c5523851b5db586:

  drm/i915/fbc: Call intel_fbc_activate() directly from frontbuffer flush 
(2022-04-13 17:20:49 +0300)

are available in the Git repository at:

  https://github.com/intel/gvt-linux tags/gvt-next-2022-04-20

for you to fetch changes up to 888471711a80b22c53547f3a625f20f487714f28:

  vfio/mdev: Remove mdev drvdata (2022-04-20 03:20:16 -0400)


gvt-next-2022-04-20

- Separating the MMIO table from GVT-g. (Zhi)
- GVT-g re-factor. (Christoph)
- GVT-g mdev API cleanup. (Jason)
- GVT-g trace/makefile cleanup. (Jani)


Christoph Hellwig (27):
  drm/i915/gvt: remove module refcounting in 
intel_gvt_{,un}register_hypervisor
  drm/i915/gvt: remove enum hypervisor_type
  drm/i915/gvt: rename intel_vgpu_ops to intel_vgpu_mdev_ops
  drm/i915/gvt: move the gvt code into kvmgt.ko
  drm/i915/gvt: remove intel_gvt_ops
  drm/i915/gvt: remove the map_gfn_to_mfn and set_trap_area ops
  drm/i915/gvt: remove the unused from_virt_to_mfn op
  drm/i915/gvt: merge struct kvmgt_vdev into struct intel_vgpu
  drm/i915/gvt: merge struct kvmgt_guest_info into strut intel_vgpu
  drm/i915/gvt: remove vgpu->handle
  drm/i915/gvt: devirtualize ->{read,write}_gpa
  drm/i915/gvt: devirtualize ->{get,put}_vfio_device
  drm/i915/gvt: devirtualize ->set_edid and ->set_opregion
  drm/i915/gvt: devirtualize ->detach_vgpu
  drm/i915/gvt: devirtualize ->inject_msi
  drm/i915/gvt: devirtualize ->is_valid_gfn
  drm/i915/gvt: devirtualize ->gfn_to_mfn
  drm/i915/gvt: devirtualize ->{enable,disable}_page_track
  drm/i915/gvt: devirtualize ->dma_{,un}map_guest_page
  drm/i915/gvt: devirtualize dma_pin_guest_page
  drm/i915/gvt: remove struct intel_gvt_mpt
  drm/i915/gvt: remove the extra vfio_device refcounting for dmabufs
  drm/i915/gvt: streamline intel_vgpu_create
  drm/i915/gvt: pass a struct intel_vgpu to the vfio read/write helpers
  drm/i915/gvt: remove kvmgt_guest_{init,exit}
  drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev
  drm/i915/gvt: merge gvt.c into kvmgvt.c

Jani Nikula (2):
  drm/i915/gvt: fix trace TRACE_INCLUDE_PATH
  drm/i915/gvt: better align the Makefile with i915 Makefile

Jason Gunthorpe (5):
  vfio/mdev: Remove vfio_mdev.c
  vfio/mdev: Remove mdev_parent_ops dev_attr_groups
  vfio/mdev: Remove mdev_parent_ops
  vfio/mdev: Use the driver core to create the 'remove' file
  vfio/mdev: Remove mdev drvdata

Zhi Wang (3):
  i915/gvt: Separate the MMIO tracking table from GVT-g
  i915/gvt: Save the initial HW state snapshot in i915
  i915/gvt: Use the initial HW state snapshot saved in i915

 Documentation/driver-api/vfio-mediated-device.rst |   27 +-
 drivers/gpu/drm/i915/Kconfig  |   36 +-
 drivers/gpu/drm/i915/Makefile |8 +-
 drivers/gpu/drm/i915/gvt/Makefile |   30 +-
 drivers/gpu/drm/i915/gvt/cfg_space.c  |   89 +-
 drivers/gpu/drm/i915/gvt/cmd_parser.c |4 +-
 drivers/gpu/drm/i915/gvt/dmabuf.c |   36 +-
 drivers/gpu/drm/i915/gvt/execlist.c   |   12 +-
 drivers/gpu/drm/i915/gvt/firmware.c   |   25 +-
 drivers/gpu/drm/i915/gvt/gtt.c|   55 +-
 drivers/gpu/drm/i915/gvt/gvt.c|  340 --
 drivers/gpu/drm/i915/gvt/gvt.h|  128 +-
 drivers/gpu/drm/i915/gvt/handlers.c   | 1033 +++-
 drivers/gpu/drm/i915/gvt/hypercall.h  |   82 --
 drivers/gpu/drm/i915/gvt/interrupt.c  |   40 +-
 drivers/gpu/drm/i915/gvt/kvmgt.c  | 1097 +
 drivers/gpu/drm/i915/gvt/mmio.c   |4 +-
 drivers/gpu/drm/i915/gvt/mmio.h   |1 -
 drivers/gpu/drm/i915/gvt/mpt.h|  400 ---
 drivers/gpu/drm/i915/gvt/opregion.c   |  148 +--
 drivers/gpu/drm/i915/gvt/page_track.c |8 +-
 drivers/gpu/drm/i915/gvt/reg.h|9 +-
 drivers/gpu/drm/i915/gvt/scheduler.c  |   37 +-
 drivers/gpu/drm/i915/gvt/trace.h  |2 +-
 drivers/gpu/drm/i915/gvt/vgpu.c   |   22 +-
 drivers/gpu/drm/i915/i915_driver.c|7 -
 

Re: [Intel-gfx] refactor the i915 GVT support and move to the modern mdev API v3

[Wang, Zhi A]

On 4/20/22 7:08 AM, Christoph Hellwig wrote:
> On Thu, Apr 14, 2022 at 11:38:59AM -0300, Jason Gunthorpe wrote:
>> pull requests can flow through more than one tree concurrently. The
>> purpose of the topic branch is to allow all the commits to be in all
>> the trees they need to be in at once.
>>
>> So you should send this branch as a PR to the next logical upstream
>> tree gvt patches normally go through, in the usual way that you send
>> PRs. Especially in this case where there is a small merge conflict
>> internal to DRM to resolve. I'm assuming this is the drm-intel-next
>> tree?
>>
>> Once DRM is internally happy then VFIO can merge it as well. You can
>> view VFIO as the secondary path to Linus, DRM as primary. Alex will
>> mention in his pull request that VFIO has a 'shared branch with DRM
>> for gvt'.
> 
> Where do we stand here?  The (somewhat misnamed) topic/for-christoph
> branch looks fine to me now except for the mіssing "static inline" on
> the intel_gvt_iterate_mmio_table stub.
> 
> When can we expect it in the i915 tree and linux-next?
> 
Qur QA finished the test yesterday and I just made a tag. The pull
request is going to be sent today. Yes, I will fix that.

Thanks,
Zhi.

Re: [Intel-gfx] refactor the i915 GVT support and move to the modern mdev API v3

[Wang, Zhi A]

On 4/14/22 1:43 PM, Jason Gunthorpe wrote:
> On Thu, Apr 14, 2022 at 04:40:11PM +0300, Jani Nikula wrote:
> 
>> git clone https://github.com/intel/gvt-linux -b for-christoph
>
> There are alot of extra commits on there - is it possible to base this
> straight on rc1 not on some kind of existing DRM tree?
>
> Why did you choose drm/i915/fbc: Call intel_fbc_activate() directly
> from frontbuffer flush  as a base?
>
> Jason
>

 Hi Jason:

 I updated the branch. You can check if those are what you are expecting. :)
>>>
>>> This is better, except for the first commit:
>>>
>>>  [DON'T PULL] drm/i915/dmc: split out dmc registers to a separate file
>>>  THIS PATCH WILL GO THROUGH DRM-INTEL-NEXT TO UPSTREAM
>>>
>>>  Clean up the massive i915_reg.h a bit with this isolated set of
>>>  registers.
>>>
>>>  v2: Remove stale comment (Lucas)
>>>
>>> Clean the commit message and send that as a proper PR to
>>> drm-intel-next, then everything else is OK.
>>
>> It's already in drm-intel-next, I guess the problem is basing the branch
>> on something that doesn't have it. I'd probably just base everything
>> cleanly on -rc1, and whoever does the merge between the two will need to
>> account for the missing include in the result. It's just adding one line
>> in the right place.
> 
> That makes sense to me, especially if you can do the merge fixup
> internally in DRM.
> 
> So drop the '[DONT PULL]' commit and send a PR to the next DRM tree -
> when that is confirmed send the same PR to vfio,

I updated the branch again, but I am confused. What is the purpose of sending
the PR to next DRM tree? I suppose all the patches will go through VFIO? If
I understand correctly?
> 
> Thanks,
> Jason
> 

Re: [Intel-gfx] refactor the i915 GVT support and move to the modern mdev API v3

[Wang, Zhi A]

On 4/14/22 1:34 PM, Jason Gunthorpe wrote:
> On Thu, Apr 14, 2022 at 12:20:42PM +0000, Wang, Zhi A wrote:
>> On 4/13/22 11:20 PM, Jason Gunthorpe wrote:
>>> On Wed, Apr 13, 2022 at 11:13:06PM +, Wang, Zhi A wrote:
>>>> Hi folks:
>>>>
>>>> Thanks so much for the efforts. I prepared a branch which contains all our 
>>>> patches.The aim of the branch is for the VFIO maintainers to pull the 
>>>> whole bunch easily after the drm-intel-next got merged through drm (as one 
>>>> of the MMIO patches depends on a patch in drm-intel-next).
>>>>
>>>> I dropped patch 4 and patch 5 as they have been covered by Jani's patches. 
>>>> Some conflicts was solved.
>>>> QA is going to test it today. 
>>>>
>>>> You can find it here:
>>>>
>>>> git clone https://github.com/intel/gvt-linux -b for-christoph
>>>
>>> There are alot of extra commits on there - is it possible to base this
>>> straight on rc1 not on some kind of existing DRM tree?
>>>
>>> Why did you choose drm/i915/fbc: Call intel_fbc_activate() directly
>>> from frontbuffer flush  as a base?
>>>
>>> Jason
>>>
>>
>> Hi Jason:
>>
This one belongs to i915, which has already been queued in drm-intel-next, but
not yet reached to the top. When it is landed in -rc, I will rebase this branch
on it, then we can drop this patch in this branch.

>> I updated the branch. You can check if those are what you are expecting. :)
> 
> This is better, except for the first commit:
> 
>  [DON'T PULL] drm/i915/dmc: split out dmc registers to a separate file
>  THIS PATCH WILL GO THROUGH DRM-INTEL-NEXT TO UPSTREAM
> 
>  Clean up the massive i915_reg.h a bit with this isolated set of
>  registers.
> 
>  v2: Remove stale comment (Lucas)
> 
> Clean the commit message and send that as a proper PR to
> drm-intel-next, then everything else is OK.
> 
> Jason
> 

Re: [Intel-gfx] refactor the i915 GVT support and move to the modern mdev API v3

[Wang, Zhi A]

On 4/13/22 11:20 PM, Jason Gunthorpe wrote:
> On Wed, Apr 13, 2022 at 11:13:06PM +0000, Wang, Zhi A wrote:
>> Hi folks:
>>
>> Thanks so much for the efforts. I prepared a branch which contains all our 
>> patches.The aim of the branch is for the VFIO maintainers to pull the whole 
>> bunch easily after the drm-intel-next got merged through drm (as one of the 
>> MMIO patches depends on a patch in drm-intel-next).
>>
>> I dropped patch 4 and patch 5 as they have been covered by Jani's patches. 
>> Some conflicts was solved.
>> QA is going to test it today. 
>>
>> You can find it here:
>>
>> git clone https://github.com/intel/gvt-linux -b for-christoph
> 
> There are alot of extra commits on there - is it possible to base this
> straight on rc1 not on some kind of existing DRM tree?
> 
> Why did you choose drm/i915/fbc: Call intel_fbc_activate() directly
> from frontbuffer flush  as a base?
> 
> Jason
> 

Hi Jason:

I updated the branch. You can check if those are what you are expecting. :)

Thanks,
Zhi.

Re: [Intel-gfx] refactor the i915 GVT support and move to the modern mdev API v3

[Wang, Zhi A]

Hi folks:

Thanks so much for the efforts. I prepared a branch which contains all our 
patches.The aim of the branch is for the VFIO maintainers to pull the whole 
bunch easily after the drm-intel-next got merged through drm (as one of the 
MMIO patches depends on a patch in drm-intel-next).

I dropped patch 4 and patch 5 as they have been covered by Jani's patches. Some 
conflicts was solved.
QA is going to test it today. 

You can find it here:

git clone https://github.com/intel/gvt-linux -b for-christoph

Thanks,
Zhi.

On 4/13/22 3:58 PM, Jani Nikula wrote:
> On Wed, 13 Apr 2022, Christoph Hellwig  wrote:
>> On Wed, Apr 13, 2022 at 01:47:05PM +0000, Wang, Zhi A wrote:
>>>> the GVT code in the i915 is a bit of a mess right now due to strange
>>>> abstractions and lots of indirect calls.  This series refactors various
>>>> bits to clean that up.  The main user visible change is that almost all
>>>> of the GVT code moves out of the main i915 driver and into the kvmgt
>>>> module.
>>>
>>> Hi Christoph:
>>>
>>> Do you want me to merge the GVT-g patches in this series? Or you want them 
>>> to get merged from your side?
>>
>> The two option here are drm tree via gvt and i915 trees or the vfio
>> tree, neither of which really is my tree.
>>
>> We already have a fair bit of vfio changes at the tail end of the series,
>> and Jason has some more that should sit on top of it, and I have some
>> more that I haven't sent yet.
>>
>> So if we could get the MMIO table and Makefile cleanups into a topic
>> branch that we could pull into the vfio tree and merge it through that
>> that would seem easiest to me, assuming that is ok with the i915, drm
>> and vfio maintainers.
> 
> AFAICS the changes are mostly to gvt/, and at least I'm fine with the
> minor changes to i915 (in this series and in my two patches) being
> merged via whichever tree you all see fit.
> 
> Acked-by: Jani Nikula 
> 
> Joonas, Tvrtko, Rodrigo, chime in now if you have any issues with that.
> 
> 
> BR,
> Jani.
> 
> 

Re: [Intel-gfx] [PATCH 1/9] vfio: Make vfio_(un)register_notifier accept a vfio_device

[Wang, Zhi A]

On 4/13/22 8:04 PM, Jason Gunthorpe wrote:
> On Wed, Apr 13, 2022 at 07:17:52PM +0000, Wang, Zhi A wrote:
>> On 4/13/22 5:37 PM, Jason Gunthorpe wrote:
>>> On Wed, Apr 13, 2022 at 06:29:46PM +0200, Christoph Hellwig wrote:
>>>> On Wed, Apr 13, 2022 at 01:18:14PM -0300, Jason Gunthorpe wrote:
>>>>> Yeah, I was thinking about that too, but on the other hand I think it
>>>>> is completely wrong that gvt requires kvm at all. A vfio_device is not
>>>>> supposed to be tightly linked to KVM - the only exception possibly
>>>>> being s390..
>>>>
>>>> So i915/gvt uses it for:
>>>>
>>>>  - poking into the KVM GFN translations
>>>>  - using the KVM page track notifier
>>>>
>>>> No idea how these could be solved in a more generic way.
>>>
>>> TBH I'm not sure how any of this works fully correctly..
>>>
>>> I see this code getting something it calls a GFN and then passing
>>> them to vfio - which makes no sense. Either a value is a GFN - the
>>> physical memory address of the VM, or it is an IOVA. VFIO only takes
>>> in IOVA and kvm only takes in GFN. So these are probably IOVAs really..
>>>
>> Can you let me know the place? So that I can take a look.
> 
> Well, for instance:
> 
> static int gvt_pin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn,
>   unsigned long size, struct page **page)
> 
> There is no way that is a GFN, it is an IOVA.
> 
I see. The name is vague. There is an promised 1:1 mapping between guest GFN
and host IOVA when a PCI device is passed to a VM, I guess mdev is just
leveraging it as they are sharing the same code path in QEMU. It's in a
function called vfio_listener_region_add() in the source code of QEMU.
Are you planning to change the architecture? It would be nice to know your plan.

>>> It seems the purpose is to shadow a page table, and it is capturing
>>> user space CPU writes to this page table memory I guess?
>>>
>> Yes.The shadow page will be built according to the guest GPU page table.
>> When a guest workload is executed in the GPU, the root pointer of the
>> shadow page table in the shadow GPU context is used. If the host enables
>> the IOMMU, the pages used by the shadow page table needs to be mapped as
>> IOVA, and the PFNs in the shadow entries are IOVAs.
> 
> So if the page table in the guest has IOVA addreses then why can you
> use them as GFNs?
> 
That's another problem. We don't support a guess enabling the guest IOMMU
(aka virtual IOMMU). The guest/virtual IOMMU is implemented in QEMU, so
does the translation between guest IOVA and GFN. For a mdev model
implemented in the kernel, there isn't any mechanism so far to reach there.

People were discussing it before. But none agreement was achieved. Is it
possible to implement it in the kernel? Would like to discuss more about it
if there is any good idea.

> Or is it that only the page table levels themselves are GFNs and the
> actual DMA's are IOVA? The unclear mixing of GFN as IOVA in the code
> makes it inscrutible.
>

No. Even the HW is capable of controlling the level of translation, but
it's not used like this in the existing driver. It's definitely an
architecture open.
 
> Jason
> 

Re: [Intel-gfx] [PATCH 1/9] vfio: Make vfio_(un)register_notifier accept a vfio_device

[Wang, Zhi A]

On 4/13/22 5:37 PM, Jason Gunthorpe wrote:
> On Wed, Apr 13, 2022 at 06:29:46PM +0200, Christoph Hellwig wrote:
>> On Wed, Apr 13, 2022 at 01:18:14PM -0300, Jason Gunthorpe wrote:
>>> Yeah, I was thinking about that too, but on the other hand I think it
>>> is completely wrong that gvt requires kvm at all. A vfio_device is not
>>> supposed to be tightly linked to KVM - the only exception possibly
>>> being s390..
>>
>> So i915/gvt uses it for:
>>
>>  - poking into the KVM GFN translations
>>  - using the KVM page track notifier
>>
>> No idea how these could be solved in a more generic way.
> 
> TBH I'm not sure how any of this works fully correctly..
> 
> I see this code getting something it calls a GFN and then passing
> them to vfio - which makes no sense. Either a value is a GFN - the
> physical memory address of the VM, or it is an IOVA. VFIO only takes
> in IOVA and kvm only takes in GFN. So these are probably IOVAs really..
> 
Can you let me know the place? So that I can take a look.

> But then, I see this code taking GFNs (which are probably IOVAs?) and
> passing them to the kvm page track notifier? That can't be right, VFIO
> needs to translate the IOVA to a GFN, not assume 1:1...
> 
GFNs are from the guest page table. It takes the GFN from a entry belongs
to a guest page table and request the kvm_page_track to track it, so that
the shadow page table can be updated accordingly.

> It seems the purpose is to shadow a page table, and it is capturing
> user space CPU writes to this page table memory I guess?
> 
Yes.The shadow page will be built according to the guest GPU page table.
When a guest workload is executed in the GPU, the root pointer of the
shadow page table in the shadow GPU context is used. If the host enables
the IOMMU, the pages used by the shadow page table needs to be mapped as
IOVA, and the PFNs in the shadow entries are IOVAs.

> GFN's seems to come from gen8_gtt_get_pfn which seems to be parsing
> some guest page table?
> 
Yes. It's to extract the PFNs from a page table entry.

> Jason
> 

Re: [Intel-gfx] [PATCH 05/34] drm/i915/gvt: cleanup the Makefile

[Wang, Zhi A]

On 4/13/22 1:43 PM, Jason Gunthorpe wrote:
> On Wed, Apr 13, 2022 at 01:39:35PM +0000, Wang, Zhi A wrote:
> 
>> It seems Jani's makefile clean patch has already included this one, I can
>> just simply drop this one so that Christoph won't need to re-send everything.
>>
>> For the branch to move on, I am merging the patches and will re-generate the
>> gvt-staging branch, which combines the newest drm-tip vfio-upstream and other
>> gvt branches.
>>
>> If you are in a rush of re-basing the patches of non-GVT-g stuff, you can use
>> gvt-staging branch until my pull request landed in drm-intel-next.
>>
>> Also our QA will test gvt-staging-branch before the pull request. I suppose
>> it will take one or two days.
> 
> When you are wrangling the branches it would be great if Christoph's
> series and it's minimal dependencies could be on a single branch that
> could reasonably be pulled to the VFIO tree too, thanks
> 
> Jason
> 

Hi Jason:

I am thinking about the process of merging process. Here are the dependence:

1) My patches depend on one patch in drm-intel/drm-intel-next. So it has to
go through drm.
My patches of GVT-g will go through drm-intel-next -> drm -> upstream. 

2) Christoph's patches depends on my patches, but part of them are for VFIO.

a. If they are fully going through VFIO repo, they might have to wait my
patches to get landed first.

b. If only the GVT-g parts goes through GVT repo, and rest of them goes
through VFIO, the rest part still needs to wait.

What would be a better process?

Thanks,
Zhi.

Re: [Intel-gfx] refactor the i915 GVT support and move to the modern mdev API v3

[Wang, Zhi A]

On 4/11/22 2:13 PM, Christoph Hellwig wrote:
> Hi all,
> 
> the GVT code in the i915 is a bit of a mess right now due to strange
> abstractions and lots of indirect calls.  This series refactors various
> bits to clean that up.  The main user visible change is that almost all
> of the GVT code moves out of the main i915 driver and into the kvmgt
> module.

Hi Christoph:

Do you want me to merge the GVT-g patches in this series? Or you want them to 
get merged from your side?

Thanks,
Zhi.

> 
> Tested on my Thinkpad with a Kaby Lake CPU and integrated graphics.
> 
> Git tree:
> 
> git://git.infradead.org/users/hch/misc.git i915-gvt
> 
> Gitweb:
> 
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/i915-gvt
> 
> Changes since v2:
>  - rebased on top of Linx 5.18-rc +
>"Refactor GVT-g MMIO tracking table and handlers"
>  - don't fold the gvt Makefile into the main Makefile
>  - add the mdev patches to remove the legacy interface that is now
>unused to the end of the series
> 
> Changes since v1:
>  - rebased on Linux 5.15
>  - allow the kvmgvt module to be loaded at any time and thus solve
>the deadlock when both i915 amd kvmgvt are modular
>  - include the conversion to the modern mdev API
> 
> Note that I do expect to rebased this again against 5.16-rc1 once
> released, but I'd like to get this out for review ASAP.
> 
> Diffstat:
>  b/drivers/gpu/drm/i915/Kconfig  |   33 
>  b/drivers/gpu/drm/i915/Makefile |   31 
>  b/drivers/gpu/drm/i915/gvt/cfg_space.c  |   89 --
>  b/drivers/gpu/drm/i915/gvt/cmd_parser.c |4 
>  b/drivers/gpu/drm/i915/gvt/dmabuf.c |   36 -
>  b/drivers/gpu/drm/i915/gvt/execlist.c   |   12 
>  b/drivers/gpu/drm/i915/gvt/gtt.c|   55 +
>  b/drivers/gpu/drm/i915/gvt/gvt.h|  125 ++-
>  b/drivers/gpu/drm/i915/gvt/interrupt.c  |   38 +
>  b/drivers/gpu/drm/i915/gvt/kvmgt.c  | 1099 
> +++-
>  b/drivers/gpu/drm/i915/gvt/mmio.c   |4 
>  b/drivers/gpu/drm/i915/gvt/opregion.c   |  148 
>  b/drivers/gpu/drm/i915/gvt/page_track.c |8 
>  b/drivers/gpu/drm/i915/gvt/scheduler.c  |   37 -
>  b/drivers/gpu/drm/i915/gvt/trace.h  |2 
>  b/drivers/gpu/drm/i915/gvt/vgpu.c   |   22 
>  b/drivers/gpu/drm/i915/i915_drv.c   |7 
>  b/drivers/gpu/drm/i915/i915_drv.h   |1 
>  b/drivers/gpu/drm/i915/i915_trace.h |1 
>  b/drivers/gpu/drm/i915/intel_gvt.c  |  162 +++-
>  b/drivers/gpu/drm/i915/intel_gvt.h  |   17 
>  drivers/gpu/drm/i915/gvt/Makefile   |9 
>  drivers/gpu/drm/i915/gvt/gvt.c  |  340 -
>  drivers/gpu/drm/i915/gvt/hypercall.h|   82 --
>  drivers/gpu/drm/i915/gvt/mpt.h  |  400 ---
>  25 files changed, 929 insertions(+), 1833 deletions(-)
> 

Re: [Intel-gfx] [PATCH 0/2] drm/i915/gvt: clean up makefile

[Wang, Zhi A]

On 4/13/22 12:25 PM, Jani Nikula wrote:
> This is [1] rebased on gvt-next. (Which means it won't build on CI.)
> 
> BR,
> Jani.
> 
> 
> [1] https://patchwork.freedesktop.org/series/102003/
> 
> 
> Cc: Zhi Wang 
> Cc: Christoph Hellwig 
> 
> 
> Jani Nikula (2):
>   drm/i915/gvt: fix trace TRACE_INCLUDE_PATH
>   drm/i915/gvt: better align the Makefile with i915 Makefile
> 
>  drivers/gpu/drm/i915/Makefile |  6 +++---
>  drivers/gpu/drm/i915/gvt/Makefile | 30 +++---
>  drivers/gpu/drm/i915/gvt/trace.h  |  2 +-
>  3 files changed, 27 insertions(+), 11 deletions(-)
> 

Thanks so much for the patch. 
Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH 05/34] drm/i915/gvt: cleanup the Makefile

[Wang, Zhi A]

On 4/13/22 12:33 PM, Jani Nikula wrote:
> On Mon, 11 Apr 2022, Christoph Hellwig  wrote:
>> On Mon, Apr 11, 2022 at 07:11:03PM +0300, Jani Nikula wrote:
 Up to you but I usually sort these lists
>>>
>>> Yeah, please do. Otherwise matches what I sent, so ack.
>>
>> Let's just merge your 2 patch series ASAP and I'll rebase on top of
>> that.
> 
> I rebased them on top of current gvt-next and resent [1]. Zhenyu, Zhi,
> please pick them up if you approve.
> 
>> What branch in drm-intel should I use as the base for the next version
>> btw?  Or does gvt go through yet another tree?
> 
> It's yet another tree... Basically gvt is developed on top of gvt-next
> (see MAINTAINERS) and Zhenyu and Zhi send pull requests for
> drm-intel-next.
> 
Hi Jani and Christoph:

It seems Jani's makefile clean patch has already included this one, I can
just simply drop this one so that Christoph won't need to re-send everything.

For the branch to move on, I am merging the patches and will re-generate the
gvt-staging branch, which combines the newest drm-tip vfio-upstream and other
gvt branches.

If you are in a rush of re-basing the patches of non-GVT-g stuff, you can use
gvt-staging branch until my pull request landed in drm-intel-next.

Also our QA will test gvt-staging-branch before the pull request. I suppose
it will take one or two days.

Again, thanks so much for making this happen. 

Thanks,
Zhi.
> > BR,
> Jani.
> 
> 
> [1] https://lore.kernel.org/all/cover.1649852517.git.jani.nik...@intel.com
> 

Re: [Intel-gfx] [PATCH v9 1/3] i915/gvt: Separate the MMIO tracking table from GVT-g

[Wang, Zhi A]

Hi Jani:

Previously when I sent a pull request, it will be top of a tag in 
drm-intel-next. The following patches move the DMC related registers, which is 
used by GVT-g into intel_dmc_regs.h and it is not included in the latest tag. 

commit 9c67d9e84c7d4a3a2371a54ee2dddc4699002000
Author: Jani Nikula 
Date:   Wed Mar 30 14:34:17 2022 +0300

drm/i915/dmc: split out dmc registers to a separate file

Clean up the massive i915_reg.h a bit with this isolated set of
registers.

v2: Remove stale comment (Lucas)

Signed-off-by: Jani Nikula 
Reviewed-by: Lucas De Marchi 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20220330113417.220964-3-jani.nik...@intel.com

If I still sent the pull request based on the latest tag, after the pull got 
merged, the compiling of the GVT-g module will be broken, as a new header needs 
to be included.

Can I sent my pull request not based on a tag in drm-intel-next, just the 
latest drm-intel-next?

Thanks,
Zhi.

On 4/13/22 9:31 AM, Jani Nikula wrote:
> On Fri, 08 Apr 2022, "Wang, Zhi A"  wrote:
>> Hi Jani:
>>
>> Thanks so much for the help. Can you generate a new tag on
>> drm-intel-next? I noticed that there was one patch moving the DMC
>> related registers into display/intel_dmc_regs.h, which is not included
>> in the latest tag on drm-intel-next.
> 
> Sorry, I'm not sure what you're asking exactly. We do tags when we
> create pull requests for drm-next, but I don't see the connection to
> gvt.
> 
> BR,
> Jani.
> 
>>
>> Guess it would be better that I can change this patch according to it
>> when checking in. This would prevent a conflict in future.
>>
>> Thanks,
>> Zhi.
>>
>> On 4/7/22 3:03 PM, Jani Nikula wrote:
>>> On Thu, 07 Apr 2022, Zhi Wang  wrote:
>>>> diff --git a/drivers/gpu/drm/i915/intel_gvt.h 
>>>> b/drivers/gpu/drm/i915/intel_gvt.h
>>>> index d7d3fb6186fd..7665d7cf0bdd 100644
>>>> --- a/drivers/gpu/drm/i915/intel_gvt.h
>>>> +++ b/drivers/gpu/drm/i915/intel_gvt.h
>>>> @@ -26,7 +26,17 @@
>>>>  
>>>>  struct drm_i915_private;
>>>>  
>>>> +#include 
>>>
>>> You only need . Please add it before the forward
>>> declaration above.
>>>
>>>> +
>>>>  #ifdef CONFIG_DRM_I915_GVT
>>>> +
>>>> +struct intel_gvt_mmio_table_iter {
>>>> +  struct drm_i915_private *i915;
>>>> +  void *data;
>>>> +  int (*handle_mmio_cb)(struct intel_gvt_mmio_table_iter *iter,
>>>> +u32 offset, u32 size);
>>>> +};
>>>> +
>>>>  int intel_gvt_init(struct drm_i915_private *dev_priv);
>>>>  void intel_gvt_driver_remove(struct drm_i915_private *dev_priv);
>>>>  int intel_gvt_init_device(struct drm_i915_private *dev_priv);
>>>> @@ -34,6 +44,7 @@ void intel_gvt_clean_device(struct drm_i915_private 
>>>> *dev_priv);
>>>>  int intel_gvt_init_host(void);
>>>>  void intel_gvt_sanitize_options(struct drm_i915_private *dev_priv);
>>>>  void intel_gvt_resume(struct drm_i915_private *dev_priv);
>>>> +int intel_gvt_iterate_mmio_table(struct intel_gvt_mmio_table_iter *iter);
>>>>  #else
>>>>  static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
>>>>  {
>>>> @@ -51,6 +62,16 @@ static inline void intel_gvt_sanitize_options(struct 
>>>> drm_i915_private *dev_priv)
>>>>  static inline void intel_gvt_resume(struct drm_i915_private *dev_priv)
>>>>  {
>>>>  }
>>>> +
>>>> +unsigned long intel_gvt_get_device_type(struct drm_i915_private *i915)
>>>> +{
>>>> +  return 0;
>>>> +}
>>>
>>> The CONFIG_DRM_I915_GVT=y counterpart for this is in mmio.h. Should be
>>> both in the same header.
>>>
>>>> +
>>>> +int intel_gvt_iterate_mmio_table(struct intel_gvt_mmio_table_iter *iter)
>>>> +{
>>>> +  return 0;
>>>> +}
>>>>  #endif
>>>>  
>>>>  #endif /* _INTEL_GVT_H_ */
>>>> diff --git a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c 
>>>> b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
>>>> new file mode 100644
>>>> index ..d29491a6d209
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
>>>> @@ -0,0 +1,1290 @@
>>>> +// SPDX-License-Identifier: MIT
>>>> +/*
>>>> + * Copyright © 2020 Intel Corporation
>>>> + */
>>>> +
>>>> +#include "i915_drv.h"
>>>> +#include "i915_reg.h"
>>>> +#include "display/vlv_dsi_pll_regs.h"
>>>> +#include "gt/intel_gt_regs.h"
>>>> +#include "intel_mchbar_regs.h"
>>>> +#include "i915_pvinfo.h"
>>>> +#include "intel_gvt.h"
>>>> +#include "gvt/gvt.h"
>>>
>>> Generally we have the include lists sorted.
>>>
>>> Other than the nitpicks above, the series is
>>>
>>> Acked-by: Jani Nikula 
>>>
>>>
>>> BR,
>>> Jani.
>>>
>>>
>>
> 

Re: [Intel-gfx] [PATCH v9 1/3] i915/gvt: Separate the MMIO tracking table from GVT-g

[Wang, Zhi A]

Ping. :)

On 4/8/22 2:07 PM, Zhi Wang wrote:
> Hi Jani:
> 
> Thanks so much for the help. Can you generate a new tag on drm-intel-next? I 
> noticed that there was one patch moving the DMC related registers into 
> display/intel_dmc_regs.h, which is not included in the latest tag on 
> drm-intel-next.
> 
> Guess it would be better that I can change this patch according to it when 
> checking in. This would prevent a conflict in future.
> 
> Thanks,
> Zhi.
> 
> On 4/7/22 3:03 PM, Jani Nikula wrote:
>> On Thu, 07 Apr 2022, Zhi Wang  wrote:
>>> diff --git a/drivers/gpu/drm/i915/intel_gvt.h 
>>> b/drivers/gpu/drm/i915/intel_gvt.h
>>> index d7d3fb6186fd..7665d7cf0bdd 100644
>>> --- a/drivers/gpu/drm/i915/intel_gvt.h
>>> +++ b/drivers/gpu/drm/i915/intel_gvt.h
>>> @@ -26,7 +26,17 @@
>>>  
>>>  struct drm_i915_private;
>>>  
>>> +#include 
>>
>> You only need . Please add it before the forward
>> declaration above.
>>
>>> +
>>>  #ifdef CONFIG_DRM_I915_GVT
>>> +
>>> +struct intel_gvt_mmio_table_iter {
>>> +   struct drm_i915_private *i915;
>>> +   void *data;
>>> +   int (*handle_mmio_cb)(struct intel_gvt_mmio_table_iter *iter,
>>> + u32 offset, u32 size);
>>> +};
>>> +
>>>  int intel_gvt_init(struct drm_i915_private *dev_priv);
>>>  void intel_gvt_driver_remove(struct drm_i915_private *dev_priv);
>>>  int intel_gvt_init_device(struct drm_i915_private *dev_priv);
>>> @@ -34,6 +44,7 @@ void intel_gvt_clean_device(struct drm_i915_private 
>>> *dev_priv);
>>>  int intel_gvt_init_host(void);
>>>  void intel_gvt_sanitize_options(struct drm_i915_private *dev_priv);
>>>  void intel_gvt_resume(struct drm_i915_private *dev_priv);
>>> +int intel_gvt_iterate_mmio_table(struct intel_gvt_mmio_table_iter *iter);
>>>  #else
>>>  static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
>>>  {
>>> @@ -51,6 +62,16 @@ static inline void intel_gvt_sanitize_options(struct 
>>> drm_i915_private *dev_priv)
>>>  static inline void intel_gvt_resume(struct drm_i915_private *dev_priv)
>>>  {
>>>  }
>>> +
>>> +unsigned long intel_gvt_get_device_type(struct drm_i915_private *i915)
>>> +{
>>> +   return 0;
>>> +}
>>
>> The CONFIG_DRM_I915_GVT=y counterpart for this is in mmio.h. Should be
>> both in the same header.
>>
>>> +
>>> +int intel_gvt_iterate_mmio_table(struct intel_gvt_mmio_table_iter *iter)
>>> +{
>>> +   return 0;
>>> +}
>>>  #endif
>>>  
>>>  #endif /* _INTEL_GVT_H_ */
>>> diff --git a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c 
>>> b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
>>> new file mode 100644
>>> index ..d29491a6d209
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
>>> @@ -0,0 +1,1290 @@
>>> +// SPDX-License-Identifier: MIT
>>> +/*
>>> + * Copyright © 2020 Intel Corporation
>>> + */
>>> +
>>> +#include "i915_drv.h"
>>> +#include "i915_reg.h"
>>> +#include "display/vlv_dsi_pll_regs.h"
>>> +#include "gt/intel_gt_regs.h"
>>> +#include "intel_mchbar_regs.h"
>>> +#include "i915_pvinfo.h"
>>> +#include "intel_gvt.h"
>>> +#include "gvt/gvt.h"
>>
>> Generally we have the include lists sorted.
>>
>> Other than the nitpicks above, the series is
>>
>> Acked-by: Jani Nikula 
>>
>>
>> BR,
>> Jani.
>>
>>
> 

Re: [Intel-gfx] [PATCH v9 1/3] i915/gvt: Separate the MMIO tracking table from GVT-g

[Wang, Zhi A]

Hi Jani:

Thanks so much for the help. Can you generate a new tag on drm-intel-next? I 
noticed that there was one patch moving the DMC related registers into 
display/intel_dmc_regs.h, which is not included in the latest tag on 
drm-intel-next.

Guess it would be better that I can change this patch according to it when 
checking in. This would prevent a conflict in future.

Thanks,
Zhi.

On 4/7/22 3:03 PM, Jani Nikula wrote:
> On Thu, 07 Apr 2022, Zhi Wang  wrote:
>> diff --git a/drivers/gpu/drm/i915/intel_gvt.h 
>> b/drivers/gpu/drm/i915/intel_gvt.h
>> index d7d3fb6186fd..7665d7cf0bdd 100644
>> --- a/drivers/gpu/drm/i915/intel_gvt.h
>> +++ b/drivers/gpu/drm/i915/intel_gvt.h
>> @@ -26,7 +26,17 @@
>>  
>>  struct drm_i915_private;
>>  
>> +#include 
> 
> You only need . Please add it before the forward
> declaration above.
> 
>> +
>>  #ifdef CONFIG_DRM_I915_GVT
>> +
>> +struct intel_gvt_mmio_table_iter {
>> +struct drm_i915_private *i915;
>> +void *data;
>> +int (*handle_mmio_cb)(struct intel_gvt_mmio_table_iter *iter,
>> +  u32 offset, u32 size);
>> +};
>> +
>>  int intel_gvt_init(struct drm_i915_private *dev_priv);
>>  void intel_gvt_driver_remove(struct drm_i915_private *dev_priv);
>>  int intel_gvt_init_device(struct drm_i915_private *dev_priv);
>> @@ -34,6 +44,7 @@ void intel_gvt_clean_device(struct drm_i915_private 
>> *dev_priv);
>>  int intel_gvt_init_host(void);
>>  void intel_gvt_sanitize_options(struct drm_i915_private *dev_priv);
>>  void intel_gvt_resume(struct drm_i915_private *dev_priv);
>> +int intel_gvt_iterate_mmio_table(struct intel_gvt_mmio_table_iter *iter);
>>  #else
>>  static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
>>  {
>> @@ -51,6 +62,16 @@ static inline void intel_gvt_sanitize_options(struct 
>> drm_i915_private *dev_priv)
>>  static inline void intel_gvt_resume(struct drm_i915_private *dev_priv)
>>  {
>>  }
>> +
>> +unsigned long intel_gvt_get_device_type(struct drm_i915_private *i915)
>> +{
>> +return 0;
>> +}
> 
> The CONFIG_DRM_I915_GVT=y counterpart for this is in mmio.h. Should be
> both in the same header.
> 
>> +
>> +int intel_gvt_iterate_mmio_table(struct intel_gvt_mmio_table_iter *iter)
>> +{
>> +return 0;
>> +}
>>  #endif
>>  
>>  #endif /* _INTEL_GVT_H_ */
>> diff --git a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c 
>> b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
>> new file mode 100644
>> index ..d29491a6d209
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
>> @@ -0,0 +1,1290 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2020 Intel Corporation
>> + */
>> +
>> +#include "i915_drv.h"
>> +#include "i915_reg.h"
>> +#include "display/vlv_dsi_pll_regs.h"
>> +#include "gt/intel_gt_regs.h"
>> +#include "intel_mchbar_regs.h"
>> +#include "i915_pvinfo.h"
>> +#include "intel_gvt.h"
>> +#include "gvt/gvt.h"
> 
> Generally we have the include lists sorted.
> 
> Other than the nitpicks above, the series is
> 
> Acked-by: Jani Nikula 
> 
> 
> BR,
> Jani.
> 
> 

Re: [Intel-gfx] [PATCH v9 1/3] i915/gvt: Separate the MMIO tracking table from GVT-g

[Wang, Zhi A]

Thanks so much!

Jani and Joonas, it would be better to have one rb from i915 maintainers as 
this patches also modify i915 code.

Thanks,
Zhi.

On 4/7/22 8:06 AM, Christoph Hellwig wrote:
> The whole series looks good and works fine on by Kaby Lake Thinkpad:
> 
> Reviewed-by: Christoph Hellwig 
> Tested-by: Christoph Hellwig 
> 

Re: [Intel-gfx] [PATCH v7 1/3] i915/gvt: Separate the MMIO tracking table from GVT-g

[Wang, Zhi A]

Hi Jani:

Thanks so much for the guidance. :) I included it in the v8 patch. :)

Thanks,
Zhi.

On 3/31/22 8:25 AM, Jani Nikula wrote:
> On Thu, 31 Mar 2022, "Wang, Zhi A"  wrote:
>> Hi Jani and Joonas:
>>
>> Are you OK with these patches? I noticed I need to change the license
>> of the new file. Will do that when check-in if you are OK with these.
> 
> Use SPDX license header instead of the full text?
> 
> I don't know much about the actual contents, I'll leave that part to
> others.
> 
> Seems that you are dropping const in a number of places where I thought
> you could perhaps retain it.
> 
> Also in drivers/gpu/drm/i915/intel_gvt_mmio_table.c:
> 
> #include "gvt.h" 
> 
> looks bad. It should be "gvt/gvt.h". I realize you can do that because
> gvt/Makefile has:
> 
> ccflags-y += -I $(srctree)/$(src) -I $(srctree)/$(src)/$(GVT_DIR)/
> 
> which I think should be removed.
> 
> I sent patches fixing this to give you an idea. No need to queue them
> first, I can rebase them later. But please make sure this builds without
> the ccflags.
> 
> 
> BR,
> Jani.
> 
> 
>>
>> Thanks,
>> Zhi.
>>
>> On 3/28/22 6:50 AM, Christoph Hellwig wrote:
>>> On Fri, Mar 25, 2022 at 01:52:49PM -0400, Zhi Wang wrote:
>>>>
>>>> v7:
>>>>
>>>> - Keep the marcos of device generation in GVT-g. (Christoph, Jani)
>>>
>>> The changelog go under the "---" line (also for the other patches).
>>>
>>> Otherwise looks good:
>>>
>>> Reviewed-by: Christoph Hellwig 
>>>
>>
> 

Re: [Intel-gfx] [PATCH v7 1/3] i915/gvt: Separate the MMIO tracking table from GVT-g

[Wang, Zhi A]

Hi Chris:

Thanks for the testing. Can you attach the dmesg? I tested mostly on my skylake 
desktop with some 3D workload.

Thanks,
Zhi.

On 3/31/22 7:42 AM, Christoph Hellwig wrote:
> On Thu, Mar 31, 2022 at 07:11:07AM +0000, Wang, Zhi A wrote:
>> Hi Jani and Joonas:
>>
>> Are you OK with these patches? I noticed I need to change the license of the 
>> new file. Will do that when check-in if you are OK with these.
> 
> So I actually ended up testing the patches, and first they fail to
> compile against current mainline (trivial fix attached), but then
> the guest (also latest mainline as of today) also hangs during the
> initialization of th i915 driver on i7-8550U-based Thinkpad.  Plain
> mainline as the host boots fine, but spews a lot of warnings.
> 
> Host and guest configs are also attached.
> 

Re: [Intel-gfx] [PATCH v7 1/3] i915/gvt: Separate the MMIO tracking table from GVT-g

[Wang, Zhi A]

Hi Jani and Joonas:

Are you OK with these patches? I noticed I need to change the license of the 
new file. Will do that when check-in if you are OK with these.

Thanks,
Zhi.

On 3/28/22 6:50 AM, Christoph Hellwig wrote:
> On Fri, Mar 25, 2022 at 01:52:49PM -0400, Zhi Wang wrote:
>>
>> v7:
>>
>> - Keep the marcos of device generation in GVT-g. (Christoph, Jani)
> 
> The changelog go under the "---" line (also for the other patches).
> 
> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig 
> 

Re: [Intel-gfx] [PATCH] drm/i915/reg: Fix spelling mistake "Unsupport" -> "Unsupported"

[Wang, Zhi A]

Thanks for the contribution, Colin. Queued.

On 3/15/22 8:24 PM, Colin Ian King wrote:
> There is a spelling mistake in a gvt_vgpu_err error message. Fix it.
> 
> 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 520a7e1942f3..a01e3a893e24 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -914,7 +914,7 @@ static int update_fdi_rx_iir_status(struct intel_vgpu 
> *vgpu,
>   else if (FDI_RX_IMR_TO_PIPE(offset) != INVALID_INDEX)
>   index = FDI_RX_IMR_TO_PIPE(offset);
>   else {
> - gvt_vgpu_err("Unsupport registers %x\n", offset);
> + gvt_vgpu_err("Unsupported registers %x\n", offset);
>   return -EINVAL;
>   }
>  
> 
Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH v6 1/3] i915/gvt: Introduce the mmio table to support VFIO new mdev API

[Wang, Zhi A]

I was actually testing it for almost two weeks, but still I met some hang and I 
was trying to figure where the problem is as this is quite a big change. Let's 
see if I can figure it out this week.

Thanks,
Zhi.

On 3/15/22 8:48 AM, Christoph Hellwig wrote:
> On Tue, Mar 15, 2022 at 10:46:53AM +0200, Jani Nikula wrote:
>> On Tue, 15 Mar 2022, Christoph Hellwig  wrote:
>>> Just curious, what is the state of this seris?  It would be good to
>>> have it ready early on for the next merge window as there is quite
>>> a backlog that depends on it.
>>
>> Can't speak for the status of the series, but for drm the deadline for
>> changes headed for the merge window is around -rc5/-rc6 timeframe
>> i.e. this has already missed the upcoming merge window.
> 
> I know.  I meant the next one, not the one ending now.  And I don't
> want to miss another one.
> 

Re: [Intel-gfx] [GIT PULL] GVT next changes for drm-intel-next

[Wang, Zhi A]

Hi Joonas:

Thanks so much. Will do. :)

Thanks,
Zhi.

On 3/8/22 3:19 PM, Joonas Lahtinen wrote:
> Quoting Wang, Zhi A (2022-03-08 12:07:04)
>> Which suits better for you? For me, I am OK with both. If you are not in a 
>> rush of closing the window, I can submit through drm-intel-next-fixes.
> 
> I pulled this into drm-intel-next-fixes now.
> 
> For future reference, let's have fixes only PRs as gvt-fixes and PRs
> with features as gvt-next and each as a new mail thread instead of a
> reply to older, so they will be easy to spot :)
> 
> Regards, Joonas
> 
>>
>> Thanks,
>> Zhi.
>>
>> On 3/8/22 9:56 AM, Nikula, Jani wrote:
>>> On Tue, 08 Mar 2022, "Wang, Zhi A"  wrote:
>>>> Hi folks:
>>>>
>>>> Here is a new pull request of gvt-next. It contains a small patch to add 
>>>> the missing
>>>> mdev attribute name, which will be used by the middleware, like kubevirt.
>>>
>>> I'm wondering if I should pull this to drm-intel-next, which is already
>>> targeting v5.19, or if it should be pulled to drm-intel-next-fixes
>>> targeting v5.18. It does look like a fix.
>>>
>>> BR,
>>> Jani.
>>>
>>>
>>>>
>>>> This pull has been tested by:
>>>>
>>>> $ dim apply-pull drm-intel-next < this_email.eml
>>>>
>>>> The following changes since commit 
>>>> 30424ebae8df0f786835e7a31ad790fa00764f35:
>>>>
>>>>   Merge tag 'drm-intel-gt-next-2022-02-17' of 
>>>> git://anongit.freedesktop.org/drm/drm-intel into drm-intel-next 
>>>> (2022-02-23 15:03:51 -0500)
>>>>
>>>> are available in the Git repository at:
>>>>
>>>>   https://github.com/intel/gvt-linux tags/gvt-next-2022-03-07
>>>>
>>>> for you to fetch changes up to 43d26c4fc6c446d766253d546f0083d78023d34a:
>>>>
>>>>   drm/i915/gvt: add the missing mdev attribute "name" (2022-03-07 12:21:58 
>>>> -0500)
>>>>
>>>> 
>>>> - add the missing attribute "name" in VFIO mdev hierarchy.
>>>>
>>>> 
>>>> Zhi Wang (1):
>>>>   drm/i915/gvt: add the missing mdev attribute "name"
>>>>
>>>>  drivers/gpu/drm/i915/gvt/kvmgt.c | 15 +++
>>>>  1 file changed, 15 insertions(+)
>>>>
>>>
>>

Re: [Intel-gfx] [GIT PULL] GVT next changes for drm-intel-next

[Wang, Zhi A]

Which suits better for you? For me, I am OK with both. If you are not in a rush 
of closing the window, I can submit through drm-intel-next-fixes.

Thanks,
Zhi.

On 3/8/22 9:56 AM, Nikula, Jani wrote:
> On Tue, 08 Mar 2022, "Wang, Zhi A"  wrote:
>> Hi folks:
>>
>> Here is a new pull request of gvt-next. It contains a small patch to add the 
>> missing
>> mdev attribute name, which will be used by the middleware, like kubevirt.
> 
> I'm wondering if I should pull this to drm-intel-next, which is already
> targeting v5.19, or if it should be pulled to drm-intel-next-fixes
> targeting v5.18. It does look like a fix.
> 
> BR,
> Jani.
> 
> 
>>
>> This pull has been tested by:
>>
>> $ dim apply-pull drm-intel-next < this_email.eml
>>
>> The following changes since commit 30424ebae8df0f786835e7a31ad790fa00764f35:
>>
>>   Merge tag 'drm-intel-gt-next-2022-02-17' of 
>> git://anongit.freedesktop.org/drm/drm-intel into drm-intel-next (2022-02-23 
>> 15:03:51 -0500)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/intel/gvt-linux tags/gvt-next-2022-03-07
>>
>> for you to fetch changes up to 43d26c4fc6c446d766253d546f0083d78023d34a:
>>
>>   drm/i915/gvt: add the missing mdev attribute "name" (2022-03-07 12:21:58 
>> -0500)
>>
>> 
>> - add the missing attribute "name" in VFIO mdev hierarchy.
>>
>> 
>> Zhi Wang (1):
>>   drm/i915/gvt: add the missing mdev attribute "name"
>>
>>  drivers/gpu/drm/i915/gvt/kvmgt.c | 15 +++
>>  1 file changed, 15 insertions(+)
>>
> 

[Intel-gfx] [GIT PULL] GVT next changes for drm-intel-next

[Wang, Zhi A]

Hi folks:

Here is a new pull request of gvt-next. It contains a small patch to add the 
missing
mdev attribute name, which will be used by the middleware, like kubevirt.

This pull has been tested by:

$ dim apply-pull drm-intel-next < this_email.eml

The following changes since commit 30424ebae8df0f786835e7a31ad790fa00764f35:

  Merge tag 'drm-intel-gt-next-2022-02-17' of 
git://anongit.freedesktop.org/drm/drm-intel into drm-intel-next (2022-02-23 
15:03:51 -0500)

are available in the Git repository at:

  https://github.com/intel/gvt-linux tags/gvt-next-2022-03-07

for you to fetch changes up to 43d26c4fc6c446d766253d546f0083d78023d34a:

  drm/i915/gvt: add the missing mdev attribute "name" (2022-03-07 12:21:58 
-0500)


- add the missing attribute "name" in VFIO mdev hierarchy.


Zhi Wang (1):
  drm/i915/gvt: add the missing mdev attribute "name"

 drivers/gpu/drm/i915/gvt/kvmgt.c | 15 +++
 1 file changed, 15 insertions(+)

Re: [Intel-gfx] [PATCH] drm/i915/gvt: #include drm_edid.h for drm_edid_block_valid()

[Wang, Zhi A]

On 2/15/22 12:20 PM, Nikula, Jani wrote:
> As the excessive #includes from i915_drv.h were axed, kvmgt.c build
> started failing. Add the necessary #include where needed.
> 
> Reported-by: Stephen Rothwell 
> Fixes: 14da21cc4671 ("drm/i915: axe lots of unnecessary includes from 
> i915_drv.h")
> Cc: Tvrtko Ursulin 
> Cc: Zhenyu Wang 
> Cc: Zhi Wang 
> Cc: intel-gvt-...@lists.freedesktop.org
> Signed-off-by: Jani Nikula 
> 
> ---
> 
> Going to merge this via drm-intel-next along with the regressing commit.
> ---
>  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 20b82fb036f8..e8d6c76e9234 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -46,6 +46,8 @@
>  
>  #include 
>  
> +#include 
> +
>  #include "i915_drv.h"
>  #include "gvt.h"
>  
> 
Hi:

Thanks for the notification!

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [GVT PULL] gvt-fixes for drm-intel-fixes

[Wang, Zhi A]

Feel free to let me know if I need to re-base on the newest tag since it has 
been quite some time.

On 2/10/22 8:51 AM, Jani Nikula wrote:
> 
> +Tvrtko
> 
> On Wed, 09 Feb 2022, "Wang, Zhi A"  wrote:
>> Hi folks:
>>
>> Ping. This pull seems not got merged.
>>
>> Thanks,
>> Zhi.
>>
>> -Original Message-
>> From: Zhi Wang  
>> Sent: Saturday, January 15, 2022 12:46 PM
>> To: Vivi, Rodrigo ; jani.nik...@linux.intel.com; 
>> joonas.lahti...@linux.intel.com
>> Cc: intel-gvt-...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; 
>> Wang, Zhi A 
>> Subject: [GVT PULL] gvt-fixes for drm-intel-fixes
>>
>> Hi folks:
>>
>> Here is the gvt-fixes pull for drm-intel-fixes. It contains:
>>
>> - Make DRM_I915_GVT depend on X86 (Siva Mullati)
>> - Clean kernel doc in gtt.c (Randy Dunlap)
>>
>> This pull has been tested by: dim apply-pull drm-intel-fixes < this_email.eml
>>
>> Zhi.
>>
>> The following changes since commit d46f329a3f6048e04736e86cb13c880645048792:
>>
>>   drm/i915: Increment composite fence seqno (2021-12-27 11:33:40 +0200)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/intel/gvt-linux.git tags/gvt-fixes-2022-01-13
>>
>> for you to fetch changes up to d72d69abfdb6e0375981cfdda8eb45143f12c77d:
>>
>>   drm/i915/gvt: Make DRM_I915_GVT depend on X86 (2022-01-13 18:13:12 +)
>>
>> 
>> gvt-fixes-2022-01-13
>>
>> - Make DRM_I915_GVT depend on X86 (Siva Mullati)
>> - Clean kernel doc in gtt.c (Randy Dunlap)
>>
>> 
>> Randy Dunlap (1):
>>   drm/i915/gvt: clean up kernel-doc in gtt.c
>>
>> Siva Mullati (1):
>>   drm/i915/gvt: Make DRM_I915_GVT depend on X86
>>
>>  drivers/gpu/drm/i915/Kconfig   | 1 +
>>  drivers/gpu/drm/i915/gvt/gtt.c | 4 ++--
>>  2 files changed, 3 insertions(+), 2 deletions(-)
> 

Re: [Intel-gfx] [PATCH v6 1/3] i915/gvt: Introduce the mmio table to support VFIO new mdev API

[Wang, Zhi A]

On 2/9/22 9:04 AM, Jani Nikula wrote:
> On Wed, 09 Feb 2022, Christoph Hellwig  wrote:
>> On Tue, Feb 08, 2022 at 05:15:00PM +0200, Jani Nikula wrote:
  #ifdef CONFIG_DRM_I915_GVT
 +
 +#define D_BDW   (1 << 0)
 +#define D_SKL (1 << 1)
 +#define D_KBL (1 << 2)
 +#define D_BXT (1 << 3)
 +#define D_CFL (1 << 4)
 +
 +#define D_GEN9PLUS(D_SKL | D_KBL | D_BXT | D_CFL)
 +#define D_GEN8PLUS(D_BDW | D_SKL | D_KBL | D_BXT | D_CFL)
 +
 +#define D_SKL_PLUS(D_SKL | D_KBL | D_BXT | D_CFL)
 +#define D_BDW_PLUS(D_BDW | D_SKL | D_KBL | D_BXT | D_CFL)
 +
 +#define D_PRE_SKL (D_BDW)
 +#define D_ALL (D_BDW | D_SKL | D_KBL | D_BXT | D_CFL)
>>>
>>> If these really need to be in a header in i915/, I think they need to be
>>> longer with some namespacing or something. I do wish these could be
>>> hidden though.
>>
>> I think we could actually kill them off entirely.  They are used as
>> arguments to the macros that setup the mmio table.
>>
>> Thefunctions to build these tabls are already organized by families,
>> so we'd need relatively few conditions to just build them the right
>> way.  There also are some runtime checks in the callbacks, but they
>> seem entirely superflous as far as I can tell.
>>
>> Only the cmd parser is a bit messy.  So maybe we could keep these
>> constants just for the cmd parser inside of gvt for now (and clean
>> that up later) and remove them entirely from the mmio table.
> 
> I'm fine with cleaning this up in follow-up, provided the follow-up
> actually happens! ;)

Thanks so much for the comments and the support. :)

> 
> BR,
> Jani.
> 
> 

Re: [Intel-gfx] [PATCH v6 1/3] i915/gvt: Introduce the mmio table to support VFIO new mdev API

[Wang, Zhi A]

On 2/9/22 7:28 AM, Christoph Hellwig wrote:
> On Tue, Feb 08, 2022 at 05:15:00PM +0200, Jani Nikula wrote:
>>>  #ifdef CONFIG_DRM_I915_GVT
>>> +
>>> +#define D_BDW   (1 << 0)
>>> +#define D_SKL  (1 << 1)
>>> +#define D_KBL  (1 << 2)
>>> +#define D_BXT  (1 << 3)
>>> +#define D_CFL  (1 << 4)
>>> +
>>> +#define D_GEN9PLUS (D_SKL | D_KBL | D_BXT | D_CFL)
>>> +#define D_GEN8PLUS (D_BDW | D_SKL | D_KBL | D_BXT | D_CFL)
>>> +
>>> +#define D_SKL_PLUS (D_SKL | D_KBL | D_BXT | D_CFL)
>>> +#define D_BDW_PLUS (D_BDW | D_SKL | D_KBL | D_BXT | D_CFL)
>>> +
>>> +#define D_PRE_SKL  (D_BDW)
>>> +#define D_ALL  (D_BDW | D_SKL | D_KBL | D_BXT | D_CFL)
>>
>> If these really need to be in a header in i915/, I think they need to be
>> longer with some namespacing or something. I do wish these could be
>> hidden though.
> 
> I think we could actually kill them off entirely.  They are used as
> arguments to the macros that setup the mmio table.
> 
> Thefunctions to build these tabls are already organized by families,
> so we'd need relatively few conditions to just build them the right
> way.  There also are some runtime checks in the callbacks, but they
> seem entirely superflous as far as I can tell.
> 
> Only the cmd parser is a bit messy.  So maybe we could keep these
> constants just for the cmd parser inside of gvt for now (and clean
> that up later) and remove them entirely from the mmio table.
> 
I agree that's the correct way for not exporting this to i915 by just 
organizing them in the functions, like what you said.
But I guess it's also matter of time and schedule as well. If we go that 
direction, it might take longer time for coding as
this is a big re-factor. Also, we need our QA to do another full test run. That 
needs to be considered. (If we are ok with that)

Besides, we have to have a methodology to make sure everything is the same as 
before.
Currently I am comparing the numbers of tracked mmio and the mmio snapshot. It 
would be nice to have more insight. :)

Thanks,
Zhi.

Re: [Intel-gfx] [PATCH v6 2/3] i915/gvt: Save the initial HW state snapshot in i915

[Wang, Zhi A]

On 2/9/22 7:32 AM, Christoph Hellwig wrote:
> On Tue, Feb 08, 2022 at 06:11:50AM -0500, Zhi Wang wrote:
>> +struct drm_i915_private *dev_priv = iter->i915;
>> +u32 *mmio, i;
>> +
>> +for (i = offset; i < offset + size; i += 4) {
>> +mmio = iter->data + i;
>> +*mmio = intel_uncore_read_notrace(to_gt(dev_priv)->uncore,
>> +  _MMIO(i));
> 
> This reads much stranger than:
> 
>   u32 *mmio = iter->data;
> 
>   for (i = offset; i < offset + size; i += 4) {
>   mmio[i] = intel_uncore_read_notrace(to_gt(dev_priv)->uncore,
>   _MMIO(i));
>   }
> 
I am not sure the suggestion is correct. That's the reason why I didn't take 
the comments in the previous review.

if mmio is u32 *, the step of mmio[0] -> mmio[1] will be 4, and i will be 
increased by 4 in each loop.
I guess the correct one would be mmio[i/4] = x; would that looks better? if 
yes, I will do that in the next version.

>> +static int handle_mmio(struct intel_gvt_mmio_table_iter *iter,
>> +   u32 offset, u32 device, u32 size)
>> +{
>> +if (WARN_ON(!IS_ALIGNED(offset, 4)))
>> +return -EINVAL;
> 
> Shouldn't this be in the caller of the method?
> 
>> +save_mmio(iter, offset, size);
>> +return 0;
> 
Yes. You are right. It's because I get rid of the mmio_block in intel_gvt.c

> Now that the block callback is gone save_mmio and handle_mmio
> can be merged.
> 
>> +mem = vzalloc(2 * SZ_1M);
> 
> Don't we want a driver-wide constant for this instead of a magic number?
> 

We actually have one in i915, but it's not exported. Should we export that one?

Thanks,
Zhi.

Re: [Intel-gfx] [GVT PULL] gvt-fixes for drm-intel-fixes

[Wang, Zhi A]

Hi folks:

Ping. This pull seems not got merged.

Thanks,
Zhi.

-Original Message-
From: Zhi Wang  
Sent: Saturday, January 15, 2022 12:46 PM
To: Vivi, Rodrigo ; jani.nik...@linux.intel.com; 
joonas.lahti...@linux.intel.com
Cc: intel-gvt-...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Wang, 
Zhi A 
Subject: [GVT PULL] gvt-fixes for drm-intel-fixes

Hi folks:

Here is the gvt-fixes pull for drm-intel-fixes. It contains:

- Make DRM_I915_GVT depend on X86 (Siva Mullati)
- Clean kernel doc in gtt.c (Randy Dunlap)

This pull has been tested by: dim apply-pull drm-intel-fixes < this_email.eml

Zhi.

The following changes since commit d46f329a3f6048e04736e86cb13c880645048792:

  drm/i915: Increment composite fence seqno (2021-12-27 11:33:40 +0200)

are available in the Git repository at:

  https://github.com/intel/gvt-linux.git tags/gvt-fixes-2022-01-13

for you to fetch changes up to d72d69abfdb6e0375981cfdda8eb45143f12c77d:

  drm/i915/gvt: Make DRM_I915_GVT depend on X86 (2022-01-13 18:13:12 +)


gvt-fixes-2022-01-13

- Make DRM_I915_GVT depend on X86 (Siva Mullati)
- Clean kernel doc in gtt.c (Randy Dunlap)


Randy Dunlap (1):
  drm/i915/gvt: clean up kernel-doc in gtt.c

Siva Mullati (1):
  drm/i915/gvt: Make DRM_I915_GVT depend on X86

 drivers/gpu/drm/i915/Kconfig   | 1 +
 drivers/gpu/drm/i915/gvt/gtt.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

Re: [Intel-gfx] [GIT PULL] GVT next changes for drm-intel-next-queued

[Wang, Zhi A]

On 1/11/22 2:40 PM, Vivi, Rodrigo wrote:
> On Tue, Jan 11, 2022 at 06:08:28AM -0500, Wang, Zhi A wrote:
>> On 1/11/22 6:08 AM, Wang, Zhi A wrote:
>>> On 1/11/2022 12:52 AM, Vivi, Rodrigo wrote:
>>>> On Fri, 2022-01-07 at 14:43 +, Wang, Zhi A wrote:
>>>>> Hi folks:
>>>>>
>>>>> Happy holidays! This pull mostly contains the code re-factors patches
>>>>> from Guenter Roeck and Rikard. Also a minor change from Zhenyu.
>>>>>
>>>>> Zhi
>>>>>
>>>>> The following changes since commit
>>>>> 3bfa7d40ce736ffbbfe07127061f54b359ee2b12:
>>>>>
>>>>> drm/i915/dg2: Add support for new DG2-G11 revid 0x5 (2021-08-06
>>>>> 09:03:10 -0700)
>>>>>
>>>>> are available in the Git repository at:
>>>>>
>>>>> https://github.com/intel/gvt-linux tags/gvt-next-2022-01-07
>>>>>
>>>>> for you to fetch changes up to
>>>>> d7a8585430f2b6df5960bbc305edcec5a55180f3:
>>>> I'm not sure what's going on here, but:
>>>>
>>>> dim: no pull request found
>>>>
>>>> did you do anything different on this round for generating and sending
>>>> out this pull request email?
>>> Strange.
>>>
>>> I generated this pull request by git pull-request since there is no
>>> support for generating pull-request for gvt-next right in dim now. Can
>>> you share me the command line of dim you were using for apply this pull
>>> request? I can try to apply it before sending the pull request.
>>>
>>> Thanks,
>>>
>>> Zhi.
>>>
>> Hi Vivi:
>>
>> I did some checks and dumped the plain source of the email I sent. I
>> guess I figured out the reason. It's the problem of thunderbird in
>> Windows. When it sends the plain email, it will replace some space with
>> "Â", which caused the dim cannot find the git repo url. I have no idea
>> how that can happen since Thunderbird in Linux worked totally fine with
>> the same settings.
>>
>> Before the vacation, my VPN certificate in Linux has been expired. I had
>> to use the thunderbird in Windows, which caused the problem above.
>>
>> Will re-sent. Sorry for the bumps.
> 
> understood. no problem at all. it happens.
> 
> Thanks for resending, but there's something else now...
> 
> dim attempt a mega rebase of thousands and thousands of patches
> when trying to apply this.
> 
> Could you please rebase on a more recent drm-intel-next tag?
> 
> Thanks,
> Rodrigo.
> 
Hi Vivi:

I sent the V3 just now. Thanks so much for the patience. I just took this over 
from Zhenyu recently. Feel free to let me know if I make any mistakes. 
Appreciate it!

Thanks,
Zhi.

>>
>> Zhi.
>>
>>>>> drm/i915/gvt: Constify vgpu_types (2021-12-16 09:13:02 -0500)
>>>>>
>>>>> 
>>>>> Guenter Roeck (1):
>>>>> drm/i915/gvt: Use list_entry to access list members
>>>>>
>>>>> Rikard Falkeborn (9):
>>>>> drm/i915/gvt: Constify intel_gvt_gtt_gma_ops
>>>>> drm/i915/gvt: Constify intel_gvt_gtt_pte_ops
>>>>> drm/i915/gvt: Constify intel_gvt_irq_ops
>>>>> drm/i915/gvt: Constify intel_gvt_sched_policy_ops
>>>>> drm/i915/gvt: Constify gvt_mmio_block
>>>>> drm/i915/gvt: Constify cmd_interrupt_events
>>>>> drm/i915/gvt: Constify formats
>>>>> drm/i915/gvt: Constify gtt_type_table_entry
>>>>> drm/i915/gvt: Constify vgpu_types
>>>>>
>>>>> Zhenyu Wang (1):
>>>>> drm/i915/gvt: Fix cmd parser error for Passmark9
>>>>>
>>>>>drivers/gpu/drm/i915/gvt/cmd_parser.c   |  2 +-
>>>>>drivers/gpu/drm/i915/gvt/dmabuf.c   | 18 +++--
>>>>>drivers/gpu/drm/i915/gvt/fb_decoder.c   | 24 ++--
>>>>>drivers/gpu/drm/i915/gvt/gtt.c  | 68
>>>>> -
>>>>>drivers/gpu/drm/i915/gvt/gtt.h  |  4 +-
>>>>>drivers/gpu/drm/i915/gvt/gvt.h  |  2 +-
>>>>>drivers/gpu/drm/i915/gvt/handlers.c | 13 ---
>>>>>drivers/gpu/drm/i915/gvt/interrupt.c| 10 ++---
>>>>>drivers/gpu/drm/i915/gvt/interrupt.h|  2 +-
>>>>>drivers/gpu/drm/i915/gvt/sched_policy.c |  2 +-
>>>>>drivers/gpu/drm/i915/gvt/scheduler.h|  2 +-
>>>>>drivers/gpu/drm/i915/gvt/vgpu.c |  4 +-
>>>>>12 files changed, 72 insertions(+), 79 deletions(-)
>>>>>
>>>
>>

[Intel-gfx] [GIT PULL RESEND] GVT next changes for drm-intel-next-queued

[Wang, Zhi A]

Hi folks:

Happy holidays! This pull mostly contains the code re-factors patches from 
Guenter Roeck and Rikard. Also a minor change from Zhenyu.

v3:

- Merge drm-intel-next-2021-12-14 into gvt-next

v2:

- Fix the problem of message body encoding in Thunderbird.

The following changes since commit 96db14432d979532be4cb6d5d52a127317e68b3f:

  drm/i915: Fix implicit use of struct pci_dev (2021-12-14 10:38:29 +0200)

are available in the Git repository at:

  https://github.com/intel/gvt-linux tags/gvt-next-2022-01-11

for you to fetch changes up to 5786a889b2c7a8374d5f843f7499db88cd09823b:

  drm/i915/gvt: Constify vgpu_types (2022-01-11 10:51:11 -0500)


Guenter Roeck (1):
  drm/i915/gvt: Use list_entry to access list members

Rikard Falkeborn (9):
  drm/i915/gvt: Constify intel_gvt_gtt_gma_ops
  drm/i915/gvt: Constify intel_gvt_gtt_pte_ops
  drm/i915/gvt: Constify intel_gvt_irq_ops
  drm/i915/gvt: Constify intel_gvt_sched_policy_ops
  drm/i915/gvt: Constify gvt_mmio_block
  drm/i915/gvt: Constify cmd_interrupt_events
  drm/i915/gvt: Constify formats
  drm/i915/gvt: Constify gtt_type_table_entry
  drm/i915/gvt: Constify vgpu_types

Zhenyu Wang (1):
  drm/i915/gvt: Fix cmd parser error for Passmark9

 drivers/gpu/drm/i915/gvt/cmd_parser.c   |  2 +-
 drivers/gpu/drm/i915/gvt/dmabuf.c   | 18 +-
 drivers/gpu/drm/i915/gvt/fb_decoder.c   | 24 
 drivers/gpu/drm/i915/gvt/gtt.c  | 68 
++--
 drivers/gpu/drm/i915/gvt/gtt.h  |  4 ++--
 drivers/gpu/drm/i915/gvt/gvt.h  |  2 +-
 drivers/gpu/drm/i915/gvt/handlers.c | 13 +++--
 drivers/gpu/drm/i915/gvt/interrupt.c| 10 +-
 drivers/gpu/drm/i915/gvt/interrupt.h|  2 +-
 drivers/gpu/drm/i915/gvt/sched_policy.c |  2 +-
 drivers/gpu/drm/i915/gvt/scheduler.h|  2 +-
 drivers/gpu/drm/i915/gvt/vgpu.c |  4 ++--
 12 files changed, 72 insertions(+), 79 deletions(-)

Re: [Intel-gfx] [PATCH] drm/i915/gvt: Make DRM_I915_GVT depend on X86

[Wang, Zhi A]

On 1/7/22 9:52 AM, Mullati Siva wrote:
> From: Siva Mullati 
> 
> GVT is not supported on non-x86 platforms, So add
> dependency of X86 on config parameter DRM_I915_GVT.
> 
> Signed-off-by: Siva Mullati 
> ---
>  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 a4c94dc2e216..cfd932514da2 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -101,6 +101,7 @@ config DRM_I915_USERPTR
>  config DRM_I915_GVT
>   bool "Enable Intel GVT-g graphics virtualization host support"
>   depends on DRM_I915
> + depends on X86
>   depends on 64BIT
>   default n
>   help
> 

Thanks so much for the fix. Queued.

[Intel-gfx] [GIT PULL RESEND] GVT next changes for drm-intel-next-queued

[Wang, Zhi A]

Hi folks:

Happy holidays! This pull mostly contains the code re-factors patches from 
Guenter Roeck and Rikard. Also a minor change from Zhenyu.

v2:

- Fix the problem of message body encoding in Thunderbird.

The following changes since commit 3bfa7d40ce736ffbbfe07127061f54b359ee2b12:

  drm/i915/dg2: Add support for new DG2-G11 revid 0x5 (2021-08-06 09:03:10 
-0700)

are available in the Git repository at:

  https://github.com/intel/gvt-linux tags/gvt-next-2022-01-07

for you to fetch changes up to d7a8585430f2b6df5960bbc305edcec5a55180f3:

  drm/i915/gvt: Constify vgpu_types (2021-12-16 09:13:02 -0500)


Guenter Roeck (1):
  drm/i915/gvt: Use list_entry to access list members

Rikard Falkeborn (9):
  drm/i915/gvt: Constify intel_gvt_gtt_gma_ops
  drm/i915/gvt: Constify intel_gvt_gtt_pte_ops
  drm/i915/gvt: Constify intel_gvt_irq_ops
  drm/i915/gvt: Constify intel_gvt_sched_policy_ops
  drm/i915/gvt: Constify gvt_mmio_block
  drm/i915/gvt: Constify cmd_interrupt_events
  drm/i915/gvt: Constify formats
  drm/i915/gvt: Constify gtt_type_table_entry
  drm/i915/gvt: Constify vgpu_types

Zhenyu Wang (1):
  drm/i915/gvt: Fix cmd parser error for Passmark9

 drivers/gpu/drm/i915/gvt/cmd_parser.c   |  2 +-
 drivers/gpu/drm/i915/gvt/dmabuf.c   | 18 +++--
 drivers/gpu/drm/i915/gvt/fb_decoder.c   | 24 ++--
 drivers/gpu/drm/i915/gvt/gtt.c  | 68 -
 drivers/gpu/drm/i915/gvt/gtt.h  |  4 +-
 drivers/gpu/drm/i915/gvt/gvt.h  |  2 +-
 drivers/gpu/drm/i915/gvt/handlers.c | 13 ---
 drivers/gpu/drm/i915/gvt/interrupt.c| 10 ++---
 drivers/gpu/drm/i915/gvt/interrupt.h|  2 +-
 drivers/gpu/drm/i915/gvt/sched_policy.c |  2 +-
 drivers/gpu/drm/i915/gvt/scheduler.h|  2 +-
 drivers/gpu/drm/i915/gvt/vgpu.c |  4 +-
 12 files changed, 72 insertions(+), 79 deletions(-)

Re: [Intel-gfx] [GIT PULL] GVT next changes for drm-intel-next-queued

[Wang, Zhi A]

On 1/11/22 6:08 AM, Wang, Zhi A wrote:
> On 1/11/2022 12:52 AM, Vivi, Rodrigo wrote:
>> On Fri, 2022-01-07 at 14:43 +0000, Wang, Zhi A wrote:
>>> Hi folks:
>>>
>>> Happy holidays! This pull mostly contains the code re-factors patches
>>> from Guenter Roeck and Rikard. Also a minor change from Zhenyu.
>>>
>>> Zhi
>>>
>>> The following changes since commit
>>> 3bfa7d40ce736ffbbfe07127061f54b359ee2b12:
>>>
>>>     drm/i915/dg2: Add support for new DG2-G11 revid 0x5 (2021-08-06
>>> 09:03:10 -0700)
>>>
>>> are available in the Git repository at:
>>>
>>>     https://github.com/intel/gvt-linux tags/gvt-next-2022-01-07
>>>
>>> for you to fetch changes up to
>>> d7a8585430f2b6df5960bbc305edcec5a55180f3:
>> I'm not sure what's going on here, but:
>>
>> dim: no pull request found
>>
>> did you do anything different on this round for generating and sending
>> out this pull request email?
> Strange.
> 
> I generated this pull request by git pull-request since there is no 
> support for generating pull-request for gvt-next right in dim now. Can 
> you share me the command line of dim you were using for apply this pull 
> request? I can try to apply it before sending the pull request.
> 
> Thanks,
> 
> Zhi.
> 
Hi Vivi:

I did some checks and dumped the plain source of the email I sent. I 
guess I figured out the reason. It's the problem of thunderbird in 
Windows. When it sends the plain email, it will replace some space with 
"Â", which caused the dim cannot find the git repo url. I have no idea 
how that can happen since Thunderbird in Linux worked totally fine with 
the same settings.

Before the vacation, my VPN certificate in Linux has been expired. I had 
to use the thunderbird in Windows, which caused the problem above.

Will re-sent. Sorry for the bumps.

Zhi.

>>>     drm/i915/gvt: Constify vgpu_types (2021-12-16 09:13:02 -0500)
>>>
>>> 
>>> Guenter Roeck (1):
>>>     drm/i915/gvt: Use list_entry to access list members
>>>
>>> Rikard Falkeborn (9):
>>>     drm/i915/gvt: Constify intel_gvt_gtt_gma_ops
>>>     drm/i915/gvt: Constify intel_gvt_gtt_pte_ops
>>>     drm/i915/gvt: Constify intel_gvt_irq_ops
>>>     drm/i915/gvt: Constify intel_gvt_sched_policy_ops
>>>     drm/i915/gvt: Constify gvt_mmio_block
>>>     drm/i915/gvt: Constify cmd_interrupt_events
>>>     drm/i915/gvt: Constify formats
>>>     drm/i915/gvt: Constify gtt_type_table_entry
>>>     drm/i915/gvt: Constify vgpu_types
>>>
>>> Zhenyu Wang (1):
>>>     drm/i915/gvt: Fix cmd parser error for Passmark9
>>>
>>>    drivers/gpu/drm/i915/gvt/cmd_parser.c   |  2 +-
>>>    drivers/gpu/drm/i915/gvt/dmabuf.c   | 18 +++--
>>>    drivers/gpu/drm/i915/gvt/fb_decoder.c   | 24 ++--
>>>    drivers/gpu/drm/i915/gvt/gtt.c  | 68
>>> -
>>>    drivers/gpu/drm/i915/gvt/gtt.h  |  4 +-
>>>    drivers/gpu/drm/i915/gvt/gvt.h  |  2 +-
>>>    drivers/gpu/drm/i915/gvt/handlers.c | 13 ---
>>>    drivers/gpu/drm/i915/gvt/interrupt.c    | 10 ++---
>>>    drivers/gpu/drm/i915/gvt/interrupt.h    |  2 +-
>>>    drivers/gpu/drm/i915/gvt/sched_policy.c |  2 +-
>>>    drivers/gpu/drm/i915/gvt/scheduler.h    |  2 +-
>>>    drivers/gpu/drm/i915/gvt/vgpu.c |  4 +-
>>>    12 files changed, 72 insertions(+), 79 deletions(-)
>>>
> 

Re: [Intel-gfx] [GIT PULL] GVT next changes for drm-intel-next-queued

[Wang, Zhi A]

On 1/11/2022 12:52 AM, Vivi, Rodrigo wrote:
> On Fri, 2022-01-07 at 14:43 +0000, Wang, Zhi A wrote:
>> Hi folks:
>>
>> Happy holidays! This pull mostly contains the code re-factors patches
>> from Guenter Roeck and Rikard. Also a minor change from Zhenyu.
>>
>> Zhi
>>
>> The following changes since commit
>> 3bfa7d40ce736ffbbfe07127061f54b359ee2b12:
>>
>> drm/i915/dg2: Add support for new DG2-G11 revid 0x5 (2021-08-06
>> 09:03:10 -0700)
>>
>> are available in the Git repository at:
>>
>> https://github.com/intel/gvt-linux tags/gvt-next-2022-01-07
>>
>> for you to fetch changes up to
>> d7a8585430f2b6df5960bbc305edcec5a55180f3:
> I'm not sure what's going on here, but:
>
> dim: no pull request found
>
> did you do anything different on this round for generating and sending
> out this pull request email?
Strange.

I generated this pull request by git pull-request since there is no 
support for generating pull-request for gvt-next right in dim now. Can 
you share me the command line of dim you were using for apply this pull 
request? I can try to apply it before sending the pull request.

Thanks,

Zhi.

>> drm/i915/gvt: Constify vgpu_types (2021-12-16 09:13:02 -0500)
>>
>> 
>> Guenter Roeck (1):
>> drm/i915/gvt: Use list_entry to access list members
>>
>> Rikard Falkeborn (9):
>> drm/i915/gvt: Constify intel_gvt_gtt_gma_ops
>> drm/i915/gvt: Constify intel_gvt_gtt_pte_ops
>> drm/i915/gvt: Constify intel_gvt_irq_ops
>> drm/i915/gvt: Constify intel_gvt_sched_policy_ops
>> drm/i915/gvt: Constify gvt_mmio_block
>> drm/i915/gvt: Constify cmd_interrupt_events
>> drm/i915/gvt: Constify formats
>> drm/i915/gvt: Constify gtt_type_table_entry
>> drm/i915/gvt: Constify vgpu_types
>>
>> Zhenyu Wang (1):
>> drm/i915/gvt: Fix cmd parser error for Passmark9
>>
>>drivers/gpu/drm/i915/gvt/cmd_parser.c   |  2 +-
>>drivers/gpu/drm/i915/gvt/dmabuf.c   | 18 +++--
>>drivers/gpu/drm/i915/gvt/fb_decoder.c   | 24 ++--
>>drivers/gpu/drm/i915/gvt/gtt.c  | 68
>> -
>>drivers/gpu/drm/i915/gvt/gtt.h  |  4 +-
>>drivers/gpu/drm/i915/gvt/gvt.h  |  2 +-
>>drivers/gpu/drm/i915/gvt/handlers.c | 13 ---
>>drivers/gpu/drm/i915/gvt/interrupt.c| 10 ++---
>>drivers/gpu/drm/i915/gvt/interrupt.h|  2 +-
>>drivers/gpu/drm/i915/gvt/sched_policy.c |  2 +-
>>drivers/gpu/drm/i915/gvt/scheduler.h|  2 +-
>>drivers/gpu/drm/i915/gvt/vgpu.c |  4 +-
>>12 files changed, 72 insertions(+), 79 deletions(-)
>>

[Intel-gfx] [GIT PULL] GVT next changes for drm-intel-next-queued

[Wang, Zhi A]

Hi folks:

Happy holidays! This pull mostly contains the code re-factors patches 
from Guenter Roeck and Rikard. Also a minor change from Zhenyu.

Zhi

The following changes since commit 3bfa7d40ce736ffbbfe07127061f54b359ee2b12:

   drm/i915/dg2: Add support for new DG2-G11 revid 0x5 (2021-08-06 
09:03:10 -0700)

are available in the Git repository at:

   https://github.com/intel/gvt-linux tags/gvt-next-2022-01-07

for you to fetch changes up to d7a8585430f2b6df5960bbc305edcec5a55180f3:

   drm/i915/gvt: Constify vgpu_types (2021-12-16 09:13:02 -0500)


Guenter Roeck (1):
   drm/i915/gvt: Use list_entry to access list members

Rikard Falkeborn (9):
   drm/i915/gvt: Constify intel_gvt_gtt_gma_ops
   drm/i915/gvt: Constify intel_gvt_gtt_pte_ops
   drm/i915/gvt: Constify intel_gvt_irq_ops
   drm/i915/gvt: Constify intel_gvt_sched_policy_ops
   drm/i915/gvt: Constify gvt_mmio_block
   drm/i915/gvt: Constify cmd_interrupt_events
   drm/i915/gvt: Constify formats
   drm/i915/gvt: Constify gtt_type_table_entry
   drm/i915/gvt: Constify vgpu_types

Zhenyu Wang (1):
   drm/i915/gvt: Fix cmd parser error for Passmark9

  drivers/gpu/drm/i915/gvt/cmd_parser.c   |  2 +-
  drivers/gpu/drm/i915/gvt/dmabuf.c   | 18 +++--
  drivers/gpu/drm/i915/gvt/fb_decoder.c   | 24 ++--
  drivers/gpu/drm/i915/gvt/gtt.c  | 68 
-
  drivers/gpu/drm/i915/gvt/gtt.h  |  4 +-
  drivers/gpu/drm/i915/gvt/gvt.h  |  2 +-
  drivers/gpu/drm/i915/gvt/handlers.c | 13 ---
  drivers/gpu/drm/i915/gvt/interrupt.c    | 10 ++---
  drivers/gpu/drm/i915/gvt/interrupt.h    |  2 +-
  drivers/gpu/drm/i915/gvt/sched_policy.c |  2 +-
  drivers/gpu/drm/i915/gvt/scheduler.h    |  2 +-
  drivers/gpu/drm/i915/gvt/vgpu.c |  4 +-
  12 files changed, 72 insertions(+), 79 deletions(-)

Re: [Intel-gfx] [PATCH v4 1/2] i915/gvt: Introduce the mmio_info_table.c to support VFIO new mdev API

[Wang, Zhi A]

On 11/30/2021 6:46 PM, Christoph Hellwig wrote:
> I still think this goes into the wrong direction.
>
> Something closer to your first version that also saves away the
> gvt->mmio.mmio_attribute flags in the core i915 module, and which
> splits the MMIO table into one that contains just the offset, size
> and flags (core i915) and one that has the read-only mask and handlers
> (gvt) would be much simpler and not create this super-tight coupling
> between core i915 and gvt.
>
> Bonus points for moving your new intel_gvt_hw_state structure out
> of struct intel_gvt and into struct i915_virtual_gpu.

Hi Christoph:

Sorry for the late reply as I am supporting the customers recently. I 
will refresh this after the christmas.

Thanks,

Zhi.

Re: [Intel-gfx] [PATCH 0/9] drm/i915/gvt: Constify static structs

[Wang, Zhi A]

On 12/12/2021 3:25 PM, Rikard Falkeborn worte:
> On Fri, Dec 10, 2021 at 09:00:56AM +0000, Wang, Zhi A wrote:
>> On 12/4/2021 12:55 PM, Rikard Falkeborn wrote:
>>> Constify a number of static structs that are never modified to allow the
>>> compiler to put them in read-only memory. In order to do this, constify a
>>> number of local variables and pointers in structs.
>>>
>>> This is most important for structs that contain function pointers, and
>>> the patches for those structs are placed first in the series.
>>>
>>> Rikard Falkeborn (9):
>>> drm/i915/gvt: Constify intel_gvt_gtt_pte_ops
>>> drm/i915/gvt: Constify intel_gvt_gtt_pte_ops
>>> drm/i915/gvt: Constify intel_gvt_irq_ops
>>> drm/i915/gvt: Constify intel_gvt_sched_policy_ops
>>> drm/i915/gvt: Constify gvt_mmio_block
>>> drm/i915/gvt: Constify cmd_interrupt_events
>>> drm/i915/gvt: Constify formats
>>> drm/i915/gvt: Constify gtt_type_table_entry
>>> drm/i915/gvt: Constify vgpu_types
>>>
>>>drivers/gpu/drm/i915/gvt/cmd_parser.c   |  2 +-
>>>drivers/gpu/drm/i915/gvt/fb_decoder.c   | 24 -
>>>drivers/gpu/drm/i915/gvt/gtt.c  | 68 -
>>>drivers/gpu/drm/i915/gvt/gtt.h  |  4 +-
>>>drivers/gpu/drm/i915/gvt/gvt.h  |  2 +-
>>>drivers/gpu/drm/i915/gvt/handlers.c | 12 ++---
>>>drivers/gpu/drm/i915/gvt/interrupt.c| 10 ++--
>>>drivers/gpu/drm/i915/gvt/interrupt.h|  2 +-
>>>drivers/gpu/drm/i915/gvt/sched_policy.c |  2 +-
>>>drivers/gpu/drm/i915/gvt/scheduler.h|  2 +-
>>>drivers/gpu/drm/i915/gvt/vgpu.c |  4 +-
>>>11 files changed, 66 insertions(+), 66 deletions(-)
>>>
>> Thanks so much for the contribuition. You only need to refine the PATCH
>> 2 a little bit and re-send it.
>>
> Thanks for reviewing. Just to clarify, did you mean patch 7 (since
> that's the one you commented on)? And is it enough to send just that
> patch or do you want the entire series resent?
>
> Rikard

Hi Rikard,  no worries then. I have already corrected them and queue 
them in the gvt-next branch, you can double check them if you like. They 
are going through a QA test cycle first and later start their journey to 
the upstream. All good. :) Thanks so much for the contribution again.

Zhi.

Re: [Intel-gfx] [PATCH 0/9] drm/i915/gvt: Constify static structs

[Wang, Zhi A]

On 12/4/2021 12:55 PM, Rikard Falkeborn wrote:
> Constify a number of static structs that are never modified to allow the
> compiler to put them in read-only memory. In order to do this, constify a
> number of local variables and pointers in structs.
>
> This is most important for structs that contain function pointers, and
> the patches for those structs are placed first in the series.
>
> Rikard Falkeborn (9):
>drm/i915/gvt: Constify intel_gvt_gtt_pte_ops
>drm/i915/gvt: Constify intel_gvt_gtt_pte_ops
>drm/i915/gvt: Constify intel_gvt_irq_ops
>drm/i915/gvt: Constify intel_gvt_sched_policy_ops
>drm/i915/gvt: Constify gvt_mmio_block
>drm/i915/gvt: Constify cmd_interrupt_events
>drm/i915/gvt: Constify formats
>drm/i915/gvt: Constify gtt_type_table_entry
>drm/i915/gvt: Constify vgpu_types
>
>   drivers/gpu/drm/i915/gvt/cmd_parser.c   |  2 +-
>   drivers/gpu/drm/i915/gvt/fb_decoder.c   | 24 -
>   drivers/gpu/drm/i915/gvt/gtt.c  | 68 -
>   drivers/gpu/drm/i915/gvt/gtt.h  |  4 +-
>   drivers/gpu/drm/i915/gvt/gvt.h  |  2 +-
>   drivers/gpu/drm/i915/gvt/handlers.c | 12 ++---
>   drivers/gpu/drm/i915/gvt/interrupt.c| 10 ++--
>   drivers/gpu/drm/i915/gvt/interrupt.h|  2 +-
>   drivers/gpu/drm/i915/gvt/sched_policy.c |  2 +-
>   drivers/gpu/drm/i915/gvt/scheduler.h|  2 +-
>   drivers/gpu/drm/i915/gvt/vgpu.c |  4 +-
>   11 files changed, 66 insertions(+), 66 deletions(-)
>
Thanks so much for the contribuition. You only need to refine the PATCH 
2 a little bit and re-send it.

Re: [Intel-gfx] [PATCH 1/9] drm/i915/gvt: Constify intel_gvt_gtt_pte_ops

[Wang, Zhi A]

On 12/4/2021 12:55 PM, Rikard Falkeborn wrote:
> These are never modified, so make them const to allow the compiler to
> put them in read-only memory.
>
> Signed-off-by: Rikard Falkeborn 
> ---
>   drivers/gpu/drm/i915/gvt/gtt.c | 4 ++--
>   drivers/gpu/drm/i915/gvt/gtt.h | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 53d0cb327539..6efa48727052 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -516,7 +516,7 @@ static struct intel_gvt_gtt_pte_ops gen8_gtt_pte_ops = {
>   .set_pfn = gen8_gtt_set_pfn,
>   };
>   
> -static struct intel_gvt_gtt_gma_ops gen8_gtt_gma_ops = {
> +static const struct intel_gvt_gtt_gma_ops gen8_gtt_gma_ops = {
>   .gma_to_ggtt_pte_index = gma_to_ggtt_pte_index,
>   .gma_to_pte_index = gen8_gma_to_pte_index,
>   .gma_to_pde_index = gen8_gma_to_pde_index,
> @@ -2097,7 +2097,7 @@ unsigned long intel_vgpu_gma_to_gpa(struct 
> intel_vgpu_mm *mm, unsigned long gma)
>   struct intel_vgpu *vgpu = mm->vgpu;
>   struct intel_gvt *gvt = vgpu->gvt;
>   struct intel_gvt_gtt_pte_ops *pte_ops = gvt->gtt.pte_ops;
> - struct intel_gvt_gtt_gma_ops *gma_ops = gvt->gtt.gma_ops;
> + const struct intel_gvt_gtt_gma_ops *gma_ops = gvt->gtt.gma_ops;
>   unsigned long gpa = INTEL_GVT_INVALID_ADDR;
>   unsigned long gma_index[4];
>   struct intel_gvt_gtt_entry e;
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
> index 3bf45672ef98..d0d598322404 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.h
> +++ b/drivers/gpu/drm/i915/gvt/gtt.h
> @@ -92,7 +92,7 @@ struct intel_gvt_gtt_gma_ops {
>   
>   struct intel_gvt_gtt {
>   struct intel_gvt_gtt_pte_ops *pte_ops;
> - struct intel_gvt_gtt_gma_ops *gma_ops;
> + const struct intel_gvt_gtt_gma_ops *gma_ops;
>   int (*mm_alloc_page_table)(struct intel_vgpu_mm *mm);
>   void (*mm_free_page_table)(struct intel_vgpu_mm *mm);
>   struct list_head oos_page_use_list_head;

It seems like the tittle doesn't match with the modification. I guess it 
should be Constify intel_gvt_gtt_gma_ops. I can fix that for you when 
applied.

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH 9/9] drm/i915/gvt: Constify vgpu_types

[Wang, Zhi A]

On 12/4/2021 12:55 PM, Rikard Falkeborn wrote:
> It is never modified, so make it const to allow the compiler to put it
> in read-only memory. While at it, make name a const char*.
>
> Signed-off-by: Rikard Falkeborn 
> ---
>   drivers/gpu/drm/i915/gvt/vgpu.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index fa6b92615799..80a940a1 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -77,7 +77,7 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
>   #define VGPU_WEIGHT(vgpu_num)   \
>   (VGPU_MAX_WEIGHT / (vgpu_num))
>   
> -static struct {
> +static const struct {
>   unsigned int low_mm;
>   unsigned int high_mm;
>   unsigned int fence;
> @@ -88,7 +88,7 @@ static struct {
>*/
>   unsigned int weight;
>   enum intel_vgpu_edid edid;
> - char *name;
> + const char *name;
>   } vgpu_types[] = {
>   /* Fixed vGPU type table */
>   { MB_TO_BYTES(64), MB_TO_BYTES(384), 4, VGPU_WEIGHT(8), 
> GVT_EDID_1024_768, "8" },

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH 8/9] drm/i915/gvt: Constify gtt_type_table_entry

[Wang, Zhi A]

On 12/4/2021 12:55 PM, Rikard Falkeborn wrote:
> It is never modified, so make it const to allow the compiler to put it
> in read-only memory.
>
> Signed-off-by: Rikard Falkeborn 
> ---
>   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 c8cd6bf28ea8..614156856f16 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -185,7 +185,7 @@ struct gtt_type_table_entry {
>   .pse_entry_type = pse_type, \
>   }
>   
> -static struct gtt_type_table_entry gtt_type_table[] = {
> +static const struct gtt_type_table_entry gtt_type_table[] = {
>   GTT_TYPE_TABLE_ENTRY(GTT_TYPE_PPGTT_ROOT_L4_ENTRY,
>   GTT_TYPE_PPGTT_ROOT_L4_ENTRY,
>   GTT_TYPE_INVALID,

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH 7/9] drm/i915/gvt: Constify formats

[Wang, Zhi A]

On 12/4/2021 12:55 PM, Rikard Falkeborn wrote:
> These are never modified, so make them const to allow the compiler to
> put them in read-only memory. WHile at it, make the description const
> char* since it is never modified.
>
> Signed-off-by: Rikard Falkeborn 
> ---
>   drivers/gpu/drm/i915/gvt/fb_decoder.c | 24 
>   1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c 
> b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> index 11a8baba6822..3c8736ae8fed 100644
> --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> @@ -40,12 +40,12 @@
>   
>   #define PRIMARY_FORMAT_NUM  16
>   struct pixel_format {
> - int drm_format; /* Pixel format in DRM definition */
> - int bpp;/* Bits per pixel, 0 indicates invalid */
> - char*desc;  /* The description */
> + int drm_format; /* Pixel format in DRM definition */
> + int bpp;/* Bits per pixel, 0 indicates invalid 
> */
> + const char  *desc;  /* The description */
>   };
Thanks so much for this. According to the code of i915, we prefer using 
one space as seperator.
>   
> -static struct pixel_format bdw_pixel_formats[] = {
> +static const struct pixel_format bdw_pixel_formats[] = {
>   {DRM_FORMAT_C8, 8, "8-bit Indexed"},
>   {DRM_FORMAT_RGB565, 16, "16-bit BGRX (5:6:5 MSB-R:G:B)"},
>   {DRM_FORMAT_XRGB, 32, "32-bit BGRX (8:8:8:8 MSB-X:R:G:B)"},
> @@ -58,7 +58,7 @@ static struct pixel_format bdw_pixel_formats[] = {
>   {0, 0, NULL},
>   };
>   
> -static struct pixel_format skl_pixel_formats[] = {
> +static const struct pixel_format skl_pixel_formats[] = {
>   {DRM_FORMAT_YUYV, 16, "16-bit packed YUYV (8:8:8:8 MSB-V:Y2:U:Y1)"},
>   {DRM_FORMAT_UYVY, 16, "16-bit packed UYVY (8:8:8:8 MSB-Y2:V:Y1:U)"},
>   {DRM_FORMAT_YVYU, 16, "16-bit packed YVYU (8:8:8:8 MSB-U:Y2:V:Y1)"},
> @@ -278,14 +278,14 @@ int intel_vgpu_decode_primary_plane(struct intel_vgpu 
> *vgpu,
>   
>   #define CURSOR_FORMAT_NUM   (1 << 6)
>   struct cursor_mode_format {
> - int drm_format; /* Pixel format in DRM definition */
> - u8  bpp;/* Bits per pixel; 0 indicates invalid */
> - u32 width;  /* In pixel */
> - u32 height; /* In lines */
> - char*desc;  /* The description */
> + int drm_format; /* Pixel format in DRM definition */
> + u8  bpp;/* Bits per pixel; 0 indicates invalid 
> */
> + u32 width;  /* In pixel */
> + u32 height; /* In lines */
> + const char  *desc;  /* The description */
>   };
The same comment as above.
> -static struct cursor_mode_format cursor_pixel_formats[] = {
> +static const struct cursor_mode_format cursor_pixel_formats[] = {
>   {DRM_FORMAT_ARGB, 32, 128, 128, "128x128 32bpp ARGB"},
>   {DRM_FORMAT_ARGB, 32, 256, 256, "256x256 32bpp ARGB"},
>   {DRM_FORMAT_ARGB, 32, 64, 64, "64x64 32bpp ARGB"},
> @@ -391,7 +391,7 @@ int intel_vgpu_decode_cursor_plane(struct intel_vgpu 
> *vgpu,
>   
>   #define SPRITE_FORMAT_NUM   (1 << 3)
>   
> -static struct pixel_format sprite_pixel_formats[SPRITE_FORMAT_NUM] = {
> +static const struct pixel_format sprite_pixel_formats[SPRITE_FORMAT_NUM] = {
>   [0x0] = {DRM_FORMAT_YUV422, 16, "YUV 16-bit 4:2:2 packed"},
>   [0x1] = {DRM_FORMAT_XRGB2101010, 32, "RGB 32-bit 2:10:10:10"},
>   [0x2] = {DRM_FORMAT_XRGB, 32, "RGB 32-bit 8:8:8:8"},

The rest of the patch looks good to me.

Re: [Intel-gfx] [PATCH 6/9] drm/i915/gvt: Constify cmd_interrupt_events

[Wang, Zhi A]

On 12/4/2021 12:55 PM, Rikard Falkeborn wrote:
> It is never modified, so make it const to allow the compiler to put it
> in read-only memory.
>
> Signed-off-by: Rikard Falkeborn 
> ---
>   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 c4118b808268..ce9307546e7f 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -1144,7 +1144,7 @@ struct cmd_interrupt_event {
>   int mi_user_interrupt;
>   };
>   
> -static struct cmd_interrupt_event cmd_interrupt_events[] = {
> +static const struct cmd_interrupt_event cmd_interrupt_events[] = {
>   [RCS0] = {
>   .pipe_control_notify = RCS_PIPE_CONTROL,
>   .mi_flush_dw = INTEL_GVT_EVENT_RESERVED,

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH 5/9] drm/i915/gvt: Constify gvt_mmio_block

[Wang, Zhi A]

On 12/4/2021 12:55 PM, Rikard Falkeborn wrote:
> These are never modified, so make them const to allow the compiler to
> put it in read-only memory.
>
> Signed-off-by: Rikard Falkeborn 
> ---
>   drivers/gpu/drm/i915/gvt/gvt.h  |  2 +-
>   drivers/gpu/drm/i915/gvt/handlers.c | 12 ++--
>   2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 0c0615602343..0ebffc327528 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -272,7 +272,7 @@ struct intel_gvt_mmio {
>   /* Value of command write of this reg needs to be patched */
>   #define F_CMD_WRITE_PATCH   (1 << 8)
>   
> - struct gvt_mmio_block *mmio_block;
> + const struct gvt_mmio_block *mmio_block;
>   unsigned int num_mmio_block;
>   
>   DECLARE_HASHTABLE(mmio_info_table, INTEL_GVT_MMIO_HASH_BITS);
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index cde0a477fb49..5e85a77da257 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -3627,11 +3627,11 @@ static int init_bxt_mmio_info(struct intel_gvt *gvt)
>   return 0;
>   }
>   
> -static struct gvt_mmio_block *find_mmio_block(struct intel_gvt *gvt,
> -   unsigned int offset)
> +static const struct gvt_mmio_block *find_mmio_block(struct intel_gvt *gvt,
> + unsigned int offset)
>   {
>   unsigned long device = intel_gvt_get_device_type(gvt);
> - struct gvt_mmio_block *block = gvt->mmio.mmio_block;
> + const struct gvt_mmio_block *block = gvt->mmio.mmio_block;
>   int num = gvt->mmio.num_mmio_block;
>   int i;
>   
> @@ -3670,7 +3670,7 @@ void intel_gvt_clean_mmio_info(struct intel_gvt *gvt)
>* accessible (should have no F_CMD_ACCESS flag).
>* otherwise, need to update cmd_reg_handler in cmd_parser.c
>*/
> -static struct gvt_mmio_block mmio_blocks[] = {
> +static const struct gvt_mmio_block mmio_blocks[] = {
>   {D_SKL_PLUS, _MMIO(DMC_MMIO_START_RANGE), 0x3000, NULL, NULL},
>   {D_ALL, _MMIO(MCHBAR_MIRROR_BASE_SNB), 0x4, NULL, NULL},
>   {D_ALL, _MMIO(VGT_PVINFO_PAGE), VGT_PVINFO_SIZE,
> @@ -3753,7 +3753,7 @@ int intel_gvt_for_each_tracked_mmio(struct intel_gvt 
> *gvt,
>   int (*handler)(struct intel_gvt *gvt, u32 offset, void *data),
>   void *data)
>   {
> - struct gvt_mmio_block *block = gvt->mmio.mmio_block;
> + const struct gvt_mmio_block *block = gvt->mmio.mmio_block;
>   struct intel_gvt_mmio_info *e;
>   int i, j, ret;
>   
> @@ -3871,7 +3871,7 @@ int intel_vgpu_mmio_reg_rw(struct intel_vgpu *vgpu, 
> unsigned int offset,
>   struct drm_i915_private *i915 = vgpu->gvt->gt->i915;
>   struct intel_gvt *gvt = vgpu->gvt;
>   struct intel_gvt_mmio_info *mmio_info;
> - struct gvt_mmio_block *mmio_block;
> + const struct gvt_mmio_block *mmio_block;
>   gvt_mmio_func func;
>   int ret;
>   

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH 4/9] drm/i915/gvt: Constify intel_gvt_sched_policy_ops

[Wang, Zhi A]

On 12/4/2021 12:55 PM, Rikard Falkeborn wrote:
> These are never modified, so make them const to allow the compiler to
> put them in read-only memory.
>
> Signed-off-by: Rikard Falkeborn 
> ---
>   drivers/gpu/drm/i915/gvt/sched_policy.c | 2 +-
>   drivers/gpu/drm/i915/gvt/scheduler.h| 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c 
> b/drivers/gpu/drm/i915/gvt/sched_policy.c
> index 036b74fe9298..c077fb4674f0 100644
> --- a/drivers/gpu/drm/i915/gvt/sched_policy.c
> +++ b/drivers/gpu/drm/i915/gvt/sched_policy.c
> @@ -368,7 +368,7 @@ static void tbs_sched_stop_schedule(struct intel_vgpu 
> *vgpu)
>   vgpu_data->active = false;
>   }
>   
> -static struct intel_gvt_sched_policy_ops tbs_schedule_ops = {
> +static const struct intel_gvt_sched_policy_ops tbs_schedule_ops = {
>   .init = tbs_sched_init,
>   .clean = tbs_sched_clean,
>   .init_vgpu = tbs_sched_init_vgpu,
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.h 
> b/drivers/gpu/drm/i915/gvt/scheduler.h
> index 7c86984a842f..1f391b3da2cc 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.h
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.h
> @@ -56,7 +56,7 @@ struct intel_gvt_workload_scheduler {
>   wait_queue_head_t waitq[I915_NUM_ENGINES];
>   
>   void *sched_data;
> - struct intel_gvt_sched_policy_ops *sched_ops;
> + const struct intel_gvt_sched_policy_ops *sched_ops;
>   };
>   
>   #define INDIRECT_CTX_ADDR_MASK 0xffc0

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH 3/9] drm/i915/gvt: Constify intel_gvt_irq_ops

[Wang, Zhi A]

On 12/4/2021 12:55 PM, Rikard Falkeborn wrote:
> These are never modified, so make them const to allow the compiler to
> put them in read-only memory.
>
> Signed-off-by: Rikard Falkeborn 
> ---
>   drivers/gpu/drm/i915/gvt/interrupt.c | 10 +-
>   drivers/gpu/drm/i915/gvt/interrupt.h |  2 +-
>   2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c 
> b/drivers/gpu/drm/i915/gvt/interrupt.c
> index 614b951d919f..9ccc6b1ecc28 100644
> --- a/drivers/gpu/drm/i915/gvt/interrupt.c
> +++ b/drivers/gpu/drm/i915/gvt/interrupt.c
> @@ -176,7 +176,7 @@ int intel_vgpu_reg_imr_handler(struct intel_vgpu *vgpu,
>   unsigned int reg, void *p_data, unsigned int bytes)
>   {
>   struct intel_gvt *gvt = vgpu->gvt;
> - struct intel_gvt_irq_ops *ops = gvt->irq.ops;
> + const struct intel_gvt_irq_ops *ops = gvt->irq.ops;
>   u32 imr = *(u32 *)p_data;
>   
>   trace_write_ir(vgpu->id, "IMR", reg, imr, vgpu_vreg(vgpu, reg),
> @@ -206,7 +206,7 @@ int intel_vgpu_reg_master_irq_handler(struct intel_vgpu 
> *vgpu,
>   unsigned int reg, void *p_data, unsigned int bytes)
>   {
>   struct intel_gvt *gvt = vgpu->gvt;
> - struct intel_gvt_irq_ops *ops = gvt->irq.ops;
> + const struct intel_gvt_irq_ops *ops = gvt->irq.ops;
>   u32 ier = *(u32 *)p_data;
>   u32 virtual_ier = vgpu_vreg(vgpu, reg);
>   
> @@ -246,7 +246,7 @@ int intel_vgpu_reg_ier_handler(struct intel_vgpu *vgpu,
>   {
>   struct intel_gvt *gvt = vgpu->gvt;
>   struct drm_i915_private *i915 = gvt->gt->i915;
> - struct intel_gvt_irq_ops *ops = gvt->irq.ops;
> + const struct intel_gvt_irq_ops *ops = gvt->irq.ops;
>   struct intel_gvt_irq_info *info;
>   u32 ier = *(u32 *)p_data;
>   
> @@ -604,7 +604,7 @@ static void gen8_init_irq(
>   SET_BIT_INFO(irq, 25, PCU_PCODE2DRIVER_MAILBOX, INTEL_GVT_IRQ_INFO_PCU);
>   }
>   
> -static struct intel_gvt_irq_ops gen8_irq_ops = {
> +static const struct intel_gvt_irq_ops gen8_irq_ops = {
>   .init_irq = gen8_init_irq,
>   .check_pending_irq = gen8_check_pending_irq,
>   };
> @@ -626,7 +626,7 @@ void intel_vgpu_trigger_virtual_event(struct intel_vgpu 
> *vgpu,
>   struct intel_gvt *gvt = vgpu->gvt;
>   struct intel_gvt_irq *irq = >irq;
>   gvt_event_virt_handler_t handler;
> - struct intel_gvt_irq_ops *ops = gvt->irq.ops;
> + const struct intel_gvt_irq_ops *ops = gvt->irq.ops;
>   
>   handler = get_event_virt_handler(irq, event);
>   drm_WARN_ON(>drm, !handler);
> diff --git a/drivers/gpu/drm/i915/gvt/interrupt.h 
> b/drivers/gpu/drm/i915/gvt/interrupt.h
> index 6c47d3e33161..0989e180ed54 100644
> --- a/drivers/gpu/drm/i915/gvt/interrupt.h
> +++ b/drivers/gpu/drm/i915/gvt/interrupt.h
> @@ -203,7 +203,7 @@ struct intel_gvt_irq_map {
>   
>   /* structure containing device specific IRQ state */
>   struct intel_gvt_irq {
> - struct intel_gvt_irq_ops *ops;
> + const struct intel_gvt_irq_ops *ops;
>   struct intel_gvt_irq_info *info[INTEL_GVT_IRQ_INFO_MAX];
>   DECLARE_BITMAP(irq_info_bitmap, INTEL_GVT_IRQ_INFO_MAX);
>   struct intel_gvt_event_info events[INTEL_GVT_EVENT_MAX];

Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH 2/9] drm/i915/gvt: Constify intel_gvt_gtt_pte_ops

[Wang, Zhi A]

On 12/4/2021 12:55 PM, Rikard Falkeborn wrote:
> These are never modified, so make them const to allow the compiler to
> put them in read-only memory.
>
> Signed-off-by: Rikard Falkeborn 
> ---
>   drivers/gpu/drm/i915/gvt/gtt.c | 62 +-
>   drivers/gpu/drm/i915/gvt/gtt.h |  2 +-
>   2 files changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 6efa48727052..c8cd6bf28ea8 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -499,7 +499,7 @@ DEFINE_PPGTT_GMA_TO_INDEX(gen8, l3_pdp, (gma >> 30 & 
> 0x3));
>   DEFINE_PPGTT_GMA_TO_INDEX(gen8, l4_pdp, (gma >> 30 & 0x1ff));
>   DEFINE_PPGTT_GMA_TO_INDEX(gen8, pml4, (gma >> 39 & 0x1ff));
>   
> -static struct intel_gvt_gtt_pte_ops gen8_gtt_pte_ops = {
> +static const struct intel_gvt_gtt_pte_ops gen8_gtt_pte_ops = {
>   .get_entry = gtt_get_entry64,
>   .set_entry = gtt_set_entry64,
>   .clear_present = gtt_entry_clear_present,
> @@ -526,7 +526,7 @@ static const struct intel_gvt_gtt_gma_ops 
> gen8_gtt_gma_ops = {
>   };
>   
>   /* Update entry type per pse and ips bit. */
> -static void update_entry_type_for_real(struct intel_gvt_gtt_pte_ops *pte_ops,
> +static void update_entry_type_for_real(const struct intel_gvt_gtt_pte_ops 
> *pte_ops,
>   struct intel_gvt_gtt_entry *entry, bool ips)
>   {
>   switch (entry->type) {
> @@ -553,7 +553,7 @@ static void _ppgtt_get_root_entry(struct intel_vgpu_mm 
> *mm,
>   struct intel_gvt_gtt_entry *entry, unsigned long index,
>   bool guest)
>   {
> - struct intel_gvt_gtt_pte_ops *pte_ops = mm->vgpu->gvt->gtt.pte_ops;
> + const struct intel_gvt_gtt_pte_ops *pte_ops = 
> mm->vgpu->gvt->gtt.pte_ops;
>   
>   GEM_BUG_ON(mm->type != INTEL_GVT_MM_PPGTT);
>   
> @@ -580,7 +580,7 @@ static void _ppgtt_set_root_entry(struct intel_vgpu_mm 
> *mm,
>   struct intel_gvt_gtt_entry *entry, unsigned long index,
>   bool guest)
>   {
> - struct intel_gvt_gtt_pte_ops *pte_ops = mm->vgpu->gvt->gtt.pte_ops;
> + const struct intel_gvt_gtt_pte_ops *pte_ops = 
> mm->vgpu->gvt->gtt.pte_ops;
>   
>   pte_ops->set_entry(guest ? mm->ppgtt_mm.guest_pdps :
>  mm->ppgtt_mm.shadow_pdps,
> @@ -596,7 +596,7 @@ static inline void ppgtt_set_shadow_root_entry(struct 
> intel_vgpu_mm *mm,
>   static void ggtt_get_guest_entry(struct intel_vgpu_mm *mm,
>   struct intel_gvt_gtt_entry *entry, unsigned long index)
>   {
> - struct intel_gvt_gtt_pte_ops *pte_ops = mm->vgpu->gvt->gtt.pte_ops;
> + const struct intel_gvt_gtt_pte_ops *pte_ops = 
> mm->vgpu->gvt->gtt.pte_ops;
>   
>   GEM_BUG_ON(mm->type != INTEL_GVT_MM_GGTT);
>   
> @@ -608,7 +608,7 @@ static void ggtt_get_guest_entry(struct intel_vgpu_mm *mm,
>   static void ggtt_set_guest_entry(struct intel_vgpu_mm *mm,
>   struct intel_gvt_gtt_entry *entry, unsigned long index)
>   {
> - struct intel_gvt_gtt_pte_ops *pte_ops = mm->vgpu->gvt->gtt.pte_ops;
> + const struct intel_gvt_gtt_pte_ops *pte_ops = 
> mm->vgpu->gvt->gtt.pte_ops;
>   
>   GEM_BUG_ON(mm->type != INTEL_GVT_MM_GGTT);
>   
> @@ -619,7 +619,7 @@ static void ggtt_set_guest_entry(struct intel_vgpu_mm *mm,
>   static void ggtt_get_host_entry(struct intel_vgpu_mm *mm,
>   struct intel_gvt_gtt_entry *entry, unsigned long index)
>   {
> - struct intel_gvt_gtt_pte_ops *pte_ops = mm->vgpu->gvt->gtt.pte_ops;
> + const struct intel_gvt_gtt_pte_ops *pte_ops = 
> mm->vgpu->gvt->gtt.pte_ops;
>   
>   GEM_BUG_ON(mm->type != INTEL_GVT_MM_GGTT);
>   
> @@ -629,7 +629,7 @@ static void ggtt_get_host_entry(struct intel_vgpu_mm *mm,
>   static void ggtt_set_host_entry(struct intel_vgpu_mm *mm,
>   struct intel_gvt_gtt_entry *entry, unsigned long index)
>   {
> - struct intel_gvt_gtt_pte_ops *pte_ops = mm->vgpu->gvt->gtt.pte_ops;
> + const struct intel_gvt_gtt_pte_ops *pte_ops = 
> mm->vgpu->gvt->gtt.pte_ops;
>   unsigned long offset = index;
>   
>   GEM_BUG_ON(mm->type != INTEL_GVT_MM_GGTT);
> @@ -655,7 +655,7 @@ static inline int ppgtt_spt_get_entry(
>   bool guest)
>   {
>   struct intel_gvt *gvt = spt->vgpu->gvt;
> - struct intel_gvt_gtt_pte_ops *ops = gvt->gtt.pte_ops;
> + const struct intel_gvt_gtt_pte_ops *ops = gvt->gtt.pte_ops;
>   int ret;
>   
>   e->type = get_entry_type(type);
> @@ -684,7 +684,7 @@ static inline int ppgtt_spt_set_entry(
>   bool guest)
>   {
>   struct intel_gvt *gvt = spt->vgpu->gvt;
> - struct intel_gvt_gtt_pte_ops *ops = gvt->gtt.pte_ops;
> + const struct intel_gvt_gtt_pte_ops *ops = gvt->gtt.pte_ops;
>   
>   if (WARN(!gtt_type_is_entry(e->type), "invalid entry type\n"))
>   return -EINVAL;
> @@ -947,7 +947,7 @@ static int ppgtt_invalidate_spt_by_shadow_entry(struct 
> intel_vgpu *vgpu,
>   struct 

Re: [Intel-gfx] [PATCH 1/3] i915/gvt: seperate tracked MMIO table from handlers.c

[Wang, Zhi A]

On 11/9/2021 12:58 PM, h...@lst.de wrote:
> On Tue, Nov 09, 2021 at 10:51:27AM +0000, Wang, Zhi A wrote:
>> Can you elaborate more about this? We need the hash query from the table
>> ASAP when the hypervisor trapped a mmio access. It's a critical path and
>> we tried different data structure in the kernel and the hash table gives
>> the best performance.
> Ok, I misunderstood the hashtable.h interface.  hash_for_each_possible
> actually does a hash lookup instead of an interation despite the rather
> misleading name.

Yes. Maybe with a keyword "lookup" in the name of the interface would be 
better since it's widely used in the kernel. :)

Re: [Intel-gfx] refactor the i915 GVT support and move to the modern mdev API v2

[Wang, Zhi A]

Hi Joonas and Christoph:

We were testing the patch series since Monday and planning to reply after we 
get the test result. Mostly, we are concerned about patch 6 and how it would 
affect the test result. Patch 6 changes the timing of loading GVT-g. According 
to the discussion in the last email, this will break our design of golden MMIO 
snapshot. Also moving GVT-g code into kvmgt.ko requires the discussion of 
defining and shrinking the interfaces between i915 and kvmgt.  I guess the 
ideal way to take Christoph's patch is:

1) We have to figure out how to deal with golden MMIO snapshot. It's a little 
bit hard to take the re-factor patch before settling this down. In the previous 
discussion, we would like to find a way to do the snapshot in intel_gvt.c
2) Shrink and refine the exported interfaces because of moving the code into 
kvmgt.ko
3) Get patches reviewed and merged.

For 1) I was thinking to separated the MMIO handler table from handlers.c and 
let it build different data structures depends on where it got referenced.
2) That's might require some more discussion.

Is it possible to separate the refactor part from the using new mdev API stuff? 
So that the design opens in the re-factor patches wouldn’t block the process of 
mdev API improvement?

Thanks,
Zhi.

-Original Message-
From: Joonas Lahtinen  
Sent: Thursday, November 4, 2021 2:59 PM
To: Christoph Hellwig ; Jani Nikula ; 
Vivi, Rodrigo ; Zhenyu Wang ; 
Wang, Zhi A 
Cc: Jason Gunthorpe ; intel-gfx@lists.freedesktop.org; 
intel-gvt-...@lists.freedesktop.org; dri-de...@lists.freedesktop.org; 
linux-ker...@vger.kernel.org
Subject: Re: refactor the i915 GVT support and move to the modern mdev API v2

Hi Zhenyu and Zhi,

Can you have somebody from the GVT team to review the patches that are fully 
contained in gvt/ ?

I also started discussion on patch 6 which is about defining the interface 
between the modules. I remember there is prior work to shrink the interface. Do 
you have links to such patches?

The minimal we should do is to eliminate the double underscore prefixed 
functions. But I would prefer to have the symbol exports by default so that we 
can enable the functionality just by loading the module.

Regards, Joonas

Quoting Christoph Hellwig (2021-11-02 09:05:32)
> Hi all,
> 
> the GVT code in the i915 is a bit of a mess right now due to strange 
> abstractions and lots of indirect calls.  This series refactors 
> various bits to clean that up.  The main user visible change is that 
> almost all of the GVT code moves out of the main i915 driver and into 
> the kvmgt module.
> 
> Tested on my Thinkpad with a Kaby Lake CPU and integrated graphics.
> 
> Git tree:
> 
> git://git.infradead.org/users/hch/misc.git i915-gvt
> 
> Gitweb:
> 
> 
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/i915-g
> vt
> 
> Changes since v1:
>  - rebased on Linux 5.15
>  - allow the kvmgvt module to be loaded at any time and thus solve
>the deadlock when both i915 amd kvmgvt are modular
>  - include the conversion to the modern mdev API
> 
> Note that I do expect to rebased this again against 5.16-rc1 once 
> released, but I'd like to get this out for review ASAP.
> 
> Diffstat:
>  b/drivers/gpu/drm/i915/Kconfig  |   33 
>  b/drivers/gpu/drm/i915/Makefile |   31 
>  b/drivers/gpu/drm/i915/gvt/cfg_space.c  |   89 --
>  b/drivers/gpu/drm/i915/gvt/cmd_parser.c |4 
>  b/drivers/gpu/drm/i915/gvt/dmabuf.c |   36 -
>  b/drivers/gpu/drm/i915/gvt/execlist.c   |   12 
>  b/drivers/gpu/drm/i915/gvt/gtt.c|   55 +
>  b/drivers/gpu/drm/i915/gvt/gvt.h|  125 ++-
>  b/drivers/gpu/drm/i915/gvt/interrupt.c  |   38 +
>  b/drivers/gpu/drm/i915/gvt/kvmgt.c  | 1099 
> +++-
>  b/drivers/gpu/drm/i915/gvt/mmio.c   |4 
>  b/drivers/gpu/drm/i915/gvt/opregion.c   |  148 
>  b/drivers/gpu/drm/i915/gvt/page_track.c |8 
>  b/drivers/gpu/drm/i915/gvt/scheduler.c  |   37 -
>  b/drivers/gpu/drm/i915/gvt/trace.h  |2 
>  b/drivers/gpu/drm/i915/gvt/vgpu.c   |   22 
>  b/drivers/gpu/drm/i915/i915_drv.c   |7 
>  b/drivers/gpu/drm/i915/i915_drv.h   |1 
>  b/drivers/gpu/drm/i915/i915_trace.h |1 
>  b/drivers/gpu/drm/i915/intel_gvt.c  |  162 +++-
>  b/drivers/gpu/drm/i915/intel_gvt.h  |   17 
>  drivers/gpu/drm/i915/gvt/Makefile   |9 
>  drivers/gpu/drm/i915/gvt/gvt.c  |  340 -
>  drivers/gpu/drm/i915/gvt/hypercall.h|   82 --
>  drivers/gpu/drm/i915/gvt/mpt.h  |  400 ---
>  25 files changed, 929 insertions(+), 1833 deletions(-)

Re: [Intel-gfx] [PATCH] drm/i915/gvt: clean up kernel-doc in gtt.c

[Wang, Zhi A]

On 10/3/21 5:23 AM, Randy Dunlap wrote:
> Fix kernel-doc warnings in gtt.c:
> 
> gtt.c:1152: warning: This comment starts with '/**', but isn't a kernel-doc 
> comment. Refer Documentation/doc-guide/kernel-doc.rst
>   * Check if can do 2M page
> gtt.c:1152: warning: missing initial short description on line:
>   * Check if can do 2M page
> gtt.c:2209: warning: expecting prototype for 
> intel_vgpu_emulate_gtt_mmio_read(). Prototype was for 
> intel_vgpu_emulate_ggtt_mmio_read() instead
> 
> Fixes: a752b070a678 ("drm/i915/gvt: Fix function comment doc errors")
> Fixes: 2707e4446688 ("drm/i915/gvt: vGPU graphics memory virtualization")
> Signed-off-by: Randy Dunlap 
> Reported-by: kernel test robot 
> Cc: Zhenyu Wang 
> Cc: Zhi Wang 
> Cc: Colin Xu 
> Cc: Jani Nikula 
> Cc: Joonas Lahtinen 
> Cc: Rodrigo Vivi 
> Cc: intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-...@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: David Airlie 
> Cc: Daniel Vetter 
> ---
>   drivers/gpu/drm/i915/gvt/gtt.c |4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- linux-next-20211001.orig/drivers/gpu/drm/i915/gvt/gtt.c
> +++ linux-next-20211001/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1148,7 +1148,7 @@ static inline void ppgtt_generate_shadow
>   ops->set_pfn(se, s->shadow_page.mfn);
>   }
>   
> -/**
> +/*
>* Check if can do 2M page
>* @vgpu: target vgpu
>* @entry: target pfn's gtt entry
> @@ -2193,7 +2193,7 @@ static int emulate_ggtt_mmio_read(struct
>   }
>   
>   /**
> - * intel_vgpu_emulate_gtt_mmio_read - emulate GTT MMIO register read
> + * intel_vgpu_emulate_ggtt_mmio_read - emulate GTT MMIO register read
>* @vgpu: a vGPU
>* @off: register offset
>* @p_data: data will be returned to guest
> 
Thanks for the patch. queued.
Reviewed-by: Zhi Wang 

Re: [Intel-gfx] [PATCH 2/5] drm/i915: check dri root before debugfs init

[Wang, Zhi A]

On 10/8/21 9:17 AM, Nirmoy Das wrote:
> Return early if dri minor root dentry is NULL.
> 
> CC: Zhenyu Wang 
> CC: Zhi Wang 
> CC: Jani Nikula 
> CC: Joonas Lahtinen 
> CC: Rodrigo Vivi 
> CC: David Airlie 
> CC: Daniel Vetter 
> Signed-off-by: Nirmoy Das 
> ---
>   drivers/gpu/drm/i915/gvt/debugfs.c  | 3 +++
>   drivers/gpu/drm/i915/i915_debugfs.c | 3 +++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/debugfs.c 
> b/drivers/gpu/drm/i915/gvt/debugfs.c
> index 9f1c209d9251..2d47acaa03ee 100644
> --- a/drivers/gpu/drm/i915/gvt/debugfs.c
> +++ b/drivers/gpu/drm/i915/gvt/debugfs.c
> @@ -187,6 +187,9 @@ void intel_gvt_debugfs_init(struct intel_gvt *gvt)
>   {
>   struct drm_minor *minor = gvt->gt->i915->drm.primary;
> 
> + if (!minor->debugfs_root)
> + return;
> +
>   gvt->debugfs_root = debugfs_create_dir("gvt", minor->debugfs_root);
> 
>   debugfs_create_ulong("num_tracked_mmio", 0444, gvt->debugfs_root,
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 44969f5dde50..d572b686edeb 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1012,6 +1012,9 @@ void i915_debugfs_register(struct drm_i915_private 
> *dev_priv)
>   struct drm_minor *minor = dev_priv->drm.primary;
>   int i;
> 
> + if (!minor->debugfs_root)
> + return;
> +
>   i915_debugfs_params(dev_priv);
> 
>   debugfs_create_file("i915_forcewake_user", S_IRUSR, minor->debugfs_root,
> --
> 2.32.0
> 
Thanks for the patch. queued.
Reviewed-by: Zhi Wang 

Re: [Intel-gfx] refactor the i915 GVT support

[Wang, Zhi A]

Hi folks:

It seems we haven't reached a possible solution of this refactor patch 
series. The current patch series needs to be re-worked because of the 
module/symbol dependency(The root cause has been discussed in another 
email). I have to get them off from our gvt-next repo so that we can 
continue our development and pull-request to upstream. Thanks so much 
for the patch and the discussion.

Thanks,
Zhi.

On 10/1/21 1:01 PM, Wang, Zhi A wrote:
> On 9/29/21 6:55 PM, Jason Gunthorpe wrote:
>> On Wed, Sep 29, 2021 at 06:27:16PM +0000, Wang, Zhi A wrote:
>>> On 9/28/21 3:05 PM, Jason Gunthorpe wrote:
>>>> On Tue, Sep 28, 2021 at 02:35:06PM +, Wang, Zhi A wrote:
>>>>
>>>>> Yes. I was thinking of the possibility of putting off some work 
>>>>> later so
>>>>> that we don't need to make a lot of changes. GVT-g needs to take a
>>>>> snapshot of GPU registers as the initial virtual states for other 
>>>>> vGPUs,
>>>>> which requires the initialization happens at a certain early time of
>>>>> initialization of i915. I was thinking maybe we can take other 
>>>>> patches
>>>>> from Christoph like "de-virtualize*" except this one because 
>>>>> currently
>>>>> we have to maintain a TEST-ONLY patch on our tree to prevent i915 
>>>>> built
>>>>> as kernel module.
>>>> How about just capture these registers in the main module/device and
>>>> not try so hard to isolate it to the gvt stuff?
>>> Hi Jason:
>>>
>>> Thanks for the idea. I am not sure i915 guys would take this idea since
>>> that it's only for GVT-g, i915 doesn't use this at all. We need to take
>>> a snapshot of both PCI configuration space and MMIO registers before
>>> i915 driver starts to touch the HW.
>> Given the code is already linked into i915 I don't see there is much
>> to object to here. It can remain conditional on the kernel parameter
>> as today.
>>
>> As a general philosophy this would all be much less strange if the
>> mdev .ko is truely optional. It should be cleanly seperate from its
>> base device and never request_module'd..
>>
>> In this case auxiliary device might be a good option, have i915 create
>> one and the mdev module be loaded against it.
>>
>> In the mean time is there some shortcut to get this series to move
>> ahead? Is patch 4 essential to the rest of the series?
>>
>> A really awful hack would be to push the pci_driver_register into a
>> WQ so that the request_module is guarenteed to not be part of the
>> module_init callchain.
>
> Hi Jason and folks:
>
> Thanks so much for the ideas. That sounds great and I was keeping 
> thinking how to make progress on this. How about we do like this: We 
> don't do request_module("kvmgt") in i915.ko, which resolves the 
> circular module dependency. We keep the code of doing snapshot of 
> registers in intel_gvt.c. When i915.enable_gvt=1, we do the snapshot. 
> Then we export functions for kvmgt.ko in intel_gvt.c to check if gvt 
> in i915 is enabled or not and get the snapshots.
>
> How does that sounds? I just need to write another patch and put it on 
> top of Christoph's series.
>
> Thanks,
>
> Zhi.
>
>>> Also I was thinking if moving gvt into kvmgt.ko is the right direction.
>>> It seems the module loading system in kernel is not designed for 
>>> "module
>>> A loading module B, which needs symbols from module A, in the
>>> initialization path of module A".
>> Of course not, that is a circular module dependency, it should not be
>> that way. The SW layers need to be clean and orderly - meaning the
>> i915 module needs to have the minimal amount of code to support the
>> mdev module.
>>
>> Jason
>
>

Re: [Intel-gfx] refactor the i915 GVT support

[Wang, Zhi A]

On 9/29/21 6:55 PM, Jason Gunthorpe wrote:
> On Wed, Sep 29, 2021 at 06:27:16PM +0000, Wang, Zhi A wrote:
>> On 9/28/21 3:05 PM, Jason Gunthorpe wrote:
>>> On Tue, Sep 28, 2021 at 02:35:06PM +, Wang, Zhi A wrote:
>>>
>>>> Yes. I was thinking of the possibility of putting off some work later so
>>>> that we don't need to make a lot of changes. GVT-g needs to take a
>>>> snapshot of GPU registers as the initial virtual states for other vGPUs,
>>>> which requires the initialization happens at a certain early time of
>>>> initialization of i915. I was thinking maybe we can take other patches
>>>> from Christoph like "de-virtualize*" except this one because currently
>>>> we have to maintain a TEST-ONLY patch on our tree to prevent i915 built
>>>> as kernel module.
>>> How about just capture these registers in the main module/device and
>>> not try so hard to isolate it to the gvt stuff?
>> Hi Jason:
>>
>> Thanks for the idea. I am not sure i915 guys would take this idea since
>> that it's only for GVT-g, i915 doesn't use this at all. We need to take
>> a snapshot of both PCI configuration space and MMIO registers before
>> i915 driver starts to touch the HW.
> Given the code is already linked into i915 I don't see there is much
> to object to here. It can remain conditional on the kernel parameter
> as today.
>
> As a general philosophy this would all be much less strange if the
> mdev .ko is truely optional. It should be cleanly seperate from its
> base device and never request_module'd..
>
> In this case auxiliary device might be a good option, have i915 create
> one and the mdev module be loaded against it.
>
> In the mean time is there some shortcut to get this series to move
> ahead? Is patch 4 essential to the rest of the series?
>
> A really awful hack would be to push the pci_driver_register into a
> WQ so that the request_module is guarenteed to not be part of the
> module_init callchain.

Hi Jason and folks:

Thanks so much for the ideas. That sounds great and I was keeping 
thinking how to make progress on this. How about we do like this: We 
don't do request_module("kvmgt") in i915.ko, which resolves the circular 
module dependency. We keep the code of doing snapshot of registers in 
intel_gvt.c. When i915.enable_gvt=1, we do the snapshot. Then we export 
functions for kvmgt.ko in intel_gvt.c to check if gvt in i915 is enabled 
or not and get the snapshots.

How does that sounds? I just need to write another patch and put it on 
top of Christoph's series.

Thanks,

Zhi.

>> Also I was thinking if moving gvt into kvmgt.ko is the right direction.
>> It seems the module loading system in kernel is not designed for "module
>> A loading module B, which needs symbols from module A, in the
>> initialization path of module A".
> Of course not, that is a circular module dependency, it should not be
> that way. The SW layers need to be clean and orderly - meaning the
> i915 module needs to have the minimal amount of code to support the
> mdev module.
>
> Jason

Re: [Intel-gfx] refactor the i915 GVT support

[Wang, Zhi A]

On 9/28/21 3:05 PM, Jason Gunthorpe wrote:
> On Tue, Sep 28, 2021 at 02:35:06PM +0000, Wang, Zhi A wrote:
>
>> Yes. I was thinking of the possibility of putting off some work later so
>> that we don't need to make a lot of changes. GVT-g needs to take a
>> snapshot of GPU registers as the initial virtual states for other vGPUs,
>> which requires the initialization happens at a certain early time of
>> initialization of i915. I was thinking maybe we can take other patches
>> from Christoph like "de-virtualize*" except this one because currently
>> we have to maintain a TEST-ONLY patch on our tree to prevent i915 built
>> as kernel module.
> How about just capture these registers in the main module/device and
> not try so hard to isolate it to the gvt stuff?

Hi Jason:

Thanks for the idea. I am not sure i915 guys would take this idea since 
that it's only for GVT-g, i915 doesn't use this at all. We need to take 
a snapshot of both PCI configuration space and MMIO registers before 
i915 driver starts to touch the HW.

One idea is we capture the registers in intel_gvt.c during the early 
initialization and do everything else when i915.ko is fully loaded. But 
how about dependence between i915.ko and kvmgt.ko? We cannot do 
request_module("kvmgt") in i915.ko.

Maybe Christoph can give more input on this and how we can deal with 
this? Before we figure out an solution, we have to pick that patch out 
since it blocks our pull request schedule.

Also I was thinking if moving gvt into kvmgt.ko is the right direction. 
It seems the module loading system in kernel is not designed for "module 
A loading module B, which needs symbols from module A, in the 
initialization path of module A".

Thanks,

Zhi.

> Jason

Re: [Intel-gfx] refactor the i915 GVT support

[Wang, Zhi A]

On 9/28/21 2:00 PM, Luis Chamberlain wrote:
> On Tue, Sep 28, 2021 at 07:41:00AM +0000, Wang, Zhi A wrote:
>> Hey guys:
>>
>> After some investigation, I found the root cause this problem ("i915"
>> module loading will be stuck with Christoph's refactor patches), which
>> can be reproduced by building both i915 and kvmgt as kernel module and
>> the loading i915.
> Thanks for looking into this!
>
>> The root cause is: in Linux kernel loading, before a kernel module
>> loading is finished, its symbols can not be reached by other module when
>> resolving the symbols (even they can be found in /proc/kallsyms).
>> Because the status of the kernel module is MODULE_STATE_COMING and
>> resolve_symbol() from another kernel module will check this and return a
>> -EBUSY.
> Well, it would seem that way but...
>
>> In this case, before i915 loading is finished, the requested module
>> "kvmgt" cannot reach the symbols in module i915. Thus it kept waiting
>> and left message like below in the dmesg:
>>
>> [  644.152021] kvmgt: gave up waiting for init of module i915.
>> [  644.152039] kvmgt: Unknown symbol i915_gem_object_set_to_cpu_domain
>> (err -16)
>> [  674.871409] kvmgt: gave up waiting for init of module i915.
>> [  674.871427] kvmgt: Unknown symbol intel_ring_begin (err -16)
>> [  705.590586] kvmgt: gave up waiting for init of module i915.
>> [  705.590604] kvmgt: Unknown symbol i915_vma_move_to_active (err -16)
>> [  736.310230] kvmgt: gave up waiting for init of module i915.
>> [  736.310248] kvmgt: Unknown symbol shmem_unpin_map (err -16)
>> ...
>>
>> The error message is from execution path below:
>>
>> kernel/module.c:
>>
>> [i915 module loading] ->
>> request_module("kvmgt")->[modprobe]->init_module("kvmgt")->load_module()->simplify_symbols()->resolve_symbol_wait():
>>
>> static const struct kernel_symbol *
>> resolve_symbol_wait(struct module *mod,
>>               const struct load_info *info,
>>               const char *name)
>> {
>>       const struct kernel_symbol *ksym;
>>       char owner[MODULE_NAME_LEN];
>>
>>       if (wait_event_interruptible_timeout(module_wq,
>>               !IS_ERR(ksym = resolve_symbol(mod, info, name, owner))
>>               || PTR_ERR(ksym) != -EBUSY,
>>                        30 * HZ) <= 0) {
>>           pr_warn("%s: gave up waiting for init of module %s.\n",
>>               mod->name, owner);
>>
>> }
> Commit 9bea7f23952d5 ("module: fix bne2 "gave up waiting for init of
> module libcrc32c") is worth reviewing. It dealt with a similar issue,
> and in particular it addressed the issue with -EBUSY being returned
> by ref_module().
>
> And so, in theory that case should be dealt with in resolve_symbol_wait()
> already. And so can you try this just to verify something:
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 40ec9a030eec..98f87cbb37de 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1459,7 +1459,7 @@ resolve_symbol_wait(struct module *mod,
>   if (wait_event_interruptible_timeout(module_wq,
>   !IS_ERR(ksym = resolve_symbol(mod, info, name, owner))
>   || PTR_ERR(ksym) != -EBUSY,
> -  30 * HZ) <= 0) {
> +  160 * HZ) <= 0) {
>   pr_warn("%s: gave up waiting for init of module %s.\n",
>   mod->name, owner);
>   }
>
Hi Luis:

Thanks so much for the reply and patch.:)

I am afraid that this patch wouldn't work in this case as the 
request_module("kvmgt") happens in the init_module of i915. Before the 
initialization path is finished in i915, the i915 symbols are not 
available to be referenced. Unfortunately, It's matter of sequence, not 
of delay. :(

>> code:
>> https://github.com/intel/gvt-linux/blob/bd950a66c7919d7121d2530f30984351534a96dc/kernel/module.c#L1452
>>
>> In resolve_symbol_wait(), it calls resolve_symbol() to resolve the
>> symbols in "i915". In resolve_symbol() -> ref_module() ->
>> strong_try_module_get(), it will check the status of the module which
>> owns the symbol.
>>
>> static inline int strong_try_module_get(struct module *mod)
>> {
>>       BUG_ON(mod && mod->state == MODULE_STATE_UNFORMED);
>>       if (mod && mod->state == MODULE_STATE_COMING)
>>           return -EBUSY;
>>       if (try_module_get(mod))
>>           return 0

Re: [Intel-gfx] refactor the i915 GVT support

[Wang, Zhi A]

Hey guys:

After some investigation, I found the root cause this problem ("i915" 
module loading will be stuck with Christoph's refactor patches), which 
can be reproduced by building both i915 and kvmgt as kernel module and 
the loading i915.

The root cause is: in Linux kernel loading, before a kernel module 
loading is finished, its symbols can not be reached by other module when 
resolving the symbols (even they can be found in /proc/kallsyms). 
Because the status of the kernel module is MODULE_STATE_COMING and 
resolve_symbol() from another kernel module will check this and return a 
-EBUSY.

In this case, before i915 loading is finished, the requested module 
"kvmgt" cannot reach the symbols in module i915. Thus it kept waiting 
and left message like below in the dmesg:

[  644.152021] kvmgt: gave up waiting for init of module i915.
[  644.152039] kvmgt: Unknown symbol i915_gem_object_set_to_cpu_domain 
(err -16)
[  674.871409] kvmgt: gave up waiting for init of module i915.
[  674.871427] kvmgt: Unknown symbol intel_ring_begin (err -16)
[  705.590586] kvmgt: gave up waiting for init of module i915.
[  705.590604] kvmgt: Unknown symbol i915_vma_move_to_active (err -16)
[  736.310230] kvmgt: gave up waiting for init of module i915.
[  736.310248] kvmgt: Unknown symbol shmem_unpin_map (err -16)
...

The error message is from execution path below:

kernel/module.c:

[i915 module loading] -> 
request_module("kvmgt")->[modprobe]->init_module("kvmgt")->load_module()->simplify_symbols()->resolve_symbol_wait():

static const struct kernel_symbol *
resolve_symbol_wait(struct module *mod,
             const struct load_info *info,
             const char *name)
{
     const struct kernel_symbol *ksym;
     char owner[MODULE_NAME_LEN];

     if (wait_event_interruptible_timeout(module_wq,
             !IS_ERR(ksym = resolve_symbol(mod, info, name, owner))
             || PTR_ERR(ksym) != -EBUSY,
                      30 * HZ) <= 0) {
         pr_warn("%s: gave up waiting for init of module %s.\n",
             mod->name, owner);

}

code: 
https://github.com/intel/gvt-linux/blob/bd950a66c7919d7121d2530f30984351534a96dc/kernel/module.c#L1452

In resolve_symbol_wait(), it calls resolve_symbol() to resolve the 
symbols in "i915". In resolve_symbol() -> ref_module() -> 
strong_try_module_get(), it will check the status of the module which 
owns the symbol.

static inline int strong_try_module_get(struct module *mod)
{
     BUG_ON(mod && mod->state == MODULE_STATE_UNFORMED);
     if (mod && mod->state == MODULE_STATE_COMING)
         return -EBUSY;
     if (try_module_get(mod))
         return 0;
     else
         return -ENOENT;
}

code:https://github.com/intel/gvt-linux/blob/bd950a66c7919d7121d2530f30984351534a96dc/kernel/module.c#L318

But unfortunately, this execution path begins in i915 module loading, at 
this time, the status of kernel module "i915" is MODULE_STATE_COMING 
until loading of "kvmgt" is finished. Thus a -EBUSY is always returned 
when kernel is trying to resolve symbols for "kvmgt".

This patch below might need re-work:

Author: Christoph Hellwig 
Date:   Wed Jul 21 17:53:38 2021 +0200

     drm/i915/gvt: move the gvt code into kvmgt.ko

     Instead of having an option to build the gvt code into the main i915
     module, just move it into the kvmgt.ko module.  This only requires
     a new struct with three entries that the main i915 module needs to
     request before enabling VGPU passthrough operations.

     This also conveniently streamlines the GVT initialization and avoids
     the need for the global device pointer.

     Signed-off-by: Christoph Hellwig 
     Signed-off-by: Zhenyu Wang 
     Link: 
http://patchwork.freedesktop.org/patch/msgid/20210721155355.173183-5-...@lst.de
     Acked-by: Zhenyu Wang 

On 8/26/21 6:12 AM, Zhenyu Wang wrote:
> On 2021.08.20 12:56:34 -0700, Luis Chamberlain wrote:
>> On Fri, Aug 20, 2021 at 04:17:24PM +0200, Christoph Hellwig wrote:
>>> On Thu, Aug 19, 2021 at 04:29:29PM +0800, Zhenyu Wang wrote:
 I'm working on below patch to resolve this. But I met a weird issue in
 case when building i915 as module and also kvmgt module, it caused
 busy wait on request_module("kvmgt") when boot, it doesn't happen if
 building i915 into kernel. I'm not sure what could be the reason?
>>> Luis, do you know if there is a problem with a request_module from
>>> a driver ->probe routine that is probably called by a module_init
>>> function itself?
>> Generally no, but you can easily foot yourself in the feet by creating
>> cross dependencies and not dealing with them properly. I'd make sure
>> to keep module initialization as simple as possible, and run whatever
>> takes more time asynchronously, then use a state machine to allow
>> you to verify where you are in the initialization phase or query it
>> or wait for a completion with a timeout.
>>
>> It seems the code in question is getting some spring cleaning, and its
>> unclear where the code is I 

Re: [Intel-gfx] refactor the i915 GVT support

[Wang, Zhi A]

On 7/28/2021 8:59 PM, Jason Gunthorpe wrote:
> On Wed, Jul 28, 2021 at 01:38:58PM +0000, Wang, Zhi A wrote:
>
>> I guess those APIs you were talking about are KVM-only. For other
>> hypervisors, e.g. Xen, ARCN cannot use the APIs you mentioned. Not
>> sure if you have already noticed that VFIO is KVM-only right now.
> There is very little hard connection between VFIO and KVM, so no, I
> don't think that is completely true.
>
> In an event, an in-tree version of other hypervisor support for GVT
> needs to go through enabling VFIO support so that the existing API
> multiplexers we have can be used properly, not adding a shim layer
> trying to recreate VFIO inside a GPU driver.

We were delivering the presentation of GVT-g in Xen summit 2018 and we 
were thinking and talking about supporting VFIO in Xen during the 
presentation (the video can be found from Youtube). But we didn't see 
any motiviation from the Xen community to adopt it.

If people take a look into the code in QEMU, in the PCI-passthrough 
part, Xen is actually not using VFIO even nowadays. We would be glad to 
see someone can influence on that part, especically making all the 
in-kernel hypervisor to use VFIO in PCI-passthrough and supporting mdev. 
That would be a huge benefit for all the users.

>> GVT-g is designed for many hypervisors not only KVM. In the design,
>> we implemented an abstraction layer for different hypervisors. You
>> can check the link in the previous email which has an example of how
>> the MPT module "xengt" supports GVT-g running under Xen.  For
>> example, injecting a msi in VFIO/KVM is via playing with
>> eventfd. But in Xen, we need to issue a hypercall from Dom0.
> This is obviously bad design, Xen should plug into the standardized
> eventfd scheme as well and trigger its hypercall this way. Then it can
> integrate with the existing VFIO interrupt abstraction infrastructure.
>
>> others, like querying mappings between GFN and HFN.
> This should be done through VFIO containers, there is nothing KVM
> specific there.
>
>> As you can see, to survive from this situation, we have to rely on
>> an abstraction layer so that we can prevent introducing coding
>> blocks like in the core logic:
> No, you have to fix the abstractions we already have to support the
> matrix of things you care about. If this can't be done then maybe we
> can add new abstractions, but abstractions like this absoultely should
> not be done inside drivers.
>
> Jason

That's a good point and we were actually thinking about this before and 
I believe that's the correct direction. But just like the situation 
mentioned above, it would be nice if people can really put a great 
influence on all in-kernel hypervisors to use VFIO which can really 
benefit all the users.

For now, we are just going to take christoph's patches.

Zhi

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx