Re: [PATCH v1 3/5] xen/PMU: Initialization code for Xen PMU

From: Konrad Rzeszutek Wilk
Date: Mon Sep 23 2013 - 09:26:48 EST

Next message: Jeff Moyer: "Re: [PATCH] block: check CAP_SYS_ADMIN before calling driver BLKROSET"
Previous message: Konrad Rzeszutek Wilk: "Re: [PATCH v1 2/5] xen/PMU: Sysfs interface for setting Xen PMU mode"
Next in thread: Boris Ostrovsky: "Re: [PATCH v1 3/5] xen/PMU: Initialization code for Xen PMU"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, Sep 10, 2013 at 11:31:48AM -0400, Boris Ostrovsky wrote:
> Map shared data structure that will hold CPU registers, VPMU context,
> VCPU/PCPI IDs of the VCPU interrupted by PMU interrupt. Hypervisor
> fills this information in its handler and passes it to the guest for
> further processing.
>
> Set up PMU VIRQ.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> ---
> arch/x86/xen/Makefile | 2 +-
> arch/x86/xen/pmu.c | 122 +++++++++++++++++++++++++++++++++++++++++
> arch/x86/xen/pmu.h | 12 ++++
> arch/x86/xen/smp.c | 31 ++++++++++-
> include/xen/interface/xen.h | 1 +
> include/xen/interface/xenpmu.h | 77 ++++++++++++++++++++++++++
> 6 files changed, 243 insertions(+), 2 deletions(-)
> create mode 100644 arch/x86/xen/pmu.c
> create mode 100644 arch/x86/xen/pmu.h
>
> diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
> index 96ab2c0..b187df5 100644
> --- a/arch/x86/xen/Makefile
> +++ b/arch/x86/xen/Makefile
> @@ -13,7 +13,7 @@ CFLAGS_mmu.o := $(nostackp)
> obj-y := enlighten.o setup.o multicalls.o mmu.o irq.o \
> time.o xen-asm.o xen-asm_$(BITS).o \
> grant-table.o suspend.o platform-pci-unplug.o \
> - p2m.o
> + p2m.o pmu.o

Perhaps guard the build of this based on CONFIG_PERF_EVENTS?

That would of course mean you also have to create in xenpmu.h
static inline empy functions for xen_pmu_finish and xen_pmu_init in case
CONFIG_PERF_EVENTS is not set.

>
> obj-$(CONFIG_EVENT_TRACING) += trace.o
>
> diff --git a/arch/x86/xen/pmu.c b/arch/x86/xen/pmu.c
> new file mode 100644
> index 0000000..da061d4
> --- /dev/null
> +++ b/arch/x86/xen/pmu.c
> @@ -0,0 +1,122 @@
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +
> +#include <asm/xen/hypercall.h>
> +#include <xen/page.h>
> +#include <xen/interface/xen.h>
> +#include <xen/interface/vcpu.h>
> +#include <xen/interface/xenpmu.h>
> +
> +#include "xen-ops.h"
> +#include "pmu.h"
> +
> +/* x86_pmu.handle_irq definition */
> +#include <../kernel/cpu/perf_event.h>
> +
> +
> +/* Shared page between hypervisor and domain */
> +DEFINE_PER_CPU(struct xenpmu_data *, xenpmu_shared);
> +
> +/* perf callbacks*/
> +int xen_is_in_guest(void)
> +{
> + struct xenpmu_data *xenpmu_data = per_cpu(xenpmu_shared,
> + smp_processor_id());
> +
> + if (!xen_initial_domain() ||
> + xenpmu_data->domain_id > DOMID_SELF || xenpmu_data->domain_id == 0)
> + return 0;
> +
> + return 1;
> +}
> +
> +static int xen_is_user_mode(void)
> +{
> + struct xenpmu_data *xenpmu_data = per_cpu(xenpmu_shared,
> + smp_processor_id());
> + return ((xenpmu_data->regs.cs & 3) == 3);
> +}
> +
> +static unsigned long xen_get_guest_ip(void)
> +{
> + struct xenpmu_data *xenpmu_data = per_cpu(xenpmu_shared,
> + smp_processor_id());
> + return xenpmu_data->regs.eip;
> +}
> +
> +static struct perf_guest_info_callbacks xen_guest_cbs = {
> + .is_in_guest = xen_is_in_guest,
> + .is_user_mode = xen_is_user_mode,
> + .get_guest_ip = xen_get_guest_ip,
> +};
> +
> +/* Convert registers from Xen's format to Linux' */
> +static void xen_convert_regs(struct cpu_user_regs *xen_regs,
> + struct pt_regs *regs)
> +{
> + regs->ip = xen_regs->eip;
> + regs->cs = xen_regs->cs;
> +}
> +
> +irqreturn_t xen_pmu_irq_handler(int irq, void *dev_id)
> +{
> + int ret = IRQ_NONE;
> + struct pt_regs regs;
> + struct xenpmu_data *xenpmu_data = per_cpu(xenpmu_shared,
> + smp_processor_id());
> +
> + xen_convert_regs(&xenpmu_data->regs, &regs);
> + if (x86_pmu.handle_irq(&regs))
> + ret = IRQ_HANDLED;
> +
> + return ret;
> +}
> +
> +int xen_pmu_init(int cpu)
> +{
> + int ret = 0;
> + struct xenpmu_params xp;
> + unsigned long pfn;
> + struct xenpmu_data *xenpmu_data;
> +
> + BUILD_BUG_ON(sizeof(struct xenpmu_data) > PAGE_SIZE);
> + xenpmu_data = vmalloc(PAGE_SIZE);
> + if (!xenpmu_data) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> + pfn = vmalloc_to_pfn((char *)xenpmu_data);
> +
> + xp.mfn = pfn_to_mfn(pfn);
> + xp.vcpu = cpu;
> + xp.version.maj = XENPMU_VER_MAJ;
> + xp.version.min = XENPMU_VER_MIN;
> + ret = HYPERVISOR_xenpmu_op(XENPMU_init, &xp);
> + if (ret)
> + goto fail;
> +
> + per_cpu(xenpmu_shared, cpu) = xenpmu_data;
> +
> + if (cpu == 0)
> + perf_register_guest_info_callbacks(&xen_guest_cbs);
> +
> + return ret;
> +
> +fail:
> + vfree(xenpmu_data);
> + return ret;
> +}
> +
> +void xen_pmu_finish(int cpu)
> +{
> + struct xenpmu_params xp;
> +
> + xp.vcpu = cpu;
> + xp.version.maj = XENPMU_VER_MAJ;
> + xp.version.min = XENPMU_VER_MIN;
> +
> + (void)HYPERVISOR_xenpmu_op(XENPMU_finish, &xp);
> +
> + vfree(per_cpu(xenpmu_shared, cpu));
> + per_cpu(xenpmu_shared, cpu) = NULL;

I think you are missing:

perf_unregister_guest_info_callbacks when this is the bootup CPU.

And you should probably move this around to be:

if (cpu == 0 && num_online_cpus() == 1)
perf_unregister_guest_info_callbacks(..)
per_cpu(xenpmu_shared, cpu) = NULL;
vfree(per_cpu(xenpmu_shared, cpu))

?
> +}
> diff --git a/arch/x86/xen/pmu.h b/arch/x86/xen/pmu.h
> new file mode 100644
> index 0000000..51de7d2
> --- /dev/null
> +++ b/arch/x86/xen/pmu.h
> @@ -0,0 +1,12 @@
> +#ifndef __XEN_PMU_H
> +#define __XEN_PMU_H
> +
> +#include <xen/interface/xenpmu.h>
> +
> +irqreturn_t xen_pmu_irq_handler(int irq, void *dev_id);
> +int xen_pmu_init(int cpu);
> +void xen_pmu_finish(int cpu);
> +
> +DECLARE_PER_CPU(struct xenpmu_data *, xenpmu_shared);
> +
> +#endif /* __XEN_PMU_H */
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index ca92754..17a88d1 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -26,6 +26,7 @@
>
> #include <xen/interface/xen.h>
> #include <xen/interface/vcpu.h>
> +#include <xen/interface/xenpmu.h>
>
> #include <asm/xen/interface.h>
> #include <asm/xen/hypercall.h>
> @@ -37,6 +38,7 @@
> #include <xen/hvc-console.h>
> #include "xen-ops.h"
> #include "mmu.h"
> +#include "pmu.h"
>
> cpumask_var_t xen_cpu_initialized_map;
>
> @@ -49,6 +51,7 @@ static DEFINE_PER_CPU(struct xen_common_irq, xen_callfunc_irq) = { .irq = -1 };
> static DEFINE_PER_CPU(struct xen_common_irq, xen_callfuncsingle_irq) = { .irq = -1 };
> static DEFINE_PER_CPU(struct xen_common_irq, xen_irq_work) = { .irq = -1 };
> static DEFINE_PER_CPU(struct xen_common_irq, xen_debug_irq) = { .irq = -1 };
> +static DEFINE_PER_CPU(struct xen_common_irq, xen_pmu_irq) = { .irq = -1 };
>
> static irqreturn_t xen_call_function_interrupt(int irq, void *dev_id);
> static irqreturn_t xen_call_function_single_interrupt(int irq, void *dev_id);
> @@ -139,11 +142,18 @@ static void xen_smp_intr_free(unsigned int cpu)
> kfree(per_cpu(xen_irq_work, cpu).name);
> per_cpu(xen_irq_work, cpu).name = NULL;
> }
> +
> + if (per_cpu(xen_pmu_irq, cpu).irq >= 0) {
> + unbind_from_irqhandler(per_cpu(xen_pmu_irq, cpu).irq, NULL);
> + per_cpu(xen_pmu_irq, cpu).irq = -1;
> + kfree(per_cpu(xen_pmu_irq, cpu).name);
> + per_cpu(xen_pmu_irq, cpu).name = NULL;
> + }
> };
> static int xen_smp_intr_init(unsigned int cpu)
> {
> int rc;
> - char *resched_name, *callfunc_name, *debug_name;
> + char *resched_name, *callfunc_name, *debug_name, *pmu_name;
>
> resched_name = kasprintf(GFP_KERNEL, "resched%d", cpu);
> rc = bind_ipi_to_irqhandler(XEN_RESCHEDULE_VECTOR,
> @@ -209,6 +219,18 @@ static int xen_smp_intr_init(unsigned int cpu)
> per_cpu(xen_irq_work, cpu).irq = rc;
> per_cpu(xen_irq_work, cpu).name = callfunc_name;
>
> + if (per_cpu(xenpmu_shared, cpu)) {
> + pmu_name = kasprintf(GFP_KERNEL, "pmu%d", cpu);
> + rc = bind_virq_to_irqhandler(VIRQ_XENPMU, cpu,
> + xen_pmu_irq_handler,
> + IRQF_PERCPU|IRQF_NOBALANCING,
> + pmu_name, NULL);
> + if (rc < 0)
> + goto fail;
> + per_cpu(xen_pmu_irq, cpu).irq = rc;
> + per_cpu(xen_pmu_irq, cpu).name = pmu_name;
> + }
> +
> return 0;
>
> fail:
> @@ -307,6 +329,9 @@ static void __init xen_smp_prepare_cpus(unsigned int max_cpus)
> }
> set_cpu_sibling_map(0);
>
> + if (xen_pmu_init(0))
> + pr_err("Could not initialize VPMU for VCPU 0\n");
> +
> if (xen_smp_intr_init(0))
> BUG();
>
> @@ -427,6 +452,9 @@ static int xen_cpu_up(unsigned int cpu, struct task_struct *idle)
> /* Just in case we booted with a single CPU. */
> alternatives_enable_smp();
>
> + if (xen_pmu_init(cpu))
> + pr_err("Could not initialize VPMU for VCPU %u\n", cpu);
> +
> rc = xen_smp_intr_init(cpu);
> if (rc)
> return rc;
> @@ -468,6 +496,7 @@ static void xen_cpu_die(unsigned int cpu)
> xen_smp_intr_free(cpu);
> xen_uninit_lock_cpu(cpu);
> xen_teardown_timer(cpu);
> + xen_pmu_finish(cpu);
> }
>
> static void xen_play_dead(void) /* used only with HOTPLUG_CPU */

That all looks OK
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Jeff Moyer: "Re: [PATCH] block: check CAP_SYS_ADMIN before calling driver BLKROSET"
Previous message: Konrad Rzeszutek Wilk: "Re: [PATCH v1 2/5] xen/PMU: Sysfs interface for setting Xen PMU mode"
Next in thread: Boris Ostrovsky: "Re: [PATCH v1 3/5] xen/PMU: Initialization code for Xen PMU"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]