Re: [PATCH v6 2/5] x86/mm: fix sme_populate_pgd() to update page flags

[Brijesh Singh]

On 9/10/18 6:36 AM, Borislav Petkov wrote:
> On Fri, Sep 07, 2018 at 12:57:27PM -0500, Brijesh Singh wrote:
>> Fix sme_populate_pgd() to update page flags if the PMD/PTE entry
>> already exists.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
>> Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
>> Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
>> Cc: kvm@xxxxxxxxxxxxxxx
>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> Cc: Borislav Petkov <bp@xxxxxxx>
>> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>> Cc: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
>> Cc: kvm@xxxxxxxxxxxxxxx
>> Cc: "Radim KrÄmÃÅ" <rkrcmar@xxxxxxxxxx>
>> ---
>>  arch/x86/mm/mem_encrypt_identity.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
>> index 92265d3..7659e65 100644
>> --- a/arch/x86/mm/mem_encrypt_identity.c
>> +++ b/arch/x86/mm/mem_encrypt_identity.c
>> @@ -154,9 +154,6 @@ static void __init sme_populate_pgd_large(struct sme_populate_pgd_data *ppd)
>>  		return;
>>  
>>  	pmd = pmd_offset(pud, ppd->vaddr);
>> -	if (pmd_large(*pmd))
>> -		return;
>> -
>>  	set_pmd(pmd, __pmd(ppd->paddr | ppd->pmd_flags));
>>  }
>>  
>> @@ -182,8 +179,7 @@ static void __init sme_populate_pgd(struct sme_populate_pgd_data *ppd)
>>  		return;
>>  
>>  	pte = pte_offset_map(pmd, ppd->vaddr);
>> -	if (pte_none(*pte))
>> -		set_pte(pte, __pte(ppd->paddr | ppd->pte_flags));
>> +	set_pte(pte, __pte(ppd->paddr | ppd->pte_flags));
>>  }
>>  
>>  static void __init __sme_map_range_pmd(struct sme_populate_pgd_data *ppd)
>> -- 
> This looks like a bugfix to me and as such should be:
>
> * at the beginning of the series
> * contain a Fixes: tag
> * contain Cc: <stable@xxxxxxxxxxxxxxx>
>
> Right?

Based on your advice IÂ was going to submit this series to stable
separately. This particular issue is not affecting anyone right now. The
only user to this function is sme_kernel_encrypt() which never updates
the PTE/PMD entries. I can update the commit message like this this to
clarify it.

Enhance the sme_populate_pgd() to update page flags if the PMD/PTE entry
already exists.



> With that addressed:
>
> Reviewed-by: Borislav Petkov <bp@xxxxxxx>
>