On Mon, Jun 22, 2020 at 10:54:00AM +0200, Roger Pau Monné wrote:
> On Fri, Jun 19, 2020 at 10:00:16PM +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 <gorba...@gmail.com>
> > ---
> > Cc: Jan Beulich <jbeul...@suse.com>
> > Cc: Andrew Cooper <andrew.coop...@citrix.com>
> > Cc: Wei Liu <w...@xen.org>
> > Cc: "Roger Pau Monné" <roger....@citrix.com>
> > Cc: marma...@invisiblethingslab.com
> > Cc: cont...@puzio.waw.pl
> > Cc: ja...@bartmin.ski
> > Cc: j.nowa...@student.uw.edu.pl
> >
> > Changes in v2:
> > - Check pm timer access width
> > - Proper timer width is set when xpm block is not present
> > - Cleanup timer initialization
> >
> > Changes in v3:
> > - Check pm timer bit offset
> > - Warn about acpi firmware bugs
> > - Drop int cast in init_pmtimer
> > - Merge if's in init_pmtimer
> > ---
> > xen/arch/x86/acpi/boot.c | 19 +++++++++++++++----
> > xen/arch/x86/time.c | 12 ++++--------
> > xen/include/acpi/acmacros.h | 8 ++++++++
> > 3 files changed, 27 insertions(+), 12 deletions(-)
> >
> > diff --git a/xen/arch/x86/acpi/boot.c b/xen/arch/x86/acpi/boot.c
> > index bcba52e232..24fd236eb5 100644
> > --- a/xen/arch/x86/acpi/boot.c
> > +++ b/xen/arch/x86/acpi/boot.c
> > @@ -475,10 +475,17 @@ static int __init acpi_parse_fadt(struct
> > acpi_table_header *table)
> >
> > #ifdef CONFIG_X86_PM_TIMER
> > /* detect the location of the ACPI PM Timer */
> > - if (fadt->header.revision >= FADT2_REVISION_ID) {
> > + if (fadt->header.revision >= FADT2_REVISION_ID &&
> > + fadt->xpm_timer_block.space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> > /* FADT rev. 2 */
> > - if (fadt->xpm_timer_block.space_id ==
> > - ACPI_ADR_SPACE_SYSTEM_IO) {
> > + if (fadt->xpm_timer_block.access_width != 0 &&
> > + ACPI_ACCESS_BIT_WIDTH(fadt->xpm_timer_block.access_width)
> > != 32)
> > + printk(KERN_WARNING PREFIX "PM-Timer has invalid access
> > width(%u)\n",
> > + fadt->xpm_timer_block.access_width);
> > + else if (fadt->xpm_timer_block.bit_offset != 0)
>
> This should be a plain if instead of an else if, it's possible the
> block has both the access_width and the bit_offset wrong.
My bad, realized this avoids setting pmtmr_ioport.
Roger.
