On Mon, Aug 19, 2019 at 07:19:29PM +0100, Robin Murphy wrote:
> TTBR1 values have so far been redundant since no users implement any
> support for split address spaces. Crucially, though, one of the main
> reasons for wanting to do so is to be able to manage each half entirely
> independently, e.g. context-switching one set of mappings without
> disturbing the other. Thus it seems unlikely that tying two tables
> together in a single io_pgtable_cfg would ever be particularly desirable
> or useful.
>
> Streamline the configs to just a single conceptual TTBR value
> representing the allocated table. This paves the way for future users to
> support split address spaces by simply allocating a table and dealing
> with the detailed TTBRn logistics themselves.
>
> Signed-off-by: Robin Murphy <[email protected]>
> ---
> drivers/iommu/arm-smmu-v3.c | 2 +-
> drivers/iommu/arm-smmu.c | 9 ++++-----
> drivers/iommu/io-pgtable-arm-v7s.c | 16 +++++++---------
> drivers/iommu/io-pgtable-arm.c | 7 +++----
> drivers/iommu/ipmmu-vmsa.c | 2 +-
> drivers/iommu/msm_iommu.c | 4 ++--
> drivers/iommu/mtk_iommu.c | 4 ++--
> drivers/iommu/qcom_iommu.c | 3 +--
> include/linux/io-pgtable.h | 4 ++--
> 9 files changed, 23 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 2a8db896d698..2e50cf49c3c4 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1722,7 +1722,7 @@ static int arm_smmu_domain_finalise_s1(struct
> arm_smmu_domain *smmu_domain,
> }
>
> cfg->cd.asid = (u16)asid;
> - cfg->cd.ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
> + cfg->cd.ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
> cfg->cd.tcr = pgtbl_cfg->arm_lpae_s1_cfg.tcr;
> cfg->cd.mair = pgtbl_cfg->arm_lpae_s1_cfg.mair;
> return 0;
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 184ca41e9de7..19030c4b5904 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -473,13 +473,12 @@ static void arm_smmu_init_context_bank(struct
> arm_smmu_domain *smmu_domain,
> /* TTBRs */
> if (stage1) {
> if (cfg->fmt == ARM_SMMU_CTX_FMT_AARCH32_S) {
> - cb->ttbr[0] = pgtbl_cfg->arm_v7s_cfg.ttbr[0];
> - cb->ttbr[1] = pgtbl_cfg->arm_v7s_cfg.ttbr[1];
> + cb->ttbr[0] = pgtbl_cfg->arm_v7s_cfg.ttbr;
> + cb->ttbr[1] = 0;
> } else {
> - cb->ttbr[0] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
> + cb->ttbr[0] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
> cb->ttbr[0] |= FIELD_PREP(TTBRn_ASID, cfg->asid);
> - cb->ttbr[1] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1];
> - cb->ttbr[1] |= FIELD_PREP(TTBRn_ASID, cfg->asid);
> + cb->ttbr[1] = FIELD_PREP(TTBRn_ASID, cfg->asid);
Why do you continue to put the ASID in here?
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index 34bb357b3cfa..de55b6d82ef1 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -247,10 +247,9 @@ static int qcom_iommu_init_domain(struct iommu_domain
> *domain,
>
> /* TTBRs */
> iommu_writeq(ctx, ARM_SMMU_CB_TTBR0,
> - pgtbl_cfg.arm_lpae_s1_cfg.ttbr[0] |
> + pgtbl_cfg.arm_lpae_s1_cfg.ttbr |
> FIELD_PREP(TTBRn_ASID, ctx->asid));
> iommu_writeq(ctx, ARM_SMMU_CB_TTBR1,
> - pgtbl_cfg.arm_lpae_s1_cfg.ttbr[1] |
> FIELD_PREP(TTBRn_ASID, ctx->asid));
Same here.
> diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
> index a6c8aa204733..7a0905d7a006 100644
> --- a/include/linux/io-pgtable.h
> +++ b/include/linux/io-pgtable.h
> @@ -90,7 +90,7 @@ struct io_pgtable_cfg {
> /* Low-level data specific to the table format */
> union {
> struct {
> - u64 ttbr[2];
> + u64 ttbr;
> u64 tcr;
> u64 mair;
> } arm_lpae_s1_cfg;
> @@ -101,7 +101,7 @@ struct io_pgtable_cfg {
> } arm_lpae_s2_cfg;
>
> struct {
> - u32 ttbr[2];
> + u32 ttbr;
We could probably do with a comment for these 'ttbr' field now saying that
they refer to ttbr0 (since the tcr will have EPD1 set).
Will
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
