On 2018/7/25 6:46, Robin Murphy wrote:
> On 2018-07-12 7:18 AM, Zhen Lei wrote:
>> Because the non-strict mode introduces a vulnerability window, so add a
>> bootup option to make the manager can choose which mode to be used. The
>> default mode is IOMMU_STRICT.
>>
>> Signed-off-by: Zhen Lei <[email protected]>
>> ---
>> Documentation/admin-guide/kernel-parameters.txt | 12 ++++++++++
>> drivers/iommu/arm-smmu-v3.c | 32
>> ++++++++++++++++++++++---
>> 2 files changed, 41 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt
>> b/Documentation/admin-guide/kernel-parameters.txt
>> index efc7aa7..0cc80bc 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -1720,6 +1720,18 @@
>> nobypass [PPC/POWERNV]
>> Disable IOMMU bypass, using IOMMU for PCI devices.
>> + iommu_strict_mode= [arm-smmu-v3]
>
> If anything this should belong to iommu-dma, as that's where the actual
> decision of whether to use a flush queue or not happens. Also it would be
> nice to stick to the iommu.* option namespace in the hope of maintaining some
> consistency.
How about arm_iommu? like "s390_iommu" and "intel_iommu". After all it only
affect smmu now.
>
>> + 0 - strict mode
>> + Make sure all related TLBs to be invalidated before the
>> + memory released.
>> + 1 - non-strict mode
>> + Put off TLBs invalidation and release memory first. This mode
>> + introduces a vlunerability window, an untrusted device can
>> + access the reused memory because the TLBs may still valid.
>> + Please take full consideration before choosing this mode.
>> + Note that, VFIO is always use strict mode.
>> + others - strict mode
>> +
>> iommu.passthrough=
>> [ARM64] Configure DMA to bypass the IOMMU by default.
>> Format: { "0" | "1" }
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 4a198a0..9b72fc4 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -631,6 +631,24 @@ struct arm_smmu_option_prop {
>> { 0, NULL},
>> };
>> +static u32 iommu_strict_mode __read_mostly = IOMMU_STRICT;
>> +
>> +static int __init setup_iommu_strict_mode(char *str)
>> +{
>> + u32 strict_mode = IOMMU_STRICT;
>> +
>> + get_option(&str, &strict_mode);
>> + if (strict_mode == IOMMU_NON_STRICT) {
>> + iommu_strict_mode = strict_mode;
>> + pr_warn("WARNING: iommu non-strict mode is chose.\n"
>> + "It's good for scatter-gather performance but lacks full
>> isolation\n");
>> + add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
>> + }
>> +
>> + return 0;
>> +}
>> +early_param("iommu_strict_mode", setup_iommu_strict_mode);
>> +
>> static inline void __iomem *arm_smmu_page1_fixup(unsigned long offset,
>> struct arm_smmu_device *smmu)
>> {
>> @@ -1441,7 +1459,7 @@ static bool arm_smmu_capable(enum iommu_cap cap)
>> case IOMMU_CAP_NOEXEC:
>> return true;
>> case IOMMU_CAP_NON_STRICT:
>> - return true;
>> + return (iommu_strict_mode == IOMMU_NON_STRICT) ? true : false;
>
> Ugh. The "completely redundant ternany" idiom hurts my soul :(
OK, I will modify it.
>
> Robin.
>
>> default:
>> return false;
>> }
>> @@ -1750,6 +1768,14 @@ static int arm_smmu_attach_dev(struct iommu_domain
>> *domain, struct device *dev)
>> return ret;
>> }
>> +static u32 arm_smmu_strict_mode(struct iommu_domain *domain)
>> +{
>> + if (iommu_strict_mode == IOMMU_NON_STRICT)
>> + return IOMMU_DOMAIN_STRICT_MODE(domain);
>> +
>> + return IOMMU_STRICT;
>> +}
>> +
>> static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
>> phys_addr_t paddr, size_t size, int prot)
>> {
>> @@ -1769,7 +1795,7 @@ static int arm_smmu_map(struct iommu_domain *domain,
>> unsigned long iova,
>> if (!ops)
>> return 0;
>> - return ops->unmap(ops, iova | IOMMU_DOMAIN_STRICT_MODE(domain), size);
>> + return ops->unmap(ops, iova | arm_smmu_strict_mode(domain), size);
>> }
>> static void arm_smmu_flush_iotlb_all(struct iommu_domain *domain)
>> @@ -1784,7 +1810,7 @@ static void arm_smmu_iotlb_sync(struct iommu_domain
>> *domain)
>> {
>> struct arm_smmu_device *smmu = to_smmu_domain(domain)->smmu;
>> - if (smmu && (IOMMU_DOMAIN_STRICT_MODE(domain) == IOMMU_STRICT))
>> + if (smmu && (arm_smmu_strict_mode(domain) == IOMMU_STRICT))
>> __arm_smmu_tlb_sync(smmu);
>> }
>>
>
> .
>
--
Thanks!
BestRegards
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
