Re: [PATCH 1/3] virtio_iommu: Clear IOMMUPciBus pointer cache when system reset

[Eric Auger]

Hi Zhenzhong,

On 1/23/24 11:03, Duan, Zhenzhong wrote:
>
>> -----Original Message-----
>> From: Cédric Le Goater <clg@redhat.com>
>> Subject: Re: [PATCH 1/3] virtio_iommu: Clear IOMMUPciBus pointer cache
>> when system reset
>>
>> On 1/22/24 07:40, Zhenzhong Duan wrote:
>>> IOMMUPciBus pointer cache is indexed by bus number, bus number
>>> may not always be a fixed value, i.e., guest reboot to different
>>> kernel which set bus number with different algorithm.
this cannot harm to reset it but I don't know if this can be encountered.
>>>
>>> This could lead to endpoint binding to wrong iommu MR in
>>> virtio_iommu_get_endpoint(), then vfio device setup wrong
>>> mapping from other device.
>>>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>   hw/virtio/virtio-iommu.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>>> index 8a4bd933c6..bfce3237f3 100644
>>> --- a/hw/virtio/virtio-iommu.c
>>> +++ b/hw/virtio/virtio-iommu.c
>>> @@ -1264,6 +1264,8 @@ static void virtio_iommu_system_reset(void
>> *opaque)
>>>       trace_virtio_iommu_system_reset();
>>>
>>> +    memset(s->iommu_pcibus_by_bus_num, 0, sizeof(s-
>>> iommu_pcibus_by_bus_num));
>>> +
>>>       /*
>>>        * config.bypass is sticky across device reset, but should be 
>>> restored on
>>>        * system reset
>> you could remove the memset in virtio_iommu_device_realize() then ?
> Good suggestion, will do.
By the way what about the vtd implementation. s->vtd_address_spaces is
hash table but I don't see it reset either?
Also if is requested here we would need it on smmuv3 as well.

Thanks

Eric
>
> Thanks
> Zhenzhong