| Message ID | 1379066741-23689-1-git-send-email-hanjun.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
NOT_IMPLEMENTED refers to the software only; therefore AE_SUPPORT should be returned. Otherwise, seems like this may be a good idea. > -----Original Message----- > From: Hanjun Guo [mailto:hanjun.guo@linaro.org] > Sent: Friday, September 13, 2013 3:06 AM > To: Rafael J. Wysocki; Len Brown > Cc: Zheng, Lv; Moore, Robert; linux-acpi@vger.kernel.org; > patches@linaro.org; linaro-kernel@lists.linaro.org; linaro- > acpi@lists.linaro.org; Hanjun Guo > Subject: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to prevent > accessing PM registers > > On hardware reduced platforms, there is no support for the following: > - PM Event and Control registers > - SCI interrupt (and handler) > - Fixed Events > - General Purpose Events (GPEs) > - Global Lock > - ACPI PM timer > - FACS table (Waking vectors and Global Lock) > > acpi_gbl_reduced_hardware is a global flag and can be used to prevent > misbehavior on hardware reduced platforms, and this flag is initialized at > the very beginning of the system boot when FADT is parsed, this flag is > set to 1 when the FACP Hardware Reduced flag is set, so we can use it to > prevent accessing registers which do not exist on reduced hardware > platform. > > but when FACP Hardware Reduced flag is set, ACPI drivers will still access > some registers directly which do not exist on reduced hardware platform > and cause boot failure: > > [ 9.024370] Unable to handle kernel paging request at virtual address > ffffffbffbe00001 > [ 9.024423] pgd = ffffffc00007d000 > [ 9.024468] [ffffffbffbe00001] *pgd=000000008007f003, > *pmd=0000000000000000 > [ 9.024538] Internal error: Oops: 96000006 [#1] SMP > [ 9.024577] Modules linked in: > [ 9.024627] CPU: 0 Not tainted (3.9.0+ #3) > [ 9.024691] PC is at acpi_os_read_port+0x58/0xc8 > [ 9.024753] LR is at acpi_hw_read_port+0x4c/0xc4 > [ 9.024810] pc : [<ffffffc0002810cc>] lr : [<ffffffc0002a4fc4>] pstate: > 600003c5 > .... > [ 9.031956] Call trace: > [ 9.032021] [<ffffffc0002810cc>] acpi_os_read_port+0x58/0xc8 > [ 9.032094] [<ffffffc0002a4fc4>] acpi_hw_read_port+0x4c/0xc4 > [ 9.032165] [<ffffffc0002a4198>] acpi_hw_read+0x6c/0x100 > [ 9.032237] [<ffffffc0002a4250>] acpi_hw_read_multiple+0x24/0x70 > [ 9.032312] [<ffffffc0002a457c>] acpi_hw_register_read+0xa8/0x164 > [ 9.032386] [<ffffffc0002a52dc>] acpi_write_bit_register+0x9c/0x194 > [ 9.032472] [<ffffffc0002bffb4>] > acpi_processor_get_power_info+0x6c4/0x748 > [ 9.032550] [<ffffffc000565220>] acpi_processor_power_init+0xac/0x138 > [ 9.032630] [<ffffffc0003f3c00>] acpi_processor_start+0x48/0x138 > [ 9.032704] [<ffffffc000565118>] acpi_processor_add+0x43c/0x498 > [ 9.032784] [<ffffffc000283a74>] acpi_device_probe+0x3c/0x198 > [ 9.032862] [<ffffffc0002ef0fc>] driver_probe_device+0x90/0x348 > [ 9.032940] [<ffffffc0002ef450>] __driver_attach+0x9c/0xa0 > [ 9.033015] [<ffffffc0002ed40c>] bus_for_each_dev+0x4c/0x8c > [ 9.033090] [<ffffffc0002eeb98>] driver_attach+0x20/0x28 > [ 9.033166] [<ffffffc0002ee6e0>] bus_add_driver+0xfc/0x254 > [ 9.033244] [<ffffffc0002ef8b0>] driver_register+0x6c/0x184 > [ 9.033325] [<ffffffc000284894>] acpi_bus_register_driver+0x34/0x44 > [ 9.033400] [<ffffffc000559c60>] acpi_processor_init+0x28/0x40 > [ 9.033469] [<ffffffc000081438>] do_one_initcall+0x100/0x138 > [ 9.033544] [<ffffffc0005448c8>] kernel_init_freeable+0x128/0x1cc > [ 9.033620] [<ffffffc0003f2f90>] kernel_init+0x10/0xcc > [ 9.033700] Code: d2bf7c02 f2dff7e2 f2ffffe2 8b020000 (79400000) > [ 9.033843] ---[ end trace 64376967e6bc20a9 ]--- > [ 9.033995] Kernel panic - not syncing: Attempted to kill init! > exitcode=0x0000000b > > So acpi_gbl_reduced_hardware should be used to check if reduced hardware > or not, if it is reduced hardware, just return with some error and do not > access the no-existent registers. > > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > --- > drivers/acpi/acpica/hwregs.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/acpi/acpica/hwregs.c b/drivers/acpi/acpica/hwregs.c > index 8d2e866..4c270c3 100644 > --- a/drivers/acpi/acpica/hwregs.c > +++ b/drivers/acpi/acpica/hwregs.c > @@ -265,6 +265,11 @@ acpi_status acpi_hw_clear_acpi_status(void) > > ACPI_FUNCTION_TRACE(hw_clear_acpi_status); > > + /* If Hardware Reduced flag is set, there are no GPEs */ > + if (acpi_gbl_reduced_hardware) { > + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); > + } > + > ACPI_DEBUG_PRINT((ACPI_DB_IO, "About to write %04X to %8.8X%8.8X\n", > ACPI_BITMASK_ALL_FIXED_STATUS, > ACPI_FORMAT_UINT64(acpi_gbl_xpm1a_status.address))); > @@ -305,6 +310,10 @@ struct acpi_bit_register_info > *acpi_hw_get_bit_register_info(u32 register_id) { > ACPI_FUNCTION_ENTRY(); > > + if (acpi_gbl_reduced_hardware) { > + return (NULL); > + } > + > if (register_id > ACPI_BITREG_MAX) { > ACPI_ERROR((AE_INFO, "Invalid BitRegister ID: 0x%X", > register_id)); > @@ -337,6 +346,11 @@ acpi_status acpi_hw_write_pm1_control(u32 > pm1a_control, u32 pm1b_control) > > ACPI_FUNCTION_TRACE(hw_write_pm1_control); > > + /* If Hardware Reduced flag is set, there are no PM Control > registers */ > + if (acpi_gbl_reduced_hardware) { > + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); > + } > + > status = > acpi_hw_write(pm1a_control, &acpi_gbl_FADT.xpm1a_control_block); > if (ACPI_FAILURE(status)) { > @@ -370,6 +384,11 @@ acpi_status acpi_hw_register_read(u32 register_id, > u32 *return_value) > > ACPI_FUNCTION_TRACE(hw_register_read); > > + /* If Hardware Reduced flag is set, there are no PM Control > registers */ > + if (acpi_gbl_reduced_hardware) { > + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); > + } > + > switch (register_id) { > case ACPI_REGISTER_PM1_STATUS: /* PM1 A/B: 16-bit access each > */ > > @@ -465,6 +484,11 @@ acpi_status acpi_hw_register_write(u32 register_id, > u32 value) > > ACPI_FUNCTION_TRACE(hw_register_write); > > + /* If Hardware Reduced flag is set, there are no PM Control > registers */ > + if (acpi_gbl_reduced_hardware) { > + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); > + } > + > switch (register_id) { > case ACPI_REGISTER_PM1_STATUS: /* PM1 A/B: 16-bit access each > */ > /* > -- > 1.7.9.5
> -----Original Message----- > From: Moore, Robert > Sent: Friday, September 13, 2013 6:08 AM > To: Hanjun Guo; Rafael J. Wysocki; Len Brown > Cc: Zheng, Lv; linux-acpi@vger.kernel.org; patches@linaro.org; linaro- > kernel@lists.linaro.org; linaro-acpi@lists.linaro.org > Subject: RE: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to > prevent accessing PM registers > > NOT_IMPLEMENTED refers to the software only; therefore AE_SUPPORT should > be returned. > > Otherwise, seems like this may be a good idea. On the other hand, could one not argue that the host OS should darn well know that it is executing on a hardware-reduced platform and not call these functions in the first place? Bob > > > > -----Original Message----- > > From: Hanjun Guo [mailto:hanjun.guo@linaro.org] > > Sent: Friday, September 13, 2013 3:06 AM > > To: Rafael J. Wysocki; Len Brown > > Cc: Zheng, Lv; Moore, Robert; linux-acpi@vger.kernel.org; > > patches@linaro.org; linaro-kernel@lists.linaro.org; linaro- > > acpi@lists.linaro.org; Hanjun Guo > > Subject: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to > > prevent accessing PM registers > > > > On hardware reduced platforms, there is no support for the following: > > - PM Event and Control registers > > - SCI interrupt (and handler) > > - Fixed Events > > - General Purpose Events (GPEs) > > - Global Lock > > - ACPI PM timer > > - FACS table (Waking vectors and Global Lock) > > > > acpi_gbl_reduced_hardware is a global flag and can be used to prevent > > misbehavior on hardware reduced platforms, and this flag is > > initialized at the very beginning of the system boot when FADT is > > parsed, this flag is set to 1 when the FACP Hardware Reduced flag is > > set, so we can use it to prevent accessing registers which do not > > exist on reduced hardware platform. > > > > but when FACP Hardware Reduced flag is set, ACPI drivers will still > > access some registers directly which do not exist on reduced hardware > > platform and cause boot failure: > > > > [ 9.024370] Unable to handle kernel paging request at virtual address > > ffffffbffbe00001 > > [ 9.024423] pgd = ffffffc00007d000 > > [ 9.024468] [ffffffbffbe00001] *pgd=000000008007f003, > > *pmd=0000000000000000 > > [ 9.024538] Internal error: Oops: 96000006 [#1] SMP > > [ 9.024577] Modules linked in: > > [ 9.024627] CPU: 0 Not tainted (3.9.0+ #3) > > [ 9.024691] PC is at acpi_os_read_port+0x58/0xc8 > > [ 9.024753] LR is at acpi_hw_read_port+0x4c/0xc4 > > [ 9.024810] pc : [<ffffffc0002810cc>] lr : [<ffffffc0002a4fc4>] > pstate: > > 600003c5 > > .... > > [ 9.031956] Call trace: > > [ 9.032021] [<ffffffc0002810cc>] acpi_os_read_port+0x58/0xc8 > > [ 9.032094] [<ffffffc0002a4fc4>] acpi_hw_read_port+0x4c/0xc4 > > [ 9.032165] [<ffffffc0002a4198>] acpi_hw_read+0x6c/0x100 > > [ 9.032237] [<ffffffc0002a4250>] acpi_hw_read_multiple+0x24/0x70 > > [ 9.032312] [<ffffffc0002a457c>] acpi_hw_register_read+0xa8/0x164 > > [ 9.032386] [<ffffffc0002a52dc>] acpi_write_bit_register+0x9c/0x194 > > [ 9.032472] [<ffffffc0002bffb4>] > > acpi_processor_get_power_info+0x6c4/0x748 > > [ 9.032550] [<ffffffc000565220>] acpi_processor_power_init+0xac/0x138 > > [ 9.032630] [<ffffffc0003f3c00>] acpi_processor_start+0x48/0x138 > > [ 9.032704] [<ffffffc000565118>] acpi_processor_add+0x43c/0x498 > > [ 9.032784] [<ffffffc000283a74>] acpi_device_probe+0x3c/0x198 > > [ 9.032862] [<ffffffc0002ef0fc>] driver_probe_device+0x90/0x348 > > [ 9.032940] [<ffffffc0002ef450>] __driver_attach+0x9c/0xa0 > > [ 9.033015] [<ffffffc0002ed40c>] bus_for_each_dev+0x4c/0x8c > > [ 9.033090] [<ffffffc0002eeb98>] driver_attach+0x20/0x28 > > [ 9.033166] [<ffffffc0002ee6e0>] bus_add_driver+0xfc/0x254 > > [ 9.033244] [<ffffffc0002ef8b0>] driver_register+0x6c/0x184 > > [ 9.033325] [<ffffffc000284894>] acpi_bus_register_driver+0x34/0x44 > > [ 9.033400] [<ffffffc000559c60>] acpi_processor_init+0x28/0x40 > > [ 9.033469] [<ffffffc000081438>] do_one_initcall+0x100/0x138 > > [ 9.033544] [<ffffffc0005448c8>] kernel_init_freeable+0x128/0x1cc > > [ 9.033620] [<ffffffc0003f2f90>] kernel_init+0x10/0xcc > > [ 9.033700] Code: d2bf7c02 f2dff7e2 f2ffffe2 8b020000 (79400000) > > [ 9.033843] ---[ end trace 64376967e6bc20a9 ]--- > > [ 9.033995] Kernel panic - not syncing: Attempted to kill init! > > exitcode=0x0000000b > > > > So acpi_gbl_reduced_hardware should be used to check if reduced > > hardware or not, if it is reduced hardware, just return with some > > error and do not access the no-existent registers. > > > > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > > --- > > drivers/acpi/acpica/hwregs.c | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/drivers/acpi/acpica/hwregs.c > > b/drivers/acpi/acpica/hwregs.c index 8d2e866..4c270c3 100644 > > --- a/drivers/acpi/acpica/hwregs.c > > +++ b/drivers/acpi/acpica/hwregs.c > > @@ -265,6 +265,11 @@ acpi_status acpi_hw_clear_acpi_status(void) > > > > ACPI_FUNCTION_TRACE(hw_clear_acpi_status); > > > > + /* If Hardware Reduced flag is set, there are no GPEs */ > > + if (acpi_gbl_reduced_hardware) { > > + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); > > + } > > + > > ACPI_DEBUG_PRINT((ACPI_DB_IO, "About to write %04X to %8.8X%8.8X\n", > > ACPI_BITMASK_ALL_FIXED_STATUS, > > ACPI_FORMAT_UINT64(acpi_gbl_xpm1a_status.address))); > > @@ -305,6 +310,10 @@ struct acpi_bit_register_info > > *acpi_hw_get_bit_register_info(u32 register_id) { > > ACPI_FUNCTION_ENTRY(); > > > > + if (acpi_gbl_reduced_hardware) { > > + return (NULL); > > + } > > + > > if (register_id > ACPI_BITREG_MAX) { > > ACPI_ERROR((AE_INFO, "Invalid BitRegister ID: 0x%X", > > register_id)); > > @@ -337,6 +346,11 @@ acpi_status acpi_hw_write_pm1_control(u32 > > pm1a_control, u32 pm1b_control) > > > > ACPI_FUNCTION_TRACE(hw_write_pm1_control); > > > > + /* If Hardware Reduced flag is set, there are no PM Control > > registers */ > > + if (acpi_gbl_reduced_hardware) { > > + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); > > + } > > + > > status = > > acpi_hw_write(pm1a_control, &acpi_gbl_FADT.xpm1a_control_block); > > if (ACPI_FAILURE(status)) { > > @@ -370,6 +384,11 @@ acpi_status acpi_hw_register_read(u32 > > register_id, > > u32 *return_value) > > > > ACPI_FUNCTION_TRACE(hw_register_read); > > > > + /* If Hardware Reduced flag is set, there are no PM Control > > registers */ > > + if (acpi_gbl_reduced_hardware) { > > + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); > > + } > > + > > switch (register_id) { > > case ACPI_REGISTER_PM1_STATUS: /* PM1 A/B: 16-bit access each > > */ > > > > @@ -465,6 +484,11 @@ acpi_status acpi_hw_register_write(u32 > > register_id, > > u32 value) > > > > ACPI_FUNCTION_TRACE(hw_register_write); > > > > + /* If Hardware Reduced flag is set, there are no PM Control > > registers */ > > + if (acpi_gbl_reduced_hardware) { > > + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); > > + } > > + > > switch (register_id) { > > case ACPI_REGISTER_PM1_STATUS: /* PM1 A/B: 16-bit access each > > */ > > /* > > -- > > 1.7.9.5
On 2013-9-14 4:16, Moore, Robert wrote: > > >> -----Original Message----- >> From: Moore, Robert >> Sent: Friday, September 13, 2013 6:08 AM >> To: Hanjun Guo; Rafael J. Wysocki; Len Brown >> Cc: Zheng, Lv; linux-acpi@vger.kernel.org; patches@linaro.org; linaro- >> kernel@lists.linaro.org; linaro-acpi@lists.linaro.org >> Subject: RE: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to >> prevent accessing PM registers >> >> NOT_IMPLEMENTED refers to the software only; therefore AE_SUPPORT should >> be returned. >> >> Otherwise, seems like this may be a good idea. > > > On the other hand, could one not argue that the host OS should darn well know that > it is executing on a hardware-reduced platform and not call these functions in the > first place? Well, there is a macro ACPI_REDUCED_HARDWARE and used to switch off the compiling of these functions on hardware-reduced platform, if ACPI_REDUCED_HARDWARE is TRUE, those functions will never be called. But is that reasonable? When somebody want to use the ACPI code on hardware-reduced platform, they should introduce a patch just like: - #define ACPI_REDUCED_HARDWARE FALSE + #define ACPI_REDUCED_HARDWARE TRUE in include/acpi/acconfig.h and this patch does not exist in upstream, and it will never be accepted by upstream too, it is weird. so we should proposal another way. I thought about introducing a kernel config such as CONFIG_ACPI_REDUCED_HARDWARE to enable ACPI_REDUCED_HARDWARE, but it is just the same as a macro, and I drop it. So, this is a problem should be solved for hardware-reduced platform, use falg acpi_gbl_reduced_hardware seems a good solution. Thanks Hanjun > > Bob
+ #define ACPI_REDUCED_HARDWARE TRUE The intent of this feature is of course, to remove all code that is not needed -- specifically for hardware-reduced machines where the size of the kernel is important. On a larger machine, the hardware-reduced flag should be sufficient. However, I would think that the host OS would look at this flag and realize that it should not be doing certain ACPI hardware-related things up front, rather than later when it finds out that a write to some ACPI hardware fails because the hardware isn't there. This is not to say that it is probably a good thing to return an error from the ACPI hardware code in the hardware-reduced case. Bob > -----Original Message----- > From: Hanjun Guo [mailto:hanjun.guo@linaro.org] > Sent: Sunday, September 15, 2013 7:40 PM > To: Moore, Robert > Cc: 'Rafael J. Wysocki'; 'Len Brown'; Box, David E; Zheng, Lv; 'linux- > acpi@vger.kernel.org'; 'patches@linaro.org'; 'linaro- > kernel@lists.linaro.org'; 'linaro-acpi@lists.linaro.org' > Subject: Re: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to > prevent accessing PM registers > > On 2013-9-14 4:16, Moore, Robert wrote: > > > > > >> -----Original Message----- > >> From: Moore, Robert > >> Sent: Friday, September 13, 2013 6:08 AM > >> To: Hanjun Guo; Rafael J. Wysocki; Len Brown > >> Cc: Zheng, Lv; linux-acpi@vger.kernel.org; patches@linaro.org; > >> linaro- kernel@lists.linaro.org; linaro-acpi@lists.linaro.org > >> Subject: RE: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to > >> prevent accessing PM registers > >> > >> NOT_IMPLEMENTED refers to the software only; therefore AE_SUPPORT > >> should be returned. > >> > >> Otherwise, seems like this may be a good idea. > > > > > > On the other hand, could one not argue that the host OS should darn > > well know that it is executing on a hardware-reduced platform and not > > call these functions in the first place? > > Well, there is a macro ACPI_REDUCED_HARDWARE and used to switch off the > compiling of these functions on hardware-reduced platform, if > ACPI_REDUCED_HARDWARE is TRUE, those functions will never be called. > > But is that reasonable? When somebody want to use the ACPI code on > hardware-reduced platform, they should introduce a patch just like: > > - #define ACPI_REDUCED_HARDWARE FALSE > + #define ACPI_REDUCED_HARDWARE TRUE > in include/acpi/acconfig.h > > and this patch does not exist in upstream, and it will never be accepted > by upstream too, it is weird. so we should proposal another way. > > I thought about introducing a kernel config such as > CONFIG_ACPI_REDUCED_HARDWARE to enable ACPI_REDUCED_HARDWARE, but it is > just the same as a macro, and I drop it. > > So, this is a problem should be solved for hardware-reduced platform, use > falg acpi_gbl_reduced_hardware seems a good solution. > > Thanks > Hanjun > > > > > Bob
On 2013-9-17 1:26, Moore, Robert wrote: > + #define ACPI_REDUCED_HARDWARE TRUE > > The intent of this feature is of course, to remove all code that is not needed -- specifically for hardware-reduced machines where the size of the kernel is important. > > On a larger machine, the hardware-reduced flag should be sufficient. However, I would think that the host OS would look at this flag and realize that it should not be doing certain ACPI hardware-related things up front, rather than later when it finds out that a write to some ACPI hardware fails because the hardware isn't there. Do you mean we should change the ACPI device driver instead of changing the ACPICA code? that would be a hard job, because hardware ACPI is used everywhere. Thanks Hanjun > > This is not to say that it is probably a good thing to return an error from the ACPI hardware code in the hardware-reduced case. > > Bob >
> -----Original Message----- > From: Hanjun Guo [mailto:hanjun.guo@linaro.org] > Sent: Wednesday, September 18, 2013 2:32 AM > To: Moore, Robert > Cc: 'Rafael J. Wysocki'; 'Len Brown'; Box, David E; Zheng, Lv; 'linux- > acpi@vger.kernel.org'; 'patches@linaro.org'; 'linaro- > kernel@lists.linaro.org'; 'linaro-acpi@lists.linaro.org' > Subject: Re: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to > prevent accessing PM registers > > On 2013-9-17 1:26, Moore, Robert wrote: > > + #define ACPI_REDUCED_HARDWARE TRUE > > > > The intent of this feature is of course, to remove all code that is not > needed -- specifically for hardware-reduced machines where the size of the > kernel is important. > > > > On a larger machine, the hardware-reduced flag should be sufficient. > However, I would think that the host OS would look at this flag and > realize that it should not be doing certain ACPI hardware-related things > up front, rather than later when it finds out that a write to some ACPI > hardware fails because the hardware isn't there. > > Do you mean we should change the ACPI device driver instead of changing > the ACPICA code? that would be a hard job, because hardware ACPI is used > everywhere. > I don't really know the answer to this, but something tells me that bad things may happen when a driver expects the ACPI hardware to be there, and it finds out that it isn't, simply by calling one of the ACPI hardware interfaces. Or, we could word it this way: if a driver is expecting the ACPI hardware to exist, and we are running on a hardware-reduced platform, why is the driver being loaded in the first place? BTW, hardware-reduced is not restricted to ARM platforms. > Thanks > Hanjun > > > > > This is not to say that it is probably a good thing to return an error > from the ACPI hardware code in the hardware-reduced case. > > > > Bob > > >
While we are at it, here is the *complete* list of ACPICA interfaces that are meaningless on a hardware-reduced platform. If we are going to dynamically disable some of these interfaces, we will need to disable all of them -- for completeness. So, this is actually not a trivial change. I'll let the linux experts chime in on this one. Bob AcpiInstallSciHandler AcpiRemoveSciHandler AcpiInstallGlobalEventHandler AcpiInstallFixedEventHandler AcpiRemoveFixedEventHandler AcpiInstallGpeHandler AcpiRemoveGpeHandler AcpiAcquireGlobalLock AcpiReleaseGlobalLock AcpiEnable AcpiDisable AcpiEnableEvent AcpiDisableEvent AcpiClearEvent AcpiGetEventStatus AcpiUpdateAllGpes AcpiEnableGpe AcpiDisableGpe AcpiSetGpe AcpiSetupGpeForWake AcpiSetGpeWakeMask AcpiClearGpe AcpiGetGpeStatus AcpiFinishGpe AcpiDisableAllGpes AcpiEnableAllRuntimeGpes AcpiInstallGpeBlock AcpiRemoveGpeBlock AcpiGetGpeDevice AcpiGetTimerResolution AcpiGetTimer AcpiGetTimerDuration AcpiReadBitRegister AcpiWriteBitRegister AcpiSetFirmwareWakingVector AcpiSetFirmwareWakingVector64 AcpiEnterSleepStateS4bios > -----Original Message----- > From: Moore, Robert > Sent: Wednesday, September 18, 2013 8:09 AM > To: Hanjun Guo > Cc: 'Rafael J. Wysocki'; 'Len Brown'; Box, David E; Zheng, Lv; 'linux- > acpi@vger.kernel.org'; 'patches@linaro.org'; 'linaro- > kernel@lists.linaro.org'; 'linaro-acpi@lists.linaro.org' > Subject: RE: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to > prevent accessing PM registers > > > > > -----Original Message----- > > From: Hanjun Guo [mailto:hanjun.guo@linaro.org] > > Sent: Wednesday, September 18, 2013 2:32 AM > > To: Moore, Robert > > Cc: 'Rafael J. Wysocki'; 'Len Brown'; Box, David E; Zheng, Lv; 'linux- > > acpi@vger.kernel.org'; 'patches@linaro.org'; 'linaro- > > kernel@lists.linaro.org'; 'linaro-acpi@lists.linaro.org' > > Subject: Re: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to > > prevent accessing PM registers > > > > On 2013-9-17 1:26, Moore, Robert wrote: > > > + #define ACPI_REDUCED_HARDWARE TRUE > > > > > > The intent of this feature is of course, to remove all code that is > > > not > > needed -- specifically for hardware-reduced machines where the size of > > the kernel is important. > > > > > > On a larger machine, the hardware-reduced flag should be sufficient. > > However, I would think that the host OS would look at this flag and > > realize that it should not be doing certain ACPI hardware-related > > things up front, rather than later when it finds out that a write to > > some ACPI hardware fails because the hardware isn't there. > > > > Do you mean we should change the ACPI device driver instead of > > changing the ACPICA code? that would be a hard job, because hardware > > ACPI is used everywhere. > > > > > I don't really know the answer to this, but something tells me that bad > things may happen when a driver expects the ACPI hardware to be there, and > it finds out that it isn't, simply by calling one of the ACPI hardware > interfaces. > > Or, we could word it this way: if a driver is expecting the ACPI hardware > to exist, and we are running on a hardware-reduced platform, why is the > driver being loaded in the first place? > > BTW, hardware-reduced is not restricted to ARM platforms. > > > > > > Thanks > > Hanjun > > > > > > > > This is not to say that it is probably a good thing to return an > > > error > > from the ACPI hardware code in the hardware-reduced case. > > > > > > Bob > > > > >
Ummh... > -----Original Message----- > From: linaro-acpi-bounces@lists.linaro.org [mailto:linaro-acpi- > bounces@lists.linaro.org] On Behalf Of Moore, Robert > Sent: Wednesday, September 18, 2013 9:38 PM > To: 'Hanjun Guo' > Cc: Box, David E; 'linaro-kernel@lists.linaro.org'; > 'patches@linaro.org'; 'linaro-acpi@lists.linaro.org'; 'Rafael J. > Wysocki'; 'linux-acpi@vger.kernel.org'; Zheng, Lv; 'Len Brown' > Subject: Re: [Linaro-acpi] [PATCH] ACPICA / hwreg: Use > acpi_gbl_reduced_hardware to prevent accessing PM registers > > While we are at it, here is the *complete* list of ACPICA interfaces > that are meaningless on a hardware-reduced platform. Events are supported by hw-reduced ACPI 5.0, just in a different (non-legacy) way: GPIO-signaled events. So I'm wondering if perhaps a bit of refactoring is in order, rather than complete removal of all those functions? Leo > If we are going to dynamically disable some of these interfaces, we will need to disable > all of them -- for completeness. So, this is actually not a trivial > change. > > I'll let the linux experts chime in on this one. > Bob > > > AcpiInstallSciHandler > AcpiRemoveSciHandler > AcpiInstallGlobalEventHandler > AcpiInstallFixedEventHandler > AcpiRemoveFixedEventHandler > AcpiInstallGpeHandler > AcpiRemoveGpeHandler > AcpiAcquireGlobalLock > AcpiReleaseGlobalLock > AcpiEnable > AcpiDisable > AcpiEnableEvent > AcpiDisableEvent > AcpiClearEvent > AcpiGetEventStatus > AcpiUpdateAllGpes > AcpiEnableGpe > AcpiDisableGpe > AcpiSetGpe > AcpiSetupGpeForWake > AcpiSetGpeWakeMask > AcpiClearGpe > AcpiGetGpeStatus > AcpiFinishGpe > AcpiDisableAllGpes > AcpiEnableAllRuntimeGpes > AcpiInstallGpeBlock > AcpiRemoveGpeBlock > AcpiGetGpeDevice > AcpiGetTimerResolution > AcpiGetTimer > AcpiGetTimerDuration > AcpiReadBitRegister > AcpiWriteBitRegister > AcpiSetFirmwareWakingVector > AcpiSetFirmwareWakingVector64 > AcpiEnterSleepStateS4bios > > > > > > > > > -----Original Message----- > > From: Moore, Robert > > Sent: Wednesday, September 18, 2013 8:09 AM > > To: Hanjun Guo > > Cc: 'Rafael J. Wysocki'; 'Len Brown'; Box, David E; Zheng, Lv; > 'linux- > > acpi@vger.kernel.org'; 'patches@linaro.org'; 'linaro- > > kernel@lists.linaro.org'; 'linaro-acpi@lists.linaro.org' > > Subject: RE: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to > > prevent accessing PM registers > > > > > > > > > -----Original Message----- > > > From: Hanjun Guo [mailto:hanjun.guo@linaro.org] > > > Sent: Wednesday, September 18, 2013 2:32 AM > > > To: Moore, Robert > > > Cc: 'Rafael J. Wysocki'; 'Len Brown'; Box, David E; Zheng, Lv; > 'linux- > > > acpi@vger.kernel.org'; 'patches@linaro.org'; 'linaro- > > > kernel@lists.linaro.org'; 'linaro-acpi@lists.linaro.org' > > > Subject: Re: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware > to > > > prevent accessing PM registers > > > > > > On 2013-9-17 1:26, Moore, Robert wrote: > > > > + #define ACPI_REDUCED_HARDWARE TRUE > > > > > > > > The intent of this feature is of course, to remove all code that > is > > > > not > > > needed -- specifically for hardware-reduced machines where the size > of > > > the kernel is important. > > > > > > > > On a larger machine, the hardware-reduced flag should be > sufficient. > > > However, I would think that the host OS would look at this flag and > > > realize that it should not be doing certain ACPI hardware-related > > > things up front, rather than later when it finds out that a write > to > > > some ACPI hardware fails because the hardware isn't there. > > > > > > Do you mean we should change the ACPI device driver instead of > > > changing the ACPICA code? that would be a hard job, because > hardware > > > ACPI is used everywhere. > > > > > > > > > I don't really know the answer to this, but something tells me that > bad > > things may happen when a driver expects the ACPI hardware to be > there, and > > it finds out that it isn't, simply by calling one of the ACPI > hardware > > interfaces. > > > > Or, we could word it this way: if a driver is expecting the ACPI > hardware > > to exist, and we are running on a hardware-reduced platform, why is > the > > driver being loaded in the first place? > > > > BTW, hardware-reduced is not restricted to ARM platforms. > > > > > > > > > > > Thanks > > > Hanjun > > > > > > > > > > > This is not to say that it is probably a good thing to return an > > > > error > > > from the ACPI hardware code in the hardware-reduced case. > > > > > > > > Bob > > > > > > > > > > _______________________________________________ > Linaro-acpi mailing list > Linaro-acpi@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-acpi
> -----Original Message----- > From: Duran, Leo [mailto:leo.duran@amd.com] > Sent: Wednesday, September 18, 2013 8:53 PM > To: Moore, Robert; 'Hanjun Guo' > Cc: Box, David E; 'linaro-kernel@lists.linaro.org'; 'patches@linaro.org'; > 'linaro-acpi@lists.linaro.org'; 'Rafael J. Wysocki'; 'linux- > acpi@vger.kernel.org'; Zheng, Lv; 'Len Brown' > Subject: RE: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to > prevent accessing PM registers > > Ummh... > > > -----Original Message----- > > From: linaro-acpi-bounces@lists.linaro.org [mailto:linaro-acpi- > > bounces@lists.linaro.org] On Behalf Of Moore, Robert > > Sent: Wednesday, September 18, 2013 9:38 PM > > To: 'Hanjun Guo' > > Cc: Box, David E; 'linaro-kernel@lists.linaro.org'; > > 'patches@linaro.org'; 'linaro-acpi@lists.linaro.org'; 'Rafael J. > > Wysocki'; 'linux-acpi@vger.kernel.org'; Zheng, Lv; 'Len Brown' > > Subject: Re: [Linaro-acpi] [PATCH] ACPICA / hwreg: Use > > acpi_gbl_reduced_hardware to prevent accessing PM registers > > > > While we are at it, here is the *complete* list of ACPICA interfaces > > that are meaningless on a hardware-reduced platform. > > Events are supported by hw-reduced ACPI 5.0, just in a different (non- > legacy) way: GPIO-signaled events. > So I'm wondering if perhaps a bit of refactoring is in order, rather than > complete removal of all those functions? > Leo > GPIO events are drastically different than any other ACPI events. The model that ACPICA is using for these is that the native GPIO driver loads on detection of a GPIO _HID during the device detection namespace walk. Then: Text from the ACPI specification below. Comments in parentheses are mine: OSPM (GPIO driver) handles GPIO-signaled events as follows: * The GPIO interrupt is handled by OSPM (GPIO driver) because it is listed in the _AEI object under a GPIO controller. * When the event fires, OSPM (GPIO driver) handles the interrupt according to its mode and invokes the _EVT method (via AcpiEvaluateObject), passing it the pin number of the event. * From this point on, handling is exactly like that for GPEs. The _EVT method does a Notify() on the appropriate device, (AcpiInstallNotifyHandler) and OS-specific mechanisms are used to notify the driver of the event. None of the ACPICA interfaces below are required for this sequence of events. Bob > > If we are going to dynamically disable some of these interfaces, we > > will need to disable all of them -- for completeness. So, this is > > actually not a trivial change. > > > > I'll let the linux experts chime in on this one. > > Bob > > > > > > AcpiInstallSciHandler > > AcpiRemoveSciHandler > > AcpiInstallGlobalEventHandler > > AcpiInstallFixedEventHandler > > AcpiRemoveFixedEventHandler > > AcpiInstallGpeHandler > > AcpiRemoveGpeHandler > > AcpiAcquireGlobalLock > > AcpiReleaseGlobalLock > > AcpiEnable > > AcpiDisable > > AcpiEnableEvent > > AcpiDisableEvent > > AcpiClearEvent > > AcpiGetEventStatus > > AcpiUpdateAllGpes > > AcpiEnableGpe > > AcpiDisableGpe > > AcpiSetGpe > > AcpiSetupGpeForWake > > AcpiSetGpeWakeMask > > AcpiClearGpe > > AcpiGetGpeStatus > > AcpiFinishGpe > > AcpiDisableAllGpes > > AcpiEnableAllRuntimeGpes > > AcpiInstallGpeBlock > > AcpiRemoveGpeBlock > > AcpiGetGpeDevice > > AcpiGetTimerResolution > > AcpiGetTimer > > AcpiGetTimerDuration > > AcpiReadBitRegister > > AcpiWriteBitRegister > > AcpiSetFirmwareWakingVector > > AcpiSetFirmwareWakingVector64 > > AcpiEnterSleepStateS4bios > > > > > > > > > > > > > > > > > -----Original Message----- > > > From: Moore, Robert > > > Sent: Wednesday, September 18, 2013 8:09 AM > > > To: Hanjun Guo > > > Cc: 'Rafael J. Wysocki'; 'Len Brown'; Box, David E; Zheng, Lv; > > 'linux- > > > acpi@vger.kernel.org'; 'patches@linaro.org'; 'linaro- > > > kernel@lists.linaro.org'; 'linaro-acpi@lists.linaro.org' > > > Subject: RE: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware > > > to prevent accessing PM registers > > > > > > > > > > > > > -----Original Message----- > > > > From: Hanjun Guo [mailto:hanjun.guo@linaro.org] > > > > Sent: Wednesday, September 18, 2013 2:32 AM > > > > To: Moore, Robert > > > > Cc: 'Rafael J. Wysocki'; 'Len Brown'; Box, David E; Zheng, Lv; > > 'linux- > > > > acpi@vger.kernel.org'; 'patches@linaro.org'; 'linaro- > > > > kernel@lists.linaro.org'; 'linaro-acpi@lists.linaro.org' > > > > Subject: Re: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware > > to > > > > prevent accessing PM registers > > > > > > > > On 2013-9-17 1:26, Moore, Robert wrote: > > > > > + #define ACPI_REDUCED_HARDWARE TRUE > > > > > > > > > > The intent of this feature is of course, to remove all code that > > is > > > > > not > > > > needed -- specifically for hardware-reduced machines where the > > > > size > > of > > > > the kernel is important. > > > > > > > > > > On a larger machine, the hardware-reduced flag should be > > sufficient. > > > > However, I would think that the host OS would look at this flag > > > > and realize that it should not be doing certain ACPI > > > > hardware-related things up front, rather than later when it finds > > > > out that a write > > to > > > > some ACPI hardware fails because the hardware isn't there. > > > > > > > > Do you mean we should change the ACPI device driver instead of > > > > changing the ACPICA code? that would be a hard job, because > > hardware > > > > ACPI is used everywhere. > > > > > > > > > > > > > I don't really know the answer to this, but something tells me that > > bad > > > things may happen when a driver expects the ACPI hardware to be > > there, and > > > it finds out that it isn't, simply by calling one of the ACPI > > hardware > > > interfaces. > > > > > > Or, we could word it this way: if a driver is expecting the ACPI > > hardware > > > to exist, and we are running on a hardware-reduced platform, why is > > the > > > driver being loaded in the first place? > > > > > > BTW, hardware-reduced is not restricted to ARM platforms. > > > > > > > > > > > > > > > > Thanks > > > > Hanjun > > > > > > > > > > > > > > This is not to say that it is probably a good thing to return an > > > > > error > > > > from the ACPI hardware code in the hardware-reduced case. > > > > > > > > > > Bob > > > > > > > > > > > > > > > _______________________________________________ > > Linaro-acpi mailing list > > Linaro-acpi@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/linaro-acpi >
Hi Bob, Sorry for the late reply, I had being on holiday last few days. On 2013-9-18 23:09, Moore, Robert wrote: > > >> -----Original Message----- >> From: Hanjun Guo [mailto:hanjun.guo@linaro.org] >> Sent: Wednesday, September 18, 2013 2:32 AM >> To: Moore, Robert >> Cc: 'Rafael J. Wysocki'; 'Len Brown'; Box, David E; Zheng, Lv; 'linux- >> acpi@vger.kernel.org'; 'patches@linaro.org'; 'linaro- >> kernel@lists.linaro.org'; 'linaro-acpi@lists.linaro.org' >> Subject: Re: [PATCH] ACPICA / hwreg: Use acpi_gbl_reduced_hardware to >> prevent accessing PM registers >> >> On 2013-9-17 1:26, Moore, Robert wrote: >>> + #define ACPI_REDUCED_HARDWARE TRUE >>> >>> The intent of this feature is of course, to remove all code that is not >> needed -- specifically for hardware-reduced machines where the size of the >> kernel is important. >>> >>> On a larger machine, the hardware-reduced flag should be sufficient. >> However, I would think that the host OS would look at this flag and >> realize that it should not be doing certain ACPI hardware-related things >> up front, rather than later when it finds out that a write to some ACPI >> hardware fails because the hardware isn't there. >> >> Do you mean we should change the ACPI device driver instead of changing >> the ACPICA code? that would be a hard job, because hardware ACPI is used >> everywhere. >> > > > I don't really know the answer to this, but something tells me that bad things > may happen when a driver expects the ACPI hardware to be there, and it finds > out that it isn't, simply by calling one of the ACPI hardware interfaces. > > Or, we could word it this way: if a driver is expecting the ACPI hardware to > exist, and we are running on a hardware-reduced platform, why is the driver > being loaded in the first place? ok, that would be a reasonable solution. Oh, things get complicated now, could some linux experts have comments here please? > > BTW, hardware-reduced is not restricted to ARM platforms. Thanks for reminding, may be some other platforms (not only x86, IA64, ARM) will use ACPI in the future. Thanks Hanjun >
On 2013-9-19 10:37, Moore, Robert wrote: > While we are at it, here is the *complete* list of ACPICA interfaces that are > meaningless on a hardware-reduced platform. If we are going to dynamically > disable some of these interfaces, we will need to disable all of them -- for > completeness. So, this is actually not a trivial change. Yes, you are right. Actually, before I send this patch out, I searched all the ACPICA API on a hardware-reduced platform as you did, I found some of them will finally call acpi_hw_register_read/write(), so some of the ACPICA API do not need to change. Some of these APIs are already disabled in linux kernel tree now, such as acpi_enable(): acpi_status acpi_enable(void) { acpi_status status; int retry; ACPI_FUNCTION_TRACE(acpi_enable); /* ACPI tables must be present */ if (!acpi_tb_tables_loaded()) { return_ACPI_STATUS(AE_NO_ACPI_TABLES); } /* If the Hardware Reduced flag is set, machine is always in acpi mode */ if (acpi_gbl_reduced_hardware) { return_ACPI_STATUS(AE_OK); } ... } and some of them need to be disabled as you said. > > I'll let the linux experts chime in on this one. > Bob > > > AcpiInstallSciHandler > AcpiRemoveSciHandler > AcpiInstallGlobalEventHandler > AcpiInstallFixedEventHandler > AcpiRemoveFixedEventHandler > AcpiInstallGpeHandler > AcpiRemoveGpeHandler > AcpiAcquireGlobalLock > AcpiReleaseGlobalLock > AcpiEnable > AcpiDisable > AcpiEnableEvent > AcpiDisableEvent > AcpiClearEvent > AcpiGetEventStatus > AcpiUpdateAllGpes > AcpiEnableGpe > AcpiDisableGpe > AcpiSetGpe > AcpiSetupGpeForWake > AcpiSetGpeWakeMask > AcpiClearGpe > AcpiGetGpeStatus > AcpiFinishGpe > AcpiDisableAllGpes > AcpiEnableAllRuntimeGpes > AcpiInstallGpeBlock > AcpiRemoveGpeBlock > AcpiGetGpeDevice > AcpiGetTimerResolution > AcpiGetTimer > AcpiGetTimerDuration > AcpiReadBitRegister > AcpiWriteBitRegister > AcpiSetFirmwareWakingVector > AcpiSetFirmwareWakingVector64 > AcpiEnterSleepStateS4bios
> From: Hanjun Guo [mailto:hanjun.guo@linaro.org] > Sent: Sunday, September 22, 2013 11:27 AM > > On 2013-9-19 10:37, Moore, Robert wrote: > > While we are at it, here is the *complete* list of ACPICA interfaces that are > > meaningless on a hardware-reduced platform. If we are going to dynamically > > disable some of these interfaces, we will need to disable all of them -- for > > completeness. So, this is actually not a trivial change. > > Yes, you are right. > Actually, before I send this patch out, I searched all the ACPICA API on > a hardware-reduced platform as you did, I found some of them will finally call > acpi_hw_register_read/write(), so some of the ACPICA API do not need to change. > I agreed. My idea is to keep the call path implemented using non-HWREDUCED ACPI silicon not-changed, but changing the code inside of a function implemented with the non-HWREDUCED logic to contain a HWREDUCED logic. This might be more reasonable. So IMO, it is not a good idea to reduce the APIs, but need to re-implement the APIs using HWREDUCED logic. And the non-HWREDUCED code inside of an API should be marked with ACPI_HARDWARE_REDUCED macro rather than the API itself be marked out. If the APIs (aka., ACPI functionalities) are kept un-modified, the device drivers can be kept un-modified on HWREDUCED platforms. Thanks -Lv > Some of these APIs are already disabled in linux kernel tree now, such as > acpi_enable(): > acpi_status acpi_enable(void) > { > acpi_status status; > int retry; > > ACPI_FUNCTION_TRACE(acpi_enable); > > /* ACPI tables must be present */ > > if (!acpi_tb_tables_loaded()) { > return_ACPI_STATUS(AE_NO_ACPI_TABLES); > } > > /* If the Hardware Reduced flag is set, machine is always in acpi mode */ > > if (acpi_gbl_reduced_hardware) { > return_ACPI_STATUS(AE_OK); > } > ... > } > > and some of them need to be disabled as you said. > > > > > I'll let the linux experts chime in on this one. > > Bob > > > > > > AcpiInstallSciHandler > > AcpiRemoveSciHandler > > AcpiInstallGlobalEventHandler > > AcpiInstallFixedEventHandler > > AcpiRemoveFixedEventHandler > > AcpiInstallGpeHandler > > AcpiRemoveGpeHandler > > AcpiAcquireGlobalLock > > AcpiReleaseGlobalLock > > AcpiEnable > > AcpiDisable > > AcpiEnableEvent > > AcpiDisableEvent > > AcpiClearEvent > > AcpiGetEventStatus > > AcpiUpdateAllGpes > > AcpiEnableGpe > > AcpiDisableGpe > > AcpiSetGpe > > AcpiSetupGpeForWake > > AcpiSetGpeWakeMask > > AcpiClearGpe > > AcpiGetGpeStatus > > AcpiFinishGpe > > AcpiDisableAllGpes > > AcpiEnableAllRuntimeGpes > > AcpiInstallGpeBlock > > AcpiRemoveGpeBlock > > AcpiGetGpeDevice > > AcpiGetTimerResolution > > AcpiGetTimer > > AcpiGetTimerDuration > > AcpiReadBitRegister > > AcpiWriteBitRegister > > AcpiSetFirmwareWakingVector > > AcpiSetFirmwareWakingVector64 > > AcpiEnterSleepStateS4bios >
Hi Lv, On 24 September 2013 08:20, Zheng, Lv <lv.zheng@intel.com> wrote: > > From: Hanjun Guo [mailto:hanjun.guo@linaro.org] > > Sent: Sunday, September 22, 2013 11:27 AM > > > > On 2013-9-19 10:37, Moore, Robert wrote: > > > While we are at it, here is the *complete* list of ACPICA interfaces > that are > > > meaningless on a hardware-reduced platform. If we are going to > dynamically > > > disable some of these interfaces, we will need to disable all of them > -- for > > > completeness. So, this is actually not a trivial change. > > > > Yes, you are right. > > Actually, before I send this patch out, I searched all the ACPICA API on > > a hardware-reduced platform as you did, I found some of them will > finally call > > acpi_hw_register_read/write(), so some of the ACPICA API do not need to > change. > > > > I agreed. > > My idea is to keep the call path implemented using non-HWREDUCED ACPI > silicon not-changed, but changing the code inside of a function implemented > with the non-HWREDUCED logic to contain a HWREDUCED logic. > This might be more reasonable. > So IMO, it is not a good idea to reduce the APIs, but need to re-implement > the APIs using HWREDUCED logic. > And the non-HWREDUCED code inside of an API should be marked with > ACPI_HARDWARE_REDUCED macro rather than the API itself be marked out. > If the APIs (aka., ACPI functionalities) are kept un-modified, the device > drivers can be kept un-modified on HWREDUCED platforms. > Totally agreed, so will you do that or just let me to finish that? Thanks Hanjun
diff --git a/drivers/acpi/acpica/hwregs.c b/drivers/acpi/acpica/hwregs.c index 8d2e866..4c270c3 100644 --- a/drivers/acpi/acpica/hwregs.c +++ b/drivers/acpi/acpica/hwregs.c @@ -265,6 +265,11 @@ acpi_status acpi_hw_clear_acpi_status(void) ACPI_FUNCTION_TRACE(hw_clear_acpi_status); + /* If Hardware Reduced flag is set, there are no GPEs */ + if (acpi_gbl_reduced_hardware) { + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); + } + ACPI_DEBUG_PRINT((ACPI_DB_IO, "About to write %04X to %8.8X%8.8X\n", ACPI_BITMASK_ALL_FIXED_STATUS, ACPI_FORMAT_UINT64(acpi_gbl_xpm1a_status.address))); @@ -305,6 +310,10 @@ struct acpi_bit_register_info *acpi_hw_get_bit_register_info(u32 register_id) { ACPI_FUNCTION_ENTRY(); + if (acpi_gbl_reduced_hardware) { + return (NULL); + } + if (register_id > ACPI_BITREG_MAX) { ACPI_ERROR((AE_INFO, "Invalid BitRegister ID: 0x%X", register_id)); @@ -337,6 +346,11 @@ acpi_status acpi_hw_write_pm1_control(u32 pm1a_control, u32 pm1b_control) ACPI_FUNCTION_TRACE(hw_write_pm1_control); + /* If Hardware Reduced flag is set, there are no PM Control registers */ + if (acpi_gbl_reduced_hardware) { + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); + } + status = acpi_hw_write(pm1a_control, &acpi_gbl_FADT.xpm1a_control_block); if (ACPI_FAILURE(status)) { @@ -370,6 +384,11 @@ acpi_status acpi_hw_register_read(u32 register_id, u32 *return_value) ACPI_FUNCTION_TRACE(hw_register_read); + /* If Hardware Reduced flag is set, there are no PM Control registers */ + if (acpi_gbl_reduced_hardware) { + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); + } + switch (register_id) { case ACPI_REGISTER_PM1_STATUS: /* PM1 A/B: 16-bit access each */ @@ -465,6 +484,11 @@ acpi_status acpi_hw_register_write(u32 register_id, u32 value) ACPI_FUNCTION_TRACE(hw_register_write); + /* If Hardware Reduced flag is set, there are no PM Control registers */ + if (acpi_gbl_reduced_hardware) { + return_ACPI_STATUS(AE_NOT_IMPLEMENTED); + } + switch (register_id) { case ACPI_REGISTER_PM1_STATUS: /* PM1 A/B: 16-bit access each */ /*
On hardware reduced platforms, there is no support for the following: - PM Event and Control registers - SCI interrupt (and handler) - Fixed Events - General Purpose Events (GPEs) - Global Lock - ACPI PM timer - FACS table (Waking vectors and Global Lock) acpi_gbl_reduced_hardware is a global flag and can be used to prevent misbehavior on hardware reduced platforms, and this flag is initialized at the very beginning of the system boot when FADT is parsed, this flag is set to 1 when the FACP Hardware Reduced flag is set, so we can use it to prevent accessing registers which do not exist on reduced hardware platform. but when FACP Hardware Reduced flag is set, ACPI drivers will still access some registers directly which do not exist on reduced hardware platform and cause boot failure: [ 9.024370] Unable to handle kernel paging request at virtual address ffffffbffbe00001 [ 9.024423] pgd = ffffffc00007d000 [ 9.024468] [ffffffbffbe00001] *pgd=000000008007f003, *pmd=0000000000000000 [ 9.024538] Internal error: Oops: 96000006 [#1] SMP [ 9.024577] Modules linked in: [ 9.024627] CPU: 0 Not tainted (3.9.0+ #3) [ 9.024691] PC is at acpi_os_read_port+0x58/0xc8 [ 9.024753] LR is at acpi_hw_read_port+0x4c/0xc4 [ 9.024810] pc : [<ffffffc0002810cc>] lr : [<ffffffc0002a4fc4>] pstate: 600003c5 .... [ 9.031956] Call trace: [ 9.032021] [<ffffffc0002810cc>] acpi_os_read_port+0x58/0xc8 [ 9.032094] [<ffffffc0002a4fc4>] acpi_hw_read_port+0x4c/0xc4 [ 9.032165] [<ffffffc0002a4198>] acpi_hw_read+0x6c/0x100 [ 9.032237] [<ffffffc0002a4250>] acpi_hw_read_multiple+0x24/0x70 [ 9.032312] [<ffffffc0002a457c>] acpi_hw_register_read+0xa8/0x164 [ 9.032386] [<ffffffc0002a52dc>] acpi_write_bit_register+0x9c/0x194 [ 9.032472] [<ffffffc0002bffb4>] acpi_processor_get_power_info+0x6c4/0x748 [ 9.032550] [<ffffffc000565220>] acpi_processor_power_init+0xac/0x138 [ 9.032630] [<ffffffc0003f3c00>] acpi_processor_start+0x48/0x138 [ 9.032704] [<ffffffc000565118>] acpi_processor_add+0x43c/0x498 [ 9.032784] [<ffffffc000283a74>] acpi_device_probe+0x3c/0x198 [ 9.032862] [<ffffffc0002ef0fc>] driver_probe_device+0x90/0x348 [ 9.032940] [<ffffffc0002ef450>] __driver_attach+0x9c/0xa0 [ 9.033015] [<ffffffc0002ed40c>] bus_for_each_dev+0x4c/0x8c [ 9.033090] [<ffffffc0002eeb98>] driver_attach+0x20/0x28 [ 9.033166] [<ffffffc0002ee6e0>] bus_add_driver+0xfc/0x254 [ 9.033244] [<ffffffc0002ef8b0>] driver_register+0x6c/0x184 [ 9.033325] [<ffffffc000284894>] acpi_bus_register_driver+0x34/0x44 [ 9.033400] [<ffffffc000559c60>] acpi_processor_init+0x28/0x40 [ 9.033469] [<ffffffc000081438>] do_one_initcall+0x100/0x138 [ 9.033544] [<ffffffc0005448c8>] kernel_init_freeable+0x128/0x1cc [ 9.033620] [<ffffffc0003f2f90>] kernel_init+0x10/0xcc [ 9.033700] Code: d2bf7c02 f2dff7e2 f2ffffe2 8b020000 (79400000) [ 9.033843] ---[ end trace 64376967e6bc20a9 ]--- [ 9.033995] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b So acpi_gbl_reduced_hardware should be used to check if reduced hardware or not, if it is reduced hardware, just return with some error and do not access the no-existent registers. Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> --- drivers/acpi/acpica/hwregs.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)