On 10/03/2022 18:34, Dongli Si wrote:
> From: Dongli Si <[email protected]>
>
> With nested virtualization, when the guest hypervisor runs a nested guest
> and if uses "perf record" in an AMD Milan guest hypervisor, the guest
> hypervisor dmesg will reports the following warning message:
I think it might be clearer with L0/L1/L2 terminology. Maybe something
like the following?
"With nested virtualization on AMD Milan, if "perf record" is run in an
L1 hypervisor with an L2 guest, the following warning is emitted in
the L1 guest."
>
> [] unchecked MSR access error: WRMSR to 0xc0010200 (tried to write 0x0000020000510076)
> at rIP: 0xffffffff81003a50 (x86_pmu_enable_all+0x60/0x100)
> [] Call Trace:
> [] <IRQ>
> [] ? x86_pmu_enable+0x146/0x300
> [] __perf_install_in_context+0x150/0x170
>
> The AMD64_EVENTSEL_HOSTONLY bit is defined and used on the host, while
> the guest hypervisor performance monitor unit should avoid such use.
"The AMD64_EVENTSEL_HOSTONLY bit is defined and used on the host (L0),
while the L1 hypervisor Performance Monitor Unit should avoid such use."
>
> Fixes: 1018faa6cf23 ("perf/x86/kvm: Fix Host-Only/Guest-Only counting with SVM disabled")
> Signed-off-by: Dongli Si <[email protected]>
Tested-by: Liam Merwick <[email protected]>
Reviewed-by: Liam Merwick <[email protected]>
> ---
> v2: Add run_as_host function and improve description
> v1: https://lore.kernel.org/all/[email protected]/
>
> arch/x86/events/amd/core.c | 4 +++-
> arch/x86/include/asm/hypervisor.h | 10 ++++++++++
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> index 9687a8aef01c..14cd079243a4 100644
> --- a/arch/x86/events/amd/core.c
> +++ b/arch/x86/events/amd/core.c
> @@ -8,6 +8,7 @@
> #include <linux/jiffies.h>
> #include <asm/apicdef.h>
> #include <asm/nmi.h>
> +#include <asm/hypervisor.h>
>
> #include "../perf_event.h"
>
> @@ -1027,7 +1028,8 @@ void amd_pmu_enable_virt(void)
> {
> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>
> - cpuc->perf_ctr_virt_mask = 0;
> + if (run_as_host())
> + cpuc->perf_ctr_virt_mask = 0;
>
> /* Reload all events */
> amd_pmu_disable_all();
> diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
> index e41cbf2ec41d..fcc66c23cc72 100644
> --- a/arch/x86/include/asm/hypervisor.h
> +++ b/arch/x86/include/asm/hypervisor.h
> @@ -73,11 +73,21 @@ static inline bool hypervisor_is_type(enum x86_hypervisor_type type)
> {
> return x86_hyper_type == type;
> }
> +
> +static inline bool run_as_host(void)
> +{
> + return hypervisor_is_type(X86_HYPER_NATIVE);
> +}
> #else
> static inline void init_hypervisor_platform(void) { }
> static inline bool hypervisor_is_type(enum x86_hypervisor_type type)
> {
> return type == X86_HYPER_NATIVE;
> }
> +
> +static inline bool run_as_host(void)
> +{
> + return true;
> +}
> #endif /* CONFIG_HYPERVISOR_GUEST */
> #endif /* _ASM_X86_HYPERVISOR_H */
On 11/03/2022 22:25, Liam Merwick wrote:
> On 10/03/2022 18:34, Dongli Si wrote:
> > From: Dongli Si <[email protected]>
> >
> > With nested virtualization, when the guest hypervisor runs a nested guest
> > and if uses "perf record" in an AMD Milan guest hypervisor, the guest
> > hypervisor dmesg will reports the following warning message:
>
> I think it might be clearer with L0/L1/L2 terminology. Maybe something
> like the following?
>
> "With nested virtualization on AMD Milan, if "perf record" is run in an
> L1 hypervisor with an L2 guest, the following warning is emitted in
> the L1 guest."
>
>
> >
> > [] unchecked MSR access error: WRMSR to 0xc0010200 (tried to write 0x0000020000510076)
> > at rIP: 0xffffffff81003a50 (x86_pmu_enable_all+0x60/0x100)
> > [] Call Trace:
> > [] <IRQ>
> > [] ? x86_pmu_enable+0x146/0x300
> > [] __perf_install_in_context+0x150/0x170
> >
> > The AMD64_EVENTSEL_HOSTONLY bit is defined and used on the host, while
> > the guest hypervisor performance monitor unit should avoid such use.
>
> "The AMD64_EVENTSEL_HOSTONLY bit is defined and used on the host (L0),
> while the L1 hypervisor Performance Monitor Unit should avoid such use."
>
>
>
> >
> > Fixes: 1018faa6cf23 ("perf/x86/kvm: Fix Host-Only/Guest-Only counting with SVM disabled")
> > Signed-off-by: Dongli Si <[email protected]>
>
> Tested-by: Liam Merwick <[email protected]>
> Reviewed-by: Liam Merwick <[email protected]>
Hi Liam, I will improve the description based on your suggestion
and resend the patch, thanks!