Re: [RFC PATCH 2/4] KVM: x86: Add KVM exit for IOAPIC EOIs

[Paolo Bonzini <pbonzini@redhat.com>]

On 13/05/2015 03:47, Steve Rutherford wrote:
> Adds KVM_EXIT_IOAPIC_EOI which passes the interrupt vector up to
> userspace.
> 
> Uses a per VCPU exit bitmap to decide whether or not the IOAPIC needs
> to be informed (which is identical to the EOI_EXIT_BITMAP field used
> by modern x86 processors, but can also be used to elide kvm IOAPIC EOI
> exits on older processors).
> 
> [Note: A prototype using ResampleFDs found that decoupling the EOI
> from the VCPU's thread made it possible for the VCPU to not see a
> recent EOI after reentering the guest. This does not match real
> hardware.]

Can you explain this better (and perhaps add a testcase to ioapic.c)?

Paolo

> Compile tested for Intel x86.
> 
> Signed-off-by: Steve Rutherford <srutherford@google.com>
> ---
>  Documentation/virtual/kvm/api.txt | 10 ++++++++++
>  arch/x86/include/asm/kvm_host.h   |  3 +++
>  arch/x86/kvm/lapic.c              |  9 +++++++++
>  arch/x86/kvm/x86.c                | 11 +++++++++++
>  include/linux/kvm_host.h          |  1 +
>  include/uapi/linux/kvm.h          |  5 +++++
>  6 files changed, 39 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 0744b4e..dd92996 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -3285,6 +3285,16 @@ Valid values for 'type' are:
>  	 */
>  	__u64 kvm_valid_regs;
>  	__u64 kvm_dirty_regs;
> +
> +	/* KVM_EXIT_IOAPIC_EOI */
> +        struct {
> +	       __u8 vector;
> +        } eoi;
> +
> +Indicates that an eoi of a level triggered IOAPIC interrupt on vector has
> +occurred, which should be handled by the userspace IOAPIC. Triggers when
> +the Irqchip has been split between userspace and the kernel.
> +
>  	union {
>  		struct kvm_sync_regs regs;
>  		char padding[1024];
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 3ddc134..b1978f1 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -539,6 +539,9 @@ struct kvm_vcpu_arch {
>  	struct {
>  		bool pv_unhalted;
>  	} pv;
> +
> +	u64 eoi_exit_bitmaps[4];
> +	int pending_ioapic_eoi;
>  };
>  
>  struct kvm_lpage_info {
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index bc392a6..42fada6f 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -860,6 +860,15 @@ int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2)
>  
>  static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector)
>  {
> +	if (irqchip_split(apic->vcpu->kvm)) {
> +		if (test_bit(vector,
> +			     (void *) apic->vcpu->arch.eoi_exit_bitmaps)) {
> +			apic->vcpu->arch.pending_ioapic_eoi = vector;
> +			kvm_make_request(KVM_REQ_IOAPIC_EOI_EXIT, apic->vcpu);
> +		}
> +		return;
> +	}
> +
>  	if (kvm_ioapic_handles_vector(apic->vcpu->kvm, vector)) {
>  		int trigger_mode;
>  		if (apic_test_vector(vector, apic->regs + APIC_TMR))
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 7505b39..cc27c35 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6324,6 +6324,17 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>  			kvm_handle_pmu_event(vcpu);
>  		if (kvm_check_request(KVM_REQ_PMI, vcpu))
>  			kvm_deliver_pmi(vcpu);
> +		if (kvm_check_request(KVM_REQ_IOAPIC_EOI_EXIT, vcpu)) {
> +			BUG_ON(vcpu->arch.pending_ioapic_eoi > 255);
> +			if (test_bit(vcpu->arch.pending_ioapic_eoi,
> +				     (void *) vcpu->arch.eoi_exit_bitmaps)) {
> +				vcpu->run->exit_reason = KVM_EXIT_IOAPIC_EOI;
> +				vcpu->run->eoi.vector =
> +						vcpu->arch.pending_ioapic_eoi;
> +				r = 0;
> +				goto out;
> +			}
> +		}
>  		if (kvm_check_request(KVM_REQ_SCAN_IOAPIC, vcpu))
>  			vcpu_scan_ioapic(vcpu);
>  		if (kvm_check_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu))
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 277b7a1..cef20ad 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page)
>  #define KVM_REQ_ENABLE_IBS        23
>  #define KVM_REQ_DISABLE_IBS       24
>  #define KVM_REQ_APIC_PAGE_RELOAD  25
> +#define KVM_REQ_IOAPIC_EOI_EXIT   26
>  
>  #define KVM_USERSPACE_IRQ_SOURCE_ID		0
>  #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID	1
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 7d06dc4..2305948 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -183,6 +183,7 @@ struct kvm_s390_skeys {
>  #define KVM_EXIT_EPR              23
>  #define KVM_EXIT_SYSTEM_EVENT     24
>  #define KVM_EXIT_S390_STSI        25
> +#define KVM_EXIT_IOAPIC_EOI       26
>  
>  /* For KVM_EXIT_INTERNAL_ERROR */
>  /* Emulate instruction failed. */
> @@ -329,6 +330,10 @@ struct kvm_run {
>  			__u8 sel1;
>  			__u16 sel2;
>  		} s390_stsi;
> +		/* KVM_EXIT_IOAPIC_EOI */
> +		struct {
> +			__u8 vector;
> +		} eoi;
>  		/* Fix the size of the union. */
>  		char padding[256];
>  	};
>