* [PATCH] tools/hvmloader: Don't perform AML hotplug debugging in production
@ 2015-03-30 14:20 Andrew Cooper
2015-03-30 15:22 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2015-03-30 14:20 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich
It is number of vmexits and a moderate quantity of qemu logging which can
safely be avoided when not specifically debugging a PCI hotplug issue.
As mk_dsdt is a build system tool, pass 'debug' as a command line parameter
rather than "hardcoding" it via the compilation of mk_dsdt itself.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
---
tools/firmware/hvmloader/acpi/Makefile | 4 ++--
tools/firmware/hvmloader/acpi/mk_dsdt.c | 29 ++++++++++++++++++++++-------
2 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/tools/firmware/hvmloader/acpi/Makefile b/tools/firmware/hvmloader/acpi/Makefile
index 8d91881..33b714a 100644
--- a/tools/firmware/hvmloader/acpi/Makefile
+++ b/tools/firmware/hvmloader/acpi/Makefile
@@ -36,12 +36,12 @@ mk_dsdt: mk_dsdt.c
dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
awk 'NR > 1 {print s} {s=$$0}' $< > $@
- ./mk_dsdt --dm-version qemu-xen >> $@
+ ./mk_dsdt --debug=$(debug) --dm-version qemu-xen >> $@
# NB. awk invocation is a portable alternative to 'head -n -1'
dsdt_%cpu.asl: dsdt.asl mk_dsdt
awk 'NR > 1 {print s} {s=$$0}' $< > $@
- ./mk_dsdt --maxcpu $* >> $@
+ ./mk_dsdt --debug=$(debug) --maxcpu $* >> $@
$(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl
iasl -vs -p $* -tc $*.asl
diff --git a/tools/firmware/hvmloader/acpi/mk_dsdt.c b/tools/firmware/hvmloader/acpi/mk_dsdt.c
index 1392525..d7736aa 100644
--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
@@ -4,9 +4,11 @@
#include <string.h>
#include <getopt.h>
#include <stdlib.h>
+#include <stdbool.h>
#include <xen/hvm/hvm_info_table.h>
static unsigned int indent_level;
+static bool debug = false;
typedef enum dm_version {
QEMU_XEN_TRADITIONAL,
@@ -83,6 +85,7 @@ static void decision_tree(
static struct option options[] = {
{ "maxcpu", 1, 0, 'c' },
{ "dm-version", 1, 0, 'q' },
+ { "debug", 1, 0, 'd' },
{ 0, 0, 0, 0 }
};
@@ -125,6 +128,10 @@ int main(int argc, char **argv)
return -1;
}
break;
+ case 'd':
+ if (*optarg == 'y')
+ debug = true;
+ break;
default:
return -1;
}
@@ -344,14 +351,20 @@ int main(int argc, char **argv)
/* _SUN == dev */
stmt("Name", "_SUN, 0x%08x", slot >> 3);
push_block("Method", "_EJ0, 1");
- stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
- stmt("Store", "0x88, \\_GPE.DPT2");
+ if ( debug )
+ {
+ stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
+ stmt("Store", "0x88, \\_GPE.DPT2");
+ }
stmt("Store", "0x%02x, \\_GPE.PH%02X", /* eject */
(slot & 1) ? 0x10 : 0x01, slot & ~1);
pop_block();
push_block("Method", "_STA, 0");
- stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
- stmt("Store", "0x89, \\_GPE.DPT2");
+ if (debug)
+ {
+ stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
+ stmt("Store", "0x89, \\_GPE.DPT2");
+ }
if ( slot & 1 )
stmt("ShiftRight", "0x4, \\_GPE.PH%02X, Local1", slot & ~1);
else
@@ -421,9 +434,11 @@ int main(int argc, char **argv)
stmt("And", "Local1, 0xf, EVT");
stmt("Store", "PSTB, Local1"); /* XXX: Store (PSTB, SLT) ? */
stmt("And", "Local1, 0xff, SLT");
- /* Debug */
- stmt("Store", "SLT, DPT1");
- stmt("Store", "EVT, DPT2");
+ if (debug)
+ {
+ stmt("Store", "SLT, DPT1");
+ stmt("Store", "EVT, DPT2");
+ }
/* Decision tree */
decision_tree(0x00, 0x100, "SLT", pci_hotplug_notify);
pop_block();
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] tools/hvmloader: Don't perform AML hotplug debugging in production
2015-03-30 14:20 [PATCH] tools/hvmloader: Don't perform AML hotplug debugging in production Andrew Cooper
@ 2015-03-30 15:22 ` Konrad Rzeszutek Wilk
2015-03-30 15:30 ` Andrew Cooper
0 siblings, 1 reply; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-30 15:22 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Keir Fraser, Jan Beulich, Xen-devel
On Mon, Mar 30, 2015 at 03:20:19PM +0100, Andrew Cooper wrote:
> It is number of vmexits and a moderate quantity of qemu logging which can
> safely be avoided when not specifically debugging a PCI hotplug issue.
Could we just make qemu-X not include this data when the we
run in production? And in the field if this needs to be diagnosed
we can just pass in an extra flag to QEMU and it would do it?
>
> As mk_dsdt is a build system tool, pass 'debug' as a command line parameter
> rather than "hardcoding" it via the compilation of mk_dsdt itself.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> ---
> tools/firmware/hvmloader/acpi/Makefile | 4 ++--
> tools/firmware/hvmloader/acpi/mk_dsdt.c | 29 ++++++++++++++++++++++-------
> 2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/tools/firmware/hvmloader/acpi/Makefile b/tools/firmware/hvmloader/acpi/Makefile
> index 8d91881..33b714a 100644
> --- a/tools/firmware/hvmloader/acpi/Makefile
> +++ b/tools/firmware/hvmloader/acpi/Makefile
> @@ -36,12 +36,12 @@ mk_dsdt: mk_dsdt.c
>
> dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
> awk 'NR > 1 {print s} {s=$$0}' $< > $@
> - ./mk_dsdt --dm-version qemu-xen >> $@
> + ./mk_dsdt --debug=$(debug) --dm-version qemu-xen >> $@
>
> # NB. awk invocation is a portable alternative to 'head -n -1'
> dsdt_%cpu.asl: dsdt.asl mk_dsdt
> awk 'NR > 1 {print s} {s=$$0}' $< > $@
> - ./mk_dsdt --maxcpu $* >> $@
> + ./mk_dsdt --debug=$(debug) --maxcpu $* >> $@
>
> $(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl
> iasl -vs -p $* -tc $*.asl
> diff --git a/tools/firmware/hvmloader/acpi/mk_dsdt.c b/tools/firmware/hvmloader/acpi/mk_dsdt.c
> index 1392525..d7736aa 100644
> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
> @@ -4,9 +4,11 @@
> #include <string.h>
> #include <getopt.h>
> #include <stdlib.h>
> +#include <stdbool.h>
> #include <xen/hvm/hvm_info_table.h>
>
> static unsigned int indent_level;
> +static bool debug = false;
>
> typedef enum dm_version {
> QEMU_XEN_TRADITIONAL,
> @@ -83,6 +85,7 @@ static void decision_tree(
> static struct option options[] = {
> { "maxcpu", 1, 0, 'c' },
> { "dm-version", 1, 0, 'q' },
> + { "debug", 1, 0, 'd' },
> { 0, 0, 0, 0 }
> };
>
> @@ -125,6 +128,10 @@ int main(int argc, char **argv)
> return -1;
> }
> break;
> + case 'd':
> + if (*optarg == 'y')
> + debug = true;
> + break;
> default:
> return -1;
> }
> @@ -344,14 +351,20 @@ int main(int argc, char **argv)
> /* _SUN == dev */
> stmt("Name", "_SUN, 0x%08x", slot >> 3);
> push_block("Method", "_EJ0, 1");
> - stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
> - stmt("Store", "0x88, \\_GPE.DPT2");
> + if ( debug )
> + {
> + stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
> + stmt("Store", "0x88, \\_GPE.DPT2");
> + }
> stmt("Store", "0x%02x, \\_GPE.PH%02X", /* eject */
> (slot & 1) ? 0x10 : 0x01, slot & ~1);
> pop_block();
> push_block("Method", "_STA, 0");
> - stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
> - stmt("Store", "0x89, \\_GPE.DPT2");
> + if (debug)
> + {
> + stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
> + stmt("Store", "0x89, \\_GPE.DPT2");
> + }
> if ( slot & 1 )
> stmt("ShiftRight", "0x4, \\_GPE.PH%02X, Local1", slot & ~1);
> else
> @@ -421,9 +434,11 @@ int main(int argc, char **argv)
> stmt("And", "Local1, 0xf, EVT");
> stmt("Store", "PSTB, Local1"); /* XXX: Store (PSTB, SLT) ? */
> stmt("And", "Local1, 0xff, SLT");
> - /* Debug */
> - stmt("Store", "SLT, DPT1");
> - stmt("Store", "EVT, DPT2");
> + if (debug)
> + {
> + stmt("Store", "SLT, DPT1");
> + stmt("Store", "EVT, DPT2");
> + }
> /* Decision tree */
> decision_tree(0x00, 0x100, "SLT", pci_hotplug_notify);
> pop_block();
> --
> 1.7.10.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] tools/hvmloader: Don't perform AML hotplug debugging in production
2015-03-30 15:22 ` Konrad Rzeszutek Wilk
@ 2015-03-30 15:30 ` Andrew Cooper
2015-03-30 15:47 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2015-03-30 15:30 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: Keir Fraser, Jan Beulich, Xen-devel
On 30/03/15 16:22, Konrad Rzeszutek Wilk wrote:
> On Mon, Mar 30, 2015 at 03:20:19PM +0100, Andrew Cooper wrote:
>> It is number of vmexits and a moderate quantity of qemu logging which can
>> safely be avoided when not specifically debugging a PCI hotplug issue.
> Could we just make qemu-X not include this data when the we
> run in production? And in the field if this needs to be diagnosed
> we can just pass in an extra flag to QEMU and it would do it?
What do we not include? This is data which gets bundled into hvmloader
itself.
A little while ago, I proposed a different solution:
http://lists.xen.org/archives/html/xen-devel/2014-01/msg01526.html
which allowed this to all be controlled at runtime. The important point
is to avoid the vmexits if not needed, rather than to simply ignore the
debugging they are giving.
~Andrew
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tools/hvmloader: Don't perform AML hotplug debugging in production
2015-03-30 15:30 ` Andrew Cooper
@ 2015-03-30 15:47 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-30 15:47 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Keir Fraser, Jan Beulich, Xen-devel
On Mon, Mar 30, 2015 at 04:30:01PM +0100, Andrew Cooper wrote:
> On 30/03/15 16:22, Konrad Rzeszutek Wilk wrote:
> > On Mon, Mar 30, 2015 at 03:20:19PM +0100, Andrew Cooper wrote:
> >> It is number of vmexits and a moderate quantity of qemu logging which can
> >> safely be avoided when not specifically debugging a PCI hotplug issue.
> > Could we just make qemu-X not include this data when the we
> > run in production? And in the field if this needs to be diagnosed
> > we can just pass in an extra flag to QEMU and it would do it?
>
> What do we not include? This is data which gets bundled into hvmloader
> itself.
>
> A little while ago, I proposed a different solution:
>
> http://lists.xen.org/archives/html/xen-devel/2014-01/msg01526.html
.. in which I proposed an path that you had choosen in this patch.
>
> which allowed this to all be controlled at runtime. The important point
> is to avoid the vmexits if not needed, rather than to simply ignore the
> debugging they are giving.
<nods>
I know I've been using this in the field (so no need to deploy a
new build) - but then eventually I had to build an custom Xen binary.
Anyhow thank you for refreshing my memory on this patchset.
and in regards to the patch:
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> ~Andrew
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-03-30 15:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-30 14:20 [PATCH] tools/hvmloader: Don't perform AML hotplug debugging in production Andrew Cooper
2015-03-30 15:22 ` Konrad Rzeszutek Wilk
2015-03-30 15:30 ` Andrew Cooper
2015-03-30 15:47 ` Konrad Rzeszutek Wilk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.