Re: [PATCH] arm64/io: Don't use WZR in writel

[Marc Gonzalez <marc.w.gonzalez@free.fr>]

On 02/05/2019 18:50, Marc Gonzalez wrote:

> Are you saying that when writing to any of
> 
> 	gr0_base + ARM_SMMU_GR0_TLBIALLH
> 	gr0_base + ARM_SMMU_GR0_TLBIALLNSNH
> 	base + ARM_SMMU_GR0_sTLBGSYNC
> 
> the actual value written does not matter? Is it ignored by the HW?
> 
> We could write 0xdeadbeef?


http://infocenter.arm.com/help/topic/com.arm.doc.ihi0062d.c/IHI0062D_c_system_mmu_architecture_specification.pdf

> 0x068	SMMU_TLBIALLNSNH	WO	32	TLB Invalidate All Non-secure Non-Hyp
> 0x06C	SMMU_TLBIALLH		WO	32	TLB Invalidate All Hyp
> 0x070	SMMU_sTLBGSYNC		WO	32	Global Synchronize TLB Invalidate
> 
> The SMMU_TLBIALLNSNH	bit assignments are reserved.
> The SMMU_TLBIALLH	bit assignments are reserved.
> The SMMU_sTLBGSYNC	bit assignments are reserved.
> 
> 
> Reserved:
> 
> Unless otherwise stated in the architecture or product documentation, reserved:
> o Instruction and 32-bit system control register encodings are UNPREDICTABLE.
> o 64-bit system control register encodings are UNDEFINED.
> o Register bit fields are UNK/SBZP.
> 
> 
> UNK/SBZP:
> 
> Hardware must implement the field as Read-As-Zero, and must ignore writes to the field.
> 
> Software must not rely on the field reading as all 0s, and except for writing back to the register
> it must treat the value as if it is UNKNOWN. Software must use an SBZP policy to write to the field.
> 
> This description can apply to a single bit that should be written as its preserved value or as 0,
> or to a field that should be written as its preserved value or as all 0s.
> 
> See also Read-As-Zero (RAZ), Should-Be-Zero-or-Preserved (SBZP), and UNKNOWN.
> 
> 
> UNKNOWN:
> 
> An UNKNOWN value does not contain valid data, and can vary from moment to moment,
> instruction to instruction, and implementation to implementation.
> 
> An UNKNOWN value must not be a security hole. An UNKNOWN value must not be documented
> or promoted as having a defined value or effect.
> 
> When UNKNOWN appears in body text, it is always in SMALL CAPITALS.
> 
> 
> Should-Be-Zero-or-Preserved (SBZP)
> 
> From the introduction of ARMv7 Large Physical Address Extension, the definition of SBZP is modified
> for register bits that are SBZP in some but not all contexts. The generic definition of SBZP given
> here applies only to bits that are not affected by this modification.
> 
> Hardware must ignore writes to the field.
> 
> If software has read the field since the PE implementing the field was last reset and initialized,
> it must preserve the value of the field by writing the value that it previously read from the field.
> Otherwise, it must write the field as all 0s.
> 
> If software writes a value to the field that is not a value previously read for the field and is
> not all 0s, it must expect an UNPREDICTABLE result.


Considering the above, instead of writing any random value, what do
you think of writing back the current value, as in:


diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 930c07635956..fe27908d5295 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -417,13 +417,18 @@ static void __arm_smmu_free_bitmap(unsigned long *map, int idx)
 	clear_bit(idx, map);
 }
 
+static inline void sbzp_writel(void __iomem *reg)
+{
+	writel_relaxed(readl_relaxed(reg), reg);
+}
+
 /* Wait for any pending TLB invalidations to complete */
 static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu,
 				void __iomem *sync, void __iomem *status)
 {
 	unsigned int spin_cnt, delay;
 
-	writel_relaxed(0, sync);
+	sbzp_writel(sync);
 	for (delay = 1; delay < TLB_LOOP_TIMEOUT; delay *= 2) {
 		for (spin_cnt = TLB_SPIN_COUNT; spin_cnt > 0; spin_cnt--) {
 			if (!(readl_relaxed(status) & sTLBGSTATUS_GSACTIVE))
@@ -1761,8 +1766,8 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
 	}
 
 	/* Invalidate the TLB, just in case */
-	writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLH);
-	writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLNSNH);
+	sbzp_writel(gr0_base + ARM_SMMU_GR0_TLBIALLH);
+	sbzp_writel(gr0_base + ARM_SMMU_GR0_TLBIALLNSNH);
 
 	reg = readl_relaxed(ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel