Re: [RFC PATCH 00/12] SMMUv3 nested translation support

[Nicolin Chen]

Hi Mostafa,

On Mon, Mar 25, 2024 at 10:13:56AM +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)

IIUIC, with this series, vSMMU will support a virtualized 2-stage
translation in a guest VM, right? I wonder how it would interact
with the ongoing 2-stage nesting support with host and guest. Or
is it supposed to be just a total orthogonal feature without any
interaction with the host system?

Thanks
Nicolin

> Main changes(architecture):
> ============================
> 1) CDs are considered IPA and translated with stage-2.
> 2) TTBx and tables for stage-1 are considered IPA and translated
>    with stage-2.
> 3) Translate the IPA address with stage-2.
> 
> TLBs:
> ======
> TLBs are the most tricky part.
> 
> 1) General design
>    Unified(Combined) design is used, where a new tag is added "stage"
>    which has 2 valid values:
>    - STAGE_1: Meaning this entry translates VA to PADDR, it can be
>      cached from fully nested configuration or from stage-1 only.
>      It doesn't support separate cached entries (VA to IPA).
> 
>    - STAGE_2: Meaning this translates IPA to PADDR, cached from
>      stage-2  only configuration.
> 
>    TLBs are also modified to cache 2 permissions, a new permission added
>    "parent_perm."
> 
>    For non-nested configuration, perm == parent_perm and nothing
>    changes. This is used to know which stage to use in case there is
>    a permission fault from a TLB entry.
> 
> 2) Caching in TLB
>    Stage-1 and stage-2 are inserted in the TLB as is.
>    For nested translation, both entries are combined into one TLB
>    entry. Everything is used from stage-1, except:
>    - transatled_addr from stage-2.
>    - parent_perm is from stage-2.
>    - addr_mask: is the minimum of both.
> 
> 3) TLB Lookup
>    For stage-1 and nested translations, it look for STAGE_1 entries.
>    For stage-2 it look for STAGE_2 TLB entries.
> 
> 4) TLB invalidation
>    - Stage-1 commands (CMD_TLBI_NH_VAA, SMMU_CMD_TLBI_NH_VA,
>      SMMU_CMD_TLBI_NH_ALL): Invalidate TLBs tagged with SMMU_STAGE_1.
>    - Stage-2 commands (CMD_TLBI_S2_IPA): Invalidate TLBs tagged with
>      SMMU_STAGE_2.
>    - All (SMMU_CMD_TLBI_S12_VMALL): Will invalidate both, this is
>      communicated to the TLB as SMMU_NESTED which is (SMMU_STAGE_1 |
>      SMMU_STAGE_2) which uses it as a mask.
> 
>    As far as I understand, this is compliant with the ARM
>    architecture, based on:
>    - ARM ARM DDI 0487J.a: RLGSCG, RTVTYQ, RGNJPZ
>    - ARM IHI 0070F.b: 16.2 Caching
> 
>    An alternative approach would be to instantiate 2 TLBs, one per
>    each stage. I haven’t investigated that.
> 
> Others
> =======
> - Advertise SMMUv3.2-S2FWB, it is NOP for QEMU as it doesn’t support
>   attributes.
> 
> - OAS: A typical setup with nesting is to share CPU stage-2 with the
>   SMMU, and according to the user manual, SMMU OAS must match the
>   system physical address.
> 
>   This was discussed before in
>   https://lore.kernel.org/all/20230226220650.1480786-11-smostafa@google.com/
>   The implementation here, follows the discussion, where migration is
>   added and oas is set up from the board (virt). However, the OAS is
>   chosen based on the CPU PARANGE as there is no fixed one.
> 
> - For nested configuration, IOVA notifier only notifies for stage-1
>   invalidations (as far as I understand this is the intended
>   behaviour as it notifies for IOVA)
> 
> - Stop ignoring VMID for stage-1 if stage-2 is also supported.
> 
> 
> Future improvements:
> =====================
> 1) One small improvement, that I don’t think it’s worth the extra
>    complexity, is in case of Stage-1 TLB miss for nested translation,
>    we can do stage-1 walk and lookup for stage-2 TLBs, instead of
>    doing the full walk.
> 
> 2) Patch 0006 (hw/arm/smmuv3: Translate CD and TT using stage-2 table)
>    introduces a macro to use functions that rely on cfg for stage-2,
>    I don’t like it. However, I didn’t find a simple way around it,
>    either we change many functions to have a separate stage argument,
>    or add another arg in config, which is probably more code.
> 
> Testing
> ========
> 1) IOMMUFD + VFIO
>    Kernel: 
> https://lore.kernel.org/all/cover.1683688960.git.nicolinc@nvidia.com/
>    VMM: 
> https://qemu-devel.nongnu.narkive.com/o815DqpI/rfc-v5-0-8-arm-smmuv3-emulation-support
> 
>    By assigning 
> “virtio-net-pci,netdev=net0,disable-legacy=on,iommu_platform=on,ats=on”,
>    to a guest VM (on top of QEMU guest) with VIFO and IOMMUFD.
> 
> 2) Work in progress prototype I am hacking on for nesting on KVM
>    (this is nowhere near complete, and misses many stuff but it
>    doesn't require VMs/VFIO) also with virtio-net-pci and git
>    cloning a bunch of stuff and also observing traces.
>    
> https://android-kvm.googlesource.com/linux/+log/refs/heads/smostafa/android15-6.6-smmu-nesting-wip
> 
> hw/arm/smmuv3: Split smmuv3_translate() better viewed with --color-moved
> 
> 
> Mostafa Saleh (12):
>   hw/arm/smmu: Use enum for SMMU stage
>   hw/arm/smmu: Split smmuv3_translate()
>   hw/arm/smmu: Add stage to TLB
>   hw/arm/smmu: Support nesting in commands
>   hw/arm/smmuv3: Support nested SMMUs in smmuv3_notify_iova()
>   hw/arm/smmuv3: Translate CD and TT using stage-2 table
>   hw/arm/smmu-common: Support nested translation
>   hw/arm/smmuv3: Support and advertise nesting
>   hw/arm/smmuv3: Advertise S2FWB
>   hw/arm/smmu: Refactor SMMU OAS
>   hw/arm/smmuv3: Add property for OAS
>   hw/arm/virt: Set SMMU OAS based on CPU PARANGE
> 
>  hw/arm/smmu-common.c         | 256 ++++++++++++++++++----
>  hw/arm/smmu-internal.h       |   2 +
>  hw/arm/smmuv3-internal.h     |  17 +-
>  hw/arm/smmuv3.c              | 405 ++++++++++++++++++++++-------------
>  hw/arm/trace-events          |  14 +-
>  hw/arm/virt.c                |  14 +-
>  include/hw/arm/smmu-common.h |  46 +++-
>  include/hw/arm/smmuv3.h      |   1 +
>  target/arm/cpu.h             |   2 +
>  target/arm/cpu64.c           |   5 +
>  10 files changed, 533 insertions(+), 229 deletions(-)
> 
> --
> 2.44.0.396.g6e790dbe36-goog
>