Re: [PATCH v5 00/18] SMMUv3 nested translation support

[Peter Maydell]

On Sat, 20 Jul 2024 at 23:11, Mostafa Saleh <smostafa@google.com> wrote:
>
> Hi Peter,
>
> On Fri, Jul 19, 2024 at 04:57:18PM +0100, Peter Maydell wrote:
> > On Fri, 19 Jul 2024 at 16:36, Julien Grall <julien@xen.org> wrote:
> > >
> > > Hi,
> > >
> > > On 18/07/2024 10:43, Julien Grall wrote:
> > > > Hi Eric,
> > > >
> > > > On 17/07/2024 18:43, Eric Auger wrote:
> > > >> Hi Peter, Richard,
> > > >>
> > > >> On 7/17/24 17:09, Jean-Philippe Brucker wrote:
> > > >>> On Mon, Jul 15, 2024 at 08:45:00AM +0000, Mostafa Saleh wrote:
> > > >>>> Currently, QEMU supports emulating either stage-1 or stage-2 SMMUs
> > > >>>> but not nested instances.
> > > >>>> This patch series adds support for nested translation in SMMUv3,
> > > >>>> this is controlled by property “arm-smmuv3.stage=nested”, and
> > > >>>> advertised to guests as (IDR0.S1P == 1 && IDR0.S2P == 2)
> > > >>> For the whole series (3-9, 11, 12, 15, 16, 18):
> > > >>>
> > > >>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > >>>
> > > >>> (and I think patch 16 is missing Eric's R-b)
> > > >>
> > > >> Jean-Philippe and I have followed up the progress of this series,
> > > >> Mostafa took into account all our comments and all the patches were
> > > >> reviewed. It seems to be in a pretty decent state so if you don't have
> > > >> any objection, please consider pulling it for 9.1.
> > > >>
> > > >> On my end I did some testing in non nesting mode with virtio-net/vhost
> > > >> and I have not noticed any regression.
> > > >> Would be nice if someone could send his T-b for the nested part though
> > > >> (Julien?).
> > > >
> > > > I haven't yet tried the latest version. I will do that in the next
> > > > couple of days.
> > > I see this is already merged. If this still matters:
> > >
> > > Tested-by: Julien Grall <jgrall@amazon.com>
> >
> > We can't retrospectively add the tag, but the testing itself
> > is still important -- thanks for doing it.
> >
> > Q: is there any reason not to:
> >  (a) change the default to "nested" rather than "1"
> >  (b) make the virt board (for new virt machine versions) use
> >      "nested"?
> >
> > AIUI "nested" should be a superset of "stage-1 only", the guest
> > can just ignore stage-2 if it doesn't care about it. Or is
> > there a performance hit from having stage-2 around even if the
> > guest doesn't enable it?
>
> I didn’t do benchmarks, but from the code, I don’t think there
> would be a difference from using stage-1 only or nested stages
> with stage-1 config.
> I didn’t make “nested” the default stage or used it for the virt
> board, as I was worried about compatibility issues (I think that
> breaks backward migration), but otherwise I don’t see issues.
>
> But if I understand correctly, setting that for virt board 9.1
> (virt_machine_9_1_options) would be fine?

Yes, we would need to retain the old stage-1-only config on
the older machine version types, but we could switch to
nested by default on the newer ones. It's a little late
I think to make that change for the virt board at this
point in the 9.1 release cycle, but we could do it for 9.2.

thanks
-- PMM