On 03/05/2019 12:36, Marc Gonzalez wrote:
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.
That "Hardware [...] must ignore writes to the field" is the crux of the
matter here.
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.
In practice, this is a forwards-compatibility provision - the
architecture is effectively making a promise that if a
previously-reserved field becomes meaningful in a future version, 0 will
remain a "safe" value which does not enable any new and unexpected
behaviour that current software won't understand.
In this case, however, there is zero chance of fields in these
particular registers ever being defined, so I'm happy to take advantage
of assumptions about hardware's end of the bargain. Note that even the
architecture spec itself provides this example:
MOV R0,#SMMU_CBn_TLBSYNC
STR R0,[R0] ; Initiate TLB SYNC
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:
They're defined as write-only registers...
Even if the bits *within* a register nominally behave as RAZ/WI, I don't
think that gives you any guarantee that the register itself will
actually be readable (for starters, consider that the register as a
whole does *not* ignore writes, because its fundamental reason for
existing is to trigger an operation when written to).
Anyway, I'll clean up my patch and post it properly - thanks to you and
Bjorn for testing.
Robin.
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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
