diff mbox series

[v2,14/15] kvm: ioapic: Delay update IOAPIC EOI for RTC

Message ID 1565886293-115836-15-git-send-email-suravee.suthikulpanit@amd.com (mailing list archive)
State New, archived
Headers show
Series kvm: x86: Support AMD SVM AVIC w/ in-kernel irqchip mode | expand

Commit Message

Suthikulpanit, Suravee Aug. 15, 2019, 4:25 p.m. UTC
In-kernel IOAPIC does not update RTC pending EOI info with AMD SVM /w AVIC
when interrupt is delivered as edge-triggered since AMD processors
cannot exit on EOI for these interrupts.

Add code to also check LAPIC pending EOI before injecting any new RTC
interrupts on AMD SVM when AVIC is activated.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/kvm/ioapic.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

Comments

Alexander Graf Aug. 19, 2019, 11 a.m. UTC | #1
On 15.08.19 18:25, Suthikulpanit, Suravee wrote:
> In-kernel IOAPIC does not update RTC pending EOI info with AMD SVM /w AVIC
> when interrupt is delivered as edge-triggered since AMD processors
> cannot exit on EOI for these interrupts.
> 
> Add code to also check LAPIC pending EOI before injecting any new RTC
> interrupts on AMD SVM when AVIC is activated.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>   arch/x86/kvm/ioapic.c | 36 ++++++++++++++++++++++++++++++++----
>   1 file changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index 1add1bc..45e7bb0 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c
> @@ -39,6 +39,7 @@
>   #include <asm/processor.h>
>   #include <asm/page.h>
>   #include <asm/current.h>
> +#include <asm/virtext.h>
>   #include <trace/events/kvm.h>
>   
>   #include "ioapic.h"
> @@ -173,6 +174,7 @@ static bool rtc_irq_check_coalesced(struct kvm_ioapic *ioapic)
>   	return false;
>   }
>   
> +#define APIC_DEST_NOSHORT		0x0
>   static int ioapic_set_irq(struct kvm_ioapic *ioapic, unsigned int irq,
>   		int irq_level, bool line_status)
>   {
> @@ -201,10 +203,36 @@ static int ioapic_set_irq(struct kvm_ioapic *ioapic, unsigned int irq,
>   	 * interrupts lead to time drift in Windows guests.  So we track
>   	 * EOI manually for the RTC interrupt.
>   	 */
> -	if (irq == RTC_GSI && line_status &&
> -		rtc_irq_check_coalesced(ioapic)) {
> -		ret = 0;
> -		goto out;
> +	if (irq == RTC_GSI && line_status) {
> +		struct kvm *kvm = ioapic->kvm;
> +		union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
> +
> +		/*
> +		 * Since, AMD SVM AVIC accelerates write access to APIC EOI
> +		 * register for edge-trigger interrupts, IOAPIC will not be
> +		 * able to receive the EOI. In this case, we do lazy update
> +		 * of the pending EOI when trying to set IOAPIC irq for RTC.
> +		 */
> +		if (cpu_has_svm(NULL) &&
> +		    (kvm->arch.apicv_state == APICV_ACTIVATED) &&
> +		    (entry->fields.trig_mode == IOAPIC_EDGE_TRIG)) {
> +			int i;
> +			struct kvm_vcpu *vcpu;
> +
> +			kvm_for_each_vcpu(i, vcpu, kvm)
> +				if (kvm_apic_match_dest(vcpu, NULL,
> +							KVM_APIC_DEST_NOSHORT,
> +							entry->fields.dest_id,
> +							entry->fields.dest_mode)) {
> +					__rtc_irq_eoi_tracking_restore_one(vcpu);

I don't understand why this works. This code just means we're injecting 
an EOI on the first CPU that has the vector mapped, right when we're 
setting it to trigger, no?


Alex


> +					break;
> +				}
> +		}
> +
> +		if (rtc_irq_check_coalesced(ioapic)) {
> +			ret = 0;
> +			goto out;
> +		}
>   	}
>   
>   	old_irr = ioapic->irr;
>
Suthikulpanit, Suravee Sept. 4, 2019, 2:06 a.m. UTC | #2
Alex,

On 8/19/19 6:00 AM, Alexander Graf wrote:
> 
> 
> On 15.08.19 18:25, Suthikulpanit, Suravee wrote:
>> In-kernel IOAPIC does not update RTC pending EOI info with AMD SVM /w 
>> AVIC
>> when interrupt is delivered as edge-triggered since AMD processors
>> cannot exit on EOI for these interrupts.
>>
>> Add code to also check LAPIC pending EOI before injecting any new RTC
>> interrupts on AMD SVM when AVIC is activated.
>>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> ---
>>   arch/x86/kvm/ioapic.c | 36 ++++++++++++++++++++++++++++++++----
>>   1 file changed, 32 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
>> index 1add1bc..45e7bb0 100644
>> --- a/arch/x86/kvm/ioapic.c
>> +++ b/arch/x86/kvm/ioapic.c
>> @@ -39,6 +39,7 @@
>>   #include <asm/processor.h>
>>   #include <asm/page.h>
>>   #include <asm/current.h>
>> +#include <asm/virtext.h>
>>   #include <trace/events/kvm.h>
>>   #include "ioapic.h"
>> @@ -173,6 +174,7 @@ static bool rtc_irq_check_coalesced(struct 
>> kvm_ioapic *ioapic)
>>       return false;
>>   }
>> +#define APIC_DEST_NOSHORT        0x0
>>   static int ioapic_set_irq(struct kvm_ioapic *ioapic, unsigned int irq,
>>           int irq_level, bool line_status)
>>   {
>> @@ -201,10 +203,36 @@ static int ioapic_set_irq(struct kvm_ioapic 
>> *ioapic, unsigned int irq,
>>        * interrupts lead to time drift in Windows guests.  So we track
>>        * EOI manually for the RTC interrupt.
>>        */
>> -    if (irq == RTC_GSI && line_status &&
>> -        rtc_irq_check_coalesced(ioapic)) {
>> -        ret = 0;
>> -        goto out;
>> +    if (irq == RTC_GSI && line_status) {
>> +        struct kvm *kvm = ioapic->kvm;
>> +        union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
>> +
>> +        /*
>> +         * Since, AMD SVM AVIC accelerates write access to APIC EOI
>> +         * register for edge-trigger interrupts, IOAPIC will not be
>> +         * able to receive the EOI. In this case, we do lazy update
>> +         * of the pending EOI when trying to set IOAPIC irq for RTC.
>> +         */
>> +        if (cpu_has_svm(NULL) &&
>> +            (kvm->arch.apicv_state == APICV_ACTIVATED) &&
>> +            (entry->fields.trig_mode == IOAPIC_EDGE_TRIG)) {
>> +            int i;
>> +            struct kvm_vcpu *vcpu;
>> +
>> +            kvm_for_each_vcpu(i, vcpu, kvm)
>> +                if (kvm_apic_match_dest(vcpu, NULL,
>> +                            KVM_APIC_DEST_NOSHORT,
>> +                            entry->fields.dest_id,
>> +                            entry->fields.dest_mode)) {
>> +                    __rtc_irq_eoi_tracking_restore_one(vcpu);
> 
> I don't understand why this works. This code just means we're injecting 
> an EOI on the first CPU that has the vector mapped, right when we're 
> setting it to trigger, no?

Actually, this is similar to the approach in patch 12/15, where we check 
if there is any pending EOI for the RTC_GSI on the destination vcpu. 
Otherwise, the __rtc_irq_eoi_tracking_restore_one() should clear IOAPIC 
pending EOI for RTC.

Suravee
diff mbox series

Patch

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 1add1bc..45e7bb0 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -39,6 +39,7 @@ 
 #include <asm/processor.h>
 #include <asm/page.h>
 #include <asm/current.h>
+#include <asm/virtext.h>
 #include <trace/events/kvm.h>
 
 #include "ioapic.h"
@@ -173,6 +174,7 @@  static bool rtc_irq_check_coalesced(struct kvm_ioapic *ioapic)
 	return false;
 }
 
+#define APIC_DEST_NOSHORT		0x0
 static int ioapic_set_irq(struct kvm_ioapic *ioapic, unsigned int irq,
 		int irq_level, bool line_status)
 {
@@ -201,10 +203,36 @@  static int ioapic_set_irq(struct kvm_ioapic *ioapic, unsigned int irq,
 	 * interrupts lead to time drift in Windows guests.  So we track
 	 * EOI manually for the RTC interrupt.
 	 */
-	if (irq == RTC_GSI && line_status &&
-		rtc_irq_check_coalesced(ioapic)) {
-		ret = 0;
-		goto out;
+	if (irq == RTC_GSI && line_status) {
+		struct kvm *kvm = ioapic->kvm;
+		union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
+
+		/*
+		 * Since, AMD SVM AVIC accelerates write access to APIC EOI
+		 * register for edge-trigger interrupts, IOAPIC will not be
+		 * able to receive the EOI. In this case, we do lazy update
+		 * of the pending EOI when trying to set IOAPIC irq for RTC.
+		 */
+		if (cpu_has_svm(NULL) &&
+		    (kvm->arch.apicv_state == APICV_ACTIVATED) &&
+		    (entry->fields.trig_mode == IOAPIC_EDGE_TRIG)) {
+			int i;
+			struct kvm_vcpu *vcpu;
+
+			kvm_for_each_vcpu(i, vcpu, kvm)
+				if (kvm_apic_match_dest(vcpu, NULL,
+							KVM_APIC_DEST_NOSHORT,
+							entry->fields.dest_id,
+							entry->fields.dest_mode)) {
+					__rtc_irq_eoi_tracking_restore_one(vcpu);
+					break;
+				}
+		}
+
+		if (rtc_irq_check_coalesced(ioapic)) {
+			ret = 0;
+			goto out;
+		}
 	}
 
 	old_irr = ioapic->irr;