Re: [PATCH] tools/hvmloader: Don't perform AML hotplug debugging in production

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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