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: <016d281b-7e40-f1bd-66ee-c19c3cc56efe@suse.com>
Date:   Tue, 5 Jul 2022 17:56:36 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH] x86/PAT: have pat_enabled() properly reflect state when
 running on e.g. Xen
Content-Language: en-US
To:     Borislav Petkov <bp@alien8.de>
Cc:     Andrew Lutomirski <luto@kernel.org>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        lkml <linux-kernel@vger.kernel.org>,
        "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
References: <9385fa60-fa5d-f559-a137-6608408f88b0@suse.com>
 <YsRTAGI2PhfZ5V7M@zn.tnic>
From:   Jan Beulich <jbeulich@suse.com>
In-Reply-To: <YsRTAGI2PhfZ5V7M@zn.tnic>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?alVHbjZOWHFRSGU4Z2NmK1RtRkhQZDZiM2ZXVzlpdklnV0xWQkYrK0FJSXIz?=
 =?utf-8?B?MlhkMXhoVDBwQnJxeEtac2FjK00wbHNaNnRCY3hiSVdYVEx0MStYcnFkNWZG?=
 =?utf-8?B?OW5wcVR3bUliVnVGUGo5V1h6R0hXaytjNUQ3OTV0ZnFFbDhIYnBVWk95RnE1?=
 =?utf-8?B?WnVlTzg3c2RmNmcvQkx0R1dUQ3g0SGYyWnFuajRYNk1LeC9nK1FqT0s3dk9D?=
 =?utf-8?B?NmhESVJtQUVNQXlhSnNVWXFocjlHRmhmbnA3RDBXcXpjSUxRemZrMlczK1cr?=
 =?utf-8?B?SWZwWEJvRkMzZlZlUnhTOGdiS2w3bnBDVktEa3RQL1lZeUNmVEtMT0w2V3Vm?=
 =?utf-8?B?Wm9OM2NUL2RPV1pyTnBiTlRzUjNGaC85RjVscDFKYjdkUlJkVUNhbXBMSzNo?=
 =?utf-8?B?eGkwUVk4ZXp3Vkp5RTJySG5SNGord2RabVNVYkg2d3RUVzlHaE1RM3NiVHJP?=
 =?utf-8?B?TFRkRW1FWEF5RllrQW0rU2tYdndqRlVBNDh1d1E4Ly9PTFhJRVhQMDlremJF?=
 =?utf-8?B?VXBIVkpnTTgxTEcrMG04RGE5S3ZFSzZ3eXJERUMxS3dDVjNqU25LZW03QTQ0?=
 =?utf-8?B?TkpxVXg4L0pSYjZzZDJwZFhXRElCNWthQTFIc09YMytxUzY5N0hUQ3EvUTVv?=
 =?utf-8?B?bTJyWjRXQTlSTW1ZanlKR0JqM1pXL1dGU3l1eldhWWphMGU5SGRPRndEcW1j?=
 =?utf-8?B?VmVzVWlhQ3p5UC8rZDdhREY4Tm9wNTU0RWxBc0cxL2c3NStYSXFQd1ZuQTlG?=
 =?utf-8?B?ZXV6cFp6Tkt1eS9kNmtpbVR4d0Y4SDlmSHRST0FXclJNVnkwaWx4dnVQbkh1?=
 =?utf-8?B?SVk5R1hxSml1enovcytSdTlNT0R6THk1R1dwbVVKeVBrYUpnU3RmK1V5bzVs?=
 =?utf-8?B?S3pXV21hOEFOdkVLYXZ2cW82aWJSdzI2eEhWTWgrZGlaKzdjSnZnc0hXUDVT?=
 =?utf-8?B?ZnR4NFVlVjdJUG1zbXdUSE5ldUdjQ1owcUdtZkRnVVl3bkVMelg0cnNjd1JY?=
 =?utf-8?B?ZnAzeE5GZ2pnOUk4MEFINFZKNFd6SmtNNGhJTFpmN1lNajBoLzh0eVRLWlVD?=
 =?utf-8?B?dmJlVWlLVkduSGRnVEFMZ0N1cE1zSDM1OXF4ayt1RWdPb3lZdDhBQ3hmbHVn?=
 =?utf-8?B?YW5uQ0hPSldrR1BvYXpnKzJrSlBFNVYwbEFBNk50ejdUK3BCYmhJaStyU3Jy?=
 =?utf-8?B?TFdLWjhrS0hFRmRWQ3JMWjNiRzVONkJEdUVCRVRZVzNlMUttMC9aQjVTNngy?=
 =?utf-8?B?UjNZc29wc2VQc0g1KzdoNE81VlpZMXdnd1pwWWVyNWVmcmt4R2VNVlNJNGNy?=
 =?utf-8?B?dm96ZTRwODVJemx2dFpPRXVLcGUzbEw4d0NSaUdsOTEzTXdaZE1iU2VqUXRB?=
 =?utf-8?B?STlBUGl3R1lwdVQ3REVoeUk1QzhhVE1MRFdlYjV4UnczNTRvbDJWd2F3cUNH?=
 =?utf-8?B?WjR4QzBRbEhVTmRrWCtrTzZRd2wyRXlWTHBDbG5SeW91ZHNWTERuMzVFM1Rt?=
 =?utf-8?B?WWtEckhZanBrL3ZWM0xieDZZaDRKem9PZG1pOVNkY3IwSkdUVFlBckdEN1N1?=
 =?utf-8?B?WDBoZUxjMWhNRFhxU3Rkb2tnNTNKV0d2QUZKaFBBRnFkeFpwWHBVbDZ0YWtP?=
 =?utf-8?B?RmlUTVZkNy8wZ3RRSmU3VHZBSG1XeFcyS3hsYXB1V3NKSnZtWEZhMlVwM1B4?=
 =?utf-8?B?T05mODE1VWUxOXRrNXBibFJMVm1BcUtPakRJYy90VVVCUURkTys5SnFQQWxh?=
 =?utf-8?B?NXBqMFZIK0V0K3MyeEJpclhQeEUvOEtTRGZwclZ3OTA1Sk1tNGJqTDJWU1Er?=
 =?utf-8?B?dHF2VXdhRzFCb1Q5eFRUZlliODJKZVJYMmd1NEkrOE1lbC9VQWRxNDJoeGRR?=
 =?utf-8?B?SEZkOE90K3ZYRmpWM0pZaFp3S05TR2QzYVJSUXR4UEZpbzVrczJDUXZVK1VJ?=
 =?utf-8?B?TzRyZGxsMWJsMEFkNWc4SUNsaDBXL0hGaFc5ME5HUmY5L0d2eGpEUTI4R3Nq?=
 =?utf-8?B?WWl2YjJhV3JCc1IyaTZ4TEwzSi9Rc2tYQWFiZ0xsQXQzaEZiZ0h6ZXYxeTFS?=
 =?utf-8?B?ZU1JcUdybk5hNURmeTJpa2RXQWJ1Tk91SVdta2RpaXlJWC9iS3ZkUG96WVJw?=
 =?utf-8?Q?02F52fNzM/vlJgIYpSKXSK6m1?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c671b9b6-8487-434b-a49e-08da5e9ef23f
X-MS-Exchange-CrossTenant-AuthSource: VE1PR04MB6560.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jul 2022 15:56:39.0166
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: EhSO+smMmAB5qE7JdaKbv119E4Ynaei8gIxxtgY8+BQ1WdkhgLWS4RvsorVC3CY2qMYVQDTEn1xqn/E44GEz7w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR04MB6552
X-Spam-Status: No, score=-2.1 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,T_SCC_BODY_TEXT_LINE
        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 05.07.2022 17:04, Borislav Petkov wrote:
> On Thu, Apr 28, 2022 at 04:50:29PM +0200, Jan Beulich wrote:
>> --- a/arch/x86/mm/pat/memtype.c
>> +++ b/arch/x86/mm/pat/memtype.c
>> @@ -62,6 +62,7 @@
>>  
>>  static bool __read_mostly pat_bp_initialized;
>>  static bool __read_mostly pat_disabled = !IS_ENABLED(CONFIG_X86_PAT);
>> +static bool __initdata pat_force_disabled = !IS_ENABLED(CONFIG_X86_PAT);
>>  static bool __read_mostly pat_bp_enabled;
>>  static bool __read_mostly pat_cm_initialized;
> 
> Why yet another boolean var?

Re-using pat_disabled like you do in your suggestion below won't
work, because mtrr_bp_init() calls pat_disable() when MTRRs
appear to be disabled (from the kernel's view). The goal is to
honor "nopat" without honoring any other calls to pat_disable().

> Why not extend pat_enabled() to reflect the Xen case and explain it
> properly above it?

I can probably fiddle with pat_enabled() instead of with
init_cache_modes(), but when making the change I had the feeling
this might be less liked (as looking more hacky, at least to me).

But besides the "where" the other question is: Do you really want
me to limit this to Xen/PV, rather than - as I have it now -
extending it to any hypervisor, which may behave in similar ways?

Jan

> My comment is likely wrong because I don't know what the Xen HV hides or
> doesn't but you get the idea...
> 
> diff --git a/arch/x86/mm/pat/memtype.c b/arch/x86/mm/pat/memtype.c
> index d5ef64ddd35e..a8f1a02f9bc2 100644
> --- a/arch/x86/mm/pat/memtype.c
> +++ b/arch/x86/mm/pat/memtype.c
> @@ -92,6 +92,13 @@ early_param("nopat", nopat);
>  
>  bool pat_enabled(void)
>  {
> +	/*
> +	 * Xen PV doesn't expose the PAT MSR to dom0 so the proper init path
> +	 * there cannot be exercised. Announce PAT is enabled in that case too.
> +	 */
> +	if (cpu_feature_enabled(X86_FEATURE_XENPV) && !pat_disabled)
> +		return true;
> +
>  	return pat_bp_enabled;
>  }
>  EXPORT_SYMBOL_GPL(pat_enabled);
>