* [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init
2022-11-23 13:48 [iommufd 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open Yi Liu
@ 2022-11-23 13:48 ` Yi Liu
2022-11-24 7:07 ` Tian, Kevin
2022-11-28 10:22 ` Wang, Zhi A
2022-11-23 13:48 ` [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable Yi Liu
` (2 subsequent siblings)
3 siblings, 2 replies; 22+ messages in thread
From: Yi Liu @ 2022-11-23 13:48 UTC (permalink / raw)
To: jgg
Cc: alex.williamson, kevin.tian, yi.l.liu, chao.p.peng, kvm,
yi.y.sun, Zhenyu Wang, Zhi Wang
vfio_iommufd_bind() creates an access which has an unmap callback, which
can be called immediately. So dma_unmap() callback should tolerate the
unmaps that come before the emulated device is opened.
To achieve above, move the protect_table_init and gvt_cache_init into the
init op which is supposed to be triggered prior to the open_device() op.
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Zhi Wang <zhi.a.wang@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/gpu/drm/i915/gvt/gvt.h | 2 ++
drivers/gpu/drm/i915/gvt/kvmgt.c | 7 ++-----
drivers/gpu/drm/i915/gvt/vgpu.c | 2 ++
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index dbf8d7470b2c..a3a7e16078ba 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -754,6 +754,8 @@ void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu);
void intel_gvt_debugfs_init(struct intel_gvt *gvt);
void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
+void gvt_cache_init(struct intel_vgpu *vgpu);
+void kvmgt_protect_table_init(struct intel_vgpu *info);
int intel_gvt_page_track_add(struct intel_vgpu *info, u64 gfn);
int intel_gvt_page_track_remove(struct intel_vgpu *info, u64 gfn);
int intel_gvt_dma_pin_guest_page(struct intel_vgpu *vgpu, dma_addr_t dma_addr);
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 579b230a0f58..cb21b1ba4162 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -322,7 +322,7 @@ static void gvt_cache_destroy(struct intel_vgpu *vgpu)
}
}
-static void gvt_cache_init(struct intel_vgpu *vgpu)
+void gvt_cache_init(struct intel_vgpu *vgpu)
{
vgpu->gfn_cache = RB_ROOT;
vgpu->dma_addr_cache = RB_ROOT;
@@ -330,7 +330,7 @@ static void gvt_cache_init(struct intel_vgpu *vgpu)
mutex_init(&vgpu->cache_lock);
}
-static void kvmgt_protect_table_init(struct intel_vgpu *info)
+void kvmgt_protect_table_init(struct intel_vgpu *info)
{
hash_init(info->ptable);
}
@@ -671,9 +671,6 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev)
vgpu->attached = true;
- kvmgt_protect_table_init(vgpu);
- gvt_cache_init(vgpu);
-
vgpu->track_node.track_write = kvmgt_page_track_write;
vgpu->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 56c71474008a..036e1a72a26b 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -382,6 +382,8 @@ int intel_gvt_create_vgpu(struct intel_vgpu *vgpu,
intel_gvt_update_reg_whitelist(vgpu);
mutex_unlock(&gvt->lock);
+ kvmgt_protect_table_init(vgpu);
+ gvt_cache_init(vgpu);
return 0;
out_clean_sched_policy:
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* RE: [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init
2022-11-23 13:48 ` [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init Yi Liu
@ 2022-11-24 7:07 ` Tian, Kevin
2022-11-24 9:15 ` Yi Liu
2022-11-28 10:22 ` Wang, Zhi A
1 sibling, 1 reply; 22+ messages in thread
From: Tian, Kevin @ 2022-11-24 7:07 UTC (permalink / raw)
To: Liu, Yi L, jgg
Cc: alex.williamson, chao.p.peng, kvm, yi.y.sun, Zhenyu Wang, Wang, Zhi A
> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Wednesday, November 23, 2022 9:49 PM
>
> vfio_iommufd_bind() creates an access which has an unmap callback, which
> can be called immediately. So dma_unmap() callback should tolerate the
> unmaps that come before the emulated device is opened.
this should first talk about how it works today and then why iommufd changes
it.
>
> To achieve above, move the protect_table_init and gvt_cache_init into the
> init op which is supposed to be triggered prior to the open_device() op.
what about below?
--
vfio container registers .dma_unmap() callback after the device is opened.
So it's fine for mdev drivers to initialize internal mapping cache in
.open_device(). See vfio_device_container_register().
Now with iommufd an access ops with an unmap callback is registered
when the device is bound to iommufd which is before .open_device()
is called. This implies gvt's .dma_unmap() could be called before its
internal mapping cache is initialized.
The fix is moving gvt mapping cache initialization to vGPU creation.
While at it also move ptable initialization together.
>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/gpu/drm/i915/gvt/gvt.h | 2 ++
> drivers/gpu/drm/i915/gvt/kvmgt.c | 7 ++-----
> drivers/gpu/drm/i915/gvt/vgpu.c | 2 ++
> 3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index dbf8d7470b2c..a3a7e16078ba 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -754,6 +754,8 @@ void intel_gvt_debugfs_remove_vgpu(struct
> intel_vgpu *vgpu);
> void intel_gvt_debugfs_init(struct intel_gvt *gvt);
> void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
>
> +void gvt_cache_init(struct intel_vgpu *vgpu);
> +void kvmgt_protect_table_init(struct intel_vgpu *info);
> int intel_gvt_page_track_add(struct intel_vgpu *info, u64 gfn);
> int intel_gvt_page_track_remove(struct intel_vgpu *info, u64 gfn);
> int intel_gvt_dma_pin_guest_page(struct intel_vgpu *vgpu, dma_addr_t
> dma_addr);
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 579b230a0f58..cb21b1ba4162 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -322,7 +322,7 @@ static void gvt_cache_destroy(struct intel_vgpu *vgpu)
> }
> }
>
> -static void gvt_cache_init(struct intel_vgpu *vgpu)
> +void gvt_cache_init(struct intel_vgpu *vgpu)
those are local functions. Just move to vgpu.c.
or you can remove the function wrap and directly put the internal lines
in intel_gvt_create_vgpu()
> {
> vgpu->gfn_cache = RB_ROOT;
> vgpu->dma_addr_cache = RB_ROOT;
> @@ -330,7 +330,7 @@ static void gvt_cache_init(struct intel_vgpu *vgpu)
> mutex_init(&vgpu->cache_lock);
> }
>
> -static void kvmgt_protect_table_init(struct intel_vgpu *info)
> +void kvmgt_protect_table_init(struct intel_vgpu *info)
> {
> hash_init(info->ptable);
> }
> @@ -671,9 +671,6 @@ static int intel_vgpu_open_device(struct vfio_device
> *vfio_dev)
>
> vgpu->attached = true;
>
> - kvmgt_protect_table_init(vgpu);
> - gvt_cache_init(vgpu);
> -
> vgpu->track_node.track_write = kvmgt_page_track_write;
> vgpu->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
> kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c
> b/drivers/gpu/drm/i915/gvt/vgpu.c
> index 56c71474008a..036e1a72a26b 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -382,6 +382,8 @@ int intel_gvt_create_vgpu(struct intel_vgpu *vgpu,
>
> intel_gvt_update_reg_whitelist(vgpu);
> mutex_unlock(&gvt->lock);
> + kvmgt_protect_table_init(vgpu);
> + gvt_cache_init(vgpu);
> return 0;
>
> out_clean_sched_policy:
> --
> 2.34.1
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init
2022-11-24 7:07 ` Tian, Kevin
@ 2022-11-24 9:15 ` Yi Liu
2022-11-25 6:04 ` Zhenyu Wang
0 siblings, 1 reply; 22+ messages in thread
From: Yi Liu @ 2022-11-24 9:15 UTC (permalink / raw)
To: Tian, Kevin, jgg
Cc: alex.williamson, chao.p.peng, kvm, yi.y.sun, Zhenyu Wang, Wang, Zhi A
On 2022/11/24 15:07, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Wednesday, November 23, 2022 9:49 PM
>>
>> vfio_iommufd_bind() creates an access which has an unmap callback, which
>> can be called immediately. So dma_unmap() callback should tolerate the
>> unmaps that come before the emulated device is opened.
>
> this should first talk about how it works today and then why iommufd changes
> it.
>
>>
>> To achieve above, move the protect_table_init and gvt_cache_init into the
>> init op which is supposed to be triggered prior to the open_device() op.
>
> what about below?
> --
> vfio container registers .dma_unmap() callback after the device is opened.
> So it's fine for mdev drivers to initialize internal mapping cache in
> .open_device(). See vfio_device_container_register().
>
> Now with iommufd an access ops with an unmap callback is registered
> when the device is bound to iommufd which is before .open_device()
> is called. This implies gvt's .dma_unmap() could be called before its
> internal mapping cache is initialized.
>
> The fix is moving gvt mapping cache initialization to vGPU creation.
> While at it also move ptable initialization together.
much clearer :-)
>>
>> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
>> Cc: Zhi Wang <zhi.a.wang@intel.com>
>> Cc: Kevin Tian <kevin.tian@intel.com>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>> drivers/gpu/drm/i915/gvt/gvt.h | 2 ++
>> drivers/gpu/drm/i915/gvt/kvmgt.c | 7 ++-----
>> drivers/gpu/drm/i915/gvt/vgpu.c | 2 ++
>> 3 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
>> index dbf8d7470b2c..a3a7e16078ba 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.h
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
>> @@ -754,6 +754,8 @@ void intel_gvt_debugfs_remove_vgpu(struct
>> intel_vgpu *vgpu);
>> void intel_gvt_debugfs_init(struct intel_gvt *gvt);
>> void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
>>
>> +void gvt_cache_init(struct intel_vgpu *vgpu);
>> +void kvmgt_protect_table_init(struct intel_vgpu *info);
>> int intel_gvt_page_track_add(struct intel_vgpu *info, u64 gfn);
>> int intel_gvt_page_track_remove(struct intel_vgpu *info, u64 gfn);
>> int intel_gvt_dma_pin_guest_page(struct intel_vgpu *vgpu, dma_addr_t
>> dma_addr);
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 579b230a0f58..cb21b1ba4162 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -322,7 +322,7 @@ static void gvt_cache_destroy(struct intel_vgpu *vgpu)
>> }
>> }
>>
>> -static void gvt_cache_init(struct intel_vgpu *vgpu)
>> +void gvt_cache_init(struct intel_vgpu *vgpu)
>
> those are local functions. Just move to vgpu.c.
>
> or you can remove the function wrap and directly put the internal lines
> in intel_gvt_create_vgpu()
yes. maybe see Zhenyu and Zhi's input. which way is preferred by them.
>> {
>> vgpu->gfn_cache = RB_ROOT;
>> vgpu->dma_addr_cache = RB_ROOT;
>> @@ -330,7 +330,7 @@ static void gvt_cache_init(struct intel_vgpu *vgpu)
>> mutex_init(&vgpu->cache_lock);
>> }
>>
>> -static void kvmgt_protect_table_init(struct intel_vgpu *info)
>> +void kvmgt_protect_table_init(struct intel_vgpu *info)
>> {
>> hash_init(info->ptable);
>> }
>> @@ -671,9 +671,6 @@ static int intel_vgpu_open_device(struct vfio_device
>> *vfio_dev)
>>
>> vgpu->attached = true;
>>
>> - kvmgt_protect_table_init(vgpu);
>> - gvt_cache_init(vgpu);
>> -
>> vgpu->track_node.track_write = kvmgt_page_track_write;
>> vgpu->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
>> kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
>> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c
>> b/drivers/gpu/drm/i915/gvt/vgpu.c
>> index 56c71474008a..036e1a72a26b 100644
>> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
>> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
>> @@ -382,6 +382,8 @@ int intel_gvt_create_vgpu(struct intel_vgpu *vgpu,
>>
>> intel_gvt_update_reg_whitelist(vgpu);
>> mutex_unlock(&gvt->lock);
>> + kvmgt_protect_table_init(vgpu);
>> + gvt_cache_init(vgpu);
>> return 0;
>>
>> out_clean_sched_policy:
>> --
>> 2.34.1
>
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init
2022-11-24 9:15 ` Yi Liu
@ 2022-11-25 6:04 ` Zhenyu Wang
2022-11-25 9:06 ` Yi Liu
0 siblings, 1 reply; 22+ messages in thread
From: Zhenyu Wang @ 2022-11-25 6:04 UTC (permalink / raw)
To: Yi Liu
Cc: Tian, Kevin, jgg, alex.williamson, chao.p.peng, kvm, yi.y.sun,
Zhenyu Wang, Wang, Zhi A
[-- Attachment #1: Type: text/plain, Size: 4997 bytes --]
On 2022.11.24 17:15:12 +0800, Yi Liu wrote:
> On 2022/11/24 15:07, Tian, Kevin wrote:
> > > From: Liu, Yi L <yi.l.liu@intel.com>
> > > Sent: Wednesday, November 23, 2022 9:49 PM
> > >
> > > vfio_iommufd_bind() creates an access which has an unmap callback, which
> > > can be called immediately. So dma_unmap() callback should tolerate the
> > > unmaps that come before the emulated device is opened.
> >
> > this should first talk about how it works today and then why iommufd changes
> > it.
> >
> > >
> > > To achieve above, move the protect_table_init and gvt_cache_init into the
> > > init op which is supposed to be triggered prior to the open_device() op.
> >
> > what about below?
> > --
> > vfio container registers .dma_unmap() callback after the device is opened.
> > So it's fine for mdev drivers to initialize internal mapping cache in
> > .open_device(). See vfio_device_container_register().
> >
> > Now with iommufd an access ops with an unmap callback is registered
> > when the device is bound to iommufd which is before .open_device()
> > is called. This implies gvt's .dma_unmap() could be called before its
> > internal mapping cache is initialized.
> >
> > The fix is moving gvt mapping cache initialization to vGPU creation.
> > While at it also move ptable initialization together.
>
> much clearer :-)
>
Current gvt internal cache is handled with .open_device() and .close_device() pair,
so those internal cache is now re-initialized for each device session, how is that
handled for iommufd? Looks that's missed in this patch..
> > >
> > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > Cc: Zhi Wang <zhi.a.wang@intel.com>
> > > Cc: Kevin Tian <kevin.tian@intel.com>
> > > Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/gvt/gvt.h | 2 ++
> > > drivers/gpu/drm/i915/gvt/kvmgt.c | 7 ++-----
> > > drivers/gpu/drm/i915/gvt/vgpu.c | 2 ++
> > > 3 files changed, 6 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> > > index dbf8d7470b2c..a3a7e16078ba 100644
> > > --- a/drivers/gpu/drm/i915/gvt/gvt.h
> > > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > > @@ -754,6 +754,8 @@ void intel_gvt_debugfs_remove_vgpu(struct
> > > intel_vgpu *vgpu);
> > > void intel_gvt_debugfs_init(struct intel_gvt *gvt);
> > > void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
> > >
> > > +void gvt_cache_init(struct intel_vgpu *vgpu);
> > > +void kvmgt_protect_table_init(struct intel_vgpu *info);
> > > int intel_gvt_page_track_add(struct intel_vgpu *info, u64 gfn);
> > > int intel_gvt_page_track_remove(struct intel_vgpu *info, u64 gfn);
> > > int intel_gvt_dma_pin_guest_page(struct intel_vgpu *vgpu, dma_addr_t
> > > dma_addr);
> > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > index 579b230a0f58..cb21b1ba4162 100644
> > > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > @@ -322,7 +322,7 @@ static void gvt_cache_destroy(struct intel_vgpu *vgpu)
> > > }
> > > }
> > >
> > > -static void gvt_cache_init(struct intel_vgpu *vgpu)
> > > +void gvt_cache_init(struct intel_vgpu *vgpu)
> >
> > those are local functions. Just move to vgpu.c.
> >
> > or you can remove the function wrap and directly put the internal lines
> > in intel_gvt_create_vgpu()
>
> yes. maybe see Zhenyu and Zhi's input. which way is preferred by them.
>
> > > {
> > > vgpu->gfn_cache = RB_ROOT;
> > > vgpu->dma_addr_cache = RB_ROOT;
> > > @@ -330,7 +330,7 @@ static void gvt_cache_init(struct intel_vgpu *vgpu)
> > > mutex_init(&vgpu->cache_lock);
> > > }
> > >
> > > -static void kvmgt_protect_table_init(struct intel_vgpu *info)
> > > +void kvmgt_protect_table_init(struct intel_vgpu *info)
> > > {
> > > hash_init(info->ptable);
> > > }
> > > @@ -671,9 +671,6 @@ static int intel_vgpu_open_device(struct vfio_device
> > > *vfio_dev)
> > >
> > > vgpu->attached = true;
> > >
> > > - kvmgt_protect_table_init(vgpu);
> > > - gvt_cache_init(vgpu);
> > > -
> > > vgpu->track_node.track_write = kvmgt_page_track_write;
> > > vgpu->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
> > > kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
> > > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c
> > > b/drivers/gpu/drm/i915/gvt/vgpu.c
> > > index 56c71474008a..036e1a72a26b 100644
> > > --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> > > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> > > @@ -382,6 +382,8 @@ int intel_gvt_create_vgpu(struct intel_vgpu *vgpu,
> > >
> > > intel_gvt_update_reg_whitelist(vgpu);
> > > mutex_unlock(&gvt->lock);
> > > + kvmgt_protect_table_init(vgpu);
> > > + gvt_cache_init(vgpu);
> > > return 0;
> > >
> > > out_clean_sched_policy:
> > > --
> > > 2.34.1
> >
>
> --
> Regards,
> Yi Liu
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init
2022-11-25 6:04 ` Zhenyu Wang
@ 2022-11-25 9:06 ` Yi Liu
2022-11-28 6:28 ` Tian, Kevin
0 siblings, 1 reply; 22+ messages in thread
From: Yi Liu @ 2022-11-25 9:06 UTC (permalink / raw)
To: Zhenyu Wang
Cc: Tian, Kevin, jgg, alex.williamson, chao.p.peng, kvm, yi.y.sun,
Wang, Zhi A
On 2022/11/25 14:04, Zhenyu Wang wrote:
> On 2022.11.24 17:15:12 +0800, Yi Liu wrote:
>> On 2022/11/24 15:07, Tian, Kevin wrote:
>>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>>> Sent: Wednesday, November 23, 2022 9:49 PM
>>>>
>>>> vfio_iommufd_bind() creates an access which has an unmap callback, which
>>>> can be called immediately. So dma_unmap() callback should tolerate the
>>>> unmaps that come before the emulated device is opened.
>>>
>>> this should first talk about how it works today and then why iommufd changes
>>> it.
>>>
>>>>
>>>> To achieve above, move the protect_table_init and gvt_cache_init into the
>>>> init op which is supposed to be triggered prior to the open_device() op.
>>>
>>> what about below?
>>> --
>>> vfio container registers .dma_unmap() callback after the device is opened.
>>> So it's fine for mdev drivers to initialize internal mapping cache in
>>> .open_device(). See vfio_device_container_register().
>>>
>>> Now with iommufd an access ops with an unmap callback is registered
>>> when the device is bound to iommufd which is before .open_device()
>>> is called. This implies gvt's .dma_unmap() could be called before its
>>> internal mapping cache is initialized.
>>>
>>> The fix is moving gvt mapping cache initialization to vGPU creation.
>>> While at it also move ptable initialization together.
>>
>> much clearer :-)
>>
>
> Current gvt internal cache is handled with .open_device() and .close_device() pair,
> so those internal cache is now re-initialized for each device session, how is that
> handled for iommufd? Looks that's missed in this patch..
you are right. I noticed below two helpers are used to destroy. However,
the code seems to be more clear the internal cache. So seems no need to
re-initialize. I'm no expert here. :)
gvt_cache_destroy()
kvmgt_protect_table_destroy()
>>>>
>>>> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
>>>> Cc: Zhi Wang <zhi.a.wang@intel.com>
>>>> Cc: Kevin Tian <kevin.tian@intel.com>
>>>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/gvt/gvt.h | 2 ++
>>>> drivers/gpu/drm/i915/gvt/kvmgt.c | 7 ++-----
>>>> drivers/gpu/drm/i915/gvt/vgpu.c | 2 ++
>>>> 3 files changed, 6 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
>>>> index dbf8d7470b2c..a3a7e16078ba 100644
>>>> --- a/drivers/gpu/drm/i915/gvt/gvt.h
>>>> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
>>>> @@ -754,6 +754,8 @@ void intel_gvt_debugfs_remove_vgpu(struct
>>>> intel_vgpu *vgpu);
>>>> void intel_gvt_debugfs_init(struct intel_gvt *gvt);
>>>> void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
>>>>
>>>> +void gvt_cache_init(struct intel_vgpu *vgpu);
>>>> +void kvmgt_protect_table_init(struct intel_vgpu *info);
>>>> int intel_gvt_page_track_add(struct intel_vgpu *info, u64 gfn);
>>>> int intel_gvt_page_track_remove(struct intel_vgpu *info, u64 gfn);
>>>> int intel_gvt_dma_pin_guest_page(struct intel_vgpu *vgpu, dma_addr_t
>>>> dma_addr);
>>>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>>>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>>>> index 579b230a0f58..cb21b1ba4162 100644
>>>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>>>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>>>> @@ -322,7 +322,7 @@ static void gvt_cache_destroy(struct intel_vgpu *vgpu)
>>>> }
>>>> }
>>>>
>>>> -static void gvt_cache_init(struct intel_vgpu *vgpu)
>>>> +void gvt_cache_init(struct intel_vgpu *vgpu)
>>>
>>> those are local functions. Just move to vgpu.c.
>>>
>>> or you can remove the function wrap and directly put the internal lines
>>> in intel_gvt_create_vgpu()
>>
>> yes. maybe see Zhenyu and Zhi's input. which way is preferred by them.
>>
>>>> {
>>>> vgpu->gfn_cache = RB_ROOT;
>>>> vgpu->dma_addr_cache = RB_ROOT;
>>>> @@ -330,7 +330,7 @@ static void gvt_cache_init(struct intel_vgpu *vgpu)
>>>> mutex_init(&vgpu->cache_lock);
>>>> }
>>>>
>>>> -static void kvmgt_protect_table_init(struct intel_vgpu *info)
>>>> +void kvmgt_protect_table_init(struct intel_vgpu *info)
>>>> {
>>>> hash_init(info->ptable);
>>>> }
>>>> @@ -671,9 +671,6 @@ static int intel_vgpu_open_device(struct vfio_device
>>>> *vfio_dev)
>>>>
>>>> vgpu->attached = true;
>>>>
>>>> - kvmgt_protect_table_init(vgpu);
>>>> - gvt_cache_init(vgpu);
>>>> -
>>>> vgpu->track_node.track_write = kvmgt_page_track_write;
>>>> vgpu->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
>>>> kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
>>>> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c
>>>> b/drivers/gpu/drm/i915/gvt/vgpu.c
>>>> index 56c71474008a..036e1a72a26b 100644
>>>> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
>>>> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
>>>> @@ -382,6 +382,8 @@ int intel_gvt_create_vgpu(struct intel_vgpu *vgpu,
>>>>
>>>> intel_gvt_update_reg_whitelist(vgpu);
>>>> mutex_unlock(&gvt->lock);
>>>> + kvmgt_protect_table_init(vgpu);
>>>> + gvt_cache_init(vgpu);
>>>> return 0;
>>>>
>>>> out_clean_sched_policy:
>>>> --
>>>> 2.34.1
>>>
>>
>> --
>> Regards,
>> Yi Liu
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init
2022-11-25 9:06 ` Yi Liu
@ 2022-11-28 6:28 ` Tian, Kevin
0 siblings, 0 replies; 22+ messages in thread
From: Tian, Kevin @ 2022-11-28 6:28 UTC (permalink / raw)
To: Liu, Yi L, Zhenyu Wang
Cc: jgg, alex.williamson, chao.p.peng, kvm, yi.y.sun, Wang, Zhi A
> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Friday, November 25, 2022 5:07 PM
> >
> > Current gvt internal cache is handled with .open_device()
> and .close_device() pair,
> > so those internal cache is now re-initialized for each device session, how is
> that
> > handled for iommufd? Looks that's missed in this patch..
>
> you are right. I noticed below two helpers are used to destroy. However,
> the code seems to be more clear the internal cache. So seems no need to
> re-initialize. I'm no expert here. :)
>
> gvt_cache_destroy()
> kvmgt_protect_table_destroy()
>
It's common practice to have init in the probe path and then do the
housekeeping in the close path.
Re-init in every open is unnecessary.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init
2022-11-23 13:48 ` [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init Yi Liu
2022-11-24 7:07 ` Tian, Kevin
@ 2022-11-28 10:22 ` Wang, Zhi A
2022-11-28 13:10 ` Yi Liu
1 sibling, 1 reply; 22+ messages in thread
From: Wang, Zhi A @ 2022-11-28 10:22 UTC (permalink / raw)
To: Liu, Yi L, jgg
Cc: alex.williamson, Tian, Kevin, chao.p.peng, kvm, yi.y.sun, Zhenyu Wang
On 11/23/22 13:48, Liu, Yi L wrote:
> vfio_iommufd_bind() creates an access which has an unmap callback, which
> can be called immediately. So dma_unmap() callback should tolerate the
> unmaps that come before the emulated device is opened.
After reading code on Jason's tree, I think it would be nice to document
the requirements for a vfio driver somewhere, like what they must do and
must not do in the callbacks of vfio_device_ops. maybe in include/linux/vfio.h?
> To achieve above, move the protect_table_init and gvt_cache_init into the
> init op which is supposed to be triggered prior to the open_device() op.
>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/gpu/drm/i915/gvt/gvt.h | 2 ++
> drivers/gpu/drm/i915/gvt/kvmgt.c | 7 ++-----
> drivers/gpu/drm/i915/gvt/vgpu.c | 2 ++
> 3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index dbf8d7470b2c..a3a7e16078ba 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -754,6 +754,8 @@ void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu);
> void intel_gvt_debugfs_init(struct intel_gvt *gvt);
> void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
>
> +void gvt_cache_init(struct intel_vgpu *vgpu);
> +void kvmgt_protect_table_init(struct intel_vgpu *info);
> int intel_gvt_page_track_add(struct intel_vgpu *info, u64 gfn);
> int intel_gvt_page_track_remove(struct intel_vgpu *info, u64 gfn);
> int intel_gvt_dma_pin_guest_page(struct intel_vgpu *vgpu, dma_addr_t dma_addr);
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 579b230a0f58..cb21b1ba4162 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -322,7 +322,7 @@ static void gvt_cache_destroy(struct intel_vgpu *vgpu)
> }
> }
>
> -static void gvt_cache_init(struct intel_vgpu *vgpu)
> +void gvt_cache_init(struct intel_vgpu *vgpu)
> {
> vgpu->gfn_cache = RB_ROOT;
> vgpu->dma_addr_cache = RB_ROOT;
> @@ -330,7 +330,7 @@ static void gvt_cache_init(struct intel_vgpu *vgpu)
> mutex_init(&vgpu->cache_lock);
> }
>
> -static void kvmgt_protect_table_init(struct intel_vgpu *info)
> +void kvmgt_protect_table_init(struct intel_vgpu *info)
> {
> hash_init(info->ptable);
> }
> @@ -671,9 +671,6 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev)
>
> vgpu->attached = true;
>
> - kvmgt_protect_table_init(vgpu);
> - gvt_cache_init(vgpu);
> -
> vgpu->track_node.track_write = kvmgt_page_track_write;
> vgpu->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
> kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index 56c71474008a..036e1a72a26b 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -382,6 +382,8 @@ int intel_gvt_create_vgpu(struct intel_vgpu *vgpu,
>
> intel_gvt_update_reg_whitelist(vgpu);
> mutex_unlock(&gvt->lock);
> + kvmgt_protect_table_init(vgpu);
> + gvt_cache_init(vgpu);
It would be nice that you can still keep the initialization in the kvmgt.c as they are
kvmgt related.
With the changes above: Reviewed-by: Zhi Wang <zhi.a.wang@intel.com>
> return 0;
>
> out_clean_sched_policy:
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init
2022-11-28 10:22 ` Wang, Zhi A
@ 2022-11-28 13:10 ` Yi Liu
0 siblings, 0 replies; 22+ messages in thread
From: Yi Liu @ 2022-11-28 13:10 UTC (permalink / raw)
To: Wang, Zhi A, jgg
Cc: alex.williamson, Tian, Kevin, chao.p.peng, kvm, yi.y.sun, Zhenyu Wang
On 2022/11/28 18:22, Wang, Zhi A wrote:
> On 11/23/22 13:48, Liu, Yi L wrote:
>> vfio_iommufd_bind() creates an access which has an unmap callback, which
>> can be called immediately. So dma_unmap() callback should tolerate the
>> unmaps that come before the emulated device is opened.
>
> After reading code on Jason's tree, I think it would be nice to document
>
> the requirements for a vfio driver somewhere, like what they must do and
>
> must not do in the callbacks of vfio_device_ops. maybe in include/linux/vfio.h?
yes. maybe vfio.rst. Altough I found it is out of date. I've got one fix
and may send out later.
>> To achieve above, move the protect_table_init and gvt_cache_init into the
>> init op which is supposed to be triggered prior to the open_device() op.
>>
>> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
>> Cc: Zhi Wang <zhi.a.wang@intel.com>
>> Cc: Kevin Tian <kevin.tian@intel.com>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>> drivers/gpu/drm/i915/gvt/gvt.h | 2 ++
>> drivers/gpu/drm/i915/gvt/kvmgt.c | 7 ++-----
>> drivers/gpu/drm/i915/gvt/vgpu.c | 2 ++
>> 3 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
>> index dbf8d7470b2c..a3a7e16078ba 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.h
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
>> @@ -754,6 +754,8 @@ void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu);
>> void intel_gvt_debugfs_init(struct intel_gvt *gvt);
>> void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
>>
>> +void gvt_cache_init(struct intel_vgpu *vgpu);
>> +void kvmgt_protect_table_init(struct intel_vgpu *info);
>> int intel_gvt_page_track_add(struct intel_vgpu *info, u64 gfn);
>> int intel_gvt_page_track_remove(struct intel_vgpu *info, u64 gfn);
>> int intel_gvt_dma_pin_guest_page(struct intel_vgpu *vgpu, dma_addr_t dma_addr);
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 579b230a0f58..cb21b1ba4162 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -322,7 +322,7 @@ static void gvt_cache_destroy(struct intel_vgpu *vgpu)
>> }
>> }
>>
>> -static void gvt_cache_init(struct intel_vgpu *vgpu)
>> +void gvt_cache_init(struct intel_vgpu *vgpu)
>> {
>> vgpu->gfn_cache = RB_ROOT;
>> vgpu->dma_addr_cache = RB_ROOT;
>> @@ -330,7 +330,7 @@ static void gvt_cache_init(struct intel_vgpu *vgpu)
>> mutex_init(&vgpu->cache_lock);
>> }
>>
>> -static void kvmgt_protect_table_init(struct intel_vgpu *info)
>> +void kvmgt_protect_table_init(struct intel_vgpu *info)
>> {
>> hash_init(info->ptable);
>> }
>> @@ -671,9 +671,6 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev)
>>
>> vgpu->attached = true;
>>
>> - kvmgt_protect_table_init(vgpu);
>> - gvt_cache_init(vgpu);
>> -
>> vgpu->track_node.track_write = kvmgt_page_track_write;
>> vgpu->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
>> kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
>> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
>> index 56c71474008a..036e1a72a26b 100644
>> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
>> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
>> @@ -382,6 +382,8 @@ int intel_gvt_create_vgpu(struct intel_vgpu *vgpu,
>>
>> intel_gvt_update_reg_whitelist(vgpu);
>> mutex_unlock(&gvt->lock);
>> + kvmgt_protect_table_init(vgpu);
>> + gvt_cache_init(vgpu);
>
> It would be nice that you can still keep the initialization in the kvmgt.c as they are
ok, so I'll leave the function implementation in kvmgt.c.
> kvmgt related.
>
> With the changes above: Reviewed-by: Zhi Wang <zhi.a.wang@intel.com>
thanks. :-)
>> return 0;
>>
>> out_clean_sched_policy:
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 22+ messages in thread
* [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable
2022-11-23 13:48 [iommufd 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open Yi Liu
2022-11-23 13:48 ` [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init Yi Liu
@ 2022-11-23 13:48 ` Yi Liu
2022-11-24 7:08 ` Tian, Kevin
2022-11-24 6:50 ` [iommufd 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open Tian, Kevin
2022-11-25 6:06 ` Zhenyu Wang
3 siblings, 1 reply; 22+ messages in thread
From: Yi Liu @ 2022-11-23 13:48 UTC (permalink / raw)
To: jgg
Cc: alex.williamson, kevin.tian, yi.l.liu, chao.p.peng, kvm,
yi.y.sun, Matthew Rosato, Tony Krowiak, Halil Pasic, Jason Herne
From: Matthew Rosato <mjrosato@linux.ibm.com>
vfio_iommufd_bind() creates an access which has an unmap callback, which
can be called immediately. So dma_unmap() callback should tolerate the
unmaps that come before the emulated device is opened.
To achieve above, vfio_ap_mdev_dma_unmap() needs to validate that unmap
request matches with one or more of these stashed values before
attempting unpins.
Currently, each mapped iova is stashed in its associated vfio_ap_queue;
Each stashed iova represents IRQ that was enabled for a queue. Therefore,
if a match is found, trigger IRQ disable for this queue to ensure that
underlying firmware will no longer try to use the associated pfn after
the page is unpinned. IRQ disable will also handle the associated unpin.
Cc: Tony Krowiak <akrowiak@linux.ibm.com>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Jason Herne <jjherne@linux.ibm.com>
Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/s390/crypto/vfio_ap_ops.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index bb7776d20792..62bfca2bbe6d 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -1535,13 +1535,35 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
return 0;
}
+static void unmap_iova(struct ap_matrix_mdev *matrix_mdev, u64 iova, u64 length)
+{
+ struct ap_queue_table *qtable = &matrix_mdev->qtable;
+ u64 iova_pfn_end = (iova + length - 1) >> PAGE_SHIFT;
+ u64 iova_pfn_start = iova >> PAGE_SHIFT;
+ struct vfio_ap_queue *q;
+ int loop_cursor;
+ u64 pfn;
+
+ hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) {
+ pfn = q->saved_iova >> PAGE_SHIFT;
+ if (pfn >= iova_pfn_start && pfn <= iova_pfn_end) {
+ vfio_ap_irq_disable(q);
+ break;
+ }
+ }
+}
+
static void vfio_ap_mdev_dma_unmap(struct vfio_device *vdev, u64 iova,
u64 length)
{
struct ap_matrix_mdev *matrix_mdev =
container_of(vdev, struct ap_matrix_mdev, vdev);
- vfio_unpin_pages(&matrix_mdev->vdev, iova, 1);
+ mutex_lock(&matrix_dev->mdevs_lock);
+
+ unmap_iova(matrix_mdev, iova, length);
+
+ mutex_unlock(&matrix_dev->mdevs_lock);
}
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* RE: [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable
2022-11-23 13:48 ` [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable Yi Liu
@ 2022-11-24 7:08 ` Tian, Kevin
2022-11-24 12:59 ` Jason Gunthorpe
0 siblings, 1 reply; 22+ messages in thread
From: Tian, Kevin @ 2022-11-24 7:08 UTC (permalink / raw)
To: Liu, Yi L, jgg
Cc: alex.williamson, chao.p.peng, kvm, yi.y.sun, Matthew Rosato,
Tony Krowiak, Halil Pasic, Jason Herne
> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Wednesday, November 23, 2022 9:49 PM
>
> From: Matthew Rosato <mjrosato@linux.ibm.com>
>
> vfio_iommufd_bind() creates an access which has an unmap callback, which
> can be called immediately. So dma_unmap() callback should tolerate the
> unmaps that come before the emulated device is opened.
>
> To achieve above, vfio_ap_mdev_dma_unmap() needs to validate that
> unmap
> request matches with one or more of these stashed values before
> attempting unpins.
>
> Currently, each mapped iova is stashed in its associated vfio_ap_queue;
> Each stashed iova represents IRQ that was enabled for a queue. Therefore,
> if a match is found, trigger IRQ disable for this queue to ensure that
> underlying firmware will no longer try to use the associated pfn after
> the page is unpinned. IRQ disable will also handle the associated unpin.
>
> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Jason Herne <jjherne@linux.ibm.com>
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/s390/crypto/vfio_ap_ops.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
> b/drivers/s390/crypto/vfio_ap_ops.c
> index bb7776d20792..62bfca2bbe6d 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -1535,13 +1535,35 @@ static int vfio_ap_mdev_set_kvm(struct
> ap_matrix_mdev *matrix_mdev,
> return 0;
> }
>
> +static void unmap_iova(struct ap_matrix_mdev *matrix_mdev, u64 iova,
> u64 length)
> +{
> + struct ap_queue_table *qtable = &matrix_mdev->qtable;
> + u64 iova_pfn_end = (iova + length - 1) >> PAGE_SHIFT;
> + u64 iova_pfn_start = iova >> PAGE_SHIFT;
> + struct vfio_ap_queue *q;
> + int loop_cursor;
> + u64 pfn;
> +
> + hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) {
> + pfn = q->saved_iova >> PAGE_SHIFT;
> + if (pfn >= iova_pfn_start && pfn <= iova_pfn_end) {
> + vfio_ap_irq_disable(q);
> + break;
does this need a WARN_ON if the length is more than one page?
> + }
> + }
> +}
> +
> static void vfio_ap_mdev_dma_unmap(struct vfio_device *vdev, u64 iova,
> u64 length)
> {
> struct ap_matrix_mdev *matrix_mdev =
> container_of(vdev, struct ap_matrix_mdev, vdev);
>
> - vfio_unpin_pages(&matrix_mdev->vdev, iova, 1);
> + mutex_lock(&matrix_dev->mdevs_lock);
> +
> + unmap_iova(matrix_mdev, iova, length);
> +
> + mutex_unlock(&matrix_dev->mdevs_lock);
> }
>
> /**
> --
> 2.34.1
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable
2022-11-24 7:08 ` Tian, Kevin
@ 2022-11-24 12:59 ` Jason Gunthorpe
2022-11-28 6:31 ` Tian, Kevin
2022-11-28 15:40 ` Matthew Rosato
0 siblings, 2 replies; 22+ messages in thread
From: Jason Gunthorpe @ 2022-11-24 12:59 UTC (permalink / raw)
To: Tian, Kevin
Cc: Liu, Yi L, alex.williamson, chao.p.peng, kvm, yi.y.sun,
Matthew Rosato, Tony Krowiak, Halil Pasic, Jason Herne
On Thu, Nov 24, 2022 at 07:08:06AM +0000, Tian, Kevin wrote:
> > From: Liu, Yi L <yi.l.liu@intel.com>
> > Sent: Wednesday, November 23, 2022 9:49 PM
> >
> > From: Matthew Rosato <mjrosato@linux.ibm.com>
> >
> > vfio_iommufd_bind() creates an access which has an unmap callback, which
> > can be called immediately. So dma_unmap() callback should tolerate the
> > unmaps that come before the emulated device is opened.
> >
> > To achieve above, vfio_ap_mdev_dma_unmap() needs to validate that
> > unmap
> > request matches with one or more of these stashed values before
> > attempting unpins.
> >
> > Currently, each mapped iova is stashed in its associated vfio_ap_queue;
> > Each stashed iova represents IRQ that was enabled for a queue. Therefore,
> > if a match is found, trigger IRQ disable for this queue to ensure that
> > underlying firmware will no longer try to use the associated pfn after
> > the page is unpinned. IRQ disable will also handle the associated unpin.
> >
> > Cc: Tony Krowiak <akrowiak@linux.ibm.com>
> > Cc: Halil Pasic <pasic@linux.ibm.com>
> > Cc: Jason Herne <jjherne@linux.ibm.com>
> > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> > Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> > ---
> > drivers/s390/crypto/vfio_ap_ops.c | 24 +++++++++++++++++++++++-
> > 1 file changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/s390/crypto/vfio_ap_ops.c
> > b/drivers/s390/crypto/vfio_ap_ops.c
> > index bb7776d20792..62bfca2bbe6d 100644
> > --- a/drivers/s390/crypto/vfio_ap_ops.c
> > +++ b/drivers/s390/crypto/vfio_ap_ops.c
> > @@ -1535,13 +1535,35 @@ static int vfio_ap_mdev_set_kvm(struct
> > ap_matrix_mdev *matrix_mdev,
> > return 0;
> > }
> >
> > +static void unmap_iova(struct ap_matrix_mdev *matrix_mdev, u64 iova,
> > u64 length)
> > +{
> > + struct ap_queue_table *qtable = &matrix_mdev->qtable;
> > + u64 iova_pfn_end = (iova + length - 1) >> PAGE_SHIFT;
> > + u64 iova_pfn_start = iova >> PAGE_SHIFT;
> > + struct vfio_ap_queue *q;
> > + int loop_cursor;
> > + u64 pfn;
> > +
> > + hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) {
> > + pfn = q->saved_iova >> PAGE_SHIFT;
> > + if (pfn >= iova_pfn_start && pfn <= iova_pfn_end) {
> > + vfio_ap_irq_disable(q);
> > + break;
>
> does this need a WARN_ON if the length is more than one page?
The iova and length are the range being invalidated, the driver has no
control over them and length is probably multiple pages.
But this test doesn't look right?
if (iova > q->saved_iova && q->saved_iova < iova + length)
Since the page was pinned we can assume iova and length are already
PAGE_SIZE aligned.
Jason
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable
2022-11-24 12:59 ` Jason Gunthorpe
@ 2022-11-28 6:31 ` Tian, Kevin
2022-11-28 15:32 ` Matthew Rosato
2022-11-28 15:40 ` Matthew Rosato
1 sibling, 1 reply; 22+ messages in thread
From: Tian, Kevin @ 2022-11-28 6:31 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Liu, Yi L, alex.williamson, chao.p.peng, kvm, yi.y.sun,
Matthew Rosato, Tony Krowiak, Halil Pasic, Jason Herne
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, November 24, 2022 8:59 PM
>
> On Thu, Nov 24, 2022 at 07:08:06AM +0000, Tian, Kevin wrote:
> > > From: Liu, Yi L <yi.l.liu@intel.com>
> > > Sent: Wednesday, November 23, 2022 9:49 PM
> > > +static void unmap_iova(struct ap_matrix_mdev *matrix_mdev, u64 iova,
> > > u64 length)
> > > +{
> > > + struct ap_queue_table *qtable = &matrix_mdev->qtable;
> > > + u64 iova_pfn_end = (iova + length - 1) >> PAGE_SHIFT;
> > > + u64 iova_pfn_start = iova >> PAGE_SHIFT;
> > > + struct vfio_ap_queue *q;
> > > + int loop_cursor;
> > > + u64 pfn;
> > > +
> > > + hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) {
> > > + pfn = q->saved_iova >> PAGE_SHIFT;
> > > + if (pfn >= iova_pfn_start && pfn <= iova_pfn_end) {
> > > + vfio_ap_irq_disable(q);
> > > + break;
> >
> > does this need a WARN_ON if the length is more than one page?
>
> The iova and length are the range being invalidated, the driver has no
> control over them and length is probably multiple pages.
Yes. I'm misled by the 'break'. Presumably all queues covered by
the unmapped range should have interrupt disabled while above only
disables interrupt for the first covered queue.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable
2022-11-28 6:31 ` Tian, Kevin
@ 2022-11-28 15:32 ` Matthew Rosato
0 siblings, 0 replies; 22+ messages in thread
From: Matthew Rosato @ 2022-11-28 15:32 UTC (permalink / raw)
To: Tian, Kevin, Jason Gunthorpe
Cc: Liu, Yi L, alex.williamson, chao.p.peng, kvm, yi.y.sun,
Tony Krowiak, Halil Pasic, Jason Herne
On 11/28/22 1:31 AM, Tian, Kevin wrote:
>> From: Jason Gunthorpe <jgg@nvidia.com>
>> Sent: Thursday, November 24, 2022 8:59 PM
>>
>> On Thu, Nov 24, 2022 at 07:08:06AM +0000, Tian, Kevin wrote:
>>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>>> Sent: Wednesday, November 23, 2022 9:49 PM
>>>> +static void unmap_iova(struct ap_matrix_mdev *matrix_mdev, u64 iova,
>>>> u64 length)
>>>> +{
>>>> + struct ap_queue_table *qtable = &matrix_mdev->qtable;
>>>> + u64 iova_pfn_end = (iova + length - 1) >> PAGE_SHIFT;
>>>> + u64 iova_pfn_start = iova >> PAGE_SHIFT;
>>>> + struct vfio_ap_queue *q;
>>>> + int loop_cursor;
>>>> + u64 pfn;
>>>> +
>>>> + hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) {
>>>> + pfn = q->saved_iova >> PAGE_SHIFT;
>>>> + if (pfn >= iova_pfn_start && pfn <= iova_pfn_end) {
>>>> + vfio_ap_irq_disable(q);
>>>> + break;
>>>
>>> does this need a WARN_ON if the length is more than one page?
>>
>> The iova and length are the range being invalidated, the driver has no
>> control over them and length is probably multiple pages.
>
> Yes. I'm misled by the 'break'. Presumably all queues covered by
> the unmapped range should have interrupt disabled while above only
> disables interrupt for the first covered queue.
Oops, yeah the break shouldn't be there; we want to disable any queue in the table that falls within the iova range.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable
2022-11-24 12:59 ` Jason Gunthorpe
2022-11-28 6:31 ` Tian, Kevin
@ 2022-11-28 15:40 ` Matthew Rosato
2022-11-28 20:28 ` Jason Gunthorpe
1 sibling, 1 reply; 22+ messages in thread
From: Matthew Rosato @ 2022-11-28 15:40 UTC (permalink / raw)
To: Jason Gunthorpe, Tian, Kevin
Cc: Liu, Yi L, alex.williamson, chao.p.peng, kvm, yi.y.sun,
Tony Krowiak, Halil Pasic, Jason Herne
On 11/24/22 7:59 AM, Jason Gunthorpe wrote:
> On Thu, Nov 24, 2022 at 07:08:06AM +0000, Tian, Kevin wrote:
>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>> Sent: Wednesday, November 23, 2022 9:49 PM
>>>
>>> From: Matthew Rosato <mjrosato@linux.ibm.com>
>>>
>>> vfio_iommufd_bind() creates an access which has an unmap callback, which
>>> can be called immediately. So dma_unmap() callback should tolerate the
>>> unmaps that come before the emulated device is opened.
>>>
>>> To achieve above, vfio_ap_mdev_dma_unmap() needs to validate that
>>> unmap
>>> request matches with one or more of these stashed values before
>>> attempting unpins.
>>>
>>> Currently, each mapped iova is stashed in its associated vfio_ap_queue;
>>> Each stashed iova represents IRQ that was enabled for a queue. Therefore,
>>> if a match is found, trigger IRQ disable for this queue to ensure that
>>> underlying firmware will no longer try to use the associated pfn after
>>> the page is unpinned. IRQ disable will also handle the associated unpin.
>>>
>>> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
>>> Cc: Halil Pasic <pasic@linux.ibm.com>
>>> Cc: Jason Herne <jjherne@linux.ibm.com>
>>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
>>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>>> ---
>>> drivers/s390/crypto/vfio_ap_ops.c | 24 +++++++++++++++++++++++-
>>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
>>> b/drivers/s390/crypto/vfio_ap_ops.c
>>> index bb7776d20792..62bfca2bbe6d 100644
>>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>>> @@ -1535,13 +1535,35 @@ static int vfio_ap_mdev_set_kvm(struct
>>> ap_matrix_mdev *matrix_mdev,
>>> return 0;
>>> }
>>>
>>> +static void unmap_iova(struct ap_matrix_mdev *matrix_mdev, u64 iova,
>>> u64 length)
>>> +{
>>> + struct ap_queue_table *qtable = &matrix_mdev->qtable;
>>> + u64 iova_pfn_end = (iova + length - 1) >> PAGE_SHIFT;
>>> + u64 iova_pfn_start = iova >> PAGE_SHIFT;
>>> + struct vfio_ap_queue *q;
>>> + int loop_cursor;
>>> + u64 pfn;
>>> +
>>> + hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) {
>>> + pfn = q->saved_iova >> PAGE_SHIFT;
>>> + if (pfn >= iova_pfn_start && pfn <= iova_pfn_end) {
>>> + vfio_ap_irq_disable(q);
>>> + break;
>>
>> does this need a WARN_ON if the length is more than one page?
>
> The iova and length are the range being invalidated, the driver has no
> control over them and length is probably multiple pages.
>
> But this test doesn't look right?
>
> if (iova > q->saved_iova && q->saved_iova < iova + length)>
> Since the page was pinned we can assume iova and length are already
> PAGE_SIZE aligned.
Yeah, I think that would be fine with a minor tweak to pick up q->saved_iova at the very start of the iova range:
if (iova >= q->saved_iova && q->saved_iova < iova + length)
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable
2022-11-28 15:40 ` Matthew Rosato
@ 2022-11-28 20:28 ` Jason Gunthorpe
2022-11-29 2:43 ` Tian, Kevin
2022-11-29 9:42 ` Yi Liu
0 siblings, 2 replies; 22+ messages in thread
From: Jason Gunthorpe @ 2022-11-28 20:28 UTC (permalink / raw)
To: Matthew Rosato
Cc: Tian, Kevin, Liu, Yi L, alex.williamson, chao.p.peng, kvm,
yi.y.sun, Tony Krowiak, Halil Pasic, Jason Herne
On Mon, Nov 28, 2022 at 10:40:48AM -0500, Matthew Rosato wrote:
> On 11/24/22 7:59 AM, Jason Gunthorpe wrote:
> > The iova and length are the range being invalidated, the driver has no
> > control over them and length is probably multiple pages.
> >
> > But this test doesn't look right?
> >
> > if (iova > q->saved_iova && q->saved_iova < iova + length)>
> > Since the page was pinned we can assume iova and length are already
> > PAGE_SIZE aligned.
>
> Yeah, I think that would be fine with a minor tweak to pick up q->saved_iova at the very start of the iova range:
>
> if (iova >= q->saved_iova && q->saved_iova < iova + length)
>
Yi can you update and repost this please?
I don't know if we will get a rc8, but we must be prepared with a
final branch by Friday in case not.
Thanks,
Jason
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable
2022-11-28 20:28 ` Jason Gunthorpe
@ 2022-11-29 2:43 ` Tian, Kevin
2022-11-29 9:42 ` Yi Liu
1 sibling, 0 replies; 22+ messages in thread
From: Tian, Kevin @ 2022-11-29 2:43 UTC (permalink / raw)
To: Jason Gunthorpe, Matthew Rosato
Cc: Liu, Yi L, alex.williamson, chao.p.peng, kvm, yi.y.sun,
Tony Krowiak, Halil Pasic, Jason Herne
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Tuesday, November 29, 2022 4:28 AM
>
> On Mon, Nov 28, 2022 at 10:40:48AM -0500, Matthew Rosato wrote:
> > On 11/24/22 7:59 AM, Jason Gunthorpe wrote:
> > > The iova and length are the range being invalidated, the driver has no
> > > control over them and length is probably multiple pages.
> > >
> > > But this test doesn't look right?
> > >
> > > if (iova > q->saved_iova && q->saved_iova < iova + length)>
> > > Since the page was pinned we can assume iova and length are already
> > > PAGE_SIZE aligned.
> >
> > Yeah, I think that would be fine with a minor tweak to pick up q-
> >saved_iova at the very start of the iova range:
> >
> > if (iova >= q->saved_iova && q->saved_iova < iova + length)
> >
>
> Yi can you update and repost this please?
>
> I don't know if we will get a rc8, but we must be prepared with a
> final branch by Friday in case not.
>
Just in case, Linus already said:
--
As a result, I'm now pretty sure that this is going to be one of those
"we'll have an extra week and I'll make an rc8" releases.
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable
2022-11-28 20:28 ` Jason Gunthorpe
2022-11-29 2:43 ` Tian, Kevin
@ 2022-11-29 9:42 ` Yi Liu
1 sibling, 0 replies; 22+ messages in thread
From: Yi Liu @ 2022-11-29 9:42 UTC (permalink / raw)
To: Jason Gunthorpe, Matthew Rosato
Cc: Tian, Kevin, alex.williamson, chao.p.peng, kvm, yi.y.sun,
Tony Krowiak, Halil Pasic, Jason Herne
On 2022/11/29 04:28, Jason Gunthorpe wrote:
> On Mon, Nov 28, 2022 at 10:40:48AM -0500, Matthew Rosato wrote:
>> On 11/24/22 7:59 AM, Jason Gunthorpe wrote:
>>> The iova and length are the range being invalidated, the driver has no
>>> control over them and length is probably multiple pages.
>>>
>>> But this test doesn't look right?
>>>
>>> if (iova > q->saved_iova && q->saved_iova < iova + length)>
>>> Since the page was pinned we can assume iova and length are already
>>> PAGE_SIZE aligned.
>>
>> Yeah, I think that would be fine with a minor tweak to pick up q->saved_iova at the very start of the iova range:
>>
>> if (iova >= q->saved_iova && q->saved_iova < iova + length)
>>
>
> Yi can you update and repost this please?
>
> I don't know if we will get a rc8, but we must be prepared with a
> final branch by Friday in case not.
done.
https://lore.kernel.org/kvm/20221129093535.359357-1-yi.l.liu@intel.com/T/#m261c22a7d9c9b2fe4258c5156f53ea6da97a27c9
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [iommufd 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open
2022-11-23 13:48 [iommufd 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open Yi Liu
2022-11-23 13:48 ` [iommufd 1/2] i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into init Yi Liu
2022-11-23 13:48 ` [iommufd 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable Yi Liu
@ 2022-11-24 6:50 ` Tian, Kevin
2022-11-24 9:44 ` Yi Liu
2022-11-25 6:06 ` Zhenyu Wang
3 siblings, 1 reply; 22+ messages in thread
From: Tian, Kevin @ 2022-11-24 6:50 UTC (permalink / raw)
To: Liu, Yi L, jgg
Cc: alex.williamson, chao.p.peng, kvm, yi.y.sun, Tony Krowiak,
Halil Pasic, Jason Herne, Zhenyu Wang, Wang, Zhi A
> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Wednesday, November 23, 2022 9:49 PM
>
> Jason's "Connect VFIO to IOMMUFD" introduces vfio iommufd compat mode.
> Under
> this mode, vfio_iommufd_bind() creates an access which has an unmap
> callback,
> which can be called immediately. This means mdev drivers may receive
> unmap
> requests before the mdev is opened. For now, most dma_unmap() callbacks
> are
> tolerant with such unmap requests, except for gvt-g and vfio-ap. This series
> tries to enhance the two drivers.
>
there are only three drivers (gvt, vfio-ap and vfio-ccw) providing
dma_unmap().
I'd not call the situation where only one driver is OK as "most
dma_unmap() callbacks are tolerant".
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [iommufd 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open
2022-11-24 6:50 ` [iommufd 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open Tian, Kevin
@ 2022-11-24 9:44 ` Yi Liu
0 siblings, 0 replies; 22+ messages in thread
From: Yi Liu @ 2022-11-24 9:44 UTC (permalink / raw)
To: Tian, Kevin, jgg
Cc: alex.williamson, chao.p.peng, kvm, yi.y.sun, Tony Krowiak,
Halil Pasic, Jason Herne, Zhenyu Wang, Wang, Zhi A
On 2022/11/24 14:50, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Wednesday, November 23, 2022 9:49 PM
>>
>> Jason's "Connect VFIO to IOMMUFD" introduces vfio iommufd compat mode.
>> Under
>> this mode, vfio_iommufd_bind() creates an access which has an unmap
>> callback,
>> which can be called immediately. This means mdev drivers may receive
>> unmap
>> requests before the mdev is opened. For now, most dma_unmap() callbacks
>> are
>> tolerant with such unmap requests, except for gvt-g and vfio-ap. This series
>> tries to enhance the two drivers.
>>
>
> there are only three drivers (gvt, vfio-ap and vfio-ccw) providing
> dma_unmap().
>
> I'd not call the situation where only one driver is OK as "most
> dma_unmap() callbacks are tolerant".
oh, yes. will refine it.
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [iommufd 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open
2022-11-23 13:48 [iommufd 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open Yi Liu
` (2 preceding siblings ...)
2022-11-24 6:50 ` [iommufd 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open Tian, Kevin
@ 2022-11-25 6:06 ` Zhenyu Wang
2022-11-25 8:58 ` Yi Liu
3 siblings, 1 reply; 22+ messages in thread
From: Zhenyu Wang @ 2022-11-25 6:06 UTC (permalink / raw)
To: Yi Liu
Cc: jgg, alex.williamson, kevin.tian, chao.p.peng, kvm, yi.y.sun,
Tony Krowiak, Halil Pasic, Jason Herne, Zhenyu Wang, Zhi Wang
[-- Attachment #1: Type: text/plain, Size: 1549 bytes --]
On 2022.11.23 05:48:30 -0800, Yi Liu wrote:
> Jason's "Connect VFIO to IOMMUFD" introduces vfio iommufd compat mode. Under
> this mode, vfio_iommufd_bind() creates an access which has an unmap callback,
> which can be called immediately. This means mdev drivers may receive unmap
> requests before the mdev is opened. For now, most dma_unmap() callbacks are
> tolerant with such unmap requests, except for gvt-g and vfio-ap. This series
> tries to enhance the two drivers.
>
> This series is based on Jason's below branch.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/log/?h=for-next
> (commit: 57f62422b6f0477afaddd2fc77a4bb9b94275f42)
>
> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Jason Herne <jjherne@linux.ibm.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
>
> Regards,
> Yi Liu
>
> Matthew Rosato (1):
> vfio/ap: validate iova during dma_unmap and trigger irq disable
>
> Yi Liu (1):
> i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into
> init
btw, for gvt change, pls at least add intel-gvt-dev@lists.freedesktop.org in cc.
thanks
>
> drivers/gpu/drm/i915/gvt/gvt.h | 2 ++
> drivers/gpu/drm/i915/gvt/kvmgt.c | 7 ++-----
> drivers/gpu/drm/i915/gvt/vgpu.c | 2 ++
> drivers/s390/crypto/vfio_ap_ops.c | 24 +++++++++++++++++++++++-
> 4 files changed, 29 insertions(+), 6 deletions(-)
>
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [iommufd 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open
2022-11-25 6:06 ` Zhenyu Wang
@ 2022-11-25 8:58 ` Yi Liu
0 siblings, 0 replies; 22+ messages in thread
From: Yi Liu @ 2022-11-25 8:58 UTC (permalink / raw)
To: Zhenyu Wang
Cc: jgg, alex.williamson, kevin.tian, chao.p.peng, kvm, yi.y.sun,
Tony Krowiak, Halil Pasic, Jason Herne, Zhi Wang
On 2022/11/25 14:06, Zhenyu Wang wrote:
> On 2022.11.23 05:48:30 -0800, Yi Liu wrote:
>> Jason's "Connect VFIO to IOMMUFD" introduces vfio iommufd compat mode. Under
>> this mode, vfio_iommufd_bind() creates an access which has an unmap callback,
>> which can be called immediately. This means mdev drivers may receive unmap
>> requests before the mdev is opened. For now, most dma_unmap() callbacks are
>> tolerant with such unmap requests, except for gvt-g and vfio-ap. This series
>> tries to enhance the two drivers.
>>
>> This series is based on Jason's below branch.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/log/?h=for-next
>> (commit: 57f62422b6f0477afaddd2fc77a4bb9b94275f42)
>>
>> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
>> Cc: Halil Pasic <pasic@linux.ibm.com>
>> Cc: Jason Herne <jjherne@linux.ibm.com>
>> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
>> Cc: Zhi Wang <zhi.a.wang@intel.com>
>> Cc: Kevin Tian <kevin.tian@intel.com>
>>
>> Regards,
>> Yi Liu
>>
>> Matthew Rosato (1):
>> vfio/ap: validate iova during dma_unmap and trigger irq disable
>>
>> Yi Liu (1):
>> i915/gvt: Move kvmgt_protect_table_init() and gvt_cache_init() into
>> init
>
> btw, for gvt change, pls at least add intel-gvt-dev@lists.freedesktop.org in cc.
sure. :-)
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 22+ messages in thread