[PATCH V2] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Robin Murphy
robin.murphy at arm.com
Wed Feb 24 05:38:56 PST 2016
On 23/02/16 23:56, Tirumalesh Chalamarla wrote:
>
>
> On 02/23/2016 04:19 AM, Robin Murphy wrote:
>> On 18/02/16 18:29, tchalamarla at caviumnetworks.com wrote:
>>> From: Tirumalesh Chalamarla <tchalamarla at caviumnetworks.com>
>>>
>>> Due to Errata#27704 CN88xx SMMUv2,supports only shared ASID and VMID
>>> namespaces; specifically within a given node SMMU0 and SMMU1 share,
>>> as does SMMU2 and SMMU3.
>>>
>>> This patch tries to address these issuee by supplying asid and vmid
>>> base from devicetree.
>>>
>>> changes from V1:
>>> - rebased on top of 16 bit VMID patch
>>> - removed redundent options from DT
>>> - insted of transform, DT now supplies starting ASID/VMID
>>>
>>> Signed-off-by: Akula Geethasowjanya
>>> <Geethasowjanya.Akula at caviumnetworks.com>
>>> Signed-off-by: Tirumalesh Chalamarla <tchalamarla at caviumnetworks.com>
>>> ---
>>> .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++
>>> drivers/iommu/arm-smmu.c | 37
>>> +++++++++++++++-------
>>> 2 files changed, 34 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> index bb7e569..80b8484 100644
>>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> @@ -57,6 +57,14 @@ conditions.
>>>
>>> - smmu-enable-vmid16 : Enable 16 bit VMID, if allowed.
>>>
>>> +- asid-base : Buggy SMMUv2 implementations which doesn't satisfy the
>>> + ASID namespace needs, use this field to specify starting
>>> + ASID for the SMMU.
>>> +
>>> +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfy
>>> the VMID
>>> + namespace needs, use this field to specify starting VMID
>>> + for the SMMU.
>>> +
>>
>> From how you've described the situation, this is still just some
>> arbitrary number for the sake of a driver workaround and not an actual
>> property of the hardware. Either way there should be a proper compatible
>> string for this implementation, e.g. something like "cavium,cn88xx-smmu".
>>
>> Given that, you then shouldn't actually need anything else, because you
>> can simply keep track of how many Cavium SMMUs have been probed and
>> automatically generate a suitable offset, e.g. (smmu->index *
>> ARM_SMMU_MAX_CBS) to ensure global ASID/VMID uniqueness.
>
> This particular solution might not be accurate in all cases. what
> happens if SMMU stage 1 is used by a guest?
Even if we supported passing through stage 1 contexts, which we don't, a
malicious guest can freely ignore your arbitrary numbers in the DT, put
whatever nonsense it feels like in the TTBRs, and corrupt I/O in other
guests and/or the host.
You cannot safely pass through raw stage 1 contexts on this hardware, so
it's a non-issue.
Robin.
>>> Example:
>>>
>>> smmu {
>>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>>> index 003c442..dc46b9a 100644
>>> --- a/drivers/iommu/arm-smmu.c
>>> +++ b/drivers/iommu/arm-smmu.c
>>> @@ -320,6 +320,9 @@ struct arm_smmu_device {
>>> unsigned long ipa_size;
>>> unsigned long pa_size;
>>>
>>> + u32 asid_base;
>>> + u32 vmid_base;
>>> +
>>> u32 num_global_irqs;
>>> u32 num_context_irqs;
>>> unsigned int *irqs;
>>> @@ -335,8 +338,8 @@ struct arm_smmu_cfg {
>>> };
>>> #define INVALID_IRPTNDX 0xff
>>>
>>> -#define ARM_SMMU_CB_ASID(cfg) ((cfg)->cbndx)
>>> -#define ARM_SMMU_CB_VMID(cfg) ((cfg)->cbndx + 1)
>>> +#define ARM_SMMU_CB_ASID(smmu, cfg) ((u16)((smmu)->asid_base +
>>> (cfg)->cbndx))
>>> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)((smmu)->vmid_base +
>>> (cfg)->cbndx))
>>>
>>> enum arm_smmu_domain_stage {
>>> ARM_SMMU_DOMAIN_S1 = 0,
>>> @@ -576,11 +579,11 @@ static void arm_smmu_tlb_inv_context(void *cookie)
>>>
>>> if (stage1) {
>>> base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
>>> - writel_relaxed(ARM_SMMU_CB_ASID(cfg),
>>> + writel_relaxed(ARM_SMMU_CB_ASID(smmu, cfg),
>>> base + ARM_SMMU_CB_S1_TLBIASID);
>>> } else {
>>> base = ARM_SMMU_GR0(smmu);
>>> - writel_relaxed(ARM_SMMU_CB_VMID(cfg),
>>> + writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg),
>>> base + ARM_SMMU_GR0_TLBIVMID);
>>> }
>>>
>>> @@ -602,7 +605,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned
>>> long iova, size_t size,
>>>
>>> if (!IS_ENABLED(CONFIG_64BIT) || smmu->version ==
>>> ARM_SMMU_V1) {
>>> iova &= ~12UL;
>>> - iova |= ARM_SMMU_CB_ASID(cfg);
>>> + iova |= ARM_SMMU_CB_ASID(smmu, cfg);
>>> do {
>>> writel_relaxed(iova, reg);
>>> iova += granule;
>>> @@ -610,7 +613,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned
>>> long iova, size_t size,
>>> #ifdef CONFIG_64BIT
>>> } else {
>>> iova >>= 12;
>>> - iova |= (u64)ARM_SMMU_CB_ASID(cfg) << 48;
>>> + iova |= (u64)ARM_SMMU_CB_ASID(smmu, cfg) << 48;
>>> do {
>>> writeq_relaxed(iova, reg);
>>> iova += granule >> 12;
>>> @@ -630,7 +633,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned
>>> long iova, size_t size,
>>> #endif
>>> } else {
>>> reg = ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_TLBIVMID;
>>> - writel_relaxed(ARM_SMMU_CB_VMID(cfg), reg);
>>> + writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg), reg);
>>> }
>>> }
>>>
>>> @@ -744,7 +747,7 @@ static void arm_smmu_init_context_bank(struct
>>> arm_smmu_domain *smmu_domain,
>>> #endif
>>> /* if 16bit VMID required set VMID in CBA2R */
>>> if (smmu->options & ARM_SMMU_OPT_ENABLE_VMID16)
>>> - reg |= ARM_SMMU_CB_VMID(cfg) << CBA2R_VMID_SHIFT;
>>> + reg |= ARM_SMMU_CB_VMID(smmu, cfg) << CBA2R_VMID_SHIFT;
>>>
>>> writel_relaxed(reg, gr1_base +
>>> ARM_SMMU_GR1_CBA2R(cfg->cbndx));
>>> }
>>> @@ -763,7 +766,7 @@ static void arm_smmu_init_context_bank(struct
>>> arm_smmu_domain *smmu_domain,
>>> (CBAR_S1_MEMATTR_WB << CBAR_S1_MEMATTR_SHIFT);
>>> } else if (!(smmu->options & ARM_SMMU_OPT_ENABLE_VMID16)) {
>>> /*16 bit VMID is not enabled set 8 bit VMID here */
>>> - reg |= ARM_SMMU_CB_VMID(cfg) << CBAR_VMID_SHIFT;
>>> + reg |= ARM_SMMU_CB_VMID(smmu, cfg) << CBAR_VMID_SHIFT;
>>> }
>>> writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBAR(cfg->cbndx));
>>>
>>> @@ -771,11 +774,11 @@ static void arm_smmu_init_context_bank(struct
>>> arm_smmu_domain *smmu_domain,
>>> if (stage1) {
>>> reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
>>>
>>> - reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << TTBRn_ASID_SHIFT;
>>> + reg64 |= ((u64)ARM_SMMU_CB_ASID(smmu, cfg)) <<
>>> TTBRn_ASID_SHIFT;
>>> smmu_writeq(reg64, cb_base + ARM_SMMU_CB_TTBR0);
>>>
>>> reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1];
>>> - reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << TTBRn_ASID_SHIFT;
>>> + reg64 |= ((u64)ARM_SMMU_CB_ASID(smmu, cfg)) <<
>>> TTBRn_ASID_SHIFT;
>>> smmu_writeq(reg64, cb_base + ARM_SMMU_CB_TTBR1);
>>> } else {
>>> reg64 = pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
>>> @@ -1812,6 +1815,18 @@ static int arm_smmu_device_dt_probe(struct
>>> platform_device *pdev)
>>>
>>> parse_driver_options(smmu);
>>>
>>> + if (of_property_read_u32(dev->of_node, "#asid-base",
>>> + &smmu->asid_base)) {
>>> + smmu->asid_base = 0;
>>> + }
>>> +
>>> + if (of_property_read_u32(dev->of_node, "#vmid-base",
>>> + &smmu->vmid_base)) {
>>> + smmu->vmid_base = 1;
>>> + }
>>> + if (smmu->vmid_base == 0)
>>> + smmu->vmid_base = 1;
>>> +
>>> if (smmu->version > ARM_SMMU_V1 &&
>>> smmu->num_context_banks != smmu->num_context_irqs) {
>>> dev_err(dev,
>>>
>>
>
More information about the linux-arm-kernel
mailing list