On 08.10.18 13:36, Ralf Ramsauer wrote:
> In some configurations, MSR_X2APIC_ICR is the most frequently accessed MSR.
> Accounting it allows for getting an overview of its noise.
> 
> Achieve this by splitting up the MSR count to msr_x2apic_icr and
> msr_other. The sum of msr_x2apic_icr and msr_other denotes the sum of
> all MSRs.
> 
> Instead of accounting MSRs in the vcpu_handle_exit() path, move it over
> to vcpu.c and implement it with a tiny inlined helper, which is called
> by vcpu_handle_msr_{write,read}. With this, we have the same path for
> both, svm and vmx.
> 
> Concerning the ABI, move MSR_* exits to the end of the end in order to
> prepare for more individual split ups of MSR accounting.
> 
> Signed-off-by: Ralf Ramsauer <ralf.ramsa...@oth-regensburg.de>
> ---
>   driver/sysfs.c                             |  7 ++++--
>   hypervisor/arch/x86/svm.c                  |  1 -
>   hypervisor/arch/x86/vcpu.c                 | 26 +++++++++++++++++-----
>   hypervisor/arch/x86/vmx.c                  |  2 --
>   include/arch/x86/asm/jailhouse_hypercall.h | 12 +++++-----
>   5 files changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/driver/sysfs.c b/driver/sysfs.c
> index 876fddc2..4232769e 100644
> --- a/driver/sysfs.c
> +++ b/driver/sysfs.c
> @@ -104,11 +104,13 @@ 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_cpuid, JAILHOUSE_CPU_STAT_VMEXITS_CPUID);
>   JAILHOUSE_CPU_STATS_ATTR(vmexits_xsetbv, JAILHOUSE_CPU_STAT_VMEXITS_XSETBV);
>   JAILHOUSE_CPU_STATS_ATTR(vmexits_exception,
>                        JAILHOUSE_CPU_STAT_VMEXITS_EXCEPTION);
> +JAILHOUSE_CPU_STATS_ATTR(vmexits_msr_other, 
> JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER);
> +JAILHOUSE_CPU_STATS_ATTR(vmexits_msr_x2apic_icr,
> +                      JAILHOUSE_CPU_STAT_VMEXITS_MSR_X2APIC_ICR);
>   #elif defined(CONFIG_ARM) || defined(CONFIG_ARM64)
>   JAILHOUSE_CPU_STATS_ATTR(vmexits_maintenance, 
> JAILHOUSE_CPU_STAT_VMEXITS_MAINTENANCE);
>   JAILHOUSE_CPU_STATS_ATTR(vmexits_virt_irq, JAILHOUSE_CPU_STAT_VMEXITS_VIRQ);
> @@ -128,10 +130,11 @@ static struct attribute *no_attrs[] = {
>       &vmexits_pio_attr.kattr.attr,
>       &vmexits_xapic_attr.kattr.attr,
>       &vmexits_cr_attr.kattr.attr,
> -     &vmexits_msr_attr.kattr.attr,
>       &vmexits_cpuid_attr.kattr.attr,
>       &vmexits_xsetbv_attr.kattr.attr,
>       &vmexits_exception_attr.kattr.attr,
> +     &vmexits_msr_other_attr.kattr.attr,
> +     &vmexits_msr_x2apic_icr_attr.kattr.attr,
>   #elif defined(CONFIG_ARM) || defined(CONFIG_ARM64)
>       &vmexits_maintenance_attr.kattr.attr,
>       &vmexits_virt_irq_attr.kattr.attr,
> diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
> index 1808e74e..e40ff0ec 100644
> --- a/hypervisor/arch/x86/svm.c
> +++ b/hypervisor/arch/x86/svm.c
> @@ -924,7 +924,6 @@ 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]++;

Now you will need to instrument svm_handle_msr_write /wrt MSR_EFER.

>               if (!vmcb->exitinfo1)
>                       res = vcpu_handle_msr_read();
>               else
> diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
> index 42184e9d..ee21d28e 100644
> --- a/hypervisor/arch/x86/vcpu.c
> +++ b/hypervisor/arch/x86/vcpu.c
> @@ -265,11 +265,24 @@ invalid_access:
>       return false;
>   }
>   
> +static inline void account_msr_exit(u64 msr)

Let's do

static inline void account_msr_exit(struct public_per_cpu *cpu_public, u64 msr)

and let the caller pass cpu_data->public.

> +{
> +     struct public_per_cpu *cpu_public = &this_cpu_data()->public;
> +
> +     if (msr == MSR_X2APIC_ICR)
> +             cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR_X2APIC_ICR]++;
> +     else
> +             cpu_public->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER]++;
> +}
> +
>   bool vcpu_handle_msr_read(void)
>   {
>       struct per_cpu *cpu_data = this_cpu_data();
> +     unsigned long msr = cpu_data->guest_regs.rcx;
> +
> +     account_msr_exit(msr);
>   
> -     switch (cpu_data->guest_regs.rcx) {
> +     switch (msr) {
>       case MSR_X2APIC_BASE ... MSR_X2APIC_END:
>               x2apic_handle_read();
>               break;
> @@ -281,8 +294,7 @@ bool vcpu_handle_msr_read(void)
>                               cpu_data->mtrr_def_type);
>               break;
>       default:
> -             panic_printk("FATAL: Unhandled MSR read: %lx\n",
> -                          cpu_data->guest_regs.rcx);
> +             panic_printk("FATAL: Unhandled MSR read: %lx\n", msr);
>               return false;
>       }
>   
> @@ -293,10 +305,13 @@ bool vcpu_handle_msr_read(void)
>   bool vcpu_handle_msr_write(void)
>   {
>       struct per_cpu *cpu_data = this_cpu_data();
> +     unsigned long msr = cpu_data->guest_regs.rcx;
>       unsigned int bit_pos, pa;
>       unsigned long val;
>   
> -     switch (cpu_data->guest_regs.rcx) {
> +     account_msr_exit(msr);
> +
> +     switch (msr) {
>       case MSR_X2APIC_BASE ... MSR_X2APIC_END:
>               if (!x2apic_handle_write())
>                       return false;
> @@ -329,8 +344,7 @@ bool vcpu_handle_msr_write(void)
>                                         cpu_data->pat : 0);
>               break;
>       default:
> -             panic_printk("FATAL: Unhandled MSR write: %lx\n",
> -                          cpu_data->guest_regs.rcx);
> +             panic_printk("FATAL: Unhandled MSR write: %lx\n", msr);
>               return false;
>       }
>   
> diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
> index 29967015..e9d66331 100644
> --- a/hypervisor/arch/x86/vmx.c
> +++ b/hypervisor/arch/x86/vmx.c
> @@ -1183,12 +1183,10 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
>                       return;
>               break;
>       case EXIT_REASON_MSR_READ:
> -             cpu_data->public.stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++;
>               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) {
>                       /* ignore writes */
>                       vcpu_skip_emulated_instruction(X86_INST_LEN_WRMSR);

Same story as with MSR_EFER on AMD.

> diff --git a/include/arch/x86/asm/jailhouse_hypercall.h 
> b/include/arch/x86/asm/jailhouse_hypercall.h
> index 3a52599f..9e72b277 100644
> --- a/include/arch/x86/asm/jailhouse_hypercall.h
> +++ b/include/arch/x86/asm/jailhouse_hypercall.h
> @@ -58,11 +58,13 @@
>   #define JAILHOUSE_CPU_STAT_VMEXITS_PIO              
> JAILHOUSE_GENERIC_CPU_STATS
>   #define JAILHOUSE_CPU_STAT_VMEXITS_XAPIC    JAILHOUSE_GENERIC_CPU_STATS + 1
>   #define JAILHOUSE_CPU_STAT_VMEXITS_CR               
> JAILHOUSE_GENERIC_CPU_STATS + 2
> -#define JAILHOUSE_CPU_STAT_VMEXITS_MSR               
> JAILHOUSE_GENERIC_CPU_STATS + 3
> -#define JAILHOUSE_CPU_STAT_VMEXITS_CPUID     JAILHOUSE_GENERIC_CPU_STATS + 4
> -#define JAILHOUSE_CPU_STAT_VMEXITS_XSETBV    JAILHOUSE_GENERIC_CPU_STATS + 5
> -#define JAILHOUSE_CPU_STAT_VMEXITS_EXCEPTION JAILHOUSE_GENERIC_CPU_STATS + 6
> -#define JAILHOUSE_NUM_CPU_STATS                      
> JAILHOUSE_GENERIC_CPU_STATS + 7
> +#define JAILHOUSE_CPU_STAT_VMEXITS_CPUID     JAILHOUSE_GENERIC_CPU_STATS + 3
> +#define JAILHOUSE_CPU_STAT_VMEXITS_XSETBV    JAILHOUSE_GENERIC_CPU_STATS + 4
> +#define JAILHOUSE_CPU_STAT_VMEXITS_EXCEPTION JAILHOUSE_GENERIC_CPU_STATS + 5
> +#define JAILHOUSE_CPU_STAT_VMEXITS_MSR_OTHER JAILHOUSE_GENERIC_CPU_STATS + 6
> +#define JAILHOUSE_CPU_STAT_VMEXITS_MSR_X2APIC_ICR \
> +                                             JAILHOUSE_GENERIC_CPU_STATS + 7
> +#define JAILHOUSE_NUM_CPU_STATS                      
> JAILHOUSE_GENERIC_CPU_STATS + 8
>   
>   /* CPUID interface */
>   #define JAILHOUSE_CPUID_SIGNATURE           0x40000000
> 

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jailhouse-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to