* iommu/arm-smmu-v2 ASID/VMID calculation [not found] <198F501C-8D30-4EB5-BC40-4F40BB75D40B@caviumnetworks.com> @ 2016-01-25 17:03 ` Will Deacon 2016-01-26 0:48 ` Chalamarla, Tirumalesh 0 siblings, 1 reply; 8+ messages in thread From: Will Deacon @ 2016-01-25 17:03 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jan 21, 2016 at 06:52:34PM +0000, Chalamarla, Tirumalesh wrote: > Hi Will, Hello, > Current ASID/VMID calculation logic makes lot of assumption about internal TLB > implementation of SMMU, Not really. It makes assumptions that the hardware follows the architecture, which is hardly unreasonable as a starting point. > Systems like ThunderX have more than one smmu in the system and it can use same > TLBs with more than one of them and expects ASID to be unique ... but that's broken. If you built a system where the CPUs shared a TLB, you would run into issues as well. How does this work with things like arm_smmu_tlb_sync and the TLBGSTATUS register? > Current logic > > #define ARM_SMMU_CB_ASID(cfg) ((cfg)->cbndx) > #define ARM_SMMU_CB_VMID(cfg) ((cfg)->cbndx + 1) > > > Can this be replaced by something like > > > #define ARM_SMMU_CB_ASID(cfg, smmu) (((smmu)->idx << (smmu)->asid_shift) | (cfg)->cbndx) > #define ARM_SMMU_CB_VMID(cfg, smmu) (((smmu)->idx << (smmu)->vmid_shift) | (cfg)->cbndx + 1) > > > Idx and shift can be passed from device-tree. > > > Please let me know if this is acceptable, I will prepare a proper patch > and send to list. > > > If this is not acceptable through an alternative suggestion. If we're going to put something into the device-tree, then it should be an erratum property describing the offset to be applied to ASID/VMIDs. You also need to take care not to describe overlapping numberspaces. How many context banks do you implement in each SMMU? Will ^ permalink raw reply [flat|nested] 8+ messages in thread
* iommu/arm-smmu-v2 ASID/VMID calculation 2016-01-25 17:03 ` iommu/arm-smmu-v2 ASID/VMID calculation Will Deacon @ 2016-01-26 0:48 ` Chalamarla, Tirumalesh 2016-01-26 3:11 ` Chalamarla, Tirumalesh 0 siblings, 1 reply; 8+ messages in thread From: Chalamarla, Tirumalesh @ 2016-01-26 0:48 UTC (permalink / raw) To: linux-arm-kernel I don?t think it followed spec. The SMMU spec (IHI0062C) says (section 2.2): - The exact behavior of any TLB functionality is IMPLEMENTATION DEFINED Section 2.2 describes the constraints on sharing (e.g. multiple contexts using the same ASID says): "If multiple context banks have hte same attributes but describe different translations, the results of a TLB lookup are UNPREDICTABLE". The present smmu driver assumes ASID should only be unique per SMMU, this might not be true for all Implementations. On 1/25/16, 9:03 AM, "Will Deacon" <will.deacon@arm.com> wrote: >On Thu, Jan 21, 2016 at 06:52:34PM +0000, Chalamarla, Tirumalesh wrote: >> Hi Will, > >Hello, > >> Current ASID/VMID calculation logic makes lot of assumption about internal TLB >> implementation of SMMU, > >Not really. It makes assumptions that the hardware follows the architecture, >which is hardly unreasonable as a starting point. > >> Systems like ThunderX have more than one smmu in the system and it can use same >> TLBs with more than one of them and expects ASID to be unique > >... but that's broken. If you built a system where the CPUs shared a TLB, >you would run into issues as well. > >How does this work with things like arm_smmu_tlb_sync and the TLBGSTATUS >register? > >> Current logic >> >> #define ARM_SMMU_CB_ASID(cfg) ((cfg)->cbndx) >> #define ARM_SMMU_CB_VMID(cfg) ((cfg)->cbndx + 1) >> >> >> Can this be replaced by something like >> >> >> #define ARM_SMMU_CB_ASID(cfg, smmu) (((smmu)->idx << (smmu)->asid_shift) | (cfg)->cbndx) >> #define ARM_SMMU_CB_VMID(cfg, smmu) (((smmu)->idx << (smmu)->vmid_shift) | (cfg)->cbndx + 1) >> >> >> Idx and shift can be passed from device-tree. >> >> >> Please let me know if this is acceptable, I will prepare a proper patch >> and send to list. >> >> >> If this is not acceptable through an alternative suggestion. > >If we're going to put something into the device-tree, then it should be >an erratum property describing the offset to be applied to ASID/VMIDs. >You also need to take care not to describe overlapping numberspaces. I don?t think that is an errata. > >How many context banks do you implement in each SMMU? 128. > >Will ^ permalink raw reply [flat|nested] 8+ messages in thread
* iommu/arm-smmu-v2 ASID/VMID calculation 2016-01-26 0:48 ` Chalamarla, Tirumalesh @ 2016-01-26 3:11 ` Chalamarla, Tirumalesh 2016-01-26 11:48 ` Robin Murphy 2016-01-26 11:54 ` Will Deacon 0 siblings, 2 replies; 8+ messages in thread From: Chalamarla, Tirumalesh @ 2016-01-26 3:11 UTC (permalink / raw) To: linux-arm-kernel one my colleague points out, ASIDPNE also implementation defined. Thanks, Tirumalesh. On 1/25/16, 4:48 PM, "linux-arm-kernel on behalf of Chalamarla, Tirumalesh" <linux-arm-kernel-bounces at lists.infradead.org on behalf of Tirumalesh.Chalamarla@caviumnetworks.com> wrote: >I don?t think it followed spec. > >The SMMU spec (IHI0062C) says (section 2.2): > > - The exact behavior of any TLB functionality is IMPLEMENTATION DEFINED > >Section 2.2 describes the constraints on sharing (e.g. multiple contexts >using the same ASID says): > > "If multiple context banks have hte same attributes but describe > different translations, the results of a TLB lookup are UNPREDICTABLE". > > > >The present smmu driver assumes ASID should only be unique per SMMU, this might not be true for all >Implementations. > > > > >On 1/25/16, 9:03 AM, "Will Deacon" <will.deacon@arm.com> wrote: > >>On Thu, Jan 21, 2016 at 06:52:34PM +0000, Chalamarla, Tirumalesh wrote: >>> Hi Will, >> >>Hello, >> >>> Current ASID/VMID calculation logic makes lot of assumption about internal TLB >>> implementation of SMMU, >> >>Not really. It makes assumptions that the hardware follows the architecture, >>which is hardly unreasonable as a starting point. >> >>> Systems like ThunderX have more than one smmu in the system and it can use same >>> TLBs with more than one of them and expects ASID to be unique >> >>... but that's broken. If you built a system where the CPUs shared a TLB, >>you would run into issues as well. >> >>How does this work with things like arm_smmu_tlb_sync and the TLBGSTATUS >>register? >> >>> Current logic >>> >>> #define ARM_SMMU_CB_ASID(cfg) ((cfg)->cbndx) >>> #define ARM_SMMU_CB_VMID(cfg) ((cfg)->cbndx + 1) >>> >>> >>> Can this be replaced by something like >>> >>> >>> #define ARM_SMMU_CB_ASID(cfg, smmu) (((smmu)->idx << (smmu)->asid_shift) | (cfg)->cbndx) >>> #define ARM_SMMU_CB_VMID(cfg, smmu) (((smmu)->idx << (smmu)->vmid_shift) | (cfg)->cbndx + 1) >>> >>> >>> Idx and shift can be passed from device-tree. >>> >>> >>> Please let me know if this is acceptable, I will prepare a proper patch >>> and send to list. >>> >>> >>> If this is not acceptable through an alternative suggestion. >> >>If we're going to put something into the device-tree, then it should be >>an erratum property describing the offset to be applied to ASID/VMIDs. >>You also need to take care not to describe overlapping numberspaces. >I don?t think that is an errata. >> >>How many context banks do you implement in each SMMU? >128. >> >>Will >_______________________________________________ >linux-arm-kernel mailing list >linux-arm-kernel at lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* iommu/arm-smmu-v2 ASID/VMID calculation 2016-01-26 3:11 ` Chalamarla, Tirumalesh @ 2016-01-26 11:48 ` Robin Murphy 2016-01-27 19:05 ` Chalamarla, Tirumalesh 2016-01-26 11:54 ` Will Deacon 1 sibling, 1 reply; 8+ messages in thread From: Robin Murphy @ 2016-01-26 11:48 UTC (permalink / raw) To: linux-arm-kernel On 26/01/16 03:11, Chalamarla, Tirumalesh wrote: > one my colleague points out, ASIDPNE also implementation defined. ASIDPNE only covers whether the SMMU's TLBs may ignore broadcast _invalidation_ or not, not lookups. Broadcast TLB lookups are not a thing. > Thanks, > Tirumalesh. > > > > On 1/25/16, 4:48 PM, "linux-arm-kernel on behalf of Chalamarla, Tirumalesh" <linux-arm-kernel-bounces at lists.infradead.org on behalf of Tirumalesh.Chalamarla@caviumnetworks.com> wrote: > >> I don?t think it followed spec. >> >> The SMMU spec (IHI0062C) says (section 2.2): >> >> - The exact behavior of any TLB functionality is IMPLEMENTATION DEFINED >> >> Section 2.2 describes the constraints on sharing (e.g. multiple contexts >> using the same ASID says): >> >> "If multiple context banks have hte same attributes but describe >> different translations, the results of a TLB lookup are UNPREDICTABLE". Multiple context banks within the same SMMU, that is. >> The present smmu driver assumes ASID should only be unique per SMMU, this might not be true for all >> Implementations. Where "SMMU" implies "set of context banks behind a single programming interface", i.e. it should be true for all architecturally-compliant implementations. What if you take your system, fire up two cores with SMP disabled, and run an independent instance of Linux on each, with all the system resources divided up equally between them and no communication? If one OS using one SMMU can make the other OS using a different SMMU go wrong, then the hardware is simply broken. Sure, we can probably quirk ASID allocation in the driver, but it raises the question of what else we would need to do to keep SMMUs which believe they're supposed to be independent from DoS'ing each other.. Robin. >> On 1/25/16, 9:03 AM, "Will Deacon" <will.deacon@arm.com> wrote: >> >>> On Thu, Jan 21, 2016 at 06:52:34PM +0000, Chalamarla, Tirumalesh wrote: >>>> Hi Will, >>> >>> Hello, >>> >>>> Current ASID/VMID calculation logic makes lot of assumption about internal TLB >>>> implementation of SMMU, >>> >>> Not really. It makes assumptions that the hardware follows the architecture, >>> which is hardly unreasonable as a starting point. >>> >>>> Systems like ThunderX have more than one smmu in the system and it can use same >>>> TLBs with more than one of them and expects ASID to be unique >>> >>> ... but that's broken. If you built a system where the CPUs shared a TLB, >>> you would run into issues as well. >>> >>> How does this work with things like arm_smmu_tlb_sync and the TLBGSTATUS >>> register? >>> >>>> Current logic >>>> >>>> #define ARM_SMMU_CB_ASID(cfg) ((cfg)->cbndx) >>>> #define ARM_SMMU_CB_VMID(cfg) ((cfg)->cbndx + 1) >>>> >>>> >>>> Can this be replaced by something like >>>> >>>> >>>> #define ARM_SMMU_CB_ASID(cfg, smmu) (((smmu)->idx << (smmu)->asid_shift) | (cfg)->cbndx) >>>> #define ARM_SMMU_CB_VMID(cfg, smmu) (((smmu)->idx << (smmu)->vmid_shift) | (cfg)->cbndx + 1) >>>> >>>> >>>> Idx and shift can be passed from device-tree. >>>> >>>> >>>> Please let me know if this is acceptable, I will prepare a proper patch >>>> and send to list. >>>> >>>> >>>> If this is not acceptable through an alternative suggestion. >>> >>> If we're going to put something into the device-tree, then it should be >>> an erratum property describing the offset to be applied to ASID/VMIDs. >>> You also need to take care not to describe overlapping numberspaces. >> I don?t think that is an errata. >>> >>> How many context banks do you implement in each SMMU? >> 128. >>> >>> Will >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel at lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ > iommu mailing list > iommu at lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu > ^ permalink raw reply [flat|nested] 8+ messages in thread
* iommu/arm-smmu-v2 ASID/VMID calculation 2016-01-26 11:48 ` Robin Murphy @ 2016-01-27 19:05 ` Chalamarla, Tirumalesh 0 siblings, 0 replies; 8+ messages in thread From: Chalamarla, Tirumalesh @ 2016-01-27 19:05 UTC (permalink / raw) To: linux-arm-kernel On 1/26/16, 3:48 AM, "Robin Murphy" <robin.murphy@arm.com> wrote: >On 26/01/16 03:11, Chalamarla, Tirumalesh wrote: >> one my colleague points out, ASIDPNE also implementation defined. > >ASIDPNE only covers whether the SMMU's TLBs may ignore broadcast >_invalidation_ or not, not lookups. Broadcast TLB lookups are not a thing. Agreed. > >> Thanks, >> Tirumalesh. >> >> >> >> On 1/25/16, 4:48 PM, "linux-arm-kernel on behalf of Chalamarla, Tirumalesh" <linux-arm-kernel-bounces at lists.infradead.org on behalf of Tirumalesh.Chalamarla@caviumnetworks.com> wrote: >> >>> I don?t think it followed spec. >>> >>> The SMMU spec (IHI0062C) says (section 2.2): >>> >>> - The exact behavior of any TLB functionality is IMPLEMENTATION DEFINED >>> >>> Section 2.2 describes the constraints on sharing (e.g. multiple contexts >>> using the same ASID says): >>> >>> "If multiple context banks have hte same attributes but describe >>> different translations, the results of a TLB lookup are UNPREDICTABLE". > >Multiple context banks within the same SMMU, that is. > >>> The present smmu driver assumes ASID should only be unique per SMMU, this might not be true for all >>> Implementations. > >Where "SMMU" implies "set of context banks behind a single programming >interface", i.e. it should be true for all architecturally-compliant >implementations. > >What if you take your system, fire up two cores with SMP disabled, and >run an independent instance of Linux on each, with all the system >resources divided up equally between them and no communication? If one >OS using one SMMU can make the other OS using a different SMMU go wrong, >then the hardware is simply broken. Sure, we can probably quirk ASID >allocation in the driver, but it raises the question of what else we >would need to do to keep SMMUs which believe they're supposed to be >independent from DoS'ing each other.. This analogy is wrong, if 2 different entities running on two cores, SMMU is the least thing that needs to be worried. Rest aside they must cooperate with each other to coexists. that?s why we needed all the glory Virtualization. > >Robin. > >>> On 1/25/16, 9:03 AM, "Will Deacon" <will.deacon@arm.com> wrote: >>> >>>> On Thu, Jan 21, 2016 at 06:52:34PM +0000, Chalamarla, Tirumalesh wrote: >>>>> Hi Will, >>>> >>>> Hello, >>>> >>>>> Current ASID/VMID calculation logic makes lot of assumption about internal TLB >>>>> implementation of SMMU, >>>> >>>> Not really. It makes assumptions that the hardware follows the architecture, >>>> which is hardly unreasonable as a starting point. >>>> >>>>> Systems like ThunderX have more than one smmu in the system and it can use same >>>>> TLBs with more than one of them and expects ASID to be unique >>>> >>>> ... but that's broken. If you built a system where the CPUs shared a TLB, >>>> you would run into issues as well. >>>> >>>> How does this work with things like arm_smmu_tlb_sync and the TLBGSTATUS >>>> register? >>>> >>>>> Current logic >>>>> >>>>> #define ARM_SMMU_CB_ASID(cfg) ((cfg)->cbndx) >>>>> #define ARM_SMMU_CB_VMID(cfg) ((cfg)->cbndx + 1) >>>>> >>>>> >>>>> Can this be replaced by something like >>>>> >>>>> >>>>> #define ARM_SMMU_CB_ASID(cfg, smmu) (((smmu)->idx << (smmu)->asid_shift) | (cfg)->cbndx) >>>>> #define ARM_SMMU_CB_VMID(cfg, smmu) (((smmu)->idx << (smmu)->vmid_shift) | (cfg)->cbndx + 1) >>>>> >>>>> >>>>> Idx and shift can be passed from device-tree. >>>>> >>>>> >>>>> Please let me know if this is acceptable, I will prepare a proper patch >>>>> and send to list. >>>>> >>>>> >>>>> If this is not acceptable through an alternative suggestion. >>>> >>>> If we're going to put something into the device-tree, then it should be >>>> an erratum property describing the offset to be applied to ASID/VMIDs. >>>> You also need to take care not to describe overlapping numberspaces. >>> I don?t think that is an errata. >>>> >>>> How many context banks do you implement in each SMMU? >>> 128. >>>> >>>> Will >>> _______________________________________________ >>> linux-arm-kernel mailing list >>> linux-arm-kernel at lists.infradead.org >>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> _______________________________________________ >> iommu mailing list >> iommu at lists.linux-foundation.org >> https://lists.linuxfoundation.org/mailman/listinfo/iommu >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* iommu/arm-smmu-v2 ASID/VMID calculation 2016-01-26 3:11 ` Chalamarla, Tirumalesh 2016-01-26 11:48 ` Robin Murphy @ 2016-01-26 11:54 ` Will Deacon 2016-01-27 19:06 ` Chalamarla, Tirumalesh 1 sibling, 1 reply; 8+ messages in thread From: Will Deacon @ 2016-01-26 11:54 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jan 26, 2016 at 03:11:27AM +0000, Chalamarla, Tirumalesh wrote: > one my colleague points out, ASIDPNE also implementation defined. It looks pretty well defined to me, despite being a hint: This ASIDPNE indication is only a hint, and an SMMU can ignore it. In particular, if the SMMU implementation does not support the ASIDPNE feature, broadcast TLB invalidate operations ignore this hint and invalidate matching TLB entries when broadcast TLB invalidation is enabled. But what does this have to do with the problem at hand? > >I don?t think it followed spec. > > > >The SMMU spec (IHI0062C) says (section 2.2): > > > > - The exact behavior of any TLB functionality is IMPLEMENTATION DEFINED ... and immediately qualifies that with: The SMMU architecture does not limit how a TLB is implemented, provided it obeys the TLB Invalidate operations. which is to say, you can build the TLB how you like, providing that it follows the programmers model for invalidation. Otherwise portable software will break and people will send patches to "fix" it. > >Section 2.2 describes the constraints on sharing (e.g. multiple contexts > >using the same ASID says): > > > > "If multiple context banks have hte same attributes but describe > > different translations, the results of a TLB lookup are UNPREDICTABLE". That's describing a TLB conflict. These don't occur, because we ensure each of your 128 context banks has different attributes (i.e. ASID/VMID). The problem is that you have shared state between multiple SMMU instances, which I don't think is correct. I'm fine with a workaround, but I don't want this logic to be used on other implementations where it is not needed. > >The present smmu driver assumes ASID should only be unique per SMMU, this > >might not be true for all Implementations. So far, it appears to be true for all Implementations apart from yours and therefore needs to be quirked. Will ^ permalink raw reply [flat|nested] 8+ messages in thread
* iommu/arm-smmu-v2 ASID/VMID calculation 2016-01-26 11:54 ` Will Deacon @ 2016-01-27 19:06 ` Chalamarla, Tirumalesh 2016-01-27 19:08 ` Will Deacon 0 siblings, 1 reply; 8+ messages in thread From: Chalamarla, Tirumalesh @ 2016-01-27 19:06 UTC (permalink / raw) To: linux-arm-kernel On 1/26/16, 3:54 AM, "Will Deacon" <will.deacon@arm.com> wrote: >On Tue, Jan 26, 2016 at 03:11:27AM +0000, Chalamarla, Tirumalesh wrote: >> one my colleague points out, ASIDPNE also implementation defined. > >It looks pretty well defined to me, despite being a hint: > > This ASIDPNE indication is only a hint, and an SMMU can ignore it. In > particular, if the SMMU implementation does not support the ASIDPNE > feature, broadcast TLB invalidate operations ignore this hint and > invalidate matching TLB entries when broadcast TLB invalidation is > enabled. > >But what does this have to do with the problem at hand? > >> >I don?t think it followed spec. >> > >> >The SMMU spec (IHI0062C) says (section 2.2): >> > >> > - The exact behavior of any TLB functionality is IMPLEMENTATION DEFINED > >... and immediately qualifies that with: > > The SMMU architecture does not limit how a TLB is implemented, provided > it obeys the TLB Invalidate operations. > >which is to say, you can build the TLB how you like, providing that it >follows the programmers model for invalidation. Otherwise portable >software will break and people will send patches to "fix" it. > >> >Section 2.2 describes the constraints on sharing (e.g. multiple contexts >> >using the same ASID says): >> > >> > "If multiple context banks have hte same attributes but describe >> > different translations, the results of a TLB lookup are UNPREDICTABLE". > >That's describing a TLB conflict. These don't occur, because we ensure >each of your 128 context banks has different attributes (i.e. ASID/VMID). > >The problem is that you have shared state between multiple SMMU instances, >which I don't think is correct. I'm fine with a workaround, but I don't >want this logic to be used on other implementations where it is not >needed. Ok, I will treat it as Errata and provide a patch for workaround this. > >> >The present smmu driver assumes ASID should only be unique per SMMU, this >> >might not be true for all Implementations. > >So far, it appears to be true for all Implementations apart from yours >and therefore needs to be quirked. > >Will ^ permalink raw reply [flat|nested] 8+ messages in thread
* iommu/arm-smmu-v2 ASID/VMID calculation 2016-01-27 19:06 ` Chalamarla, Tirumalesh @ 2016-01-27 19:08 ` Will Deacon 0 siblings, 0 replies; 8+ messages in thread From: Will Deacon @ 2016-01-27 19:08 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jan 27, 2016 at 07:06:48PM +0000, Chalamarla, Tirumalesh wrote: > On 1/26/16, 3:54 AM, "Will Deacon" <will.deacon@arm.com> wrote: > >The problem is that you have shared state between multiple SMMU instances, > >which I don't think is correct. I'm fine with a workaround, but I don't > >want this logic to be used on other implementations where it is not > >needed. > > Ok, I will treat it as Errata and provide a patch for workaround this. Thank you. Will ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-01-27 19:08 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <198F501C-8D30-4EB5-BC40-4F40BB75D40B@caviumnetworks.com>
2016-01-25 17:03 ` iommu/arm-smmu-v2 ASID/VMID calculation Will Deacon
2016-01-26 0:48 ` Chalamarla, Tirumalesh
2016-01-26 3:11 ` Chalamarla, Tirumalesh
2016-01-26 11:48 ` Robin Murphy
2016-01-27 19:05 ` Chalamarla, Tirumalesh
2016-01-26 11:54 ` Will Deacon
2016-01-27 19:06 ` Chalamarla, Tirumalesh
2016-01-27 19:08 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).