On Oct 3, 2014, at 3:49 PM, Radim Krčmář <[email protected]> wrote:

> 2014-10-03 00:30+0300, Nadav Amit:
>> KVM does not deliver x2APIC broadcast messages with physical mode.  Intel SDM
>> (10.12.9 ICR Operation in x2APIC Mode) states: "A destination ID value of
>> FFFF_FFFFH is used for broadcast of interrupts in both logical destination 
>> and
>> physical destination modes."
>> 
>> In addition, the local-apic enables cluster mode broadcast. As Intel SDM
>> 10.6.2.2 says: "Broadcast to all local APICs is achieved by setting all
>> destination bits to one." This patch enables cluster mode broadcast.
>> 
>> The fix tries to combine broadcast in different modes through a unified code.
>> 
>> One rare case occurs when the source of IPI has its APIC disabled.  In such
>> case, the source can still issue IPIs, but since the source is not obliged to
>> have the same LAPIC mode as the enabled ones, we cannot rely on it.
>> Since it is a rare case, it is unoptimized and done on the slow-path.
>> 
>> ---
> 
> Thanks!
> 
> Reviewed-by: Radim Krčmář <[email protected]>
> 
>> Changes v1->v2:
>> - Follow Radim's review: setting constants, preferring simplicity to marginal
>>  performance gain, etc.
>> - Combine the cluster mode and x2apic mode patches
>> 
>> Signed-off-by: Nadav Amit <[email protected]>
>> ---
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index b8345dd..ee04adf 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -68,6 +68,9 @@
>> #define MAX_APIC_VECTOR                      256
>> #define APIC_VECTORS_PER_REG         32
>> 
>> +#define APIC_BROADCAST                      0xFF
>> +#define X2APIC_BROADCAST            0xFFFFFFFFul
> 
> (int is better -- using long introduces an interesting feature with
> implicit retyping: (int)X2APIC_BROADCAST != X2APIC_BROADCAST, and we
> don't compile with -Wtype-limits to notice it.  It poses no problem
> now, so I can change it in an inevitable cleanup series / convince lkml
> to endorse stricter warnings.)
Would unsigned int ease your mind?

> 
>> +
>> #define VEC_POS(v) ((v) & (32 - 1))
>> #define REG_POS(v) (((v) >> 5) << 4)
>> 
>> @@ -558,16 +563,25 @@ static void apic_set_tpr(struct kvm_lapic *apic, u32 
>> tpr)
>>      apic_update_ppr(apic);
>> }
>> 
>> -int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest)
>> +static int kvm_apic_broadcast(struct kvm_lapic *apic, u32 dest)
> 
> (bool is better.)
Ok. I will do it for v3.

Nadav

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

Reply via email to