x86/x2APIC: tighten check in cluster mode IPI sending

Commit Message

Jan Beulich June 13, 2019, 1:06 p.m. UTC

It is only of limited use to check the full accumulated 32-bit value,
because the high halves are the cluster ID. What needs to be non-zero is
the bit map at the bottom, or else APIC errors will result.

Signed-off-by: Jan Beulich <[e-mail]>

Comments

Andrew Cooper June 13, 2019, 1:11 p.m. UTC | #1

On 13/06/2019 14:06, Jan Beulich wrote:
> It is only of limited use to check the full accumulated 32-bit value,
> because the high halves are the cluster ID. What needs to be non-zero is
> the bit map at the bottom, or else APIC errors will result.
>
> Signed-off-by: Jan Beulich <[e-mail]>

I take it you located the the source of the APIC errors.

I can't find anything in the manual which explicitly states that APIC
errors will arrive, but 10.12.10.2 does clearly identify that a logical
x2APIC ID is non-zero in its bottom 16 bits.

Reviewed-by: Andrew Cooper <[e-mail]>

Jan Beulich June 13, 2019, 1:21 p.m. UTC | #2

>>> On 13.06.19 at 15:11, <[e-mail]> wrote:
> On 13/06/2019 14:06, Jan Beulich wrote:
>> It is only of limited use to check the full accumulated 32-bit value,
>> because the high halves are the cluster ID. What needs to be non-zero is
>> the bit map at the bottom, or else APIC errors will result.
>>
>> Signed-off-by: Jan Beulich <[e-mail]>
> 
> I take it you located the the source of the APIC errors.

Yes, albeit the change here is just a result of me guessing where they
might have come from, i.e. it wasn't the IPI sending in the end. Instead
there was one issue each with losing part or all of the 32-bit destination
in both the I/O-APIC RTE and the MSI message massaging functions.
IOW things would also not have worked correctly in physical mode with
more than 256 CPUs (or fewer, with discontiguous physical APIC IDs).

> I can't find anything in the manual which explicitly states that APIC
> errors will arrive, but 10.12.10.2 does clearly identify that a logical
> x2APIC ID is non-zero in its bottom 16 bits.
> 
> Reviewed-by: Andrew Cooper <[e-mail]>

Thanks.

Jan

Patch

--- a/xen/arch/x86/genapic/x2apic.c
+++ b/xen/arch/x86/genapic/x2apic.c
@@ -154,7 +154,7 @@  static void send_IPI_mask_x2apic_cluster
             msr_content |= per_cpu(cpu_2_logical_apicid, cpu);
         }
 
-        BUG_ON(!msr_content);
+        BUG_ON(!(msr_content & 0xffff));
         msr_content = (msr_content << 32) | APIC_DM_FIXED |
                       APIC_DEST_LOGICAL | vector;
         apic_wrmsr(APIC_ICR, msr_content);