| Message ID | 20220620113441.23961-1-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] x86/pat: fix x86_has_pat_wp() | expand |
+ Tom. On Mon, Jun 20, 2022 at 01:34:41PM +0200, Juergen Gross wrote: > x86_has_pat_wp() is using a wrong test, as it relies on the normal > PAT configuration used by the kernel. In case the PAT MSR has been > setup by another entity (e.g. BIOS or Xen hypervisor) it might return > false even if the PAT configuration is allowing WP mappings. ... because Xen doesn't allow writing the PAT MSR. Please explain exactly what happens because we will forget. > The correct way to test for WP support is: > > 1. Get the PTE protection bits needed to select WP mode by reading > __cachemode2pte_tbl[_PAGE_CACHE_MODE_WP] (depending on the PAT MSR > setting this might return protection bits for a stronger mode, e.g. > UC-) > 2. Translate those bits back into the real cache mode selected by those > PTE bits by reading __pte2cachemode_tbl[__pte2cm_idx(prot)] > 3. Test for the cache mode to be _PAGE_CACHE_MODE_WP Yes, this is a good explanation albeit a bit too verbose. You can stick a shorter version of it as a comment over the function so that we don't have to swap it all back in next time. > Fixes: 1f6f655e01ad ("x86/mm: Add a x86_has_pat_wp() helper") If anything, this should be: f88a68facd9a ("x86/mm: Extend early_memremap() support with additional attrs") Also, I'm thinking CC:stable here. > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > V2: > - fix indexing into __pte2cachemode_tbl[] Yes, in any case, I see it now. The key aspect being in the comment above it: * Index into __pte2cachemode_tbl[] are the caching attribute bits of the pte * (_PAGE_PWT, _PAGE_PCD, _PAGE_PAT) at index bit positions 0, 1, 2. which is how one should index into that array. Thx.
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c index d8cfce221275..914ac5f29fca 100644 --- a/arch/x86/mm/init.c +++ b/arch/x86/mm/init.c @@ -80,7 +80,9 @@ static uint8_t __pte2cachemode_tbl[8] = { /* Check that the write-protect PAT entry is set for write-protect */ bool x86_has_pat_wp(void) { - return __pte2cachemode_tbl[_PAGE_CACHE_MODE_WP] == _PAGE_CACHE_MODE_WP; + uint16_t prot = __cachemode2pte_tbl[_PAGE_CACHE_MODE_WP]; + + return __pte2cachemode_tbl[__pte2cm_idx(prot)] == _PAGE_CACHE_MODE_WP; } enum page_cache_mode pgprot2cachemode(pgprot_t pgprot)
x86_has_pat_wp() is using a wrong test, as it relies on the normal PAT configuration used by the kernel. In case the PAT MSR has been setup by another entity (e.g. BIOS or Xen hypervisor) it might return false even if the PAT configuration is allowing WP mappings. The correct way to test for WP support is: 1. Get the PTE protection bits needed to select WP mode by reading __cachemode2pte_tbl[_PAGE_CACHE_MODE_WP] (depending on the PAT MSR setting this might return protection bits for a stronger mode, e.g. UC-) 2. Translate those bits back into the real cache mode selected by those PTE bits by reading __pte2cachemode_tbl[__pte2cm_idx(prot)] 3. Test for the cache mode to be _PAGE_CACHE_MODE_WP Fixes: 1f6f655e01ad ("x86/mm: Add a x86_has_pat_wp() helper") Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - fix indexing into __pte2cachemode_tbl[] --- arch/x86/mm/init.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)