On 10/04/2024 22:27, Beata Michalska wrote:
On Wed, Apr 10, 2024 at 08:56:39PM +0200, Beata Michalska wrote:
On Fri, Mar 01, 2024 at 11:15:26AM +0100, Kevin Brodsky wrote:
On 26/02/2024 09:20, Beata Michalska wrote:
Extend the definition of kvm_pgtabe_prot enum to incorporate PBHA bits for both stage-1 and stage-2 page tables. At this point, those are being enabled only for Morello, which implements FEAT_HPDS2 feature by default and as such does not require any additional checks for the above mentioned feature status.
Signed-off-by: Beata Michalska beata.michalska@arm.com
arch/arm64/include/asm/kvm_arm.h | 3 +++ arch/arm64/include/asm/kvm_pgtable.h | 20 ++++++++++++++++++++ arch/arm64/kvm/arm.c | 2 ++ arch/arm64/kvm/hyp/pgtable.c | 10 ++++++++-- 4 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h index e80e1b7a91e0..010d837f8f1d 100644 --- a/arch/arm64/include/asm/kvm_arm.h +++ b/arch/arm64/include/asm/kvm_arm.h @@ -132,6 +132,9 @@ #define VTCR_EL2_HWU61 TCR_EL2_HWU61 #define VTCR_EL2_HWU60 TCR_EL2_HWU60 #define VTCR_EL2_HWU59 TCR_EL2_HWU59 +#define VTCR_EL2_PBHA_SHIFT 25
Maybe use VTCR_EL2_HWU59 instead of 25?
Could do though in majority of cases the XXX_SHIFT(s) are being defined explicitely across this file, though no strong objections to the idea. Will update that in v2, but will use VTCR_EL2_HWU59 instead.
Replying too fast I guess: First of all, you did suggest VTCR_EL2_HWU59. Not sure why I did read TCR_EL2_HWU59 .... Secondly: This needs to stay as is as this is needed for VTCR_EL2_PBHA to represent all HW bits: VTCR_EL2_HWU59 represets single bit set.
Yes right VTCR_EL2_HWU59 is 1 << 25, not 25. Sorry for the noise.
BR Beata
+#define VTCR_EL2_PBHA_MASK (0xf << VTCR_EL2_PBHA_SHIFT) +#define VTCR_EL2_PBHA VTCR_EL2_PBHA_MASK #define VTCR_EL2_HD (1 << 22) #define VTCR_EL2_HA (1 << 21) #define VTCR_EL2_PS_SHIFT TCR_EL2_PS_SHIFT diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h index d3e354bb8351..97ff8a2634d5 100644 --- a/arch/arm64/include/asm/kvm_pgtable.h +++ b/arch/arm64/include/asm/kvm_pgtable.h @@ -173,6 +173,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_HW0: PBHA[0]
- @KVM_PGTABLE_PROT_HW1: PBHA[1]
- @KVM_PGTABLE_PROT_HW2: PBHA[2]
*/
- @KVM_PGTABLE_PROT_HW3: PBHA[3]
enum kvm_pgtable_prot { KVM_PGTABLE_PROT_X = BIT(0), @@ -185,17 +189,33 @@ 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_HW0 = BIT(59),
- KVM_PGTABLE_PROT_HW1 = BIT(60),
- KVM_PGTABLE_PROT_HW2 = BIT(61),
- KVM_PGTABLE_PROT_HW3 = BIT(62),
}; +#ifdef CONFIG_ARM64_MORELLO
To reduce the #ifdef'ing, we could use a similar approach as in <asm/pgtable-prot.h>, that is define MAYBE macros that expand to 0 in !MORELLO. The other advantage would be to make it clearer what each bit does, e.g. KVM_PGTABLE_PROT_MAYBE_LS_CAPS. Getting pretty long though, maybe we could shorten the prefix...
Right, not really a big fan of XXX_MAYBE_YYY really: what MAYBE is referring to actually ? also we would need separate defines for RO and still that requires #ifdef'ing any way so not sure what we will be gaining here ? (apart from trading off one #ifdef block ). Having the HW bits being somehow mapped through the naming to their actual meaning is probably a good idea. Let me ponder a bit to see if I can improve it somehow.
MAYBE refers to the fact that it may expand to nothing, there is a precedent with PTE_MAYBE_NG. Either way I'm fine with keeping the #ifdef'ing. It would still be good to define macros that clarify what those bits actually do in the Morello case, like PTE_LOAD_CAPS and PTE_STORE_CAPS, instead of using KVM_PGTABLE_PROT_HW* directly.
Kevin
+#define KVM_PGTABLE_PROT_RW (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W | \
KVM_PGTABLE_PROT_HW1 | KVM_PGTABLE_PROT_HW2)+#else #define KVM_PGTABLE_PROT_RW (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W) +#endif #define KVM_PGTABLE_PROT_RWX (KVM_PGTABLE_PROT_RW | KVM_PGTABLE_PROT_X) #define PKVM_HOST_MEM_PROT KVM_PGTABLE_PROT_RWX #define PKVM_HOST_MMIO_PROT KVM_PGTABLE_PROT_RW #define PAGE_HYP KVM_PGTABLE_PROT_RW +#ifdef CONFIG_ARM64_MORELLO +#define PAGE_HYP_EXEC (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_X | \
KVM_PGTABLE_PROT_HW2)+#define PAGE_HYP_RO (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_HW2) +#else #define PAGE_HYP_EXEC (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_X) #define PAGE_HYP_RO (KVM_PGTABLE_PROT_R) +#endif #define PAGE_HYP_DEVICE (PAGE_HYP | KVM_PGTABLE_PROT_DEVICE) typedef bool (*kvm_pgtable_force_pte_cb_t)(u64 addr, u64 end,