Re: [PATCH 1/1] x86/acpi: Use FADT flags to determine the PMTMR width

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Sun, Jun 14, 2020 at 02:36:28PM +0000, Grzegorz Uriasz wrote:
> On some computers the bit width of the PM Timer as reported
> by ACPI is 32 bits when in fact the FADT flags report correctly
> that the timer is 24 bits wide. On affected machines such as the
> ASUS FX504GM and never gaming laptops this results in the inability
> to resume the machine from suspend. Without this patch suspend is
> broken on affected machines and even if a machine manages to resume
> correctly then the kernel time and xen timers are trashed.
> 
> Signed-off-by: Grzegorz Uriasz <gorbak25@xxxxxxxxx>
> Tested-by: Grzegorz Uriasz <gorbak25@xxxxxxxxx>

Thanks for the patch! I'm not sure it's required to add your
Tested-by, I usually assume a patch has been tested by it's author
unless told otherwise.

> ---
>  xen/arch/x86/acpi/boot.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/acpi/boot.c b/xen/arch/x86/acpi/boot.c
> index bcba52e232..2ad3eb4abc 100644
> --- a/xen/arch/x86/acpi/boot.c
> +++ b/xen/arch/x86/acpi/boot.c
> @@ -480,7 +480,10 @@ static int __init acpi_parse_fadt(struct 
> acpi_table_header *table)
>               if (fadt->xpm_timer_block.space_id ==
>                   ACPI_ADR_SPACE_SYSTEM_IO) {
>                       pmtmr_ioport = fadt->xpm_timer_block.address;
> -                     pmtmr_width = fadt->xpm_timer_block.bit_width;
> +                     if (fadt->flags & ACPI_FADT_32BIT_TIMER)
> +                             pmtmr_width = 32;
> +                     else
> +                             pmtmr_width = 24;

I think there's also a block below that you need to fix, when
xpm_timer_block is not set the data is fetched from pm_timer_block
instead, which AFAICT also needs to take ACPI_FADT_32BIT_TIMER into
account in order to use the correct size.

FWIW, I would set pmtmr_width only once after pmtmr_ioport has been
set, since regardless of whether the port is discovered using
xpm_timer_block or pm_timer_block the size will always be derived from
the flags field.

Thanks, Roger.