|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v12 for-xen-4.5 11/20] x86/VPMU: Interface for setting PMU mode and flags
>>> On 29.09.14 at 15:25, <dietmar.hahn@xxxxxxxxxxxxxx> wrote:
> Only a minor note below.
>
> Am Donnerstag 25 September 2014, 15:28:47 schrieb Boris Ostrovsky:
>> Add runtime interface for setting PMU mode and flags. Three main modes are
>> provided:
>> * XENPMU_MODE_OFF: PMU is not virtualized
>> * XENPMU_MODE_SELF: Guests can access PMU MSRs and receive PMU interrupts.
>> * XENPMU_MODE_HV: Same as XENPMU_MODE_SELF for non-proviledged guests, dom0
>> can profile itself and the hypervisor.
>>
>> Note that PMU modes are different from what can be provided at Xen's boot
> line
>> with 'vpmu' argument. An 'off' (or '0') value is equivalent to
> XENPMU_MODE_OFF.
>> Any other value, on the other hand, will cause VPMU mode to be set to
>> XENPMU_MODE_SELF during boot.
>>
>> For feature flags only Intel's BTS is currently supported.
>>
>> Mode and flags are set via HYPERVISOR_xenpmu_op hypercall.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>> ---
>> tools/flask/policy/policy/modules/xen/xen.te | 3 +
>> xen/arch/x86/domain.c | 6 +-
>> xen/arch/x86/hvm/svm/vpmu.c | 4 +-
>> xen/arch/x86/hvm/vmx/vpmu_core2.c | 10 +-
>> xen/arch/x86/hvm/vpmu.c | 206
> +++++++++++++++++++++++++--
>> xen/arch/x86/x86_64/compat/entry.S | 4 +
>> xen/arch/x86/x86_64/entry.S | 4 +
>> xen/include/Makefile | 2 +
>> xen/include/asm-x86/hvm/vpmu.h | 27 ++--
>> xen/include/public/pmu.h | 44 ++++++
>> xen/include/public/xen.h | 1 +
>> xen/include/xen/hypercall.h | 4 +
>> xen/include/xlat.lst | 4 +
>> xen/include/xsm/dummy.h | 15 ++
>> xen/include/xsm/xsm.h | 6 +
>> xen/xsm/dummy.c | 1 +
>> xen/xsm/flask/hooks.c | 18 +++
>> xen/xsm/flask/policy/access_vectors | 2 +
>> 18 files changed, 334 insertions(+), 27 deletions(-)
>>
>> diff --git a/tools/flask/policy/policy/modules/xen/xen.te
> b/tools/flask/policy/policy/modules/xen/xen.te
>> index 1937883..fb761cd 100644
>> --- a/tools/flask/policy/policy/modules/xen/xen.te
>> +++ b/tools/flask/policy/policy/modules/xen/xen.te
>> @@ -64,6 +64,9 @@ allow dom0_t xen_t:xen {
>> getidle debug getcpuinfo heap pm_op mca_op lockprof cpupool_op tmem_op
>> tmem_control getscheduler setscheduler
>> };
>> +allow dom0_t xen_t:xen2 {
>> + pmu_ctrl
>> +};
>> allow dom0_t xen_t:mmu memorymap;
>>
>> # Allow dom0 to use these domctls on itself. For domctls acting on other
>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index 7b1dfe6..6a07737 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -1503,7 +1503,7 @@ void context_switch(struct vcpu *prev, struct vcpu
> *next)
>> if ( is_hvm_vcpu(prev) )
>> {
>> if (prev != next)
>> - vpmu_save(prev);
>> + vpmu_switch_from(prev, next);
>>
>> if ( !list_empty(&prev->arch.hvm_vcpu.tm_list) )
>> pt_save_timer(prev);
>> @@ -1546,9 +1546,9 @@ void context_switch(struct vcpu *prev, struct vcpu
> *next)
>> !is_hardware_domain(next->domain));
>> }
>>
>> - if (is_hvm_vcpu(next) && (prev != next) )
>> + if ( is_hvm_vcpu(prev) && (prev != next) )
>> /* Must be done with interrupts enabled */
>> - vpmu_load(next);
>> + vpmu_switch_to(prev, next);
>>
>> context_saved(prev);
>>
>> diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
>> index 124b147..37d8228 100644
>> --- a/xen/arch/x86/hvm/svm/vpmu.c
>> +++ b/xen/arch/x86/hvm/svm/vpmu.c
>> @@ -479,14 +479,14 @@ struct arch_vpmu_ops amd_vpmu_ops = {
>> .arch_vpmu_dump = amd_vpmu_dump
>> };
>>
>> -int svm_vpmu_initialise(struct vcpu *v, unsigned int vpmu_flags)
>> +int svm_vpmu_initialise(struct vcpu *v)
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>> uint8_t family = current_cpu_data.x86;
>> int ret = 0;
>>
>> /* vpmu enabled? */
>> - if ( !vpmu_flags )
>> + if ( vpmu_mode == XENPMU_MODE_OFF )
>> return 0;
>>
>> switch ( family )
>> diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> b/xen/arch/x86/hvm/vmx/vpmu_core2.c
>> index beff5c3..c0a45cd 100644
>> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
>> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
>> @@ -703,13 +703,13 @@ static int core2_vpmu_do_interrupt(struct
>> cpu_user_regs
> *regs)
>> return 1;
>> }
>>
>> -static int core2_vpmu_initialise(struct vcpu *v, unsigned int vpmu_flags)
>> +static int core2_vpmu_initialise(struct vcpu *v)
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>> u64 msr_content;
>> static bool_t ds_warned;
>>
>> - if ( !(vpmu_flags & VPMU_BOOT_BTS) )
>> + if ( !(vpmu_features & XENPMU_FEATURE_INTEL_BTS) )
>> goto func_out;
>> /* Check the 'Debug Store' feature in the CPUID.EAX[1]:EDX[21] */
>> while ( boot_cpu_has(X86_FEATURE_DS) )
>> @@ -824,7 +824,7 @@ struct arch_vpmu_ops core2_no_vpmu_ops = {
>> .do_cpuid = core2_no_vpmu_do_cpuid,
>> };
>>
>> -int vmx_vpmu_initialise(struct vcpu *v, unsigned int vpmu_flags)
>> +int vmx_vpmu_initialise(struct vcpu *v)
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>> uint8_t family = current_cpu_data.x86;
>> @@ -832,7 +832,7 @@ int vmx_vpmu_initialise(struct vcpu *v, unsigned int
> vpmu_flags)
>> int ret = 0;
>>
>> vpmu->arch_vpmu_ops = &core2_no_vpmu_ops;
>> - if ( !vpmu_flags )
>> + if ( vpmu_mode == XENPMU_MODE_OFF )
>> return 0;
>>
>> if ( family == 6 )
>> @@ -875,7 +875,7 @@ int vmx_vpmu_initialise(struct vcpu *v, unsigned int
> vpmu_flags)
>> /* future: */
>> case 0x3d:
>> case 0x4e:
>> - ret = core2_vpmu_initialise(v, vpmu_flags);
>> + ret = core2_vpmu_initialise(v);
>> if ( !ret )
>> vpmu->arch_vpmu_ops = &core2_vpmu_ops;
>> return ret;
>> diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
>> index 071b869..5fcee0e 100644
>> --- a/xen/arch/x86/hvm/vpmu.c
>> +++ b/xen/arch/x86/hvm/vpmu.c
>> @@ -21,6 +21,8 @@
>> #include <xen/config.h>
>> #include <xen/sched.h>
>> #include <xen/xenoprof.h>
>> +#include <xen/event.h>
>> +#include <xen/guest_access.h>
>> #include <asm/regs.h>
>> #include <asm/types.h>
>> #include <asm/msr.h>
>> @@ -32,13 +34,22 @@
>> #include <asm/hvm/svm/vmcb.h>
>> #include <asm/apic.h>
>> #include <public/pmu.h>
>> +#include <xen/tasklet.h>
>> +#include <xsm/xsm.h>
>> +
>> +#include <compat/pmu.h>
>> +CHECK_pmu_params;
>> +CHECK_pmu_intel_ctxt;
>> +CHECK_pmu_amd_ctxt;
>> +CHECK_pmu_cntr_pair;
>>
>> /*
>> * "vpmu" : vpmu generally enabled
>> * "vpmu=off" : vpmu generally disabled
>> * "vpmu=bts" : vpmu enabled and Intel BTS feature switched on.
>> */
>> -static unsigned int __read_mostly opt_vpmu_enabled;
>> +uint64_t __read_mostly vpmu_mode = XENPMU_MODE_OFF;
>> +uint64_t __read_mostly vpmu_features = 0;
>> static void parse_vpmu_param(char *s);
>> custom_param("vpmu", parse_vpmu_param);
>>
>> @@ -52,7 +63,7 @@ static void __init parse_vpmu_param(char *s)
>> break;
>> default:
>> if ( !strcmp(s, "bts") )
>> - opt_vpmu_enabled |= VPMU_BOOT_BTS;
>> + vpmu_features |= XENPMU_FEATURE_INTEL_BTS;
>> else if ( *s )
>> {
>> printk("VPMU: unknown flag: %s - vpmu disabled!\n", s);
>> @@ -60,7 +71,8 @@ static void __init parse_vpmu_param(char *s)
>> }
>> /* fall through */
>> case 1:
>> - opt_vpmu_enabled |= VPMU_BOOT_ENABLED;
>> + /* Default VPMU mode */
>> + vpmu_mode = XENPMU_MODE_SELF;
>> break;
>> }
>> }
>> @@ -77,6 +89,9 @@ int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
> uint64_t supported)
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>>
>> + if ( !(vpmu_mode & (XENPMU_MODE_SELF | XENPMU_MODE_HV)) )
>> + return 0;
>> +
>> if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
>> return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content, supported);
>> return 0;
>> @@ -86,6 +101,9 @@ int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>>
>> + if ( !(vpmu_mode & (XENPMU_MODE_SELF | XENPMU_MODE_HV)) )
>> + return 0;
>> +
>> if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_rdmsr )
>> return vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content);
>> return 0;
>> @@ -242,19 +260,19 @@ void vpmu_initialise(struct vcpu *v)
>> switch ( vendor )
>> {
>> case X86_VENDOR_AMD:
>> - if ( svm_vpmu_initialise(v, opt_vpmu_enabled) != 0 )
>> - opt_vpmu_enabled = 0;
>> + if ( svm_vpmu_initialise(v) != 0 )
>> + vpmu_mode = XENPMU_MODE_OFF;
>> break;
>>
>> case X86_VENDOR_INTEL:
>> - if ( vmx_vpmu_initialise(v, opt_vpmu_enabled) != 0 )
>> - opt_vpmu_enabled = 0;
>> + if ( vmx_vpmu_initialise(v) != 0 )
>> + vpmu_mode = XENPMU_MODE_OFF;
>> break;
>>
>> default:
>> printk("VPMU: Initialization failed. "
>> "Unknown CPU vendor %d\n", vendor);
>> - opt_vpmu_enabled = 0;
>> + vpmu_mode = XENPMU_MODE_OFF;
>> break;
>> }
>> }
>> @@ -276,3 +294,175 @@ void vpmu_dump(struct vcpu *v)
>> vpmu->arch_vpmu_ops->arch_vpmu_dump(v);
>> }
>>
>> +static atomic_t vpmu_sched_counter;
>> +
>> +static void vpmu_sched_checkin(unsigned long unused)
>> +{
>> + atomic_inc(&vpmu_sched_counter);
>> +}
>> +
>> +static int vpmu_force_context_switch(void)
>> +{
>> + unsigned i, j, allbutself_num, mycpu;
>> + static s_time_t start, now;
>> + struct tasklet **sync_task;
>> + struct vcpu *curr_vcpu = current;
>> + int ret = 0;
>> +
>> + allbutself_num = num_online_cpus() - 1;
>> +
>> + sync_task = xzalloc_array(struct tasklet *, allbutself_num);
>> + if ( !sync_task )
>> + {
>> + printk(XENLOG_WARNING "vpmu_force_context_switch: out of
> memory\n");
>> + return -ENOMEM;
>> + }
>> +
>> + for ( i = 0; i < allbutself_num; i++ )
>> + {
>> + sync_task[i] = xmalloc(struct tasklet);
>> + if ( sync_task[i] == NULL )
>> + {
>> + printk(XENLOG_WARNING "vpmu_force_context_switch: out of
> memory\n");
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> + tasklet_init(sync_task[i], vpmu_sched_checkin, 0);
>
> Only a question of understanding.
> Is there a special reason not to use a single memory allocation
> except for memory fragmentation on systems with a large number of cpus?
>
> struct tasklet *sync_task;
> sync_task = xmalloc(sizeof(struct tasklet) * allbutself_num);
Apart from this then needing to be xmalloc_array() - yes, the
reason here is to avoid non-order-zero runtime allocations. I.e.
the alternative would be to provide something vmalloc()-like to
be used here (or open code it as we do in a couple of other
places).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |