Re: [PATCH 20/59] KVM: arm64: nv: Trap CPACR_EL1 access in virtual EL2
On 6/21/19 10:38 AM, Marc Zyngier wrote:
> From: Jintack Lim
>
> For the same reason we trap virtual memory register accesses in virtual
> EL2, we trap CPACR_EL1 access too; We allow the virtual EL2 mode to
> access EL1 system register state instead of the virtual EL2 one.
>
> Signed-off-by: Jintack Lim
> Signed-off-by: Marc Zyngier
> ---
> arch/arm64/include/asm/kvm_arm.h | 3 ++-
> arch/arm64/kvm/hyp/switch.c | 2 ++
> arch/arm64/kvm/sys_regs.c| 2 +-
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_arm.h
> b/arch/arm64/include/asm/kvm_arm.h
> index b2e363ac624d..48e15af2bece 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -278,12 +278,13 @@
> #define CPTR_EL2_TFP_SHIFT 10
>
> /* Hyp Coprocessor Trap Register */
> -#define CPTR_EL2_TCPAC (1 << 31)
> +#define CPTR_EL2_TCPAC (1U << 31)
> #define CPTR_EL2_TTA (1 << 20)
> #define CPTR_EL2_TFP (1 << CPTR_EL2_TFP_SHIFT)
> #define CPTR_EL2_TZ (1 << 8)
> #define CPTR_EL2_RES10x32ff /* known RES1 bits in CPTR_EL2 */
> #define CPTR_EL2_DEFAULT CPTR_EL2_RES1
> +#define CPTR_EL2_E2H_TCPAC (1U << 31)
I'm not sure why CPTR_EL2_TCPAC is being renamed to CPTR_EL2_E2H_TCPAC.
CPTR_EL2.TCPAC is always bit 31, regardless of the value of HCR_EL2.E2H. I also
did a grep and it's only used in the one place added by this patch.
>
> /* Hyp Debug Configuration Register bits */
> #define MDCR_EL2_TPMS(1 << 14)
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 791b26570347..62359c7c3d6b 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -108,6 +108,8 @@ static void activate_traps_vhe(struct kvm_vcpu *vcpu)
> val &= ~CPACR_EL1_FPEN;
> __activate_traps_fpsimd32(vcpu);
> }
> + if (vcpu_mode_el2(vcpu) && !vcpu_el2_e2h_is_set(vcpu))
> + val |= CPTR_EL2_E2H_TCPAC;
>
> write_sysreg(val, cpacr_el1);
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 7fc87657382d..1d1312425cf2 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1773,7 +1773,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> ID_UNALLOCATED(7,7),
>
> { SYS_DESC(SYS_SCTLR_EL1), access_vm_reg, reset_val, SCTLR_EL1,
> 0x00C50078 },
> - { SYS_DESC(SYS_CPACR_EL1), NULL, reset_val, CPACR_EL1, 0 },
> + { SYS_DESC(SYS_CPACR_EL1), access_rw, reset_val, CPACR_EL1, 0 },
> { SYS_DESC(SYS_ZCR_EL1), NULL, reset_val, ZCR_EL1, 0, .visibility =
> sve_visibility },
> { SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 },
> { SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 },
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 07/59] KVM: arm64: nv: Add EL2 system registers to vcpu context
Hi Marc,
On 21/06/2019 10:37, Marc Zyngier wrote:
From: Jintack Lim
ARM v8.3 introduces a new bit in the HCR_EL2, which is the NV bit. When
this bit is set, accessing EL2 registers in EL1 traps to EL2. In
addition, executing the following instructions in EL1 will trap to EL2:
tlbi, at, eret, and msr/mrs instructions to access SP_EL1. Most of the
instructions that trap to EL2 with the NV bit were undef at EL1 prior to
ARM v8.3. The only instruction that was not undef is eret.
This patch sets up a handler for EL2 registers and SP_EL1 register
accesses at EL1. The host hypervisor keeps those register values in
memory, and will emulate their behavior.
This patch doesn't set the NV bit yet. It will be set in a later patch
once nested virtualization support is completed.
Signed-off-by: Jintack Lim
Signed-off-by: Marc Zyngier
---
arch/arm64/include/asm/kvm_host.h | 37 +++-
arch/arm64/include/asm/sysreg.h | 50 -
arch/arm64/kvm/sys_regs.c | 74 ---
3 files changed, 154 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
index 4bcd9c1291d5..2d4290d2513a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -173,12 +173,47 @@ enum vcpu_sysreg {
APGAKEYLO_EL1,
APGAKEYHI_EL1,
- /* 32bit specific registers. Keep them at the end of the range */
+ /* 32bit specific registers. */
DACR32_EL2, /* Domain Access Control Register */
IFSR32_EL2, /* Instruction Fault Status Register */
FPEXC32_EL2,/* Floating-Point Exception Control Register */
DBGVCR32_EL2, /* Debug Vector Catch Register */
+ /* EL2 registers sorted ascending by Op0, Op1, CRn, CRm, Op2 */
+ FIRST_EL2_SYSREG,
+ VPIDR_EL2 = FIRST_EL2_SYSREG,
+ /* Virtualization Processor ID Register */
+ VMPIDR_EL2, /* Virtualization Multiprocessor ID Register */
+ SCTLR_EL2, /* System Control Register (EL2) */
+ ACTLR_EL2, /* Auxiliary Control Register (EL2) */
+ HCR_EL2,/* Hypervisor Configuration Register */
+ MDCR_EL2, /* Monitor Debug Configuration Register (EL2) */
+ CPTR_EL2, /* Architectural Feature Trap Register (EL2) */
+ HSTR_EL2, /* Hypervisor System Trap Register */
+ HACR_EL2, /* Hypervisor Auxiliary Control Register */
+ TTBR0_EL2, /* Translation Table Base Register 0 (EL2) */
+ TTBR1_EL2, /* Translation Table Base Register 1 (EL2) */
+ TCR_EL2,/* Translation Control Register (EL2) */
+ VTTBR_EL2, /* Virtualization Translation Table Base Register */
+ VTCR_EL2, /* Virtualization Translation Control Register */
+ SPSR_EL2, /* EL2 saved program status register */
+ ELR_EL2,/* EL2 exception link register */
+ AFSR0_EL2, /* Auxiliary Fault Status Register 0 (EL2) */
+ AFSR1_EL2, /* Auxiliary Fault Status Register 1 (EL2) */
+ ESR_EL2,/* Exception Syndrome Register (EL2) */
+ FAR_EL2,/* Hypervisor IPA Fault Address Register */
+ HPFAR_EL2, /* Hypervisor IPA Fault Address Register */
+ MAIR_EL2, /* Memory Attribute Indirection Register (EL2) */
+ AMAIR_EL2, /* Auxiliary Memory Attribute Indirection Register
(EL2) */
+ VBAR_EL2, /* Vector Base Address Register (EL2) */
+ RVBAR_EL2, /* Reset Vector Base Address Register */
+ RMR_EL2,/* Reset Management Register */
+ CONTEXTIDR_EL2, /* Context ID Register (EL2) */
+ TPIDR_EL2, /* EL2 Software Thread ID Register */
+ CNTVOFF_EL2,/* Counter-timer Virtual Offset register */
+ CNTHCTL_EL2,/* Counter-timer Hypervisor Control register */
+ SP_EL2, /* EL2 Stack Pointer */
Does it need to include the following registers Counter-timer Hyp Physical timer
registers ? i.e, CNTHP_{CTL,CVAL,TVAL}_EL2. Or do we have some other magic with
NV for the virtual EL2 ?
Cheers
Suzuki
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 18/59] KVM: arm64: nv: Trap EL1 VM register accesses in virtual EL2
On 6/21/19 10:38 AM, Marc Zyngier wrote:
> From: Christoffer Dall
>
> When running in virtual EL2 mode, we actually run the hardware in EL1
> and therefore have to use the EL1 registers to ensure correct operation.
>
> By setting the HCR.TVM and HCR.TVRM we ensure that the virtual EL2 mode
> doesn't shoot itself in the foot when setting up what it believes to be
> a different mode's system register state (for example when preparing to
> switch to a VM).
A guest hypervisor with vhe enabled uses the _EL12 register names when preparing
to run a guest, and accesses to those registers are already trapped when setting
HCR_EL2.NV. This patch affects only non-vhe guest hypervisors, would you mind
updating the message to reflect that?
>
> We can leverage the existing sysregs infrastructure to support trapped
> accesses to these registers.
>
> Signed-off-by: Christoffer Dall
> Signed-off-by: Marc Zyngier
> ---
> arch/arm64/kvm/hyp/switch.c | 4
> arch/arm64/kvm/sys_regs.c | 7 ++-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 7b55c11b30fb..791b26570347 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -135,6 +135,10 @@ static void __hyp_text __activate_traps(struct kvm_vcpu
> *vcpu)
> {
> u64 hcr = vcpu->arch.hcr_el2;
>
> + /* Trap VM sysreg accesses if an EL2 guest is not using VHE. */
> + if (vcpu_mode_el2(vcpu) && !vcpu_el2_e2h_is_set(vcpu))
> + hcr |= HCR_TVM | HCR_TRVM;
> +
> write_sysreg(hcr, hcr_el2);
>
> if (cpus_have_const_cap(ARM64_HAS_RAS_EXTN) && (hcr & HCR_VSE))
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index e181359adadf..0464d8e29cba 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -440,7 +440,12 @@ static bool access_vm_reg(struct kvm_vcpu *vcpu,
> u64 val;
> int reg = r->reg;
>
> - BUG_ON(!p->is_write);
> + BUG_ON(!vcpu_mode_el2(vcpu) && !p->is_write);
> +
> + if (!p->is_write) {
> + p->regval = vcpu_read_sys_reg(vcpu, reg);
> + return true;
> + }
>
> /* See the 32bit mapping in kvm_host.h */
> if (p->is_aarch32)
For more context:
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index e181359adadf..0464d8e29cba 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -428,31 +428,36 @@ static bool access_dcsw(struct kvm_vcpu *vcpu,
}
/*
* Generic accessor for VM registers. Only called as long as HCR_TVM
* is set. If the guest enables the MMU, we stop trapping the VM
* sys_regs and leave it in complete control of the caches.
*/
static bool access_vm_reg(struct kvm_vcpu *vcpu,
struct sys_reg_params *p,
const struct sys_reg_desc *r)
{
bool was_enabled = vcpu_has_cache_enabled(vcpu);
u64 val;
int reg = r->reg;
- BUG_ON(!p->is_write);
+ BUG_ON(!vcpu_mode_el2(vcpu) && !p->is_write);
+
+ if (!p->is_write) {
+ p->regval = vcpu_read_sys_reg(vcpu, reg);
+ return true;
+ }
/* See the 32bit mapping in kvm_host.h */
if (p->is_aarch32)
reg = r->reg / 2;
if (!p->is_aarch32 || !p->is_32bit) {
val = p->regval;
} else {
val = vcpu_read_sys_reg(vcpu, reg);
if (r->reg % 2)
val = (p->regval << 32) | (u64)lower_32_bits(val);
else
val = ((u64)upper_32_bits(val) << 32) |
lower_32_bits(p->regval);
}
Perhaps the function comment should be updated to reflect that the function is
also used for VM register traps for a non-vhe guest hypervisor?
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 43/59] KVM: arm64: nv: Trap and emulate AT instructions from virtual EL2
On 21/06/2019 10:38, Marc Zyngier wrote: > From: Jintack Lim > > When supporting nested virtualization a guest hypervisor executing AT > instructions must be trapped and emulated by the host hypervisor, > because untrapped AT instructions operating on S1E1 will use the wrong > translation regieme (the one used to emulate virtual EL2 in EL1 instead > of virtual EL1) and AT instructions operating on S12 will not work from > EL1. > > This patch does several things. > > 1. List and define all AT system instructions to emulate and document > the emulation design. > > 2. Implement AT instruction handling logic in EL2. This will be used to > emulate AT instructions executed in the virtual EL2. > > AT instruction emulation works by loading the proper processor > context, which depends on the trapped instruction and the virtual > HCR_EL2, to the EL1 virtual memory control registers and executing AT > instructions. Note that ctxt->hw_sys_regs is expected to have the > proper processor context before calling the handling > function(__kvm_at_insn) implemented in this patch. > > 4. Emulate AT S1E[01] instructions by issuing the same instructions in > EL2. We set the physical EL1 registers, NV and NV1 bits as described in > the AT instruction emulation overview. > > 5. Emulate AT A12E[01] instructions in two steps: First, do the stage-1 > translation by reusing the existing AT emulation functions. Second, do > the stage-2 translation by walking the guest hypervisor's stage-2 page > table in software. Record the translation result to PAR_EL1. > > 6. Emulate AT S1E2 instructions by issuing the corresponding S1E1 > instructions in EL2. We set the physical EL1 registers and the HCR_EL2 > register as described in the AT instruction emulation overview. > > 7. Forward system instruction traps to the virtual EL2 if the corresponding > virtual AT bit is set in the virtual HCR_EL2. > > [ Much logic above has been reworked by Marc Zyngier ] > > Signed-off-by: Jintack Lim > Signed-off-by: Marc Zyngier > Signed-off-by: Christoffer Dall > --- > arch/arm64/include/asm/kvm_arm.h | 2 + > arch/arm64/include/asm/kvm_asm.h | 2 + > arch/arm64/include/asm/sysreg.h | 17 +++ > arch/arm64/kvm/hyp/Makefile | 1 + > arch/arm64/kvm/hyp/at.c | 217 +++ > arch/arm64/kvm/hyp/switch.c | 13 +- > arch/arm64/kvm/sys_regs.c| 202 +++- > 7 files changed, 450 insertions(+), 4 deletions(-) > create mode 100644 arch/arm64/kvm/hyp/at.c > > diff --git a/arch/arm64/include/asm/kvm_arm.h > b/arch/arm64/include/asm/kvm_arm.h > index 1e4dbe0b1c8e..9903f10f6343 100644 > --- a/arch/arm64/include/asm/kvm_arm.h > +++ b/arch/arm64/include/asm/kvm_arm.h > @@ -24,6 +24,7 @@ > > /* Hyp Configuration Register (HCR) bits */ > #define HCR_FWB (UL(1) << 46) > +#define HCR_AT (UL(1) << 44) > #define HCR_NV1 (UL(1) << 43) > #define HCR_NV (UL(1) << 42) > #define HCR_API (UL(1) << 41) > @@ -119,6 +120,7 @@ > #define VTCR_EL2_TG0_16K TCR_TG0_16K > #define VTCR_EL2_TG0_64K TCR_TG0_64K > #define VTCR_EL2_SH0_MASKTCR_SH0_MASK > +#define VTCR_EL2_SH0_SHIFT TCR_SH0_SHIFT > #define VTCR_EL2_SH0_INNER TCR_SH0_INNER > #define VTCR_EL2_ORGN0_MASK TCR_ORGN0_MASK > #define VTCR_EL2_ORGN0_WBWA TCR_ORGN0_WBWA > diff --git a/arch/arm64/include/asm/kvm_asm.h > b/arch/arm64/include/asm/kvm_asm.h > index 5e956c2cd9b4..1cfa4d2cf772 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -69,6 +69,8 @@ extern void __kvm_tlb_flush_vmid(struct kvm_s2_mmu *mmu); > extern void __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu); > > extern void __kvm_timer_set_cntvoff(u32 cntvoff_low, u32 cntvoff_high); > +extern void __kvm_at_s1e01(struct kvm_vcpu *vcpu, u32 op, u64 vaddr); > +extern void __kvm_at_s1e2(struct kvm_vcpu *vcpu, u32 op, u64 vaddr); > > extern int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu); > > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > index 8b95f2c42c3d..b3a8d21c07b3 100644 > --- a/arch/arm64/include/asm/sysreg.h > +++ b/arch/arm64/include/asm/sysreg.h > @@ -536,6 +536,23 @@ > > #define SYS_SP_EL2 sys_reg(3, 6, 4, 1, 0) > > +/* AT instructions */ > +#define AT_Op0 1 > +#define AT_CRn 7 > + > +#define OP_AT_S1E1R sys_insn(AT_Op0, 0, AT_CRn, 8, 0) > +#define OP_AT_S1E1W sys_insn(AT_Op0, 0, AT_CRn, 8, 1) > +#define OP_AT_S1E0R sys_insn(AT_Op0, 0, AT_CRn, 8, 2) > +#define OP_AT_S1E0W sys_insn(AT_Op0, 0, AT_CRn, 8, 3) > +#define OP_AT_S1E1RP sys_insn(AT_Op0, 0, AT_CRn, 9, 0) > +#define OP_AT_S1E1WP sys_insn(AT_Op0, 0, AT_CRn, 9, 1) > +#define OP_AT_S1E2R sys_insn(AT_Op0, 4, AT_CRn, 8, 0) > +#define OP_AT_S1E2W sys_insn(AT_Op0, 4, AT_CRn, 8, 1) > +#define OP_AT_S12E1R sys_insn(AT_Op0, 4, AT_CRn, 8, 4) > +#define OP_AT_S12E1W sys_insn(AT_Op0, 4, AT_CRn, 8, 5) > +#define
Re: [PATCH v2 4/9] KVM: arm/arm64: vgic-its: Invalidate MSI-LPI translation cache on specific commands
Hi Marc, On 6/11/19 7:03 PM, Marc Zyngier wrote: > The LPI translation cache needs to be discarded when an ITS command > may affect the translation of an LPI (DISCARD and MAPD with V=0) or > the routing of an LPI to a redistributor with disabled LPIs (MOVI, > MOVALL). > > We decide to perform a full invalidation of the cache, irrespective > of the LPI that is affected. Commands are supposed to be rare enough > that it doesn't matter. > > Signed-off-by: Marc Zyngier > --- > virt/kvm/arm/vgic/vgic-its.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c > index 9b6b66204b97..5254bb762e1b 100644 > --- a/virt/kvm/arm/vgic/vgic-its.c > +++ b/virt/kvm/arm/vgic/vgic-its.c > @@ -733,6 +733,8 @@ static int vgic_its_cmd_handle_discard(struct kvm *kvm, > struct vgic_its *its, >* don't bother here since we clear the ITTE anyway and the >* pending state is a property of the ITTE struct. >*/ > + vgic_its_invalidate_cache(kvm); > + > its_free_ite(kvm, ite); > return 0; > } > @@ -768,6 +770,8 @@ static int vgic_its_cmd_handle_movi(struct kvm *kvm, > struct vgic_its *its, > ite->collection = collection; > vcpu = kvm_get_vcpu(kvm, collection->target_addr); > > + vgic_its_invalidate_cache(kvm); > + > return update_affinity(ite->irq, vcpu); > } > > @@ -996,6 +1000,8 @@ static void vgic_its_free_device(struct kvm *kvm, struct > its_device *device) > list_for_each_entry_safe(ite, temp, >itt_head, ite_list) > its_free_ite(kvm, ite); > > + vgic_its_invalidate_cache(kvm); > + > list_del(>dev_list); > kfree(device); > } > @@ -1249,6 +1255,8 @@ static int vgic_its_cmd_handle_movall(struct kvm *kvm, > struct vgic_its *its, > vgic_put_irq(kvm, irq); > } > > + vgic_its_invalidate_cache(kvm); All the commands are executed with the its_lock held. Now we don't take it anymore on the fast cache injection path. Don't we have a window where the move has been applied at table level and the cache is not yet invalidated? Same question for vgic_its_free_device(). Thanks Eric > + > kfree(intids); > return 0; > } > ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 13/59] KVM: arm64: nv: Handle virtual EL2 registers in vcpu_read/write_sys_reg()
On 6/21/19 10:37 AM, Marc Zyngier wrote:
> From: Andre Przywara
>
> KVM internally uses accessor functions when reading or writing the
> guest's system registers. This takes care of accessing either the stored
> copy or using the "live" EL1 system registers when the host uses VHE.
>
> With the introduction of virtual EL2 we add a bunch of EL2 system
> registers, which now must also be taken care of:
> - If the guest is running in vEL2, and we access an EL1 sysreg, we must
> revert to the stored version of that, and not use the CPU's copy.
> - If the guest is running in vEL1, and we access an EL2 sysreg, we must
> also use the stored version, since the CPU carries the EL1 copy.
> - Some EL2 system registers are supposed to affect the current execution
> of the system, so we need to put them into their respective EL1
> counterparts. For this we need to define a mapping between the two.
> This is done using the newly introduced struct el2_sysreg_map.
> - Some EL2 system registers have a different format than their EL1
> counterpart, so we need to translate them before writing them to the
> CPU. This is done using an (optional) translate function in the map.
> - There are the three special registers SP_EL2, SPSR_EL2 and ELR_EL2,
> which need some separate handling.
>
> All of these cases are now wrapped into the existing accessor functions,
> so KVM users wouldn't need to care whether they access EL2 or EL1
> registers and also which state the guest is in.
>
> This handles what was formerly known as the "shadow state" dynamically,
> without requiring a separate copy for each vCPU EL.
>
> Signed-off-by: Andre Przywara
> Signed-off-by: Marc Zyngier
> ---
> arch/arm64/include/asm/kvm_emulate.h | 6 +
> arch/arm64/include/asm/kvm_host.h| 5 +
> arch/arm64/kvm/sys_regs.c| 163 +++
> 3 files changed, 174 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h
> b/arch/arm64/include/asm/kvm_emulate.h
> index c43aac5fed69..f37006b6eec4 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -70,6 +70,12 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu);
> int kvm_inject_nested_sync(struct kvm_vcpu *vcpu, u64 esr_el2);
> int kvm_inject_nested_irq(struct kvm_vcpu *vcpu);
>
> +u64 translate_tcr(u64 tcr);
> +u64 translate_cptr(u64 tcr);
> +u64 translate_sctlr(u64 tcr);
> +u64 translate_ttbr0(u64 tcr);
> +u64 translate_cnthctl(u64 tcr);
> +
> static inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
> {
> return !(vcpu->arch.hcr_el2 & HCR_RW);
> diff --git a/arch/arm64/include/asm/kvm_host.h
> b/arch/arm64/include/asm/kvm_host.h
> index 2d4290d2513a..dae9c42a7219 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -217,6 +217,11 @@ enum vcpu_sysreg {
> NR_SYS_REGS /* Nothing after this line! */
> };
>
> +static inline bool sysreg_is_el2(int reg)
> +{
> + return reg >= FIRST_EL2_SYSREG && reg < NR_SYS_REGS;
> +}
> +
> /* 32bit mapping */
> #define c0_MPIDR (MPIDR_EL1 * 2) /* MultiProcessor ID Register */
> #define c0_CSSELR(CSSELR_EL1 * 2)/* Cache Size Selection Register */
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 693dd063c9c2..d024114da162 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -76,11 +76,142 @@ static bool write_to_read_only(struct kvm_vcpu *vcpu,
> return false;
> }
>
> +static u64 tcr_el2_ips_to_tcr_el1_ps(u64 tcr_el2)
> +{
> + return ((tcr_el2 & TCR_EL2_PS_MASK) >> TCR_EL2_PS_SHIFT)
> + << TCR_IPS_SHIFT;
> +}
> +
> +u64 translate_tcr(u64 tcr)
> +{
> + return TCR_EPD1_MASK | /* disable TTBR1_EL1 */
> +((tcr & TCR_EL2_TBI) ? TCR_TBI0 : 0) |
> +tcr_el2_ips_to_tcr_el1_ps(tcr) |
> +(tcr & TCR_EL2_TG0_MASK) |
> +(tcr & TCR_EL2_ORGN0_MASK) |
> +(tcr & TCR_EL2_IRGN0_MASK) |
> +(tcr & TCR_EL2_T0SZ_MASK);
> +}
> +
> +u64 translate_cptr(u64 cptr_el2)
> +{
> + u64 cpacr_el1 = 0;
> +
> + if (!(cptr_el2 & CPTR_EL2_TFP))
> + cpacr_el1 |= CPACR_EL1_FPEN;
> + if (cptr_el2 & CPTR_EL2_TTA)
> + cpacr_el1 |= CPACR_EL1_TTA;
> + if (!(cptr_el2 & CPTR_EL2_TZ))
> + cpacr_el1 |= CPACR_EL1_ZEN;
> +
> + return cpacr_el1;
> +}
> +
> +u64 translate_sctlr(u64 sctlr)
> +{
> + /* Bit 20 is RES1 in SCTLR_EL1, but RES0 in SCTLR_EL2 */
> + return sctlr | BIT(20);
> +}
> +
> +u64 translate_ttbr0(u64 ttbr0)
> +{
> + /* Force ASID to 0 (ASID 0 or RES0) */
> + return ttbr0 & ~GENMASK_ULL(63, 48);
> +}
> +
> +u64 translate_cnthctl(u64 cnthctl)
> +{
> + return ((cnthctl & 0x3) << 10) | (cnthctl & 0xfc);
> +}
> +
> +#define EL2_SYSREG(el2, el1, translate) \
> + [el2 - FIRST_EL2_SYSREG] = { el2, el1, translate }
> +#define PURE_EL2_SYSREG(el2) \
> + [el2 -
Re: [PATCH 16/59] KVM: arm64: nv: Save/Restore vEL2 sysregs
On 6/21/19 10:38 AM, Marc Zyngier wrote:
> From: Andre Przywara
>
> Whenever we need to restore the guest's system registers to the CPU, we
> now need to take care of the EL2 system registers as well. Most of them
> are accessed via traps only, but some have an immediate effect and also
> a guest running in VHE mode would expect them to be accessible via their
> EL1 encoding, which we do not trap.
>
> Split the current __sysreg_{save,restore}_el1_state() functions into
> handling common sysregs, then differentiate between the guest running in
> vEL2 and vEL1.
>
> For vEL2 we write the virtual EL2 registers with an identical format directly
> into their EL1 counterpart, and translate the few registers that have a
> different format for the same effect on the execution when running a
> non-VHE guest guest hypervisor.
>
> [ Commit message reworked and many bug fixes applied by Marc Zyngier
> and Christoffer Dall. ]
>
> Signed-off-by: Andre Przywara
> Signed-off-by: Marc Zyngier
> Signed-off-by: Christoffer Dall
> ---
> arch/arm64/kvm/hyp/sysreg-sr.c | 160 +++--
> 1 file changed, 153 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index 62866a68e852..2abb9c3ff24f 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
>
> /*
> * Non-VHE: Both host and guest must save everything.
> @@ -51,11 +52,9 @@ static void __hyp_text __sysreg_save_user_state(struct
> kvm_cpu_context *ctxt)
> ctxt->sys_regs[TPIDRRO_EL0] = read_sysreg(tpidrro_el0);
> }
>
> -static void __hyp_text __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
> +static void __hyp_text __sysreg_save_vel1_state(struct kvm_cpu_context *ctxt)
> {
> - ctxt->sys_regs[CSSELR_EL1] = read_sysreg(csselr_el1);
> ctxt->sys_regs[SCTLR_EL1] = read_sysreg_el1(SYS_SCTLR);
> - ctxt->sys_regs[ACTLR_EL1] = read_sysreg(actlr_el1);
> ctxt->sys_regs[CPACR_EL1] = read_sysreg_el1(SYS_CPACR);
> ctxt->sys_regs[TTBR0_EL1] = read_sysreg_el1(SYS_TTBR0);
> ctxt->sys_regs[TTBR1_EL1] = read_sysreg_el1(SYS_TTBR1);
> @@ -69,14 +68,58 @@ static void __hyp_text __sysreg_save_el1_state(struct
> kvm_cpu_context *ctxt)
> ctxt->sys_regs[CONTEXTIDR_EL1] = read_sysreg_el1(SYS_CONTEXTIDR);
> ctxt->sys_regs[AMAIR_EL1] = read_sysreg_el1(SYS_AMAIR);
> ctxt->sys_regs[CNTKCTL_EL1] = read_sysreg_el1(SYS_CNTKCTL);
> - ctxt->sys_regs[PAR_EL1] = read_sysreg(par_el1);
> - ctxt->sys_regs[TPIDR_EL1] = read_sysreg(tpidr_el1);
>
> ctxt->gp_regs.sp_el1= read_sysreg(sp_el1);
> ctxt->gp_regs.elr_el1 = read_sysreg_el1(SYS_ELR);
> ctxt->gp_regs.spsr[KVM_SPSR_EL1]= read_sysreg_el1(SYS_SPSR);
> }
>
> +static void __sysreg_save_vel2_state(struct kvm_cpu_context *ctxt)
> +{
> + ctxt->sys_regs[ESR_EL2] = read_sysreg_el1(SYS_ESR);
> + ctxt->sys_regs[AFSR0_EL2] = read_sysreg_el1(SYS_AFSR0);
> + ctxt->sys_regs[AFSR1_EL2] = read_sysreg_el1(SYS_AFSR1);
> + ctxt->sys_regs[FAR_EL2] = read_sysreg_el1(SYS_FAR);
> + ctxt->sys_regs[MAIR_EL2]= read_sysreg_el1(SYS_MAIR);
> + ctxt->sys_regs[VBAR_EL2]= read_sysreg_el1(SYS_VBAR);
> + ctxt->sys_regs[CONTEXTIDR_EL2] = read_sysreg_el1(SYS_CONTEXTIDR);
> + ctxt->sys_regs[AMAIR_EL2] = read_sysreg_el1(SYS_AMAIR);
> +
> + /*
> + * In VHE mode those registers are compatible between EL1 and EL2,
> + * and the guest uses the _EL1 versions on the CPU naturally.
> + * So we save them into their _EL2 versions here.
> + * For nVHE mode we trap accesses to those registers, so our
> + * _EL2 copy in sys_regs[] is always up-to-date and we don't need
> + * to save anything here.
> + */
> + if (__vcpu_el2_e2h_is_set(ctxt)) {
> + ctxt->sys_regs[SCTLR_EL2] = read_sysreg_el1(SYS_SCTLR);
> + ctxt->sys_regs[CPTR_EL2]= read_sysreg_el1(SYS_CPACR);
> + ctxt->sys_regs[TTBR0_EL2] = read_sysreg_el1(SYS_TTBR0);
> + ctxt->sys_regs[TTBR1_EL2] = read_sysreg_el1(SYS_TTBR1);
> + ctxt->sys_regs[TCR_EL2] = read_sysreg_el1(SYS_TCR);
> + ctxt->sys_regs[CNTHCTL_EL2] = read_sysreg_el1(SYS_CNTKCTL);
This goes against how the register is declared in arch/arm64/kvm/sys_regs.c
(added by patch 13), where it's declared as a "pure" EL2 register with no EL1
counterpart. I think this is correct, and having it as a pure register is not
the right approach, I'll explain why in patch 13.
> + }
> +
> + ctxt->sys_regs[SP_EL2] = read_sysreg(sp_el1);
> + ctxt->sys_regs[ELR_EL2] = read_sysreg_el1(SYS_ELR);
> + ctxt->sys_regs[SPSR_EL2]= __fixup_spsr_el2_read(ctxt,
>
Re: [PATCH 13/59] KVM: arm64: nv: Handle virtual EL2 registers in vcpu_read/write_sys_reg()
On 6/25/19 4:18 PM, Alexandru Elisei wrote:
> Hi Marc,
>
> A question regarding this patch. This patch modifies vcpu_{read,write}_sys_reg
> to handle virtual EL2 registers. However, the file kvm/emulate-nested.c added
> by
> patch 10/59 "KVM: arm64: nv: Support virtual EL2 exceptions" already uses
> vcpu_{read,write}_sys_reg to access EL2 registers. In my opinion, it doesn't
> really matter which comes first because nested support is only enabled in the
> last patch of the series, but I thought I should bring this up in case it is
> not
> what you intended.
>
> On 6/21/19 10:37 AM, Marc Zyngier wrote:
>> From: Andre Przywara
>>
>> KVM internally uses accessor functions when reading or writing the
>> guest's system registers. This takes care of accessing either the stored
>> copy or using the "live" EL1 system registers when the host uses VHE.
>>
>> With the introduction of virtual EL2 we add a bunch of EL2 system
>> registers, which now must also be taken care of:
>> - If the guest is running in vEL2, and we access an EL1 sysreg, we must
>> revert to the stored version of that, and not use the CPU's copy.
>> - If the guest is running in vEL1, and we access an EL2 sysreg, we must
>> also use the stored version, since the CPU carries the EL1 copy.
>> - Some EL2 system registers are supposed to affect the current execution
>> of the system, so we need to put them into their respective EL1
>> counterparts. For this we need to define a mapping between the two.
>> This is done using the newly introduced struct el2_sysreg_map.
>> - Some EL2 system registers have a different format than their EL1
>> counterpart, so we need to translate them before writing them to the
>> CPU. This is done using an (optional) translate function in the map.
>> - There are the three special registers SP_EL2, SPSR_EL2 and ELR_EL2,
>> which need some separate handling.
> I see no change in this patch related to SPSR_EL2. Special handling of
> SPSR_EL2
> is added in the next patch, patch 14/59 "KVM: arm64: nv: Handle SPSR_EL2
> specially".
>> All of these cases are now wrapped into the existing accessor functions,
>> so KVM users wouldn't need to care whether they access EL2 or EL1
>> registers and also which state the guest is in.
>>
>> This handles what was formerly known as the "shadow state" dynamically,
>> without requiring a separate copy for each vCPU EL.
>>
>> Signed-off-by: Andre Przywara
>> Signed-off-by: Marc Zyngier
>> ---
>> arch/arm64/include/asm/kvm_emulate.h | 6 +
>> arch/arm64/include/asm/kvm_host.h| 5 +
>> arch/arm64/kvm/sys_regs.c| 163 +++
>> 3 files changed, 174 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/kvm_emulate.h
>> b/arch/arm64/include/asm/kvm_emulate.h
>> index c43aac5fed69..f37006b6eec4 100644
>> --- a/arch/arm64/include/asm/kvm_emulate.h
>> +++ b/arch/arm64/include/asm/kvm_emulate.h
>> @@ -70,6 +70,12 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu);
>> int kvm_inject_nested_sync(struct kvm_vcpu *vcpu, u64 esr_el2);
>> int kvm_inject_nested_irq(struct kvm_vcpu *vcpu);
>>
>> +u64 translate_tcr(u64 tcr);
>> +u64 translate_cptr(u64 tcr);
>> +u64 translate_sctlr(u64 tcr);
>> +u64 translate_ttbr0(u64 tcr);
>> +u64 translate_cnthctl(u64 tcr);
>> +
>> static inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
>> {
>> return !(vcpu->arch.hcr_el2 & HCR_RW);
>> diff --git a/arch/arm64/include/asm/kvm_host.h
>> b/arch/arm64/include/asm/kvm_host.h
>> index 2d4290d2513a..dae9c42a7219 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -217,6 +217,11 @@ enum vcpu_sysreg {
>> NR_SYS_REGS /* Nothing after this line! */
>> };
>>
>> +static inline bool sysreg_is_el2(int reg)
>> +{
>> +return reg >= FIRST_EL2_SYSREG && reg < NR_SYS_REGS;
>> +}
>> +
>> /* 32bit mapping */
>> #define c0_MPIDR(MPIDR_EL1 * 2) /* MultiProcessor ID Register */
>> #define c0_CSSELR (CSSELR_EL1 * 2)/* Cache Size Selection Register */
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index 693dd063c9c2..d024114da162 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -76,11 +76,142 @@ static bool write_to_read_only(struct kvm_vcpu *vcpu,
>> return false;
>> }
>>
>> +static u64 tcr_el2_ips_to_tcr_el1_ps(u64 tcr_el2)
> The code seems to suggest that you are translating TCR_EL2.PS to TCR_EL1.IPS.
> Perhaps the function should be named tcr_el2_ps_to_tcr_el1_ips?
>> +{
>> +return ((tcr_el2 & TCR_EL2_PS_MASK) >> TCR_EL2_PS_SHIFT)
>> +<< TCR_IPS_SHIFT;
>> +}
>> +
>> +u64 translate_tcr(u64 tcr)
>> +{
>> +return TCR_EPD1_MASK | /* disable TTBR1_EL1 */
>> + ((tcr & TCR_EL2_TBI) ? TCR_TBI0 : 0) |
>> + tcr_el2_ips_to_tcr_el1_ps(tcr) |
>> + (tcr & TCR_EL2_TG0_MASK) |
>> + (tcr & TCR_EL2_ORGN0_MASK) |
>> + (tcr & TCR_EL2_IRGN0_MASK) |
>>
Re: [PATCH RFC 11/14] arm64: Move the ASID allocator code in a separate file
On Sun, Jun 30, 2019 at 12:29:46PM +0800, Guo Ren wrote:
> On Mon, Jun 24, 2019 at 11:38 PM Catalin Marinas
> wrote:
> > On Mon, Jun 24, 2019 at 12:35:35AM +0800, Guo Ren wrote:
> > > On Fri, Jun 21, 2019 at 10:16 PM Catalin Marinas
> > > wrote:
> > > > BTW, if you find the algorithm fairly straightforward ;), see this
> > > > bug-fix which took a formal model to identify: a8ffaaa060b8 ("arm64:
> > > > asid: Do not replace active_asids if already 0").
> > [...]
> > > Btw, Is this detected by arm's aisd allocator TLA+ model ? Or a real
> > > bug report ?
> >
> > This specific bug was found by the TLA+ model checker (at the time we
> > were actually tracking down another bug with multi-threaded CPU sharing
> > the TLB, bug also confirmed by the formal model).
>
> Could you tell me the ref-link about "another bug with multi-threaded
> CPU sharing the TLB" ?
>
> In my concept, the multi-core asid mechanism is also applicable to
> multi-thread shared TLB, but it will generate redundant tlbflush. From
> the software design logic, multi-threaded is treated as multi-cores
> without error, but performance is not optimized.
>From the ASID reservation/allocation perspective, the mechanism is the
same between multi-threaded with a shared TLB and multi-core. On arm64,
a local_flush_tlb_all() on a thread invalidates the TLB for the other
threads of the same core.
The actual problem with multi-threaded CPUs is a lot more subtle.
Digging some internal email from 1.5 years ago and pasting it below
(where "current ASID algorithm" refers to the one prior to the fix and
CnP - Common Not Private - means shared TLBs on a multi-threaded CPU):
The current ASID roll-over algorithm allows for a small window where
active_asids for a CPU (P1) is different from the actual ASID in TTBR0.
This can lead to a roll-over on a different CPU (P2) allocating an ASID
(for a different task) which is still hardware-active on P1.
A TLBI on a CPU (or a peer CPU with CnP) does not guarantee that all the
entries corresponding to a valid TTBRx are removed as they can still be
speculatively loaded immediately after TLBI.
While having two different page tables with the same ASID on different
CPUs should be fine without CnP, it becomes problematic when CnP is
enabled:
P1 P2
-- --
TTBR0.BADDR = T1
TTBR0.ASID = A1
check_and_switch_context(T2,A2)
asid_maps[P1] = A2
goto fastpath
check_and_switch_context(T3,A0)
new_context
ASID roll-over allocates A1
since it is not active
TLBI ALL
speculate TTBR0.ASID = A1 entry
TTBR0.BADDR = T3
TTBR0.ASID = A1
TTBR0.BADDR = T2
TTBR0.ASID = A2
After this, the common TLB on P1 and P2 (CnP) contains entries
corresponding to the old T1 and A1. Task T3 using the same ASID A1 can
hit such entries. (T1,A1) will eventually be removed from the TLB on the
next context switch on P1 since tlb_flush_pending was set but this is
not guaranteed to happen.
The fix on arm64 (as part of 5ffdfaedfa0a - "arm64: mm: Support Common
Not Private translations") was to set the reserved TTBR0 in
check_and_switch_context(), preventing speculative loads into the TLB
being tagged with the wrong ASID. So this is specific to the ARM CPUs
behaviour w.r.t. speculative TLB loads, it may not be the case (yet) for
your architecture.
--
Catalin
Re: [PATCH 39/59] KVM: arm64: nv: Move last_vcpu_ran to be per s2 mmu
On 21/06/2019 10:38, Marc Zyngier wrote:
> last_vcpu_ran has to be per s2 mmu now that we can have multiple S2
> per VM. Let's take this opportunity to perform some cleanup.
>
> Signed-off-by: Marc Zyngier
> ---
> arch/arm/include/asm/kvm_host.h | 6 +++---
> arch/arm/include/asm/kvm_mmu.h| 2 +-
> arch/arm64/include/asm/kvm_host.h | 6 +++---
> arch/arm64/include/asm/kvm_mmu.h | 2 +-
> arch/arm64/kvm/nested.c | 13 ++---
> virt/kvm/arm/arm.c| 22 --
> virt/kvm/arm/mmu.c| 26 --
> 7 files changed, 38 insertions(+), 39 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index b821eb2383ad..cc761610e41e 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -63,15 +63,15 @@ struct kvm_s2_mmu {
> pgd_t *pgd;
> phys_addr_t pgd_phys;
>
> + /* The last vcpu id that ran on each physical CPU */
> + int __percpu *last_vcpu_ran;
> +
> struct kvm *kvm;
> };
>
> struct kvm_arch {
> struct kvm_s2_mmu mmu;
>
> - /* The last vcpu id that ran on each physical CPU */
> - int __percpu *last_vcpu_ran;
> -
> /* Stage-2 page table */
> pgd_t *pgd;
> phys_addr_t pgd_phys;
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index afabf1fd1d17..7a6e9008ed45 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -52,7 +52,7 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t
> size,
> void free_hyp_pgds(void);
>
> void stage2_unmap_vm(struct kvm *kvm);
> -int kvm_alloc_stage2_pgd(struct kvm_s2_mmu *mmu);
> +int kvm_init_stage2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu);
> void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu);
> int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
> phys_addr_t pa, unsigned long size, bool writable);
> diff --git a/arch/arm64/include/asm/kvm_host.h
> b/arch/arm64/include/asm/kvm_host.h
> index cc238de170d2..b71a7a237f95 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -104,6 +104,9 @@ struct kvm_s2_mmu {
>* >0: Somebody is actively using this.
>*/
> atomic_t refcnt;
> +
> + /* The last vcpu id that ran on each physical CPU */
> + int __percpu *last_vcpu_ran;
> };
>
> static inline bool kvm_s2_mmu_valid(struct kvm_s2_mmu *mmu)
> @@ -124,9 +127,6 @@ struct kvm_arch {
> /* VTCR_EL2 value for this VM */
> u64vtcr;
>
> - /* The last vcpu id that ran on each physical CPU */
> - int __percpu *last_vcpu_ran;
> -
> /* The maximum number of vCPUs depends on the used GIC model */
> int max_vcpus;
>
> diff --git a/arch/arm64/include/asm/kvm_mmu.h
> b/arch/arm64/include/asm/kvm_mmu.h
> index f4c5ac5eb95f..53103607065a 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -169,7 +169,7 @@ void free_hyp_pgds(void);
>
> void kvm_unmap_stage2_range(struct kvm_s2_mmu *mmu, phys_addr_t start, u64
> size);
> void stage2_unmap_vm(struct kvm *kvm);
> -int kvm_alloc_stage2_pgd(struct kvm_s2_mmu *mmu);
> +int kvm_init_stage2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu);
> void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu);
> int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
> phys_addr_t pa, unsigned long size, bool writable);
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 8880033fb6e0..09afafbdc8fe 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -52,18 +52,17 @@ int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
>GFP_KERNEL | __GFP_ZERO);
>
> if (tmp) {
> - if (tmp != kvm->arch.nested_mmus)
> + if (tmp != kvm->arch.nested_mmus) {
> kfree(kvm->arch.nested_mmus);
> + kvm->arch.nested_mmus = NULL;
> + kvm->arch.nested_mmus_size = 0;
> + }
>
> - tmp[num_mmus - 1].kvm = kvm;
> - atomic_set([num_mmus - 1].refcnt, 0);
> - ret = kvm_alloc_stage2_pgd([num_mmus - 1]);
> + ret = kvm_init_stage2_mmu(kvm, [num_mmus - 1]);
> if (ret)
> goto out;
>
> - tmp[num_mmus - 2].kvm = kvm;
> - atomic_set([num_mmus - 2].refcnt, 0);
> - ret = kvm_alloc_stage2_pgd([num_mmus - 2]);
> + ret = kvm_init_stage2_mmu(kvm, [num_mmus - 2]);
> if (ret) {
> kvm_free_stage2_pgd([num_mmus - 1]);
> goto out;
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index bcca27d5c481..e8b584b79847 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -99,29 +99,21 @@ void kvm_arch_check_processor_compat(void
Re: KVM works on RPi4
On 6/30/19 11:49 AM, Jan Kiszka wrote:
> On 30.06.19 12:18, Jan Kiszka wrote:
>> On 30.06.19 11:34, Jan Kiszka wrote:
>>> On 30.06.19 00:42, Marc Zyngier wrote:
On Sat, 29 Jun 2019 19:09:37 +0200
Jan Kiszka wrote:
> However, as the Raspberry kernel is not yet ready for 64-bit (and
> upstream is not in sight), I had to use legacy 32-bit mode. And there we
> stumble over the core detection. This little patch made it work, though:
>
> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> index 2b8de885b2bf..01606aad73cc 100644
> --- a/arch/arm/kvm/guest.c
> +++ b/arch/arm/kvm/guest.c
> @@ -290,6 +290,7 @@ int __attribute_const__ kvm_target_cpu(void)
> case ARM_CPU_PART_CORTEX_A7:
> return KVM_ARM_TARGET_CORTEX_A7;
> case ARM_CPU_PART_CORTEX_A15:
> + case ARM_CPU_PART_CORTEX_A72:
> return KVM_ARM_TARGET_CORTEX_A15;
> default:
> return -EINVAL;
>
> That raises the question if this is hack or a valid change and if there
> is general interest in mapping 64-bit cores on 32-bit if they happen to
> run in 32-bit mode.
The real thing to do here would be to move to a generic target, much
like we did on the 64bit side. Could you investigate that instead? It
would also allow KVM to be used on other 32bit cores such as
A12/A17/A32.
>>>
>>> You mean something like KVM_ARM_TARGET_GENERIC_V8? Need to study that...
>>>
>>
>> Hmm, looking at what KVM_ARM_TARGET_CORTEX_A7 and ..._A15 differentiates, I
>> found nothing so far:
>>
>> kvm_reset_vcpu:
>> switch (vcpu->arch.target) {
>> case KVM_ARM_TARGET_CORTEX_A7:
>> case KVM_ARM_TARGET_CORTEX_A15:
>> reset_regs = _regs_reset;
>> vcpu->arch.midr = read_cpuid_id();
>> break;
>>
>> And arch/arm/kvm/coproc_a15.c looks like a copy of coproc_a7.c, just with
>> some
>> symbols renamed.
>
> OK, found it: The reset values of SCTLR differ, in one bit. A15 starts with
> branch prediction (11) off, A7 with that feature enabled. Quite some
> boilerplate
> code for managing a single bit.
>
> For a generic target, can we simply assume A15 reset behaviour?
IIUC, it'd work only for ARCH_VIRT guest, which is known not to touch IMP_DEF
registers. Unfortunately, other variants of guests (ARCH_VEXPRESS?) might touch
such registers, for instance, l2ctlr is often used for querying number of
populated
cpus, and it might not be present at all on v8. Also, content of IMP_DEF
register
is not fixed and meaning of the bits may wary between different
implementations, so
guests may react differently.
Just my 2p.
Cheers
Vladimir
>
> Jan
> ___
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 38/59] KVM: arm64: nv: Unmap/flush shadow stage 2 page tables
On 21/06/2019 10:38, Marc Zyngier wrote:
> From: Christoffer Dall
>
> Unmap/flush shadow stage 2 page tables for the nested VMs as well as the
> stage 2 page table for the guest hypervisor.
>
> Note: A bunch of the code in mmu.c relating to MMU notifiers is
> currently dealt with in an extremely abrupt way, for example by clearing
> out an entire shadow stage-2 table. This will be handled in a more
> efficient way using the reverse mapping feature in a later version of
> the patch series.
>
> Signed-off-by: Christoffer Dall
> Signed-off-by: Jintack Lim
> Signed-off-by: Marc Zyngier
> ---
> arch/arm64/include/asm/kvm_mmu.h| 3 +++
> arch/arm64/include/asm/kvm_nested.h | 3 +++
> arch/arm64/kvm/nested.c | 39 +++
> virt/kvm/arm/arm.c | 4 ++-
> virt/kvm/arm/mmu.c | 42 +++--
> 5 files changed, 82 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_mmu.h
> b/arch/arm64/include/asm/kvm_mmu.h
> index 32bcaa1845dc..f4c5ac5eb95f 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -163,6 +163,8 @@ int create_hyp_io_mappings(phys_addr_t phys_addr, size_t
> size,
> void __iomem **haddr);
> int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size,
>void **haddr);
> +void kvm_stage2_flush_range(struct kvm_s2_mmu *mmu,
> + phys_addr_t addr, phys_addr_t end);
> void free_hyp_pgds(void);
>
> void kvm_unmap_stage2_range(struct kvm_s2_mmu *mmu, phys_addr_t start, u64
> size);
> @@ -171,6 +173,7 @@ int kvm_alloc_stage2_pgd(struct kvm_s2_mmu *mmu);
> void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu);
> int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
> phys_addr_t pa, unsigned long size, bool writable);
> +void kvm_stage2_wp_range(struct kvm_s2_mmu *mmu, phys_addr_t addr,
> phys_addr_t end);
>
> int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run);
>
> diff --git a/arch/arm64/include/asm/kvm_nested.h
> b/arch/arm64/include/asm/kvm_nested.h
> index 052d46d96201..3b415bc76ced 100644
> --- a/arch/arm64/include/asm/kvm_nested.h
> +++ b/arch/arm64/include/asm/kvm_nested.h
> @@ -48,6 +48,9 @@ extern int kvm_walk_nested_s2(struct kvm_vcpu *vcpu,
> phys_addr_t gipa,
> extern int kvm_s2_handle_perm_fault(struct kvm_vcpu *vcpu,
> struct kvm_s2_trans *trans);
> extern int kvm_inject_s2_fault(struct kvm_vcpu *vcpu, u64 esr_el2);
> +extern void kvm_nested_s2_wp(struct kvm *kvm);
> +extern void kvm_nested_s2_clear(struct kvm *kvm);
> +extern void kvm_nested_s2_flush(struct kvm *kvm);
> int handle_wfx_nested(struct kvm_vcpu *vcpu, bool is_wfe);
> extern bool forward_traps(struct kvm_vcpu *vcpu, u64 control_bit);
> extern bool forward_nv_traps(struct kvm_vcpu *vcpu);
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 023027fa2db5..8880033fb6e0 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -456,6 +456,45 @@ int kvm_inject_s2_fault(struct kvm_vcpu *vcpu, u64
> esr_el2)
> return kvm_inject_nested_sync(vcpu, esr_el2);
> }
>
> +/* expects kvm->mmu_lock to be held */
> +void kvm_nested_s2_wp(struct kvm *kvm)
> +{
> + int i;
> +
> + for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
> + struct kvm_s2_mmu *mmu = >arch.nested_mmus[i];
> +
> + if (kvm_s2_mmu_valid(mmu))
> + kvm_stage2_wp_range(mmu, 0, kvm_phys_size(kvm));
> + }
> +}
> +
> +/* expects kvm->mmu_lock to be held */
> +void kvm_nested_s2_clear(struct kvm *kvm)
> +{
> + int i;
> +
> + for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
> + struct kvm_s2_mmu *mmu = >arch.nested_mmus[i];
> +
> + if (kvm_s2_mmu_valid(mmu))
> + kvm_unmap_stage2_range(mmu, 0, kvm_phys_size(kvm));
> + }
> +}
> +
> +/* expects kvm->mmu_lock to be held */
> +void kvm_nested_s2_flush(struct kvm *kvm)
> +{
> + int i;
> +
> + for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
> + struct kvm_s2_mmu *mmu = >arch.nested_mmus[i];
> +
> + if (kvm_s2_mmu_valid(mmu))
> + kvm_stage2_flush_range(mmu, 0, kvm_phys_size(kvm));
> + }
> +}
> +
> /*
> * Inject wfx to the virtual EL2 if this is not from the virtual EL2 and
> * the virtual HCR_EL2.TWX is set. Otherwise, let the host hypervisor
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 4e3cbfa1ecbe..bcca27d5c481 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1005,8 +1005,10 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct
> kvm_vcpu *vcpu,
>* Ensure a rebooted VM will fault in RAM pages and detect if the
>* guest MMU is turned off and flush the caches as needed.
>*/
> - if (vcpu->arch.has_run_once)
> + if
