Re: [Xen-devel] [PATCH 2/2] hvmloader: Allow the toolstack to choose WAET optimisations

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-11-29 at 10:57 +0000, Andrew Cooper wrote:
> On 29/11/13 09:55, Ian Campbell wrote:
> > On Tue, 2013-11-26 at 20:39 +0000, Andrew Cooper wrote:
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >> CC: Keir Fraser <keir@xxxxxxx>
> >> CC: Jan Beulich <JBeulich@xxxxxxxx>
> >> CC: Tim Deegan <tim@xxxxxxx>
> >> CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> >> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> >> ---
> >>  tools/firmware/hvmloader/acpi/acpi2_0.h       |    4 ++++
> >>  tools/firmware/hvmloader/acpi/build.c         |   32 
> >> +++++++++++++++++++++++++
> >>  tools/firmware/hvmloader/acpi/static_tables.c |   12 +---------
> >>  3 files changed, 37 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/tools/firmware/hvmloader/acpi/acpi2_0.h 
> >> b/tools/firmware/hvmloader/acpi/acpi2_0.h
> >> index 7b22d80..bebe4e6 100644
> >> --- a/tools/firmware/hvmloader/acpi/acpi2_0.h
> >> +++ b/tools/firmware/hvmloader/acpi/acpi2_0.h
> >> @@ -303,6 +303,10 @@ struct acpi_20_waet {
> >>      struct acpi_header header;
> >>      uint32_t           flags;
> >>  };
> >> +#define ACPI_WAET_RTC_NO_ACK        (1<<0) /* RTC requires no int 
> >> acknowledge */
> >> +#define ACPI_WAET_TIMER_ONE_READ    (1<<1) /* PM timer requires only one 
> >> read */
> >> +
> >> +#define ACPI_WAET_DEFAULT_FLAGS (ACPI_WAET_TIMER_ONE_READ)
> >>  
> >>  /*
> >>   * Multiple APIC Flags.
> >> diff --git a/tools/firmware/hvmloader/acpi/build.c 
> >> b/tools/firmware/hvmloader/acpi/build.c
> >> index f1dd3f0..b9e209a 100644
> >> --- a/tools/firmware/hvmloader/acpi/build.c
> >> +++ b/tools/firmware/hvmloader/acpi/build.c
> >> @@ -23,6 +23,8 @@
> >>  #include "ssdt_pm.h"
> >>  #include "../config.h"
> >>  #include "../util.h"
> >> +#include "../hypercall.h"
> >> +#include <xen/hvm/params.h>
> >>  #include <xen/hvm/hvm_xs_strings.h>
> >>  
> >>  #define ACPI_MAX_SECONDARY_TABLES 16
> >> @@ -189,6 +191,9 @@ static struct acpi_20_hpet *construct_hpet(void)
> >>  static struct acpi_20_waet *construct_waet(void)
> >>  {
> >>      struct acpi_20_waet *waet;
> >> +    const char *s;
> >> +    struct xen_hvm_param p =
> >> +        { .domid = DOMID_SELF, .index = HVM_PARAM_RTC_MODE };
> >>  
> >>      waet = mem_alloc(sizeof(*waet), 16);
> >>      if (!waet) return NULL;
> >> @@ -196,8 +201,35 @@ static struct acpi_20_waet *construct_waet(void)
> >>      memcpy(waet, &Waet, sizeof(*waet));
> >>  
> >>      waet->header.length = sizeof(*waet);
> >> +
> >> +    s = xenstore_read("platform/waet-rtc-noack", NULL);
> >> +    if ( s )
> >> +    {
> >> +        if ( !strncmp(s, "1", 1) || !strncmp(s, "true", 4) )
> > Oh, and we generally seem to only care in hvmloader about "1" and "0"
> > rather than true/false wibble/woggle etc. Since this is an internal
> > protocol I think we don't need to be terribly accepting.
> >
> > You need to update docs/misc/xenstore-paths.markdown.
> >
> > Ian.
> >
> 
> Xapi works with "true/false" in these values.

It sounds like xapi needs fixing then.

>   In the past, all platform
> flags were not dealt with by hvmloader directly - they were all integers
> in HVMPARAMS or the start-info table.
> 
> From a "debugging issues with reference to xenstore" point of view, it
> is substantially easier to have booleans as true/false, to visually
> differentiate them from genuine integer 0/1's

Regardless, the xenstore convention is 0 and 1, mixing styles is worse
than either option.

Ian.

> In my copious free time, I was going to formally introduce
> "xenstore_read_bool()" in hvmloader.
> 
> ~Andrew



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