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

Re: [PATCH v2] Subject: x86/PAT: Report PAT on CPUs that support PAT without MTRR


  • To: Chuck Zmudzinski <brchuckz@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 13 Jul 2022 08:18:05 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=mGf2VtS2XfS4FW9gt7nfASnSt8IWDWM5M09W0hzIawE=; b=lFaPlFz00w3zuDrfbzDlfdTagPvkktBzA1FSEt2DW3l1MonLRMJZ7NS/XEg1/GR1yr48V7FJw+mG+z7+A6xhrnOdJtQVGt6i8ZKal5OwzQvOdTyCtriJUWwYRGRu756UEkYfLkg9sUcmQOg9z8G1R52eleTbJoz2+ODizoH40VMlsoD3GjB3ojbusfgJVAKZTnS+ar9EKe2twwTsMQzrteD+P70V0EnShxdzf6loohHox3RggGsqU38kH2d0rpentI2bqCHD81OUnRfwvUFIwUJIZu6DKGoYsT16xrbzSqp3Zf5go4fzPN70mZ736AV8fmcP2/vXN1Belw1CIhtlNw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Zx9YaJZvFrwrhnAdf5cj3aZKu10jI9rD/SlIT38tlbaR+cK2FZDksxxfcbZfQnUcBvk0lNNcP27ZI+M3D4iWEe295Qz7ZqVkQxQ4x6pZjIng93OHuohpKTDFeEYkcIEsxywE8VXIOf4R9Iall3ceTcs5FgL7H8RLURyjXVAJ8N7KSNMeSmnOyX2Mxsurf+rpQJDLQhPi+K8ELknqjHt443OYTX2OwC+c9OxjYEjMnyXYtR7RynIuMyw8NrLllPhdezLEL2MndZ6wiMlR4TjbWhYHia81nc0cqW5w+WNALbf6SsFU5DcmJDS4d/k9sE3paDhFIjdmvDZRaTFh6hvWPA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>, Andy Lutomirski <luto@xxxxxxxxxx>, Peter Zijlstra <peterz@xxxxxxxxxxxxx>, Thomas Gleixner <tglx@xxxxxxxxxxxxx>, Ingo Molnar <mingo@xxxxxxxxxx>, Borislav Petkov <bp@xxxxxxxxx>, x86@xxxxxxxxxx, "H. Peter Anvin" <hpa@xxxxxxxxx>, Dan Williams <dan.j.williams@xxxxxxxxx>, "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>, Tom Lendacky <thomas.lendacky@xxxxxxx>, Jane Chu <jane.chu@xxxxxxxxxx>, Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>, Randy Dunlap <rdunlap@xxxxxxxxxxxxx>, Sean Christopherson <seanjc@xxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, stable@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
  • Delivery-date: Wed, 13 Jul 2022 06:18:22 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 13.07.2022 03:36, Chuck Zmudzinski wrote:
> v2: *Add force_pat_disabled variable to fix "nopat" on Xen PV (Jan Beulich)
>     *Add the necessary code to incorporate the "nopat" fix
>     *void init_cache_modes(void) -> void __init init_cache_modes(void)
>     *Add Jan Beulich as Co-developer (Jan has not signed off yet)
>     *Expand the commit message to include relevant parts of the commit
>      message of Jan Beulich's proposed patch for this problem
>     *Fix 'else if ... {' placement and indentation
>     *Remove indication the backport to stable branches is only back to 5.17.y
> 
> I think these changes address all the comments on the original patch
> 
> I added Jan Beulich as a Co-developer because Juergen Gross asked me to
> include Jan's idea for fixing "nopat" that was missing from the first
> version of the patch.

You've sufficiently altered this change to clearly no longer want my
S-o-b; unfortunately in fact I think you broke things:

> @@ -292,7 +294,7 @@ void init_cache_modes(void)
>               rdmsrl(MSR_IA32_CR_PAT, pat);
>       }
>  
> -     if (!pat) {
> +     if (!pat || pat_force_disabled) {

By checking the new variable here ...

>               /*
>                * No PAT. Emulate the PAT table that corresponds to the two
>                * cache bits, PWT (Write Through) and PCD (Cache Disable).
> @@ -313,6 +315,16 @@ void init_cache_modes(void)
>                */
>               pat = PAT(0, WB) | PAT(1, WT) | PAT(2, UC_MINUS) | PAT(3, UC) |
>                     PAT(4, WB) | PAT(5, WT) | PAT(6, UC_MINUS) | PAT(7, UC);

... you put in place a software view which doesn't match hardware. I
continue to think that ...

> +     } else if (!pat_bp_enabled) {

... the variable wants checking here instead (at which point, yes,
this comes quite close to simply being a v2 of my original patch).

By using !pat_bp_enabled here you actually broaden where the change
would take effect. Iirc Boris had asked to narrow things (besides
voicing opposition to this approach altogether). Even without that
request I wonder whether you aren't going to far with this.

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.