Currently, the function waiting for completion of mailbox operation is
'wait_for_completion()'. The PCC method will be permanently blocked if
this mailbox message fails to execute. So this patch replaces it with
'wait_for_completion_timeout()'. And set the timeout interval to an
arbitrary retries on top of nominal to prevent the remote processor is
slow to respond to PCC commands.
Fixes: 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype")
Signed-off-by: Huisong Li <[email protected]>
---
drivers/acpi/acpi_pcc.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
index a12b55d81209..a1052fe998bf 100644
--- a/drivers/acpi/acpi_pcc.c
+++ b/drivers/acpi/acpi_pcc.c
@@ -23,6 +23,12 @@
#include <acpi/pcc.h>
+/*
+ * Arbitrary retries in case the remote processor is slow to respond
+ * to PCC commands
+ */
+#define PCC_CMD_WAIT_RETRIES_NUM 500
+
struct pcc_data {
struct pcc_mbox_chan *pcc_chan;
void __iomem *pcc_comm_addr;
@@ -86,6 +92,7 @@ acpi_pcc_address_space_handler(u32 function, acpi_physical_address addr,
{
int ret;
struct pcc_data *data = region_context;
+ u64 usecs_lat;
reinit_completion(&data->done);
@@ -96,8 +103,20 @@ acpi_pcc_address_space_handler(u32 function, acpi_physical_address addr,
if (ret < 0)
return AE_ERROR;
- if (data->pcc_chan->mchan->mbox->txdone_irq)
- wait_for_completion(&data->done);
+ if (data->pcc_chan->mchan->mbox->txdone_irq) {
+ /*
+ * pcc_chan->latency is just a Nominal value. In reality the remote
+ * processor could be much slower to reply. So add an arbitrary
+ * amount of wait on top of Nominal.
+ */
+ usecs_lat = PCC_CMD_WAIT_RETRIES_NUM * data->pcc_chan->latency;
+ ret = wait_for_completion_timeout(&data->done,
+ usecs_to_jiffies(usecs_lat));
+ if (ret == 0) {
+ pr_err("PCC command executed timeout!\n");
+ return AE_TIME;
+ }
+ }
mbox_client_txdone(data->pcc_chan->mchan, ret);
--
2.33.0
On Tue, Sep 20, 2022 at 05:44:59PM +0800, Huisong Li wrote:
> Currently, the function waiting for completion of mailbox operation is
> 'wait_for_completion()'. The PCC method will be permanently blocked if
> this mailbox message fails to execute. So this patch replaces it with
> 'wait_for_completion_timeout()'. And set the timeout interval to an
> arbitrary retries on top of nominal to prevent the remote processor is
> slow to respond to PCC commands.
>
Sounds good to me. The only concern(may be not serious) is what happens
if we receive response from the platform after the timeout. I have tested
for that in non ACPI non PCC context. I don't have a setup to trigger that
with ACPI PCC + this patch to test. Other than that, I am fine with this:
Reviewed-by: Sudeep Holla <[email protected]>
--
Regards,
Sudeep
在 2022/9/21 23:47, Sudeep Holla 写道:
> On Tue, Sep 20, 2022 at 05:44:59PM +0800, Huisong Li wrote:
>> Currently, the function waiting for completion of mailbox operation is
>> 'wait_for_completion()'. The PCC method will be permanently blocked if
>> this mailbox message fails to execute. So this patch replaces it with
>> 'wait_for_completion_timeout()'. And set the timeout interval to an
>> arbitrary retries on top of nominal to prevent the remote processor is
>> slow to respond to PCC commands.
>>
> Sounds good to me. The only concern(may be not serious) is what happens
> if we receive response from the platform after the timeout. I have tested
If OS still cann't receive response in noramal latency(must be filled
accurately
as protocol said) + retries, there is a high probability that an
exception occurs.
Even if we receive response after the timeout, I think there may be no
impact,
but the response data in PCC share memory is ignored.
> for that in non ACPI non PCC context. I don't have a setup to trigger that
> with ACPI PCC + this patch to test. Other than that, I am fine with this:
>
> Reviewed-by: Sudeep Holla <[email protected]>
>