[PATCH] acpi: add Local APIC NMI Structure to MADT.
Add Local APIC NMI Structure to ACPI MADT. Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com> --- src/acpi.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) Index: seabios/src/acpi.c =================================================================== --- seabios.orig/src/acpi.c +++ seabios/src/acpi.c @@ -134,6 +134,14 @@ struct madt_intsrcovr { u16 flags; } PACKED; +struct madt_local_nmi { + ACPI_SUB_HEADER_DEF + u8 processor_id; /* ACPI processor id */ + u16 flags; /* MPS INTI flags */ + u8 lint; /* Local APIC LINT# */ +} PACKED; + + /* * ACPI 2.0 Generic Address Space definition. */ @@ -288,7 +296,9 @@ build_madt(void) int madt_size = (sizeof(struct multiple_apic_table) + sizeof(struct madt_processor_apic) * MaxCountCPUs + sizeof(struct madt_io_apic) - + sizeof(struct madt_intsrcovr) * 16); + + sizeof(struct madt_intsrcovr) * 16 + + sizeof(struct madt_local_nmi) * MaxCountCPUs); + struct multiple_apic_table *madt = malloc_high(madt_size); if (!madt) { warn_noalloc(); @@ -340,7 +350,17 @@ build_madt(void) intsrcovr++; } - build_header((void*)madt, APIC_SIGNATURE, (void*)intsrcovr - (void*)madt, 1); + struct madt_local_nmi *local_nmi = (void*)intsrcovr; + for (i = 0; i < MaxCountCPUs; i++) { + local_nmi->type = APIC_LOCAL_NMI; + local_nmi->length = sizeof(*local_nmi); + local_nmi->processor_id = i; + local_nmi->flags = 0; + local_nmi->lint = 1; /* LINT1 */ + local_nmi++; + } + + build_header((void*)madt, APIC_SIGNATURE, (void*)local_nmi - (void*)madt, 1); return madt; }
On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
Add Local APIC NMI Structure to ACPI MADT.
Thanks. Can you give a brief description of what this is needed by? Also, is this for kvm, qemu, or some other emulator? -Kevin
(2011/09/23 13:31), Kevin O'Connor wrote:
On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
Add Local APIC NMI Structure to ACPI MADT.
Thanks. Can you give a brief description of what this is needed by?
Also, is this for kvm, qemu, or some other emulator?
This describes LINT pin (LINT0 or LINT1) to which NMI is connected, and this information is needed by OS to initialize local APIC. Current seabios provides this information through MP configuration table but doesn't provide it through ACPI MADT. I found this when I reviewed "inject NMI" feature in qemu/kvm. By the way, I think usually NMI is connected to all processors' LINT1 as MultiProcessor spec indicates, and my patch assumes this. But, I noticed that seabios's MP configuration table provides only a NMI information connected to BSP. Is there any special reason about this? Regards, Kenji Kaneshige
On Mon, Sep 26, 2011 at 07:58:56PM +0900, Kenji Kaneshige wrote:
(2011/09/23 13:31), Kevin O'Connor wrote:
On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
Add Local APIC NMI Structure to ACPI MADT.
Thanks. Can you give a brief description of what this is needed by?
Also, is this for kvm, qemu, or some other emulator?
This describes LINT pin (LINT0 or LINT1) to which NMI is connected, and this information is needed by OS to initialize local APIC. Current seabios provides this information through MP configuration table but doesn't provide it through ACPI MADT. I found this when I reviewed "inject NMI" feature in qemu/kvm.
By the way, I think usually NMI is connected to all processors' LINT1 as MultiProcessor spec indicates, and my patch assumes this. But, I noticed that seabios's MP configuration table provides only a NMI information connected to BSP. Is there any special reason about this?
I'm afraid I'm not very knowledgable on the intricacies of LINTx setup. Your patch looks okay to me, but I would like to see an Ack from one of the QEmu or KVM maintainers as well. -Kevin
(2011/09/30 8:43), Kevin O'Connor wrote:
On Mon, Sep 26, 2011 at 07:58:56PM +0900, Kenji Kaneshige wrote:
(2011/09/23 13:31), Kevin O'Connor wrote:
On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
Add Local APIC NMI Structure to ACPI MADT.
Thanks. Can you give a brief description of what this is needed by?
Also, is this for kvm, qemu, or some other emulator?
This describes LINT pin (LINT0 or LINT1) to which NMI is connected, and this information is needed by OS to initialize local APIC. Current seabios provides this information through MP configuration table but doesn't provide it through ACPI MADT. I found this when I reviewed "inject NMI" feature in qemu/kvm.
By the way, I think usually NMI is connected to all processors' LINT1 as MultiProcessor spec indicates, and my patch assumes this. But, I noticed that seabios's MP configuration table provides only a NMI information connected to BSP. Is there any special reason about this?
I'm afraid I'm not very knowledgable on the intricacies of LINTx setup.
Your patch looks okay to me, but I would like to see an Ack from one of the QEmu or KVM maintainers as well.
Ok, thank you. I will try. Regards, Kenji Kaneshige
On Mon, Sep 26, 2011 at 07:58:56PM +0900, Kenji Kaneshige wrote:
(2011/09/23 13:31), Kevin O'Connor wrote:
On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
Add Local APIC NMI Structure to ACPI MADT.
Thanks. Can you give a brief description of what this is needed by?
Also, is this for kvm, qemu, or some other emulator?
This describes LINT pin (LINT0 or LINT1) to which NMI is connected, and this information is needed by OS to initialize local APIC. Current seabios provides this information through MP configuration table but doesn't provide it through ACPI MADT. I found this when I reviewed "inject NMI" feature in qemu/kvm.
By the way, I think usually NMI is connected to all processors' LINT1 as MultiProcessor spec indicates, and my patch assumes this. But, I noticed that seabios's MP configuration table provides only a NMI information connected to BSP. Is there any special reason about this?
Should be changed to 0xff too. -- Gleb.
(2011/10/02 17:50), Gleb Natapov wrote:
On Mon, Sep 26, 2011 at 07:58:56PM +0900, Kenji Kaneshige wrote:
(2011/09/23 13:31), Kevin O'Connor wrote:
On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
Add Local APIC NMI Structure to ACPI MADT.
Thanks. Can you give a brief description of what this is needed by?
Also, is this for kvm, qemu, or some other emulator?
This describes LINT pin (LINT0 or LINT1) to which NMI is connected, and this information is needed by OS to initialize local APIC. Current seabios provides this information through MP configuration table but doesn't provide it through ACPI MADT. I found this when I reviewed "inject NMI" feature in qemu/kvm.
By the way, I think usually NMI is connected to all processors' LINT1 as MultiProcessor spec indicates, and my patch assumes this. But, I noticed that seabios's MP configuration table provides only a NMI information connected to BSP. Is there any special reason about this?
Should be changed to 0xff too.
Thank you. Will send the patch soon. Regards, Kenji Kaneshige
On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
Add Local APIC NMI Structure to ACPI MADT.
Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com> --- src/acpi.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)
Index: seabios/src/acpi.c =================================================================== --- seabios.orig/src/acpi.c +++ seabios/src/acpi.c @@ -134,6 +134,14 @@ struct madt_intsrcovr { u16 flags; } PACKED;
+struct madt_local_nmi { + ACPI_SUB_HEADER_DEF + u8 processor_id; /* ACPI processor id */ + u16 flags; /* MPS INTI flags */ + u8 lint; /* Local APIC LINT# */ +} PACKED; + + /* * ACPI 2.0 Generic Address Space definition. */ @@ -288,7 +296,9 @@ build_madt(void) int madt_size = (sizeof(struct multiple_apic_table) + sizeof(struct madt_processor_apic) * MaxCountCPUs + sizeof(struct madt_io_apic) - + sizeof(struct madt_intsrcovr) * 16); + + sizeof(struct madt_intsrcovr) * 16 + + sizeof(struct madt_local_nmi) * MaxCountCPUs); + struct multiple_apic_table *madt = malloc_high(madt_size); if (!madt) { warn_noalloc(); @@ -340,7 +350,17 @@ build_madt(void) intsrcovr++; }
- build_header((void*)madt, APIC_SIGNATURE, (void*)intsrcovr - (void*)madt, 1); + struct madt_local_nmi *local_nmi = (void*)intsrcovr; + for (i = 0; i < MaxCountCPUs; i++) { + local_nmi->type = APIC_LOCAL_NMI; + local_nmi->length = sizeof(*local_nmi); + local_nmi->processor_id = i; Spec says that value 0xFF signifies that this applies to all processors in the machine, so you need to create only one APIC_LOCAL_NMI entry with 0xFF as a processor id.
+ local_nmi->flags = 0; + local_nmi->lint = 1; /* LINT1 */ + local_nmi++; + } + + build_header((void*)madt, APIC_SIGNATURE, (void*)local_nmi - (void*)madt, 1); return madt; }
_______________________________________________ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
-- Gleb.
(2011/10/02 17:31), Gleb Natapov wrote:
On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
Add Local APIC NMI Structure to ACPI MADT.
Signed-off-by: Kenji Kaneshige<kaneshige.kenji@jp.fujitsu.com> --- src/acpi.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)
Index: seabios/src/acpi.c =================================================================== --- seabios.orig/src/acpi.c +++ seabios/src/acpi.c @@ -134,6 +134,14 @@ struct madt_intsrcovr { u16 flags; } PACKED;
+struct madt_local_nmi { + ACPI_SUB_HEADER_DEF + u8 processor_id; /* ACPI processor id */ + u16 flags; /* MPS INTI flags */ + u8 lint; /* Local APIC LINT# */ +} PACKED; + + /* * ACPI 2.0 Generic Address Space definition. */ @@ -288,7 +296,9 @@ build_madt(void) int madt_size = (sizeof(struct multiple_apic_table) + sizeof(struct madt_processor_apic) * MaxCountCPUs + sizeof(struct madt_io_apic) - + sizeof(struct madt_intsrcovr) * 16); + + sizeof(struct madt_intsrcovr) * 16 + + sizeof(struct madt_local_nmi) * MaxCountCPUs); + struct multiple_apic_table *madt = malloc_high(madt_size); if (!madt) { warn_noalloc(); @@ -340,7 +350,17 @@ build_madt(void) intsrcovr++; }
- build_header((void*)madt, APIC_SIGNATURE, (void*)intsrcovr - (void*)madt, 1); + struct madt_local_nmi *local_nmi = (void*)intsrcovr; + for (i = 0; i< MaxCountCPUs; i++) { + local_nmi->type = APIC_LOCAL_NMI; + local_nmi->length = sizeof(*local_nmi); + local_nmi->processor_id = i; Spec says that value 0xFF signifies that this applies to all processors in the machine, so you need to create only one APIC_LOCAL_NMI entry with 0xFF as a processor id.
Thank you for your comment. Actually I had the same idea. But according to the revision number in FADT, seabios uses ACPI1.0 spec which doesn't support the value '0xFF'. Could you double check? Regards, Kenji Kaneshige
On Mon, Oct 03, 2011 at 10:47:11AM +0900, Kenji Kaneshige wrote:
(2011/10/02 17:31), Gleb Natapov wrote:
On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
Add Local APIC NMI Structure to ACPI MADT.
Signed-off-by: Kenji Kaneshige<kaneshige.kenji@jp.fujitsu.com> --- src/acpi.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)
Index: seabios/src/acpi.c =================================================================== --- seabios.orig/src/acpi.c +++ seabios/src/acpi.c @@ -134,6 +134,14 @@ struct madt_intsrcovr { u16 flags; } PACKED;
+struct madt_local_nmi { + ACPI_SUB_HEADER_DEF + u8 processor_id; /* ACPI processor id */ + u16 flags; /* MPS INTI flags */ + u8 lint; /* Local APIC LINT# */ +} PACKED; + + /* * ACPI 2.0 Generic Address Space definition. */ @@ -288,7 +296,9 @@ build_madt(void) int madt_size = (sizeof(struct multiple_apic_table) + sizeof(struct madt_processor_apic) * MaxCountCPUs + sizeof(struct madt_io_apic) - + sizeof(struct madt_intsrcovr) * 16); + + sizeof(struct madt_intsrcovr) * 16 + + sizeof(struct madt_local_nmi) * MaxCountCPUs); + struct multiple_apic_table *madt = malloc_high(madt_size); if (!madt) { warn_noalloc(); @@ -340,7 +350,17 @@ build_madt(void) intsrcovr++; }
- build_header((void*)madt, APIC_SIGNATURE, (void*)intsrcovr - (void*)madt, 1); + struct madt_local_nmi *local_nmi = (void*)intsrcovr; + for (i = 0; i< MaxCountCPUs; i++) { + local_nmi->type = APIC_LOCAL_NMI; + local_nmi->length = sizeof(*local_nmi); + local_nmi->processor_id = i; Spec says that value 0xFF signifies that this applies to all processors in the machine, so you need to create only one APIC_LOCAL_NMI entry with 0xFF as a processor id.
Thank you for your comment. Actually I had the same idea. But according to the revision number in FADT, seabios uses ACPI1.0 spec which doesn't support the value '0xFF'. Could you double check?
Seabios ACPI contains bits from various versions. And since 0xFF as a wildcard destination is mention in mptable spec, which predates ACPI1.0 by a couple of years, it is safe to assume that even for ACPI1.0 it is correct value to use. -- Gleb.
(2011/10/03 16:17), Gleb Natapov wrote:
On Mon, Oct 03, 2011 at 10:47:11AM +0900, Kenji Kaneshige wrote:
(2011/10/02 17:31), Gleb Natapov wrote:
On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
Add Local APIC NMI Structure to ACPI MADT.
Signed-off-by: Kenji Kaneshige<kaneshige.kenji@jp.fujitsu.com> --- src/acpi.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)
Index: seabios/src/acpi.c =================================================================== --- seabios.orig/src/acpi.c +++ seabios/src/acpi.c @@ -134,6 +134,14 @@ struct madt_intsrcovr { u16 flags; } PACKED;
+struct madt_local_nmi { + ACPI_SUB_HEADER_DEF + u8 processor_id; /* ACPI processor id */ + u16 flags; /* MPS INTI flags */ + u8 lint; /* Local APIC LINT# */ +} PACKED; + + /* * ACPI 2.0 Generic Address Space definition. */ @@ -288,7 +296,9 @@ build_madt(void) int madt_size = (sizeof(struct multiple_apic_table) + sizeof(struct madt_processor_apic) * MaxCountCPUs + sizeof(struct madt_io_apic) - + sizeof(struct madt_intsrcovr) * 16); + + sizeof(struct madt_intsrcovr) * 16 + + sizeof(struct madt_local_nmi) * MaxCountCPUs); + struct multiple_apic_table *madt = malloc_high(madt_size); if (!madt) { warn_noalloc(); @@ -340,7 +350,17 @@ build_madt(void) intsrcovr++; }
- build_header((void*)madt, APIC_SIGNATURE, (void*)intsrcovr - (void*)madt, 1); + struct madt_local_nmi *local_nmi = (void*)intsrcovr; + for (i = 0; i< MaxCountCPUs; i++) { + local_nmi->type = APIC_LOCAL_NMI; + local_nmi->length = sizeof(*local_nmi); + local_nmi->processor_id = i; Spec says that value 0xFF signifies that this applies to all processors in the machine, so you need to create only one APIC_LOCAL_NMI entry with 0xFF as a processor id.
Thank you for your comment. Actually I had the same idea. But according to the revision number in FADT, seabios uses ACPI1.0 spec which doesn't support the value '0xFF'. Could you double check?
Seabios ACPI contains bits from various versions. And since 0xFF as a wildcard destination is mention in mptable spec, which predates ACPI1.0 by a couple of years, it is safe to assume that even for ACPI1.0 it is correct value to use.
Ok, thank you. I'll update the patch. Regards, Kenji Kaneshige
participants (3)
-
Gleb Natapov -
Kenji Kaneshige -
Kevin O'Connor