So here's the plan: move all hotplug handling out to ssdt, this way it'll keep working even with a user-supplied dsdt. Next step we can patch this ssdt at runtime.
There's little point in this change alone, so posting as RFC, will repost with the patching part when it's ready, posting now to present opportunity for early feedback.
Compiled only.
Signed-off-by: Michael S. Tsirkin mst@redhat.com --- Makefile | 2 +- src/acpi-dsdt.dsl | 179 ------------------------------------------------- src/acpi.c | 4 + src/ssdt-pcihp.dsl | 188 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 193 insertions(+), 180 deletions(-) create mode 100644 src/ssdt-pcihp.dsl
diff --git a/Makefile b/Makefile index 540f1ea..ef8e15d 100644 --- a/Makefile +++ b/Makefile @@ -200,7 +200,7 @@ src/%.hex: src/%.dsl ./tools/acpi_extract_preprocess.py ./tools/acpi_extract.py $(Q)./tools/acpi_extract.py $(OUT)$*.lst > $(OUT)$*.off $(Q)cat $(OUT)$*.off > $@
-$(OUT)ccode32flat.o: src/acpi-dsdt.hex src/ssdt-proc.hex +$(OUT)ccode32flat.o: src/acpi-dsdt.hex src/ssdt-proc.hex src/ssdt-pcihp.hex
####### Kconfig rules export HOSTCC := $(CC) diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index b9b06f2..32eaeee 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -119,25 +119,6 @@ DefinitionBlock ( prt_slot3(0x001f), })
- OperationRegion(PCST, SystemIO, 0xae00, 0x08) - Field (PCST, DWordAcc, NoLock, WriteAsZeros) - { - PCIU, 32, - PCID, 32, - } - - OperationRegion(SEJ, SystemIO, 0xae08, 0x04) - Field (SEJ, DWordAcc, NoLock, WriteAsZeros) - { - B0EJ, 32, - } - - OperationRegion(RMVC, SystemIO, 0xae0c, 0x04) - Field(RMVC, DWordAcc, NoLock, WriteAsZeros) - { - PCRM, 32, - } - Name (_CRS, ResourceTemplate () { WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, @@ -458,162 +439,6 @@ DefinitionBlock ( } } } - - -/**************************************************************** - * PCI hotplug - ****************************************************************/ - - Scope(_SB.PCI0) { - /* Methods called by bulk generated PCI devices below */ - Method (PRMV, 1, NotSerialized) { - // _RMV method - check if device can be removed - If (And(_SB.PCI0.PCRM, ShiftLeft(1, Arg0))) { - Return (0x1) - } - Return (0x0) - } - -#define gen_pci_device(slot) \ - Device(SL##slot) { \ - Name (_ADR, 0x##slot##0000) \ - Method (_RMV) { Return (PRMV(0x##slot)) } \ - Name (_SUN, 0x##slot) \ - } - - /* VGA (slot 1) and ISA bus (slot 2) defined above */ - gen_pci_device(03) - gen_pci_device(04) - gen_pci_device(05) - gen_pci_device(06) - gen_pci_device(07) - gen_pci_device(08) - gen_pci_device(09) - gen_pci_device(0a) - gen_pci_device(0b) - gen_pci_device(0c) - gen_pci_device(0d) - gen_pci_device(0e) - gen_pci_device(0f) - gen_pci_device(10) - gen_pci_device(11) - gen_pci_device(12) - gen_pci_device(13) - gen_pci_device(14) - gen_pci_device(15) - gen_pci_device(16) - gen_pci_device(17) - gen_pci_device(18) - gen_pci_device(19) - gen_pci_device(1a) - gen_pci_device(1b) - gen_pci_device(1c) - gen_pci_device(1d) - gen_pci_device(1e) - gen_pci_device(1f) - - /* Methods called by bulk generated hotplug devices below */ - Method (PCEJ, 1, NotSerialized) { - // _EJ0 method - eject callback - Store(ShiftLeft(1, Arg0), B0EJ) - Return (0x0) - } - - /* Bulk generated PCI hotplug devices */ -#define hotplug_slot(slot) \ - Device (S##slot) { \ - Name (_ADR, 0x##slot##0000) \ - Method (_EJ0, 1) { Return(PCEJ(0x##slot)) } \ - Name (_SUN, 0x##slot) \ - } - - hotplug_slot(01) - hotplug_slot(02) - hotplug_slot(03) - hotplug_slot(04) - hotplug_slot(05) - hotplug_slot(06) - hotplug_slot(07) - hotplug_slot(08) - hotplug_slot(09) - hotplug_slot(0a) - hotplug_slot(0b) - hotplug_slot(0c) - hotplug_slot(0d) - hotplug_slot(0e) - hotplug_slot(0f) - hotplug_slot(10) - hotplug_slot(11) - hotplug_slot(12) - hotplug_slot(13) - hotplug_slot(14) - hotplug_slot(15) - hotplug_slot(16) - hotplug_slot(17) - hotplug_slot(18) - hotplug_slot(19) - hotplug_slot(1a) - hotplug_slot(1b) - hotplug_slot(1c) - hotplug_slot(1d) - hotplug_slot(1e) - hotplug_slot(1f) - - /* PCI hotplug notify method */ - Method(PCNF, 0) { - // Local0 = iterator - Store (Zero, Local0) - While (LLess(Local0, 31)) { - Increment(Local0) - If (And(PCIU, ShiftLeft(1, Local0))) { - PCNT(Local0, 1) - } - If (And(PCID, ShiftLeft(1, Local0))) { - PCNT(Local0, 3) - } - } - Return(One) - } - -#define gen_pci_hotplug(slot) \ - If (LEqual(Arg0, 0x##slot)) { Notify(S##slot, Arg1) } - - Method(PCNT, 2) { - gen_pci_hotplug(01) - gen_pci_hotplug(02) - gen_pci_hotplug(03) - gen_pci_hotplug(04) - gen_pci_hotplug(05) - gen_pci_hotplug(06) - gen_pci_hotplug(07) - gen_pci_hotplug(08) - gen_pci_hotplug(09) - gen_pci_hotplug(0a) - gen_pci_hotplug(0b) - gen_pci_hotplug(0c) - gen_pci_hotplug(0d) - gen_pci_hotplug(0e) - gen_pci_hotplug(0f) - gen_pci_hotplug(10) - gen_pci_hotplug(11) - gen_pci_hotplug(12) - gen_pci_hotplug(13) - gen_pci_hotplug(14) - gen_pci_hotplug(15) - gen_pci_hotplug(16) - gen_pci_hotplug(17) - gen_pci_hotplug(18) - gen_pci_hotplug(19) - gen_pci_hotplug(1a) - gen_pci_hotplug(1b) - gen_pci_hotplug(1c) - gen_pci_hotplug(1d) - gen_pci_hotplug(1e) - gen_pci_hotplug(1f) - } - } - - /**************************************************************** * PCI IRQs ****************************************************************/ @@ -837,10 +662,6 @@ DefinitionBlock ( Method(_L00) { Return(0x01) } - Method(_L01) { - // PCI hotplug event - Return(_SB.PCI0.PCNF()) - } Method(_L02) { // CPU hotplug event Return(_SB.PRSC()) diff --git a/src/acpi.c b/src/acpi.c index a3770ae..97cc6dd 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -406,6 +406,7 @@ encodeLen(u8 *ssdt_ptr, int length, int bytes) #define SD_PROC (ssdp_proc_aml + *ssdt_proc_start)
#define SSDT_SIGNATURE 0x54445353 // SSDT + static void* build_ssdt(void) { @@ -633,6 +634,8 @@ static const struct pci_device_id acpi_find_tbl[] = {
struct rsdp_descriptor *RsdpAddr;
+#include "ssdt-pcihp.hex" + #define MAX_ACPI_TABLES 20 void acpi_bios_init(void) @@ -664,6 +667,7 @@ acpi_bios_init(void) ACPI_INIT_TABLE(build_madt()); ACPI_INIT_TABLE(build_hpet()); ACPI_INIT_TABLE(build_srat()); + ACPI_INIT_TABLE(ssdp_pcihp_aml);
u16 i, external_tables = qemu_cfg_acpi_additional_tables();
diff --git a/src/ssdt-pcihp.dsl b/src/ssdt-pcihp.dsl new file mode 100644 index 0000000..7ed7eef --- /dev/null +++ b/src/ssdt-pcihp.dsl @@ -0,0 +1,188 @@ +ACPI_EXTRACT_ALL_CODE ssdp_pcihp_aml + +DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1) +{ + +/**************************************************************** + * PCI hotplug + ****************************************************************/ + + External (_SB.PCI0, DeviceObj) + + Scope(_SB.PCI0) { + OperationRegion(PCST, SystemIO, 0xae00, 0x08) + Field (PCST, DWordAcc, NoLock, WriteAsZeros) + { + PCIU, 32, + PCID, 32, + } + + OperationRegion(SEJ, SystemIO, 0xae08, 0x04) + Field (SEJ, DWordAcc, NoLock, WriteAsZeros) + { + B0EJ, 32, + } + + OperationRegion(RMVC, SystemIO, 0xae0c, 0x04) + Field(RMVC, DWordAcc, NoLock, WriteAsZeros) + { + PCRM, 32, + } + + /* Methods called by bulk generated PCI devices below */ + Method (PRMV, 1, NotSerialized) { + // _RMV method - check if device can be removed + If (And(_SB.PCI0.PCRM, ShiftLeft(1, Arg0))) { + Return (0x1) + } + Return (0x0) + } + +#define gen_pci_device(slot) \ + Device(SL##slot) { \ + Name (_ADR, 0x##slot##0000) \ + Method (_RMV) { Return (PRMV(0x##slot)) } \ + Name (_SUN, 0x##slot) \ + } + + /* VGA (slot 1) and ISA bus (slot 2) defined in DSDT */ + gen_pci_device(03) + gen_pci_device(04) + gen_pci_device(05) + gen_pci_device(06) + gen_pci_device(07) + gen_pci_device(08) + gen_pci_device(09) + gen_pci_device(0a) + gen_pci_device(0b) + gen_pci_device(0c) + gen_pci_device(0d) + gen_pci_device(0e) + gen_pci_device(0f) + gen_pci_device(10) + gen_pci_device(11) + gen_pci_device(12) + gen_pci_device(13) + gen_pci_device(14) + gen_pci_device(15) + gen_pci_device(16) + gen_pci_device(17) + gen_pci_device(18) + gen_pci_device(19) + gen_pci_device(1a) + gen_pci_device(1b) + gen_pci_device(1c) + gen_pci_device(1d) + gen_pci_device(1e) + gen_pci_device(1f) + + /* Methods called by bulk generated hotplug devices below */ + Method (PCEJ, 1, NotSerialized) { + // _EJ0 method - eject callback + Store(ShiftLeft(1, Arg0), B0EJ) + Return (0x0) + } + + /* Bulk generated PCI hotplug devices */ +#define hotplug_slot(slot) \ + Device (S##slot) { \ + Name (_ADR, 0x##slot##0000) \ + Method (_EJ0, 1) { Return(PCEJ(0x##slot)) } \ + Name (_SUN, 0x##slot) \ + } + + hotplug_slot(01) + hotplug_slot(02) + hotplug_slot(03) + hotplug_slot(04) + hotplug_slot(05) + hotplug_slot(06) + hotplug_slot(07) + hotplug_slot(08) + hotplug_slot(09) + hotplug_slot(0a) + hotplug_slot(0b) + hotplug_slot(0c) + hotplug_slot(0d) + hotplug_slot(0e) + hotplug_slot(0f) + hotplug_slot(10) + hotplug_slot(11) + hotplug_slot(12) + hotplug_slot(13) + hotplug_slot(14) + hotplug_slot(15) + hotplug_slot(16) + hotplug_slot(17) + hotplug_slot(18) + hotplug_slot(19) + hotplug_slot(1a) + hotplug_slot(1b) + hotplug_slot(1c) + hotplug_slot(1d) + hotplug_slot(1e) + hotplug_slot(1f) + + /* PCI hotplug notify method */ + Method(PCNF, 0) { + // Local0 = iterator + Store (Zero, Local0) + While (LLess(Local0, 31)) { + Increment(Local0) + If (And(PCIU, ShiftLeft(1, Local0))) { + PCNT(Local0, 1) + } + If (And(PCID, ShiftLeft(1, Local0))) { + PCNT(Local0, 3) + } + } + Return(One) + } + +#define gen_pci_hotplug(slot) \ + If (LEqual(Arg0, 0x##slot)) { Notify(S##slot, Arg1) } + + Method(PCNT, 2) { + gen_pci_hotplug(01) + gen_pci_hotplug(02) + gen_pci_hotplug(03) + gen_pci_hotplug(04) + gen_pci_hotplug(05) + gen_pci_hotplug(06) + gen_pci_hotplug(07) + gen_pci_hotplug(08) + gen_pci_hotplug(09) + gen_pci_hotplug(0a) + gen_pci_hotplug(0b) + gen_pci_hotplug(0c) + gen_pci_hotplug(0d) + gen_pci_hotplug(0e) + gen_pci_hotplug(0f) + gen_pci_hotplug(10) + gen_pci_hotplug(11) + gen_pci_hotplug(12) + gen_pci_hotplug(13) + gen_pci_hotplug(14) + gen_pci_hotplug(15) + gen_pci_hotplug(16) + gen_pci_hotplug(17) + gen_pci_hotplug(18) + gen_pci_hotplug(19) + gen_pci_hotplug(1a) + gen_pci_hotplug(1b) + gen_pci_hotplug(1c) + gen_pci_hotplug(1d) + gen_pci_hotplug(1e) + gen_pci_hotplug(1f) + } + } + + Scope (_GPE) + { + Method(_L01) { + // PCI hotplug event + Return(_SB.PCI0.PCNF()) + } + } + +}
On Tue, Nov 01, 2011 at 09:11:40PM +0200, Michael S. Tsirkin wrote:
So here's the plan: move all hotplug handling out to ssdt, this way it'll keep working even with a user-supplied dsdt. Next step we can patch this ssdt at runtime.
There's little point in this change alone, so posting as RFC, will repost with the patching part when it's ready, posting now to present opportunity for early feedback.
[...]
OperationRegion(PCST, SystemIO, 0xae00, 0x08)Field (PCST, DWordAcc, NoLock, WriteAsZeros){PCIU, 32,PCID, 32,}OperationRegion(SEJ, SystemIO, 0xae08, 0x04)Field (SEJ, DWordAcc, NoLock, WriteAsZeros){B0EJ, 32,}OperationRegion(RMVC, SystemIO, 0xae0c, 0x04)Field(RMVC, DWordAcc, NoLock, WriteAsZeros){PCRM, 32,}
[...]
- Scope(_SB.PCI0) {
/* Methods called by bulk generated PCI devices below */Method (PRMV, 1, NotSerialized) {// _RMV method - check if device can be removedIf (And(\_SB.PCI0.PCRM, ShiftLeft(1, Arg0))) {Return (0x1)}Return (0x0)}
[...]
/* Methods called by bulk generated hotplug devices below */Method (PCEJ, 1, NotSerialized) {// _EJ0 method - eject callbackStore(ShiftLeft(1, Arg0), B0EJ)Return (0x0)}
[...]
/* PCI hotplug notify method */Method(PCNF, 0) {// Local0 = iteratorStore (Zero, Local0)While (LLess(Local0, 31)) {Increment(Local0)If (And(PCIU, ShiftLeft(1, Local0))) {PCNT(Local0, 1)}If (And(PCID, ShiftLeft(1, Local0))) {PCNT(Local0, 3)}}Return(One)}
[...]
Method(_L01) {// PCI hotplug eventReturn(\_SB.PCI0.PCNF())}
Can we leave these parts in the DSDT and only move the bulk generated stuff to the SSDT?
-Kevin
On Tue, Nov 01, 2011 at 06:59:01PM -0400, Kevin O'Connor wrote:
On Tue, Nov 01, 2011 at 09:11:40PM +0200, Michael S. Tsirkin wrote:
So here's the plan: move all hotplug handling out to ssdt, this way it'll keep working even with a user-supplied dsdt. Next step we can patch this ssdt at runtime.
There's little point in this change alone, so posting as RFC, will repost with the patching part when it's ready, posting now to present opportunity for early feedback.
[...]
OperationRegion(PCST, SystemIO, 0xae00, 0x08)Field (PCST, DWordAcc, NoLock, WriteAsZeros){PCIU, 32,PCID, 32,}OperationRegion(SEJ, SystemIO, 0xae08, 0x04)Field (SEJ, DWordAcc, NoLock, WriteAsZeros){B0EJ, 32,}OperationRegion(RMVC, SystemIO, 0xae0c, 0x04)Field(RMVC, DWordAcc, NoLock, WriteAsZeros){PCRM, 32,}[...]
- Scope(_SB.PCI0) {
/* Methods called by bulk generated PCI devices below */Method (PRMV, 1, NotSerialized) {// _RMV method - check if device can be removedIf (And(\_SB.PCI0.PCRM, ShiftLeft(1, Arg0))) {Return (0x1)}Return (0x0)}[...]
/* Methods called by bulk generated hotplug devices below */Method (PCEJ, 1, NotSerialized) {// _EJ0 method - eject callbackStore(ShiftLeft(1, Arg0), B0EJ)Return (0x0)}[...]
/* PCI hotplug notify method */Method(PCNF, 0) {// Local0 = iteratorStore (Zero, Local0)While (LLess(Local0, 31)) {Increment(Local0)If (And(PCIU, ShiftLeft(1, Local0))) {PCNT(Local0, 1)}If (And(PCID, ShiftLeft(1, Local0))) {PCNT(Local0, 3)}}Return(One)}[...]
Method(_L01) {// PCI hotplug eventReturn(\_SB.PCI0.PCNF())}Can we leave these parts in the DSDT and only move the bulk generated stuff to the SSDT?
-Kevin
They can, but I thought one of the reasons we do the split is to make it possible for users to supply their own DSDT? If so creating calls from SSDT into DSDT would make this very fragile.
Two other things to note: 1. I didn't find a way to avoid such a dependency completely: there is a requirement for the DSDT to call the root bus PCI0. 2. The RMV will be removed from asl and move into BIOS for runtime patching.
What do you think?
On Wed, Nov 02, 2011 at 10:54:42AM +0200, Michael S. Tsirkin wrote:
On Tue, Nov 01, 2011 at 06:59:01PM -0400, Kevin O'Connor wrote:
Can we leave these parts in the DSDT and only move the bulk generated stuff to the SSDT?
They can, but I thought one of the reasons we do the split is to make it possible for users to supply their own DSDT? If so creating calls from SSDT into DSDT would make this very fragile.
I think it's reasonable to require that a user supplied DSDT still fill certain requirements. Keep in mind that the reason for the "user supplied" DSDT was for new platform (q35) development - not necessarily so each individual user could set their own.
It's not great to have the DSDT and SSDT tied to each other. However, once RMV is removed, it's only two methods (PCNT supplied by the SSDT and PCEJ supplied by the DSDT).
I do see the upside to moving it all to the SSDT - but that has the disadvantage of taking away the ability of a user supplied DSDT to tweak how the hotplug functions work. Also, we'd then want to redo CPU hotplug to be the same way.
-Kevin
At 11/03/2011 08:30 AM, Kevin O'Connor Write:
On Wed, Nov 02, 2011 at 10:54:42AM +0200, Michael S. Tsirkin wrote:
On Tue, Nov 01, 2011 at 06:59:01PM -0400, Kevin O'Connor wrote:
Can we leave these parts in the DSDT and only move the bulk generated stuff to the SSDT?
They can, but I thought one of the reasons we do the split is to make it possible for users to supply their own DSDT? If so creating calls from SSDT into DSDT would make this very fragile.
I think it's reasonable to require that a user supplied DSDT still fill certain requirements. Keep in mind that the reason for the "user supplied" DSDT was for new platform (q35) development - not necessarily so each individual user could set their own.
I do not think it's reasonable to require that a user supplied DSDT still fill certain requiements.
I think we should not use default SSDT if we use user supplied DSDT. If we use user supplied DSDT, we should use user supplied SSDT too.
Thanks Wen Congyang
It's not great to have the DSDT and SSDT tied to each other. However, once RMV is removed, it's only two methods (PCNT supplied by the SSDT and PCEJ supplied by the DSDT).
I do see the upside to moving it all to the SSDT - but that has the disadvantage of taking away the ability of a user supplied DSDT to tweak how the hotplug functions work. Also, we'd then want to redo CPU hotplug to be the same way.
-Kevin
SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
On Thu, Nov 03, 2011 at 09:04:57AM +0800, Wen Congyang wrote:
At 11/03/2011 08:30 AM, Kevin O'Connor Write:
I think it's reasonable to require that a user supplied DSDT still fill certain requirements. Keep in mind that the reason for the "user supplied" DSDT was for new platform (q35) development - not necessarily so each individual user could set their own.
I do not think it's reasonable to require that a user supplied DSDT still fill certain requiements.
Please elaborate on why.
The dependency on the DSDT is a few methods (PCEJ, CPEJ, CPST, CPMA). Under what circumstances would it be difficult for these methods to be provided?
I think we should not use default SSDT if we use user supplied DSDT. If we use user supplied DSDT, we should use user supplied SSDT too.
That's possible. However, how do you plan to provide a DSDT that accounts for the dynamic nature of the virtual system configuration. A different qemu/kvm command line (which configures pci or cpu count) will then require a different DSDT.
-Kevin
At 11/03/2011 09:36 AM, Kevin O'Connor Write:
On Thu, Nov 03, 2011 at 09:04:57AM +0800, Wen Congyang wrote:
At 11/03/2011 08:30 AM, Kevin O'Connor Write:
I think it's reasonable to require that a user supplied DSDT still fill certain requirements. Keep in mind that the reason for the "user supplied" DSDT was for new platform (q35) development - not necessarily so each individual user could set their own.
I do not think it's reasonable to require that a user supplied DSDT still fill certain requiements.
Please elaborate on why.
If we require it, I think it is very bad designed, because the user should read seabios before he writes DSDT. I think the user should only need to know how to pass his DSDT and SSDT to seabios. He should be able to write his DSDT and SSDT without knowing seabios's DSDT and SSDT.
Another problem is that: if we require it, but the user does not do it, I do not know the behavior. According to ACPI, SSDT can only add data, and can not overwrite the data. So the user must know the data in SSDT. If we update SSDT, and add new data that may exist in user supplied DSDT, the behavior is undefined.
So I think we should not use default SSDT when we use user supplied DSDT. If so, the user can write his DSDT easier.
The dependency on the DSDT is a few methods (PCEJ, CPEJ, CPST, CPMA). Under what circumstances would it be difficult for these methods to be provided?
I think we should not use default SSDT if we use user supplied DSDT. If we use user supplied DSDT, we should use user supplied SSDT too.
That's possible. However, how do you plan to provide a DSDT that accounts for the dynamic nature of the virtual system configuration. A different qemu/kvm command line (which configures pci or cpu count) will then require a different DSDT.
Hmm, I do not think it now. I think DSDT and SSDT can be generated in qemu, but it is very complex.
I have another question: is seabios used only by qemu now? Or it is a common bios, and is not only for qemu?
Thanks Wen Congyang
-Kevin
On Wed, Nov 02, 2011 at 09:36:58PM -0400, Kevin O'Connor wrote:
On Thu, Nov 03, 2011 at 09:04:57AM +0800, Wen Congyang wrote:
At 11/03/2011 08:30 AM, Kevin O'Connor Write:
I think it's reasonable to require that a user supplied DSDT still fill certain requirements. Keep in mind that the reason for the "user supplied" DSDT was for new platform (q35) development - not necessarily so each individual user could set their own.
I do not think it's reasonable to require that a user supplied DSDT still fill certain requiements.
Please elaborate on why.
The dependency on the DSDT is a few methods (PCEJ, CPEJ, CPST, CPMA).
The one that seems unavoidable is PCI0.
On Wed, Nov 02, 2011 at 08:30:28PM -0400, Kevin O'Connor wrote:
On Wed, Nov 02, 2011 at 10:54:42AM +0200, Michael S. Tsirkin wrote:
On Tue, Nov 01, 2011 at 06:59:01PM -0400, Kevin O'Connor wrote:
Can we leave these parts in the DSDT and only move the bulk generated stuff to the SSDT?
They can, but I thought one of the reasons we do the split is to make it possible for users to supply their own DSDT? If so creating calls from SSDT into DSDT would make this very fragile.
I think it's reasonable to require that a user supplied DSDT still fill certain requirements. Keep in mind that the reason for the "user supplied" DSDT was for new platform (q35) development - not necessarily so each individual user could set their own.
It's not great to have the DSDT and SSDT tied to each other. However, once RMV is removed, it's only two methods (PCNT supplied by the SSDT and PCEJ supplied by the DSDT).
I do see the upside to moving it all to the SSDT - but that has the disadvantage of taking away the ability of a user supplied DSDT to tweak how the hotplug functions work.
Is this something q35 needs?
Also, we'd then want to redo CPU hotplug to be the same way.
OK, I'll do that if we decide this is a good direcction.
-Kevin
On Thu, Nov 03, 2011 at 02:31:18PM +0200, Michael S. Tsirkin wrote:
On Wed, Nov 02, 2011 at 08:30:28PM -0400, Kevin O'Connor wrote:
On Wed, Nov 02, 2011 at 10:54:42AM +0200, Michael S. Tsirkin wrote:
On Tue, Nov 01, 2011 at 06:59:01PM -0400, Kevin O'Connor wrote:
Can we leave these parts in the DSDT and only move the bulk generated stuff to the SSDT?
They can, but I thought one of the reasons we do the split is to make it possible for users to supply their own DSDT? If so creating calls from SSDT into DSDT would make this very fragile.
I think it's reasonable to require that a user supplied DSDT still fill certain requirements. Keep in mind that the reason for the "user supplied" DSDT was for new platform (q35) development - not necessarily so each individual user could set their own.
It's not great to have the DSDT and SSDT tied to each other. However, once RMV is removed, it's only two methods (PCNT supplied by the SSDT and PCEJ supplied by the DSDT).
I do see the upside to moving it all to the SSDT - but that has the disadvantage of taking away the ability of a user supplied DSDT to tweak how the hotplug functions work.
Is this something q35 needs?
q35 emulator will discard qemu-specific pci hotplug(hw/acpi_piix4.c). Instead, pcie hotplug is introduced. (that's my plan at least.) So pci hotplug code in acpi (DSDT or SSDT) for acpi_piix4 needs to be suppressed somehow.
thanks,
Also, we'd then want to redo CPU hotplug to be the same way.
OK, I'll do that if we decide this is a good direcction.
-Kevin
SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
At 11/02/2011 03:11 AM, Michael S. Tsirkin Write:
So here's the plan: move all hotplug handling out to ssdt, this way it'll keep working even with a user-supplied dsdt. Next step we can patch this ssdt at runtime.
There's little point in this change alone, so posting as RFC, will repost with the patching part when it's ready, posting now to present opportunity for early feedback.
Compiled only.
Hot plug on PCI bus 0 can not work with this patch. It can works without this patch.
I have met the following warning messages when hot removing a PCI device:
WARNING: at /builddir/build/BUILD/kernel-2.6.32-195.el6/linux-2.6.32-195.el6.i686/arch/x86/include/asm/dma-mapping.h:154 ___free_dma_mem_cluster+0xe1/0xf0 [sym53c8xx]() (Tainted : G W ---------------- ) Hardware name: Bochs Modules linked in: sym53c8xx scsi_transport_spi autofs4 sunrpc ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 dm_mirror dm_region_hash dm_log virtio_net i6300esb virtio_balloon 8139too 8139cp mii i2c_piix4 i2c_core sg ext4 mbcache jbd2 virtio_blk sr_mod cdrom sd_mod crc_t10dif virtio_pci virtio_ring virtio pata_acpi ata_generic ata_piix dm_mod [last unloaded: mperf] Pid: 39, comm: kacpi_notify Tainted: G W ---------------- 2.6.32-195.el6.i686 #1 Call Trace: [<c0454b11>] ? warn_slowpath_common+0x81/0xc0 [<e13bb961>] ? ___free_dma_mem_cluster+0xe1/0xf0 [sym53c8xx] [<e13bb961>] ? ___free_dma_mem_cluster+0xe1/0xf0 [sym53c8xx] [<c0454b6b>] ? warn_slowpath_null+0x1b/0x20 [<e13bb961>] ? ___free_dma_mem_cluster+0xe1/0xf0 [sym53c8xx] [<e13bb764>] ? __sym_mfree+0xa4/0xf0 [sym53c8xx] [<e13bb806>] ? __sym_mfree_dma+0x56/0xd0 [sym53c8xx] [<e13b288f>] ? sym_free_resources+0x4f/0x70 [sym53c8xx] [<e13b2947>] ? sym_detach+0x97/0xc0 [sym53c8xx] [<e13b2994>] ? sym2_remove+0x24/0x50 [sym53c8xx] [<c0610a36>] ? pci_device_remove+0x16/0x40 [<c06b9c8d>] ? __device_release_driver+0x4d/0xb0 [<c06b9d9d>] ? device_release_driver+0x1d/0x30 [<c06b8e5c>] ? bus_remove_device+0x7c/0xe0 [<c06b6f6f>] ? device_del+0xdf/0x160 [<c06b7009>] ? device_unregister+0x19/0x60 [<c06471f9>] ? acpi_os_execute_deferred+0x0/0x23 [<c060afc6>] ? pci_stop_bus_device+0x66/0x80 [<c061fee0>] ? acpiphp_disable_slot+0x80/0x1a0 [<c06471f9>] ? acpi_os_execute_deferred+0x0/0x23 [<c0620649>] ? handle_hotplug_event_func+0xb9/0x180 [<c047b78b>] ? up+0xb/0x40 [<c064711d>] ? acpi_os_signal_semaphore+0x1e/0x23 [<c0665c14>] ? acpi_ut_release_mutex+0x50/0x53 [<c065cd5e>] ? acpi_get_data+0x4a/0x58 [<c06471f9>] ? acpi_os_execute_deferred+0x0/0x23 [<c064931e>] ? acpi_bus_get_device+0x18/0x2c [<c065481b>] ? acpi_ev_notify_dispatch+0x4c/0x55 [<c0647213>] ? acpi_os_execute_deferred+0x1a/0x23 [<c047181b>] ? worker_thread+0x11b/0x230 [<c0475f50>] ? autoremove_wake_function+0x0/0x40 [<c0471700>] ? worker_thread+0x0/0x230 [<c0475d14>] ? kthread+0x74/0x80 [<c0475ca0>] ? kthread+0x0/0x80 [<c0409fff>] ? kernel_thread_helper+0x7/0x10 ---[ end trace 1261f0bed0518ca9 ]---
Signed-off-by: Michael S. Tsirkin mst@redhat.com
At 11/02/2011 02:08 PM, Wen Congyang Write:
At 11/02/2011 03:11 AM, Michael S. Tsirkin Write:
So here's the plan: move all hotplug handling out to ssdt, this way it'll keep working even with a user-supplied dsdt. Next step we can patch this ssdt at runtime.
There's little point in this change alone, so posting as RFC, will repost with the patching part when it's ready, posting now to present opportunity for early feedback.
Compiled only.
Hot plug on PCI bus 0 can not work with this patch. It can works without this patch.
I think that I may find the reason: ================================ # ls -l /sys/firmware/acpi/tables/ total 0 -r--------. 1 root root 0 Nov 2 14:23 APIC -r--------. 1 root root 0 Nov 2 14:23 DSDT drwxr-xr-x. 2 root root 0 Nov 2 14:23 dynamic -r--------. 1 root root 0 Nov 2 14:23 FACP -r--------. 1 root root 0 Nov 2 14:23 FACS -r--------. 1 root root 0 Nov 2 14:23 HPET -r--------. 1 root root 0 Nov 2 14:23 SSDT # ls -l /sys/firmware/acpi/tables/dynamic/ total 0 ================================
The kernel only loads one SSDT. I use iasl to look at the content of SSDT, and find that it is ssdt-proc.dsl.
The kernel does not load ssdt-pcihp.dsl.
Thanks Wen Congyang
-
Isaku Yamahata -
Kevin O'Connor -
Michael S. Tsirkin -
Wen Congyang