Re: [RFC Part2 PATCH 03/30] x86: add helper functions for RMPUPDATE and PSMASH instruction

[Brijesh Singh]

On 4/15/21 1:00 PM, Borislav Petkov wrote:
> On Wed, Mar 24, 2021 at 12:04:09PM -0500, Brijesh Singh wrote:
>> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
>> index 06394b6d56b2..7a0138cb3e17 100644
>> --- a/arch/x86/mm/mem_encrypt.c
>> +++ b/arch/x86/mm/mem_encrypt.c
>> @@ -644,3 +644,44 @@ rmpentry_t *lookup_page_in_rmptable(struct page *page, int *level)
>>  	return entry;
>>  }
>>  EXPORT_SYMBOL_GPL(lookup_page_in_rmptable);
>> +
>> +int rmptable_psmash(struct page *page)
> psmash() should be enough like all those other wrappers around insns.

Noted.


>
>> +{
>> +	unsigned long spa = page_to_pfn(page) << PAGE_SHIFT;
>> +	int ret;
>> +
>> +	if (!static_branch_unlikely(&snp_enable_key))
>> +		return -ENXIO;
>> +
>> +	/* Retry if another processor is modifying the RMP entry. */
> Also, a comment here should say which binutils version supports the
> insn mnemonic so that it can be converted to "psmash" later. Ditto for
> rmpupdate below.
>
> Looking at the binutils repo, it looks like since version 2.36.
>
> /me rebuilds objdump...

Sure, I will add comment.

>> +	do {
>> +		asm volatile(".byte 0xF3, 0x0F, 0x01, 0xFF"
>> +			      : "=a"(ret)
>> +			      : "a"(spa)
>> +			      : "memory", "cc");
>> +	} while (ret == PSMASH_FAIL_INUSE);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(rmptable_psmash);
>> +
>> +int rmptable_rmpupdate(struct page *page, struct rmpupdate *val)
> rmpupdate()
>
>> +{
>> +	unsigned long spa = page_to_pfn(page) << PAGE_SHIFT;
>> +	bool flush = true;
>> +	int ret;
>> +
>> +	if (!static_branch_unlikely(&snp_enable_key))
>> +		return -ENXIO;
>> +
>> +	/* Retry if another processor is modifying the RMP entry. */
>> +	do {
>> +		asm volatile(".byte 0xF2, 0x0F, 0x01, 0xFE"
>> +			     : "=a"(ret)
>> +			     : "a"(spa), "c"((unsigned long)val), "d"(flush)
> 					    ^^^^^^^^^^^^^^^
>
> what's the cast for?
No need to cast it. I will drop in next round.
> "d"(flush)?

Hmm, either it copied this function from pvalidate or old internal APM
may had the flush. I will fix it in the next rev. thanks for pointing it.


>
> There's nothing in the APM talking about RMPUPDATE taking an input arg
> in %rdx?
>