| Message ID | 20220217104248.30948-1-vincent.chen@sifive.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: kvm: refine __kvm_riscv_switch_to and __kvm_riscv_switch_from function | expand |
On Thu, Feb 17, 2022 at 4:13 PM Vincent Chen <vincent.chen@sifive.com> wrote: > I think the patch subject should be: RISC-V: KVM: Refine __kvm_riscv_switch_to() implementation > Kernel uses __kvm_riscv_switch_to() and __kvm_riscv_switch_from() to switch s/__kvm_riscv_switch_from/__kvm_switch_return/ > the context of host kernel and guest kernel. Several CSRs belonging to the > context will be read and written during the context switch. To ensure > atomic read-modify-write control of CSR and ordering of CSR accesses, some > hardware blocks or flushes the pipeline when writing a CSR. In this s/hardware blocks or flushes/hardware blocks flush/ > circumstance, grouping CSR executions together as much as possible can > reduce the performance impact of the pipeline. Therefore, this commit > reorders the CSR instructions to enhance the context switch performance.. I am mostly fine with this patch except a few nits below. > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > Suggested-by: Hsinyi Lee <hsinyi.lee@sifive.com> > Suggested-by: Fu-Ching Yang <fu-ching.yang@sifive.com> > --- > arch/riscv/kvm/vcpu_switch.S | 55 ++++++++++++++++++------------------ > 1 file changed, 28 insertions(+), 27 deletions(-) > > diff --git a/arch/riscv/kvm/vcpu_switch.S b/arch/riscv/kvm/vcpu_switch.S > index 029a28a195c6..75cf21e5ed92 100644 > --- a/arch/riscv/kvm/vcpu_switch.S > +++ b/arch/riscv/kvm/vcpu_switch.S > @@ -40,34 +40,35 @@ ENTRY(__kvm_riscv_switch_to) > REG_S s9, (KVM_ARCH_HOST_S9)(a0) > REG_S s10, (KVM_ARCH_HOST_S10)(a0) > REG_S s11, (KVM_ARCH_HOST_S11)(a0) Add a new line here. Also add a single line comment as follows: /* Load Guest CSR values */ > + REG_L t0, (KVM_ARCH_GUEST_SSTATUS)(a0) > + REG_L t1, (KVM_ARCH_GUEST_HSTATUS)(a0) > + REG_L t2, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) > + la t4, __kvm_switch_return > + REG_L t5, (KVM_ARCH_GUEST_SEPC)(a0) > > /* Save Host and Restore Guest SSTATUS */ > - REG_L t0, (KVM_ARCH_GUEST_SSTATUS)(a0) > csrrw t0, CSR_SSTATUS, t0 > - REG_S t0, (KVM_ARCH_HOST_SSTATUS)(a0) > > /* Save Host and Restore Guest HSTATUS */ > - REG_L t1, (KVM_ARCH_GUEST_HSTATUS)(a0) > csrrw t1, CSR_HSTATUS, t1 > - REG_S t1, (KVM_ARCH_HOST_HSTATUS)(a0) > > /* Save Host and Restore Guest SCOUNTEREN */ > - REG_L t2, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) > csrrw t2, CSR_SCOUNTEREN, t2 > - REG_S t2, (KVM_ARCH_HOST_SCOUNTEREN)(a0) > - > - /* Save Host SSCRATCH and change it to struct kvm_vcpu_arch pointer */ > - csrrw t3, CSR_SSCRATCH, a0 > - REG_S t3, (KVM_ARCH_HOST_SSCRATCH)(a0) > > /* Save Host STVEC and change it to return path */ > - la t4, __kvm_switch_return > csrrw t4, CSR_STVEC, t4 > - REG_S t4, (KVM_ARCH_HOST_STVEC)(a0) > + > + /* Save Host SSCRATCH and change it to struct kvm_vcpu_arch pointer */ > + csrrw t3, CSR_SSCRATCH, a0 > > /* Restore Guest SEPC */ > - REG_L t0, (KVM_ARCH_GUEST_SEPC)(a0) > - csrw CSR_SEPC, t0 > + csrw CSR_SEPC, t5 > + Add a single line comment as follows: /* Store Host CSR values */ > + REG_S t0, (KVM_ARCH_HOST_SSTATUS)(a0) > + REG_S t1, (KVM_ARCH_HOST_HSTATUS)(a0) > + REG_S t2, (KVM_ARCH_HOST_SCOUNTEREN)(a0) > + REG_S t3, (KVM_ARCH_HOST_SSCRATCH)(a0) > + REG_S t4, (KVM_ARCH_HOST_STVEC)(a0) > > /* Restore Guest GPRs (except A0) */ > REG_L ra, (KVM_ARCH_GUEST_RA)(a0) > @@ -144,35 +145,35 @@ __kvm_switch_return: > REG_S t4, (KVM_ARCH_GUEST_T4)(a0) > REG_S t5, (KVM_ARCH_GUEST_T5)(a0) > REG_S t6, (KVM_ARCH_GUEST_T6)(a0) Add a new line here. Also add a single line comment as follows: /* Load Host CSR values */ > + REG_L t1, (KVM_ARCH_HOST_STVEC)(a0) > + REG_L t2, (KVM_ARCH_HOST_SSCRATCH)(a0) > + REG_L t3, (KVM_ARCH_HOST_SCOUNTEREN)(a0) > + REG_L t4, (KVM_ARCH_HOST_HSTATUS)(a0) > + REG_L t5, (KVM_ARCH_HOST_SSTATUS)(a0) > > /* Save Guest SEPC */ > csrr t0, CSR_SEPC > - REG_S t0, (KVM_ARCH_GUEST_SEPC)(a0) > - > - /* Restore Host STVEC */ > - REG_L t1, (KVM_ARCH_HOST_STVEC)(a0) > - csrw CSR_STVEC, t1 > > /* Save Guest A0 and Restore Host SSCRATCH */ > - REG_L t2, (KVM_ARCH_HOST_SSCRATCH)(a0) > csrrw t2, CSR_SSCRATCH, t2 > - REG_S t2, (KVM_ARCH_GUEST_A0)(a0) > + > + /* Restore Host STVEC */ > + csrw CSR_STVEC, t1 > > /* Save Guest and Restore Host SCOUNTEREN */ > - REG_L t3, (KVM_ARCH_HOST_SCOUNTEREN)(a0) > csrrw t3, CSR_SCOUNTEREN, t3 > - REG_S t3, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) > > /* Save Guest and Restore Host HSTATUS */ > - REG_L t4, (KVM_ARCH_HOST_HSTATUS)(a0) > csrrw t4, CSR_HSTATUS, t4 > - REG_S t4, (KVM_ARCH_GUEST_HSTATUS)(a0) > > /* Save Guest and Restore Host SSTATUS */ > - REG_L t5, (KVM_ARCH_HOST_SSTATUS)(a0) > csrrw t5, CSR_SSTATUS, t5 > - REG_S t5, (KVM_ARCH_GUEST_SSTATUS)(a0) > Add a single line comment as follows: /* Store Guest CSR values */ > + REG_S t0, (KVM_ARCH_GUEST_SEPC)(a0) > + REG_S t2, (KVM_ARCH_GUEST_A0)(a0) > + REG_S t3, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) > + REG_S t4, (KVM_ARCH_GUEST_HSTATUS)(a0) > + REG_S t5, (KVM_ARCH_GUEST_SSTATUS)(a0) Add new line here > /* Restore Host GPRs (except A0 and T0-T6) */ > REG_L ra, (KVM_ARCH_HOST_RA)(a0) > REG_L sp, (KVM_ARCH_HOST_SP)(a0) > -- > 2.17.1 > Regards, Anup
On Tue, Feb 22, 2022 at 2:43 PM Anup Patel <anup@brainfault.org> wrote: > > On Thu, Feb 17, 2022 at 4:13 PM Vincent Chen <vincent.chen@sifive.com> wrote: > > > > I think the patch subject should be: > RISC-V: KVM: Refine __kvm_riscv_switch_to() implementation > > > Kernel uses __kvm_riscv_switch_to() and __kvm_riscv_switch_from() to switch > > s/__kvm_riscv_switch_from/__kvm_switch_return/ > > > the context of host kernel and guest kernel. Several CSRs belonging to the > > context will be read and written during the context switch. To ensure > > atomic read-modify-write control of CSR and ordering of CSR accesses, some > > hardware blocks or flushes the pipeline when writing a CSR. In this > > s/hardware blocks or flushes/hardware blocks flush/ > > > circumstance, grouping CSR executions together as much as possible can > > reduce the performance impact of the pipeline. Therefore, this commit > > reorders the CSR instructions to enhance the context switch performance.. > > I am mostly fine with this patch except a few nits below. > Hi Anup, Thank you for your suggestions. I will modify them in the v2 patch. Best regards, Vincent > > > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > > Suggested-by: Hsinyi Lee <hsinyi.lee@sifive.com> > > Suggested-by: Fu-Ching Yang <fu-ching.yang@sifive.com> > > --- > > arch/riscv/kvm/vcpu_switch.S | 55 ++++++++++++++++++------------------ > > 1 file changed, 28 insertions(+), 27 deletions(-) > > > > diff --git a/arch/riscv/kvm/vcpu_switch.S b/arch/riscv/kvm/vcpu_switch.S > > index 029a28a195c6..75cf21e5ed92 100644 > > --- a/arch/riscv/kvm/vcpu_switch.S > > +++ b/arch/riscv/kvm/vcpu_switch.S > > @@ -40,34 +40,35 @@ ENTRY(__kvm_riscv_switch_to) > > REG_S s9, (KVM_ARCH_HOST_S9)(a0) > > REG_S s10, (KVM_ARCH_HOST_S10)(a0) > > REG_S s11, (KVM_ARCH_HOST_S11)(a0) > > Add a new line here. > > Also add a single line comment as follows: > /* Load Guest CSR values */ > > > + REG_L t0, (KVM_ARCH_GUEST_SSTATUS)(a0) > > + REG_L t1, (KVM_ARCH_GUEST_HSTATUS)(a0) > > + REG_L t2, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) > > + la t4, __kvm_switch_return > > + REG_L t5, (KVM_ARCH_GUEST_SEPC)(a0) > > > > /* Save Host and Restore Guest SSTATUS */ > > - REG_L t0, (KVM_ARCH_GUEST_SSTATUS)(a0) > > csrrw t0, CSR_SSTATUS, t0 > > - REG_S t0, (KVM_ARCH_HOST_SSTATUS)(a0) > > > > /* Save Host and Restore Guest HSTATUS */ > > - REG_L t1, (KVM_ARCH_GUEST_HSTATUS)(a0) > > csrrw t1, CSR_HSTATUS, t1 > > - REG_S t1, (KVM_ARCH_HOST_HSTATUS)(a0) > > > > /* Save Host and Restore Guest SCOUNTEREN */ > > - REG_L t2, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) > > csrrw t2, CSR_SCOUNTEREN, t2 > > - REG_S t2, (KVM_ARCH_HOST_SCOUNTEREN)(a0) > > - > > - /* Save Host SSCRATCH and change it to struct kvm_vcpu_arch pointer */ > > - csrrw t3, CSR_SSCRATCH, a0 > > - REG_S t3, (KVM_ARCH_HOST_SSCRATCH)(a0) > > > > /* Save Host STVEC and change it to return path */ > > - la t4, __kvm_switch_return > > csrrw t4, CSR_STVEC, t4 > > - REG_S t4, (KVM_ARCH_HOST_STVEC)(a0) > > + > > + /* Save Host SSCRATCH and change it to struct kvm_vcpu_arch pointer */ > > + csrrw t3, CSR_SSCRATCH, a0 > > > > /* Restore Guest SEPC */ > > - REG_L t0, (KVM_ARCH_GUEST_SEPC)(a0) > > - csrw CSR_SEPC, t0 > > + csrw CSR_SEPC, t5 > > + > > Add a single line comment as follows: > /* Store Host CSR values */ > > > + REG_S t0, (KVM_ARCH_HOST_SSTATUS)(a0) > > + REG_S t1, (KVM_ARCH_HOST_HSTATUS)(a0) > > + REG_S t2, (KVM_ARCH_HOST_SCOUNTEREN)(a0) > > + REG_S t3, (KVM_ARCH_HOST_SSCRATCH)(a0) > > + REG_S t4, (KVM_ARCH_HOST_STVEC)(a0) > > > > /* Restore Guest GPRs (except A0) */ > > REG_L ra, (KVM_ARCH_GUEST_RA)(a0) > > @@ -144,35 +145,35 @@ __kvm_switch_return: > > REG_S t4, (KVM_ARCH_GUEST_T4)(a0) > > REG_S t5, (KVM_ARCH_GUEST_T5)(a0) > > REG_S t6, (KVM_ARCH_GUEST_T6)(a0) > > Add a new line here. > > Also add a single line comment as follows: > /* Load Host CSR values */ > > > + REG_L t1, (KVM_ARCH_HOST_STVEC)(a0) > > + REG_L t2, (KVM_ARCH_HOST_SSCRATCH)(a0) > > + REG_L t3, (KVM_ARCH_HOST_SCOUNTEREN)(a0) > > + REG_L t4, (KVM_ARCH_HOST_HSTATUS)(a0) > > + REG_L t5, (KVM_ARCH_HOST_SSTATUS)(a0) > > > > /* Save Guest SEPC */ > > csrr t0, CSR_SEPC > > - REG_S t0, (KVM_ARCH_GUEST_SEPC)(a0) > > - > > - /* Restore Host STVEC */ > > - REG_L t1, (KVM_ARCH_HOST_STVEC)(a0) > > - csrw CSR_STVEC, t1 > > > > /* Save Guest A0 and Restore Host SSCRATCH */ > > - REG_L t2, (KVM_ARCH_HOST_SSCRATCH)(a0) > > csrrw t2, CSR_SSCRATCH, t2 > > - REG_S t2, (KVM_ARCH_GUEST_A0)(a0) > > + > > + /* Restore Host STVEC */ > > + csrw CSR_STVEC, t1 > > > > /* Save Guest and Restore Host SCOUNTEREN */ > > - REG_L t3, (KVM_ARCH_HOST_SCOUNTEREN)(a0) > > csrrw t3, CSR_SCOUNTEREN, t3 > > - REG_S t3, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) > > > > /* Save Guest and Restore Host HSTATUS */ > > - REG_L t4, (KVM_ARCH_HOST_HSTATUS)(a0) > > csrrw t4, CSR_HSTATUS, t4 > > - REG_S t4, (KVM_ARCH_GUEST_HSTATUS)(a0) > > > > /* Save Guest and Restore Host SSTATUS */ > > - REG_L t5, (KVM_ARCH_HOST_SSTATUS)(a0) > > csrrw t5, CSR_SSTATUS, t5 > > - REG_S t5, (KVM_ARCH_GUEST_SSTATUS)(a0) > > > > Add a single line comment as follows: > /* Store Guest CSR values */ > > > + REG_S t0, (KVM_ARCH_GUEST_SEPC)(a0) > > + REG_S t2, (KVM_ARCH_GUEST_A0)(a0) > > + REG_S t3, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) > > + REG_S t4, (KVM_ARCH_GUEST_HSTATUS)(a0) > > + REG_S t5, (KVM_ARCH_GUEST_SSTATUS)(a0) > > Add new line here > > > /* Restore Host GPRs (except A0 and T0-T6) */ > > REG_L ra, (KVM_ARCH_HOST_RA)(a0) > > REG_L sp, (KVM_ARCH_HOST_SP)(a0) > > -- > > 2.17.1 > > > > Regards, > Anup
diff --git a/arch/riscv/kvm/vcpu_switch.S b/arch/riscv/kvm/vcpu_switch.S index 029a28a195c6..75cf21e5ed92 100644 --- a/arch/riscv/kvm/vcpu_switch.S +++ b/arch/riscv/kvm/vcpu_switch.S @@ -40,34 +40,35 @@ ENTRY(__kvm_riscv_switch_to) REG_S s9, (KVM_ARCH_HOST_S9)(a0) REG_S s10, (KVM_ARCH_HOST_S10)(a0) REG_S s11, (KVM_ARCH_HOST_S11)(a0) + REG_L t0, (KVM_ARCH_GUEST_SSTATUS)(a0) + REG_L t1, (KVM_ARCH_GUEST_HSTATUS)(a0) + REG_L t2, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) + la t4, __kvm_switch_return + REG_L t5, (KVM_ARCH_GUEST_SEPC)(a0) /* Save Host and Restore Guest SSTATUS */ - REG_L t0, (KVM_ARCH_GUEST_SSTATUS)(a0) csrrw t0, CSR_SSTATUS, t0 - REG_S t0, (KVM_ARCH_HOST_SSTATUS)(a0) /* Save Host and Restore Guest HSTATUS */ - REG_L t1, (KVM_ARCH_GUEST_HSTATUS)(a0) csrrw t1, CSR_HSTATUS, t1 - REG_S t1, (KVM_ARCH_HOST_HSTATUS)(a0) /* Save Host and Restore Guest SCOUNTEREN */ - REG_L t2, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) csrrw t2, CSR_SCOUNTEREN, t2 - REG_S t2, (KVM_ARCH_HOST_SCOUNTEREN)(a0) - - /* Save Host SSCRATCH and change it to struct kvm_vcpu_arch pointer */ - csrrw t3, CSR_SSCRATCH, a0 - REG_S t3, (KVM_ARCH_HOST_SSCRATCH)(a0) /* Save Host STVEC and change it to return path */ - la t4, __kvm_switch_return csrrw t4, CSR_STVEC, t4 - REG_S t4, (KVM_ARCH_HOST_STVEC)(a0) + + /* Save Host SSCRATCH and change it to struct kvm_vcpu_arch pointer */ + csrrw t3, CSR_SSCRATCH, a0 /* Restore Guest SEPC */ - REG_L t0, (KVM_ARCH_GUEST_SEPC)(a0) - csrw CSR_SEPC, t0 + csrw CSR_SEPC, t5 + + REG_S t0, (KVM_ARCH_HOST_SSTATUS)(a0) + REG_S t1, (KVM_ARCH_HOST_HSTATUS)(a0) + REG_S t2, (KVM_ARCH_HOST_SCOUNTEREN)(a0) + REG_S t3, (KVM_ARCH_HOST_SSCRATCH)(a0) + REG_S t4, (KVM_ARCH_HOST_STVEC)(a0) /* Restore Guest GPRs (except A0) */ REG_L ra, (KVM_ARCH_GUEST_RA)(a0) @@ -144,35 +145,35 @@ __kvm_switch_return: REG_S t4, (KVM_ARCH_GUEST_T4)(a0) REG_S t5, (KVM_ARCH_GUEST_T5)(a0) REG_S t6, (KVM_ARCH_GUEST_T6)(a0) + REG_L t1, (KVM_ARCH_HOST_STVEC)(a0) + REG_L t2, (KVM_ARCH_HOST_SSCRATCH)(a0) + REG_L t3, (KVM_ARCH_HOST_SCOUNTEREN)(a0) + REG_L t4, (KVM_ARCH_HOST_HSTATUS)(a0) + REG_L t5, (KVM_ARCH_HOST_SSTATUS)(a0) /* Save Guest SEPC */ csrr t0, CSR_SEPC - REG_S t0, (KVM_ARCH_GUEST_SEPC)(a0) - - /* Restore Host STVEC */ - REG_L t1, (KVM_ARCH_HOST_STVEC)(a0) - csrw CSR_STVEC, t1 /* Save Guest A0 and Restore Host SSCRATCH */ - REG_L t2, (KVM_ARCH_HOST_SSCRATCH)(a0) csrrw t2, CSR_SSCRATCH, t2 - REG_S t2, (KVM_ARCH_GUEST_A0)(a0) + + /* Restore Host STVEC */ + csrw CSR_STVEC, t1 /* Save Guest and Restore Host SCOUNTEREN */ - REG_L t3, (KVM_ARCH_HOST_SCOUNTEREN)(a0) csrrw t3, CSR_SCOUNTEREN, t3 - REG_S t3, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) /* Save Guest and Restore Host HSTATUS */ - REG_L t4, (KVM_ARCH_HOST_HSTATUS)(a0) csrrw t4, CSR_HSTATUS, t4 - REG_S t4, (KVM_ARCH_GUEST_HSTATUS)(a0) /* Save Guest and Restore Host SSTATUS */ - REG_L t5, (KVM_ARCH_HOST_SSTATUS)(a0) csrrw t5, CSR_SSTATUS, t5 - REG_S t5, (KVM_ARCH_GUEST_SSTATUS)(a0) + REG_S t0, (KVM_ARCH_GUEST_SEPC)(a0) + REG_S t2, (KVM_ARCH_GUEST_A0)(a0) + REG_S t3, (KVM_ARCH_GUEST_SCOUNTEREN)(a0) + REG_S t4, (KVM_ARCH_GUEST_HSTATUS)(a0) + REG_S t5, (KVM_ARCH_GUEST_SSTATUS)(a0) /* Restore Host GPRs (except A0 and T0-T6) */ REG_L ra, (KVM_ARCH_HOST_RA)(a0) REG_L sp, (KVM_ARCH_HOST_SP)(a0)
Kernel uses __kvm_riscv_switch_to() and __kvm_riscv_switch_from() to switch the context of host kernel and guest kernel. Several CSRs belonging to the context will be read and written during the context switch. To ensure atomic read-modify-write control of CSR and ordering of CSR accesses, some hardware blocks or flushes the pipeline when writing a CSR. In this circumstance, grouping CSR executions together as much as possible can reduce the performance impact of the pipeline. Therefore, this commit reorders the CSR instructions to enhance the context switch performance.. Signed-off-by: Vincent Chen <vincent.chen@sifive.com> Suggested-by: Hsinyi Lee <hsinyi.lee@sifive.com> Suggested-by: Fu-Ching Yang <fu-ching.yang@sifive.com> --- arch/riscv/kvm/vcpu_switch.S | 55 ++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 27 deletions(-)