> On Nov 5, 2014, at 14:30, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>
>
>
> 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.
[snip]
>
> 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) {
>
I didn’t encounter the problem you try to fix, so I can’t say for sure, yet…
If I understand the SDM correctly, in such scenario (all APICs are software disabled) the mode is left as the default - flat mode (see section 10.6.2.2 "Logical Destination Mode”):
"All processors that have their APIC software enabled (using the spurious vector enable/disable bit) must have their DFRs (Destination Format Registers) programmed identically. The default mode for DFR is flat mode.”
So I think the previous behaviour (before the additional changes) is the correct one.
I might be able to confirm it, but anyhow only in a couple of weeks.
Nadav--
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
