Re: [PATCH v2 3/7] x86/sev: add support for RMPOPT instruction
From: Kalra, Ashish
Date: Wed Mar 25 2026 - 17:53:41 EST
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