Re: [Xen-devel] [PATCH] [RFC] tools/hvmloader: Control ACPI debugging using a platform flag

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>Since Qemu-trad
>  c/s 147f83f9b7d87a698c200c4f3eb2d36a0e4fe54b
>  "hw/piix4acpi: Make writes to ACPI_DBG_IO_ADDR actually work."
>
>there has been quite a lot of extra logging appearing in the VM logs.
>
>The hotplug debugging contributes 2 vmexits per slot per hotplug event,
>which
>are simply a waste of time unless a developer is trying to debug VM
>hotplugging problems.
>
>Introduce a platform flag, "acpi-debug" to indicate in the AML whether
>debugging writes are wanted or not

Why not just ifdef this whole thing with DEBUG so it will be only compiled when 
debug=y is used.

And also remove the extra print in QEMU for that port.
>
>Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>CC: Keir Fraser <keir@xxxxxxx>
>CC: Jan Beulich <JBeulich@xxxxxxxx>
>CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
>CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
>CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>---
> docs/misc/xenstore-paths.markdown       |    1 +
> tools/firmware/hvmloader/acpi/build.c   |    5 +++++
> tools/firmware/hvmloader/acpi/dsdt.asl  |    1 +
> tools/firmware/hvmloader/acpi/mk_dsdt.c |    6 ++++++
> 4 files changed, 13 insertions(+)
>
>diff --git a/docs/misc/xenstore-paths.markdown
>b/docs/misc/xenstore-paths.markdown
>index 70ab7f4..593a7b1 100644
>--- a/docs/misc/xenstore-paths.markdown
>+++ b/docs/misc/xenstore-paths.markdown
>@@ -190,6 +190,7 @@ Various platform properties.
> * acpi -- is ACPI enabled for this domain
> * acpi_s3 -- is ACPI S3 support enabled for this domain
> * acpi_s4 -- is ACPI S4 support enabled for this domain
>+* acpi-debug -- whether the AML code should write debugging messages
>to qemu
> 
> #### ~/platform/generation-id = INTEGER ":" INTEGER [HVM,INTERNAL]
> 
>diff --git a/tools/firmware/hvmloader/acpi/build.c
>b/tools/firmware/hvmloader/acpi/build.c
>index f1dd3f0..59716bb 100644
>--- a/tools/firmware/hvmloader/acpi/build.c
>+++ b/tools/firmware/hvmloader/acpi/build.c
>@@ -47,6 +47,7 @@ struct acpi_info {
>     uint8_t  com2_present:1;    /* 0[1] - System has COM2? */
>     uint8_t  lpt1_present:1;    /* 0[2] - System has LPT1? */
>     uint8_t  hpet_present:1;    /* 0[3] - System has HPET? */
>+    uint8_t  acpi_debugging:1;  /* 0[4] - ACPI debugging enabled ? */
>     uint32_t pci_min, pci_len;  /* 4, 8 - PCI I/O hole boundaries */
>     uint32_t madt_csum_addr;    /* 12   - Address of MADT checksum */
>uint32_t madt_lapic0_addr;  /* 16   - Address of first MADT LAPIC
>struct */
>@@ -404,6 +405,7 @@ void acpi_build_tables(struct acpi_config *config,
>unsigned int physical)
>     unsigned char       *dsdt;
>     unsigned long        secondary_tables[ACPI_MAX_SECONDARY_TABLES];
>     int                  nr_secondaries, i;
>+    const char          *xs_str;
> 
>     /* Allocate and initialise the acpi info area. */
>    mem_hole_populate_ram(ACPI_INFO_PHYSICAL_ADDRESS >> PAGE_SHIFT, 1);
>@@ -519,10 +521,13 @@ void acpi_build_tables(struct acpi_config
>*config, unsigned int physical)
>     if ( !new_vm_gid(acpi_info) )
>         goto oom;
> 
>+    xs_str = xenstore_read("platform/acpi-debug", "0");
>+
>     acpi_info->com1_present = uart_exists(0x3f8);
>     acpi_info->com2_present = uart_exists(0x2f8);
>     acpi_info->lpt1_present = lpt_exists(0x378);
>     acpi_info->hpet_present = hpet_exists(ACPI_HPET_ADDRESS);
>+    acpi_info->acpi_debugging = (xs_str[0] == '1');
>     acpi_info->pci_min = pci_mem_start;
>     acpi_info->pci_len = pci_mem_end - pci_mem_start;
> 
>diff --git a/tools/firmware/hvmloader/acpi/dsdt.asl
>b/tools/firmware/hvmloader/acpi/dsdt.asl
>index 247a8ad..e753286 100644
>--- a/tools/firmware/hvmloader/acpi/dsdt.asl
>+++ b/tools/firmware/hvmloader/acpi/dsdt.asl
>@@ -51,6 +51,7 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "Xen", "HVM",
>0)
>            UAR2, 1,
>            LTP1, 1,
>            HPET, 1,
>+           ADBG, 1,
>            Offset(4),
>            PMIN, 32,
>            PLEN, 32,
>diff --git a/tools/firmware/hvmloader/acpi/mk_dsdt.c
>b/tools/firmware/hvmloader/acpi/mk_dsdt.c
>index a4b693b..3f0ca74 100644
>--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
>+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
>@@ -347,14 +347,18 @@ int main(int argc, char **argv)
>             /* _SUN == dev */
>             stmt("Name", "_SUN, 0x%08x", slot >> 3);
>             push_block("Method", "_EJ0, 1");
>+            push_block("If", "LEqual( \\_SB.ADBG, ONE )");
>             stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
>             stmt("Store", "0x88, \\_GPE.DPT2");
>+            pop_block();
>             stmt("Store", "0x%02x, \\_GPE.PH%02X", /* eject */
>                  (slot & 1) ? 0x10 : 0x01, slot & ~1);
>             pop_block();
>             push_block("Method", "_STA, 0");
>+            push_block("If", "LEqual( \\_SB.ADBG, ONE )");
>             stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
>             stmt("Store", "0x89, \\_GPE.DPT2");
>+            pop_block();
>             if ( slot & 1 )
>           stmt("ShiftRight", "0x4, \\_GPE.PH%02X, Local1", slot & ~1);
>             else
>@@ -426,8 +430,10 @@ int main(int argc, char **argv)
>         stmt("Store", "PSTB, Local1"); /* XXX: Store (PSTB, SLT) ? */
>         stmt("And", "Local1, 0xff, SLT");
>         /* Debug */
>+        push_block("If", "LEqual( \\_SB.ADBG, ONE )");
>         stmt("Store", "SLT, DPT1");
>         stmt("Store", "EVT, DPT2");
>+        pop_block();
>         /* Decision tree */
>         decision_tree(0x00, 0x100, "SLT", pci_hotplug_notify);
>         pop_block();



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel