Re: [PATCH] core: statistics: x86: account for MSR_ICR
On 10/8/18 12:21 PM, Jan Kiszka wrote: > On 05.10.18 20:06, Ralf Ramsauer wrote: >> In some configurations, MSR_ICR is the most frequently accessed MSR. >> Accounting it allows for getting an overview of the noise introduced by >> it. >> >> Achieve this by splitting up the MSR count to msr_icr and msr_other. >> The sum of >> msr_icr and msr_other denotes the sum of all MSRs. >> >> Signed-off-by: Ralf Ramsauer >> --- >> driver/sysfs.c | 6 ++-- >> hypervisor/arch/x86/svm.c | 7 - >> hypervisor/arch/x86/vmx.c | 32 -- >> include/arch/x86/asm/jailhouse_hypercall.h | 11 >> 4 files changed, 40 insertions(+), 16 deletions(-) >> >> diff --git a/driver/sysfs.c b/driver/sysfs.c >> index 876fddc2..2a17f0be 100644 >> --- a/driver/sysfs.c >> +++ b/driver/sysfs.c >> @@ -104,7 +104,8 @@ JAILHOUSE_CPU_STATS_ATTR(vmexits_hypercall, >> JAILHOUSE_CPU_STATS_ATTR(vmexits_pio, JAILHOUSE_CPU_STAT_VMEXITS_PIO); >> JAILHOUSE_CPU_STATS_ATTR(vmexits_xapic, >> JAILHOUSE_CPU_STAT_VMEXITS_XAPIC); >> JAILHOUSE_CPU_STATS_ATTR(vmexits_cr, JAILHOUSE_CPU_STAT_VMEXITS_CR); >> -JAILHOUSE_CPU_STATS_ATTR(vmexits_msr, JAILHOUSE_CPU_STAT_VMEXITS_MSR); >> +JAILHOUSE_CPU_STATS_ATTR(vmexits_msr_icr, >> JAILHOUSE_CPU_STAT_VMEXITS_MSR_ICR); >> +JAILHOUSE_CPU_STATS_ATTR(vmexits_msr_other, >> JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER); >> JAILHOUSE_CPU_STATS_ATTR(vmexits_cpuid, >> JAILHOUSE_CPU_STAT_VMEXITS_CPUID); >> JAILHOUSE_CPU_STATS_ATTR(vmexits_xsetbv, >> JAILHOUSE_CPU_STAT_VMEXITS_XSETBV); >> JAILHOUSE_CPU_STATS_ATTR(vmexits_exception, >> @@ -128,7 +129,8 @@ static struct attribute *no_attrs[] = { >> _pio_attr.kattr.attr, >> _xapic_attr.kattr.attr, >> _cr_attr.kattr.attr, >> - _msr_attr.kattr.attr, >> + _msr_icr_attr.kattr.attr, >> + _msr_other_attr.kattr.attr, >> _cpuid_attr.kattr.attr, >> _xsetbv_attr.kattr.attr, >> _exception_attr.kattr.attr, >> diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c >> index 1808e74e..ed6f1d54 100644 >> --- a/hypervisor/arch/x86/svm.c >> +++ b/hypervisor/arch/x86/svm.c >> @@ -924,7 +924,12 @@ void vcpu_handle_exit(struct per_cpu *cpu_data) >> vcpu_handle_cpuid(); >> goto vmentry; >> case VMEXIT_MSR: >> - cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++; >> + if (cpu_data->guest_regs.rcx == MSR_X2APIC_ICR) >> + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR_ICR]++; >> + else >> + cpu_public->stats[ >> + JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER]++; >> + >> if (!vmcb->exitinfo1) >> res = vcpu_handle_msr_read(); >> else >> diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c >> index 29967015..ff8f179e 100644 >> --- a/hypervisor/arch/x86/vmx.c >> +++ b/hypervisor/arch/x86/vmx.c >> @@ -1159,16 +1159,18 @@ void vcpu_vendor_get_mmio_intercept(struct >> vcpu_mmio_intercept *mmio) >> void vcpu_handle_exit(struct per_cpu *cpu_data) >> { >> + struct public_per_cpu *cpu_public = _data->public; >> u32 reason = vmcs_read32(VM_EXIT_REASON); >> + u64 msr; >> - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_TOTAL]++; >> + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_TOTAL]++; >> switch (reason) { >> case EXIT_REASON_EXCEPTION_NMI: >> vmx_handle_exception_nmi(); >> return; >> case EXIT_REASON_PREEMPTION_TIMER: >> - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_MANAGEMENT]++; >> + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MANAGEMENT]++; >> vmx_check_events(); >> return; >> case EXIT_REASON_CPUID: >> @@ -1178,18 +1180,32 @@ void vcpu_handle_exit(struct per_cpu *cpu_data) >> vcpu_handle_hypercall(); >> return; >> case EXIT_REASON_CR_ACCESS: >> - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_CR]++; >> + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_CR]++; >> if (vmx_handle_cr()) >> return; >> break; >> case EXIT_REASON_MSR_READ: >> - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++; >> + msr = cpu_data->guest_regs.rcx; >> + >> + if (msr == MSR_X2APIC_ICR) >> + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR_ICR]++; >> + else >> + cpu_public->stats[ >> + JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER]++; >> + > > This is no a good place for dispatching. We better push this down into > vpcu_handle_msr_*, or even further. Good point! I tried this in the first place, but then realised that svm uses a different write path. Didn't see that it calls vcpu_handle_msr() in the end as well. Will change this. > >> if (vcpu_handle_msr_read()) >> return; >> break; >> case EXIT_REASON_MSR_WRITE: >> -
Re: [PATCH] core: statistics: x86: account for MSR_ICR
On 05.10.18 20:06, Ralf Ramsauer wrote: In some configurations, MSR_ICR is the most frequently accessed MSR. Accounting it allows for getting an overview of the noise introduced by it. Achieve this by splitting up the MSR count to msr_icr and msr_other. The sum of msr_icr and msr_other denotes the sum of all MSRs. Signed-off-by: Ralf Ramsauer --- driver/sysfs.c | 6 ++-- hypervisor/arch/x86/svm.c | 7 - hypervisor/arch/x86/vmx.c | 32 -- include/arch/x86/asm/jailhouse_hypercall.h | 11 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/driver/sysfs.c b/driver/sysfs.c index 876fddc2..2a17f0be 100644 --- a/driver/sysfs.c +++ b/driver/sysfs.c @@ -104,7 +104,8 @@ JAILHOUSE_CPU_STATS_ATTR(vmexits_hypercall, JAILHOUSE_CPU_STATS_ATTR(vmexits_pio, JAILHOUSE_CPU_STAT_VMEXITS_PIO); JAILHOUSE_CPU_STATS_ATTR(vmexits_xapic, JAILHOUSE_CPU_STAT_VMEXITS_XAPIC); JAILHOUSE_CPU_STATS_ATTR(vmexits_cr, JAILHOUSE_CPU_STAT_VMEXITS_CR); -JAILHOUSE_CPU_STATS_ATTR(vmexits_msr, JAILHOUSE_CPU_STAT_VMEXITS_MSR); +JAILHOUSE_CPU_STATS_ATTR(vmexits_msr_icr, JAILHOUSE_CPU_STAT_VMEXITS_MSR_ICR); +JAILHOUSE_CPU_STATS_ATTR(vmexits_msr_other, JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER); JAILHOUSE_CPU_STATS_ATTR(vmexits_cpuid, JAILHOUSE_CPU_STAT_VMEXITS_CPUID); JAILHOUSE_CPU_STATS_ATTR(vmexits_xsetbv, JAILHOUSE_CPU_STAT_VMEXITS_XSETBV); JAILHOUSE_CPU_STATS_ATTR(vmexits_exception, @@ -128,7 +129,8 @@ static struct attribute *no_attrs[] = { _pio_attr.kattr.attr, _xapic_attr.kattr.attr, _cr_attr.kattr.attr, - _msr_attr.kattr.attr, + _msr_icr_attr.kattr.attr, + _msr_other_attr.kattr.attr, _cpuid_attr.kattr.attr, _xsetbv_attr.kattr.attr, _exception_attr.kattr.attr, diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c index 1808e74e..ed6f1d54 100644 --- a/hypervisor/arch/x86/svm.c +++ b/hypervisor/arch/x86/svm.c @@ -924,7 +924,12 @@ void vcpu_handle_exit(struct per_cpu *cpu_data) vcpu_handle_cpuid(); goto vmentry; case VMEXIT_MSR: - cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++; + if (cpu_data->guest_regs.rcx == MSR_X2APIC_ICR) + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR_ICR]++; + else + cpu_public->stats[ + JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER]++; + if (!vmcb->exitinfo1) res = vcpu_handle_msr_read(); else diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c index 29967015..ff8f179e 100644 --- a/hypervisor/arch/x86/vmx.c +++ b/hypervisor/arch/x86/vmx.c @@ -1159,16 +1159,18 @@ void vcpu_vendor_get_mmio_intercept(struct vcpu_mmio_intercept *mmio) void vcpu_handle_exit(struct per_cpu *cpu_data) { + struct public_per_cpu *cpu_public = _data->public; u32 reason = vmcs_read32(VM_EXIT_REASON); + u64 msr; - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_TOTAL]++; + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_TOTAL]++; switch (reason) { case EXIT_REASON_EXCEPTION_NMI: vmx_handle_exception_nmi(); return; case EXIT_REASON_PREEMPTION_TIMER: - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_MANAGEMENT]++; + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MANAGEMENT]++; vmx_check_events(); return; case EXIT_REASON_CPUID: @@ -1178,18 +1180,32 @@ void vcpu_handle_exit(struct per_cpu *cpu_data) vcpu_handle_hypercall(); return; case EXIT_REASON_CR_ACCESS: - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_CR]++; + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_CR]++; if (vmx_handle_cr()) return; break; case EXIT_REASON_MSR_READ: - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++; + msr = cpu_data->guest_regs.rcx; + + if (msr == MSR_X2APIC_ICR) + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR_ICR]++; + else + cpu_public->stats[ + JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER]++; + This is no a good place for dispatching. We better push this down into vpcu_handle_msr_*, or even further. if (vcpu_handle_msr_read()) return; break; case EXIT_REASON_MSR_WRITE: - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++; - if (cpu_data->guest_regs.rcx == MSR_IA32_PERF_GLOBAL_CTRL) { + msr = cpu_data->guest_regs.rcx; + + if (msr == MSR_X2APIC_ICR) +
[PATCH] core: statistics: x86: account for MSR_ICR
In some configurations, MSR_ICR is the most frequently accessed MSR. Accounting it allows for getting an overview of the noise introduced by it. Achieve this by splitting up the MSR count to msr_icr and msr_other. The sum of msr_icr and msr_other denotes the sum of all MSRs. Signed-off-by: Ralf Ramsauer --- driver/sysfs.c | 6 ++-- hypervisor/arch/x86/svm.c | 7 - hypervisor/arch/x86/vmx.c | 32 -- include/arch/x86/asm/jailhouse_hypercall.h | 11 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/driver/sysfs.c b/driver/sysfs.c index 876fddc2..2a17f0be 100644 --- a/driver/sysfs.c +++ b/driver/sysfs.c @@ -104,7 +104,8 @@ JAILHOUSE_CPU_STATS_ATTR(vmexits_hypercall, JAILHOUSE_CPU_STATS_ATTR(vmexits_pio, JAILHOUSE_CPU_STAT_VMEXITS_PIO); JAILHOUSE_CPU_STATS_ATTR(vmexits_xapic, JAILHOUSE_CPU_STAT_VMEXITS_XAPIC); JAILHOUSE_CPU_STATS_ATTR(vmexits_cr, JAILHOUSE_CPU_STAT_VMEXITS_CR); -JAILHOUSE_CPU_STATS_ATTR(vmexits_msr, JAILHOUSE_CPU_STAT_VMEXITS_MSR); +JAILHOUSE_CPU_STATS_ATTR(vmexits_msr_icr, JAILHOUSE_CPU_STAT_VMEXITS_MSR_ICR); +JAILHOUSE_CPU_STATS_ATTR(vmexits_msr_other, JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER); JAILHOUSE_CPU_STATS_ATTR(vmexits_cpuid, JAILHOUSE_CPU_STAT_VMEXITS_CPUID); JAILHOUSE_CPU_STATS_ATTR(vmexits_xsetbv, JAILHOUSE_CPU_STAT_VMEXITS_XSETBV); JAILHOUSE_CPU_STATS_ATTR(vmexits_exception, @@ -128,7 +129,8 @@ static struct attribute *no_attrs[] = { _pio_attr.kattr.attr, _xapic_attr.kattr.attr, _cr_attr.kattr.attr, - _msr_attr.kattr.attr, + _msr_icr_attr.kattr.attr, + _msr_other_attr.kattr.attr, _cpuid_attr.kattr.attr, _xsetbv_attr.kattr.attr, _exception_attr.kattr.attr, diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c index 1808e74e..ed6f1d54 100644 --- a/hypervisor/arch/x86/svm.c +++ b/hypervisor/arch/x86/svm.c @@ -924,7 +924,12 @@ void vcpu_handle_exit(struct per_cpu *cpu_data) vcpu_handle_cpuid(); goto vmentry; case VMEXIT_MSR: - cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++; + if (cpu_data->guest_regs.rcx == MSR_X2APIC_ICR) + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR_ICR]++; + else + cpu_public->stats[ + JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER]++; + if (!vmcb->exitinfo1) res = vcpu_handle_msr_read(); else diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c index 29967015..ff8f179e 100644 --- a/hypervisor/arch/x86/vmx.c +++ b/hypervisor/arch/x86/vmx.c @@ -1159,16 +1159,18 @@ void vcpu_vendor_get_mmio_intercept(struct vcpu_mmio_intercept *mmio) void vcpu_handle_exit(struct per_cpu *cpu_data) { + struct public_per_cpu *cpu_public = _data->public; u32 reason = vmcs_read32(VM_EXIT_REASON); + u64 msr; - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_TOTAL]++; + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_TOTAL]++; switch (reason) { case EXIT_REASON_EXCEPTION_NMI: vmx_handle_exception_nmi(); return; case EXIT_REASON_PREEMPTION_TIMER: - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_MANAGEMENT]++; + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MANAGEMENT]++; vmx_check_events(); return; case EXIT_REASON_CPUID: @@ -1178,18 +1180,32 @@ void vcpu_handle_exit(struct per_cpu *cpu_data) vcpu_handle_hypercall(); return; case EXIT_REASON_CR_ACCESS: - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_CR]++; + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_CR]++; if (vmx_handle_cr()) return; break; case EXIT_REASON_MSR_READ: - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++; + msr = cpu_data->guest_regs.rcx; + + if (msr == MSR_X2APIC_ICR) + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR_ICR]++; + else + cpu_public->stats[ + JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER]++; + if (vcpu_handle_msr_read()) return; break; case EXIT_REASON_MSR_WRITE: - cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++; - if (cpu_data->guest_regs.rcx == MSR_IA32_PERF_GLOBAL_CTRL) { + msr = cpu_data->guest_regs.rcx; + + if (msr == MSR_X2APIC_ICR) + cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR_ICR]++; + else + cpu_public->stats[ +