Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5b3bfd60-c44e-2cee-34fe-cb6661ccbe51@amd.com>
Date:   Thu, 22 Sep 2022 13:57:25 -0500
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH] ACPI: processor idle: Practically limit "Dummy wait"
 workaround to old Intel systems
Content-Language: en-US
To:     "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
        Dave Hansen <dave.hansen@intel.com>,
        linux-kernel@vger.kernel.org
Cc:     Dave Hansen <dave.hansen@linux.intel.com>,
        Len Brown <lenb@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Borislav Petkov <bp@alien8.de>,
        K Prateek Nayak <kprateek.nayak@amd.com>,
        linux-acpi@vger.kernel.org, Linux PM <linux-pm@vger.kernel.org>
References: <20220922184745.3252932-1-dave.hansen@intel.com>
 <78d13a19-2806-c8af-573e-7f2625edfab8@intel.com>
From:   "Limonciello, Mario" <mario.limonciello@amd.com>
In-Reply-To: <78d13a19-2806-c8af-573e-7f2625edfab8@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?K3MvYjgwN1BnVkNlbzd6c1ZjNG5jOXQwUTlaV0hMQ1l4UkJFSVdDVHBXci9l?=
 =?utf-8?B?eXFtY0NYS0NLRmhMR01EZUhtb3FncEg4V1laMkE1bURiSklya2FodVdEUGZJ?=
 =?utf-8?B?ZWw1VWo4SFRYS050ODF1a216UHBuZjdoMXNCZS81Q0ZSVEM0R3dYaDVBdnpq?=
 =?utf-8?B?eHpzOUxVbDB4MzRCa294bEY3cHNjU1hoUkR1TDMzMHJDKzlCTGV3U0dpK2kx?=
 =?utf-8?B?YnB3d1E4OHF0dEJPRTc3b2w2Zmp5ZkE1MnhVRmNlVVE3a3hyRTl2eHBzKzV2?=
 =?utf-8?B?Lyt1Ym1IczFWMWNpeEZFUWEvYTQzVVdhc21uQ1pyNTg3L2lHN2w1cTczc1d6?=
 =?utf-8?B?eHlnZUNGa0hWUWJzd2lCVlVyNnJIL1N1S25Ob2N6L2gxc25qei80VFRwKzZV?=
 =?utf-8?B?dDdXayt2anJmVlNFVWZYMnpJZXluTU9nbnJYMmV5VVVQazRaYUdUN2JWSjlX?=
 =?utf-8?B?YW5VNko1WTNVcnM2TUtzMDVkQmRTQXJlcEp6a1Vra2pYSkM5WFMvc2FtS2dS?=
 =?utf-8?B?WlFkak5HaXk5Q1ZwRjVVMGl1MnhtTll2RU9VYWNoZDJ2R2Z2cUhJWVAwRUpp?=
 =?utf-8?B?ekZMd1V5SW9yczB4eUNuNjVnL2JNa0x2YzR0VTVHanVaWlowNVlqSXVTVU5L?=
 =?utf-8?B?QmRWRHlGalZBUEVpM0JGNmRkVXFITVVEQ2x3RkxoNEFCcEE2ZFBtWjhaeVMy?=
 =?utf-8?B?S3N3UWxqK2FSZm0yVll5ZGl5WlVWMWZXSVozMlk1R3lETmZrczhodGpvWnlQ?=
 =?utf-8?B?MjVCVTRGVEZ0NnpzUDMzcTFpQksycWxlRU52clNoQ3RBcHlrSkpCbmREdlNK?=
 =?utf-8?B?ODc2NnA4dnRtWVJ1MU84NHp0ZFVpRnRiZHd1L1hna3lYOHNvK2dQMnN1YXlj?=
 =?utf-8?B?TlV6TkxMOElsOXdIMTFEaVd0TEZGZVpwdmRQRVdpSEpROHJGY3czbUduQXhO?=
 =?utf-8?B?L1JWdXl1VUFDcUt0dWUreHozVDVZTVY4RE9YWW1KbEhRcjNsQitndVQ4YWZP?=
 =?utf-8?B?a2oxcmpHVXNiaUtHMUhqcmg5WkhKVnAzdmxrTUMzLzd2aEROUFMzQVRCZ1dz?=
 =?utf-8?B?Q2xTUjRNMWo5cHpHOFVGTFhMMExiS3dFTll5bmt0Tzd1cmY0YTJNYTVZd29X?=
 =?utf-8?B?Wmh5VkxiZ1VNVVFNdkdqWk1EemgwZXZOZm9YUVErRzMyT2MrVUxzckpabkdZ?=
 =?utf-8?B?b29tS0xRZDNFVUt4aEpoSTAzdW9uMUdNNmlKMVlPOU1NMEVqM3NjZHp4UC9X?=
 =?utf-8?B?TXladEIyT3ZvWDZMbVFWOXRSc0pHZE1tMUFsYUJmMUIvOXFhL2dSWXppbFY5?=
 =?utf-8?B?d3QvRlNJVXBTZFhOUGs4ajJJWUpZVVpvYnl2ejl1TGxkeU5uNVBNL3ZpRUNO?=
 =?utf-8?B?Q0ZzYmlmWHFVWDZLcWlnUW90ZEtFcTVBRU10TTBJYWNYRzNzdlFtUlhaQURy?=
 =?utf-8?B?V21BK0E4dktpN3IzbnlRelNMdUtHU0d2U1JnWnVnaWlRTklFWmdhaUNvcWVr?=
 =?utf-8?B?MCtmNlVpNUlVUkZITXVrSk5BaUIyUU5rZWZIMURHWEl1TXhXSlY2Rmp1RGVx?=
 =?utf-8?B?amMvcFRWWHczT2V3dUwrMHk5WUV2cEl0V2lSc3Mva2VlUmVkSDduQWZoeWNM?=
 =?utf-8?B?ME1PLzNoeXFwNmJZckIzd2lUdzhVTy9EYjhTVFA3UCtOV05BSURWTzVuSXFs?=
 =?utf-8?B?Y2gzc3pCVzhpOW9tZVoyOEdhaG94dkdsTHZqM1hIa3JvNFBpcFhrdjdYM1VQ?=
 =?utf-8?B?SDl6YUo0SGVyVzdNamJiZzBXQ3o0cmI0MlJTR2VzL2RieGtWUjRkK3FaM1dH?=
 =?utf-8?B?Rlg3ejNsSENSelA4MFByS0lsRGVsa1VMbTZwOFZPMldoQzI0SXpiemRlWDY4?=
 =?utf-8?B?WmdmRkxLbWpURFJiWnRFaHdSb0RZcFlXdS9qZllZN3dwQmhGbmtYbFVYSk9z?=
 =?utf-8?B?Mno2RDJDWWlYaXBiSE5FdjJJTisySzZpNE0wS2JEaEhXQmkrUTQ5UUt6dGRC?=
 =?utf-8?B?UzRmeUxsSElUU0VrM3pyRG03aXc1bTJUOVBNY2NNRnhQMDBzSlZhaElVV1ZP?=
 =?utf-8?B?K2l3WVY0cmpuRjZzS1dtbE1CT2pOT0loOU9jcVhmT1lhbUd5dnNXaDl0aGpF?=
 =?utf-8?Q?M0FflYxJTqM2qpY9HMJZrIcr4?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6edaf293-96d4-4c8b-fd09-08da9ccc4bb1
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Sep 2022 18:57:28.4188
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: TqLa4/4cToahppD718/Z4Mb3w1i/X/Mu+Qj8AUmXbnxljzdIxTaFTRV60eIAG1NM/3o+2qGoKh3u1HG4a4z0hA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4404
X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 9/22/2022 13:53, Rafael J. Wysocki wrote:
> On 9/22/2022 8:47 PM, Dave Hansen wrote:
>> Old, circa 2002 chipsets have a bug: they don't go idle when they are
>> supposed to.  So, a workaround was added to slow the CPU down and
>> ensure that the CPU waits a bit for the chipset to actually go idle.
>> This workaround is ancient and has been in place in some form since
>> the original kernel ACPI implementation.
>>
>> But, this workaround is very painful on modern systems.  The "inl()"
>> can take thousands of cycles (see Link: for some more detailed
>> numbers and some fun kernel archaeology).
>>
>> First and foremost, modern systems should not be using this code.
>> Typical Intel systems have not used it in over a decade because it is
>> horribly inferior to MWAIT-based idle.
>>
>> Despite this, people do seem to be tripping over this workaround on
>> AMD system today.
>>
>> Limit the "dummy wait" workaround to Intel systems.  Keep Modern AMD
>> systems from tripping over the workaround.  Remotely modern Intel
>> systems use intel_idle instead of this code and will, in practice,
>> remain unaffected by the dummy wait.
>>
>> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
>> Cc: Len Brown <lenb@kernel.org>
>> Cc: Mario Limonciello <Mario.Limonciello@amd.com>
>> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Reported-by: K Prateek Nayak <kprateek.nayak@amd.com>
>> Link: 
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20220921063638.2489-1-kprateek.nayak%40amd.com%2F&amp;data=05%7C01%7CMario.Limonciello%40amd.com%7C8460d9ef3add45bf571408da9ccbc58a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637994696248641733%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=23k2wKPZaBrgOTtcHw8ByNzfsus1RSsdXMlCACjl%2Bmc%3D&amp;reserved=0 

If agreeable, I think this should be @stable too.

Either way:

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

>>
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> or do you want me to pick this up?
> 
> 
>> ---
>>   drivers/acpi/processor_idle.c | 23 ++++++++++++++++++++---
>>   1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_idle.c 
>> b/drivers/acpi/processor_idle.c
>> index 16a1663d02d4..9f40917c49ef 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -531,10 +531,27 @@ static void wait_for_freeze(void)
>>       /* No delay is needed if we are in guest */
>>       if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
>>           return;
>> +    /*
>> +     * Modern (>=Nehalem) Intel systems use ACPI via intel_idle,
>> +     * not this code.  Assume that any Intel systems using this
>> +     * are ancient and may need the dummy wait.  This also assumes
>> +     * that the motivating chipset issue was Intel-only.
>> +     */
>> +    if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
>> +        return;
>>   #endif
>> -    /* Dummy wait op - must do something useless after P_LVL2 read
>> -       because chipsets cannot guarantee that STPCLK# signal
>> -       gets asserted in time to freeze execution properly. */
>> +    /*
>> +     * Dummy wait op - must do something useless after P_LVL2 read
>> +     * because chipsets cannot guarantee that STPCLK# signal gets
>> +     * asserted in time to freeze execution properly
>> +     *
>> +     * This workaround has been in place since the original ACPI
>> +     * implementation was merged, circa 2002.
>> +     *
>> +     * If a profile is pointing to this instruction, please first
>> +     * consider moving your system to a more modern idle
>> +     * mechanism.
>> +     */
>>       inl(acpi_gbl_FADT.xpm_timer_block.address);
>>   }
> 
>