In order to avoid declaration ambiguity with more than one capability pointer levels, provide explicit __capability annotations, thus avoiding compilation issues with implicit casting. NOTE: This is far from being perfect and should be done in a more elegant manner but for now it has to do.

Signed-off-by: Beata Michalska beata.michalska@arm.com --- arch/arm64/kvm/sys_regs.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 4735e1b37fb3..1c5d24fb7733 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -3653,7 +3653,12 @@ static u64 sys_reg_to_index(const struct sys_reg_desc *reg) (reg->Op2 << KVM_REG_ARM64_SYSREG_OP2_SHIFT)); }

-static bool copy_reg_to_user(const struct sys_reg_desc *reg, u64 __user **uind) +static bool copy_reg_to_user(const struct sys_reg_desc *reg, +#ifdef CONFIG_CHERI_PURECAP_UABI + u64 * __capability * uind) +#else + u64 __user **uind) +#endif { if (!*uind) return true; @@ -3667,7 +3672,11 @@ static bool copy_reg_to_user(const struct sys_reg_desc *reg, u64 __user **uind)

static int walk_one_sys_reg(const struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, +#ifdef CONFIG_CHERI_PURECAP_UABI + u64 * __capability * uind, +#else u64 __user **uind, +#endif unsigned int *total) { /*

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 +#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 +#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, diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 53ad8de28f48..bc945eac681d 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -1859,6 +1859,8 @@ static void __init cpu_prepare_hyp_mode(int cpu, u32 hyp_va_bits) } tcr &= ~TCR_T0SZ_MASK; tcr |= TCR_T0SZ(hyp_va_bits); + if (IS_ENABLED(CONFIG_ARM64_MORELLO)) + tcr |= TCR_EL2_HWU61 | TCR_EL2_HWU60 | TCR_EL2_HPD; params->tcr_el2 = tcr;

params->pgd_pa = kvm_mmu_get_httbr(); diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c index 1966fdee740e..88d002d7152f 100644 --- a/arch/arm64/kvm/hyp/pgtable.c +++ b/arch/arm64/kvm/hyp/pgtable.c @@ -38,6 +38,7 @@

#define KVM_PTE_LEAF_ATTR_HI GENMASK(63, 50)

+#define KVM_PTE_LEAF_ATTR_HI_HW GENMASK(62, 59) #define KVM_PTE_LEAF_ATTR_HI_SW GENMASK(58, 55)

#define KVM_PTE_LEAF_ATTR_HI_S1_XN BIT(54) @@ -411,6 +412,7 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep) attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_SH, sh); attr |= KVM_PTE_LEAF_ATTR_LO_S1_AF; attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW; + attr |= prot & KVM_PTE_LEAF_ATTR_HI_HW; *ptep = attr;

return 0; @@ -418,7 +420,7 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep)

enum kvm_pgtable_prot kvm_pgtable_hyp_pte_prot(kvm_pte_t pte) { - enum kvm_pgtable_prot prot = pte & KVM_PTE_LEAF_ATTR_HI_SW; + enum kvm_pgtable_prot prot = pte & (KVM_PTE_LEAF_ATTR_HI_SW | KVM_PTE_LEAF_ATTR_HI_HW); u32 ap;

if (!kvm_pte_valid(pte)) @@ -654,6 +656,9 @@ u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift) vtcr |= VTCR_EL2_HA; #endif /* CONFIG_ARM64_HW_AFDBM */

+ if (IS_ENABLED(CONFIG_ARM64_MORELLO)) + vtcr |= VTCR_EL2_PBHA; + /* Set the vmid bits */ vtcr |= (get_vmid_bits(mmfr1) == 16) ? VTCR_EL2_VS_16BIT : @@ -714,6 +719,7 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh); attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF; attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW; + attr |= prot & KVM_PTE_LEAF_ATTR_HI_HW; *ptep = attr;

return 0; @@ -1299,7 +1305,7 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr, u32 level; kvm_pte_t set = 0, clr = 0;

- if (prot & KVM_PTE_LEAF_ATTR_HI_SW) + if (prot & KVM_PTE_LEAF_ATTR_HI_SW || prot & KVM_PTE_LEAF_ATTR_HI_HW) return -EINVAL;

if (prot & KVM_PGTABLE_PROT_R)

Provide capability-aware set of helper macros for system register read/writes. For the time being, if the transfer register is not a capability one, a prep step is being carried out to provide it through the use of CVTP instruction. In a long run this should be replace by proper setup where a source capability is derived from an appropriate authorizing capability.

Signed-off-by: Beata Michalska beata.michalska@arm.com --- arch/arm64/include/asm/sysreg.h | 55 +++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index e2109e2c0bef..c55c4c4fa945 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -1141,6 +1141,42 @@ : : "rZ" (__val)); \ } while (0)

+#ifdef CONFIG_ARM64_MORELLO +#define __convert_to_cap(v) \ +({ \ + uintcap_t __cval; \ + asm volatile("cvtp %0, %1" : "=r" (__cval) : "r" (v)); \ + __cval; \ +}) + +#define write_cap_sysreg(v, r) \ + do { \ + uintcap_t __cval; \ + __cval = __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(v), \ + uintcap_t), \ + (uintcap_t)(v), \ + __convert_to_cap(v) \ + ); \ + asm volatile("msr " __stringify(r) ", %x0" : : "rZ" (__cval)); \ + } while (0) + +/* + * Select appropriate variant of write_sysreg based on whether + * CONFIG_ARM64_MORELLO is enabled or not. + * For the capability system registers, unless capability is being provided, + * one is created through CVTP instruction. This immplies that this macro + * is not suitable for all use-cases. Additionally, this macro is based on + * the assumption that the capability system register name can be derived + * from corresponding aarch64 one. + */ +#define write_sysreg_variant(v, r) \ + IS_BUILTIN(CONFIG_ARM64_MORELLO) \ + ? ({ write_cap_sysreg(v, c##r); }) \ + : ({ write_sysreg(v, r); }) +#else +#define write_sysreg_variant write_sysreg +#endif /* * For registers without architectural names, or simply unsupported by * GAS. @@ -1162,6 +1198,25 @@ asm volatile(__msr_s(r, "%x0") : : "rZ" (__val)); \ } while (0)

+#define read_cap_sysreg_s(r) ({ \ + uintcap_t __val; \ + u32 __maybe_unused __check_r = (u32)(r); \ + asm volatile(__mrs_s_c("%0", r) : "=r" (__val)); \ + __val; \ +}) + +#define write_cap_sysreg_s(v, r) do { \ + uintcap_t __cval; \ + u32 __maybe_unused __check_r = (u32)(r); \ + __cval = __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(v), \ + uintcap_t), \ + (uintcap_t)(v), \ + __convert_to_cap(v) \ + ); \ + asm volatile(__msr_s_c(r, "%x0") : : "rZ" (__cval)); \ +} while (0) + /* * Modify bits in a sysreg. Bits in the clear mask are zeroed, then bits in the * set mask are set. Other bits are left as-is.

Add bare minimum to enable saving & restoring relevant capability registers. Note that, at this point, only a subset of capability registers is to be handled,with the remaining ones being provided for completeness only.

Signed-off-by: Beata Michalska beata.michalska@arm.com --- arch/arm64/include/asm/kvm_asm.h | 6 +++++ arch/arm64/include/asm/kvm_host.h | 38 +++++++++++++++++++++++++++++++ arch/arm64/kernel/asm-offsets.c | 3 +++ 3 files changed, 47 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index 24b5e6b23417..4ca7ece15385 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -334,6 +334,12 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr, u64 elr_virt, #define CPU_XREG_OFFSET(x) (CPU_USER_PT_REGS + 8*x) #define CPU_LR_OFFSET CPU_XREG_OFFSET(30) #define CPU_SP_EL0_OFFSET (CPU_LR_OFFSET + 8) +#ifdef CONFIG_ARM64_MORELLO +#define CPU_CREG_OFFSET(x) (CPU_USER_CREGS + 16*x) +#define CPU_CLR_OFFSET CPU_CREG_OFFSET(30) +#define CPU_CSP_EL0_OFFSET (CPU_CLR_OFFSET + 16) +#define CPU_RCSP_EL0_OFFSET (CPU_CLR_OFFSET + 32) +#endif

/* * We treat x18 as callee-saved as the host may use it as a platform diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 824f29f04916..808645b7108d 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -451,9 +451,40 @@ enum vcpu_sysreg { NR_SYS_REGS /* Nothing after this line! */ };

+#ifdef CONFIG_ARM64_MORELLO +enum vcpu_cap_sysregs { + __INVALID_CAP_SYSREG__, + CTPIDR_EL0, + RTPIDR_EL0, + CTPIDRRO_EL0, + + CTPIDR_EL1, + CELR_EL1, + CVBAR_EL1, + CSP_EL1, + DDC_EL1, + + CELR_EL2, + CVBAR_EL2, + CTPIDR_EL2, + CSP_EL2, + DDC_EL2, + + NR_CAP_SYS_REGS +}; +#endif + struct kvm_cpu_context { struct user_pt_regs regs; /* sp = sp_el0 */

+ /* This should probably end up in user_pt_regs at one point */ +#ifdef CONFIG_ARM64_MORELLO + uintcap_t cregs[31]; + uintcap_t csp; + uintcap_t rcsp; + uintcap_t pcc; +#endif + u64 spsr_abt; u64 spsr_und; u64 spsr_irq; @@ -463,6 +494,9 @@ struct kvm_cpu_context {

u64 sys_regs[NR_SYS_REGS];

+#ifdef CONFIG_ARM64_MORELLO + uintcap_t cap_sys_regs[NR_CAP_SYS_REGS]; +#endif struct kvm_vcpu *__hyp_running_vcpu; };

@@ -833,6 +867,10 @@ struct kvm_vcpu_arch {

#define __vcpu_sys_reg(v,r) (ctxt_sys_reg(&(v)->arch.ctxt, (r)))

+#ifdef CONFIG_ARM64_MORELLO +#define __ctxt_cap_sys_reg(c, r) (&(c)->cap_sys_regs[(r)]) +#define ctxt_cap_sys_reg(c, r) (*__ctxt_cap_sys_reg(c,r)) +#endif u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg); void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg);

diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index 90ca25039583..249bf3ca2fbc 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -143,6 +143,9 @@ int main(void) DEFINE(VCPU_FAULT_DISR, offsetof(struct kvm_vcpu, arch.fault.disr_el1)); DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu, arch.hcr_el2)); DEFINE(CPU_USER_PT_REGS, offsetof(struct kvm_cpu_context, regs)); +#ifdef CONFIG_ARM64_MORELLO + DEFINE(CPU_USER_CREGS, offsetof(struct kvm_cpu_context, cregs)); +#endif DEFINE(CPU_RGSR_EL1, offsetof(struct kvm_cpu_context, sys_regs[RGSR_EL1])); DEFINE(CPU_GCR_EL1, offsetof(struct kvm_cpu_context, sys_regs[GCR_EL1])); DEFINE(CPU_APIAKEYLO_EL1, offsetof(struct kvm_cpu_context, sys_regs[APIAKEYLO_EL1]));

Disable trapping of Morello instructions on context switch.

Signed-off-by: Beata Michalska beata.michalska@arm.com --- arch/arm64/include/asm/kvm_emulate.h | 5 +++++ arch/arm64/kvm/hyp/nvhe/switch.c | 5 +++++ arch/arm64/kvm/hyp/vhe/switch.c | 7 +++++++ 3 files changed, 17 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index 78a550537b67..6ce5d7c96cad 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -587,6 +587,9 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu) CPACR_EL1_ZEN_EL1EN); if (cpus_have_final_cap(ARM64_SME)) val |= CPACR_EL1_SMEN_EL1EN; + if (IS_ENABLED(CONFIG_ARM64_MORELLO)) + val |= CPACR_EL1_CEN; + } else if (has_hvhe()) { val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN);

@@ -603,6 +606,8 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu) val |= CPTR_EL2_TZ; if (cpus_have_final_cap(ARM64_SME)) val &= ~CPTR_EL2_TSM; + if (IS_ENABLED(CONFIG_ARM64_MORELLO)) + val &= ~CPTR_EL2_TC; }

return val; diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c index e19243367408..7c4363b0691b 100644 --- a/arch/arm64/kvm/hyp/nvhe/switch.c +++ b/arch/arm64/kvm/hyp/nvhe/switch.c @@ -63,7 +63,12 @@ static void __activate_traps(struct kvm_vcpu *vcpu) __activate_traps_fpsimd32(vcpu); }

+ /* Disable trapping of Morello instructions */ + if (IS_ENABLED(CONFIG_ARM64_MORELLO)) + val &= ~CPTR_EL2_TC; + kvm_write_cptr_el2(val); + write_sysreg_variant(__this_cpu_read(kvm_hyp_vector), vbar_el2);

if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) { diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c index e4fc314a83a6..6a2540ca30aa 100644 --- a/arch/arm64/kvm/hyp/vhe/switch.c +++ b/arch/arm64/kvm/hyp/vhe/switch.c @@ -75,6 +75,13 @@ static void __activate_traps(struct kvm_vcpu *vcpu)

val |= CPTR_EL2_TAM;

+ /* + * Disable trapping Morello instructions + * For VHE (HCR.E2H) CPCR_EL1.CEN bits have the same position as + * CPTR_EL2.CEN + */ + val |= CPACR_EL1_CEN; + if (guest_owns_fp_regs(vcpu)) { if (vcpu_has_sve(vcpu)) val |= CPACR_EL1_ZEN_EL0EN | CPACR_EL1_ZEN_EL1EN;

Add support for managing kvm virtual machines through plain aarch64 tools. No support for purecap guests just yet.

Signed-off-by: Beata Michalska beata.michalska@arm.com --- Documentation/virt/kvm/api.rst | 10 +- arch/arm64/include/uapi/asm/kvm.h | 6 +- arch/arm64/kvm/arch_timer.c | 4 +- arch/arm64/kvm/arm.c | 205 ++++++++++++++++-------- arch/arm64/kvm/guest.c | 10 +- arch/arm64/kvm/hypercalls.c | 4 +- arch/arm64/kvm/pmu-emul.c | 8 +- arch/arm64/kvm/sys_regs.c | 10 +- arch/arm64/kvm/vgic/vgic-its.c | 8 +- arch/arm64/kvm/vgic/vgic-kvm-device.c | 11 +- include/linux/kvm_host.h | 8 +- include/uapi/linux/kvm.h | 10 +- tools/arch/arm64/include/uapi/asm/kvm.h | 6 +- tools/include/uapi/linux/kvm.h | 12 +- virt/kvm/Kconfig | 2 +- virt/kvm/kvm_main.c | 99 +++++++++--- virt/kvm/vfio.c | 2 +- 17 files changed, 272 insertions(+), 143 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 7025b3751027..7a8370fbb357 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -2308,8 +2308,8 @@ code being returned in a specific situation.) ::

struct kvm_one_reg { - __u64 id; - __u64 addr; + __u64 id; + __kernel_uintptr_t addr; };

Using this ioctl, a single vcpu register can be set to a specific value @@ -6160,9 +6160,9 @@ applied. #define KVM_ARM_FEATURE_ID_RANGE_SIZE (3 * 8 * 8)

struct reg_mask_range { - __u64 addr; /* Pointer to mask array */ - __u32 range; /* Requested range */ - __u32 reserved[13]; + __kernel_uintptr_t addr; /* Pointer to mask array */ + __u32 range; /* Requested range */ + __u32 reserved[13]; };

This ioctl copies the writable masks for a selected range of registers to diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 89d2fc872d9f..4f854ef1f76d 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -532,9 +532,9 @@ struct kvm_smccc_filter { #define KVM_ARM_FEATURE_ID_RANGE_SIZE (3 * 8 * 8)

struct reg_mask_range { - __u64 addr; /* Pointer to mask array */ - __u32 range; /* Requested range */ - __u32 reserved[13]; + __kernel_uintptr_t addr; /* Pointer to mask array */ + __u32 range; /* Requested range */ + __u32 reserved[13]; };

#endif diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c index 13ba691b848f..8b70eeeebf33 100644 --- a/arch/arm64/kvm/arch_timer.c +++ b/arch/arm64/kvm/arch_timer.c @@ -1559,7 +1559,7 @@ void kvm_timer_init_vhe(void)

int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) { - int __user *uaddr = (int __user *)(long)attr->addr; + int __user *uaddr = (int __user *)attr->addr; int irq, idx, ret = 0;

if (!irqchip_in_kernel(vcpu->kvm)) @@ -1611,7 +1611,7 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)

int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) { - int __user *uaddr = (int __user *)(long)attr->addr; + int __user *uaddr = (int __user *)attr->addr; struct arch_timer_context *timer; int irq;

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index bc945eac681d..a49f7603f03e 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -1512,33 +1512,38 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl, user_uintptr_t arg) { struct kvm_vcpu *vcpu = filp->private_data; - void __user *argp = (void __user *)arg; + void __user *argp; struct kvm_device_attr attr; long r;

- switch (ioctl) { - case KVM_ARM_VCPU_INIT: { - struct kvm_vcpu_init init; - - r = -EFAULT; - if (copy_from_user(&init, argp, sizeof(init))) - break; + argp = in_compat64_syscall() ? compat_ptr(arg) : (void __user *)arg;

- r = kvm_arch_vcpu_ioctl_vcpu_init(vcpu, &init); - break; - } - case KVM_SET_ONE_REG: - case KVM_GET_ONE_REG: { + switch (_IOC_NR(ioctl)) { + case _IOC_NR(KVM_SET_ONE_REG): + case _IOC_NR(KVM_GET_ONE_REG): { struct kvm_one_reg reg;

r = -ENOEXEC; if (unlikely(!kvm_vcpu_initialized(vcpu))) - break; + goto done;

r = -EFAULT; - if (copy_from_user(&reg, argp, sizeof(reg))) - break;

+ if (!in_compat64_syscall()) { + if (copy_from_user_with_ptr(&reg, argp, sizeof(reg))) + goto done; + } else { + struct { + __u64 id; + compat_uptr_t addr; + } __reg; + + if (copy_from_user(&__reg, argp, sizeof(__reg))) + goto done; + + reg.id = __reg.id; + reg.addr = (__kernel_uintptr_t)compat_ptr(__reg.addr); + } /* * We could owe a reset due to PSCI. Handle the pending reset * here to ensure userspace register accesses are ordered after @@ -1547,10 +1552,47 @@ long kvm_arch_vcpu_ioctl(struct file *filp, if (kvm_check_request(KVM_REQ_VCPU_RESET, vcpu)) kvm_reset_vcpu(vcpu);

- if (ioctl == KVM_SET_ONE_REG) + if (_IOC_NR(ioctl) == _IOC_NR(KVM_SET_ONE_REG)) r = kvm_arm_set_reg(vcpu, &reg); else r = kvm_arm_get_reg(vcpu, &reg); + + goto done; + } + case _IOC_NR(KVM_SET_DEVICE_ATTR): + case _IOC_NR(KVM_GET_DEVICE_ATTR): + case _IOC_NR(KVM_HAS_DEVICE_ATTR): { + r = -EFAULT; + + if (!in_compat64_syscall()) { + if (copy_from_user_with_ptr(&attr, argp, sizeof(attr))) + goto done; + } else { + r = kvm_get_device_attr_compat(ioctl, arg, &attr); + if (r) goto done; + } + + if (_IOC_NR(ioctl) == _IOC_NR(KVM_SET_DEVICE_ATTR)) + r = kvm_arm_vcpu_set_attr(vcpu, &attr); + else if (_IOC_NR(ioctl) == _IOC_NR(KVM_GET_DEVICE_ATTR)) + r = kvm_arm_vcpu_get_attr(vcpu, &attr); + else + r = kvm_arm_vcpu_has_attr(vcpu, &attr); + goto done; + } + default: + break; + } + + switch (ioctl) { + case KVM_ARM_VCPU_INIT: { + struct kvm_vcpu_init init; + + r = -EFAULT; + if (copy_from_user(&init, argp, sizeof(init))) + break; + + r = kvm_arch_vcpu_ioctl_vcpu_init(vcpu, &init); break; } case KVM_GET_REG_LIST: { @@ -1579,27 +1621,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp, r = kvm_arm_copy_reg_indices(vcpu, user_list->reg); break; } - case KVM_SET_DEVICE_ATTR: { - r = -EFAULT; - if (copy_from_user(&attr, argp, sizeof(attr))) - break; - r = kvm_arm_vcpu_set_attr(vcpu, &attr); - break; - } - case KVM_GET_DEVICE_ATTR: { - r = -EFAULT; - if (copy_from_user(&attr, argp, sizeof(attr))) - break; - r = kvm_arm_vcpu_get_attr(vcpu, &attr); - break; - } - case KVM_HAS_DEVICE_ATTR: { - r = -EFAULT; - if (copy_from_user(&attr, argp, sizeof(attr))) - break; - r = kvm_arm_vcpu_has_attr(vcpu, &attr); - break; - } case KVM_GET_VCPU_EVENTS: { struct kvm_vcpu_events events;

@@ -1633,7 +1654,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp, default: r = -EINVAL; } - +done: return r; }

@@ -1678,9 +1699,88 @@ static int kvm_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr) int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, user_uintptr_t arg) { struct kvm *kvm = filp->private_data; - void __user *argp = (void __user *)arg; + void __user *argp; struct kvm_device_attr attr;

+ argp = in_compat64_syscall() ? compat_ptr(arg) : (void __user *)arg; + + switch (_IOC_NR(ioctl)) { + case _IOC_NR(KVM_ARM_MTE_COPY_TAGS): { + struct kvm_arm_copy_mte_tags copy_tags; + + if (!in_compat64_syscall()) { + if (copy_from_user_with_ptr(&copy_tags, argp, + sizeof(copy_tags))) + return -EFAULT; + } else { + struct { + __u64 guest_ipa; + __u64 length; + compat_uptr_t addr; + __u64 flags; + __u64 reserved[2]; + } __copy_tags; + + if (copy_from_user(&__copy_tags, argp, + sizeof(__copy_tags))) + return -EFAULT; + + copy_tags.guest_ipa = __copy_tags.guest_ipa; + copy_tags.length = __copy_tags.length; + copy_tags.addr = compat_ptr(__copy_tags.addr); + copy_tags.flags = __copy_tags.flags; + + memcpy(copy_tags.reserved, __copy_tags.reserved, + sizeof(__copy_tags.reserved)); + } + + return kvm_vm_ioctl_mte_copy_tags(kvm, &copy_tags); + } + case _IOC_NR(KVM_HAS_DEVICE_ATTR): + case _IOC_NR(KVM_SET_DEVICE_ATTR): { + if (!in_compat64_syscall()) { + if (copy_from_user_with_ptr(&attr, argp, sizeof(attr))) + return -EFAULT; + } else { + int ret; + + ret = kvm_get_device_attr_compat(ioctl, arg, &attr); + if (ret) + return ret; + } + + return _IOC_NR(ioctl) == _IOC_NR(KVM_HAS_DEVICE_ATTR) + ? kvm_vm_has_attr(kvm, &attr) + : kvm_vm_set_attr(kvm, &attr); + } + case _IOC_NR(KVM_ARM_GET_REG_WRITABLE_MASKS): { + struct reg_mask_range range; + + if (!in_compat64_syscall()) { + if (copy_from_user_with_ptr(&range, argp, sizeof(range))) + return -EFAULT; + } else { + struct reg_mask_range { + compat_uptr_t addr; + __u32 range; + __u32 reserved[13]; + } __range; + + if (copy_from_user(&__range, argp, sizeof(__range))) + return -EFAULT; + + range.addr = (__kernel_uintptr_t)compat_ptr(__range.addr); + range.range = __range.range; + + memcpy(range.reserved, __range.reserved, + sizeof(__range.reserved)); + } + return kvm_vm_ioctl_get_reg_writable_masks(kvm, &range); + } + default: + break; + } + switch (ioctl) { case KVM_CREATE_IRQCHIP: { int ret; @@ -1708,13 +1808,6 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, user_uintptr_t arg)

return 0; } - case KVM_ARM_MTE_COPY_TAGS: { - struct kvm_arm_copy_mte_tags copy_tags; - - if (copy_from_user(&copy_tags, argp, sizeof(copy_tags))) - return -EFAULT; - return kvm_vm_ioctl_mte_copy_tags(kvm, &copy_tags); - } case KVM_ARM_SET_COUNTER_OFFSET: { struct kvm_arm_counter_offset offset;

@@ -1722,25 +1815,7 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, user_uintptr_t arg) return -EFAULT; return kvm_vm_ioctl_set_counter_offset(kvm, &offset); } - case KVM_HAS_DEVICE_ATTR: { - if (copy_from_user(&attr, argp, sizeof(attr))) - return -EFAULT; - - return kvm_vm_has_attr(kvm, &attr); - } - case KVM_SET_DEVICE_ATTR: { - if (copy_from_user(&attr, argp, sizeof(attr))) - return -EFAULT;

- return kvm_vm_set_attr(kvm, &attr); - } - case KVM_ARM_GET_REG_WRITABLE_MASKS: { - struct reg_mask_range range; - - if (copy_from_user(&range, argp, sizeof(range))) - return -EFAULT; - return kvm_vm_ioctl_get_reg_writable_masks(kvm, &range); - } default: return -EINVAL; } diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index aaf1d4939739..13c7813ed9ca 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -191,7 +191,7 @@ static int get_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) * array. Hence below, nr_regs is the number of entries, and * off the index in the "array". */ - __u32 __user *uaddr = (__u32 __user *)(unsigned long)reg->addr; + __u32 __user *uaddr = (__u32 __user *)reg->addr; int nr_regs = sizeof(struct kvm_regs) / sizeof(__u32); void *addr; u32 off; @@ -214,7 +214,7 @@ static int get_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)

static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { - __u32 __user *uaddr = (__u32 __user *)(unsigned long)reg->addr; + __u32 __user *uaddr = (__u32 __user *)reg->addr; int nr_regs = sizeof(struct kvm_regs) / sizeof(__u32); __uint128_t tmp; void *valp = &tmp, *addr; @@ -628,7 +628,7 @@ static int copy_timer_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)

static int set_timer_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { - void __user *uaddr = (void __user *)(long)reg->addr; + void __user *uaddr = (void __user *)reg->addr; u64 val; int ret;

@@ -641,7 +641,7 @@ static int set_timer_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)

static int get_timer_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { - void __user *uaddr = (void __user *)(long)reg->addr; + void __user *uaddr = (void __user *)reg->addr; u64 val;

val = kvm_arm_timer_get_reg(vcpu, reg->id); @@ -786,7 +786,7 @@ int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) return -EINVAL;

switch (reg->id & KVM_REG_ARM_COPROC_MASK) { - case KVM_REG_ARM_CORE: return set_core_reg(vcpu, reg); + case KVM_REG_ARM_CORE: return set_core_reg(vcpu, reg); case KVM_REG_ARM_FW: case KVM_REG_ARM_FW_FEAT_BMAP: return kvm_arm_set_fw_reg(vcpu, reg); diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c index 5763d979d8ca..8d3c96be4a4f 100644 --- a/arch/arm64/kvm/hypercalls.c +++ b/arch/arm64/kvm/hypercalls.c @@ -476,7 +476,7 @@ static int get_kernel_wa_level(u64 regid) int kvm_arm_get_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { struct kvm_smccc_features *smccc_feat = &vcpu->kvm->arch.smccc_feat; - void __user *uaddr = (void __user *)(long)reg->addr; + void __user *uaddr = (void __user *)reg->addr; u64 val;

switch (reg->id) { @@ -550,7 +550,7 @@ static int kvm_arm_set_fw_reg_bmap(struct kvm_vcpu *vcpu, u64 reg_id, u64 val)

int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { - void __user *uaddr = (void __user *)(long)reg->addr; + void __user *uaddr = (void __user *)reg->addr; u64 val; int wa_level;

diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index fe99b3dab6ce..5ff14b6dedc1 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -992,7 +992,7 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)

switch (attr->attr) { case KVM_ARM_VCPU_PMU_V3_IRQ: { - int __user *uaddr = (int __user *)(long)attr->addr; + int __user *uaddr = (int __user *)attr->addr; int irq;

if (!irqchip_in_kernel(kvm)) @@ -1028,7 +1028,7 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) */ nr_events = __kvm_pmu_event_mask(pmuver) + 1;

- uaddr = (struct kvm_pmu_event_filter __user *)(long)attr->addr; + uaddr = (struct kvm_pmu_event_filter __user *)attr->addr;

if (copy_from_user(&filter, uaddr, sizeof(filter))) return -EFAULT; @@ -1066,7 +1066,7 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) return 0; } case KVM_ARM_VCPU_PMU_V3_SET_PMU: { - int __user *uaddr = (int __user *)(long)attr->addr; + int __user *uaddr = (int __user *)attr->addr; int pmu_id;

if (get_user(pmu_id, uaddr)) @@ -1085,7 +1085,7 @@ int kvm_arm_pmu_v3_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) { switch (attr->attr) { case KVM_ARM_VCPU_PMU_V3_IRQ: { - int __user *uaddr = (int __user *)(long)attr->addr; + int __user *uaddr = (int __user *)attr->addr; int irq;

if (!irqchip_in_kernel(vcpu->kvm)) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 1c5d24fb7733..2146e2e8ace4 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -3541,7 +3541,7 @@ static int demux_c15_set(struct kvm_vcpu *vcpu, u64 id, void __user *uaddr) int kvm_sys_reg_get_user(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg, const struct sys_reg_desc table[], unsigned int num) { - u64 __user *uaddr = (u64 __user *)(unsigned long)reg->addr; + u64 __user *uaddr = (u64 __user *)reg->addr; const struct sys_reg_desc *r; u64 val; int ret; @@ -3559,13 +3559,12 @@ int kvm_sys_reg_get_user(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg,

if (!ret) ret = put_user(val, uaddr); - return ret; }

int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { - void __user *uaddr = (void __user *)(unsigned long)reg->addr; + void __user *uaddr = (void __user *)reg->addr; int err;

if ((reg->id & KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX) @@ -3582,7 +3581,7 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg int kvm_sys_reg_set_user(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg, const struct sys_reg_desc table[], unsigned int num) { - u64 __user *uaddr = (u64 __user *)(unsigned long)reg->addr; + u64 __user *uaddr = (u64 __user *)reg->addr; const struct sys_reg_desc *r; u64 val; int ret; @@ -3609,7 +3608,7 @@ int kvm_sys_reg_set_user(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg,

int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { - void __user *uaddr = (void __user *)(unsigned long)reg->addr; + void __user *uaddr = (void __user *)reg->addr; int err;

if ((reg->id & KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX) @@ -3618,7 +3617,6 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg err = set_invariant_sys_reg(reg->id, uaddr); if (err != -ENOENT) return err; - return kvm_sys_reg_set_user(vcpu, reg, sys_reg_descs, ARRAY_SIZE(sys_reg_descs)); } diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c index 2dad2d095160..adea62ba0fe6 100644 --- a/arch/arm64/kvm/vgic/vgic-its.c +++ b/arch/arm64/kvm/vgic/vgic-its.c @@ -2826,7 +2826,7 @@ static int vgic_its_set_attr(struct kvm_device *dev,

switch (attr->group) { case KVM_DEV_ARM_VGIC_GRP_ADDR: { - u64 __user *uaddr = (u64 __user *)(long)attr->addr; + u64 __user *uaddr = (u64 __user *)attr->addr; unsigned long type = (unsigned long)attr->attr; u64 addr;

@@ -2846,7 +2846,7 @@ static int vgic_its_set_attr(struct kvm_device *dev, case KVM_DEV_ARM_VGIC_GRP_CTRL: return vgic_its_ctrl(dev->kvm, its, attr->attr); case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { - u64 __user *uaddr = (u64 __user *)(long)attr->addr; + u64 __user *uaddr = (u64 __user *)attr->addr; u64 reg;

if (get_user(reg, uaddr)) @@ -2865,7 +2865,7 @@ static int vgic_its_get_attr(struct kvm_device *dev, case KVM_DEV_ARM_VGIC_GRP_ADDR: { struct vgic_its *its = dev->private; u64 addr = its->vgic_its_base; - u64 __user *uaddr = (u64 __user *)(long)attr->addr; + u64 __user *uaddr = (u64 __user *)attr->addr; unsigned long type = (unsigned long)attr->attr;

if (type != KVM_VGIC_ITS_ADDR_TYPE) @@ -2876,7 +2876,7 @@ static int vgic_its_get_attr(struct kvm_device *dev, break; } case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { - u64 __user *uaddr = (u64 __user *)(long)attr->addr; + u64 __user *uaddr = (u64 __user *)attr->addr; u64 reg; int ret;

diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c index f48b8dab8b3d..6608a25c95bc 100644 --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c @@ -216,7 +216,7 @@ static int vgic_set_common_attr(struct kvm_device *dev, r = kvm_vgic_addr(dev->kvm, attr, true); return (r == -ENODEV) ? -ENXIO : r; case KVM_DEV_ARM_VGIC_GRP_NR_IRQS: { - u32 __user *uaddr = (u32 __user *)(long)attr->addr; + u32 __user *uaddr = (u32 __user *)attr->addr; u32 val; int ret = 0;

@@ -292,7 +292,7 @@ static int vgic_get_common_attr(struct kvm_device *dev, r = kvm_vgic_addr(dev->kvm, attr, false); return (r == -ENODEV) ? -ENXIO : r; case KVM_DEV_ARM_VGIC_GRP_NR_IRQS: { - u32 __user *uaddr = (u32 __user *)(long)attr->addr; + u32 __user *uaddr = (u32 __user *)attr->addr;

r = put_user(dev->kvm->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS, uaddr); @@ -359,7 +359,7 @@ static int vgic_v2_attr_regs_access(struct kvm_device *dev, struct kvm_device_attr *attr, bool is_write) { - u32 __user *uaddr = (u32 __user *)(unsigned long)attr->addr; + u32 __user *uaddr = (u32 __user *)attr->addr; struct vgic_reg_attr reg_attr; gpa_t addr; struct kvm_vcpu *vcpu; @@ -533,7 +533,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev, }

if (uaccess && is_write) { - u32 __user *uaddr = (u32 __user *)(unsigned long)attr->addr; + u32 __user *uaddr = (u32 __user *)attr->addr; if (get_user(val, uaddr)) return -EFAULT; } @@ -552,6 +552,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev, goto out; }

+ switch (attr->group) { case KVM_DEV_ARM_VGIC_GRP_DIST_REGS: ret = vgic_v3_dist_uaccess(vcpu, is_write, addr, &val); @@ -588,7 +589,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev, mutex_unlock(&dev->kvm->lock);

if (!ret && uaccess && !is_write) { - u32 __user *uaddr = (u32 __user *)(unsigned long)attr->addr; + u32 __user *uaddr = (u32 __user *)attr->addr; ret = put_user(val, uaddr); }

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index b011b4ec2ddf..c2f743462148 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -1215,9 +1215,10 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc, #define __kvm_get_guest(kvm, gfn, offset, v) \ ({ \ unsigned long __addr = gfn_to_hva(kvm, gfn); \ - typeof(v) __user *__uaddr = (typeof(__uaddr))(__addr + offset); \ + typeof(v) __user *__uaddr; \ int __ret = -EFAULT; \ \ + __uaddr = uaddr_to_user_ptr(__addr + offset); \ if (!kvm_is_error_hva(__addr)) \ __ret = get_user(v, __uaddr); \ __ret; \ @@ -1235,9 +1236,10 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc, #define __kvm_put_guest(kvm, gfn, offset, v) \ ({ \ unsigned long __addr = gfn_to_hva(kvm, gfn); \ - typeof(v) __user *__uaddr = (typeof(__uaddr))(__addr + offset); \ + typeof(v) __user *__uaddr; \ int __ret = -EFAULT; \ \ + __uaddr = uaddr_to_user_ptr(__addr + offset); \ if (!kvm_is_error_hva(__addr)) \ __ret = put_user(v, __uaddr); \ if (!__ret) \ @@ -1405,6 +1407,8 @@ vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf);

int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext);

+int kvm_get_device_attr_compat(unsigned int ioctl, unsigned long arg, + struct kvm_device_attr *attr); void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm, struct kvm_memory_slot *slot, gfn_t gfn_offset, diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 211b86de35ac..1e80277bfef5 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1392,7 +1392,7 @@ struct kvm_reg_list {

struct kvm_one_reg { __u64 id; - __u64 addr; + __kernel_uintptr_t addr; };

#define KVM_MSI_VALID_DEVID (1U << 0) @@ -1422,10 +1422,10 @@ struct kvm_create_device { };

struct kvm_device_attr { - __u32 flags; /* no flags currently defined */ - __u32 group; /* device-defined */ - __u64 attr; /* group-defined */ - __u64 addr; /* userspace address of attr data */ + __u32 flags; /* no flags currently defined */ + __u32 group; /* device-defined */ + __u64 attr; /* group-defined */ + __kernel_uintptr_t addr; /* userspace address of attr data */ };

#define KVM_DEV_VFIO_FILE 1 diff --git a/tools/arch/arm64/include/uapi/asm/kvm.h b/tools/arch/arm64/include/uapi/asm/kvm.h index 89d2fc872d9f..4f854ef1f76d 100644 --- a/tools/arch/arm64/include/uapi/asm/kvm.h +++ b/tools/arch/arm64/include/uapi/asm/kvm.h @@ -532,9 +532,9 @@ struct kvm_smccc_filter { #define KVM_ARM_FEATURE_ID_RANGE_SIZE (3 * 8 * 8)

struct reg_mask_range { - __u64 addr; /* Pointer to mask array */ - __u32 range; /* Requested range */ - __u32 reserved[13]; + __kernel_uintptr_t addr; /* Pointer to mask array */ + __u32 range; /* Requested range */ + __u32 reserved[13]; };

#endif diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h index 211b86de35ac..fd70c2bdf989 100644 --- a/tools/include/uapi/linux/kvm.h +++ b/tools/include/uapi/linux/kvm.h @@ -1391,8 +1391,8 @@ struct kvm_reg_list { };

struct kvm_one_reg { - __u64 id; - __u64 addr; + __u64 id; + __kernel_uintptr_t addr; };

#define KVM_MSI_VALID_DEVID (1U << 0) @@ -1422,10 +1422,10 @@ struct kvm_create_device { };

struct kvm_device_attr { - __u32 flags; /* no flags currently defined */ - __u32 group; /* device-defined */ - __u64 attr; /* group-defined */ - __u64 addr; /* userspace address of attr data */ + __u32 flags; /* no flags currently defined */ + __u32 group; /* device-defined */ + __u64 attr; /* group-defined */ + __kernel_uintptr_t addr; /* userspace address of attr data */ };

#define KVM_DEV_VFIO_FILE 1 diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig index 484d0873061c..b6ed8ed3cc4d 100644 --- a/virt/kvm/Kconfig +++ b/virt/kvm/Kconfig @@ -70,7 +70,7 @@ config KVM_GENERIC_DIRTYLOG_READ_PROTECT

config KVM_COMPAT def_bool y - depends on KVM && COMPAT && !(S390 || ARM64 || RISCV) + depends on KVM && COMPAT && !(S390 || RISCV || (ARM64 &&!ARM64_MORELLO))

config HAVE_KVM_IRQ_BYPASS bool diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index d79fd04d8b9e..bbddb168430c 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -120,7 +120,14 @@ static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl, #ifdef CONFIG_KVM_COMPAT static long kvm_vcpu_compat_ioctl(struct file *file, unsigned int ioctl, unsigned long arg); +#ifndef CONFIG_COMPAT64 #define KVM_COMPAT(c) .compat_ioctl = (c) +#define KVM_COMPAT_DIRECT(c) KVM_COMPAT(c) +#else +#define KVM_COMPAT(c) .compat_ioctl = compat_ptr_ioctl, \ + .unlocked_ioctl = (c) +#define KVM_COMPAT_DIRECT(c) .compat_ioctl = (c) +#endif #else /* * For architectures that don't implement a compat infrastructure, @@ -138,6 +145,7 @@ static int kvm_no_compat_open(struct inode *inode, struct file *file) } #define KVM_COMPAT(c) .compat_ioctl = kvm_no_compat_ioctl, \ .open = kvm_no_compat_open +#define KVM_COMPAT_DIRECT(c) KVM_COMPAT(c) #endif static int hardware_enable_all(void); static void hardware_disable_all(void); @@ -3885,7 +3893,7 @@ static struct file_operations kvm_vcpu_fops = { .unlocked_ioctl = kvm_vcpu_ioctl, .mmap = kvm_vcpu_mmap, .llseek = noop_llseek, - KVM_COMPAT(kvm_vcpu_compat_ioctl), + KVM_COMPAT_DIRECT(kvm_vcpu_compat_ioctl), };

/* @@ -4110,7 +4118,7 @@ static long kvm_vcpu_ioctl(struct file *filp, unsigned int ioctl, user_uintptr_t arg) { struct kvm_vcpu *vcpu = filp->private_data; - void __user *argp = (void __user *)arg; + void __user *argp; int r; struct kvm_fpu *fpu = NULL; struct kvm_sregs *kvm_sregs = NULL; @@ -4121,6 +4129,7 @@ static long kvm_vcpu_ioctl(struct file *filp, if (unlikely(_IOC_TYPE(ioctl) != KVMIO)) return -EINVAL;

+ argp = in_compat64_syscall() ? compat_ptr(arg) : (void __user *)arg; /* * Some architectures have vcpu ioctls that are asynchronous to vcpu * execution; mutex_lock() would break them. @@ -4317,6 +4326,45 @@ static long kvm_vcpu_ioctl(struct file *filp, return r; }

+int kvm_get_device_attr_compat(unsigned int ioctl, unsigned long arg, + struct kvm_device_attr *attr) +{ + int ret = -EINVAL; +#ifdef CONFIG_KVM_COMPAT + if (!in_compat64_syscall()) + return ret; + + switch (_IOC_NR(ioctl)) { + case _IOC_NR(KVM_HAS_DEVICE_ATTR): + case _IOC_NR(KVM_GET_DEVICE_ATTR): + case _IOC_NR(KVM_SET_DEVICE_ATTR): { + struct { + __u32 flags; + __u32 group; + __u64 attr; + compat_uptr_t addr; + } __attr; + + if (_IOC_SIZE(ioctl) != sizeof(__attr)) + break; + + ret = -EFAULT; + if (copy_from_user(&__attr, compat_ptr(arg), sizeof(__attr))) + break; + + attr->flags = __attr.flags; + attr->group = __attr.group; + attr->attr = __attr.attr; + attr->addr = (__kernel_uintptr_t)compat_ptr(__attr.addr); + ret = 0; + } + default: + break; + } +#endif + return ret; +} + #ifdef CONFIG_KVM_COMPAT static long kvm_vcpu_compat_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) @@ -4370,37 +4418,38 @@ static int kvm_device_mmap(struct file *filp, struct vm_area_struct *vma) return -ENODEV; }

-static int kvm_device_ioctl_attr(struct kvm_device *dev, +static inline int kvm_device_ioctl_attr(struct kvm_device *dev, int (*accessor)(struct kvm_device *dev, struct kvm_device_attr *attr), - user_uintptr_t arg) + struct kvm_device_attr *attr) { - struct kvm_device_attr attr; - if (!accessor) return -EPERM;

- if (copy_from_user(&attr, (void __user *)arg, sizeof(attr))) - return -EFAULT; - - return accessor(dev, &attr); + return accessor(dev, attr); }

static long kvm_device_ioctl(struct file *filp, unsigned int ioctl, user_uintptr_t arg) { struct kvm_device *dev = filp->private_data; + struct kvm_device_attr attr; + int ret;

if (dev->kvm->mm != current->mm || dev->kvm->vm_dead) return -EIO;

- switch (ioctl) { - case KVM_SET_DEVICE_ATTR: - return kvm_device_ioctl_attr(dev, dev->ops->set_attr, arg); - case KVM_GET_DEVICE_ATTR: - return kvm_device_ioctl_attr(dev, dev->ops->get_attr, arg); - case KVM_HAS_DEVICE_ATTR: - return kvm_device_ioctl_attr(dev, dev->ops->has_attr, arg); + ret = kvm_get_device_attr_compat(ioctl, arg, &attr); + if (ret) + return ret; + + switch (_IOC_NR(ioctl)) { + case _IOC_NR(KVM_SET_DEVICE_ATTR): + return kvm_device_ioctl_attr(dev, dev->ops->set_attr, &attr); + case _IOC_NR(KVM_GET_DEVICE_ATTR): + return kvm_device_ioctl_attr(dev, dev->ops->get_attr, &attr); + case _IOC_NR(KVM_HAS_DEVICE_ATTR): + return kvm_device_ioctl_attr(dev, dev->ops->has_attr, &attr); default: if (dev->ops->ioctl) return dev->ops->ioctl(dev, ioctl, arg); @@ -4787,11 +4836,13 @@ static long kvm_vm_ioctl(struct file *filp, unsigned int ioctl, user_uintptr_t arg) { struct kvm *kvm = filp->private_data; - void __user *argp = (void __user *)arg; + void __user *argp; int r;

if (kvm->mm != current->mm || kvm->vm_dead) return -EIO; + + argp = in_compat64_syscall() ? compat_ptr(arg) : (void __user *)arg; switch (ioctl) { case KVM_CREATE_VCPU: r = kvm_vm_ioctl_create_vcpu(kvm, arg); @@ -5014,13 +5065,13 @@ static long kvm_vm_compat_ioctl(struct file *filp, if (r != -ENOTTY) return r;

- switch (ioctl) { + switch (_IOC_NR(ioctl)) { #ifdef CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT - case KVM_CLEAR_DIRTY_LOG: { + case _IOC_NR(KVM_CLEAR_DIRTY_LOG): { struct compat_kvm_clear_dirty_log compat_log; struct kvm_clear_dirty_log log;

- if (copy_from_user(&compat_log, (void __user *)arg, + if (copy_from_user(&compat_log, compat_ptr(arg), sizeof(compat_log))) return -EFAULT; log.slot = compat_log.slot; @@ -5033,11 +5084,11 @@ static long kvm_vm_compat_ioctl(struct file *filp, break; } #endif - case KVM_GET_DIRTY_LOG: { + case _IOC_NR(KVM_GET_DIRTY_LOG): { struct compat_kvm_dirty_log compat_log; struct kvm_dirty_log log;

- if (copy_from_user(&compat_log, (void __user *)arg, + if (copy_from_user(&compat_log, compat_ptr(arg), sizeof(compat_log))) return -EFAULT; log.slot = compat_log.slot; @@ -5059,7 +5110,7 @@ static struct file_operations kvm_vm_fops = { .release = kvm_vm_release, .unlocked_ioctl = kvm_vm_ioctl, .llseek = noop_llseek, - KVM_COMPAT(kvm_vm_compat_ioctl), + KVM_COMPAT_DIRECT(kvm_vm_compat_ioctl), };

bool file_is_kvm(struct file *file) diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c index ca24ce120906..304abc96272a 100644 --- a/virt/kvm/vfio.c +++ b/virt/kvm/vfio.c @@ -303,7 +303,7 @@ static int kvm_vfio_set_attr(struct kvm_device *dev, switch (attr->group) { case KVM_DEV_VFIO_FILE: return kvm_vfio_set_file(dev, attr->attr, - u64_to_user_ptr(attr->addr)); + as_user_ptr(attr->addr)); }

return -ENXIO;

On 26/02/2024 09:20, Beata Michalska wrote:

With the FEAT_HPDS2 being implemented, PBHA bits of the translation table last-level descriptors can be leveraged for implementation defined hardware use. Fields HWU61,HWU60 & HWU59 are being used by Morello to control capability loads & stores from memory (with correspondign bits

Nit: s/correspondign/corresponding/

enabled in TCR_EL2 for stage 1 and VTCR_EL2 for stage 2 for EL2 translation regime). Add their respective filed descriptors, with HWU62 being provided

s/filed/field/

for completenes only.

s/completenes/completeness/

Kevin

Signed-off-by: Beata Michalska beata.michalska@arm.com

arch/arm64/include/asm/kvm_arm.h | 9 +++++++++ 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h index d1fbf885283f..e80e1b7a91e0 100644 --- a/arch/arm64/include/asm/kvm_arm.h +++ b/arch/arm64/include/asm/kvm_arm.h @@ -109,6 +109,11 @@ /* TCR_EL2 Registers bits */ #define TCR_EL2_RES1 ((1U << 31) | (1 << 23)) +#define TCR_EL2_HWU62 (1 << 28) +#define TCR_EL2_HWU61 (1 << 27) +#define TCR_EL2_HWU60 (1 << 26) +#define TCR_EL2_HWU59 (1 << 25) +#define TCR_EL2_HPD (1 << 24) #define TCR_EL2_TBI (1 << 20) #define TCR_EL2_PS_SHIFT 16 #define TCR_EL2_PS_MASK (7 << TCR_EL2_PS_SHIFT) @@ -123,6 +128,10 @@ /* VTCR_EL2 Registers bits */ #define VTCR_EL2_RES1 (1U << 31) +#define VTCR_EL2_HWU62 TCR_EL2_HWU62 +#define VTCR_EL2_HWU61 TCR_EL2_HWU61 +#define VTCR_EL2_HWU60 TCR_EL2_HWU60 +#define VTCR_EL2_HWU59 TCR_EL2_HWU59 #define VTCR_EL2_HD (1 << 22) #define VTCR_EL2_HA (1 << 21) #define VTCR_EL2_PS_SHIFT TCR_EL2_PS_SHIFT

On 26/02/2024 09:20, Beata Michalska wrote:

Add capability-aware assembly macors generating full-syntax MRS/MSR

s/macors/macros/

instructions for capability system registers.

Signed-off-by: Beata Michalska beata.michalska@arm.com

arch/arm64/include/asm/sysreg.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index 9988e289ce20..e2109e2c0bef 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -1071,28 +1071,56 @@ __emit_inst(0xd5200000|(\sreg)|(.L__gpr_num_\rt)) \ " .endm\n" +#define DEFINE_MRS_S_C \

• __DEFINE_ASM_GPR_NUMS \

+" .macro mrs_s_c, rt, sreg\n" \

• __emit_inst(0xC2900000|(\sreg)|(.L__gpr_num_\rt)) \

Nit: hex is normally lowercase (same below).

+" .endm\n"

#define DEFINE_MSR_S \ __DEFINE_ASM_GPR_NUMS \ " .macro msr_s, sreg, rt\n" \ __emit_inst(0xd5000000|(\sreg)|(.L__gpr_num_\rt)) \ " .endm\n" +#define DEFINE_MSR_S_C \

• __DEFINE_ASM_GPR_NUMS \

+" .macro msr_s_c, sreg, rt\n" \

• __emit_inst(0xC2800000|(\sreg)&~0x100000|(.L__gpr_num_\rt)) \

I think I understand the need for masking bit 20 in sreg (unlike similar instructions, bit 20 is 0 in capability MSR), but a comment wouldn't hurt.

Kevin

+" .endm\n"

#define UNDEFINE_MRS_S \ " .purgem mrs_s\n" +#define UNDEFINE_MRS_S_C \ +" .purgem mrs_s_c\n"

#define UNDEFINE_MSR_S \ " .purgem msr_s\n" +#define UNDEFINE_MSR_S_C \ +" .purgem msr_s_c\n"

#define __mrs_s(v, r) \ DEFINE_MRS_S \ " mrs_s " v ", " __stringify(r) "\n" \ UNDEFINE_MRS_S +#define __mrs_s_c(v, r) \

• DEFINE_MRS_S_C \

+" mrs_s_c " v ", " __stringify(r) "\n" \

• UNDEFINE_MRS_S_C

#define __msr_s(r, v) \ DEFINE_MSR_S \ " msr_s " __stringify(r) ", " v "\n" \ UNDEFINE_MSR_S +#define __msr_s_c(r, v) \

• DEFINE_MSR_S_C \

+" msr_s_c " __stringify(r) ", " v "\n" \

• UNDEFINE_MSR_S_C

/*

• Unlike read_cpuid, calls to read_sysreg are never expected to be
• optimized away or replaced with synthetic values.

On 26/02/2024 10:20, Beata Michalska wrote:

Add bare minimum to enable saving & restoring relevant capability registers. Note that, at this point, only a subset of capability registers is to be handled,with the remaining ones being provided for completeness only.

Signed-off-by: Beata Michalska beata.michalska@arm.com

arch/arm64/include/asm/kvm_asm.h | 6 +++++ arch/arm64/include/asm/kvm_host.h | 38 +++++++++++++++++++++++++++++++ arch/arm64/kernel/asm-offsets.c | 3 +++ 3 files changed, 47 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index 24b5e6b23417..4ca7ece15385 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -334,6 +334,12 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr, u64 elr_virt, #define CPU_XREG_OFFSET(x) (CPU_USER_PT_REGS + 8*x) #define CPU_LR_OFFSET CPU_XREG_OFFSET(30) #define CPU_SP_EL0_OFFSET (CPU_LR_OFFSET + 8) +#ifdef CONFIG_ARM64_MORELLO +#define CPU_CREG_OFFSET(x) (CPU_USER_CREGS + 16*x) +#define CPU_CLR_OFFSET CPU_CREG_OFFSET(30) +#define CPU_CSP_EL0_OFFSET (CPU_CLR_OFFSET + 16) +#define CPU_RCSP_EL0_OFFSET (CPU_CLR_OFFSET + 32) +#endif /*

• We treat x18 as callee-saved as the host may use it as a platform

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 824f29f04916..808645b7108d 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -451,9 +451,40 @@ enum vcpu_sysreg { NR_SYS_REGS /* Nothing after this line! */ }; +#ifdef CONFIG_ARM64_MORELLO +enum vcpu_cap_sysregs {

• __INVALID_CAP_SYSREG__,
• CTPIDR_EL0,
• RTPIDR_EL0,

RCTPIDR_EL0 to be consistent.

Are all the capability registers expected to be here? If so, DDC_EL0 and RDDC_EL0 are missing.

• CTPIDRRO_EL0,
• CTPIDR_EL1,
• CELR_EL1,
• CVBAR_EL1,
• CSP_EL1,
• DDC_EL1,
• CELR_EL2,
• CVBAR_EL2,
• CTPIDR_EL2,
• CSP_EL2,
• DDC_EL2,

I can't see most of these registers being saved or restored in the current series, is that expected?

• NR_CAP_SYS_REGS

+}; +#endif

struct kvm_cpu_context { struct user_pt_regs regs; /* sp = sp_el0 */

• /* This should probably end up in user_pt_regs at one point */

user_pt_regs is a uapi struct so it cannot be changed, at least not in the standard arm64 ABI (and we can't assume the userspace ABI here).

+#ifdef CONFIG_ARM64_MORELLO

• uintcap_t cregs[31];
• uintcap_t csp;
• uintcap_t rcsp;
• uintcap_t pcc;

+#endif

• u64 spsr_abt; u64 spsr_und; u64 spsr_irq;

@@ -463,6 +494,9 @@ struct kvm_cpu_context { u64 sys_regs[NR_SYS_REGS]; +#ifdef CONFIG_ARM64_MORELLO

• uintcap_t cap_sys_regs[NR_CAP_SYS_REGS];

+#endif struct kvm_vcpu *__hyp_running_vcpu; }; @@ -833,6 +867,10 @@ struct kvm_vcpu_arch { #define __vcpu_sys_reg(v,r) (ctxt_sys_reg(&(v)->arch.ctxt, (r))) +#ifdef CONFIG_ARM64_MORELLO +#define __ctxt_cap_sys_reg(c, r) (&(c)->cap_sys_regs[(r)]) +#define ctxt_cap_sys_reg(c, r) (*__ctxt_cap_sys_reg(c,r)) +#endif

Nit: empty line here.

Kevin

u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg); void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg); diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index 90ca25039583..249bf3ca2fbc 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -143,6 +143,9 @@ int main(void) DEFINE(VCPU_FAULT_DISR, offsetof(struct kvm_vcpu, arch.fault.disr_el1)); DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu, arch.hcr_el2)); DEFINE(CPU_USER_PT_REGS, offsetof(struct kvm_cpu_context, regs)); +#ifdef CONFIG_ARM64_MORELLO

• DEFINE(CPU_USER_CREGS, offsetof(struct kvm_cpu_context, cregs));

+#endif DEFINE(CPU_RGSR_EL1, offsetof(struct kvm_cpu_context, sys_regs[RGSR_EL1])); DEFINE(CPU_GCR_EL1, offsetof(struct kvm_cpu_context, sys_regs[GCR_EL1])); DEFINE(CPU_APIAKEYLO_EL1, offsetof(struct kvm_cpu_context, sys_regs[APIAKEYLO_EL1]));

On 26/02/2024 10:20, Beata Michalska wrote:

Disable trapping of Morello instructions on context switch.

Signed-off-by: Beata Michalska beata.michalska@arm.com

arch/arm64/include/asm/kvm_emulate.h | 5 +++++ arch/arm64/kvm/hyp/nvhe/switch.c | 5 +++++ arch/arm64/kvm/hyp/vhe/switch.c | 7 +++++++ 3 files changed, 17 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index 78a550537b67..6ce5d7c96cad 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -587,6 +587,9 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu) CPACR_EL1_ZEN_EL1EN); if (cpus_have_final_cap(ARM64_SME)) val |= CPACR_EL1_SMEN_EL1EN;

• if (IS_ENABLED(CONFIG_ARM64_MORELLO))
  

• 	val |= CPACR_EL1_CEN;
  

• } else if (has_hvhe()) { val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN);

@@ -603,6 +606,8 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu) val |= CPTR_EL2_TZ; if (cpus_have_final_cap(ARM64_SME)) val &= ~CPTR_EL2_TSM;

• if (IS_ENABLED(CONFIG_ARM64_MORELLO))
  

• 	val &= ~CPTR_EL2_TC;
  

  }

return val; diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c index e19243367408..7c4363b0691b 100644 --- a/arch/arm64/kvm/hyp/nvhe/switch.c +++ b/arch/arm64/kvm/hyp/nvhe/switch.c @@ -63,7 +63,12 @@ static void __activate_traps(struct kvm_vcpu *vcpu) __activate_traps_fpsimd32(vcpu); }

• /* Disable trapping of Morello instructions */
• if (IS_ENABLED(CONFIG_ARM64_MORELLO))

• val &= ~CPTR_EL2_TC;
  

• kvm_write_cptr_el2(val);

It doesn't look like this automatically does an ISB. Considering that it is immediately followed by Morello instructions, that could be an issue. Unless CPTR_EL2.TC already happens to be cleared, considering its initialisation in init_kernel_el?

Kevin

• write_sysreg_variant(__this_cpu_read(kvm_hyp_vector), vbar_el2);

if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) { diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c index e4fc314a83a6..6a2540ca30aa 100644 --- a/arch/arm64/kvm/hyp/vhe/switch.c +++ b/arch/arm64/kvm/hyp/vhe/switch.c @@ -75,6 +75,13 @@ static void __activate_traps(struct kvm_vcpu *vcpu) val |= CPTR_EL2_TAM;

• /*

• * Disable trapping Morello instructions
  

• * For VHE (HCR.E2H) CPCR_EL1.CEN bits have the same position as
  

• * CPTR_EL2.CEN
  

• */
  

• val |= CPACR_EL1_CEN;
• if (guest_owns_fp_regs(vcpu)) { if (vcpu_has_sve(vcpu)) val |= CPACR_EL1_ZEN_EL0EN | CPACR_EL1_ZEN_EL1EN;

On 26/02/2024 10:20, Beata Michalska wrote:

Add support for managing kvm virtual machines through plain aarch64 tools. No support for purecap guests just yet.

What this patch really does is adding purecap support to the KVM ioctls, as well as compat64 handling to keep it working. I would prefer doing each step in its own patch (first compat64 handling then purecap support), like we've done for io_uring, BPF, etc.

[...]

@@ -786,7 +786,7 @@ int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) return -EINVAL; switch (reg->id & KVM_REG_ARM_COPROC_MASK) {

• case KVM_REG_ARM_CORE: return set_core_reg(vcpu, reg);

• case KVM_REG_ARM_CORE: return set_core_reg(vcpu, reg);

Let's avoid unnecessary whitespace changes (however logical).

case KVM_REG_ARM_FW: case KVM_REG_ARM_FW_FEAT_BMAP: return kvm_arm_set_fw_reg(vcpu, reg);

[...] diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index b011b4ec2ddf..c2f743462148 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -1215,9 +1215,10 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc, #define __kvm_get_guest(kvm, gfn, offset, v) \ ({ \ unsigned long __addr = gfn_to_hva(kvm, gfn); \

• typeof(v) __user *__uaddr = (typeof(__uaddr))(__addr + offset); \

• typeof(v) __user *__uaddr; \ int __ret = -EFAULT; \ \
• __uaddr = uaddr_to_user_ptr(__addr + offset); \ if (!kvm_is_error_hva(__addr)) \ __ret = get_user(v, __uaddr); \ __ret; \

@@ -1235,9 +1236,10 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc, #define __kvm_put_guest(kvm, gfn, offset, v) \ ({ \ unsigned long __addr = gfn_to_hva(kvm, gfn); \

• typeof(v) __user *__uaddr = (typeof(__uaddr))(__addr + offset); \

• typeof(v) __user *__uaddr; \ int __ret = -EFAULT; \ \
• __uaddr = uaddr_to_user_ptr(__addr + offset); \

It does look like we have to create a capability ourselves here, and we're in control of the address so that's ok. uaddr_to_user_ptr() should not be used in such situations though, it is only meant as a temporary workaround for user-provided addresses. Nowadays we should use make_user_ptr_*() instead (which restricts both bounds and permissions), see [1].

Also these changes look unrelated to the ioctl adaptations, I think they belong to a separate patch.

[1] https://git.morello-project.org/morello/kernel/linux/-/blob/morello/master/D...

if (!kvm_is_error_hva(__addr)) \ __ret = put_user(v, __uaddr); \ if (!__ret) \

[...]

diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig index 484d0873061c..b6ed8ed3cc4d 100644 --- a/virt/kvm/Kconfig +++ b/virt/kvm/Kconfig @@ -70,7 +70,7 @@ config KVM_GENERIC_DIRTYLOG_READ_PROTECT config KVM_COMPAT def_bool y

•   depends on KVM && COMPAT && !(S390 || ARM64 || RISCV)
  

•   depends on KVM && COMPAT && !(S390 || RISCV || (ARM64 &&!ARM64_MORELLO))
  

What matters is not so much that ARM64_MORELLO is selected, but rather that PCuABI is selected (regardless of the architecture). I think the condition could be something like:

KVM && COMPAT && (CHERI_PURECAP_UABI || !(S390 || ARM64 || RISCV))

config HAVE_KVM_IRQ_BYPASS bool diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index d79fd04d8b9e..bbddb168430c 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -120,7 +120,14 @@ static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl, #ifdef CONFIG_KVM_COMPAT static long kvm_vcpu_compat_ioctl(struct file *file, unsigned int ioctl, unsigned long arg); +#ifndef CONFIG_COMPAT64 #define KVM_COMPAT(c) .compat_ioctl = (c) +#define KVM_COMPAT_DIRECT(c) KVM_COMPAT(c) +#else +#define KVM_COMPAT(c) .compat_ioctl = compat_ptr_ioctl, \

I don't think there is any issue with using compat_ptr_ioctl regardless of the kind of compat. The compat_ptr() conversion is not actually specific to compat64, it just happens that very few compat ioctl handlers already do the right thing.

Specifically, what I'm suggesting is to leave all of these macros unchanged, and for kvm_device_ioctl and kvm_dev_ioctl, use KVM_COMPAT(compat_ptr_ioctl).

• 	.unlocked_ioctl = (c)
  

It seems to me that this additional .unlocked_ioctl assignment does nothing (it is already present in the struct definitions where KVM_COMPAT() is used). Also worth noting that overriding an initialiser can trigger a warning (though not in the default build configuration).

+#define KVM_COMPAT_DIRECT(c) .compat_ioctl = (c) +#endif #else /*

• For architectures that don't implement a compat infrastructure,

@@ -138,6 +145,7 @@ static int kvm_no_compat_open(struct inode *inode, struct file *file) } #define KVM_COMPAT(c) .compat_ioctl = kvm_no_compat_ioctl, \ .open = kvm_no_compat_open +#define KVM_COMPAT_DIRECT(c) KVM_COMPAT(c) #endif static int hardware_enable_all(void); static void hardware_disable_all(void); @@ -3885,7 +3893,7 @@ static struct file_operations kvm_vcpu_fops = { .unlocked_ioctl = kvm_vcpu_ioctl, .mmap = kvm_vcpu_mmap, .llseek = noop_llseek,

• KVM_COMPAT(kvm_vcpu_compat_ioctl),

• KVM_COMPAT_DIRECT(kvm_vcpu_compat_ioctl),

}; /* @@ -4110,7 +4118,7 @@ static long kvm_vcpu_ioctl(struct file *filp, unsigned int ioctl, user_uintptr_t arg) { struct kvm_vcpu *vcpu = filp->private_data;

• void __user *argp = (void __user *)arg;

• void __user *argp; int r; struct kvm_fpu *fpu = NULL; struct kvm_sregs *kvm_sregs = NULL;

@@ -4121,6 +4129,7 @@ static long kvm_vcpu_ioctl(struct file *filp, if (unlikely(_IOC_TYPE(ioctl) != KVMIO)) return -EINVAL;

• argp = in_compat64_syscall() ? compat_ptr(arg) : (void __user *)arg;

Instead of this, I would suggest doing the easy thing: have kvm_vcpu_compat_ioctl() call kvm_vcpu_ioctl(filp, ioctl, (user_uintptr_t)argp), so that the native handler can assume that arg is a valid pointer. This patch effectively does this already for kvm_device_ioctl() and kvm_dev_ioctl() by using compat_ptr_ioctl, and we might as well do it for all the handlers. It is not 100% correct in the sense that some commands do not expect a pointer, but at this point I am really not convinced that the additional complexity created by using compat_ptr() only when the argument is actually a pointer is justified. The overhead of calling compat_ptr() should be fairly minimal, and it is very difficult to imagine a case where it would be a security issue.

/* * Some architectures have vcpu ioctls that are asynchronous to vcpu * execution; mutex_lock() would break them. @@ -4317,6 +4326,45 @@ static long kvm_vcpu_ioctl(struct file *filp, return r; } +int kvm_get_device_attr_compat(unsigned int ioctl, unsigned long arg,

• 	       struct kvm_device_attr *attr)
  

+{

• int ret = -EINVAL;

+#ifdef CONFIG_KVM_COMPAT

• if (!in_compat64_syscall())

• return ret;
  

• switch (_IOC_NR(ioctl)) {
• case _IOC_NR(KVM_HAS_DEVICE_ATTR):
• case _IOC_NR(KVM_GET_DEVICE_ATTR):
• case _IOC_NR(KVM_SET_DEVICE_ATTR): {

• struct {
  

• 	__u32		flags;
  

• 	__u32		group;
  

• 	__u64		attr;
  

• 	compat_uptr_t	addr;
  

• } __attr;
  

• if (_IOC_SIZE(ioctl) != sizeof(__attr))
  

We do need this check indeed. At the moment I don't think we are checking this systematically though, it should be done whenever we transform a switch to use _IOC_NR(), both in native and in compat. That does beg the question of whether this is actually simpler / shorter than having a separate switch for compat64... This works well in the DRM case because the driver knows what the expected size is just based on _IOC_NR(cmd), but we don't have such infrastructure here. Maybe we can figure out some clever wrapper for copy_from_user that also does some command size checking, though.

A further issue is that we should check the remaining fields of the command value as well, i.e. _IOC_TYPE() and _IOC_DIR(). The former can be checked unconditionally at the beginning, but not the latter. Here as well it gets rather cumbersome without DRM's infrastructure.

If we go for a separate switch for compat64, we can make our lives easier by introducing a macro that replaces the size field in a command value (like KVM_HAS_DEVICE_ATTR) with a new value, i.e. the size of the compat struct.

• 	break;
  

• ret = -EFAULT;
  

• if (copy_from_user(&__attr, compat_ptr(arg), sizeof(__attr)))
  

• 	break;
  

• attr->flags = __attr.flags;
  

• attr->group = __attr.group;
  

• attr->attr  = __attr.attr;
  

• attr->addr  = (__kernel_uintptr_t)compat_ptr(__attr.addr);
  

• ret = 0;
  

• }
• default:

• break;
  

• }

+#endif

• return ret;

+}

#ifdef CONFIG_KVM_COMPAT static long kvm_vcpu_compat_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) @@ -4370,37 +4418,38 @@ static int kvm_device_mmap(struct file *filp, struct vm_area_struct *vma) return -ENODEV; } -static int kvm_device_ioctl_attr(struct kvm_device *dev, +static inline int kvm_device_ioctl_attr(struct kvm_device *dev, int (*accessor)(struct kvm_device *dev, struct kvm_device_attr *attr),

• 		 user_uintptr_t arg)
  

• 		 struct kvm_device_attr *attr)
  

{

• struct kvm_device_attr attr;
• if (!accessor) return -EPERM;

• if (copy_from_user(&attr, (void __user *)arg, sizeof(attr)))

• return -EFAULT;
  

• return accessor(dev, &attr);

• return accessor(dev, attr);

} static long kvm_device_ioctl(struct file *filp, unsigned int ioctl, user_uintptr_t arg) { struct kvm_device *dev = filp->private_data;

• struct kvm_device_attr attr;
• int ret;

if (dev->kvm->mm != current->mm || dev->kvm->vm_dead) return -EIO;

• switch (ioctl) {
• case KVM_SET_DEVICE_ATTR:

• return kvm_device_ioctl_attr(dev, dev->ops->set_attr, arg);
  

• case KVM_GET_DEVICE_ATTR:

• return kvm_device_ioctl_attr(dev, dev->ops->get_attr, arg);
  

• case KVM_HAS_DEVICE_ATTR:

• return kvm_device_ioctl_attr(dev, dev->ops->has_attr, arg);
  

• ret = kvm_get_device_attr_compat(ioctl, arg, &attr);
• if (ret)

• return ret;
  

• switch (_IOC_NR(ioctl)) {
• case _IOC_NR(KVM_SET_DEVICE_ATTR):

• return kvm_device_ioctl_attr(dev, dev->ops->set_attr, &attr);
  

• case _IOC_NR(KVM_GET_DEVICE_ATTR):

• return kvm_device_ioctl_attr(dev, dev->ops->get_attr, &attr);
  

• case _IOC_NR(KVM_HAS_DEVICE_ATTR):

• return kvm_device_ioctl_attr(dev, dev->ops->has_attr, &attr);
  

  default: if (dev->ops->ioctl) return dev->ops->ioctl(dev, ioctl, arg);

@@ -4787,11 +4836,13 @@ static long kvm_vm_ioctl(struct file *filp, unsigned int ioctl, user_uintptr_t arg) { struct kvm *kvm = filp->private_data;

• void __user *argp = (void __user *)arg;

• void __user *argp; int r;

if (kvm->mm != current->mm || kvm->vm_dead) return -EIO;

• argp = in_compat64_syscall() ? compat_ptr(arg) : (void __user *)arg; switch (ioctl) { case KVM_CREATE_VCPU: r = kvm_vm_ioctl_create_vcpu(kvm, arg);

@@ -5014,13 +5065,13 @@ static long kvm_vm_compat_ioctl(struct file *filp, if (r != -ENOTTY) return r;

• switch (ioctl) {

• switch (_IOC_NR(ioctl)) {

#ifdef CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT

• case KVM_CLEAR_DIRTY_LOG: {

• case _IOC_NR(KVM_CLEAR_DIRTY_LOG): { struct compat_kvm_clear_dirty_log compat_log; struct kvm_clear_dirty_log log;

• if (copy_from_user(&compat_log, (void __user *)arg,
  

• if (copy_from_user(&compat_log, compat_ptr(arg),
  	   sizeof(compat_log)))
  return -EFAULT;
  

  log.slot = compat_log.slot;

@@ -5033,11 +5084,11 @@ static long kvm_vm_compat_ioctl(struct file *filp, break; } #endif

• case KVM_GET_DIRTY_LOG: {

• case _IOC_NR(KVM_GET_DIRTY_LOG): { struct compat_kvm_dirty_log compat_log; struct kvm_dirty_log log;

• if (copy_from_user(&compat_log, (void __user *)arg,
  

• if (copy_from_user(&compat_log, compat_ptr(arg),
  	   sizeof(compat_log)))
  return -EFAULT;
  

  log.slot = compat_log.slot;

@@ -5059,7 +5110,7 @@ static struct file_operations kvm_vm_fops = { .release = kvm_vm_release, .unlocked_ioctl = kvm_vm_ioctl, .llseek = noop_llseek,

• KVM_COMPAT(kvm_vm_compat_ioctl),

• KVM_COMPAT_DIRECT(kvm_vm_compat_ioctl),

}; bool file_is_kvm(struct file *file) diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c index ca24ce120906..304abc96272a 100644 --- a/virt/kvm/vfio.c +++ b/virt/kvm/vfio.c @@ -303,7 +303,7 @@ static int kvm_vfio_set_attr(struct kvm_device *dev, switch (attr->group) { case KVM_DEV_VFIO_FILE: return kvm_vfio_set_file(dev, attr->attr,

• 			 u64_to_user_ptr(attr->addr));
  

• 			 as_user_ptr(attr->addr));
  

Not sure I understand this. I would expect a simple cast to void __user *.

Kevin

} return -ENXIO;

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.

...

+#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.

...

+#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, diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 53ad8de28f48..bc945eac681d 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -1859,6 +1859,8 @@ static void __init cpu_prepare_hyp_mode(int cpu, u32 hyp_va_bits) } tcr &= ~TCR_T0SZ_MASK; tcr |= TCR_T0SZ(hyp_va_bits);

• if (IS_ENABLED(CONFIG_ARM64_MORELLO))

• tcr |= TCR_EL2_HWU61 | TCR_EL2_HWU60 | TCR_EL2_HPD;
  

  params->tcr_el2 = tcr;

params->pgd_pa = kvm_mmu_get_httbr(); diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c index 1966fdee740e..88d002d7152f 100644 --- a/arch/arm64/kvm/hyp/pgtable.c +++ b/arch/arm64/kvm/hyp/pgtable.c @@ -38,6 +38,7 @@ #define KVM_PTE_LEAF_ATTR_HI GENMASK(63, 50) +#define KVM_PTE_LEAF_ATTR_HI_HW GENMASK(62, 59) #define KVM_PTE_LEAF_ATTR_HI_SW GENMASK(58, 55) #define KVM_PTE_LEAF_ATTR_HI_S1_XN BIT(54) @@ -411,6 +412,7 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep) attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_SH, sh); attr |= KVM_PTE_LEAF_ATTR_LO_S1_AF; attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;

• attr |= prot & KVM_PTE_LEAF_ATTR_HI_HW; *ptep = attr;

return 0; @@ -418,7 +420,7 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep) enum kvm_pgtable_prot kvm_pgtable_hyp_pte_prot(kvm_pte_t pte) {

• enum kvm_pgtable_prot prot = pte & KVM_PTE_LEAF_ATTR_HI_SW;

• enum kvm_pgtable_prot prot = pte & (KVM_PTE_LEAF_ATTR_HI_SW | KVM_PTE_LEAF_ATTR_HI_HW); u32 ap;

if (!kvm_pte_valid(pte)) @@ -654,6 +656,9 @@ u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift) vtcr |= VTCR_EL2_HA; #endif /* CONFIG_ARM64_HW_AFDBM */

• if (IS_ENABLED(CONFIG_ARM64_MORELLO))

• vtcr |= VTCR_EL2_PBHA;
  

I'm not sure I understand the rationale behind the choice of HWU bits to set. Right now it looks like we're doing this:

• Stage 1: 60, 61 (SC, LC[0])
• Stage 2: 59, 60, 61, 62 (CDBM, SC, LC, <unused>)

IIUC, the stage 1 configuration only matters for host / KVM mappings. We don't use LC[1] or CDBM so it's ok to set only HWU{60,61}, though it wouldn't hurt to set them all (like we do in the kernel). OTOH we're setting all bits at stage 2, even though HWU62 has no meaning for Morello (and we don't use CDBM either). It is a detail, as only HWU{60,61} matter in both cases for our usage, but it would be nice to be more consistent.

Right, that's a fair point. Will update stage 1 setup accordingly (did end up mixing the approach taken for EL1 vs EL2)

...

• /* Set the vmid bits */ vtcr |= (get_vmid_bits(mmfr1) == 16) ? VTCR_EL2_VS_16BIT :

@@ -714,6 +719,7 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh); attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF; attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;

• attr |= prot & KVM_PTE_LEAF_ATTR_HI_HW; *ptep = attr;

return 0; @@ -1299,7 +1305,7 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr, u32 level; kvm_pte_t set = 0, clr = 0;

• if (prot & KVM_PTE_LEAF_ATTR_HI_SW)

• if (prot & KVM_PTE_LEAF_ATTR_HI_SW || prot & KVM_PTE_LEAF_ATTR_HI_HW)

Nit: could be prot & (SW | HW), like above.

Done.

Thanks.

--- BR Beata

Kevin

...

return -EINVAL;

if (prot & KVM_PGTABLE_PROT_R)

On Fri, Mar 01, 2024 at 11:22:51AM +0100, Kevin Brodsky wrote:

On 26/02/2024 09:20, Beata Michalska wrote:

...

Provide capability-aware set of helper macros for system register read/writes. For the time being, if the transfer register is not a capability one, a prep step is being carried out to provide it through the use of CVTP instruction. In a long run this should be replace by proper setup where a source capability is derived from an appropriate authorizing capability.

Signed-off-by: Beata Michalska beata.michalska@arm.com

arch/arm64/include/asm/sysreg.h | 55 +++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index e2109e2c0bef..c55c4c4fa945 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -1141,6 +1141,42 @@ : : "rZ" (__val)); \ } while (0) +#ifdef CONFIG_ARM64_MORELLO +#define __convert_to_cap(v) \ +({ \

• uintcap_t __cval;						\
  

• asm volatile("cvtp %0, %1" : "=r" (__cval) : "r" (v));		\
  

• __cval;							\
  

+})

+#define write_cap_sysreg(v, r) \

• do { \

• uintcap_t __cval;						\
  

• __cval = __builtin_choose_expr(					\
  

• 		__builtin_types_compatible_p(typeof(v),		\
  

• 					     uintcap_t),	\
  

• 		(uintcap_t)(v),					\
  

• 		__convert_to_cap(v)				\
  

• 		);						\
  

•  asm volatile("msr " __stringify(r) ", %x0" : : "rZ" (__cval));	\
  

• } while (0)

+/*

• • Select appropriate variant of write_sysreg based on whether
• • CONFIG_ARM64_MORELLO is enabled or not.
• • For the capability system registers, unless capability is being provided,
• • one is created through CVTP instruction. This immplies that this macro
• • is not suitable for all use-cases. Additionally, this macro is based on
• • the assumption that the capability system register name can be derived
• • from corresponding aarch64 one.
• */

+#define write_sysreg_variant(v, r) \

• IS_BUILTIN(CONFIG_ARM64_MORELLO) \

• ?  ({ write_cap_sysreg(v, c##r); })	\
  

• :  ({ write_sysreg(v, r); })
  

At first I wasn't convinced about having such a generic macro, but I do see the point in subsequent patches - it avoids quite a bit of #ifdef'ing. That said, I would prefer things to be more explicit. I don't see a need for write_cap_sysreg() accepting both 64-bit and capability values. I think we have two situations:

1. We've got a 64-bit code address and we want to create a valid

capability to write into a capability register on Morello (and just the 64-bit address otherwise). That seems to be relevant to VBAR/ELR. 2. We're explicitly manipulating a capability and writing it into a capability register.

For the first situation, something like this macro is useful, but I would always have it use CVTP to create a valid code capability (on Morello). Additionally, I would name it accordingly:

Did I get it right that you would prefer to drop checking the type of the argument and make it always u64 being converted to cap even if a valid one has been provided?

write_sysreg_code_addr() or similar. This is because using CVTP is not

Fair point.

appropriate for creating data capability, and we shouldn't suggest that this macro is more generic than it really is. Overall my thinking is: if a capability is going to be created, that needs to be reflected in the helper's name, and we don't want to have that happen by accident (e.g. by passing the wrong type to the helper).

If in the future we move to saving/restoring the full capabilities for VBAR/ELR, then we can introduce a new macro, maybe write_sysreg_ptr(), that would always take a capability on Morello and a 64-bit address otherwise.

...

+#else +#define write_sysreg_variant write_sysreg +#endif /*

• For registers without architectural names, or simply unsupported by
• GAS.

@@ -1162,6 +1198,25 @@ asm volatile(__msr_s(r, "%x0") : : "rZ" (__val)); \ } while (0) +#define read_cap_sysreg_s(r) ({ \

• uintcap_t __val; \
• u32 __maybe_unused __check_r = (u32)(r); \
• asm volatile(__mrs_s_c("%0", r) : "=r" (__val)); \
• __val; \

+})

+#define write_cap_sysreg_s(v, r) do { \

• uintcap_t __cval; \
• u32 __maybe_unused __check_r = (u32)(r); \
• __cval = __builtin_choose_expr( \

• 	__builtin_types_compatible_p(typeof(v),		\
  

• 				     uintcap_t),	\
  

• 	(uintcap_t)(v),					\
  

• 	__convert_to_cap(v)				\
  

• 	);						\
  

• asm volatile(__msr_s_c(r, "%x0") : : "rZ" (__cval)); \

+} while (0)

Same idea here: write_cap_sysreg_s() should do exactly what it says (write a capability). We can have a separate helper that (always) creates a capability with CVTP.

Noted.

--- BR Beata

Kevin

...

/*

• Modify bits in a sysreg. Bits in the clear mask are zeroed, then bits in the
• set mask are set. Other bits are left as-is.

On Fri, Mar 01, 2024 at 11:27:26AM +0100, Kevin Brodsky wrote:

On 26/02/2024 10:20, Beata Michalska wrote:

...

Disable trapping of Morello instructions on context switch.

Signed-off-by: Beata Michalska beata.michalska@arm.com

arch/arm64/include/asm/kvm_emulate.h | 5 +++++ arch/arm64/kvm/hyp/nvhe/switch.c | 5 +++++ arch/arm64/kvm/hyp/vhe/switch.c | 7 +++++++ 3 files changed, 17 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index 78a550537b67..6ce5d7c96cad 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -587,6 +587,9 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu) CPACR_EL1_ZEN_EL1EN); if (cpus_have_final_cap(ARM64_SME)) val |= CPACR_EL1_SMEN_EL1EN;

• if (IS_ENABLED(CONFIG_ARM64_MORELLO))
  

• 	val |= CPACR_EL1_CEN;
  

• } else if (has_hvhe()) { val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN);

@@ -603,6 +606,8 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu) val |= CPTR_EL2_TZ; if (cpus_have_final_cap(ARM64_SME)) val &= ~CPTR_EL2_TSM;

• if (IS_ENABLED(CONFIG_ARM64_MORELLO))
  

• 	val &= ~CPTR_EL2_TC;
  

  }

return val; diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c index e19243367408..7c4363b0691b 100644 --- a/arch/arm64/kvm/hyp/nvhe/switch.c +++ b/arch/arm64/kvm/hyp/nvhe/switch.c @@ -63,7 +63,12 @@ static void __activate_traps(struct kvm_vcpu *vcpu) __activate_traps_fpsimd32(vcpu); }

• /* Disable trapping of Morello instructions */
• if (IS_ENABLED(CONFIG_ARM64_MORELLO))

• val &= ~CPTR_EL2_TC;
  

• kvm_write_cptr_el2(val);

It doesn't look like this automatically does an ISB. Considering that it is immediately followed by Morello instructions, that could be an issue. Unless CPTR_EL2.TC already happens to be cleared, considering its initialisation in init_kernel_el?

It is indeed already cleared so what it does it makes sure that between the switches we will not lose that setting. I should probably add a comment why barrier is not needed in this case.

--- BR Beata

Kevin

...

• write_sysreg_variant(__this_cpu_read(kvm_hyp_vector), vbar_el2);

if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) { diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c index e4fc314a83a6..6a2540ca30aa 100644 --- a/arch/arm64/kvm/hyp/vhe/switch.c +++ b/arch/arm64/kvm/hyp/vhe/switch.c @@ -75,6 +75,13 @@ static void __activate_traps(struct kvm_vcpu *vcpu) val |= CPTR_EL2_TAM;

• /*

• * Disable trapping Morello instructions
  

• * For VHE (HCR.E2H) CPCR_EL1.CEN bits have the same position as
  

• * CPTR_EL2.CEN
  

• */
  

• val |= CPACR_EL1_CEN;
• if (guest_owns_fp_regs(vcpu)) { if (vcpu_has_sve(vcpu)) val |= CPACR_EL1_ZEN_EL0EN | CPACR_EL1_ZEN_EL1EN;

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,