diff mbox series

[v4.1] drm/i915/gvt: Explicitly check that vGPU is attached before shadowing

Message ID 20230801230520.1509954-2-seanjc@google.com (mailing list archive)
State New, archived
Headers show
Series [v4.1] drm/i915/gvt: Explicitly check that vGPU is attached before shadowing | expand

Commit Message

Sean Christopherson Aug. 1, 2023, 11:05 p.m. UTC
Move the check that a vGPU is attached from is_2MB_gtt_possible() all the
way up to shadow_ppgtt_mm() to avoid unnecessary work, and to make it more
obvious that a future cleanup of is_2MB_gtt_possible() isn't introducing a
bug.

is_2MB_gtt_possible() has only one caller, ppgtt_populate_shadow_entry(),
and all paths in ppgtt_populate_shadow_entry() eventually check for
attachment by way of intel_gvt_dma_map_guest_page().

And of the paths that lead to ppgtt_populate_shadow_entry(),
shadow_ppgtt_mm() is the only one that doesn't already check for
INTEL_VGPU_STATUS_ACTIVE or INTEL_VGPU_STATUS_ATTACHED.

  workload_thread() <= pick_next_workload() => INTEL_VGPU_STATUS_ACTIVE
  |
  -> dispatch_workload()
     |
     |-> prepare_workload()
         |
         -> intel_vgpu_sync_oos_pages()
         |  |
         |  |-> ppgtt_set_guest_page_sync()
         |      |
         |      |-> sync_oos_page()
         |          |
         |          |-> ppgtt_populate_shadow_entry()
         |
         |-> intel_vgpu_flush_post_shadow()
             |
  1:         |-> ppgtt_handle_guest_write_page_table()
                 |
                 |-> ppgtt_handle_guest_entry_add()
                     |
  2:                 | -> ppgtt_populate_spt_by_guest_entry()
                     |    |
                     |    |-> ppgtt_populate_spt()
                     |        |
                     |        |-> ppgtt_populate_shadow_entry()
                     |            |
                     |            |-> ppgtt_populate_spt_by_guest_entry() [see 2]
                     |
                     |-> ppgtt_populate_shadow_entry()

  kvmgt_page_track_write()  <= KVM callback => INTEL_VGPU_STATUS_ATTACHED
  |
  |-> intel_vgpu_page_track_handler()
      |
      |-> ppgtt_write_protection_handler()
          |
          |-> ppgtt_handle_guest_write_page_table_bytes()
              |
              |-> ppgtt_handle_guest_write_page_table() [see 1]

Signed-off-by: Sean Christopherson <seanjc@google.com>
---

v4.1:

 - Actually make the code do what the changelog says. [Yan]
 - Fix a typo in the changelog. [Yan]

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


base-commit: 03e8f77e106ba1d2fd980f8b38339dad33333a07

Comments

Yan Zhao Aug. 2, 2023, 1:22 a.m. UTC | #1
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
Tested-by: Yan Zhao <yan.y.zhao@intel.com>

On Tue, Aug 01, 2023 at 04:05:21PM -0700, Sean Christopherson wrote:
> Move the check that a vGPU is attached from is_2MB_gtt_possible() all the
> way up to shadow_ppgtt_mm() to avoid unnecessary work, and to make it more
> obvious that a future cleanup of is_2MB_gtt_possible() isn't introducing a
> bug.
> 
> is_2MB_gtt_possible() has only one caller, ppgtt_populate_shadow_entry(),
> and all paths in ppgtt_populate_shadow_entry() eventually check for
> attachment by way of intel_gvt_dma_map_guest_page().
> 
> And of the paths that lead to ppgtt_populate_shadow_entry(),
> shadow_ppgtt_mm() is the only one that doesn't already check for
> INTEL_VGPU_STATUS_ACTIVE or INTEL_VGPU_STATUS_ATTACHED.
> 
>   workload_thread() <= pick_next_workload() => INTEL_VGPU_STATUS_ACTIVE
>   |
>   -> dispatch_workload()
>      |
>      |-> prepare_workload()
>          |
>          -> intel_vgpu_sync_oos_pages()
>          |  |
>          |  |-> ppgtt_set_guest_page_sync()
>          |      |
>          |      |-> sync_oos_page()
>          |          |
>          |          |-> ppgtt_populate_shadow_entry()
>          |
>          |-> intel_vgpu_flush_post_shadow()
>              |
>   1:         |-> ppgtt_handle_guest_write_page_table()
>                  |
>                  |-> ppgtt_handle_guest_entry_add()
>                      |
>   2:                 | -> ppgtt_populate_spt_by_guest_entry()
>                      |    |
>                      |    |-> ppgtt_populate_spt()
>                      |        |
>                      |        |-> ppgtt_populate_shadow_entry()
>                      |            |
>                      |            |-> ppgtt_populate_spt_by_guest_entry() [see 2]
>                      |
>                      |-> ppgtt_populate_shadow_entry()
> 
>   kvmgt_page_track_write()  <= KVM callback => INTEL_VGPU_STATUS_ATTACHED
>   |
>   |-> intel_vgpu_page_track_handler()
>       |
>       |-> ppgtt_write_protection_handler()
>           |
>           |-> ppgtt_handle_guest_write_page_table_bytes()
>               |
>               |-> ppgtt_handle_guest_write_page_table() [see 1]
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> 
> v4.1:
> 
>  - Actually make the code do what the changelog says. [Yan]
>  - Fix a typo in the changelog. [Yan]
> 
>  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..de6a484090d7 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;
> @@ -1827,6 +1825,9 @@ static int shadow_ppgtt_mm(struct intel_vgpu_mm *mm)
>  	if (mm->ppgtt_mm.shadowed)
>  		return 0;
>  
> +	if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status))
> +		return -EINVAL;
> +
>  	mm->ppgtt_mm.shadowed = true;
>  
>  	for (index = 0; index < ARRAY_SIZE(mm->ppgtt_mm.guest_pdps); index++) {
> 
> base-commit: 03e8f77e106ba1d2fd980f8b38339dad33333a07
> -- 
> 2.41.0.585.gd2178a4bd4-goog
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 5426a27c1b71..de6a484090d7 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;
@@ -1827,6 +1825,9 @@  static int shadow_ppgtt_mm(struct intel_vgpu_mm *mm)
 	if (mm->ppgtt_mm.shadowed)
 		return 0;
 
+	if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status))
+		return -EINVAL;
+
 	mm->ppgtt_mm.shadowed = true;
 
 	for (index = 0; index < ARRAY_SIZE(mm->ppgtt_mm.guest_pdps); index++) {