* [PATCH RFC] acpi: add ssdt for cpi hotplug
@ 2011-11-01 19:11 Michael S. Tsirkin
2011-11-01 22:59 ` Kevin O'Connor
2011-11-02 6:08 ` Wen Congyang
0 siblings, 2 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2011-11-01 19:11 UTC (permalink / raw)
To: seabios; +Cc: Kevin O'Connor, Gleb Natapov, kvm, Marcelo Tosatti
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())
+ }
+ }
+
+}
--
1.7.5.53.gc233e
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH RFC] acpi: add ssdt for cpi hotplug
2011-11-01 19:11 [PATCH RFC] acpi: add ssdt for cpi hotplug Michael S. Tsirkin
@ 2011-11-01 22:59 ` Kevin O'Connor
2011-11-02 8:54 ` Michael S. Tsirkin
2011-11-02 6:08 ` Wen Congyang
1 sibling, 1 reply; 12+ messages in thread
From: Kevin O'Connor @ 2011-11-01 22:59 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: seabios, Gleb Natapov, kvm, Marcelo Tosatti
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 removed
> - If (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 callback
> - Store(ShiftLeft(1, Arg0), B0EJ)
> - Return (0x0)
> - }
[...]
> - /* 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)
> - }
[...]
> - Method(_L01) {
> - // PCI hotplug event
> - Return(\_SB.PCI0.PCNF())
> - }
Can we leave these parts in the DSDT and only move the bulk generated
stuff to the SSDT?
-Kevin
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH RFC] acpi: add ssdt for cpi hotplug
2011-11-01 22:59 ` Kevin O'Connor
@ 2011-11-02 8:54 ` Michael S. Tsirkin
2011-11-03 0:30 ` Kevin O'Connor
0 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2011-11-02 8:54 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: seabios, Gleb Natapov, kvm, Marcelo Tosatti
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 removed
> > - If (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 callback
> > - Store(ShiftLeft(1, Arg0), B0EJ)
> > - Return (0x0)
> > - }
> [...]
> > - /* 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)
> > - }
> [...]
> > - Method(_L01) {
> > - // PCI hotplug event
> > - Return(\_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?
--
MST
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH RFC] acpi: add ssdt for cpi hotplug
2011-11-02 8:54 ` Michael S. Tsirkin
@ 2011-11-03 0:30 ` Kevin O'Connor
2011-11-03 1:04 ` [SeaBIOS] " Wen Congyang
2011-11-03 12:31 ` Michael S. Tsirkin
0 siblings, 2 replies; 12+ messages in thread
From: Kevin O'Connor @ 2011-11-03 0:30 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: seabios, Gleb Natapov, kvm, Marcelo Tosatti
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [SeaBIOS] [PATCH RFC] acpi: add ssdt for cpi hotplug
2011-11-03 0:30 ` Kevin O'Connor
@ 2011-11-03 1:04 ` Wen Congyang
2011-11-03 1:36 ` Kevin O'Connor
2011-11-03 12:31 ` Michael S. Tsirkin
1 sibling, 1 reply; 12+ messages in thread
From: Wen Congyang @ 2011-11-03 1:04 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: Michael S. Tsirkin, seabios, kvm
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] acpi: add ssdt for cpi hotplug
2011-11-03 1:04 ` [SeaBIOS] " Wen Congyang
@ 2011-11-03 1:36 ` Kevin O'Connor
2011-11-03 2:14 ` [SeaBIOS] " Wen Congyang
2011-11-03 12:33 ` Michael S. Tsirkin
0 siblings, 2 replies; 12+ messages in thread
From: Kevin O'Connor @ 2011-11-03 1:36 UTC (permalink / raw)
To: Wen Congyang; +Cc: seabios, kvm, Michael S. Tsirkin
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [SeaBIOS] [PATCH RFC] acpi: add ssdt for cpi hotplug
2011-11-03 1:36 ` Kevin O'Connor
@ 2011-11-03 2:14 ` Wen Congyang
2011-11-03 12:33 ` Michael S. Tsirkin
1 sibling, 0 replies; 12+ messages in thread
From: Wen Congyang @ 2011-11-03 2:14 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: Michael S. Tsirkin, seabios, kvm
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] acpi: add ssdt for cpi hotplug
2011-11-03 1:36 ` Kevin O'Connor
2011-11-03 2:14 ` [SeaBIOS] " Wen Congyang
@ 2011-11-03 12:33 ` Michael S. Tsirkin
1 sibling, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2011-11-03 12:33 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: seabios, kvm
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.
--
MST
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] acpi: add ssdt for cpi hotplug
2011-11-03 0:30 ` Kevin O'Connor
2011-11-03 1:04 ` [SeaBIOS] " Wen Congyang
@ 2011-11-03 12:31 ` Michael S. Tsirkin
2011-11-04 6:00 ` [SeaBIOS] " Isaku Yamahata
1 sibling, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2011-11-03 12:31 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: seabios, kvm
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [SeaBIOS] [PATCH RFC] acpi: add ssdt for cpi hotplug
2011-11-03 12:31 ` Michael S. Tsirkin
@ 2011-11-04 6:00 ` Isaku Yamahata
0 siblings, 0 replies; 12+ messages in thread
From: Isaku Yamahata @ 2011-11-04 6:00 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Kevin O'Connor, seabios, kvm
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
>
--
yamahata
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [SeaBIOS] [PATCH RFC] acpi: add ssdt for cpi hotplug
2011-11-01 19:11 [PATCH RFC] acpi: add ssdt for cpi hotplug Michael S. Tsirkin
2011-11-01 22:59 ` Kevin O'Connor
@ 2011-11-02 6:08 ` Wen Congyang
2011-11-02 7:20 ` Wen Congyang
1 sibling, 1 reply; 12+ messages in thread
From: Wen Congyang @ 2011-11-02 6:08 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: seabios, kvm
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>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [SeaBIOS] [PATCH RFC] acpi: add ssdt for cpi hotplug
2011-11-02 6:08 ` Wen Congyang
@ 2011-11-02 7:20 ` Wen Congyang
0 siblings, 0 replies; 12+ messages in thread
From: Wen Congyang @ 2011-11-02 7:20 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: seabios, kvm
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-11-04 6:00 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-01 19:11 [PATCH RFC] acpi: add ssdt for cpi hotplug Michael S. Tsirkin
2011-11-01 22:59 ` Kevin O'Connor
2011-11-02 8:54 ` Michael S. Tsirkin
2011-11-03 0:30 ` Kevin O'Connor
2011-11-03 1:04 ` [SeaBIOS] " Wen Congyang
2011-11-03 1:36 ` Kevin O'Connor
2011-11-03 2:14 ` [SeaBIOS] " Wen Congyang
2011-11-03 12:33 ` Michael S. Tsirkin
2011-11-03 12:31 ` Michael S. Tsirkin
2011-11-04 6:00 ` [SeaBIOS] " Isaku Yamahata
2011-11-02 6:08 ` Wen Congyang
2011-11-02 7:20 ` Wen Congyang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).