On 3/27/23 22:37, David Woodhouse wrote:
On Mon, 2023-03-27 at 22:33 +0700, Bui Quang Minh wrote:
+ memset(deliver_bitmask, 0x00, max_apic_words * sizeof(uint32_t));
+
+ /* x2APIC broadcast id for both physical and logical (cluster) mode */
+ if (dest == 0xffffffff) {
+ apic_get_broadcast_bitmask(deliver_bitmask, true);
+ return;
+ }
+
if (dest_mode == 0) {
Might be nice to have a constant for DEST_MODE_PHYS vs.
DEST_MODE_LOGICAL to make this clearer?
I'll fix it in the next version.
+ apic_find_dest(deliver_bitmask, dest);
+ /* Broadcast to xAPIC mode apics */
if (dest == 0xff) {
- memset(deliver_bitmask, 0xff, MAX_APIC_WORDS * sizeof(uint32_t));
- } else {
- int idx = apic_find_dest(dest);
- memset(deliver_bitmask, 0x00, MAX_APIC_WORDS * sizeof(uint32_t));
- if (idx >= 0)
- apic_set_bit(deliver_bitmask, idx);
+ apic_get_broadcast_bitmask(deliver_bitmask, false);
Hrm... aren't you still interpreting destination 0x000000FF as
broadcast even for X2APIC mode? Or am I misreading this?
In case the destination is 0xFF, the IPI will be delivered to CPU has
APIC ID 0xFF if it is in x2APIC mode, and it will be delivered to all
CPUs that are in xAPIC mode. In case the destination is 0xFFFFFFFF, the
IPI is delivered to all CPUs that are in x2APIC mode. I've created
apic_get_broadcast_bitmask function and changed the apic_find_dest to
implement that logic.
Maybe I'm misreading the patch, but to me it looks that
if (dest == 0xff) apic_get_broadcast_bitmask() bit applies even in
x2apic mode? So delivering to the APIC with physical ID 255 will be
misinterpreted as a broadcast?
In case dest == 0xff the second argument to apic_get_broadcast_bitmask
is set to false which means this is xAPIC broadcast
static void apic_get_broadcast_bitmask(uint32_t *deliver_bitmask,
bool is_x2apic_broadcast)
{
int i;
APICCommonState *apic_iter;
for (i = 0; i < max_apics; i++) {
apic_iter = local_apics[i];
if (apic_iter) {
bool apic_in_x2apic = is_x2apic_mode(&apic_iter->parent_obj);
if (is_x2apic_broadcast && apic_in_x2apic) {
apic_set_bit(deliver_bitmask, i);
} else if (!is_x2apic_broadcast && !apic_in_x2apic) {
apic_set_bit(deliver_bitmask, i);
}
}
}
}
In apic_get_broadcast_bitmask, because is_x2apic_broadcast == false, the
delivery bit set only if that apic_in_x2apic == false (that CPU is in
xAPIC mode)
