lkml.org 
[lkml]   [2022]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [kvm-unit-tests PATCH 1/2] x86/pmu: Update rdpmc testcase to cover #GP and emulation path
From
On 6/10/2022 5:36 am, Sean Christopherson wrote:
> On Mon, Sep 05, 2022, Like Xu wrote:
>> From: Like Xu <likexu@tencent.com>
>>
>> Specifying an unsupported PMC encoding will cause a #GP(0).
>> All testcases should be passed when the KVM_FEP prefix is added.
>>
>> Signed-off-by: Like Xu <likexu@tencent.com>
>> ---
>> lib/x86/processor.h | 5 ++++-
>> x86/pmu.c | 13 +++++++++++++
>> 2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/x86/processor.h b/lib/x86/processor.h
>> index 10bca27..9c490d9 100644
>> --- a/lib/x86/processor.h
>> +++ b/lib/x86/processor.h
>> @@ -441,7 +441,10 @@ static inline int wrmsr_safe(u32 index, u64 val)
>> static inline uint64_t rdpmc(uint32_t index)
>> {
>> uint32_t a, d;
>> - asm volatile ("rdpmc" : "=a"(a), "=d"(d) : "c"(index));
>> + if (is_fep_available())
>> + asm volatile (KVM_FEP "rdpmc" : "=a"(a), "=d"(d) : "c"(index));
>> + else
>> + asm volatile ("rdpmc" : "=a"(a), "=d"(d) : "c"(index));
>
> Hmm, not sure how I feel about the idea of always use FEP in a common helper when
> it's available. Part of me likes the idea, but part of me is worried that it
> will cause confusion due to not being explicit.
>
> Unless there's a pressing need to force emulation, let's punt the FEP stuff for
> now. More below.

Some security researchers are very interested in these corners.

To my limited testing, most KVM emulation code (at least arch/x86/kvm/emulate.c)
are not
adequately covered by test cases, and perhaps some will move them to the user space.

>
>> return a | ((uint64_t)d << 32);
>> }
>>
>> diff --git a/x86/pmu.c b/x86/pmu.c
>> index 203a9d4..11607c0 100644
>> --- a/x86/pmu.c
>> +++ b/x86/pmu.c
>> @@ -758,12 +758,25 @@ static bool pmu_is_detected(void)
>> return detect_intel_pmu();
>> }
>>
>> +static void rdpmc_unsupported_counter(void *data)
>> +{
>> + rdpmc(64);
>> +}
>> +
>> +static void check_rdpmc_cause_gp(void)
>
> Maybe check_invalid_rdpmc_gp()? There are multiple reasons RDPMC can #GP, the
> one that is being relied on to guarantee #GP is specifically that the PMC is
> invalid.

Applied.

> dd

p, :D

>
>> +{
>> + report(test_for_exception(GP_VECTOR, rdpmc_unsupported_counter, NULL),
>
> I'd really like to move away from test_for_exception() and use ASM_TRY(). Ignoring
> FEP for the moment, the most extensible solution is to provide a safe variant:
>
> static inline int rdpmc_safe(u32 index, uint64_t *val)
> {
> uint32_t a, d;
>
> asm volatile (ASM_TRY("1f")
> "rdpmc"
> : "=a"(a), "=d"(d) : "c"(index));

asm volatile (ASM_TRY("1f")
"rdpmc\n\t"
"1:"
: "=a"(a), "=d"(d) : "c"(index) : "memory");

, otherwise the compiler will complain.

> *val = (uint64_t)a | ((uint64_t)d << 32);
> return exception_vector();
> }
>
> static inline uint64_t rdpmc(uint32_t index)
> {
> uint64_t val;
> int vector = rdpmc_safe(index, &val);
>
> assert_msg(!vector, "Unexpected %s on RDPMC(%d)",
> exception_mnemonic(vector), index);
> return val;
> }

Applied.

>
>
> For long-term emulation validation, the best idea I have at this point is to do
> add a config knob to opt-in to using FEP in _all_ common helpers (where "all"
> means everything KVM actually emulates). It'd take some macro magic, but it'd
> be easier to maintain (no need to have two paths in every helper) and would be
> controllable.

With both hands up in favour. Leave it to you, as this involves a wider change.

>
>> + "rdpmc with invalid PMC index raises #GP");
>> +}
>> +
>> int main(int ac, char **av)
>> {
>> setup_vm();
>> handle_irq(PC_VECTOR, cnt_overflow);
>> buf = malloc(N*64);
>>
>> + check_rdpmc_cause_gp();
>> +
>> if (!pmu_is_detected())
>> return report_summary();
>>
>> --
>> 2.37.3
>>

\
 
 \ /
  Last update: 2022-10-19 11:03    [W:0.074 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site