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

[Mostafa Saleh <smostafa@google.com>]

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?

Thanks,
Mostafa

> 
> thanks
> -- PMM