Re: [PATCH RFC 16/19] iommu/arm-smmu-v3: Enable HTTU for stage1 with io-pgtable mapping

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022-04-29 13:10, Joao Martins wrote:
On 4/29/22 12:35, Robin Murphy wrote:
On 2022-04-28 22:09, Joao Martins wrote:
From: Kunkun Jiang <jiangkunkun@xxxxxxxxxx>

As nested mode is not upstreamed now, we just aim to support dirty
log tracking for stage1 with io-pgtable mapping (means not support
SVA mapping). If HTTU is supported, we enable HA/HD bits in the SMMU
CD and transfer ARM_HD quirk to io-pgtable.

We additionally filter out HD|HA if not supportted. The CD.HD bit
is not particularly useful unless we toggle the DBM bit in the PTE
entries.

Co-developed-by: Keqian Zhu <zhukeqian1@xxxxxxxxxx>
Signed-off-by: Keqian Zhu <zhukeqian1@xxxxxxxxxx>
Signed-off-by: Kunkun Jiang <jiangkunkun@xxxxxxxxxx>
[joaomart:Convey HD|HA bits over to the context descriptor
   and update commit message]
Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
---
   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 +++++++++++
   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  3 +++
   include/linux/io-pgtable.h                  |  1 +
   3 files changed, 15 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 1ca72fcca930..5f728f8f20a2 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1077,10 +1077,18 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
   		 * this substream's traffic
   		 */
   	} else { /* (1) and (2) */
+		struct arm_smmu_device *smmu = smmu_domain->smmu;
+		u64 tcr = cd->tcr;
+
   		cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
   		cdptr[2] = 0;
   		cdptr[3] = cpu_to_le64(cd->mair);
+ if (!(smmu->features & ARM_SMMU_FEAT_HD))
+			tcr &= ~CTXDESC_CD_0_TCR_HD;
+		if (!(smmu->features & ARM_SMMU_FEAT_HA))
+			tcr &= ~CTXDESC_CD_0_TCR_HA;

This is very backwards...

Yes.

+
   		/*
   		 * STE is live, and the SMMU might read dwords of this CD in any
   		 * order. Ensure that it observes valid values before reading
@@ -2100,6 +2108,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
   			  FIELD_PREP(CTXDESC_CD_0_TCR_ORGN0, tcr->orgn) |
   			  FIELD_PREP(CTXDESC_CD_0_TCR_SH0, tcr->sh) |
   			  FIELD_PREP(CTXDESC_CD_0_TCR_IPS, tcr->ips) |
+			  CTXDESC_CD_0_TCR_HA | CTXDESC_CD_0_TCR_HD |

...these should be set in io-pgtable's TCR value *if* io-pgatble is
using DBM, then propagated through from there like everything else.


So the DBM bit superseedes the TCR bit -- that's strage? say if you mark a PTE as
writeable-clean with DBM set but TCR.HD unset .. then  won't trigger a perm-fault?
I need to re-read that section of the manual, as I didn't get the impression above.

No, architecturally, the {TCR,CD}.HD bit is still the "master switch" for whether the DBM field in PTEs is interpreted or not, but in terms of our abstraction, we only need to care about setting HD if io-pgtable is actually going to want to use DBM, so we may as well leave it to io-pgtable to tell us canonically. The logical interface here in general is that we use the initial io_pgtable_cfg to tell it what it *can* use, but then we read back afterwards to see exactly what it has chosen to do, and I think HA/HD also fit perfectly into that paradigm.

Robin.

   			  CTXDESC_CD_0_TCR_EPD1 | CTXDESC_CD_0_AA64;
   	cfg->cd.mair	= pgtbl_cfg->arm_lpae_s1_cfg.mair;
@@ -2203,6 +2212,8 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
   		.iommu_dev	= smmu->dev,
   	};
+ if (smmu->features & ARM_SMMU_FEAT_HD)
+		pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_ARM_HD;

You need to depend on ARM_SMMU_FEAT_COHERENCY for this as well, not
least because you don't have any of the relevant business for
synchronising non-coherent PTEs in your walk functions, but it's also
implementation-defined whether HTTU even operates on non-cacheable
pagetables, and frankly you just don't want to go there ;)

/me nods OK.

Robin.

   	if (smmu->features & ARM_SMMU_FEAT_BBML1)
   		pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_ARM_BBML1;
   	else if (smmu->features & ARM_SMMU_FEAT_BBML2)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index e15750be1d95..ff32242f2fdb 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -292,6 +292,9 @@
   #define CTXDESC_CD_0_TCR_IPS		GENMASK_ULL(34, 32)
   #define CTXDESC_CD_0_TCR_TBI0		(1ULL << 38)
+#define CTXDESC_CD_0_TCR_HA (1UL << 43)
+#define CTXDESC_CD_0_TCR_HD            (1UL << 42)
+
   #define CTXDESC_CD_0_AA64		(1UL << 41)
   #define CTXDESC_CD_0_S			(1UL << 44)
   #define CTXDESC_CD_0_R			(1UL << 45)
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index d7626ca67dbf..a11902ae9cf1 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -87,6 +87,7 @@ struct io_pgtable_cfg {
   	#define IO_PGTABLE_QUIRK_ARM_OUTER_WBWA	BIT(6)
   	#define IO_PGTABLE_QUIRK_ARM_BBML1      BIT(7)
   	#define IO_PGTABLE_QUIRK_ARM_BBML2      BIT(8)
+	#define IO_PGTABLE_QUIRK_ARM_HD         BIT(9)
unsigned long quirks;
   	unsigned long			pgsize_bitmap;



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux