Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1382739 > unrolled thread
| Started by | Eric Auger <eric.auger@linaro.org> |
|---|---|
| First post | 2016-04-19 19:30 +0200 |
| Last post | 2016-04-22 17:50 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v7 6/7] iommu/arm-smmu: do not advertise IOMMU_CAP_INTR_REMAP Eric Auger <eric.auger@linaro.org> - 2016-04-19 19:30 +0200
Re: [PATCH v7 6/7] iommu/arm-smmu: do not advertise IOMMU_CAP_INTR_REMAP Robin Murphy <robin.murphy@arm.com> - 2016-04-22 13:20 +0200
Re: [PATCH v7 6/7] iommu/arm-smmu: do not advertise IOMMU_CAP_INTR_REMAP Eric Auger <eric.auger@linaro.org> - 2016-04-22 13:50 +0200
Re: [PATCH v7 6/7] iommu/arm-smmu: do not advertise IOMMU_CAP_INTR_REMAP Robin Murphy <robin.murphy@arm.com> - 2016-04-22 17:50 +0200
| From | Eric Auger <eric.auger@linaro.org> |
|---|---|
| Date | 2016-04-19 19:30 +0200 |
| Subject | [PATCH v7 6/7] iommu/arm-smmu: do not advertise IOMMU_CAP_INTR_REMAP |
| Message-ID | <rpHDj-1Ya-9@gated-at.bofh.it> |
Do not advertise IOMMU_CAP_INTR_REMAP for arm-smmu(-v3). Indeed the irq_remapping capability is abstracted on irqchip side for ARM as opposed to Intel IOMMU featuring IRQ remapping HW. So to check IRQ remapping capability, the msi domain needs to be checked instead. This commit needs to be applied after "vfio/type1: also check IRQ remapping capability at msi domain" else the legacy interrupt assignment gets broken with arm-smmu. Signed-off-by: Eric Auger <eric.auger@linaro.org> --- drivers/iommu/arm-smmu-v3.c | 3 ++- drivers/iommu/arm-smmu.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index afd0dac..1d0106c 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1386,7 +1386,8 @@ static bool arm_smmu_capable(enum iommu_cap cap) case IOMMU_CAP_CACHE_COHERENCY: return true; case IOMMU_CAP_INTR_REMAP: - return true; /* MSIs are just memory writes */ + /* interrupt translation handled at MSI controller level */ + return false; case IOMMU_CAP_NOEXEC: return true; default: diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 492339f..6232b2a 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1312,7 +1312,8 @@ static bool arm_smmu_capable(enum iommu_cap cap) */ return true; case IOMMU_CAP_INTR_REMAP: - return true; /* MSIs are just memory writes */ + /* interrupt translation handled at MSI controller level */ + return false; case IOMMU_CAP_NOEXEC: return true; default: -- 1.9.1
[toc] | [next] | [standalone]
| From | Robin Murphy <robin.murphy@arm.com> |
|---|---|
| Date | 2016-04-22 13:20 +0200 |
| Subject | Re: [PATCH v7 6/7] iommu/arm-smmu: do not advertise IOMMU_CAP_INTR_REMAP |
| Message-ID | <rqHhU-14S-19@gated-at.bofh.it> |
| In reply to | #1382739 |
Hi Eric, Alex, On 19/04/16 18:24, Eric Auger wrote: > Do not advertise IOMMU_CAP_INTR_REMAP for arm-smmu(-v3). Indeed the > irq_remapping capability is abstracted on irqchip side for ARM as > opposed to Intel IOMMU featuring IRQ remapping HW. > > So to check IRQ remapping capability, the msi domain needs to be > checked instead. > > This commit needs to be applied after "vfio/type1: also check IRQ > remapping capability at msi domain" else the legacy interrupt > assignment gets broken with arm-smmu. Hmm, that smells of papering over a different problem. I may have missed it, but I don't see anything changing legacy interrupt behaviour in this series - are legacy INTx (or platform) interrupts intrinsically safe because they're physically wired, or intrinsically unsafe because they could be shared? If it's the latter then I don't see how the IOMMU or MSI controller changes anything in that respect, and if it's the former then surely we should support that right now without the SMMU having to lie about MSI isolation? I started looking into it but I'm a bit lost... Robin. > Signed-off-by: Eric Auger <eric.auger@linaro.org> > --- > drivers/iommu/arm-smmu-v3.c | 3 ++- > drivers/iommu/arm-smmu.c | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index afd0dac..1d0106c 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -1386,7 +1386,8 @@ static bool arm_smmu_capable(enum iommu_cap cap) > case IOMMU_CAP_CACHE_COHERENCY: > return true; > case IOMMU_CAP_INTR_REMAP: > - return true; /* MSIs are just memory writes */ > + /* interrupt translation handled at MSI controller level */ > + return false; > case IOMMU_CAP_NOEXEC: > return true; > default: > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index 492339f..6232b2a 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -1312,7 +1312,8 @@ static bool arm_smmu_capable(enum iommu_cap cap) > */ > return true; > case IOMMU_CAP_INTR_REMAP: > - return true; /* MSIs are just memory writes */ > + /* interrupt translation handled at MSI controller level */ > + return false; > case IOMMU_CAP_NOEXEC: > return true; > default: >
[toc] | [prev] | [next] | [standalone]
| From | Eric Auger <eric.auger@linaro.org> |
|---|---|
| Date | 2016-04-22 13:50 +0200 |
| Subject | Re: [PATCH v7 6/7] iommu/arm-smmu: do not advertise IOMMU_CAP_INTR_REMAP |
| Message-ID | <rqHKV-1ip-3@gated-at.bofh.it> |
| In reply to | #1385005 |
Hi Robin, On 04/22/2016 01:16 PM, Robin Murphy wrote: > Hi Eric, Alex, > > On 19/04/16 18:24, Eric Auger wrote: >> Do not advertise IOMMU_CAP_INTR_REMAP for arm-smmu(-v3). Indeed the >> irq_remapping capability is abstracted on irqchip side for ARM as >> opposed to Intel IOMMU featuring IRQ remapping HW. >> >> So to check IRQ remapping capability, the msi domain needs to be >> checked instead. >> >> This commit needs to be applied after "vfio/type1: also check IRQ >> remapping capability at msi domain" else the legacy interrupt >> assignment gets broken with arm-smmu. > > Hmm, that smells of papering over a different problem. I may have missed > it, but I don't see anything changing legacy interrupt behaviour in this > series - are legacy INTx (or platform) interrupts intrinsically safe > because they're physically wired, or intrinsically unsafe because they > could be shared? I think it is safe. With legacy/platform interrupts we have: vfio pci driver physical IRQ handler signals an irqfd. upon this irqfd signaling KVM injects a virtual IRQ. So the assigned device does not have any way to trigger a storm of interrupts on the host, as opposed to with MSI. Does it make sense to you? Best Regards Eric If it's the latter then I don't see how the IOMMU or > MSI controller changes anything in that respect, and if it's the former > then surely we should support that right now without the SMMU having to > lie about MSI isolation? I started looking into it but I'm a bit lost... > > Robin. > >> Signed-off-by: Eric Auger <eric.auger@linaro.org> >> --- >> drivers/iommu/arm-smmu-v3.c | 3 ++- >> drivers/iommu/arm-smmu.c | 3 ++- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c >> index afd0dac..1d0106c 100644 >> --- a/drivers/iommu/arm-smmu-v3.c >> +++ b/drivers/iommu/arm-smmu-v3.c >> @@ -1386,7 +1386,8 @@ static bool arm_smmu_capable(enum iommu_cap cap) >> case IOMMU_CAP_CACHE_COHERENCY: >> return true; >> case IOMMU_CAP_INTR_REMAP: >> - return true; /* MSIs are just memory writes */ >> + /* interrupt translation handled at MSI controller level */ >> + return false; >> case IOMMU_CAP_NOEXEC: >> return true; >> default: >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >> index 492339f..6232b2a 100644 >> --- a/drivers/iommu/arm-smmu.c >> +++ b/drivers/iommu/arm-smmu.c >> @@ -1312,7 +1312,8 @@ static bool arm_smmu_capable(enum iommu_cap cap) >> */ >> return true; >> case IOMMU_CAP_INTR_REMAP: >> - return true; /* MSIs are just memory writes */ >> + /* interrupt translation handled at MSI controller level */ >> + return false; >> case IOMMU_CAP_NOEXEC: >> return true; >> default: >> >
[toc] | [prev] | [next] | [standalone]
| From | Robin Murphy <robin.murphy@arm.com> |
|---|---|
| Date | 2016-04-22 17:50 +0200 |
| Subject | Re: [PATCH v7 6/7] iommu/arm-smmu: do not advertise IOMMU_CAP_INTR_REMAP |
| Message-ID | <rqLvd-49a-49@gated-at.bofh.it> |
| In reply to | #1385100 |
On 22/04/16 12:39, Eric Auger wrote: > Hi Robin, > On 04/22/2016 01:16 PM, Robin Murphy wrote: >> Hi Eric, Alex, >> >> On 19/04/16 18:24, Eric Auger wrote: >>> Do not advertise IOMMU_CAP_INTR_REMAP for arm-smmu(-v3). Indeed the >>> irq_remapping capability is abstracted on irqchip side for ARM as >>> opposed to Intel IOMMU featuring IRQ remapping HW. >>> >>> So to check IRQ remapping capability, the msi domain needs to be >>> checked instead. >>> >>> This commit needs to be applied after "vfio/type1: also check IRQ >>> remapping capability at msi domain" else the legacy interrupt >>> assignment gets broken with arm-smmu. >> >> Hmm, that smells of papering over a different problem. I may have missed >> it, but I don't see anything changing legacy interrupt behaviour in this >> series - are legacy INTx (or platform) interrupts intrinsically safe >> because they're physically wired, or intrinsically unsafe because they >> could be shared? > > I think it is safe. With legacy/platform interrupts we have: > vfio pci driver physical IRQ handler signals an irqfd. > upon this irqfd signaling KVM injects a virtual IRQ. > > So the assigned device does not have any way to trigger a storm of > interrupts on the host, as opposed to with MSI. > > Does it make sense to you? I think so, thanks for the explanation. In that case, I'm strongly in favour of applying this patch and un-breaking legacy interrupts regardless of MSI support. I'll keep investigating to see what I can figure out. Robin. > Best Regards > > Eric > > If it's the latter then I don't see how the IOMMU or >> MSI controller changes anything in that respect, and if it's the former >> then surely we should support that right now without the SMMU having to >> lie about MSI isolation? I started looking into it but I'm a bit lost... >> >> Robin. >> >>> Signed-off-by: Eric Auger <eric.auger@linaro.org> >>> --- >>> drivers/iommu/arm-smmu-v3.c | 3 ++- >>> drivers/iommu/arm-smmu.c | 3 ++- >>> 2 files changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c >>> index afd0dac..1d0106c 100644 >>> --- a/drivers/iommu/arm-smmu-v3.c >>> +++ b/drivers/iommu/arm-smmu-v3.c >>> @@ -1386,7 +1386,8 @@ static bool arm_smmu_capable(enum iommu_cap cap) >>> case IOMMU_CAP_CACHE_COHERENCY: >>> return true; >>> case IOMMU_CAP_INTR_REMAP: >>> - return true; /* MSIs are just memory writes */ >>> + /* interrupt translation handled at MSI controller level */ >>> + return false; >>> case IOMMU_CAP_NOEXEC: >>> return true; >>> default: >>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >>> index 492339f..6232b2a 100644 >>> --- a/drivers/iommu/arm-smmu.c >>> +++ b/drivers/iommu/arm-smmu.c >>> @@ -1312,7 +1312,8 @@ static bool arm_smmu_capable(enum iommu_cap cap) >>> */ >>> return true; >>> case IOMMU_CAP_INTR_REMAP: >>> - return true; /* MSIs are just memory writes */ >>> + /* interrupt translation handled at MSI controller level */ >>> + return false; >>> case IOMMU_CAP_NOEXEC: >>> return true; >>> default: >>> >> >
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web