public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Nicolin Chen <nicolinc@nvidia.com>,
	robin.murphy@arm.com, will@kernel.org, eric.auger@redhat.com,
	kevin.tian@intel.com, baolu.lu@linux.intel.com, joro@8bytes.org,
	shameerali.kolothum.thodi@huawei.com,
	linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
	linux-kernel@vger.kernel.org, yi.l.liu@intel.com
Subject: Re: [PATCH v1 02/14] iommufd: Add nesting related data structures for ARM SMMUv3
Date: Thu, 9 Mar 2023 18:26:59 +0000	[thread overview]
Message-ID: <20230309182659.GA1710571@myrica> (raw)
In-Reply-To: <ZAnx0lUkw02cVTi+@nvidia.com>

On Thu, Mar 09, 2023 at 10:48:50AM -0400, Jason Gunthorpe wrote:
> On Thu, Mar 09, 2023 at 01:42:17PM +0000, Jean-Philippe Brucker wrote:
> 
> > Although we can keep the alloc and hardware info separate for each IOMMU
> > architecture, we should try to come up with common invalidation methods.
> 
> The invalidation language is tightly linked to the actual cache block
> and cache tag in the IOMMU HW design.

Concretely though, what are the incompatibilities between the HW designs?
They all need to remove a range of TLB entries, using some address space
tag. But if there is an actual difference I do need to know.

> Generality will loose or
> obfuscate the necessary specificity that is required for creating real
> vIOMMUs.
> 
> Further, invalidation is a fast path, it is crazy to take a vIOMMU of
> a real HW receving a native invalidation request, mangle it to some
> obfuscated kernel version and then de-mangle it again in the kernel
> driver. IMHO ideally qemu will simply point the invalidation at the
> WQE in the SW vIOMMU command queue and invoke the ioctl. (Nicolin, we
> should check more into this)

Avoiding copying a few bytes won't make up for the extra context switches
to userspace. An emulated IOMMU can easily decode commands and translate
them to generic kernel structures, in a handful of CPU cycles, just like
they decode STEs. It's what they do, and it's the opposite of obfuscation.

> 
> The purpose of these interfaces is to support high performance full
> functionality vIOMMUs of the real HW, we should not loose sight of
> that goal.
> 
> We are actually planning to go futher and expose direct invalidation
> work queues complete with HW doorbells to userspace. This obviously
> must be in native HW format.

Doesn't seem relevant since direct access to command queue wouldn't use
this struct.

> 
> Nicolin, I think we should tweak the uAPI here so that the
> invalidation opaque data has a format tagged on its own, instead of
> re-using the HWPT tag. Ie you can have a ARM SMMUv3 invalidate type
> tag and also a virtio-viommu invalidate type tag.
> 
> This will allow Jean to put the invalidation decoding in the iommu
> drivers if we think that is the right direction. virtio can
> standardize it as a "HW format".
> 
> > Ideally I'd like something like this for vhost-iommu:
> > 
> > * slow path through userspace for complex requests like attach-table and
> >   probe, where the VMM can decode arch-specific information and translate
> >   them to iommufd and vhost-iommu ioctls to update the configuration.
> > 
> > * fast path within the kernel for performance-critical requests like
> >   invalidate, page request and response. It would be absurd for the
> >   vhost-iommu driver to translate generic invalidation requests from the
> >   guest into arch-specific commands with special opcodes, when the next
> >   step is calling the IOMMU driver which does that for free.
> 
> Someone has to do the conversion. If you don't think virito should do
> it then I'd be OK to add a type tag for virtio format invalidation and
> put it in the IOMMU driver.

Implementing two invalidation formats in each IOMMU driver does not seem
practical.

> 
> But given virtio overall already has to know *alot* about how the HW
> it is wrapping works I don't think it is necessarily absurd for virtio
> to do the conversion. I'd like to evaluate this in patches in context
> with how much other unique HW code ends up in kernel-side vhost-iommu.

Ideally none. I'd rather leave those, attach and probe, in userspace and
if possible compatible with iommufd to avoid register decoding. 

> 
> However, I don't know the rational for virtio-viommu, it seems like a
> strange direction to me.

A couple of reasons are relevant here: non-QEMU VMMs don't want to emulate
all vendor IOMMUs, new architectures get vIOMMU mostly for free, and vhost
provides a faster path. Also the ability to optimize paravirtual
interfaces for things like combined invalidation (IOTLB+ATC) or, later,
nested page requests.

For a while the main vIOMMU use-case was assignment to guest userspace,
mainly DPDK, which works great with a generic and slow map/unmap
interface. Since vSVA is still a niche use-case, and nesting without page
faults requires pinning the whole guest memory, map/unmap still seems more
desirable to me. But there is some renewed interest in supporting page
tables with virtio-iommu for the reasons above.

> All the iommu drivers have native command
> queues. ARM and AMD are both supporting native command queues directly
> in the guest, complete with a direct guest MMIO doorbell ring.

Arm SMMUv3 mandates a single global command queue (SMMUv2 uses registers).
An SMMUv3 can optionally implement multiple command queues, though I don't
know if they can be safely assigned to guests. For a lot of SMMUv3
implementations that have a single queue and for other architectures, we
can do better than hardware emulation.

> 
> If someone wants to optimize this I'd think the way to do it is to use
> virtio like techniques to put SW command queue processing in the
> kernel iommu driver and continue to use the HW native interface in the
> VM.

I didn't get which kernel this is.

> 
> What benifit comes from replacing the HW native interface with virtio?
> Especially on ARM where the native interface is pretty clean?
> 
> > During previous discussions we came up with generic invalidations that
> > could fit both Arm and x86 [1][2]. The only difference was the ASID
> > (called archid/id in those proposals) which VT-d didn't need. Could we try
> > to build on that?
> 
> IMHO this was just unioning all the different invalidation types
> together. It makes sense for something like virtio but it is
> illogical/obfuscated as a user/kernel interface since it still
> requires a userspace HW driver to understand what subset of the
> invalidations are used on the actual HW.

As above, decoding arch-specific structures into generic ones is what an
emulated IOMMU does, and it doesn't make a performance difference in which
format it forwards that to the kernel. The host IOMMU driver checks the
guest request and copies them into the command queue. Whether that request
comes in the form of a structure binary-compatible with Arm SMMUvX.Y, or
some generic structure, does not make a difference.

Thanks,
Jean


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-03-09 18:28 UTC|newest]

Thread overview: 165+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-09 10:53 [PATCH v1 00/14] Add Nested Translation Support for SMMUv3 Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 01/14] iommu: Add iommu_get_unmanaged_domain helper Nicolin Chen
2023-03-09 12:51   ` Robin Murphy
2023-03-09 14:19     ` Jason Gunthorpe
2023-03-09 19:04       ` Robin Murphy
2023-03-10  0:23         ` Jason Gunthorpe
2023-03-10  8:41     ` Eric Auger
2023-03-10 15:55       ` Jason Gunthorpe
2023-03-16  1:21         ` Nicolin Chen
2023-03-16 18:42           ` Robin Murphy
2023-03-16 20:01             ` Nicolin Chen
2023-03-20 12:51             ` Jason Gunthorpe
2023-03-10 10:14   ` Eric Auger
2023-03-10 15:33     ` Jason Gunthorpe
2023-03-10 15:44       ` Shameerali Kolothum Thodi
2023-03-10 15:56         ` Jason Gunthorpe
2023-03-10 16:07           ` Shameerali Kolothum Thodi
2023-03-10 16:21             ` Jason Gunthorpe
2023-03-10 16:30               ` Shameerali Kolothum Thodi
2023-03-10 17:03                 ` Jason Gunthorpe
2023-03-22 16:07                   ` Eric Auger
2023-03-22 17:02                     ` Jason Gunthorpe
2023-03-22 17:41                       ` Eric Auger
2023-03-22 18:07                         ` Jason Gunthorpe
2023-03-16 19:51                 ` Nicolin Chen
2023-03-16 19:56                   ` Shameerali Kolothum Thodi
2023-03-22 15:44                     ` Eric Auger
2023-03-09 10:53 ` [PATCH v1 02/14] iommufd: Add nesting related data structures for ARM SMMUv3 Nicolin Chen
2023-03-09 13:42   ` Jean-Philippe Brucker
2023-03-09 14:48     ` Jason Gunthorpe
2023-03-09 18:26       ` Jean-Philippe Brucker [this message]
2023-03-09 21:01         ` Jason Gunthorpe
2023-03-10 12:16           ` Jean-Philippe Brucker
2023-03-10 14:52           ` Robin Murphy
2023-03-10 15:25             ` Jason Gunthorpe
2023-03-10 15:57               ` Robin Murphy
2023-03-10 16:03                 ` Jason Gunthorpe
2023-03-17 10:10                   ` Tian, Kevin
2023-03-17 10:04                 ` Tian, Kevin
2023-03-10  4:50       ` Nicolin Chen
2023-03-10 12:54         ` Jean-Philippe Brucker
2023-03-10 14:00           ` Jason Gunthorpe
2023-03-10 16:06         ` Jason Gunthorpe
2023-03-16  0:59           ` Nicolin Chen
2023-03-09 15:26     ` Shameerali Kolothum Thodi
2023-03-09 15:40       ` Jason Gunthorpe
2023-03-09 15:51         ` Shameerali Kolothum Thodi
2023-03-09 15:59           ` Jason Gunthorpe
2023-03-09 16:07             ` Shameerali Kolothum Thodi
2023-03-10  5:26               ` Nicolin Chen
2023-03-10  5:36                 ` Nicolin Chen
2023-03-10 12:55                   ` Jason Gunthorpe
2023-03-10  5:18         ` Nicolin Chen
2023-03-10  5:04     ` Nicolin Chen
2023-03-10 11:33     ` Eric Auger
2023-03-10 12:51       ` Jason Gunthorpe
2023-03-17 10:17         ` Tian, Kevin
2023-03-09 10:53 ` [PATCH v1 03/14] iommufd/device: Setup MSI on kernel-managed domains Nicolin Chen
2023-03-10 16:45   ` Eric Auger
2023-03-11  0:17     ` Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 04/14] iommu/arm-smmu-v3: Add arm_smmu_hw_info Nicolin Chen
2023-03-09 13:03   ` Robin Murphy
2023-03-10  1:17     ` Nicolin Chen
2023-03-10 15:28       ` Robin Murphy
2023-03-16  0:13         ` Nicolin Chen
2023-03-16 15:19           ` Robin Murphy
2023-03-16 20:06             ` Nicolin Chen
2023-04-12  7:47               ` Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 05/14] iommu/arm-smmu-v3: Remove ARM_SMMU_DOMAIN_NESTED Nicolin Chen
2023-03-10 16:39   ` Eric Auger
2023-03-10 17:05     ` Jason Gunthorpe
2023-03-11  0:24       ` Nicolin Chen
2023-03-11  0:23     ` Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 06/14] iommu/arm-smmu-v3: Unset corresponding STE fields when s2_cfg is NULL Nicolin Chen
2023-03-09 13:13   ` Robin Murphy
2023-03-09 18:24     ` Shameerali Kolothum Thodi
2023-03-10  1:54       ` Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 07/14] iommu/arm-smmu-v3: Add STRTAB_STE_0_CFG_NESTED for 2-stage translation Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 08/14] iommu/arm-smmu-v3: Prepare for nested domain support Nicolin Chen
2023-03-10 20:39   ` Robin Murphy
2023-03-11 12:40     ` Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 09/14] iommu/arm-smmu-v3: Implement arm_smmu_get_unmanaged_domain Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 10/14] iommu/arm-smmu-v3: Pass in user_cfg to arm_smmu_domain_finalise Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 11/14] iommu/arm-smmu-v3: Add arm_smmu_domain_alloc_user Nicolin Chen
2023-03-24 15:28   ` Eric Auger
2023-03-24 17:40     ` Nicolin Chen
2023-03-24 17:50       ` Jason Gunthorpe
2023-03-24 18:00         ` Nicolin Chen
2023-03-24 15:33   ` Eric Auger
2023-03-24 17:43     ` Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 12/14] iommu/arm-smmu-v3: Support IOMMU_DOMAIN_NESTED type of allocations Nicolin Chen
2023-03-09 13:20   ` Robin Murphy
2023-03-09 14:28     ` Robin Murphy
2023-03-10  1:34       ` Nicolin Chen
2023-03-24 15:44   ` Eric Auger
2023-03-24 16:30     ` Jason Gunthorpe
2023-03-24 17:50     ` Nicolin Chen
2023-03-24 17:51       ` Jason Gunthorpe
2023-03-24 17:55         ` Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 13/14] iommu/arm-smmu-v3: Add CMDQ_OP_TLBI_NH_VAA and CMDQ_OP_TLBI_NH_ALL Nicolin Chen
2023-03-09 13:44   ` Robin Murphy
2023-03-10  1:19     ` Nicolin Chen
2023-03-09 10:53 ` [PATCH v1 14/14] iommu/arm-smmu-v3: Add arm_smmu_cache_invalidate_user Nicolin Chen
2023-03-09 14:49   ` Robin Murphy
2023-03-09 15:31     ` Jason Gunthorpe
2023-03-10  4:20       ` Nicolin Chen
2023-03-10 16:19         ` Jason Gunthorpe
2023-03-11 11:56           ` Nicolin Chen
2023-03-11 12:53             ` Nicolin Chen
2023-03-20 13:03             ` Jason Gunthorpe
2023-03-20 15:56               ` Nicolin Chen
2023-03-20 16:04                 ` Jason Gunthorpe
2023-03-20 16:59                   ` Nicolin Chen
2023-03-20 18:45                     ` Jason Gunthorpe
2023-03-20 21:22                       ` Nicolin Chen
2023-03-20 22:19                         ` Jason Gunthorpe
2023-03-22 20:57                           ` Nicolin Chen
2023-03-23 12:17                             ` Jason Gunthorpe
2023-03-17  9:41           ` Tian, Kevin
2023-03-17 14:24             ` Nicolin Chen
2023-03-20 12:59             ` Jason Gunthorpe
2023-03-20 16:12               ` Nicolin Chen
2023-03-20 18:00                 ` Jason Gunthorpe
2023-03-21  8:34                   ` Tian, Kevin
2023-03-21 11:48                     ` Jason Gunthorpe
2023-03-22  6:42                       ` Nicolin Chen
2023-03-22 12:43                         ` Jason Gunthorpe
2023-03-22 17:11                           ` Nicolin Chen
2023-03-22 17:28                             ` Jason Gunthorpe
2023-03-22 19:21                               ` Nicolin Chen
2023-03-22 19:41                                 ` Jason Gunthorpe
2023-03-22 20:43                                   ` Nicolin Chen
2023-03-23 12:16                                     ` Jason Gunthorpe
2023-03-23 18:13                                       ` Nicolin Chen
2023-03-23 18:27                                         ` Jason Gunthorpe
2023-03-24  9:02                         ` Tian, Kevin
2023-03-24 14:57                           ` Jason Gunthorpe
2023-03-24 17:35                             ` Nicolin Chen
2023-03-28  3:03                               ` Tian, Kevin
2023-03-24  8:47                       ` Tian, Kevin
2023-03-24 14:44                         ` Jason Gunthorpe
2023-03-28  2:48                           ` Tian, Kevin
2023-03-28 12:26                             ` Jason Gunthorpe
2023-03-31  8:09                               ` Tian, Kevin
2023-03-17  9:24       ` Tian, Kevin
2023-03-10  3:51     ` Nicolin Chen
2023-03-10 17:53       ` Robin Murphy
2023-03-10 18:49         ` Jason Gunthorpe
2023-03-11 12:38         ` Nicolin Chen
2023-03-13 13:07           ` Robin Murphy
2023-03-16  0:01             ` Nicolin Chen
2023-03-16 14:58               ` Robin Murphy
2023-03-16 21:09                 ` Nicolin Chen
2023-03-20  1:32                   ` Nicolin Chen
2023-03-20 13:11                     ` Jason Gunthorpe
2023-03-20 15:28                       ` Nicolin Chen
2023-03-20 16:01                         ` Jason Gunthorpe
2023-03-20 16:35                           ` Nicolin Chen
2023-03-20 18:07                             ` Jason Gunthorpe
2023-03-20 20:46                               ` Nicolin Chen
2023-03-20 22:14                                 ` Jason Gunthorpe
2023-03-22  5:14                                   ` Nicolin Chen
2023-03-24  8:55                                     ` Tian, Kevin
2023-03-17  9:47     ` Tian, Kevin
2023-03-17 14:16       ` Nicolin Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230309182659.GA1710571@myrica \
    --to=jean-philippe@linaro.org \
    --cc=baolu.lu@linux.intel.com \
    --cc=eric.auger@redhat.com \
    --cc=iommu@lists.linux.dev \
    --cc=jgg@nvidia.com \
    --cc=joro@8bytes.org \
    --cc=kevin.tian@intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nicolinc@nvidia.com \
    --cc=robin.murphy@arm.com \
    --cc=shameerali.kolothum.thodi@huawei.com \
    --cc=will@kernel.org \
    --cc=yi.l.liu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox