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

[Andrew Cooper <andrew.cooper3@citrix.com>]

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@citrix.com>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Tim Deegan <tim@xen.org>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> ---
>>  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.  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

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

~Andrew