Along the lines of what b89f8f054f96 ("x86/apic: Drop sync_Arb_IDs()")
said, the function is dead logic as well: All 64-bit capable Intel systems
have (at least) xAPIC (if not x2APIC).
Even if Eclair can't know it, such code is violating Misra rule 2.2 (dead
code; we didn't accept that yet, but - where possible - we probably would
better follow it). Depending on one's reading, this code may actually be a
violation of rule 2.1 (unreachable), which we did accept:
"Code is unreachable if, ..., there is no combination of program inputs
that can cause it to be executed."
Otoh it's "only" __init code.
As this removes the last user of APIC_XAPIC(), remove the macro as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/include/asm/apicdef.h
+++ b/xen/arch/x86/include/asm/apicdef.h
@@ -19,7 +19,6 @@
#define APIC_LVR_DIRECTED_EOI (1 << 24)
#define GET_APIC_VERSION(x) ((x)&0xFF)
#define GET_APIC_MAXLVT(x) (((x)>>16)&0xFF)
-#define APIC_XAPIC(x) ((x) >= 0x14)
#define APIC_TASKPRI 0x80
#define APIC_TPRI_MASK 0xFF
#define APIC_ARBPRI 0x90
--- a/xen/arch/x86/include/asm/io_apic.h
+++ b/xen/arch/x86/include/asm/io_apic.h
@@ -184,7 +184,6 @@ extern bool skip_ioapic_setup;
extern bool ioapic_ack_new;
extern bool ioapic_ack_forced;
-extern int io_apic_get_unique_id (int ioapic, int apic_id);
extern int io_apic_get_version (int ioapic);
extern int io_apic_get_redir_entries (int ioapic);
extern int io_apic_set_pci_routing (int ioapic, int pin, int irq, int edge_level, int active_high_low);
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2092,86 +2092,6 @@ void ioapic_resume(void)
spin_unlock_irqrestore(&ioapic_lock, flags);
}
-/* --------------------------------------------------------------------------
- ACPI-based IOAPIC Configuration
- -------------------------------------------------------------------------- */
-
-
-int __init io_apic_get_unique_id (int ioapic, int apic_id)
-{
- union IO_APIC_reg_00 reg_00;
- static physid_mask_t __initdata apic_id_map = PHYSID_MASK_NONE;
- unsigned long flags;
- int i = 0;
- const uint32_t broadcast_id = 0xf;
-
- /*
- * The P4 platform supports up to 256 APIC IDs on two separate APIC
- * buses (one for LAPICs, one for IOAPICs), where predecessors only
- * supports up to 16 on one shared APIC bus.
- *
- * TBD: Expand LAPIC/IOAPIC support on P4-class systems to take full
- * advantage of new APIC bus architecture.
- */
-
- if (physids_empty(apic_id_map))
- apic_id_map = phys_cpu_present_map;
-
- spin_lock_irqsave(&ioapic_lock, flags);
- reg_00.raw = io_apic_read(ioapic, 0);
- spin_unlock_irqrestore(&ioapic_lock, flags);
-
- if (apic_id >= broadcast_id) {
- printk(KERN_WARNING "IOAPIC[%d]: Invalid apic_id %d, trying "
- "%d\n", ioapic, apic_id, reg_00.bits.ID);
- apic_id = reg_00.bits.ID;
- }
-
- /*
- * Every APIC in a system must have a unique ID or we get lots of nice
- * 'stuck on smp_invalidate_needed IPI wait' messages.
- */
- if ( physid_isset(apic_id, apic_id_map) )
- {
-
- for (i = 0; i < broadcast_id; i++) {
- if ( !physid_isset(i, apic_id_map) )
- break;
- }
-
- if (i == broadcast_id)
- panic("Max apic_id exceeded\n");
-
- printk(KERN_WARNING "IOAPIC[%d]: apic_id %d already used, "
- "trying %d\n", ioapic, apic_id, i);
-
- apic_id = i;
- }
-
- physid_set(apic_id, apic_id_map);
-
- if (reg_00.bits.ID != apic_id) {
- reg_00.bits.ID = apic_id;
-
- spin_lock_irqsave(&ioapic_lock, flags);
- io_apic_write(ioapic, 0, reg_00.raw);
- reg_00.raw = io_apic_read(ioapic, 0);
- spin_unlock_irqrestore(&ioapic_lock, flags);
-
- /* Sanity check */
- if (reg_00.bits.ID != apic_id) {
- printk("IOAPIC[%d]: Unable to change apic_id!\n", ioapic);
- return -1;
- }
- }
-
- apic_printk(APIC_VERBOSE, KERN_INFO
- "IOAPIC[%d]: Assigned apic_id %d\n", ioapic, apic_id);
-
- return apic_id;
-}
-
-
int __init io_apic_get_version (int ioapic)
{
union IO_APIC_reg_01 reg_01;
--- a/xen/arch/x86/mpparse.c
+++ b/xen/arch/x86/mpparse.c
@@ -871,7 +871,6 @@ void __init mp_register_ioapic (
u32 gsi_base)
{
int idx = 0;
- int tmpid;
if (nr_ioapics >= MAX_IO_APICS) {
printk(KERN_ERR "ERROR: Max # of I/O APICs (%d) exceeded "
@@ -891,16 +890,7 @@ void __init mp_register_ioapic (
mp_ioapics[idx].mpc_apicaddr = address;
set_fixmap_nocache(FIX_IO_APIC_BASE_0 + idx, address);
- if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
- && !APIC_XAPIC(apic_version[boot_cpu_physical_apicid]))
- tmpid = io_apic_get_unique_id(idx, id);
- else
- tmpid = id;
- if (tmpid == -1) {
- nr_ioapics--;
- return;
- }
- mp_ioapics[idx].mpc_apicid = tmpid;
+ mp_ioapics[idx].mpc_apicid = id;
mp_ioapics[idx].mpc_apicver = io_apic_get_version(idx);
/*
On 03/09/2025 8:56 am, Jan Beulich wrote:
> Along the lines of what b89f8f054f96 ("x86/apic: Drop sync_Arb_IDs()")
> said, the function is dead logic as well: All 64-bit capable Intel systems
> have (at least) xAPIC (if not x2APIC).
>
> Even if Eclair can't know it, such code is violating Misra rule 2.2 (dead
> code; we didn't accept that yet, but - where possible - we probably would
> better follow it). Depending on one's reading, this code may actually be a
> violation of rule 2.1 (unreachable), which we did accept:
>
> "Code is unreachable if, ..., there is no combination of program inputs
> that can cause it to be executed."
>
> Otoh it's "only" __init code.
>
> As this removes the last user of APIC_XAPIC(), remove the macro as well.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Same feedback as with the previous patch, albeit about the middle 3
paragraphs. Again with a strong preference for those to be removed,
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
I'd not even spotted that APIC_XAPIC() existed. This being explicit
conformation of where XAPIC starts would have been helpful when doing
archaeology.
On 2025-09-03 03:56, Jan Beulich wrote:
> Along the lines of what b89f8f054f96 ("x86/apic: Drop sync_Arb_IDs()")
> said, the function is dead logic as well: All 64-bit capable Intel systems
> have (at least) xAPIC (if not x2APIC).
>
> Even if Eclair can't know it, such code is violating Misra rule 2.2 (dead
> code; we didn't accept that yet, but - where possible - we probably would
> better follow it). Depending on one's reading, this code may actually be a
> violation of rule 2.1 (unreachable), which we did accept:
>
> "Code is unreachable if, ..., there is no combination of program inputs
> that can cause it to be executed."
>
> Otoh it's "only" __init code.
>
> As this removes the last user of APIC_XAPIC(), remove the macro as well.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
© 2016 - 2025 Red Hat, Inc.