Also, you do not need to use int=0,icount=0, as they are implicitly so.

On Wed, Dec 16, 2015 at 2:52 PM, Davide Libenzi <[email protected]> wrote:

> You need to use "-cpu host" in kvm/qemu. Also, those negative numbers are
> fine.
> The IRQ trigger works that you set counter value at -icount, and wait the
> overflow to zero to trigger the PMI IRQ.
> The patch is OK, but I don't think we need to worry about PMI version 0,
> especially when we committed on anything which does not have x2apic ☺
>
>
> On Wed, Dec 16, 2015 at 2:45 PM, Barret Rhoden <[email protected]>
> wrote:
>
>> Hi -
>>
>> I get a GPF when I try to run this in qemu on the wrmsr in perf_init.
>>
>> I put together a minor fix for this (pasted below, and also at
>> origin/perf-b).  If it looks good to you, I'll merge this patch set.
>>
>> Also, I tried using it on my desktop.  The non-interrupt use looks fine:
>>
>> / $ perf record -e TLB_FLUSH:STLB_ANY,int=0,icount=0 -- sleep 10
>> Event: TLB_FLUSH:STLB_ANY
>>         7 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2
>>
>> With the interrupt:
>>
>> / $ perf record -e TLB_FLUSH:STLB_ANY,int=1,icount=20 -- sleep 10
>> Event: TLB_FLUSH:STLB_ANY
>>         281474976710643 281474976710636 281474976710636 281474976710636
>>         281474976710636 281474976710636 281474976710636 281474976710636
>>         281474976710636 28147497671
>>
>> Those are close to negative numbers (e.g. 0xfffffffffff3).  Whatever
>> the issue, we can deal with it in a follow-up patch.
>>
>> Barret
>>
>>
>> From 64d18dc7359cdda1f2980e19db62f0ec6400e31f Mon Sep 17 00:00:00 2001
>> From: Barret Rhoden <[email protected]>
>> Date: Wed, 16 Dec 2015 17:14:36 -0500
>> Subject: x86: Detect and handle missing perf support
>>
>> If a machine has perf version 0, which is the case for my Qemu, we'll get
>> a
>> GPF during initialization.  The per core initialization and any accesses
>> to
>> the Qperf file will abort if we don't have the right version.
>>
>> This assumes that if open of a Qperf fails, that there is no other way for
>> the user to trigger access to the perf MSRs.
>>
>> Signed-off-by: Barret Rhoden <[email protected]>
>> ---
>>  kern/arch/x86/devarch.c  |  2 ++
>>  kern/arch/x86/init.c     |  2 +-
>>  kern/arch/x86/perfmon.c  | 24 +++++++++++++-----------
>>  kern/arch/x86/perfmon.h  |  4 +++-
>>  kern/arch/x86/smp_boot.c |  4 ++--
>>  5 files changed, 21 insertions(+), 15 deletions(-)
>>
>> diff --git a/kern/arch/x86/devarch.c b/kern/arch/x86/devarch.c
>> index 6549891548aa..04dba60d728c 100644
>> --- a/kern/arch/x86/devarch.c
>> +++ b/kern/arch/x86/devarch.c
>> @@ -475,6 +475,8 @@ static struct chan *archopen(struct chan *c, int
>> omode)
>>         c = devopen(c, omode, archdir, Qmax, devgen);
>>         switch ((uint32_t) c->qid.path) {
>>                 case Qperf:
>> +                       if (!perfmon_supported())
>> +                               error(ENODEV, "perf is not supported");
>>                         assert(!c->aux);
>>                         c->aux = arch_create_perf_context();
>>                         break;
>> diff --git a/kern/arch/x86/init.c b/kern/arch/x86/init.c
>> index fe35276262d1..a2080485938c 100644
>> --- a/kern/arch/x86/init.c
>> +++ b/kern/arch/x86/init.c
>> @@ -80,6 +80,7 @@ void arch_init()
>>         save_fp_state(&x86_default_fpu); /* used in arch/trap.h for fpu
>> init */
>>         pci_init();
>>         vmm_init();
>> +       perfmon_global_init();
>>         // this returns when all other cores are done and ready to
>> receive IPIs
>>         #ifdef CONFIG_SINGLE_CORE
>>                 smp_percpu_init();
>> @@ -88,7 +89,6 @@ void arch_init()
>>         #endif
>>         proc_init();
>>
>> -       perfmon_init();
>>         cons_irq_init();
>>         intel_lpc_init();
>>  #ifdef CONFIG_ENABLE_LEGACY_USB
>> diff --git a/kern/arch/x86/perfmon.c b/kern/arch/x86/perfmon.c
>> index 810be3c6e5ae..4ea89d8f3ba8 100644
>> --- a/kern/arch/x86/perfmon.c
>> +++ b/kern/arch/x86/perfmon.c
>> @@ -291,23 +291,25 @@ static void perfmon_arm_irq(void)
>>         write_mmreg32(LAPIC_LVT_PERFMON, IdtLAPIC_PCINT);
>>  }
>>
>> -void perfmon_init(void)
>> +bool perfmon_supported(void)
>> +{
>> +       return cpu_caps.perfmon_version >= 2;
>> +}
>> +
>> +void perfmon_global_init(void)
>> +{
>> +       perfmon_read_cpu_caps(&cpu_caps);
>> +}
>> +
>> +void perfmon_pcpu_init(void)
>>  {
>>         int i;
>>
>> +       if (!perfmon_supported())
>> +               return;
>>         /* Enable user level access to the performance counters */
>>         lcr4(rcr4() | CR4_PCE);
>>
>> -       /* This will be called from every core, no need to execute more
>> than once.
>> -        * All the call to perfmon_init() will be done when the core
>> boots, so
>> -        * they will be no perfmon users calling it, while
>> perfmon_read_cpu_caps()
>> -        * is executing.
>> -        * All the cores will be writing the same values, so even from
>> that POV,
>> -        * no serialization is required.
>> -        */
>> -       if (cpu_caps.perfmon_version == 0)
>> -               perfmon_read_cpu_caps(&cpu_caps);
>> -
>>         /* Reset all the counters and selectors to zero.
>>          */
>>         write_msr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
>> diff --git a/kern/arch/x86/perfmon.h b/kern/arch/x86/perfmon.h
>> index a655098df9bb..822b96a9e9cf 100644
>> --- a/kern/arch/x86/perfmon.h
>> +++ b/kern/arch/x86/perfmon.h
>> @@ -49,7 +49,9 @@ struct perfmon_status {
>>         uint64_t cores_values[0];
>>  };
>>
>> -void perfmon_init(void);
>> +bool perfmon_supported(void);
>> +void perfmon_global_init(void);
>> +void perfmon_pcpu_init(void);
>>  void perfmon_interrupt(struct hw_trapframe *hw_tf, void *data);
>>  void perfmon_get_cpu_caps(struct perfmon_cpu_caps *pcc);
>>  int perfmon_open_event(const struct core_set *cset, struct
>> perfmon_session *ps,
>> diff --git a/kern/arch/x86/smp_boot.c b/kern/arch/x86/smp_boot.c
>> index 7f6f4d9bf1fb..e248d9edee1f 100644
>> --- a/kern/arch/x86/smp_boot.c
>> +++ b/kern/arch/x86/smp_boot.c
>> @@ -302,7 +302,7 @@ void __arch_pcpu_init(uint32_t coreid)
>>         assert(read_msr(MSR_KERN_GS_BASE) == (uint64_t)pcpui);
>>         /* Don't try setting up til after setting GS */
>>         x86_sysenter_init(x86_get_stacktop_tss(pcpui->tss));
>> -       /* need to init perfctr before potentiall using it in timer
>> handler */
>> -       perfmon_init();
>> +       /* need to init perfctr before potentially using it in timer
>> handler */
>> +       perfmon_pcpu_init();
>>         vmm_pcpu_init();
>>  }
>> --
>> 2.6.0.rc2.230.g3dd15c0
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Akaros" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to [email protected].
>> To post to this group, send email to [email protected].
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Akaros" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to