Hi Zhenzhong,
On 6/4/25 7:50 AM, Duan, Zhenzhong wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <eric.au...@redhat.com>
>> Sent: Tuesday, June 3, 2025 8:21 PM
>> To: Duan, Zhenzhong <zhenzhong.d...@intel.com>; qemu-devel@nongnu.org
>> Cc: alex.william...@redhat.com; c...@redhat.com; m...@redhat.com;
>> jasow...@redhat.com; pet...@redhat.com; ddut...@redhat.com;
>> j...@nvidia.com; nicol...@nvidia.com; shameerali.kolothum.th...@huawei.com;
>> joao.m.mart...@oracle.com; clement.mathieu--d...@eviden.com; Tian, Kevin
>> <kevin.t...@intel.com>; Liu, Yi L <yi.l....@intel.com>; Peng, Chao P
>> <chao.p.p...@intel.com>
>> Subject: Re: [PATCH v2 1/4] backends/iommufd: Add a helper to invalidate
>> user-
>> managed HWPT
>>
>> Hi Zhenzhong,
>>
>> On 5/30/25 11:35 AM, Zhenzhong Duan wrote:
>>> This helper passes cache invalidation request from guest to invalidate
>>> stage-1 page table cache in host hardware.
>>>
>>> Signed-off-by: Nicolin Chen <nicol...@nvidia.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>>> ---
>>> include/system/iommufd.h | 4 ++++
>>> backends/iommufd.c | 36 ++++++++++++++++++++++++++++++++++++
>>> backends/trace-events | 1 +
>>> 3 files changed, 41 insertions(+)
>>>
>>> diff --git a/include/system/iommufd.h b/include/system/iommufd.h
>>> index cbab75bfbf..83ab8e1e4c 100644
>>> --- a/include/system/iommufd.h
>>> +++ b/include/system/iommufd.h
>>> @@ -61,6 +61,10 @@ bool
>> iommufd_backend_get_dirty_bitmap(IOMMUFDBackend *be, uint32_t hwpt_id,
>>> uint64_t iova, ram_addr_t size,
>>> uint64_t page_size, uint64_t *data,
>>> Error **errp);
>>> +bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
>>> + uint32_t data_type, uint32_t
>>> entry_len,
>>> + uint32_t *entry_num, void *data,
>>> + Error **errp);
>>>
>>> #define TYPE_HOST_IOMMU_DEVICE_IOMMUFD
>> TYPE_HOST_IOMMU_DEVICE "-iommufd"
>>> #endif
>>> diff --git a/backends/iommufd.c b/backends/iommufd.c
>>> index b73f75cd0b..8bcdb60fe7 100644
>>> --- a/backends/iommufd.c
>>> +++ b/backends/iommufd.c
>>> @@ -311,6 +311,42 @@ bool
>> iommufd_backend_get_device_info(IOMMUFDBackend *be, uint32_t devid,
>>> return true;
>>> }
>>>
>>> +bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
>>> + uint32_t data_type, uint32_t
>>> entry_len,
>>> + uint32_t *entry_num, void *data,
>>> + Error **errp)
>>> +{
>>> + int ret, fd = be->fd;
>>> + uint32_t total_entries = *entry_num;
>>> + struct iommu_hwpt_invalidate cache = {
>>> + .size = sizeof(cache),
>>> + .hwpt_id = id,
>>> + .data_type = data_type,
>>> + .entry_len = entry_len,
>>> + .entry_num = total_entries,
>>> + .data_uptr = (uintptr_t)data,
>>> + };
>>> +
>>> + ret = ioctl(fd, IOMMU_HWPT_INVALIDATE, &cache);
>>> + trace_iommufd_backend_invalidate_cache(fd, id, data_type, entry_len,
>>> + total_entries, cache.entry_num,
>>> + (uintptr_t)data, ret ? errno :
>>> 0);
>>> + *entry_num = cache.entry_num;
>>> +
>>> + if (ret) {
>>> + error_setg_errno(errp, errno, "IOMMU_HWPT_INVALIDATE failed:"
>>> + " total %d entries, processed %d entries",
>>> + total_entries, cache.entry_num);
>>> + } else if (total_entries != cache.entry_num) {
>>> + error_setg(errp, "IOMMU_HWPT_INVALIDATE succeed but with
>> unprocessed"
>>> + " entries: total %d entries, processed %d
>>> entries."
>>> + " Kernel BUG?!", total_entries, cache.entry_num);
>> Can this happen? Isn't it a failure case?
> It shouldn't happen except kernel has a bug. It's a failure case, so false is
> returned.
OK. I missed it was an error case.
Eric
>
> Thanks
> Zhenzhong
>
>> Besides
>> Reviewed-by: Eric Auger <eric.au...@redhat.com>
>>
>>
>> Eric
>>> + return false;
>>> + }
>>> +
>>> + return !ret;
>>> +}
>>> +
>>> static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error
>> **errp)
>>> {
>>> HostIOMMUDeviceCaps *caps = &hiod->caps;
>>> diff --git a/backends/trace-events b/backends/trace-events
>>> index 40811a3162..7278214ea5 100644
>>> --- a/backends/trace-events
>>> +++ b/backends/trace-events
>>> @@ -18,3 +18,4 @@ iommufd_backend_alloc_hwpt(int iommufd, uint32_t
>> dev_id, uint32_t pt_id, uint32_
>>> iommufd_backend_free_id(int iommufd, uint32_t id, int ret) " iommufd=%d
>> id=%d (%d)"
>>> iommufd_backend_set_dirty(int iommufd, uint32_t hwpt_id, bool start, int
>>> ret)
>> " iommufd=%d hwpt=%u enable=%d (%d)"
>>> iommufd_backend_get_dirty_bitmap(int iommufd, uint32_t hwpt_id, uint64_t
>> iova, uint64_t size, uint64_t page_size, int ret) " iommufd=%d hwpt=%u
>> iova=0x%"PRIx64" size=0x%"PRIx64" page_size=0x%"PRIx64" (%d)"
>>> +iommufd_backend_invalidate_cache(int iommufd, uint32_t id, uint32_t
>> data_type, uint32_t entry_len, uint32_t entry_num, uint32_t done_num,
>> uint64_t
>> data_ptr, int ret) " iommufd=%d id=%u data_type=%u entry_len=%u
>> entry_num=%u done_num=%u data_ptr=0x%"PRIx64" (%d)"
