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

[Peter Maydell]

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
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.)

thanks
-- PMM