Investigating failed suspend/resume T61

[John Baldwin]

On Wednesday, May 28, 2014 6:14:34 pm Jung-uk Kim wrote:
> On 2014-05-28 17:29:35 -0400, John Baldwin wrote:
> > Err, I think it enables GPE1 as otherwise ACPICA assumes GPE1 has a
> > length of zero (and is thus invalid)?  Perhaps _PTS wants to frob
> > something that uses GPE1 that this fixes?
> 
> static void
> AcpiTbValidateFadt (
>     void)
> {
> ...
>     UINT8                   Length;
> ...
>     for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++)
>     {
> ...
>         Length = *ACPI_ADD_PTR (UINT8,
>                         &AcpiGbl_FADT, FadtInfoTable[i].Length);
> ...
> 
> Note the Length is read from the internal FADT and it is NOT a pointer.
> 
> ...
>         if (Address64->Address &&
>            (ACPI_MUL_8 (Length) <= ACPI_UINT8_MAX) &&
>            (Address64->BitWidth != ACPI_MUL_8 (Length)))
>         {
>             ACPI_BIOS_WARNING ((AE_INFO,
>                 "32/64X length mismatch in FADT/%s: %u/%u",
>                 Name, ACPI_MUL_8 (Length), Address64->BitWidth));
> +	    if (Length == 0)
> +	    {
> +		Length = ACPI_DIV_8 (Address64->BitWidth);
> +	    }
> 	}
> ...
>     }
> }
> 
> AFAICT, it does change anything in AcpiGbl_FADT.  If you really wanted
> to "patch" it, you had to do something like this:
> 
> +	    if (Length == 0)
> +	    {
> +		Length = ACPI_DIV_8 (Address64->BitWidth);
> +		*ACPI_ADD_PTR (UINT8,
> +		    &AcpiGbl_FADT, FadtInfoTable[i].Length) = Length;
> +	    }
> 
> However, it had to be done from AcpiEvGpeInitialize() in
> sys/contrib/dev/acpica/components/events/evgpeinit.c, IMHO.
> 
> ACPI_STATUS
> AcpiEvGpeInitialize (
>     void)
> {
> ...
>     if (AcpiGbl_FADT.Gpe1BlockLength &&
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                    HERE!!!
>         AcpiGbl_FADT.XGpe1Block.Address)
>     {
> ...
> 
> What did I miss?

Nothing, I've no idea how this could possibly have affected suspend/resume on 
Sean's laptop then.  Maybe it was just a coincidence.

-- 
John Baldwin