From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH v4 28/40] KVM: arm64: Defer saving/restoring 64-bit sysregs to vcpu load/put on VHE Date: Thu, 22 Feb 2018 20:48:31 +0100 Message-ID: <20180222194831.GZ29376@cbox> References: <20180215210332.8648-1-christoffer.dall@linaro.org> <20180215210332.8648-29-christoffer.dall@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Andrew Jones , kvm@vger.kernel.org, Marc Zyngier , Tomasz Nowicki , Dave Martin , Yury Norov , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, Shih-Wei Li To: Julien Grall Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org List-Id: kvm.vger.kernel.org On Thu, Feb 22, 2018 at 06:30:11PM +0000, Julien Grall wrote: > Hi Christoffer, > > On 15/02/18 21:03, Christoffer Dall wrote: > >Some system registers do not affect the host kernel's execution and can > >therefore be loaded when we are about to run a VCPU and we don't have to > >restore the host state to the hardware before the time when we are > >actually about to return to userspace or schedule out the VCPU thread. > > > >The EL1 system registers and the userspace state registers only > >affecting EL0 execution do not need to be saved and restored on every > >switch between the VM and the host, because they don't affect the host > >kernel's execution. > > > >We mark all registers which are now deffered as such in the > > NIT: s/deffered/deferred/ I think. > > >vcpu_{read,write}_sys_reg accessors in sys-regs.c to ensure the most > >up-to-date copy is always accessed. > > > >Note MPIDR_EL1 (controlled via VMPIDR_EL2) is accessed from other vcpu > >threads, for example via the GIC emulation, and therefore must be > >declared as immediate, which is fine as the guest cannot modify this > >value. > > > >The 32-bit sysregs can also be deferred but we do this in a separate > >patch as it requires a bit more infrastructure. > > > [...] > > >diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > >index b3c3f014aa61..f060309337aa 100644 > >--- a/arch/arm64/kvm/sys_regs.c > >+++ b/arch/arm64/kvm/sys_regs.c > >@@ -87,6 +87,26 @@ u64 vcpu_read_sys_reg(struct kvm_vcpu *vcpu, int reg) > > * exit from the guest but are only saved on vcpu_put. > > */ > > switch (reg) { > >+ case CSSELR_EL1: return read_sysreg_s(SYS_CSSELR_EL1); > >+ case SCTLR_EL1: return read_sysreg_s(sctlr_EL12); > >+ case ACTLR_EL1: return read_sysreg_s(SYS_ACTLR_EL1); > >+ case CPACR_EL1: return read_sysreg_s(cpacr_EL12); > >+ case TTBR0_EL1: return read_sysreg_s(ttbr0_EL12); > >+ case TTBR1_EL1: return read_sysreg_s(ttbr1_EL12); > >+ case TCR_EL1: return read_sysreg_s(tcr_EL12); > >+ case ESR_EL1: return read_sysreg_s(esr_EL12); > >+ case AFSR0_EL1: return read_sysreg_s(afsr0_EL12); > >+ case AFSR1_EL1: return read_sysreg_s(afsr1_EL12); > >+ case FAR_EL1: return read_sysreg_s(far_EL12); > >+ case MAIR_EL1: return read_sysreg_s(mair_EL12); > >+ case VBAR_EL1: return read_sysreg_s(vbar_EL12); > >+ case CONTEXTIDR_EL1: return read_sysreg_s(contextidr_EL12); > >+ case TPIDR_EL0: return read_sysreg_s(SYS_TPIDR_EL0); > >+ case TPIDRRO_EL0: return read_sysreg_s(SYS_TPIDRRO_EL0); > > I find a bit confusing to have some EL0 registers in the middle of EL1 ones. > Is it because they are listed by encoding? > They are sorted in the same way as the sysreg array defines. I can add that to the commentary. > >+ case TPIDR_EL1: return read_sysreg_s(SYS_TPIDR_EL1); > >+ case AMAIR_EL1: return read_sysreg_s(amair_EL12); > >+ case CNTKCTL_EL1: return read_sysreg_s(cntkctl_EL12); > >+ case PAR_EL1: return read_sysreg_s(SYS_PAR_EL1); > > } > > immediate_read: > >@@ -103,6 +123,26 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, int reg, u64 val) > > * entry to the guest but are only restored on vcpu_load. > > */ > > switch (reg) { > >+ case CSSELR_EL1: write_sysreg_s(val, SYS_CSSELR_EL1); return; > >+ case SCTLR_EL1: write_sysreg_s(val, sctlr_EL12); return; > >+ case ACTLR_EL1: write_sysreg_s(val, SYS_ACTLR_EL1); return; > >+ case CPACR_EL1: write_sysreg_s(val, cpacr_EL12); return; > >+ case TTBR0_EL1: write_sysreg_s(val, ttbr0_EL12); return; > >+ case TTBR1_EL1: write_sysreg_s(val, ttbr1_EL12); return; > >+ case TCR_EL1: write_sysreg_s(val, tcr_EL12); return; > >+ case ESR_EL1: write_sysreg_s(val, esr_EL12); return; > >+ case AFSR0_EL1: write_sysreg_s(val, afsr0_EL12); return; > >+ case AFSR1_EL1: write_sysreg_s(val, afsr1_EL12); return; > >+ case FAR_EL1: write_sysreg_s(val, far_EL12); return; > >+ case MAIR_EL1: write_sysreg_s(val, mair_EL12); return; > >+ case VBAR_EL1: write_sysreg_s(val, vbar_EL12); return; > >+ case CONTEXTIDR_EL1: write_sysreg_s(val, contextidr_EL12); return; > >+ case TPIDR_EL0: write_sysreg_s(val, SYS_TPIDR_EL0); return; > >+ case TPIDRRO_EL0: write_sysreg_s(val, SYS_TPIDRRO_EL0); return; > >+ case TPIDR_EL1: write_sysreg_s(val, SYS_TPIDR_EL1); return; > >+ case AMAIR_EL1: write_sysreg_s(val, amair_EL12); return; > >+ case CNTKCTL_EL1: write_sysreg_s(val, cntkctl_EL12); return; > >+ case PAR_EL1: write_sysreg_s(val, SYS_PAR_EL1); return; > > } > > immediate_write: > > > Thanks, -Christoffer From mboxrd@z Thu Jan 1 00:00:00 1970 From: christoffer.dall@linaro.org (Christoffer Dall) Date: Thu, 22 Feb 2018 20:48:31 +0100 Subject: [PATCH v4 28/40] KVM: arm64: Defer saving/restoring 64-bit sysregs to vcpu load/put on VHE In-Reply-To: References: <20180215210332.8648-1-christoffer.dall@linaro.org> <20180215210332.8648-29-christoffer.dall@linaro.org> Message-ID: <20180222194831.GZ29376@cbox> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Feb 22, 2018 at 06:30:11PM +0000, Julien Grall wrote: > Hi Christoffer, > > On 15/02/18 21:03, Christoffer Dall wrote: > >Some system registers do not affect the host kernel's execution and can > >therefore be loaded when we are about to run a VCPU and we don't have to > >restore the host state to the hardware before the time when we are > >actually about to return to userspace or schedule out the VCPU thread. > > > >The EL1 system registers and the userspace state registers only > >affecting EL0 execution do not need to be saved and restored on every > >switch between the VM and the host, because they don't affect the host > >kernel's execution. > > > >We mark all registers which are now deffered as such in the > > NIT: s/deffered/deferred/ I think. > > >vcpu_{read,write}_sys_reg accessors in sys-regs.c to ensure the most > >up-to-date copy is always accessed. > > > >Note MPIDR_EL1 (controlled via VMPIDR_EL2) is accessed from other vcpu > >threads, for example via the GIC emulation, and therefore must be > >declared as immediate, which is fine as the guest cannot modify this > >value. > > > >The 32-bit sysregs can also be deferred but we do this in a separate > >patch as it requires a bit more infrastructure. > > > [...] > > >diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > >index b3c3f014aa61..f060309337aa 100644 > >--- a/arch/arm64/kvm/sys_regs.c > >+++ b/arch/arm64/kvm/sys_regs.c > >@@ -87,6 +87,26 @@ u64 vcpu_read_sys_reg(struct kvm_vcpu *vcpu, int reg) > > * exit from the guest but are only saved on vcpu_put. > > */ > > switch (reg) { > >+ case CSSELR_EL1: return read_sysreg_s(SYS_CSSELR_EL1); > >+ case SCTLR_EL1: return read_sysreg_s(sctlr_EL12); > >+ case ACTLR_EL1: return read_sysreg_s(SYS_ACTLR_EL1); > >+ case CPACR_EL1: return read_sysreg_s(cpacr_EL12); > >+ case TTBR0_EL1: return read_sysreg_s(ttbr0_EL12); > >+ case TTBR1_EL1: return read_sysreg_s(ttbr1_EL12); > >+ case TCR_EL1: return read_sysreg_s(tcr_EL12); > >+ case ESR_EL1: return read_sysreg_s(esr_EL12); > >+ case AFSR0_EL1: return read_sysreg_s(afsr0_EL12); > >+ case AFSR1_EL1: return read_sysreg_s(afsr1_EL12); > >+ case FAR_EL1: return read_sysreg_s(far_EL12); > >+ case MAIR_EL1: return read_sysreg_s(mair_EL12); > >+ case VBAR_EL1: return read_sysreg_s(vbar_EL12); > >+ case CONTEXTIDR_EL1: return read_sysreg_s(contextidr_EL12); > >+ case TPIDR_EL0: return read_sysreg_s(SYS_TPIDR_EL0); > >+ case TPIDRRO_EL0: return read_sysreg_s(SYS_TPIDRRO_EL0); > > I find a bit confusing to have some EL0 registers in the middle of EL1 ones. > Is it because they are listed by encoding? > They are sorted in the same way as the sysreg array defines. I can add that to the commentary. > >+ case TPIDR_EL1: return read_sysreg_s(SYS_TPIDR_EL1); > >+ case AMAIR_EL1: return read_sysreg_s(amair_EL12); > >+ case CNTKCTL_EL1: return read_sysreg_s(cntkctl_EL12); > >+ case PAR_EL1: return read_sysreg_s(SYS_PAR_EL1); > > } > > immediate_read: > >@@ -103,6 +123,26 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, int reg, u64 val) > > * entry to the guest but are only restored on vcpu_load. > > */ > > switch (reg) { > >+ case CSSELR_EL1: write_sysreg_s(val, SYS_CSSELR_EL1); return; > >+ case SCTLR_EL1: write_sysreg_s(val, sctlr_EL12); return; > >+ case ACTLR_EL1: write_sysreg_s(val, SYS_ACTLR_EL1); return; > >+ case CPACR_EL1: write_sysreg_s(val, cpacr_EL12); return; > >+ case TTBR0_EL1: write_sysreg_s(val, ttbr0_EL12); return; > >+ case TTBR1_EL1: write_sysreg_s(val, ttbr1_EL12); return; > >+ case TCR_EL1: write_sysreg_s(val, tcr_EL12); return; > >+ case ESR_EL1: write_sysreg_s(val, esr_EL12); return; > >+ case AFSR0_EL1: write_sysreg_s(val, afsr0_EL12); return; > >+ case AFSR1_EL1: write_sysreg_s(val, afsr1_EL12); return; > >+ case FAR_EL1: write_sysreg_s(val, far_EL12); return; > >+ case MAIR_EL1: write_sysreg_s(val, mair_EL12); return; > >+ case VBAR_EL1: write_sysreg_s(val, vbar_EL12); return; > >+ case CONTEXTIDR_EL1: write_sysreg_s(val, contextidr_EL12); return; > >+ case TPIDR_EL0: write_sysreg_s(val, SYS_TPIDR_EL0); return; > >+ case TPIDRRO_EL0: write_sysreg_s(val, SYS_TPIDRRO_EL0); return; > >+ case TPIDR_EL1: write_sysreg_s(val, SYS_TPIDR_EL1); return; > >+ case AMAIR_EL1: write_sysreg_s(val, amair_EL12); return; > >+ case CNTKCTL_EL1: write_sysreg_s(val, cntkctl_EL12); return; > >+ case PAR_EL1: write_sysreg_s(val, SYS_PAR_EL1); return; > > } > > immediate_write: > > > Thanks, -Christoffer