Hi Stefano,
On 1/21/25 9:45 AM, Stefano Garzarella wrote:
> On Tue, Jan 21, 2025 at 09:31:53AM +0100, 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
>
> I guess you meant vhost_iommu_region_add(). I was going to comment
> exactly on that, I agree here.
>
>> 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)
>
> Good point.
>
>>
>> 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 also like this idea.
OK makes sense. I will go in this direction.
Eric
>
> Stefano
>
