linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values
@ 2021-10-15 16:14 James Morse
  2021-10-15 16:14 ` [RFC PATCH 1/7] KVM: arm64: Detect and enable PBHA for stage2 James Morse
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: James Morse @ 2021-10-15 16:14 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm; +Cc: Will Deacon, Catalin Marinas, Marc Zyngier

Hello!

Do you have hardware that uses PBHA? If so, what does the SoC do in response to
which bits, and what workload needs that behaviour?

This series is a start at trying to work out what linux needs to support to makeuse of existing SoCs using PBHA.


PBHA is a feature that adds an extra four bits to each read or write transaction
for the SoC implementer to do - whatever they like with! An obvious example
would be a hint for whether the access should allocate (or not) in the system
cache. The hint would allow better performance for some SoC specific workloads.

The arm-arm doesn't define what the bits do, only where in the page tables they
come from. It also doesn't define how these bits are combined between stage1
and stage2.

It appears that all of Arm's Cortex cores with the feature do the stage1+stage2
combining in the same way. (stage2 wins). Patch 1 turns PBHA on for stage2,
where KVM only generates the safe default value. This stops KVM guests from
using PBHA on Cortex cores. It should be harmless for any core that does not
behave like this.


The remaining patches allow firmware to describe which PBHA bits only affect
performance, and which have dangerous side effects like encryption or other
forms of corruption, that would mean the OS has to ensure all aliases are
removed.

The lists exist to allow an OS to avoid the cost of rewriting aliases when that
isn't necessary, and for KVM to determine which bits it can enable for a guest:
KVM uses the 'performance only' list to try and enable the corresponding bits
for KVM guests - but only if they can't be used to generate a value not in the
list.
This depends on knowing the CPU implements the 'stage2 wins' behaviour. I've
listed the CPUs whose TRMs describe this behaviour, and asked for other TRMs to
be updated to say what the behaviour is.

A pgprot_pbha() helper is added to allow drivers to request the 'performance
only' kind of PBHA bit for a mapping.


Supporting the 'no aliases' kind is much more involved. I've not tried to do
this. (do we need to?)

I don't have a platform that uses any of this, so I can't detect whether or not
the PBHA values were generated with the read/writes.


Thanks,

James Morse (7):
  KVM: arm64: Detect and enable PBHA for stage2
  dt-bindings: Rename the description of cpu nodes cpu.yaml
  dt-bindings: arm: Add binding for Page Based Hardware Attributes
  arm64: cpufeature: Enable PBHA bits for stage1
  arm64: mm: Add pgprot_pbha() to allow drivers to request PBHA values
  KVM: arm64: Configure PBHA bits for stage2
  Documentation: arm64: Describe the support and expectations for PBHA

 Documentation/arm64/index.rst                 |   1 +
 Documentation/arm64/pbha.rst                  |  86 +++
 .../devicetree/bindings/arm/cpu.yaml          | 544 ++++++++++++++++
 .../devicetree/bindings/arm/cpus.yaml         | 591 ++----------------
 arch/arm64/Kconfig                            |  13 +
 arch/arm64/include/asm/cpufeature.h           |   1 +
 arch/arm64/include/asm/cputype.h              |   4 +
 arch/arm64/include/asm/kvm_arm.h              |   1 +
 arch/arm64/include/asm/kvm_pgtable.h          |   9 +
 arch/arm64/include/asm/pgtable-hwdef.h        |   5 +
 arch/arm64/include/asm/pgtable.h              |  12 +
 arch/arm64/kernel/cpufeature.c                | 196 ++++++
 arch/arm64/kernel/image-vars.h                |   3 +
 arch/arm64/kvm/hyp/pgtable.c                  |  15 +-
 arch/arm64/tools/cpucaps                      |   3 +
 15 files changed, 961 insertions(+), 523 deletions(-)
 create mode 100644 Documentation/arm64/pbha.rst
 create mode 100644 Documentation/devicetree/bindings/arm/cpu.yaml

-- 
2.30.2


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 1/7] KVM: arm64: Detect and enable PBHA for stage2
  2021-10-15 16:14 [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values James Morse
@ 2021-10-15 16:14 ` James Morse
  2021-10-16 13:27   ` Marc Zyngier
  2021-10-15 16:14 ` [RFC PATCH 2/7] dt-bindings: Rename the description of cpu nodes cpu.yaml James Morse
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: James Morse @ 2021-10-15 16:14 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm; +Cc: Will Deacon, Catalin Marinas, Marc Zyngier

Page Based Hardware Attributes (PBHA, aka HPDS2) allow a page table entry
to specify up to four bits that can be used by the hardware for some
implementation defined purpose.

This is a problem for KVM guests as the host may swap guest memory using
a different combination of PBHA bits than the guest used when writing the
data. Without knowing what the PBHA bits do, its not possible to know if
this will corrupt the guest's data.

The arm-arm doesn't describe how the PBHA bits are combined between stage1
and stage2. Arm's Cortex CPUs appear to all do the same thing: stage2 wins.

Enable PBHA for stage2, where the configured value is zero. This has no
effect if PBHA isn't in use. On Cortex cores that have the 'stage2 wins'
behaviour, this disables whatever the guest may be doing. For any other
core with a sensible combination policy, it should be harmless.

Signed-off-by: James Morse <james.morse@arm.com>
---
I've checked the TRMs for Neoverse-N1, and Cortexs: A76, A77, A78 and X1.
They all have this 'stage2 wins' behaviour. The behaviour isn't documented
by A510 or A710's TRM.
---
 arch/arm64/include/asm/kvm_arm.h     | 1 +
 arch/arm64/include/asm/kvm_pgtable.h | 9 +++++++++
 arch/arm64/kernel/cpufeature.c       | 9 +++++++++
 arch/arm64/kvm/hyp/pgtable.c         | 9 +++++++++
 arch/arm64/tools/cpucaps             | 1 +
 5 files changed, 29 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 327120c0089f..bab7f0ad3724 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -126,6 +126,7 @@
 #define VTCR_EL2_VS_SHIFT	19
 #define VTCR_EL2_VS_8BIT	(0 << VTCR_EL2_VS_SHIFT)
 #define VTCR_EL2_VS_16BIT	(1 << VTCR_EL2_VS_SHIFT)
+#define VTCR_EL2_PBHA_MASK	GENMASK(28, 25)
 
 #define VTCR_EL2_T0SZ(x)	TCR_T0SZ(x)
 
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 027783829584..678bff4bfd7f 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -125,6 +125,10 @@ enum kvm_pgtable_stage2_flags {
  * @KVM_PGTABLE_PROT_SW1:	Software bit 1.
  * @KVM_PGTABLE_PROT_SW2:	Software bit 2.
  * @KVM_PGTABLE_PROT_SW3:	Software bit 3.
+ * @KVM_PGTABLE_PROT_PBHA0:	Page-Based Hardware Attribute 0.
+ * @KVM_PGTABLE_PROT_PBHA1:	Page-Based Hardware Attribute 1.
+ * @KVM_PGTABLE_PROT_PBHA2:	Page-Based Hardware Attribute 2.
+ * @KVM_PGTABLE_PROT_PBHA3:	Page-Based Hardware Attribute 3.
  */
 enum kvm_pgtable_prot {
 	KVM_PGTABLE_PROT_X			= BIT(0),
@@ -137,6 +141,11 @@ enum kvm_pgtable_prot {
 	KVM_PGTABLE_PROT_SW1			= BIT(56),
 	KVM_PGTABLE_PROT_SW2			= BIT(57),
 	KVM_PGTABLE_PROT_SW3			= BIT(58),
+
+	KVM_PGTABLE_PROT_PBHA0			= BIT(59),
+	KVM_PGTABLE_PROT_PBHA1			= BIT(60),
+	KVM_PGTABLE_PROT_PBHA2			= BIT(61),
+	KVM_PGTABLE_PROT_PBHA3			= BIT(62),
 };
 
 #define KVM_PGTABLE_PROT_RW	(KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index f8a3067d10c6..8694f9dec5e5 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2328,6 +2328,15 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.matches = has_cpuid_feature,
 		.min_field_value = 1,
 	},
+	{
+		.capability = ARM64_HAS_PBHA,
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.sys_reg = SYS_ID_AA64MMFR1_EL1,
+		.sign = FTR_UNSIGNED,
+		.field_pos = ID_AA64MMFR1_HPD_SHIFT,
+		.matches = has_cpuid_feature,
+		.min_field_value = 2,
+	},
 	{},
 };
 
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index f8ceebe4982e..7bd90ea1c61f 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -540,6 +540,15 @@ u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)
 	 */
 	vtcr |= VTCR_EL2_HA;
 
+	/*
+	 * Enable PBHA for stage2 on systems that support it. The configured
+	 * value will always be 0, which is defined as the safe default
+	 * setting. On Cortex cores, enabling PBHA for stage2 effectively
+	 * disables it for stage1.
+	 */
+	if (cpus_have_final_cap(ARM64_HAS_PBHA))
+		vtcr = FIELD_PREP(VTCR_EL2_PBHA_MASK, 0xf);
+
 	/* Set the vmid bits */
 	vtcr |= (get_vmid_bits(mmfr1) == 16) ?
 		VTCR_EL2_VS_16BIT :
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 49305c2e6dfd..132596d8b518 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -28,6 +28,7 @@ HAS_LSE_ATOMICS
 HAS_NO_FPSIMD
 HAS_NO_HW_PREFETCH
 HAS_PAN
+HAS_PBHA
 HAS_RAS_EXTN
 HAS_RNG
 HAS_SB
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/7] dt-bindings: Rename the description of cpu nodes cpu.yaml
  2021-10-15 16:14 [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values James Morse
  2021-10-15 16:14 ` [RFC PATCH 1/7] KVM: arm64: Detect and enable PBHA for stage2 James Morse
@ 2021-10-15 16:14 ` James Morse
  2021-10-15 16:14 ` [RFC PATCH 3/7] dt-bindings: arm: Add binding for Page Based Hardware Attributes James Morse
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: James Morse @ 2021-10-15 16:14 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm; +Cc: Will Deacon, Catalin Marinas, Marc Zyngier

The cpus.yaml file describes the cpu nodes, not the cpus node.
Rename it to allow integration properties of all the cpus to be described
in the cpus node.

Signed-off-by: James Morse <james.morse@arm.com>
---
 Documentation/devicetree/bindings/arm/{cpus.yaml => cpu.yaml} | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 rename Documentation/devicetree/bindings/arm/{cpus.yaml => cpu.yaml} (99%)

diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpu.yaml
similarity index 99%
rename from Documentation/devicetree/bindings/arm/cpus.yaml
rename to Documentation/devicetree/bindings/arm/cpu.yaml
index 9a2432a88074..445cb366a101 100644
--- a/Documentation/devicetree/bindings/arm/cpus.yaml
+++ b/Documentation/devicetree/bindings/arm/cpu.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/arm/cpus.yaml#
+$id: http://devicetree.org/schemas/arm/cpu.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: ARM CPUs bindings
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 3/7] dt-bindings: arm: Add binding for Page Based Hardware Attributes
  2021-10-15 16:14 [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values James Morse
  2021-10-15 16:14 ` [RFC PATCH 1/7] KVM: arm64: Detect and enable PBHA for stage2 James Morse
  2021-10-15 16:14 ` [RFC PATCH 2/7] dt-bindings: Rename the description of cpu nodes cpu.yaml James Morse
@ 2021-10-15 16:14 ` James Morse
  2021-10-15 16:14 ` [RFC PATCH 4/7] arm64: cpufeature: Enable PBHA bits for stage1 James Morse
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: James Morse @ 2021-10-15 16:14 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm; +Cc: Will Deacon, Catalin Marinas, Marc Zyngier

ARM CPUs with the FEAT_HPDS2 feature allow an IMPLEMENTATION DEFINED
hardware attribute to be encoded in the leaf page table entries and
sent with any transaction that makes an access via that entry.

Some designs are using these bits as a hint to the system-cache
that it should apply a particular policy to this access. e.g. to
prioritise the caching of particular workload data.

The arm-arm doesn't define what these bits mean. Implementations
could use this to encrypt, or otherwise corrupt data. Setting an
'incorrect' value may lead to correctness or coherency issues.
The arm-arm only defines '0' as a safe default value.

As there are only four bits, it is likely these will be combined
and treated as a four-bit value by some hardware. This binding
expects values. Using values allows firmware to describe that two
bits should not be set at the same time.

To allow these hints to be used, add a way of describing which
values only have a performance impact, and which can only be
used if all mappings use the same PBHA value. This goes in the
cpus node binding, as it must be the same for all CPUs.

Signed-off-by: James Morse <james.morse@arm.com>
---
 .../devicetree/bindings/arm/cpus.yaml         | 91 +++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cpus.yaml

diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml
new file mode 100644
index 000000000000..326e393d4de1
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/cpus.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/arm/cpus.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: CPUS, a container for CPU subnodes
+
+description: |
+  The device tree allows to describe the layout of CPUs in a system through
+  the "cpus" node, which in turn contains a number of subnodes (ie "cpu")
+  defining properties for every CPU.
+
+  Properties of the CPU integration that are common to all CPUs can be described
+  in the cpus node.
+
+  ARM CPUs with the FEAT_HPDS2 feature allow an IMPLEMENTATION DEFINED
+  hardware attribute to be encoded in the leaf page table entries and
+  sent with any transaction that makes an access via that entry.
+
+  Four bits are used in the page-tables. It is likely the individual bits will
+  be combined and used a a four bit value. The impact of any particular value is
+  up to the implementation.
+
+  0 is defined as a 'safe default setting' that behaves as if the feature
+  were not implemented. Other values may be unsafe, having coherency or
+  correctness issues leading to data-corruption or deadlock.
+
+  This binding lists the additional values that only have a performance cost
+  (or benefit), and values that can only be used if all mappings have the same
+  PBHA value.
+  For both cases, all affected values should be listed. If setting bit-2
+  requires no aliases, then the values 2, 4, 6 etc should be listed.
+
+  A hypervisor can only control individual bits, and may choose to only enable
+  bits that can only be used to build other performance-only values.
+  e.g. the value 5 is listed, but enabling bit-0 and bit-2 would allow a guest
+  to configure the values 1 or 4 too. If these 'decomposed' values only
+  affected performance, they should also be listed.
+
+  The list does not need to be in numeric order, but a hypervisor may make use
+  of the order when enabling bits.
+
+  The presence of a 'arm,pbha-no-aliases' property indicates that higher
+  exception levels and secure-world firmware do not have a mapping of any memory
+  in the memory node or UEFI memory map, other than those with a reserved-memory
+  entry or EFIReserved memory attribute.
+  Firmware mappings created based on requests from the normal world do not use
+  any of the arm,pbha-no-aliases values, or take the PBHA value to use as an
+  argument.
+
+properties:
+  $nodename:
+    const: cpus
+
+  arm,pbha-performance-only:
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    description: PBHA values that only affect performance
+    minItems: 1
+    maxItems: 15
+    items:
+      maximum: 15
+
+  arm,pbha-no-aliases:
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    description: PBHA values that must only be used if all mappings have the
+                   same value.
+    minItems: 1
+    maxItems: 15
+    items:
+      maximum: 15
+
+
+additionalProperties: true
+
+examples:
+  -|
+  /{
+    cpus {
+      arm,pbha-performance-only = /bits/ 8 <0x01 0x05 0x09>;
+      arm,pbha-no-aliases = /bits/ 8 <0x02 0x04 0x06 0x08>;
+
+      cpu@0 {
+        device_type = "cpu";
+        compatible = "arm,cortex-a57";
+        ...
+      };
+
+    };
+  };
+...
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 4/7] arm64: cpufeature: Enable PBHA bits for stage1
  2021-10-15 16:14 [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values James Morse
                   ` (2 preceding siblings ...)
  2021-10-15 16:14 ` [RFC PATCH 3/7] dt-bindings: arm: Add binding for Page Based Hardware Attributes James Morse
@ 2021-10-15 16:14 ` James Morse
  2021-10-16 13:50   ` Marc Zyngier
  2021-10-15 16:14 ` [RFC PATCH 5/7] arm64: mm: Add pgprot_pbha() to allow drivers to request PBHA values James Morse
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: James Morse @ 2021-10-15 16:14 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm; +Cc: Will Deacon, Catalin Marinas, Marc Zyngier

If the CPUs support HPDS2, and there is a DT description of PBHA values
that only affect performance, enable those bits for both TTBR0 and TTBR1.

Enabling PBHA requires the hierarchical-permissions to be disabled.
Commit 87143f404f33 ("arm64: mm: use XN table mapping attributes for
the linear region") used these, but only as an optimisation.

Only the necessary PBHA bits are enabled to reduce the risk of an
unsafe bit/value being used by accident.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/Kconfig                     | 13 +++++
 arch/arm64/include/asm/pgtable-hwdef.h |  4 ++
 arch/arm64/kernel/cpufeature.c         | 81 ++++++++++++++++++++++++++
 arch/arm64/tools/cpucaps               |  1 +
 4 files changed, 99 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 077f2ec4eeb2..9196bb932aba 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1461,6 +1461,19 @@ config ARM64_CNP
 	  at runtime, and does not affect PEs that do not implement
 	  this feature.
 
+config ARM64_PBHA
+	bool "Enable support for Page Based Hardware Attributes (PBHA)"
+	default n
+	help
+	  Page Based Hardware Attributes (PBHA) allow the SoC hardware to
+	  change behaviour depending on which mapping was used to access
+	  a page of memory. e.g. access via one mapping may always result
+	  in the data being cached, whereas using another mapping of the same
+	  physical memory.
+
+	  The behaviour of each PBHA bit is not defined. Say no unless you
+	  are very sure you want this
+
 endmenu
 
 menu "ARMv8.3 architectural features"
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index 40085e53f573..3d41242c52b0 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -268,6 +268,10 @@
 #define TCR_TBI1		(UL(1) << 38)
 #define TCR_HA			(UL(1) << 39)
 #define TCR_HD			(UL(1) << 40)
+#define TCR_HPD0		(UL(1) << 41)
+#define TCR_HPD1		(UL(1) << 42)
+#define TCR_HWU0nn_MASK		(UL(0xf) << 43)
+#define TCR_HWU1nn_MASK		(UL(0xf) << 47)
 #define TCR_TBID1		(UL(1) << 52)
 #define TCR_NFD0		(UL(1) << 53)
 #define TCR_NFD1		(UL(1) << 54)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 8694f9dec5e5..548c6f96a878 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -71,6 +71,7 @@
 #include <linux/types.h>
 #include <linux/minmax.h>
 #include <linux/mm.h>
+#include <linux/of.h>
 #include <linux/cpu.h>
 #include <linux/kasan.h>
 #include <asm/cpu.h>
@@ -110,6 +111,8 @@ DECLARE_BITMAP(boot_capabilities, ARM64_NPATCHABLE);
 bool arm64_use_ng_mappings = false;
 EXPORT_SYMBOL(arm64_use_ng_mappings);
 
+unsigned long __ro_after_init arm64_pbha_perf_only_values;
+
 /*
  * Permit PER_LINUX32 and execve() of 32-bit binaries even if not all CPUs
  * support it?
@@ -1676,6 +1679,71 @@ static bool has_hw_dbm(const struct arm64_cpu_capabilities *cap,
 
 #endif
 
+#ifdef CONFIG_ARM64_PBHA
+static u8 pbha_stage1_enable_bits;
+
+static bool plat_can_use_pbha_stage1(const struct arm64_cpu_capabilities *cap,
+				     int scope)
+{
+	u8 val;
+	struct device_node *cpus;
+	const u8 *perf_only_vals;
+	int num_perf_only_vals, i;
+
+	if (!has_cpuid_feature(cap, scope))
+		return false;
+
+	/*
+	 * Calls with scope == SCOPE_LOCAL_CPU need only testing whether this
+	 * cpu has the feature. A later 'system' scope call will check for a
+	 * firmware description.
+	 */
+	if (scope == SCOPE_LOCAL_CPU)
+		return true;
+
+	cpus = of_find_node_by_path("/cpus");
+	if (!cpus)
+		goto done;
+
+	perf_only_vals = of_get_property(cpus, "arm,pbha-performance-only",
+					 &num_perf_only_vals);
+	if (!perf_only_vals)
+		goto done;
+
+	/* any listed value is usable at stage 1 */
+	for (i = 0 ; i < num_perf_only_vals; i++) {
+		val = perf_only_vals[i];
+		if (val > 0xf)
+			continue;
+
+		pbha_stage1_enable_bits |= val;
+		set_bit(val, &arm64_pbha_perf_only_values);
+	}
+
+done:
+	of_node_put(cpus);
+
+	return !!pbha_stage1_enable_bits;
+}
+
+static void cpu_enable_pbha(struct arm64_cpu_capabilities const *cap)
+{
+	u64 tcr;
+
+	if (!pbha_stage1_enable_bits)
+		return;
+
+	tcr = read_sysreg(tcr_el1);
+	tcr |= FIELD_PREP(TCR_HWU0nn_MASK, pbha_stage1_enable_bits);
+	tcr |= FIELD_PREP(TCR_HWU1nn_MASK, pbha_stage1_enable_bits);
+	tcr |= FIELD_PREP(TCR_HPD0, 1) | FIELD_PREP(TCR_HPD1, 1);
+
+	write_sysreg(tcr, tcr_el1);
+	isb();
+	local_flush_tlb_all();
+}
+#endif /* CONFIG_ARM64_PBHA */
+
 #ifdef CONFIG_ARM64_AMU_EXTN
 
 /*
@@ -2337,6 +2405,19 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.matches = has_cpuid_feature,
 		.min_field_value = 2,
 	},
+#ifdef CONFIG_ARM64_PBHA
+	{
+		.desc = "Page Based Hardware Attributes (PBHA)",
+		.capability = ARM64_HAS_PBHA_STAGE1,
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.sys_reg = SYS_ID_AA64MMFR1_EL1,
+		.sign = FTR_UNSIGNED,
+		.field_pos = ID_AA64MMFR1_HPD_SHIFT,
+		.matches = plat_can_use_pbha_stage1,
+		.min_field_value = 2,
+		.cpu_enable = cpu_enable_pbha,
+	},
+#endif
 	{},
 };
 
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 132596d8b518..8dacca5f7c40 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -29,6 +29,7 @@ HAS_NO_FPSIMD
 HAS_NO_HW_PREFETCH
 HAS_PAN
 HAS_PBHA
+HAS_PBHA_STAGE1
 HAS_RAS_EXTN
 HAS_RNG
 HAS_SB
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 5/7] arm64: mm: Add pgprot_pbha() to allow drivers to request PBHA values
  2021-10-15 16:14 [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values James Morse
                   ` (3 preceding siblings ...)
  2021-10-15 16:14 ` [RFC PATCH 4/7] arm64: cpufeature: Enable PBHA bits for stage1 James Morse
@ 2021-10-15 16:14 ` James Morse
  2021-10-15 16:14 ` [RFC PATCH 6/7] KVM: arm64: Configure PBHA bits for stage2 James Morse
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: James Morse @ 2021-10-15 16:14 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm; +Cc: Will Deacon, Catalin Marinas, Marc Zyngier

Add a pgprot_pbha() helper that modifies a pgprot_t to include a pbha
value. The value is checked against those that were listed as only
affecting performance.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/include/asm/pgtable-hwdef.h |  1 +
 arch/arm64/include/asm/pgtable.h       | 12 ++++++++++++
 arch/arm64/kernel/cpufeature.c         |  1 +
 3 files changed, 14 insertions(+)

diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index 3d41242c52b0..bef11fb9a255 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -154,6 +154,7 @@
 #define PTE_CONT		(_AT(pteval_t, 1) << 52)	/* Contiguous range */
 #define PTE_PXN			(_AT(pteval_t, 1) << 53)	/* Privileged XN */
 #define PTE_UXN			(_AT(pteval_t, 1) << 54)	/* User XN */
+#define PTE_PBHA_MASK		(_AT(pteval_t, 0xf) << 59)	/* Page Base Hardware Attributes */
 
 #define PTE_ADDR_LOW		(((_AT(pteval_t, 1) << (48 - PAGE_SHIFT)) - 1) << PAGE_SHIFT)
 #ifdef CONFIG_ARM64_PA_BITS_52
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index dfa76afa0ccf..3bc242db2c3c 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -518,6 +518,18 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
 	__pgprot_modify(prot, PTE_ATTRINDX_MASK, \
 			PTE_ATTRINDX(MT_NORMAL_NC) | PTE_PXN | PTE_UXN)
 
+
+extern unsigned long arm64_pbha_perf_only_values;
+static inline unsigned long __pbha_check_perf_only(unsigned long pbha_val)
+{
+	if (test_bit(pbha_val, &arm64_pbha_perf_only_values))
+		return FIELD_PREP(PTE_PBHA_MASK, pbha_val);
+	return 0;
+}
+
+#define pgprot_pbha(prot, pbha_val) \
+	__pgprot_modify(prot, PTE_PBHA_MASK, __pbha_check_perf_only(pbha_val))
+
 #define __HAVE_PHYS_MEM_ACCESS_PROT
 struct file;
 extern pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 548c6f96a878..88f0f805b938 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -112,6 +112,7 @@ bool arm64_use_ng_mappings = false;
 EXPORT_SYMBOL(arm64_use_ng_mappings);
 
 unsigned long __ro_after_init arm64_pbha_perf_only_values;
+EXPORT_SYMBOL(arm64_pbha_perf_only_values);
 
 /*
  * Permit PER_LINUX32 and execve() of 32-bit binaries even if not all CPUs
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 6/7] KVM: arm64: Configure PBHA bits for stage2
  2021-10-15 16:14 [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values James Morse
                   ` (4 preceding siblings ...)
  2021-10-15 16:14 ` [RFC PATCH 5/7] arm64: mm: Add pgprot_pbha() to allow drivers to request PBHA values James Morse
@ 2021-10-15 16:14 ` James Morse
  2021-10-16 13:46   ` Marc Zyngier
  2021-10-15 16:14 ` [RFC PATCH 7/7] Documentation: arm64: Describe the support and expectations for PBHA James Morse
  2021-10-16 13:54 ` [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values Marc Zyngier
  7 siblings, 1 reply; 14+ messages in thread
From: James Morse @ 2021-10-15 16:14 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm; +Cc: Will Deacon, Catalin Marinas, Marc Zyngier

There are two conflicting use-cases for PBHA at stage2. We could copy
the stage1 PBHA bits to stage2, this would ensure the VMMs memory is
exactly reproduced for the guest, including the PBHA bits. The problem
here is how the VMM's memory is allocated with the PBHA bits set.

The other is allowing the guest to configure PBHA directly. This would
allow guest device drivers to map memory with the appropriate PBHA bits.
This would only be safe if the guest can be trusted to only generate
PBHA values that only affect performance.

The arm-arm doesn't describe how the stage1 and stage2 bits are combined.
Arm's implementations appear to all have the same behaviour, according
to the TRM: stage2 wins.

For these CPUs, we can allow a guest to use a PBHA bit by disabling it
in VTCR_EL2. We just need to know which bits...

The DT describes the values that only affect performance, but if value-5
is safe for use, we can't prevent the guest from using value-1 and value-4.
These 'decomposed' values would also need to be listed as only affecting
performance.

Add a cpufeature for CPUs that have this 'stage2 wins' behaviour.
Decompose each performance-only value (5 -> 5, 4, 1), and check each of
these values is listed as only affecting performance. If so, the bits
of the original value (5) can be used by the guest at stage1. (by clearing
the bits from VTCR_EL2)

Signed-off-by: James Morse <james.morse@arm.com>
---
I've checked the TRMs for the listed CPUs.
There are more, I've asked for the TRMs to always describe this.
---
 arch/arm64/include/asm/cpufeature.h |   1 +
 arch/arm64/include/asm/cputype.h    |   4 ++
 arch/arm64/kernel/cpufeature.c      | 105 ++++++++++++++++++++++++++++
 arch/arm64/kernel/image-vars.h      |   3 +
 arch/arm64/kvm/hyp/pgtable.c        |   8 ++-
 arch/arm64/tools/cpucaps            |   1 +
 6 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index ef6be92b1921..ec800ce15308 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -101,6 +101,7 @@ struct arm64_ftr_reg {
 };
 
 extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0;
+extern unsigned long arm64_pbha_stage2_safe_bits;
 
 /*
  * CPU capabilities:
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 6231e1f0abe7..4d7f18749d23 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -73,6 +73,8 @@
 #define ARM_CPU_PART_CORTEX_A76		0xD0B
 #define ARM_CPU_PART_NEOVERSE_N1	0xD0C
 #define ARM_CPU_PART_CORTEX_A77		0xD0D
+#define ARM_CPU_PART_CORTEX_A78		0xD41
+#define ARM_CPU_PART_CORTEX_X1		0xD44
 
 #define APM_CPU_PART_POTENZA		0x000
 
@@ -113,6 +115,8 @@
 #define MIDR_CORTEX_A76	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A76)
 #define MIDR_NEOVERSE_N1 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N1)
 #define MIDR_CORTEX_A77	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A77)
+#define MIDR_CORTEX_A78	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A78)
+#define MIDR_CORTEX_X1	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X1)
 #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
 #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
 #define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 88f0f805b938..ad2b3b179ab1 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -77,6 +77,7 @@
 #include <asm/cpu.h>
 #include <asm/cpufeature.h>
 #include <asm/cpu_ops.h>
+#include <asm/cputype.h>
 #include <asm/fpsimd.h>
 #include <asm/insn.h>
 #include <asm/kvm_host.h>
@@ -113,6 +114,7 @@ EXPORT_SYMBOL(arm64_use_ng_mappings);
 
 unsigned long __ro_after_init arm64_pbha_perf_only_values;
 EXPORT_SYMBOL(arm64_pbha_perf_only_values);
+unsigned long __ro_after_init arm64_pbha_stage2_safe_bits;
 
 /*
  * Permit PER_LINUX32 and execve() of 32-bit binaries even if not all CPUs
@@ -1680,13 +1682,50 @@ static bool has_hw_dbm(const struct arm64_cpu_capabilities *cap,
 
 #endif
 
+
 #ifdef CONFIG_ARM64_PBHA
 static u8 pbha_stage1_enable_bits;
+static DEFINE_SPINLOCK(pbha_dt_lock);
+
+/* For the value 5, return a bitmap with bits 5, 4, and 1 set. */
+static unsigned long decompose_pbha_values(u8 val)
+{
+	int i;
+	unsigned long mask = 0;
+
+	for (i = 1; i <= 15; i++) {
+		if ((i & val) == i)
+			set_bit(i, &mask);
+	}
+
+	return mask;
+}
+
+/*
+ * The bits of a value are safe if all values that can be built from those
+ * enabled bits are listed as only affecting performance.
+ * e.g. 5 would also need 1 and 4 to be listed.
+ *
+ * When there is a conflict with the bits already enabled, the new value is
+ * skipped.
+ * e.g. if 5 already caused bit-0 and bit-2 to be enabled, adding 3 to the list
+ * would need to test 7 as bit-2 is already enabled. If 7 is not listed, 3 is
+ * skipped and bit-1 is not enabled.
+ */
+static void stage2_test_pbha_value(u8 val)
+{
+	unsigned long mask;
+
+	mask = decompose_pbha_values(val | arm64_pbha_stage2_safe_bits);
+	if ((arm64_pbha_perf_only_values & mask) == mask)
+		arm64_pbha_stage2_safe_bits |= val;
+}
 
 static bool plat_can_use_pbha_stage1(const struct arm64_cpu_capabilities *cap,
 				     int scope)
 {
 	u8 val;
+	static bool dt_check_done;
 	struct device_node *cpus;
 	const u8 *perf_only_vals;
 	int num_perf_only_vals, i;
@@ -1702,6 +1741,10 @@ static bool plat_can_use_pbha_stage1(const struct arm64_cpu_capabilities *cap,
 	if (scope == SCOPE_LOCAL_CPU)
 		return true;
 
+	spin_lock(&pbha_dt_lock);
+	if (dt_check_done)
+		goto out_unlock;
+
 	cpus = of_find_node_by_path("/cpus");
 	if (!cpus)
 		goto done;
@@ -1721,9 +1764,24 @@ static bool plat_can_use_pbha_stage1(const struct arm64_cpu_capabilities *cap,
 		set_bit(val, &arm64_pbha_perf_only_values);
 	}
 
+	/*
+	 * for stage2 the values are collapsed back to 4 bits that can only
+	 * enable values in the arm64_pbha_perf_only_values mask.
+	 */
+	for (i = 0 ; i < num_perf_only_vals; i++) {
+		val = perf_only_vals[i];
+		if (val > 0xf)
+			continue;
+
+		stage2_test_pbha_value(val);
+	}
+
 done:
 	of_node_put(cpus);
+	dt_check_done = true;
 
+out_unlock:
+	spin_unlock(&pbha_dt_lock);
 	return !!pbha_stage1_enable_bits;
 }
 
@@ -1743,6 +1801,47 @@ static void cpu_enable_pbha(struct arm64_cpu_capabilities const *cap)
 	isb();
 	local_flush_tlb_all();
 }
+
+/*
+ * PBHA's behaviour is implementation defined, as is the way it combines
+ * stage1 and stage2 attributes. If the kernel has KVM supported, and booted
+ * at EL2, only these CPUs can allow PBHA in a guest, as KVM knows how the PBHA
+ * bits are combined. This prevents the host being affected by some
+ * implementation defined behaviour from the guest.
+ *
+ * The TRM for these CPUs describe stage2 as overriding stage1.
+ */
+static const struct midr_range pbha_stage2_wins[] = {
+	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
+	MIDR_ALL_VERSIONS(MIDR_CORTEX_A76),
+	MIDR_ALL_VERSIONS(MIDR_CORTEX_A77),
+	MIDR_ALL_VERSIONS(MIDR_CORTEX_A78),
+	{},
+};
+
+static bool plat_can_use_pbha_stage2(const struct arm64_cpu_capabilities *cap,
+				     int scope)
+{
+	/*  Booted at EL2? */
+	if (!is_hyp_mode_available() && !is_kernel_in_hyp_mode())
+		return false;
+
+	if (!is_midr_in_range_list(read_cpuid_id(), cap->midr_range_list))
+		return false;
+
+	/*
+	 * Calls with scope == SCOPE_LOCAL_CPU need only testing whether this
+	 * cpu has the feature. A later 'system' scope call will check for a
+	 * firmware description.
+	 */
+	if (scope == SCOPE_LOCAL_CPU)
+		return true;
+
+	if (!__system_matches_cap(ARM64_HAS_PBHA_STAGE1))
+		return false;
+
+	return !!arm64_pbha_stage2_safe_bits;
+}
 #endif /* CONFIG_ARM64_PBHA */
 
 #ifdef CONFIG_ARM64_AMU_EXTN
@@ -2418,6 +2517,12 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.min_field_value = 2,
 		.cpu_enable = cpu_enable_pbha,
 	},
+	{
+		.capability = ARM64_HAS_PBHA_STAGE2,
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.matches = plat_can_use_pbha_stage2,
+		.midr_range_list = pbha_stage2_wins,
+	},
 #endif
 	{},
 };
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index c96a9a0043bf..d6abdc53f4d9 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -134,6 +134,9 @@ KVM_NVHE_ALIAS(__hyp_rodata_end);
 /* pKVM static key */
 KVM_NVHE_ALIAS(kvm_protected_mode_initialized);
 
+/* PBHA bits for stage2 */
+KVM_NVHE_ALIAS(arm64_pbha_stage2_safe_bits);
+
 #endif /* CONFIG_KVM */
 
 #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 7bd90ea1c61f..c0bfef55a1ff 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -520,7 +520,7 @@ struct stage2_map_data {
 u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)
 {
 	u64 vtcr = VTCR_EL2_FLAGS;
-	u8 lvls;
+	u8 lvls, pbha = 0xf;
 
 	vtcr |= kvm_get_parange(mmfr0) << VTCR_EL2_PS_SHIFT;
 	vtcr |= VTCR_EL2_T0SZ(phys_shift);
@@ -545,9 +545,13 @@ u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)
 	 * value will always be 0, which is defined as the safe default
 	 * setting. On Cortex cores, enabling PBHA for stage2 effectively
 	 * disables it for stage1.
+	 * When the HAS_PBHA_STAGE2 feature is supported, clear the 'safe'
+	 * bits to allow the guest's stage1 to use these bits.
 	 */
+	if (cpus_have_final_cap(ARM64_HAS_PBHA_STAGE2))
+		pbha = pbha ^ arm64_pbha_stage2_safe_bits;
 	if (cpus_have_final_cap(ARM64_HAS_PBHA))
-		vtcr = FIELD_PREP(VTCR_EL2_PBHA_MASK, 0xf);
+		vtcr = FIELD_PREP(VTCR_EL2_PBHA_MASK, pbha);
 
 	/* Set the vmid bits */
 	vtcr |= (get_vmid_bits(mmfr1) == 16) ?
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 8dacca5f7c40..c561c20d556a 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -30,6 +30,7 @@ HAS_NO_HW_PREFETCH
 HAS_PAN
 HAS_PBHA
 HAS_PBHA_STAGE1
+HAS_PBHA_STAGE2
 HAS_RAS_EXTN
 HAS_RNG
 HAS_SB
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 7/7] Documentation: arm64: Describe the support and expectations for PBHA
  2021-10-15 16:14 [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values James Morse
                   ` (5 preceding siblings ...)
  2021-10-15 16:14 ` [RFC PATCH 6/7] KVM: arm64: Configure PBHA bits for stage2 James Morse
@ 2021-10-15 16:14 ` James Morse
  2021-10-16 13:54 ` [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values Marc Zyngier
  7 siblings, 0 replies; 14+ messages in thread
From: James Morse @ 2021-10-15 16:14 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm; +Cc: Will Deacon, Catalin Marinas, Marc Zyngier

PBHA isn't defined by the Arm CPU architecture, so may have surprising
side-effects.

Document what is, and what is not supported. List the arch code's
expectations regarding how PBHA behaves.

Signed-off-by: James Morse <james.morse@arm.com>
---
 Documentation/arm64/index.rst |  1 +
 Documentation/arm64/pbha.rst  | 86 +++++++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+)
 create mode 100644 Documentation/arm64/pbha.rst

diff --git a/Documentation/arm64/index.rst b/Documentation/arm64/index.rst
index 4f840bac083e..88fdfda86632 100644
--- a/Documentation/arm64/index.rst
+++ b/Documentation/arm64/index.rst
@@ -18,6 +18,7 @@ ARM64 Architecture
     legacy_instructions
     memory
     memory-tagging-extension
+    pbha
     perf
     pointer-authentication
     silicon-errata
diff --git a/Documentation/arm64/pbha.rst b/Documentation/arm64/pbha.rst
new file mode 100644
index 000000000000..0ba50a0df05f
--- /dev/null
+++ b/Documentation/arm64/pbha.rst
@@ -0,0 +1,86 @@
+=======================================================
+Page Based Hardware Attribute support for AArch64 Linux
+=======================================================
+
+Page Based Hardware Attributes (PBHA) allow the OS to trigger IMPLEMENTATION
+DEFINED behaviour associated with a memory access. For example, this may be
+taken as a hint to a System Cache whether it should cache the location that
+has been accessed.
+
+PBHA consists of four bits in the leaf page table entries for a virtual
+address, that are sent with any memory access via that virtual address.
+
+IMPLEMENTATION DEFINED behaviour is not specified by the arm-arm, meaning
+it varies between SoCs. There may be unexpected side effects when PBHA
+bits are used or combined.
+For example, a PBHA bit may be taken as a hint to the Memory Controller that
+it should encrypt/decrypt the memory in DRAM. If the CPU has multiple virtual
+aliases of the address, accesses that are made without this PBHA bit set may
+cause corruption.
+
+
+Use by virtual machines using KVM
+---------------------------------
+
+KVM allows an OS in a virtual machine to configure its own page tables. A
+virtual machine can also configure PBHA bits in its page tables. To prevent
+side effects that could affect the hypervisor, KVM will only allow
+combinations of PBHA bits that only affect performance. Values that cause
+changes to the data are forbidden as the Hypervisor and VMM have aliases of
+the guest memory, and may swap it to/from disk.
+
+The list of bits to allow is built from the firmware list of PBHA bit
+combinations that only affect performance. Because the guest can choose
+not to set all the bits in a value, (e.g. allowing 5 implicitly allows 1
+and 4), the values supported may differ between a host and guest.
+
+PBHA is only supported for a guest if KVM supports the mechanism the CPU uses
+to combine the values from stage1 and stage2 translation. The mechanism is not
+advertised, so which mechanism each CPU uses must also be known by the kernel.
+
+
+Use by device drivers
+---------------------
+
+Device drivers should discover the PBHA value to use for a mapping from the
+device's firmware description as these will vary between SoCs. If the value
+is also listed by firmware as only affecting performance, it can be added to
+the pgprot with pgprot_pbha().
+
+Values that require all other aliases to be removed are not supported.
+
+
+Linux's expectations around PBHA
+--------------------------------
+
+'IMPLEMENTATION DEFINED' describes a huge range of possible behaviours.
+Linux expects PBHA to behave in the same way as the read/write allocate hints
+for a memory type. Below is an incomplete list of expectations:
+
+ * PBHA values have the same meaning for all CPUs in the SoC.
+ * Use of the PBHA value does not cause mismatched type, shareability or
+   cacheability, it does not take precedence over the stage2 attributes, or
+   HCR_EL2 controls.
+ * If a PBHA value requires all other aliases to be removed, higher exception
+   levels do not have a concurrent alias. (This includes Secure World).
+ * Break before make is sufficient when changing the PBHA value.
+ * PBHA values used by a page can be changed independently without further side
+   effects.
+ * Save/restoring the page contents via a PBHA=0 mapping does not corrupt the
+   values once a non-zero PBHA mapping is re-created.
+ * The hypervisor may clean+invalidate to the PoC via a PBHA=0 mapping prior to
+   save/restore to cleanup mismatched attributes. This does not corrupt the
+   values after save/restore once a non-zero PBHA mapping is re-created.
+ * Cache maintenance via a PBHA=0 mapping to prevent stale data being visible
+   when mismatched attributes occur is sufficient even if the subsequent
+   mapping has a non-zero PBHA value.
+ * The OS/hypervisor can clean-up a page by removing all non-zero PBHA mappings,
+   then writing new data via PBHA=0 mapping of the same type, shareability and
+   cacheability. After this, only the new data is visible for data accesses.
+ * For instruction-fetch, the same maintenance as would be performed against a
+   PBHA=0 page is sufficient. (which with DIC+IDC, may be none at all).
+ * The behaviour enabled by PBHA should not depend on the size of the access, or
+   whether other SoC hardware under the control of the OS is enabled and
+   configured.
+ * EL2 is able to at least force stage1 PBHA bits to zero.
+
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 1/7] KVM: arm64: Detect and enable PBHA for stage2
  2021-10-15 16:14 ` [RFC PATCH 1/7] KVM: arm64: Detect and enable PBHA for stage2 James Morse
@ 2021-10-16 13:27   ` Marc Zyngier
  2021-10-18 17:26     ` James Morse
  0 siblings, 1 reply; 14+ messages in thread
From: Marc Zyngier @ 2021-10-16 13:27 UTC (permalink / raw)
  To: James Morse; +Cc: linux-arm-kernel, kvmarm, Will Deacon, Catalin Marinas

Hi James,

On Fri, 15 Oct 2021 17:14:10 +0100,
James Morse <james.morse@arm.com> wrote:
> 
> Page Based Hardware Attributes (PBHA, aka HPDS2) allow a page table entry
> to specify up to four bits that can be used by the hardware for some
> implementation defined purpose.
> 
> This is a problem for KVM guests as the host may swap guest memory using
> a different combination of PBHA bits than the guest used when writing the
> data. Without knowing what the PBHA bits do, its not possible to know if
> this will corrupt the guest's data.
> 
> The arm-arm doesn't describe how the PBHA bits are combined between stage1
> and stage2. Arm's Cortex CPUs appear to all do the same thing: stage2 wins.
> 
> Enable PBHA for stage2, where the configured value is zero. This has no
> effect if PBHA isn't in use. On Cortex cores that have the 'stage2 wins'
> behaviour, this disables whatever the guest may be doing. For any other
> core with a sensible combination policy, it should be harmless.

So the other side of the above is whether it has the potential to be
harmful on systems that combine PBHA bits differently. Specially if
they use VTCR_EL2.PBHA bits as a indication that they can OR S1 and S2
instead of a direct S2 override, thus letting the S1 bits that would
otherwise not being conveyed outside of the core.

I guess we have no way to know until someone reports really bad memory
corruption and loss of data. The architecture is totally broken here.

> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> I've checked the TRMs for Neoverse-N1, and Cortexs: A76, A77, A78 and X1.
> They all have this 'stage2 wins' behaviour. The behaviour isn't documented
> by A510 or A710's TRM.
> ---
>  arch/arm64/include/asm/kvm_arm.h     | 1 +
>  arch/arm64/include/asm/kvm_pgtable.h | 9 +++++++++
>  arch/arm64/kernel/cpufeature.c       | 9 +++++++++
>  arch/arm64/kvm/hyp/pgtable.c         | 9 +++++++++
>  arch/arm64/tools/cpucaps             | 1 +
>  5 files changed, 29 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 327120c0089f..bab7f0ad3724 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -126,6 +126,7 @@
>  #define VTCR_EL2_VS_SHIFT	19
>  #define VTCR_EL2_VS_8BIT	(0 << VTCR_EL2_VS_SHIFT)
>  #define VTCR_EL2_VS_16BIT	(1 << VTCR_EL2_VS_SHIFT)
> +#define VTCR_EL2_PBHA_MASK	GENMASK(28, 25)
>  
>  #define VTCR_EL2_T0SZ(x)	TCR_T0SZ(x)
>  
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index 027783829584..678bff4bfd7f 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -125,6 +125,10 @@ enum kvm_pgtable_stage2_flags {
>   * @KVM_PGTABLE_PROT_SW1:	Software bit 1.
>   * @KVM_PGTABLE_PROT_SW2:	Software bit 2.
>   * @KVM_PGTABLE_PROT_SW3:	Software bit 3.
> + * @KVM_PGTABLE_PROT_PBHA0:	Page-Based Hardware Attribute 0.
> + * @KVM_PGTABLE_PROT_PBHA1:	Page-Based Hardware Attribute 1.
> + * @KVM_PGTABLE_PROT_PBHA2:	Page-Based Hardware Attribute 2.
> + * @KVM_PGTABLE_PROT_PBHA3:	Page-Based Hardware Attribute 3.
>   */
>  enum kvm_pgtable_prot {
>  	KVM_PGTABLE_PROT_X			= BIT(0),
> @@ -137,6 +141,11 @@ enum kvm_pgtable_prot {
>  	KVM_PGTABLE_PROT_SW1			= BIT(56),
>  	KVM_PGTABLE_PROT_SW2			= BIT(57),
>  	KVM_PGTABLE_PROT_SW3			= BIT(58),
> +
> +	KVM_PGTABLE_PROT_PBHA0			= BIT(59),
> +	KVM_PGTABLE_PROT_PBHA1			= BIT(60),
> +	KVM_PGTABLE_PROT_PBHA2			= BIT(61),
> +	KVM_PGTABLE_PROT_PBHA3			= BIT(62),
>  };
>  
>  #define KVM_PGTABLE_PROT_RW	(KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W)
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index f8a3067d10c6..8694f9dec5e5 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2328,6 +2328,15 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  		.matches = has_cpuid_feature,
>  		.min_field_value = 1,
>  	},
> +	{
> +		.capability = ARM64_HAS_PBHA,
> +		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
> +		.sys_reg = SYS_ID_AA64MMFR1_EL1,
> +		.sign = FTR_UNSIGNED,
> +		.field_pos = ID_AA64MMFR1_HPD_SHIFT,
> +		.matches = has_cpuid_feature,
> +		.min_field_value = 2,
> +	},
>  	{},
>  };
>  
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index f8ceebe4982e..7bd90ea1c61f 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -540,6 +540,15 @@ u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)
>  	 */
>  	vtcr |= VTCR_EL2_HA;
>  
> +	/*
> +	 * Enable PBHA for stage2 on systems that support it. The configured
> +	 * value will always be 0, which is defined as the safe default
> +	 * setting. On Cortex cores, enabling PBHA for stage2 effectively
> +	 * disables it for stage1.
> +	 */
> +	if (cpus_have_final_cap(ARM64_HAS_PBHA))
> +		vtcr = FIELD_PREP(VTCR_EL2_PBHA_MASK, 0xf);

Err... Surely you mean 'vtcr |= FIELD_PREP(...)' here, right?

> +
>  	/* Set the vmid bits */
>  	vtcr |= (get_vmid_bits(mmfr1) == 16) ?
>  		VTCR_EL2_VS_16BIT :
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 49305c2e6dfd..132596d8b518 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -28,6 +28,7 @@ HAS_LSE_ATOMICS
>  HAS_NO_FPSIMD
>  HAS_NO_HW_PREFETCH
>  HAS_PAN
> +HAS_PBHA
>  HAS_RAS_EXTN
>  HAS_RNG
>  HAS_SB

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 6/7] KVM: arm64: Configure PBHA bits for stage2
  2021-10-15 16:14 ` [RFC PATCH 6/7] KVM: arm64: Configure PBHA bits for stage2 James Morse
@ 2021-10-16 13:46   ` Marc Zyngier
  0 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2021-10-16 13:46 UTC (permalink / raw)
  To: James Morse; +Cc: linux-arm-kernel, kvmarm, Will Deacon, Catalin Marinas

On Fri, 15 Oct 2021 17:14:15 +0100,
James Morse <james.morse@arm.com> wrote:
> 
> There are two conflicting use-cases for PBHA at stage2. We could copy
> the stage1 PBHA bits to stage2, this would ensure the VMMs memory is
> exactly reproduced for the guest, including the PBHA bits. The problem
> here is how the VMM's memory is allocated with the PBHA bits set.
> 
> The other is allowing the guest to configure PBHA directly. This would
> allow guest device drivers to map memory with the appropriate PBHA bits.
> This would only be safe if the guest can be trusted to only generate
> PBHA values that only affect performance.
> 
> The arm-arm doesn't describe how the stage1 and stage2 bits are combined.
> Arm's implementations appear to all have the same behaviour, according
> to the TRM: stage2 wins.
> 
> For these CPUs, we can allow a guest to use a PBHA bit by disabling it
> in VTCR_EL2. We just need to know which bits...
> 
> The DT describes the values that only affect performance, but if value-5
> is safe for use, we can't prevent the guest from using value-1 and value-4.
> These 'decomposed' values would also need to be listed as only affecting
> performance.
> 
> Add a cpufeature for CPUs that have this 'stage2 wins' behaviour.
> Decompose each performance-only value (5 -> 5, 4, 1), and check each of
> these values is listed as only affecting performance. If so, the bits
> of the original value (5) can be used by the guest at stage1. (by clearing
> the bits from VTCR_EL2)
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> I've checked the TRMs for the listed CPUs.
> There are more, I've asked for the TRMs to always describe this.
> ---
>  arch/arm64/include/asm/cpufeature.h |   1 +
>  arch/arm64/include/asm/cputype.h    |   4 ++
>  arch/arm64/kernel/cpufeature.c      | 105 ++++++++++++++++++++++++++++
>  arch/arm64/kernel/image-vars.h      |   3 +
>  arch/arm64/kvm/hyp/pgtable.c        |   8 ++-
>  arch/arm64/tools/cpucaps            |   1 +
>  6 files changed, 120 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index ef6be92b1921..ec800ce15308 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -101,6 +101,7 @@ struct arm64_ftr_reg {
>  };
>  
>  extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0;
> +extern unsigned long arm64_pbha_stage2_safe_bits;
>  
>  /*
>   * CPU capabilities:
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index 6231e1f0abe7..4d7f18749d23 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -73,6 +73,8 @@
>  #define ARM_CPU_PART_CORTEX_A76		0xD0B
>  #define ARM_CPU_PART_NEOVERSE_N1	0xD0C
>  #define ARM_CPU_PART_CORTEX_A77		0xD0D
> +#define ARM_CPU_PART_CORTEX_A78		0xD41
> +#define ARM_CPU_PART_CORTEX_X1		0xD44
>  
>  #define APM_CPU_PART_POTENZA		0x000
>  
> @@ -113,6 +115,8 @@
>  #define MIDR_CORTEX_A76	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A76)
>  #define MIDR_NEOVERSE_N1 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N1)
>  #define MIDR_CORTEX_A77	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A77)
> +#define MIDR_CORTEX_A78	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A78)
> +#define MIDR_CORTEX_X1	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X1)
>  #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
>  #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
>  #define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX)
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 88f0f805b938..ad2b3b179ab1 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -77,6 +77,7 @@
>  #include <asm/cpu.h>
>  #include <asm/cpufeature.h>
>  #include <asm/cpu_ops.h>
> +#include <asm/cputype.h>
>  #include <asm/fpsimd.h>
>  #include <asm/insn.h>
>  #include <asm/kvm_host.h>
> @@ -113,6 +114,7 @@ EXPORT_SYMBOL(arm64_use_ng_mappings);
>  
>  unsigned long __ro_after_init arm64_pbha_perf_only_values;
>  EXPORT_SYMBOL(arm64_pbha_perf_only_values);
> +unsigned long __ro_after_init arm64_pbha_stage2_safe_bits;
>  
>  /*
>   * Permit PER_LINUX32 and execve() of 32-bit binaries even if not all CPUs
> @@ -1680,13 +1682,50 @@ static bool has_hw_dbm(const struct arm64_cpu_capabilities *cap,
>  
>  #endif
>  
> +
>  #ifdef CONFIG_ARM64_PBHA
>  static u8 pbha_stage1_enable_bits;
> +static DEFINE_SPINLOCK(pbha_dt_lock);
> +
> +/* For the value 5, return a bitmap with bits 5, 4, and 1 set. */
> +static unsigned long decompose_pbha_values(u8 val)
> +{
> +	int i;
> +	unsigned long mask = 0;
> +
> +	for (i = 1; i <= 15; i++) {
> +		if ((i & val) == i)
> +			set_bit(i, &mask);
> +	}
> +
> +	return mask;
> +}
> +
> +/*
> + * The bits of a value are safe if all values that can be built from those
> + * enabled bits are listed as only affecting performance.
> + * e.g. 5 would also need 1 and 4 to be listed.
> + *
> + * When there is a conflict with the bits already enabled, the new value is
> + * skipped.
> + * e.g. if 5 already caused bit-0 and bit-2 to be enabled, adding 3 to the list
> + * would need to test 7 as bit-2 is already enabled. If 7 is not listed, 3 is
> + * skipped and bit-1 is not enabled.
> + */
> +static void stage2_test_pbha_value(u8 val)
> +{
> +	unsigned long mask;
> +
> +	mask = decompose_pbha_values(val | arm64_pbha_stage2_safe_bits);
> +	if ((arm64_pbha_perf_only_values & mask) == mask)
> +		arm64_pbha_stage2_safe_bits |= val;
> +}
>  
>  static bool plat_can_use_pbha_stage1(const struct arm64_cpu_capabilities *cap,
>  				     int scope)
>  {
>  	u8 val;
> +	static bool dt_check_done;
>  	struct device_node *cpus;
>  	const u8 *perf_only_vals;
>  	int num_perf_only_vals, i;
> @@ -1702,6 +1741,10 @@ static bool plat_can_use_pbha_stage1(const struct arm64_cpu_capabilities *cap,
>  	if (scope == SCOPE_LOCAL_CPU)
>  		return true;
>  
> +	spin_lock(&pbha_dt_lock);
> +	if (dt_check_done)
> +		goto out_unlock;
> +
>  	cpus = of_find_node_by_path("/cpus");
>  	if (!cpus)
>  		goto done;
> @@ -1721,9 +1764,24 @@ static bool plat_can_use_pbha_stage1(const struct arm64_cpu_capabilities *cap,
>  		set_bit(val, &arm64_pbha_perf_only_values);
>  	}
>  
> +	/*
> +	 * for stage2 the values are collapsed back to 4 bits that can only
> +	 * enable values in the arm64_pbha_perf_only_values mask.
> +	 */
> +	for (i = 0 ; i < num_perf_only_vals; i++) {
> +		val = perf_only_vals[i];
> +		if (val > 0xf)
> +			continue;
> +
> +		stage2_test_pbha_value(val);
> +	}
> +
>  done:
>  	of_node_put(cpus);
> +	dt_check_done = true;
>  
> +out_unlock:
> +	spin_unlock(&pbha_dt_lock);
>  	return !!pbha_stage1_enable_bits;
>  }
>  
> @@ -1743,6 +1801,47 @@ static void cpu_enable_pbha(struct arm64_cpu_capabilities const *cap)
>  	isb();
>  	local_flush_tlb_all();
>  }
> +
> +/*
> + * PBHA's behaviour is implementation defined, as is the way it combines
> + * stage1 and stage2 attributes. If the kernel has KVM supported, and booted
> + * at EL2, only these CPUs can allow PBHA in a guest, as KVM knows how the PBHA
> + * bits are combined. This prevents the host being affected by some
> + * implementation defined behaviour from the guest.
> + *
> + * The TRM for these CPUs describe stage2 as overriding stage1.
> + */
> +static const struct midr_range pbha_stage2_wins[] = {
> +	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
> +	MIDR_ALL_VERSIONS(MIDR_CORTEX_A76),
> +	MIDR_ALL_VERSIONS(MIDR_CORTEX_A77),
> +	MIDR_ALL_VERSIONS(MIDR_CORTEX_A78),
> +	{},
> +};
> +
> +static bool plat_can_use_pbha_stage2(const struct arm64_cpu_capabilities *cap,
> +				     int scope)
> +{
> +	/*  Booted at EL2? */
> +	if (!is_hyp_mode_available() && !is_kernel_in_hyp_mode())
> +		return false;
> +
> +	if (!is_midr_in_range_list(read_cpuid_id(), cap->midr_range_list))
> +		return false;
> +
> +	/*
> +	 * Calls with scope == SCOPE_LOCAL_CPU need only testing whether this
> +	 * cpu has the feature. A later 'system' scope call will check for a
> +	 * firmware description.
> +	 */
> +	if (scope == SCOPE_LOCAL_CPU)
> +		return true;
> +
> +	if (!__system_matches_cap(ARM64_HAS_PBHA_STAGE1))
> +		return false;
> +
> +	return !!arm64_pbha_stage2_safe_bits;
> +}
>  #endif /* CONFIG_ARM64_PBHA */
>  
>  #ifdef CONFIG_ARM64_AMU_EXTN
> @@ -2418,6 +2517,12 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  		.min_field_value = 2,
>  		.cpu_enable = cpu_enable_pbha,
>  	},
> +	{
> +		.capability = ARM64_HAS_PBHA_STAGE2,
> +		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
> +		.matches = plat_can_use_pbha_stage2,
> +		.midr_range_list = pbha_stage2_wins,
> +	},
>  #endif
>  	{},
>  };
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index c96a9a0043bf..d6abdc53f4d9 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -134,6 +134,9 @@ KVM_NVHE_ALIAS(__hyp_rodata_end);
>  /* pKVM static key */
>  KVM_NVHE_ALIAS(kvm_protected_mode_initialized);
>  
> +/* PBHA bits for stage2 */
> +KVM_NVHE_ALIAS(arm64_pbha_stage2_safe_bits);

I'm not totally keen on this, see below.

> +
>  #endif /* CONFIG_KVM */
>  
>  #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index 7bd90ea1c61f..c0bfef55a1ff 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -520,7 +520,7 @@ struct stage2_map_data {
>  u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)
>  {
>  	u64 vtcr = VTCR_EL2_FLAGS;
> -	u8 lvls;
> +	u8 lvls, pbha = 0xf;
>  
>  	vtcr |= kvm_get_parange(mmfr0) << VTCR_EL2_PS_SHIFT;
>  	vtcr |= VTCR_EL2_T0SZ(phys_shift);
> @@ -545,9 +545,13 @@ u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)
>  	 * value will always be 0, which is defined as the safe default
>  	 * setting. On Cortex cores, enabling PBHA for stage2 effectively
>  	 * disables it for stage1.
> +	 * When the HAS_PBHA_STAGE2 feature is supported, clear the 'safe'
> +	 * bits to allow the guest's stage1 to use these bits.
>  	 */
> +	if (cpus_have_final_cap(ARM64_HAS_PBHA_STAGE2))
> +		pbha = pbha ^ arm64_pbha_stage2_safe_bits;
>  	if (cpus_have_final_cap(ARM64_HAS_PBHA))
> -		vtcr = FIELD_PREP(VTCR_EL2_PBHA_MASK, 0xf);
> +		vtcr = FIELD_PREP(VTCR_EL2_PBHA_MASK, pbha);

ORing bug aside, this isn't great in the protected case, where we
ultimately don't want to trust a value that is under EL1 control for
page tables.

I'd suggest reusing the hack we have for things like kimage_voffset,
where we generate the value as an immediate that gets inlined.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 4/7] arm64: cpufeature: Enable PBHA bits for stage1
  2021-10-15 16:14 ` [RFC PATCH 4/7] arm64: cpufeature: Enable PBHA bits for stage1 James Morse
@ 2021-10-16 13:50   ` Marc Zyngier
  2021-10-18 17:26     ` James Morse
  0 siblings, 1 reply; 14+ messages in thread
From: Marc Zyngier @ 2021-10-16 13:50 UTC (permalink / raw)
  To: James Morse; +Cc: linux-arm-kernel, kvmarm, Will Deacon, Catalin Marinas

On Fri, 15 Oct 2021 17:14:13 +0100,
James Morse <james.morse@arm.com> wrote:
> 
> If the CPUs support HPDS2, and there is a DT description of PBHA values
> that only affect performance, enable those bits for both TTBR0 and TTBR1.
> 
> Enabling PBHA requires the hierarchical-permissions to be disabled.
> Commit 87143f404f33 ("arm64: mm: use XN table mapping attributes for
> the linear region") used these, but only as an optimisation.
> 
> Only the necessary PBHA bits are enabled to reduce the risk of an
> unsafe bit/value being used by accident.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  arch/arm64/Kconfig                     | 13 +++++
>  arch/arm64/include/asm/pgtable-hwdef.h |  4 ++
>  arch/arm64/kernel/cpufeature.c         | 81 ++++++++++++++++++++++++++
>  arch/arm64/tools/cpucaps               |  1 +
>  4 files changed, 99 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 077f2ec4eeb2..9196bb932aba 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1461,6 +1461,19 @@ config ARM64_CNP
>  	  at runtime, and does not affect PEs that do not implement
>  	  this feature.
>  
> +config ARM64_PBHA
> +	bool "Enable support for Page Based Hardware Attributes (PBHA)"
> +	default n
> +	help
> +	  Page Based Hardware Attributes (PBHA) allow the SoC hardware to
> +	  change behaviour depending on which mapping was used to access
> +	  a page of memory. e.g. access via one mapping may always result
> +	  in the data being cached, whereas using another mapping of the same
> +	  physical memory.
> +
> +	  The behaviour of each PBHA bit is not defined. Say no unless you
> +	  are very sure you want this
> +
>  endmenu
>  
>  menu "ARMv8.3 architectural features"
> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
> index 40085e53f573..3d41242c52b0 100644
> --- a/arch/arm64/include/asm/pgtable-hwdef.h
> +++ b/arch/arm64/include/asm/pgtable-hwdef.h
> @@ -268,6 +268,10 @@
>  #define TCR_TBI1		(UL(1) << 38)
>  #define TCR_HA			(UL(1) << 39)
>  #define TCR_HD			(UL(1) << 40)
> +#define TCR_HPD0		(UL(1) << 41)
> +#define TCR_HPD1		(UL(1) << 42)
> +#define TCR_HWU0nn_MASK		(UL(0xf) << 43)
> +#define TCR_HWU1nn_MASK		(UL(0xf) << 47)
>  #define TCR_TBID1		(UL(1) << 52)
>  #define TCR_NFD0		(UL(1) << 53)
>  #define TCR_NFD1		(UL(1) << 54)
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 8694f9dec5e5..548c6f96a878 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -71,6 +71,7 @@
>  #include <linux/types.h>
>  #include <linux/minmax.h>
>  #include <linux/mm.h>
> +#include <linux/of.h>
>  #include <linux/cpu.h>
>  #include <linux/kasan.h>
>  #include <asm/cpu.h>
> @@ -110,6 +111,8 @@ DECLARE_BITMAP(boot_capabilities, ARM64_NPATCHABLE);
>  bool arm64_use_ng_mappings = false;
>  EXPORT_SYMBOL(arm64_use_ng_mappings);
>  
> +unsigned long __ro_after_init arm64_pbha_perf_only_values;
> +
>  /*
>   * Permit PER_LINUX32 and execve() of 32-bit binaries even if not all CPUs
>   * support it?
> @@ -1676,6 +1679,71 @@ static bool has_hw_dbm(const struct arm64_cpu_capabilities *cap,
>  
>  #endif
>  
> +#ifdef CONFIG_ARM64_PBHA
> +static u8 pbha_stage1_enable_bits;
> +
> +static bool plat_can_use_pbha_stage1(const struct arm64_cpu_capabilities *cap,
> +				     int scope)
> +{
> +	u8 val;
> +	struct device_node *cpus;
> +	const u8 *perf_only_vals;
> +	int num_perf_only_vals, i;
> +
> +	if (!has_cpuid_feature(cap, scope))
> +		return false;
> +
> +	/*
> +	 * Calls with scope == SCOPE_LOCAL_CPU need only testing whether this
> +	 * cpu has the feature. A later 'system' scope call will check for a
> +	 * firmware description.
> +	 */
> +	if (scope == SCOPE_LOCAL_CPU)
> +		return true;
> +
> +	cpus = of_find_node_by_path("/cpus");
> +	if (!cpus)
> +		goto done;
> +
> +	perf_only_vals = of_get_property(cpus, "arm,pbha-performance-only",
> +					 &num_perf_only_vals);
> +	if (!perf_only_vals)
> +		goto done;
> +
> +	/* any listed value is usable at stage 1 */
> +	for (i = 0 ; i < num_perf_only_vals; i++) {
> +		val = perf_only_vals[i];
> +		if (val > 0xf)
> +			continue;
> +
> +		pbha_stage1_enable_bits |= val;
> +		set_bit(val, &arm64_pbha_perf_only_values);
> +	}

Somehow, this would need to be exposed to userspace so that a VMM
could tell a guest what it can use.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values
  2021-10-15 16:14 [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values James Morse
                   ` (6 preceding siblings ...)
  2021-10-15 16:14 ` [RFC PATCH 7/7] Documentation: arm64: Describe the support and expectations for PBHA James Morse
@ 2021-10-16 13:54 ` Marc Zyngier
  7 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2021-10-16 13:54 UTC (permalink / raw)
  To: James Morse; +Cc: linux-arm-kernel, kvmarm, Will Deacon, Catalin Marinas

Hi James,

On Fri, 15 Oct 2021 17:14:09 +0100,
James Morse <james.morse@arm.com> wrote:
> 
> Hello!
> 
> Do you have hardware that uses PBHA? If so, what does the SoC do in
> response to which bits, and what workload needs that behaviour?
> 
> This series is a start at trying to work out what linux needs to
> support to makeuse of existing SoCs using PBHA.

Thanks for starting this.

I think one part that is missing here is the device side of things.
How do these attributes get used by the SMMU, how do we convey them to
the DMA layer, and what is the expected behaviour when we have SVA?
Not having any guidance form the architecture means that we're walking
on very thin ice...

	M.

-- 
Without deviation from the norm, progress is not possible.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 1/7] KVM: arm64: Detect and enable PBHA for stage2
  2021-10-16 13:27   ` Marc Zyngier
@ 2021-10-18 17:26     ` James Morse
  0 siblings, 0 replies; 14+ messages in thread
From: James Morse @ 2021-10-18 17:26 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: linux-arm-kernel, kvmarm, Will Deacon, Catalin Marinas

Hi Marc,

On 16/10/2021 14:27, Marc Zyngier wrote:
> On Fri, 15 Oct 2021 17:14:10 +0100,
> James Morse <james.morse@arm.com> wrote:
>>
>> Page Based Hardware Attributes (PBHA, aka HPDS2) allow a page table entry
>> to specify up to four bits that can be used by the hardware for some
>> implementation defined purpose.
>>
>> This is a problem for KVM guests as the host may swap guest memory using
>> a different combination of PBHA bits than the guest used when writing the
>> data. Without knowing what the PBHA bits do, its not possible to know if
>> this will corrupt the guest's data.
>>
>> The arm-arm doesn't describe how the PBHA bits are combined between stage1
>> and stage2. Arm's Cortex CPUs appear to all do the same thing: stage2 wins.
>>
>> Enable PBHA for stage2, where the configured value is zero. This has no
>> effect if PBHA isn't in use. On Cortex cores that have the 'stage2 wins'
>> behaviour, this disables whatever the guest may be doing. For any other
>> core with a sensible combination policy, it should be harmless.

> So the other side of the above is whether it has the potential to be
> harmful on systems that combine PBHA bits differently. Specially if
> they use VTCR_EL2.PBHA bits as a indication that they can OR S1 and S2
> instead of a direct S2 override, thus letting the S1 bits that would
> otherwise not being conveyed outside of the core.

xor-ing them together would be more fun - and equally valid in this imp-def world!


> I guess we have no way to know until someone reports really bad memory
> corruption and loss of data. The architecture is totally broken here.

The alternative is to make this depend on the list of CPUs where we know the combining
behaviour. That isn't great either, as its an unmaintainable-and-outdated list of
all-cortex-cores.


>> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
>> index f8ceebe4982e..7bd90ea1c61f 100644
>> --- a/arch/arm64/kvm/hyp/pgtable.c
>> +++ b/arch/arm64/kvm/hyp/pgtable.c
>> @@ -540,6 +540,15 @@ u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)
>>  	 */
>>  	vtcr |= VTCR_EL2_HA;
>>  
>> +	/*
>> +	 * Enable PBHA for stage2 on systems that support it. The configured
>> +	 * value will always be 0, which is defined as the safe default
>> +	 * setting. On Cortex cores, enabling PBHA for stage2 effectively
>> +	 * disables it for stage1.
>> +	 */
>> +	if (cpus_have_final_cap(ARM64_HAS_PBHA))
>> +		vtcr = FIELD_PREP(VTCR_EL2_PBHA_MASK, 0xf);

> Err... Surely you mean 'vtcr |= FIELD_PREP(...)' here, right?

Gah!. I'm off to the stationery cupboard for a brown paper bag....



Thanks,

James

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 4/7] arm64: cpufeature: Enable PBHA bits for stage1
  2021-10-16 13:50   ` Marc Zyngier
@ 2021-10-18 17:26     ` James Morse
  0 siblings, 0 replies; 14+ messages in thread
From: James Morse @ 2021-10-18 17:26 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: linux-arm-kernel, kvmarm, Will Deacon, Catalin Marinas

Hi Marc,

On 16/10/2021 14:50, Marc Zyngier wrote:
> On Fri, 15 Oct 2021 17:14:13 +0100,
> James Morse <james.morse@arm.com> wrote:
>>
>> If the CPUs support HPDS2, and there is a DT description of PBHA values
>> that only affect performance, enable those bits for both TTBR0 and TTBR1.
>>
>> Enabling PBHA requires the hierarchical-permissions to be disabled.
>> Commit 87143f404f33 ("arm64: mm: use XN table mapping attributes for
>> the linear region") used these, but only as an optimisation.
>>
>> Only the necessary PBHA bits are enabled to reduce the risk of an
>> unsafe bit/value being used by accident.

>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index 8694f9dec5e5..548c6f96a878 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c

>> @@ -1676,6 +1679,71 @@ static bool has_hw_dbm(const struct arm64_cpu_capabilities *cap,

>> +static bool plat_can_use_pbha_stage1(const struct arm64_cpu_capabilities *cap,
>> +				     int scope)
>> +{
>> +	u8 val;
>> +	struct device_node *cpus;
>> +	const u8 *perf_only_vals;
>> +	int num_perf_only_vals, i;
>> +
>> +	if (!has_cpuid_feature(cap, scope))
>> +		return false;
>> +
>> +	/*
>> +	 * Calls with scope == SCOPE_LOCAL_CPU need only testing whether this
>> +	 * cpu has the feature. A later 'system' scope call will check for a
>> +	 * firmware description.
>> +	 */
>> +	if (scope == SCOPE_LOCAL_CPU)
>> +		return true;
>> +
>> +	cpus = of_find_node_by_path("/cpus");
>> +	if (!cpus)
>> +		goto done;
>> +
>> +	perf_only_vals = of_get_property(cpus, "arm,pbha-performance-only",
>> +					 &num_perf_only_vals);
>> +	if (!perf_only_vals)
>> +		goto done;
>> +
>> +	/* any listed value is usable at stage 1 */
>> +	for (i = 0 ; i < num_perf_only_vals; i++) {
>> +		val = perf_only_vals[i];
>> +		if (val > 0xf)
>> +			continue;
>> +
>> +		pbha_stage1_enable_bits |= val;
>> +		set_bit(val, &arm64_pbha_perf_only_values);
>> +	}

> Somehow, this would need to be exposed to userspace so that a VMM
> could tell a guest what it can use.

I'm assuming any user is very soc-specific... but it would help the VMM to know.

I guess KVM could return the bitmap as KVM_CAP_PBHA. There is no way to tell the VMM what
the bits do, as that is imp-def...


Thanks,

James

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-10-18 17:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-15 16:14 [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values James Morse
2021-10-15 16:14 ` [RFC PATCH 1/7] KVM: arm64: Detect and enable PBHA for stage2 James Morse
2021-10-16 13:27   ` Marc Zyngier
2021-10-18 17:26     ` James Morse
2021-10-15 16:14 ` [RFC PATCH 2/7] dt-bindings: Rename the description of cpu nodes cpu.yaml James Morse
2021-10-15 16:14 ` [RFC PATCH 3/7] dt-bindings: arm: Add binding for Page Based Hardware Attributes James Morse
2021-10-15 16:14 ` [RFC PATCH 4/7] arm64: cpufeature: Enable PBHA bits for stage1 James Morse
2021-10-16 13:50   ` Marc Zyngier
2021-10-18 17:26     ` James Morse
2021-10-15 16:14 ` [RFC PATCH 5/7] arm64: mm: Add pgprot_pbha() to allow drivers to request PBHA values James Morse
2021-10-15 16:14 ` [RFC PATCH 6/7] KVM: arm64: Configure PBHA bits for stage2 James Morse
2021-10-16 13:46   ` Marc Zyngier
2021-10-15 16:14 ` [RFC PATCH 7/7] Documentation: arm64: Describe the support and expectations for PBHA James Morse
2021-10-16 13:54 ` [RFC PATCH 0/7] arm64: mm: Prototype to allow drivers to request PBHA values Marc Zyngier

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).