Hi,
On 1/21/25 9:31 AM, Laurent Vivier wrote:
> On 20/01/2025 18:33, Eric Auger wrote:
>> When a guest exposed with a vhost device and protected by an
>> intel IOMMU gets rebooted, we sometimes observe a spurious warning:
>>
>> Fail to lookup the translated address ffffe000
>>
>> We observe that the IOMMU gets disabled through a write to the global
>> command register (CMAR_GCMD.TE) before the vhost device gets stopped.
>> When this warning happens it can be observed an inflight IOTLB
>> miss occurs after the IOMMU disable and before the vhost stop. In
>> that case a flat translation occurs and the check in
>> vhost_memory_region_lookup() fails.
>>
>> Let's disable the IOTLB callbacks when all IOMMU MRs have been
>> unregistered.
>>
>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>> ---
>> hw/virtio/vhost.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 6aa72fd434..128c2ab094 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -931,6 +931,10 @@ static void
>> vhost_iommu_region_del(MemoryListener *listener,
>> break;
>> }
>> }
>> + if (QLIST_EMPTY(&dev->iommu_list) &&
>> + dev->vhost_ops->vhost_set_iotlb_callback) {
>> + dev->vhost_ops->vhost_set_iotlb_callback(dev, false);
>> + }
>> }
>> void vhost_toggle_device_iotlb(VirtIODevice *vdev)
>
> I think you need the counterpart in vhost_iommu_region_del() (for
> instance if we have an add after a del that results in an empty list).
> But you cannot unconditionally enable it (for instance if vhost is not
> started)
if we enter vhost_iommu_region_add(), this means that the iommu_listener
has been registered in vhost_vdev_start() and thus vdev->vhost_started
is set.
>
> Perhaps you should move the vhost_set_iotlb_callback() call from
> vhost_start()/vhost_stop() to
> vhost_iommu_region_add()/vhost_iommu_region_del()?
I currently fail to understand whether we shouldn't keep listening to
iotlb callbacks when the IOMMU gets disabled. In that case shouldn't we
flush the kernel IOTLB and update the hdev->mem->regions to reflect the
IOMMR MR disablement?
Thanks
Eric
>
> Thanks,
> Laurent
>
