Re: [PATCH] core: statistics: x86: account for MSR_ICR

2018-10-08 Thread Ralf Ramsauer



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

2018-10-08 Thread Jan Kiszka

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

2018-10-05 Thread Ralf Ramsauer
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[
+