All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
To: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Cc: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Scott Branden <sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	Linux Kernel
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linux IOMMU
	<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
	BCM Kernel Feedback
	<bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	Linux ARM Kernel
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: [PATCH 3/5] iommu/arm-smmu-v3: add IOMMU_CAP_BYPASS to the ARM SMMUv3 driver
Date: Thu, 20 Jul 2017 10:10:03 +0100	[thread overview]
Message-ID: <20170720091003.GA17837@arm.com> (raw)
In-Reply-To: <CAALAos-YbVbgvPmDYXeWaF4y8MQ9i7CG3tgGuf-QLR6hmXo18Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, Jul 20, 2017 at 09:32:00AM +0530, Anup Patel wrote:
> On Wed, Jul 19, 2017 at 5:23 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> > There are two things here:
> >
> >   1. iommu_present() is pretty useless, because it applies to a "bus" which
> >      doesn't actually tell you what you need to know for things like the
> >      platform_bus, where some masters might be upstream of an SMMU and
> >      others might not be.
> 
> I agree with you. The iommu_present() check in vfio_iommu_group_get()
> is not much useful. We only reach line which checks iommu_present()
> when iommu_group_get() returns NULL for given "struct device *". If there
> is no IOMMU group for a "struct device *" then it means there is no IOMMU
> HW doing translations for such device.
> 
> If we drop the iommu_present() check (due to above reasons) in
> vfio_iommu_group_get() then we don't require the IOMMU_CAP_BYPASS
> and we can happily drop PATCH1, PATCH2, and PATCH3.
> 
> I will remove the iommu_present() check in vfio_iommu_group_get()
> because it is only comes into actions when VFIO_NOIOMMU is
> enabled. This will also help us drop PATCH1-to-PATCH3.

I don't think that's the right answer. Whilst iommu_present has obvious
shortcomings, its intention is clear: it should tell you whether a given
*device* is upstream of an IOMMU. So the right fix is to make this
per-device, instead of per-bus. Removing it altogether is worse than leaving
it like it is.

> >   2. If a master *is* upstream of an IOMMU and you want to use no-IOMMU,
> >      then the VFIO no-IOMMU code needs to be extended so that it creates
> >      an IDENTITY domain on that IOMMU.
> 
> The VFIO no-IOMMU mode is equivalent to Linux UIO hence having
> IDENTITY domain for VFIO no-IOMMU is not appropriate here.

Can you elaborate on this please? I don't understand the argument you're
making. It's like saying "I don't like eggs, therefore I don't drive a
car".

Will

WARNING: multiple messages have this Message-ID (diff)
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/5] iommu/arm-smmu-v3: add IOMMU_CAP_BYPASS to the ARM SMMUv3 driver
Date: Thu, 20 Jul 2017 10:10:03 +0100	[thread overview]
Message-ID: <20170720091003.GA17837@arm.com> (raw)
In-Reply-To: <CAALAos-YbVbgvPmDYXeWaF4y8MQ9i7CG3tgGuf-QLR6hmXo18Q@mail.gmail.com>

On Thu, Jul 20, 2017 at 09:32:00AM +0530, Anup Patel wrote:
> On Wed, Jul 19, 2017 at 5:23 PM, Will Deacon <will.deacon@arm.com> wrote:
> > There are two things here:
> >
> >   1. iommu_present() is pretty useless, because it applies to a "bus" which
> >      doesn't actually tell you what you need to know for things like the
> >      platform_bus, where some masters might be upstream of an SMMU and
> >      others might not be.
> 
> I agree with you. The iommu_present() check in vfio_iommu_group_get()
> is not much useful. We only reach line which checks iommu_present()
> when iommu_group_get() returns NULL for given "struct device *". If there
> is no IOMMU group for a "struct device *" then it means there is no IOMMU
> HW doing translations for such device.
> 
> If we drop the iommu_present() check (due to above reasons) in
> vfio_iommu_group_get() then we don't require the IOMMU_CAP_BYPASS
> and we can happily drop PATCH1, PATCH2, and PATCH3.
> 
> I will remove the iommu_present() check in vfio_iommu_group_get()
> because it is only comes into actions when VFIO_NOIOMMU is
> enabled. This will also help us drop PATCH1-to-PATCH3.

I don't think that's the right answer. Whilst iommu_present has obvious
shortcomings, its intention is clear: it should tell you whether a given
*device* is upstream of an IOMMU. So the right fix is to make this
per-device, instead of per-bus. Removing it altogether is worse than leaving
it like it is.

> >   2. If a master *is* upstream of an IOMMU and you want to use no-IOMMU,
> >      then the VFIO no-IOMMU code needs to be extended so that it creates
> >      an IDENTITY domain on that IOMMU.
> 
> The VFIO no-IOMMU mode is equivalent to Linux UIO hence having
> IDENTITY domain for VFIO no-IOMMU is not appropriate here.

Can you elaborate on this please? I don't understand the argument you're
making. It's like saying "I don't like eggs, therefore I don't drive a
car".

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: Anup Patel <anup.patel@broadcom.com>
Cc: Robin Murphy <robin.murphy@arm.com>,
	Joerg Roedel <joro@8bytes.org>,
	Baptiste Reynal <b.reynal@virtualopensystems.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	Scott Branden <sbranden@broadcom.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Linux ARM Kernel <linux-arm-kernel@lists.infradead.org>,
	Linux IOMMU <iommu@lists.linux-foundation.org>,
	kvm@vger.kernel.org,
	BCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>
Subject: Re: [PATCH 3/5] iommu/arm-smmu-v3: add IOMMU_CAP_BYPASS to the ARM SMMUv3 driver
Date: Thu, 20 Jul 2017 10:10:03 +0100	[thread overview]
Message-ID: <20170720091003.GA17837@arm.com> (raw)
In-Reply-To: <CAALAos-YbVbgvPmDYXeWaF4y8MQ9i7CG3tgGuf-QLR6hmXo18Q@mail.gmail.com>

On Thu, Jul 20, 2017 at 09:32:00AM +0530, Anup Patel wrote:
> On Wed, Jul 19, 2017 at 5:23 PM, Will Deacon <will.deacon@arm.com> wrote:
> > There are two things here:
> >
> >   1. iommu_present() is pretty useless, because it applies to a "bus" which
> >      doesn't actually tell you what you need to know for things like the
> >      platform_bus, where some masters might be upstream of an SMMU and
> >      others might not be.
> 
> I agree with you. The iommu_present() check in vfio_iommu_group_get()
> is not much useful. We only reach line which checks iommu_present()
> when iommu_group_get() returns NULL for given "struct device *". If there
> is no IOMMU group for a "struct device *" then it means there is no IOMMU
> HW doing translations for such device.
> 
> If we drop the iommu_present() check (due to above reasons) in
> vfio_iommu_group_get() then we don't require the IOMMU_CAP_BYPASS
> and we can happily drop PATCH1, PATCH2, and PATCH3.
> 
> I will remove the iommu_present() check in vfio_iommu_group_get()
> because it is only comes into actions when VFIO_NOIOMMU is
> enabled. This will also help us drop PATCH1-to-PATCH3.

I don't think that's the right answer. Whilst iommu_present has obvious
shortcomings, its intention is clear: it should tell you whether a given
*device* is upstream of an IOMMU. So the right fix is to make this
per-device, instead of per-bus. Removing it altogether is worse than leaving
it like it is.

> >   2. If a master *is* upstream of an IOMMU and you want to use no-IOMMU,
> >      then the VFIO no-IOMMU code needs to be extended so that it creates
> >      an IDENTITY domain on that IOMMU.
> 
> The VFIO no-IOMMU mode is equivalent to Linux UIO hence having
> IDENTITY domain for VFIO no-IOMMU is not appropriate here.

Can you elaborate on this please? I don't understand the argument you're
making. It's like saying "I don't like eggs, therefore I don't drive a
car".

Will

  parent reply	other threads:[~2017-07-20  9:10 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-19  9:33 [PATCH 0/5] FlexRM support in VFIO platform Anup Patel via iommu
2017-07-19  9:33 ` Anup Patel
2017-07-19  9:33 ` Anup Patel
2017-07-19  9:33 ` [PATCH 1/5] iommu: Add capability IOMMU_CAP_BYPASS Anup Patel
2017-07-19  9:33   ` Anup Patel
     [not found]   ` <1500456838-18405-2-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2017-07-19 10:58     ` Robin Murphy
2017-07-19 10:58       ` Robin Murphy
2017-07-19 10:58       ` Robin Murphy
2017-07-19 11:19       ` Anup Patel
2017-07-19 11:19         ` Anup Patel
     [not found]         ` <CAALAos_o25mtphxcVE0rr9dE8YAm-os2C_HD9oDnt41ZMDP8pA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-19 11:23           ` Will Deacon
2017-07-19 11:23             ` Will Deacon
2017-07-19 11:23             ` Will Deacon
2017-07-19 11:26             ` Anup Patel
2017-07-19 11:26               ` Anup Patel
     [not found]               ` <CAALAos8nf-d5i74J7-sAJb2o9eFj2PVDeLLVd=wun+gW1ihXSA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-19 11:29                 ` Robin Murphy
2017-07-19 11:29                   ` Robin Murphy
2017-07-19 11:29                   ` Robin Murphy
2017-07-19 11:30                 ` Will Deacon
2017-07-19 11:30                   ` Will Deacon
2017-07-19 11:30                   ` Will Deacon
     [not found]                   ` <20170719113017.GH13642-5wv7dgnIgG8@public.gmane.org>
2017-07-19 11:33                     ` Anup Patel via iommu
2017-07-19 11:33                       ` Anup Patel
2017-07-19 11:33                       ` Anup Patel
2017-07-19  9:33 ` [PATCH 2/5] iommu/arm-smmu: add IOMMU_CAP_BYPASS to the ARM SMMU driver Anup Patel
2017-07-19  9:33   ` Anup Patel
     [not found]   ` <1500456838-18405-3-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2017-07-19 10:59     ` Robin Murphy
2017-07-19 10:59       ` Robin Murphy
2017-07-19 10:59       ` Robin Murphy
     [not found] ` <1500456838-18405-1-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2017-07-19  9:33   ` [PATCH 3/5] iommu/arm-smmu-v3: add IOMMU_CAP_BYPASS to the ARM SMMUv3 driver Anup Patel via iommu
2017-07-19  9:33     ` Anup Patel
2017-07-19  9:33     ` Anup Patel
2017-07-19 11:00     ` Robin Murphy
2017-07-19 11:00       ` Robin Murphy
     [not found]       ` <ace28bd1-2dd3-482c-d11b-539c055ee9b7-5wv7dgnIgG8@public.gmane.org>
2017-07-19 11:23         ` Anup Patel via iommu
2017-07-19 11:23           ` Anup Patel
2017-07-19 11:23           ` Anup Patel
     [not found]           ` <CAALAos8y0kLGrbwgz3u=ZVyteSicuPyQZ3Yp=LPZohXzVaE5NQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-19 11:25             ` Will Deacon
2017-07-19 11:25               ` Will Deacon
2017-07-19 11:25               ` Will Deacon
     [not found]               ` <20170719112524.GF13642-5wv7dgnIgG8@public.gmane.org>
2017-07-19 11:31                 ` Anup Patel via iommu
2017-07-19 11:31                   ` Anup Patel
2017-07-19 11:31                   ` Anup Patel
     [not found]                   ` <CAALAos_xGCt7U1Oz5uCf9Q4hP6fdn4fcymFNQP5xO_UBduSY3g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-19 11:33                     ` Will Deacon
2017-07-19 11:33                       ` Will Deacon
2017-07-19 11:33                       ` Will Deacon
2017-07-19 11:39                       ` Anup Patel
2017-07-19 11:39                         ` Anup Patel
     [not found]                         ` <CAALAos99HWLoW=0P-dvFA+oNsF03TRNzNbfppyQNtqmcRELXXQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-19 11:53                           ` Will Deacon
2017-07-19 11:53                             ` Will Deacon
2017-07-19 11:53                             ` Will Deacon
     [not found]                             ` <20170719115333.GJ13642-5wv7dgnIgG8@public.gmane.org>
2017-07-20  4:02                               ` Anup Patel via iommu
2017-07-20  4:02                                 ` Anup Patel
2017-07-20  4:02                                 ` Anup Patel
     [not found]                                 ` <CAALAos-YbVbgvPmDYXeWaF4y8MQ9i7CG3tgGuf-QLR6hmXo18Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-20  9:10                                   ` Will Deacon [this message]
2017-07-20  9:10                                     ` Will Deacon
2017-07-20  9:10                                     ` Will Deacon
     [not found]                                     ` <20170720091003.GA17837-5wv7dgnIgG8@public.gmane.org>
2017-07-20 11:08                                       ` Anup Patel via iommu
2017-07-20 11:08                                         ` Anup Patel
2017-07-20 11:08                                         ` Anup Patel
     [not found]                                         ` <CAALAos8571-UVp7Ky81qFESVcNcWx_H2VOemFt_++NFCxcMg3w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-20 11:17                                           ` Will Deacon
2017-07-20 11:17                                             ` Will Deacon
2017-07-20 11:17                                             ` Will Deacon
2017-07-20 11:17                                       ` Robin Murphy
2017-07-20 11:17                                         ` Robin Murphy
2017-07-20 11:17                                         ` Robin Murphy
     [not found]                                         ` <8e82d8f5-e5e2-dd09-c774-29f9eda2ecdd-5wv7dgnIgG8@public.gmane.org>
2017-07-24 17:16                                           ` Alex Williamson
2017-07-24 17:16                                             ` Alex Williamson
2017-07-24 17:16                                             ` Alex Williamson
     [not found]                                             ` <20170724111621.7f1c3a85-DGNDKt5SQtizQB+pC5nmwQ@public.gmane.org>
2017-07-24 17:23                                               ` Robin Murphy
2017-07-24 17:23                                                 ` Robin Murphy
2017-07-24 17:23                                                 ` Robin Murphy
     [not found]                                                 ` <6468f359-1682-b9b0-5a4d-72738939cb84-5wv7dgnIgG8@public.gmane.org>
2017-07-24 19:06                                                   ` Alex Williamson
2017-07-24 19:06                                                     ` Alex Williamson
2017-07-24 19:06                                                     ` Alex Williamson
2017-07-25  8:59                                                     ` Anup Patel
2017-07-25  8:59                                                       ` Anup Patel
2017-07-19  9:33   ` [PATCH 5/5] vfio: platform: reset: Add Broadcom FlexRM reset module Anup Patel via iommu
2017-07-19  9:33     ` Anup Patel
2017-07-19  9:33     ` Anup Patel
     [not found]     ` <1500456838-18405-6-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2017-07-19 16:50       ` Scott Branden via iommu
2017-07-19 16:50         ` Scott Branden
2017-07-19 16:50         ` Scott Branden
     [not found]         ` <27236534-a9fb-53bc-f0c6-eb674833a94e-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2017-07-20  4:05           ` Anup Patel via iommu
2017-07-20  4:05             ` Anup Patel
2017-07-20  4:05             ` Anup Patel
2017-07-19  9:33 ` [PATCH 4/5] vfio: Allow No-IOMMU mode for IOMMUs with bypass capability Anup Patel
2017-07-19  9:33   ` Anup Patel
2017-07-19 10:57 ` [PATCH 0/5] FlexRM support in VFIO platform Robin Murphy
2017-07-19 10:57   ` Robin Murphy
     [not found]   ` <0dc860ed-a40c-1bfd-f584-225807edb25b-5wv7dgnIgG8@public.gmane.org>
2017-07-19 11:17     ` Anup Patel via iommu
2017-07-19 11:17       ` Anup Patel
2017-07-19 11:17       ` Anup Patel
2017-07-19 11:25       ` Robin Murphy
2017-07-19 11:25         ` Robin Murphy
     [not found]         ` <d4ef45d5-43cc-c632-082d-a92b8681faee-5wv7dgnIgG8@public.gmane.org>
2017-07-19 11:28           ` Anup Patel via iommu
2017-07-19 11:28             ` Anup Patel
2017-07-19 11:28             ` Anup Patel

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=20170720091003.GA17837@arm.com \
    --to=will.deacon-5wv7dgnigg8@public.gmane.org \
    --cc=anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.