Re: [PATCH 4/5] hw/arm/smmuv3: Move reset to exit phase

[Eric Auger]

Hi Peter,

On 2/7/25 5:58 PM, Peter Maydell wrote:
> On Fri, 7 Feb 2025 at 16:50, Eric Auger <eric.auger@redhat.com> wrote:
>>
>>
>>
>> On 2/7/25 5:37 PM, Peter Maydell wrote:
>>> On Thu, 6 Feb 2025 at 14:23, Eric Auger <eric.auger@redhat.com> wrote:
>>>> Currently the iommu may be reset before the devices
>>>> it protects. For example this happens with virtio-scsi-pci.
>>>> when system_reset is issued from qmp monitor, spurious
>>>> "virtio: zero sized buffers are not allowed" warnings can
>>>> be observed.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>> ---
>>>>  hw/arm/smmuv3.c     | 9 +++++----
>>>>  hw/arm/trace-events | 1 +
>>>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>>> index c0cf5df0f6..7522c32b24 100644
>>>> --- a/hw/arm/smmuv3.c
>>>> +++ b/hw/arm/smmuv3.c
>>>> @@ -1870,13 +1870,14 @@ static void smmu_init_irq(SMMUv3State *s, 
>>>> SysBusDevice *dev)
>>>>      }
>>>>  }
>>>>
>>>> -static void smmu_reset_hold(Object *obj, ResetType type)
>>>> +static void smmu_reset_exit(Object *obj, ResetType type)
>>>>  {
>>>>      SMMUv3State *s = ARM_SMMUV3(obj);
>>>>      SMMUv3Class *c = ARM_SMMUV3_GET_CLASS(s);
>>>>
>>>> -    if (c->parent_phases.hold) {
>>>> -        c->parent_phases.hold(obj, type);
>>>> +    trace_smmu_reset_exit();
>>>> +    if (c->parent_phases.exit) {
>>>> +        c->parent_phases.exit(obj, type);
>>>>      }
>>> If we need to do something unexpected like reset
>>> register values in the exit phase rather than the
>>> hold phase, it's a good idea to have a comment explaining
>>> why, to avoid somebody coming along afterwards and tidying
>>> it up into the more usual arrangement.
>> sure
>>> If I understand correctly we need to keep the whole IOMMU
>>> config intact until the exit phase? What's the thing the
>>> device behind the IOMMU is trying to do during its reset
>>> that triggers the warning?
>> The virtio-pci-net continues to perform DMA requests and this causes
>> some weird messages such as:
>> "virtio: bogus descriptor or out of resources"
>>
>> Also VFIO devices may continue issuing DMAs causing translation faults
> Hmm, right. I guess this only happens with KVM, or can you
> trigger it in a TCG setup too? Anyway, presumably we can
I have only tested with KVM acceleration for now.
> rely on the devices quiescing all their outstanding DMA
> by the time the hold phase comes along.
>
> (I wonder if we ought to suggest quiescing outstanding
> DMA in the enter phase? But it's probably easier to fix
> the iommus like this series does than try to get every
> dma-capable pci device to do something different.)
at least we can document this, as Peter later suggests. Indeed fixing it
at vIOMMU level looked much easier for me, hoping that no other DMA
capable device would stop DMAs at exit phase

Eric
>
> thanks
> -- PMM
>