linux-kernel - Re: [PATCH v7 6/7] iommu/arm-smmu: do not advertise IOMMU_CAP_INTR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 22 Apr 2016 12:16:26 +0100
From:	Robin Murphy <robin.murphy@....com>
To:	Eric Auger <eric.auger@...aro.org>, eric.auger@...com,
	alex.williamson@...hat.com, will.deacon@....com, joro@...tes.org,
	tglx@...utronix.de, jason@...edaemon.net, marc.zyngier@....com,
	christoffer.dall@...aro.org, linux-arm-kernel@...ts.infradead.org
Cc:	patches@...aro.org, linux-kernel@...r.kernel.org,
	Bharat.Bhushan@...escale.com, pranav.sawargaonkar@...il.com,
	p.fedin@...sung.com, iommu@...ts.linux-foundation.org,
	Jean-Philippe.Brucker@....com, julien.grall@....com
Subject: Re: [PATCH v7 6/7] iommu/arm-smmu: do not advertise
 IOMMU_CAP_INTR_REMAP

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@...aro.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:
>