[PATCH] make I/O APIC IDs and processor APIC IDs unique (asus/m4a785-m)

[Scott Duplichan]

For the AMD SB700 projects, the SB ioapic id is being assigned the same
value as one of the processor core local apic ids. Apparently this is not
such a serious problem as I had once thought. On the other hand, AMD
recommends using a unique value for each apic id.

Assigning a unique id to the SB ioapic is not as simple as choosing the
next biggest available value, because the ioapic is traditionally a 4-bit
value. Though the south bridge has an option for an 8-bit ioapic id,
using such a value might confuse older operating systems. Choosing the
next available id value for the ioapic works when the number of processor
cores is small, and that is what the AMD BKDG recommends. If this would
result in an ioapic id that exceeds 4 bits, the recommendation is to lift
the processor local apic id numbering so that ioapics can be assigned
values starting at zero. The patch below attempts to implement these
recommendations. The case of lifting local apic ids has not been tested.
In order to meet the 4-bit ioapic id limit without lifting the processor
local apic id values, the patch reduces MAX_PHYSICAL_CPUS from 2 to 1,
which is consistent with the currently supported boards and processors.
The patch is for asus/m4a785-m, but can be adapted for other supported
sb700 desktop projects. Tested by booting amd/m4a785-m/seabios to win7
on simnow. The initial and final ioapic id, as well as the value in the
acpi apic table, are 8.

Signed-off-by: Scott Duplichan

Index: src/southbridge/amd/sb700/sb700_sm.c
===================================================================
--- src/southbridge/amd/sb700/sb700_sm.c	(revision 5814)
+++ src/southbridge/amd/sb700/sb700_sm.c	(working copy)
@@ -57,12 +57,20 @@
    printk(BIOS_INFO, "sm_init().\n");

ioapic_base = pci_read_config32(dev, 0x74) & (0xffffffe0);	/* some like mem resource, but does not have  enable bit */
-	/* Don't rename APIC ID */
-	/* TODO: We should call setup_ioapic() here. But kernel hangs if cpu is K8.
-	 * We need to check out why and change back. */
+	
    clear_ioapic(ioapic_base);
+   /* I/O APIC IDs are normally limited to 4-bits. Enforce this limit. */
+   #if (CONFIG_APIC_ID_OFFSET == 0 && CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS < 16)
+	/* Assign the ioapic ID the next available number after the processor core local APIC IDs */
+	setup_ioapic(ioapic_base, CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS);
+   #elif (CONFIG_APIC_ID_OFFSET > 0)
+   /* Assign the ioapic ID the value 0. Processor APIC IDs follow. */ 
+	setup_ioapic(ioapic_base, 0);
+   #else
+   #error "The processor APIC IDs must be lifted to make room for the I/O APIC ID"
+   #endif

-	/* 2.10 Interrupt Routing/Filtering */
+ 	/* 2.10 Interrupt Routing/Filtering */
    dword = pci_read_config8(dev, 0x62);
    dword |= 3;
    pci_write_config8(dev, 0x62, dword);
Index: src/mainboard/asus/m4a785-m/Kconfig
===================================================================
--- src/mainboard/asus/m4a785-m/Kconfig	(revision 5814)
+++ src/mainboard/asus/m4a785-m/Kconfig	(working copy)
@@ -49,7 +49,7 @@

config MAX_PHYSICAL_CPUS
    int
-	default 2
+	default 1

config HW_MEM_HOLE_SIZE_AUTO_INC
    bool
Index: src/mainboard/asus/m4a785-m/acpi_tables.c
===================================================================
--- src/mainboard/asus/m4a785-m/acpi_tables.c	(revision 5814)
+++ src/mainboard/asus/m4a785-m/acpi_tables.c	(working copy)
@@ -70,7 +70,13 @@
    current = acpi_create_madt_lapics(current);

/* Write SB700 IOAPIC, only one */
-	current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *) current, 2,
+   #if (CONFIG_APIC_ID_OFFSET == 0 && CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS < 16)
+   #define IO_APIC_ID (CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS)
+   #else
+   #define IO_APIC_ID 0
+   #endif
+	current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *) current,
+					   IO_APIC_ID,
    				   IO_APIC_ADDR, 0);

current += acpi_create_madt_irqoverride((acpi_madt_irqoverride_t *)

[PATCH] make I/O APIC IDs and processor APIC IDs unique (asus/m4a785-m)

[Peter Stuge]

Scott Duplichan wrote:
> Assigning a unique id to the SB ioapic is not as simple as choosing
> the next biggest available value, because the ioapic is
> traditionally a 4-bit value.

Did you look at how the K8 support does this? I think this may
already be handled there, maybe it's a useful reference.
> +++ src/southbridge/amd/sb700/sb700_sm.c	(working copy)
> #if (CONFIG_APIC_ID_OFFSET == 0 && CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS < 16)

..
> +++ src/mainboard/asus/m4a785-m/acpi_tables.c	(working copy)
> #if (CONFIG_APIC_ID_OFFSET == 0 && CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS < 16)
> #define IO_APIC_ID (CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS)
> #else
> #define IO_APIC_ID 0
> #endif

Is there a header file in north or southbridge that could be used to
store the logic?

//Peter

[PATCH] make I/O APIC IDs and processor APIC IDs unique (asus/m4a785-m)

[Marc Jones]

On Wed, Sep 15, 2010 at 4:53 AM, Peter Stuge  wrote:
> Scott Duplichan wrote:
> > Assigning a unique id to the SB ioapic is not as simple as choosing
> > the next biggest available value, because the ioapic is
> > traditionally a 4-bit value.
>
> Did you look at how the K8 support does this? I think this may
> already be handled there, maybe it's a useful reference.

K8 and Fa10 have the same option to lift the BSP APIC ID, The code
looks stale and I don't think that it would be enough for FAM10. For
K8 you only need to move one ID to make room for the SB (2*8). For
FAM10, you need to lift more than just the BSP.
> > +++ src/southbridge/amd/sb700/sb700_sm.c      (working copy)
> > #if (CONFIG_APIC_ID_OFFSET == 0 && CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS < 16)
>
> ..

Have you tried setting CONFIG_APIC_ID_OFFSET?
> > +++ src/mainboard/asus/m4a785-m/acpi_tables.c (working copy)
> > #if (CONFIG_APIC_ID_OFFSET == 0 && CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS < 16)
> > #define IO_APIC_ID (CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS)
> > #else
> > #define IO_APIC_ID 0
> > #endif
>
> Is there a header file in north or southbridge that could be used to
> store the logic?

I agree, this doesn't belong in acpi code.  IO_APIC_ID could be used
in the sb700_sm.c as well.

Marc

[PATCH] make I/O APIC IDs and processor APIC IDs unique (asus/m4a785-m)

[Scott Duplichan]

]-----Original Message-----
]From: [e-mail] [mailto:[e-mail]] On Behalf Of Marc Jones
]Sent: Wednesday, September 15, 2010 12:36 PM
]To: [e-mail]
]Subject: Re: [coreboot] [PATCH] make I/O APIC IDs and processor APIC IDs unique (asus/m4a785-m)
]
]On Wed, Sep 15, 2010 at 4:53 AM, Peter Stuge  wrote:
]> Scott Duplichan wrote:
]>> Assigning a unique id to the SB ioapic is not as simple as choosing
]>> the next biggest available value, because the ioapic is
]>> traditionally a 4-bit value.
]>
]> Did you look at how the K8 support does this? I think this may
]> already be handled there, maybe it's a useful reference.
]>
]
]K8 and Fa10 have the same option to lift the BSP APIC ID, The code
]looks stale and I don't think that it would be enough for FAM10. For
]K8 you only need to move one ID to make room for the SB (2*8). For
]FAM10, you need to lift more than just the BSP.
]
]
]>
]>> +++ src/southbridge/amd/sb700/sb700_sm.c      (working copy)
]>> +   #if (CONFIG_APIC_ID_OFFSET == 0 && CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS < 16)
]> ..
]
]Have you tried setting CONFIG_APIC_ID_OFFSET?

I didn't even test this method because AMD recommends starting the processor
local apic ids at zero if possible. That is for better compatibility with
some older operating systems I believe. I think none of the coreboot AMD
projects currently support >= 16 cores, so processor apic IDs can start at
zero for the time being. But even if this lifting method resolves the ID
conflict, it does not solve the problem of passing the io apic id to the
os through acpi. Currently the acpi tables use a hard-coded value of 2 for
the io apic id. So there are two problems: ID conflict, and ACPI reporting.

]>
]>> +++ src/mainboard/asus/m4a785-m/acpi_tables.c (working copy)
]>> +   #if (CONFIG_APIC_ID_OFFSET == 0 && CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS < 16)
]>> +   #define IO_APIC_ID (CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS)
]>> +   #else
]>> +   #define IO_APIC_ID 0
]>> +   #endif
]>
]> Is there a header file in north or southbridge that could be used to
]> store the logic?
]
]I agree, this doesn't belong in acpi code.  IO_APIC_ID could be used
]in the sb700_sm.c as well.

It seems like a generic sysconf structure is needed. sb700_sm.c could
use such a structure to pass the io apic id base to acpi_tables.c.

]Marc
]
]-- 
]http://se-eng.com