linux-kernel - Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 12 Feb 2016 16:38:26 +0100
From:	Paolo Bonzini <pbonzini@...hat.com>
To:	Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
	joro@...tes.org, alex.williamson@...hat.com, gleb@...nel.org
Cc:	kvm@...r.kernel.org, linux-kernel@...r.kernel.org, wei@...hat.com,
	sherry.hurwitz@....com
Subject: Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC



On 12/02/2016 14:59, Suravee Suthikulpanit wrote:
> +		 "icrh:icrl=%#010x:%08x, id=%u, index=%u\n",
> +		 __func__, svm->vcpu.cpu, svm->vcpu.vcpu_id,
> +		 icrh, icrl, id, index);
> +
> +	switch (id) {
> +	case AVIC_INCMP_IPI_ERR_INVALID_INT_TYPE:
> +		/*
> +		 * AVIC hardware handles the generation of
> +		 * IPIs when the specified Message Type is Fixed
> +		 * (also known as fixed delivery mode) and
> +		 * the Trigger Mode is edge-triggered. The hardware
> +		 * also supports self and broadcast delivery modes
> +		 * specified via the Destination Shorthand(DSH)
> +		 * field of the ICRL. Logical and physical APIC ID
> +		 * formats are supported. All other IPI types cause
> +		 * a #VMEXIT, which needs to emulated.
> +		 */
> +		kvm_lapic_reg_write(apic, APIC_ICR2, icrh);
> +		kvm_lapic_reg_write(apic, APIC_ICR, icrl);
> +		break;
> +	case AVIC_INCMP_IPI_ERR_TARGET_NOT_RUN:
> +		kvm_lapic_reg_write(apic, APIC_ICR2, icrh);
> +		kvm_lapic_reg_write(apic, APIC_ICR, icrl);

Wouldn't this cause a double injection of the IPI if the following happens:

1) destination 1 is running, so the processor sets IRR and sends a
doorbell message

2) destination 2 is not running, so the processor sets IRR and exits

3) destination 1 processes the interrupt, moving it from IRR to ISR

4) destination 1 sends an EOI

5) the source exits and reinjects the interrupt

6) destination 1 then receives the interrupt again.


Or alternatively:

1) destination 1 is not running, so the processor sets IRR and exits

2) another CPU executes VMRUN for destination 1, so the processor
injects the interrupt

3) destination 1 sends an EOI

4) the source exits and reinjects the interrupt

5) destination 1 then receives the interrupt again.


The handling of races for IsRunning and incomplete IPIs has always been
very confusing to me whenever I read the AVIC specification.  It would
be great if you could clarify this.

Paolo

> +		break;
> +	case AVIC_INCMP_IPI_ERR_INV_TARGET:
> +		pr_err("SVM: %s: Invalid IPI target (icr=%#08x:%08x, idx=%u)\n",
> +		       __func__, icrh, icrl, index);
> +		BUG();
> +		break;
> +	case AVIC_INCMP_IPI_ERR_INV_BK_PAGE:
> +		pr_err("SVM: %s: Invalid bk page (icr=%#08x:%08x, idx=%u)\n",
> +		       __func__, icrh, icrl, index);
> +		BUG();