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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.12.13 at 12:09, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> --- 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)

Just like said on patch 1 - at least theoretically the safe mode
(i.e. independent on whether the guest actually looks at WAET)
is ACPI_WAET_RTC_NO_ACK set.

Furthermore I don't see why this definition needs to be in the
header file. It could easily remain in the single consuming one.

> @@ -196,8 +201,22 @@ 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() or strcmp()?

> +            waet->flags |= ACPI_WAET_RTC_NO_ACK;
> +        else
> +            waet->flags &= ~ACPI_WAET_RTC_NO_ACK;
> +    }
> +
>      set_checksum(waet, offsetof(struct acpi_header, checksum), 
> sizeof(*waet));
>  
> +    /* Inform Xen which RTC mode has been chosen */
> +    p.value = !!(waet->flags & ACPI_WAET_RTC_NO_ACK);

This either ought to be using the pseudo enumeration you added
to the public header, or have build time checks that 0 and 1 have
the expected meaning.

Jan


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