diff mbox series

LoongArch: KVM: Delay secondary mmu tlb flush before guest entry

Message ID 20240615025930.1408266-1-maobibo@loongson.cn (mailing list archive)
State New, archived
Headers show
Series LoongArch: KVM: Delay secondary mmu tlb flush before guest entry | expand

Commit Message

Bibo Mao June 15, 2024, 2:59 a.m. UTC 
If there is page fault for secondary mmu, the tlb entry need be flushed
with index fault gpa address and VMID. VMID is stored at register
CSR_GSTAT and will be recalculated before guest entry.

Currently VMID CSR_GSTAT is not saved and restored during vcpu context
switch, it is recalculated before guest entry. So CSR_GSTAT is in effect
when vcpu runs in guest mode, however it is not in effected if vcpu exits
to host mode.

Function kvm_flush_tlb_gpa() should be called with its real VMID, here
move it to guest entry. Also arch specific request id
KVM_REQ_TLB_FLUSH_GPA is added to flush tlb. And it can be optimized if
VMID is updated, since all guest tlb entries will be invalid if VMID is
updated, it is not necessary to flush tlb in this condition.

Fixes: d7f4ed4b2290 ("LoongArch: KVM: Implement virtual machine tlb operations")
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 arch/loongarch/include/asm/kvm_host.h |  2 ++
 arch/loongarch/kvm/main.c             |  1 +
 arch/loongarch/kvm/mmu.c              |  4 ++--
 arch/loongarch/kvm/tlb.c              |  5 +----
 arch/loongarch/kvm/vcpu.c             | 18 ++++++++++++++++++
 5 files changed, 24 insertions(+), 6 deletions(-)


base-commit: d20f6b3d747c36889b7ce75ee369182af3decb6b

Comments

Sean Christopherson June 17, 2024, 2:49 p.m. UTC | #1 
On Sat, Jun 15, 2024, Bibo Mao wrote:
> diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
> index c87b6ea0ec47..98078e01dd55 100644
> --- a/arch/loongarch/include/asm/kvm_host.h
> +++ b/arch/loongarch/include/asm/kvm_host.h
> @@ -30,6 +30,7 @@
>  #define KVM_PRIVATE_MEM_SLOTS		0
>  
>  #define KVM_HALT_POLL_NS_DEFAULT	500000
> +#define KVM_REQ_TLB_FLUSH_GPA		KVM_ARCH_REQ(0)
>  
>  #define KVM_GUESTDBG_SW_BP_MASK		\
>  	(KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP)
> @@ -190,6 +191,7 @@ struct kvm_vcpu_arch {
>  
>  	/* vcpu's vpid */
>  	u64 vpid;
> +	unsigned long flush_gpa;

Side topic, GPAs should really use "gpa_t" instead of "unsigned long", otherwise
32-bit kernels running on CPUs with 64-bit physical addresses will fail miserably
(which may or may not be a problem in practice for LoongArch).

> diff --git a/arch/loongarch/kvm/tlb.c b/arch/loongarch/kvm/tlb.c
> index 02535df6b51f..55f7f3621e38 100644
> --- a/arch/loongarch/kvm/tlb.c
> +++ b/arch/loongarch/kvm/tlb.c
> @@ -21,12 +21,9 @@ void kvm_flush_tlb_all(void)
>  	local_irq_restore(flags);
>  }
>  
> +/* Called with irq disabled */

Rather than add a comment, add:

	lockdep_assert_irqs_disabled()

in the function.

>  void kvm_flush_tlb_gpa(struct kvm_vcpu *vcpu, unsigned long gpa)
>  {
> -	unsigned long flags;
> -
> -	local_irq_save(flags);
>  	gpa &= (PAGE_MASK << 1);
>  	invtlb(INVTLB_GID_ADDR, read_csr_gstat() & CSR_GSTAT_GID, gpa);
> -	local_irq_restore(flags);
>  }
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 9e8030d45129..ae9ae88c11db 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -51,6 +51,16 @@ static int kvm_check_requests(struct kvm_vcpu *vcpu)
>  	return RESUME_GUEST;
>  }
>  
> +/* Check pending request with irq disabled */

Same thing here.
Bibo Mao June 18, 2024, 1:47 a.m. UTC | #2 
Sean,

Thanks for reviewing the patch, we are not familiar with open source 
community, it gives us much helps.

On 2024/6/17 下午10:49, Sean Christopherson wrote:
> On Sat, Jun 15, 2024, Bibo Mao wrote:
>> diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
>> index c87b6ea0ec47..98078e01dd55 100644
>> --- a/arch/loongarch/include/asm/kvm_host.h
>> +++ b/arch/loongarch/include/asm/kvm_host.h
>> @@ -30,6 +30,7 @@
>>   #define KVM_PRIVATE_MEM_SLOTS		0
>>   
>>   #define KVM_HALT_POLL_NS_DEFAULT	500000
>> +#define KVM_REQ_TLB_FLUSH_GPA		KVM_ARCH_REQ(0)
>>   
>>   #define KVM_GUESTDBG_SW_BP_MASK		\
>>   	(KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP)
>> @@ -190,6 +191,7 @@ struct kvm_vcpu_arch {
>>   
>>   	/* vcpu's vpid */
>>   	u64 vpid;
>> +	unsigned long flush_gpa;
> 
> Side topic, GPAs should really use "gpa_t" instead of "unsigned long", otherwise
> 32-bit kernels running on CPUs with 64-bit physical addresses will fail miserably
> (which may or may not be a problem in practice for LoongArch).
Sure, will modify.

> 
>> diff --git a/arch/loongarch/kvm/tlb.c b/arch/loongarch/kvm/tlb.c
>> index 02535df6b51f..55f7f3621e38 100644
>> --- a/arch/loongarch/kvm/tlb.c
>> +++ b/arch/loongarch/kvm/tlb.c
>> @@ -21,12 +21,9 @@ void kvm_flush_tlb_all(void)
>>   	local_irq_restore(flags);
>>   }
>>   
>> +/* Called with irq disabled */
> 
> Rather than add a comment, add:
> 
> 	lockdep_assert_irqs_disabled()
Good point, will modify.

> 
> in the function.
> 
>>   void kvm_flush_tlb_gpa(struct kvm_vcpu *vcpu, unsigned long gpa)
>>   {
>> -	unsigned long flags;
>> -
>> -	local_irq_save(flags);
>>   	gpa &= (PAGE_MASK << 1);
>>   	invtlb(INVTLB_GID_ADDR, read_csr_gstat() & CSR_GSTAT_GID, gpa);
>> -	local_irq_restore(flags);
>>   }
>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>> index 9e8030d45129..ae9ae88c11db 100644
>> --- a/arch/loongarch/kvm/vcpu.c
>> +++ b/arch/loongarch/kvm/vcpu.c
>> @@ -51,6 +51,16 @@ static int kvm_check_requests(struct kvm_vcpu *vcpu)
>>   	return RESUME_GUEST;
>>   }
>>   
>> +/* Check pending request with irq disabled */
> 
> Same thing here.
Will modify in next version.

Regards
Bibo Mao
diff mbox series

Patch

diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
index c87b6ea0ec47..98078e01dd55 100644
--- a/arch/loongarch/include/asm/kvm_host.h
+++ b/arch/loongarch/include/asm/kvm_host.h
@@ -30,6 +30,7 @@ 
 #define KVM_PRIVATE_MEM_SLOTS		0
 
 #define KVM_HALT_POLL_NS_DEFAULT	500000
+#define KVM_REQ_TLB_FLUSH_GPA		KVM_ARCH_REQ(0)
 
 #define KVM_GUESTDBG_SW_BP_MASK		\
 	(KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP)
@@ -190,6 +191,7 @@  struct kvm_vcpu_arch {
 
 	/* vcpu's vpid */
 	u64 vpid;
+	unsigned long flush_gpa;
 
 	/* Frequency of stable timer in Hz */
 	u64 timer_mhz;
diff --git a/arch/loongarch/kvm/main.c b/arch/loongarch/kvm/main.c
index 86a2f2d0cb27..844736b99d38 100644
--- a/arch/loongarch/kvm/main.c
+++ b/arch/loongarch/kvm/main.c
@@ -242,6 +242,7 @@  void kvm_check_vpid(struct kvm_vcpu *vcpu)
 		kvm_update_vpid(vcpu, cpu);
 		trace_kvm_vpid_change(vcpu, vcpu->arch.vpid);
 		vcpu->cpu = cpu;
+		kvm_clear_request(KVM_REQ_TLB_FLUSH_GPA, vcpu);
 	}
 
 	/* Restore GSTAT(0x50).vpid */
diff --git a/arch/loongarch/kvm/mmu.c b/arch/loongarch/kvm/mmu.c
index 98883aa23ab8..9e39d28fec35 100644
--- a/arch/loongarch/kvm/mmu.c
+++ b/arch/loongarch/kvm/mmu.c
@@ -908,8 +908,8 @@  int kvm_handle_mm_fault(struct kvm_vcpu *vcpu, unsigned long gpa, bool write)
 		return ret;
 
 	/* Invalidate this entry in the TLB */
-	kvm_flush_tlb_gpa(vcpu, gpa);
-
+	vcpu->arch.flush_gpa = gpa;
+	kvm_make_request(KVM_REQ_TLB_FLUSH_GPA, vcpu);
 	return 0;
 }
 
diff --git a/arch/loongarch/kvm/tlb.c b/arch/loongarch/kvm/tlb.c
index 02535df6b51f..55f7f3621e38 100644
--- a/arch/loongarch/kvm/tlb.c
+++ b/arch/loongarch/kvm/tlb.c
@@ -21,12 +21,9 @@  void kvm_flush_tlb_all(void)
 	local_irq_restore(flags);
 }
 
+/* Called with irq disabled */
 void kvm_flush_tlb_gpa(struct kvm_vcpu *vcpu, unsigned long gpa)
 {
-	unsigned long flags;
-
-	local_irq_save(flags);
 	gpa &= (PAGE_MASK << 1);
 	invtlb(INVTLB_GID_ADDR, read_csr_gstat() & CSR_GSTAT_GID, gpa);
-	local_irq_restore(flags);
 }
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 9e8030d45129..ae9ae88c11db 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -51,6 +51,16 @@  static int kvm_check_requests(struct kvm_vcpu *vcpu)
 	return RESUME_GUEST;
 }
 
+/* Check pending request with irq disabled */
+static void kvm_late_check_requests(struct kvm_vcpu *vcpu)
+{
+	if (kvm_check_request(KVM_REQ_TLB_FLUSH_GPA, vcpu))
+		if (vcpu->arch.flush_gpa != INVALID_GPA) {
+			kvm_flush_tlb_gpa(vcpu, vcpu->arch.flush_gpa);
+			vcpu->arch.flush_gpa = INVALID_GPA;
+		}
+}
+
 /*
  * Check and handle pending signal and vCPU requests etc
  * Run with irq enabled and preempt enabled
@@ -101,6 +111,13 @@  static int kvm_pre_enter_guest(struct kvm_vcpu *vcpu)
 		/* Make sure the vcpu mode has been written */
 		smp_store_mb(vcpu->mode, IN_GUEST_MODE);
 		kvm_check_vpid(vcpu);
+
+		/*
+		 * Called after function kvm_check_vpid()
+		 * Since it updates csr_gstat used by kvm_flush_tlb_gpa(),
+		 * also it may clear KVM_REQ_TLB_FLUSH_GPA pending bit
+		 */
+		kvm_late_check_requests(vcpu);
 		vcpu->arch.host_eentry = csr_read64(LOONGARCH_CSR_EENTRY);
 		/* Clear KVM_LARCH_SWCSR_LATEST as CSR will change when enter guest */
 		vcpu->arch.aux_inuse &= ~KVM_LARCH_SWCSR_LATEST;
@@ -994,6 +1011,7 @@  int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 	struct loongarch_csrs *csr;
 
 	vcpu->arch.vpid = 0;
+	vcpu->arch.flush_gpa = INVALID_GPA;
 
 	hrtimer_init(&vcpu->arch.swtimer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_PINNED);
 	vcpu->arch.swtimer.function = kvm_swtimer_wakeup;