Re: [PATCH] x86/svm: Intercept and terminate RDPRU with #UD

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/09/2021 10:57, Jan Beulich wrote:
> On 08.09.2021 18:19, Andrew Cooper wrote:
>> The RDPRU instruction isn't supported at all (and it is unclear how this can
>> ever be offered safely to guests).
> An implicit hint to me to consider "x86emul: support RDPRU" rejected? That's
> still in my queue waiting for ...
>
>>  However, a guest which ignores CPUID and
>> blindly executes RDPRU will find that it functions.
>>
>> Use the intercept and terminate with #UD.  While at it, fold SKINIT into the
>> same "unconditionally disabled" path.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> ---
>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> CC: Wei Liu <wl@xxxxxxx>
>>
>> I could have sworn that I'd posted this before, but I can't locate any
>> evidence of it.  I've got a separate patch adding the CPUID infrastructure 
>> for
>> rdpru, but that is better left until we've got more libx86 levelling logic in
>> place.
> ... this. Which - if exposure to guests makes no sense - would seem pretty
> pointless then as well?

Well - given how many people want aperf/mperf, I suspect we might end up
having it as an opt-in only thing, so no - this isn't a rejection of the
x86emul work.

We can *almost* provide a safe way for VM's to use this infrastructure
if we had something like Linux's restartable sequences infrastructure. 
In that case, only an SMI would mess things up, from the guest kernel's
perspective.

On the other hand, it's probably easier to lobby Intel and AMD for an
interface here which isn't fundamentally broken in the face of NMI/SMI/etc.

>
>> --- a/xen/arch/x86/hvm/svm/vmcb.c
>> +++ b/xen/arch/x86/hvm/svm/vmcb.c
>> @@ -70,7 +70,8 @@ static int construct_vmcb(struct vcpu *v)
>>          GENERAL2_INTERCEPT_STGI        | GENERAL2_INTERCEPT_CLGI        |
>>          GENERAL2_INTERCEPT_SKINIT      | GENERAL2_INTERCEPT_MWAIT       |
>>          GENERAL2_INTERCEPT_WBINVD      | GENERAL2_INTERCEPT_MONITOR     |
>> -        GENERAL2_INTERCEPT_XSETBV      | GENERAL2_INTERCEPT_ICEBP;
>> +        GENERAL2_INTERCEPT_XSETBV      | GENERAL2_INTERCEPT_ICEBP       |
>> +        GENERAL2_INTERCEPT_RDPRU;
> Some of the other intercepts here suggest it is okay to enable ones
> in the absence of support in the underlying hardware, but I thought
> I'd double check. I couldn't find any statement either way in the PM.
> Assuming this is fine

They're just bits in memory.  Older CPUs will ignore them, and indeed -
pre-RDPRU hardware is fine running with this intercept bit set.

Honestly, I've got half a mind to default the intercepts to ~0 rather
than 0.  For running older Xen on new hardware, it will lead to fewer
unexpected surprises.

> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

~Andrew