Re: [PATCH v2 3/7] x86/sev: add support for RMPOPT instruction

["Kalra, Ashish" <ashish.kalra@amd.com>]

On 3/4/2026 9:56 AM, Andrew Cooper wrote:
>>> +/* + * 'val' is a system physical address aligned to 1GB OR'ed with
>>> + * a function selection. Currently supported functions are 0 + *
>>> (verify and report status) and 1 (report status). + */ +static void
>>> rmpopt(void *val) +{ + asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc" +
>>> : : "a" ((u64)val & PUD_MASK), "c" ((u64)val & 0x1) + : "memory",
>>> "cc"); +}
>> Doesn't this belong in:
>>
>> arch/x86/include/asm/special_insns.h
>>
>> Also, it's not reporting *any* status here, right? So why even talk
>> about it if the kernel isn't doing any status checks? It just makes it
>> more confusing.
> 
> The "c" (val & 0x1) constraint encodes whether this is a query or a
> mutation, but both forms produce an answer via the carry flag.
> 
> Because it's void, it's a useless helper, and the overloading via one
> parameter makes specifically poor code generation.

RMPOPT instructions for a given 1 GB page can be executed concurrently across CPUs,
reducing the overall penalty of enabling the optimization, hence we use 
on_each_cpu_mask() to execute RMPOPT instructions in parallel.

Now, the issue with that is the callback function to run on_each_cpu_mask() is of the type: 
(typedef void (*smp_call_func_t)(void *info)).

Hence, the rmpopt() function here has return "void" type and additionally takes "void *"
as parameter.

> 
> It should be:
> 
> static inline bool __rmpopt(unsigned long addr, unsigned int fn)
> {
>     bool res;
> 
>     asm volatile (".byte 0xf2, 0x0f, 0x01, 0xfc"
>                  : "=ccc" (res)
>                  : "a" (addr), "c" (fn));
> 
>     return res;
> }
>

The above constraints to use on_each_cpu_mask() is forcing the use of:

void rmpopt(void *val)

Thanks,
Ashish
 
> with:
> 
>     static inline bool rmpopt_query(unsigned long addr)
>     static inline bool rmpopt_set(unsigned long addr)
> 
> built on top.
> 
> Logic asking hardware to optimise a 1G region because of no guest memory
> should at least WARN() if hardware comes back and says "well hang on now..."
> 
> The memory barrier isn't necessary and hinders the optimiser.
> 
> ~Andrew