On 06/11/2014 17:45, Radim Krčmář wrote:
> 2014-11-06 10:34+0100, Paolo Bonzini:
>> On 05/11/2014 21:45, Nadav Amit wrote:
>>> If I understand the SDM correctly, in such scenario (all APICs are
>>> software disabled) the mode is left as the default - flat mode (see
>
> APIC doesn't have any global mode (it is just KVM's simplification), so
> when a message lands on the system bus, it just compares MDA with LDR
> and DFR ...
>
>>> 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.”
>
> I think the "default mode" points to reset state, which is flat DFR;
> and it is mentioned only because of the following sentence
> If you are using cluster mode, DFRs must be programmed before the APIC
> is software enabled.
>
>> That's not what either Bochs or QEMU do, though. (Though in the case of
>> Bochs I cannot find the place where reception of IPIs is prevented for
>> software-disabled APICs, so I'm not sure how much to trust it in this case).
>>
>> I'm not sure why software-disabled APICs could have different DFRs, if
>> the APICs can receive NMI IPIs. I'll ask Intel.
>
> When changing the mode, we can't switch DFR synchronously, so it has to
> happen and NMI may be needed (watchdog?) before APIC configuration.
> Explicit statement might have been a hint to be _extra_ careful when
> using logical destination for INIT, NMI, ... I wonder what they'll say.
>
> Anyway, Paolo's patch seems to be in the right direction, I'd modify it
> a bit though:
>
> LDR=0 isn't addressable in any logical mode, so we can ignore APICs that
> don't set it and decide the mode by the last nonzero one.
> This works in a situation, where one part is configured for cluster and
> the rest is still in reset state.
>
> (It gets harder if we allow nonzero LDRs with different DFR ...
> we'd need to split our logical map to handle it.)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 758f838..6da303e1 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -148,10 +148,6 @@ static void recalculate_apic_map(struct kvm *kvm)
> goto out;
>
> new->ldr_bits = 8;
> - /* flat mode is default */
> - new->cid_shift = 8;
> - new->cid_mask = 0;
> - new->lid_mask = 0xff;
> new->broadcast = APIC_BROADCAST;
>
> kvm_for_each_vcpu(i, vcpu, kvm) {
> @@ -166,7 +162,7 @@ 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_hw_enabled(apic)) {
> + } else if (kvm_apic_get_reg(apic, APIC_LDR)) {
> if (kvm_apic_get_reg(apic, APIC_DFR) ==
> APIC_DFR_CLUSTER) {
> new->cid_shift = 4;
>
I merged this patch and Nadav's.
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
