lkml.org 
[lkml]   [2023]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 2/4] vhost-vdpa: reset vendor specific mapping to initial state in .release
From


On 10/11/2023 4:21 AM, Eugenio Perez Martin wrote:
> On Tue, Oct 10, 2023 at 11:05 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>> Devices with on-chip IOMMU or vendor specific IOTLB implementation
>> may need to restore iotlb mapping to the initial or default state
>> using the .reset_map op, as it's desirable for some parent devices
>> to solely manipulate mappings by its own, independent of virtio device
>> state. For instance, device reset does not cause mapping go away on
>> such IOTLB model in need of persistent mapping. Before vhost-vdpa
>> is going away, give them a chance to reset iotlb back to the initial
>> state in vhost_vdpa_cleanup().
>>
>> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
>> ---
>> drivers/vhost/vdpa.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
>> index 851535f..a3f8160 100644
>> --- a/drivers/vhost/vdpa.c
>> +++ b/drivers/vhost/vdpa.c
>> @@ -131,6 +131,15 @@ static struct vhost_vdpa_as *vhost_vdpa_find_alloc_as(struct vhost_vdpa *v,
>> return vhost_vdpa_alloc_as(v, asid);
>> }
>>
>> +static void vhost_vdpa_reset_map(struct vhost_vdpa *v, u32 asid)
>> +{
>> + struct vdpa_device *vdpa = v->vdpa;
>> + const struct vdpa_config_ops *ops = vdpa->config;
>> +
>> + if (ops->reset_map)
>> + ops->reset_map(vdpa, asid);
>> +}
>> +
>> static int vhost_vdpa_remove_as(struct vhost_vdpa *v, u32 asid)
>> {
>> struct vhost_vdpa_as *as = asid_to_as(v, asid);
>> @@ -140,6 +149,13 @@ static int vhost_vdpa_remove_as(struct vhost_vdpa *v, u32 asid)
>>
>> hlist_del(&as->hash_link);
>> vhost_vdpa_iotlb_unmap(v, &as->iotlb, 0ULL, 0ULL - 1, asid);
> Now I'm wondering, does this call to vhost_vdpa_iotlb_unmap sets a
> different map (via .set_map) per element of the vhost_iotlb_itree?
Yes and no, effectively this vhost_vdpa_iotlb_unmap call will pass an
empty iotlb with zero map entry down to the driver via .set_map, so for
.set_map interface it's always a different map no matter what. As for
this special case, the internal implementation of mlx5_vdpa .set_map may
choose to either destroy MR and recreate a new one, or remove all
mappings on the existing MR (currently it uses destroy+recreate for
simplicity without have to special case). But .reset_map is different -
the 1:1 DMA MR has to be recreated explicitly after destroying the
regular MR, so you see this is driver/device implementation specifics.

> Not
> a big deal since we're in the cleanup path, but it could be a nice
> optimization on top as we're going to reset the map of the asid
> anyway.
You mean wrap up what's done in vhost_vdpa_iotlb_unmap and
vhost_vdpa_reset_map to a new call, say vhost_vdpa_iotlb_reset? Yes this
is possible, but be noted that the vhost_vdpa_iotlb_unmap also takes
charge of pinning accounting other than mapping, and it has to also
maintain it's own vhost_iotlb copy in sync. There's no such much code
that can be consolidated or generalized at this point, as
vhost_vdpa_reset_map() is very specific to some device implementation,
and I don't see common need to optimize this further up in the map/unmap
hot path rather than this cleanup slow path, just as you alluded to.

Regards,
-Siwei
>
>> + /*
>> + * Devices with vendor specific IOMMU may need to restore
>> + * iotlb to the initial or default state which is not done
>> + * through device reset, as the IOTLB mapping manipulation
>> + * could be decoupled from the virtio device life cycle.
>> + */
>> + vhost_vdpa_reset_map(v, asid);
>> kfree(as);
>>
>> return 0;
>> --
>> 1.8.3.1
>>

\
 
 \ /
  Last update: 2023-10-12 08:19    [W:0.142 / U:0.916 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site