public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: robin.murphy@arm•com (Robin Murphy)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 2/4] iommu/arm-smmu: Allow disabling unmatched stream bypass
Date: Wed, 10 Feb 2016 12:10:28 +0000	[thread overview]
Message-ID: <56BB28B4.2040406@arm.com> (raw)
In-Reply-To: <20160209140631.GM22874@arm.com>

On 09/02/16 14:06, Will Deacon wrote:
> Hi Robin,
>
> On Tue, Jan 26, 2016 at 06:06:35PM +0000, Robin Murphy wrote:
>> Borrow the disable_bypass parameter from the SMMUv3 driver as a handy
>> debugging/security feature so that unmatched stream IDs (i.e. devices
>> not attached to an IOMMU domain) may be configured to fault.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm•com>
>> ---
>>   drivers/iommu/arm-smmu.c | 32 +++++++++++++++++++++++---------
>>   1 file changed, 23 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 1f9093d..d1b7dc1 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -141,6 +141,8 @@
>>   #define ID2_PTFS_16K			(1 << 13)
>>   #define ID2_PTFS_64K			(1 << 14)
>>
>> +#define sGFSR_USF			(1 << 1)
>> +
>>   /* Global TLB invalidation */
>>   #define ARM_SMMU_GR0_TLBIVMID		0x64
>>   #define ARM_SMMU_GR0_TLBIALLNSNH	0x68
>> @@ -263,6 +265,10 @@ static int force_stage;
>>   module_param_named(force_stage, force_stage, int, S_IRUGO);
>>   MODULE_PARM_DESC(force_stage,
>>   	"Force SMMU mappings to be installed at a particular stage of translation. A value of '1' or '2' forces the corresponding stage. All other values are ignored (i.e. no stage is forced). Note that selecting a specific stage will disable support for nested translation.");
>> +static bool disable_bypass;
>> +module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
>> +MODULE_PARM_DESC(disable_bypass,
>> +	"Disable bypass streams such that incoming transactions from devices that are not attached to an iommu domain will report an abort back to the device and will not be allowed to pass through the SMMU.");
>>
>>   enum arm_smmu_arch_version {
>>   	ARM_SMMU_V1 = 1,
>> @@ -704,8 +710,12 @@ static irqreturn_t arm_smmu_global_fault(int irq, void *dev)
>>   	if (!gfsr)
>>   		return IRQ_NONE;
>>
>> -	dev_err_ratelimited(smmu->dev,
>> -		"Unexpected global fault, this could be serious\n");
>> +	if (gfsr & sGFSR_USF)
>> +		dev_err_ratelimited(smmu->dev, "Unmatched stream fault: ID 0x%x\n",
>> +				    gfsynr1 & SMR_ID_MASK);
>> +	else
>
> I think I'd rather drop this message -- a global error is still indicative
> that something has gone horriby wrong, and we already print the gsynr
> registers below. Furthermore, if we take multiple faults, it might look
> like they're all exclusively due to unmatched streams.
>
> On top of that, I'm not convinced that USF will be set for an SMMU that
> doesn't implement stream-matching (because it will match an S2CR of type
> FAULT).

You're quite right, by the look of it the stream indexing case should 
report ICF rather than USF. My rationale for special-casing the message 
was that it's less of an "unexpected" fault when it's something you've 
specifically asked the driver for, but it also seems reasonable that 
someone turning on such an "I know what I'm doing" feature might be able 
to decode GFSR themselves. Considering the machinations of matching 
!MULTI and USF or ICF as appropriate just for the sake of printing a 
slightly reworded message, I'll drop this hunk.

> Other than that, looks good.

Thanks,
Robin.

  reply	other threads:[~2016-02-10 12:10 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-26 18:06 [PATCH 0/4] Miscellaneous ARM SMMU patches Robin Murphy
2016-01-26 18:06 ` [PATCH 1/4] iommu/arm-smmu: Treat all device transactions as unprivileged Robin Murphy
2016-01-27  6:00   ` Anup Patel
2016-02-09 14:08   ` Will Deacon
2016-01-26 18:06 ` [PATCH 2/4] iommu/arm-smmu: Allow disabling unmatched stream bypass Robin Murphy
2016-02-09 14:06   ` Will Deacon
2016-02-10 12:10     ` Robin Murphy [this message]
2016-02-10 14:25     ` [PATCH v2] " Robin Murphy
2016-01-26 18:06 ` [PATCH 3/4] iommu/arm-smmu: Support DMA-API domains Robin Murphy
2016-01-26 18:06 ` [PATCH 4/4] iommu/arm-smmu: Use per-context TLB sync as appropriate Robin Murphy
2016-02-09 14:15   ` Will Deacon
2016-02-10 11:58     ` Robin Murphy
2016-02-09 14:16 ` [PATCH 0/4] Miscellaneous ARM SMMU patches Will Deacon

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=56BB28B4.2040406@arm.com \
    --to=robin.murphy@arm$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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