On 02/11/2014 10:54, Nadav Amit wrote:
> Currently, the APIC logical map does not consider VCPUs whose local-apic is
> software-disabled. However, NMIs, INIT, etc. should still be delivered to such
> VCPUs. Therefore, the APIC mode should first be determined, and then the map,
> considering all VCPUs should be constructed.
>
> To address this issue, first find the APIC mode, and only then construct the
> logical map.
>
> Signed-off-by: Nadav Amit <namit@xxxxxxxxxxxxxxxxx>
> ---
> arch/x86/kvm/lapic.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 66dd173..2a838af 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -156,8 +156,6 @@ static void recalculate_apic_map(struct kvm *kvm)
>
> kvm_for_each_vcpu(i, vcpu, kvm) {
> struct kvm_lapic *apic = vcpu->arch.apic;
> - u16 cid, lid;
> - u32 ldr;
>
> if (!kvm_apic_present(vcpu))
> continue;
> @@ -175,13 +173,22 @@ static void recalculate_apic_map(struct kvm *kvm)
> new->cid_mask = (1 << KVM_X2APIC_CID_BITS) - 1;
> new->lid_mask = 0xffff;
> new->broadcast = X2APIC_BROADCAST;
> - } else if (kvm_apic_sw_enabled(apic) &&
> - !new->cid_mask /* flat mode */ &&
> - kvm_apic_get_reg(apic, APIC_DFR) == APIC_DFR_CLUSTER) {
> - new->cid_shift = 4;
> - new->cid_mask = 0xf;
> - new->lid_mask = 0xf;
> + break;
> + } else if (kvm_apic_sw_enabled(apic)) {
> + if (kvm_apic_get_reg(apic, APIC_DFR) ==
> + APIC_DFR_CLUSTER) {
> + new->cid_shift = 4;
> + new->cid_mask = 0xf;
> + new->lid_mask = 0xf;
> + }
> + break;
> }
We need to take into account DFR even if all APICs are software disabled,
in case it will be used to send NMIs.
What about this on top of your patch?
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index c98b44d0ffe6..d8b11f47f45e 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -160,29 +160,34 @@ static void recalculate_apic_map(struct kvm *kvm)
if (!kvm_apic_present(vcpu))
continue;
- /*
- * All APICs have to be configured in the same mode by an OS.
- * We take advatage of this while building logical id loockup
- * table. After reset APICs are in xapic/flat mode, so if we
- * find apic with different setting we assume this is the mode
- * OS wants all apics to be in; build lookup table accordingly.
- */
if (apic_x2apic_mode(apic)) {
new->ldr_bits = 32;
new->cid_shift = 16;
new->cid_mask = (1 << KVM_X2APIC_CID_BITS) - 1;
new->lid_mask = 0xffff;
new->broadcast = X2APIC_BROADCAST;
- break;
- } else if (kvm_apic_sw_enabled(apic)) {
+ } else if (kvm_apic_hw_enabled(apic)) {
if (kvm_apic_get_reg(apic, APIC_DFR) ==
APIC_DFR_CLUSTER) {
new->cid_shift = 4;
new->cid_mask = 0xf;
new->lid_mask = 0xf;
+ } else {
+ new->cid_shift = 8;
+ new->cid_mask = 0;
+ new->lid_mask = 0xff;
}
- break;
}
+
+ /*
+ * All APICs have to be configured in the same mode by an OS.
+ * We take advatage of this while building logical id loockup
+ * table. After reset APICs are in software disabled mode, so if
+ * we find apic with different setting we assume this is the mode
+ * OS wants all apics to be in; build lookup table accordingly.
+ */
+ if (kvm_apic_sw_enabled(apic))
+ break;
}
kvm_for_each_vcpu(i, vcpu, kvm) {
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
