Re: Definition of PMD_FLAGS_DEC_WP in arch/x86/mm/mem_encrypt_identity.c

[Tom Lendacky <thomas.lendacky@amd.com>]

On 11/8/20 10:37 AM, Arvind Sankar wrote:
> Hi, I have a question about this definition in
> arch/x86/mm/mem_encrypt_identity.c:
> 
> 	#define PMD_FLAGS_LARGE         (__PAGE_KERNEL_LARGE_EXEC & ~_PAGE_GLOBAL)
> 
> 	#define PMD_FLAGS_DEC           PMD_FLAGS_LARGE
> 	#define PMD_FLAGS_DEC_WP        ((PMD_FLAGS_DEC & ~_PAGE_CACHE_MASK) | \
> 					 (_PAGE_PAT | _PAGE_PWT))
> 
> _PAGE_CACHE_MASK and _PAGE_PAT are for 4k pages, not 2M pages. The
> definition of PMD_FLAGS_DEC_WP clears the PSE bit by masking out
> _PAGE_CACHE_MASK, and sets it again by setting _PAGE_PAT, resulting in
> PMD_FLAGS_DEC_WP actually being write-through, not write-protected,
> using PAT index 1.
> 
> Shouldn't the definition be
> 
> 	#define PMD_FLAGS_DEC_WP	(PMD_FLAGS_DEC | _PAGE_PAT_LARGE | _PAGE_PWT)
> 
> for write-protected using PAT index 5?

Yes it should. There should probably be a _PAGE_CACHE_MASK_LARGE
definition so that the end result is:

#define PMD_FLAGS_DEC_WP	((PMD_FLAGS_DEC & ~_PAGE_CACHE_MASK_LARGE) | \
				(_PAGE_PAT_LARGE | _PAGE_PWT)

> 
> I guess the difference doesn't actually matter for encrypt-in-place? But
> mem_encrypt_boot.S takes pains to initialize PA5 to be write-protected,
> and it looks like it won't actually be used.

Given how early in the boot everything occurs and the cache flushing that
is performed related to the operations, it works. But this should be
fixed.

Are you planning on sending a patch?

Thanks,
Tom

> 
> Thanks.
>