RE: [PATCH 5/6] ACPI: set waking vector in inactive facs table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>> Set wake vector using FACS pointed to by X_FIRMWARE_CTRL
>> If (FIRMWARE_CTRL is non-zero) AND (FIRMWARE_CTRL != X_FIRMWARE_CTRL)
>>     Set wake vector using FACS pointed to by FIRMWARE_CTRL
>>
>Yes. this could fix the problem too, but much more complex.


I don't have the patch in front of me, but the pseudo-code above is simply setting the vector in both FACS tables, albeit with some checking. Basically, "do what ACPI says to do" then "if FADT is screwed up, use second FACS".

The global lock issue is serious and must be resolved. I guess if all else fails, we could just use both global locks, just like we use both wake vectors.

This is getting very nasty.

Bob


>-----Original Message-----
>From: Zhang, Rui
>Sent: Thursday, October 30, 2008 11:07 PM
>To: Moore, Robert
>Cc: linux-acpi; Lin, Ming M; Len Brown
>Subject: RE: [PATCH 5/6] ACPI: set waking vector in inactive facs table
>
>On Thu, 2008-10-16 at 03:41 +0800, Moore, Robert wrote:
>> I'll go ahead and implement the following:
>>
>> http://www.acpica.org/bugzilla/show_bug.cgi?id=731
>>
>>
>> It looks like we have an issue on some machines where both the
>FIRMWARE_CTRL and X_FIRMWARE_CTRL fields in the FADT are valid, but contain
>different addresses. In this case, we don't really know which one is the
>real FACS.
>>
>> Currently, if the X_FIRMWARE_CTRL field is valid, we use it exclusively
>(as per the ACPI spec.) This doesn’t work when the “real” FACS is actually
>the one pointed to by the FIRMWARE_CTRL field.
>>
>> The proposed change for the case where both FACS pointers are valid is to
>write the firmware waking vector value to both FACS tables.
>>
>yes, that's the problem I want to fix in this patch.
>another problem is the global lock.
>we may get an invalid global lock if the wrong FACS is used, right?
>but currently we don't know which FACS is actually used by BIOS/Windows,
>this patch could only fix the waking vector issue,
>
>> We would have something like this:
>>
>> Set wake vector using FACS pointed to by X_FIRMWARE_CTRL
>> If (FIRMWARE_CTRL is non-zero) AND (FIRMWARE_CTRL != X_FIRMWARE_CTRL)
>>     Set wake vector using FACS pointed to by FIRMWARE_CTRL
>>
>Yes. this could fix the problem too, but much more complex.
>As we need to add both of the FACS tables to the
>acpi_gbl_root_table_list, while the index of FACS table is hard coded
>there, i.e. ACPI_TABLE_INDEX_FACS.
>
>Bob, do you think we should fix the multiple FACS issue in some other
>way, or just follow the proposal in this patch?
>
>thanks,
>rui
>>
>>
>> >-----Original Message-----
>> >From: Zhang, Rui
>> >Sent: Monday, October 13, 2008 6:09 PM
>> >To: Moore, Robert
>> >Cc: Len Brown; linux-acpi; Rafael J. Wysocki
>> >Subject: RE: [PATCH 5/6] ACPI: set waking vector in inactive facs table
>> >
>> >On Mon, 2008-10-13 at 11:46 -0600, Moore, Robert wrote:
>> >> So, is this the bottom line of the hwsleep patch:
>> >>
>> >> If there is a second FACS in the system (pointed to by the unused
>RSDT),
>> >then set the firmware waking vectors in BOTH FACS tables?
>> >yes.
>> >
>> >there is another way to fix this issue.
>> >as FADT1 exports two different FACS tables, setting the waking vectors
>> >in both of them (pointed to by fadt->facs and fadt->Xfadt) also works.
>> >
>> >I don't know which way is better.
>> >
>> >thanks,
>> >rui
>> >>
>> >>
>> >>
>> >> >-----Original Message-----
>> >> >From: Zhang, Rui
>> >> >Sent: Monday, October 13, 2008 1:11 AM
>> >> >To: Len Brown
>> >> >Cc: linux-acpi; Moore, Robert; Rafael J. Wysocki; Zhang, Rui
>> >> >Subject: [PATCH 5/6] ACPI: set waking vector in inactive facs table
>> >> >
>> >> >
>> >> >RSDT and XSDT may export different FACS tables.
>> >> >Some of the BIOSes only checks one of them when resuming.
>> >> >And Linux reboots instead of resuming because BIOS can't find
>> >> >the waking vector.
>> >> >For example,
>> >> >I have a platform which suspends well, but always reboots instead of
>> >> >resuming when pressing the power button.
>> >> >I found that there are two FACS tables on this platform,
>> >> >XSDT-->FADT1-->Xfacs-->FACS1
>> >> >        |----->facs-|
>> >> >                    |->FACS2
>> >> >RSDT-->FADT2-->facs-|
>> >> >Linux uses XSDT on this platform and sets the waking vector in FACS1
>> >> >when suspending.
>> >> >But it seems that the BIOS only cares for the waking vector in FACS2,
>> >> >thus it reboots when resuming because the waking vector is not set at
>> >all.
>> >> >
>> >> >Set the waking vector in inactive FACS table in this patch.
>> >> >
>> >> >Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx>
>> >> >---
>> >> > drivers/acpi/hardware/hwsleep.c |   20 ++++++++++++++++++++
>> >> > drivers/acpi/tables/tbfadt.c    |    6 ++++--
>> >> > 2 files changed, 24 insertions(+), 2 deletions(-)
>> >> >
>> >> >Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
>> >> >===================================================================
>> >> >--- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
>> >> >+++ linux-2.6/drivers/acpi/hardware/hwsleep.c
>> >> >@@ -65,6 +65,7 @@ acpi_set_firmware_waking_vector(acpi_phy
>> >> > {
>> >> >        struct acpi_table_facs *facs;
>> >> >        acpi_status status;
>> >> >+       u32 table_index;
>> >> >
>> >> >        ACPI_FUNCTION_TRACE(acpi_set_firmware_waking_vector);
>> >> >
>> >> >@@ -92,6 +93,25 @@ acpi_set_firmware_waking_vector(acpi_phy
>> >> >                facs->xfirmware_waking_vector = physical_address;
>> >> >        }
>> >> >
>> >> >+       /* set the firmware vector in the inactive facs table */
>> >> >+       for (table_index = 0, facs = NULL; ; table_index++) {
>> >> >+               status =
>acpi_get_inactive_table_by_index(table_index,
>> >> >+                               ACPI_CAST_INDIRECT_PTR(struct
>> >> >acpi_table_header,
>> >> >+                                                       &facs));
>> >> >+               if (status == AE_TABLE_NOT_SUPPORTED)
>> >> >+                       continue;
>> >> >+               if (ACPI_FAILURE(status))
>> >> >+                       break;
>> >> >+
>> >> >+               if (!ACPI_COMPARE_NAME(&facs->signature,
>ACPI_SIG_FACS))
>> >> >+                       continue;
>> >> >+
>> >> >+               if (facs->version >= 1)
>> >> >+                       facs->xfirmware_waking_vector = 0;
>> >> >+               facs->firmware_waking_vector = (u32)physical_address;
>> >> >+               break;
>> >> >+       }
>> >> >+
>> >> >        return_ACPI_STATUS(AE_OK);
>> >> > }
>> >> >
>> >> >Index: linux-2.6/drivers/acpi/tables/tbfadt.c
>> >> >===================================================================
>> >> >--- linux-2.6.orig/drivers/acpi/tables/tbfadt.c
>> >> >+++ linux-2.6/drivers/acpi/tables/tbfadt.c
>> >> >@@ -290,11 +290,13 @@ static void acpi_tb_convert_fadt(void)
>> >> >
>> >> >        if (!acpi_gbl_FADT.Xfacs) {
>> >> >                acpi_gbl_FADT.Xfacs = (u64) acpi_gbl_FADT.facs;
>> >> >-       }
>> >> >+       } else if (acpi_gbl_FADT.Xfacs != acpi_gbl_FADT.facs)
>> >> >+               ACPI_WARNING((AE_INFO, "32 bit FACS and 64 bit FACS
>not
>> >> >match\n"));
>> >> >
>> >> >        if (!acpi_gbl_FADT.Xdsdt) {
>> >> >                acpi_gbl_FADT.Xdsdt = (u64) acpi_gbl_FADT.dsdt;
>> >> >-       }
>> >> >+       } else if (acpi_gbl_FADT.Xdsdt != acpi_gbl_FADT.dsdt)
>> >> >+               ACPI_WARNING((AE_INFO, "32 bit DSDT and 64 bit DSDT
>not
>> >> >match\n"));
>> >> >
>> >> >        /*
>> >> >         * For ACPI 1.0 FADTs (revision 1 or 2), ensure that reserved
>> >> >fields which
>> >> >
>> >>
>>

��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f


[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux