Re: [Xen-devel] [PATCH v2 10/10] x86/SVM: Add AMD AVIC key handler

2017-01-10 Thread Jan Beulich 
>>> On 10.01.17 at 12:14,  wrote:
> On 01/05/2017 11:07 PM, Jan Beulich wrote:
> On 31.12.16 at 06:46,  wrote:
>>> +void __init setup_avic_dump(void)
>>> +{
>>> +register_keyhandler('j', avic_dump, "dump SVM AVIC", 1);
>>> +}
>>
>> For one the description should include the word "stats". And then
>> I'm rather uncertain this is work burning one of the few remaining
>> available keys. Could this be made a domctl instead?
> 
> Not sure how you are thinking about using domctl to output statistics. 
> Are there examples? Could you please describe?

Provide a domctl to obtain the values, and a new xl command to
wrap that domctl.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 10/10] x86/SVM: Add AMD AVIC key handler

2017-01-10 Thread Suravee Suthikulpanit 

Jan,

On 01/05/2017 11:07 PM, Jan Beulich wrote:

On 31.12.16 at 06:46,  wrote:

+void __init setup_avic_dump(void)
+{
+register_keyhandler('j', avic_dump, "dump SVM AVIC", 1);
+}


For one the description should include the word "stats". And then
I'm rather uncertain this is work burning one of the few remaining
available keys. Could this be made a domctl instead?

Jan



Not sure how you are thinking about using domctl to output statistics. 
Are there examples? Could you please describe?


Thanks,
Suravee

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 10/10] x86/SVM: Add AMD AVIC key handler

2017-01-05 Thread Jan Beulich 
>>> On 31.12.16 at 06:46,  wrote:
> +void __init setup_avic_dump(void)
> +{
> +register_keyhandler('j', avic_dump, "dump SVM AVIC", 1);
> +}

For one the description should include the word "stats". And then
I'm rather uncertain this is work burning one of the few remaining
available keys. Could this be made a domctl instead?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 10/10] x86/SVM: Add AMD AVIC key handler

2017-01-05 Thread Suravee Suthikulpanit 

Hi Boris/Andrew,

On 1/3/17 23:04, Andrew Cooper wrote:

On 03/01/17 16:01, Boris Ostrovsky wrote:


+static void avic_dump(unsigned char ch)
+{
+struct domain *d;
+struct vcpu *v;
+
+printk("*** SVM AVIC Statistics **\n");
+
+rcu_read_lock(_read_lock);
+
+for_each_domain ( d )
+{
+if ( !is_hvm_domain(d) )

|| !svm_avic_vcpu_enabled(d->v[0]) (or something along these lines)?


It isn't safe to deference the vcpu array like this, which in turn
highlights that the avic predicate should be per-domain, not per-cpu.
Under no circumstances should we have AVIC on some vcpus but not others
of the same domain.

~Andrew



Let me add something like:

static inline bool svm_is_avic_domain(struct domain *d)
{
return ( d->arch.hvm_domain.svm.avic_physical_id_table != 0 );
}

This should allow us to check whether the svm_avic_dom_init() is enabled 
successfully.


Thanks,
Suravee

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 10/10] x86/SVM: Add AMD AVIC key handler

2017-01-03 Thread Andrew Cooper 
On 03/01/17 16:01, Boris Ostrovsky wrote:
>>  
>> +static void avic_dump(unsigned char ch)
>> +{
>> +struct domain *d;
>> +struct vcpu *v;
>> +
>> +printk("*** SVM AVIC Statistics **\n");
>> +
>> +rcu_read_lock(_read_lock);
>> +
>> +for_each_domain ( d )
>> +{
>> +if ( !is_hvm_domain(d) )
> || !svm_avic_vcpu_enabled(d->v[0]) (or something along these lines)?

It isn't safe to deference the vcpu array like this, which in turn
highlights that the avic predicate should be per-domain, not per-cpu. 
Under no circumstances should we have AVIC on some vcpus but not others
of the same domain.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 10/10] x86/SVM: Add AMD AVIC key handler

2017-01-03 Thread Boris Ostrovsky 

>  
> +static void avic_dump(unsigned char ch)
> +{
> +struct domain *d;
> +struct vcpu *v;
> +
> +printk("*** SVM AVIC Statistics **\n");
> +
> +rcu_read_lock(_read_lock);
> +
> +for_each_domain ( d )
> +{
> +if ( !is_hvm_domain(d) )

|| !svm_avic_vcpu_enabled(d->v[0]) (or something along these lines)?


> +continue;
> +printk(">>> Domain %d <<<\n", d->domain_id);
> +for_each_vcpu ( d, v )
> +{
> +printk("\tVCPU %d\n", v->vcpu_id);
> +printk("\t* incomp_ipi = %u\n",
> +   v->arch.hvm_svm.cnt_avic_incomp_ipi);
> +printk("\t* noaccel= %u\n",
> +   v->arch.hvm_svm.cnt_avic_noaccel);
> +printk("\t* post_intr  = %u\n",
> +   v->arch.hvm_svm.cnt_avic_post_intr);
> +printk("\t* doorbell   = %u\n",
> +   v->arch.hvm_svm.cnt_avic_doorbell);
> +}
> +}
> +
> +rcu_read_unlock(_read_lock);
> +
> +printk("**\n");
> +
> +}
> +
> +void __init setup_avic_dump(void)
> +{

if ( !svm_avic ) return; ?


-boris


> +register_keyhandler('j', avic_dump, "dump SVM AVIC", 1);
> +}
> +


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 10/10] x86/SVM: Add AMD AVIC key handler

2016-12-30 Thread Suravee Suthikulpanit 
Adding new key-handler "j" for dumping AVIC-related information.
Here is an example of per-domain statistics being dumped.

  *** SVM AVIC Statistics **
  >>> Domain 1 <<<
  VCPU 0
  * incomp_ipi = 3110
  * noaccel= 236475
  * post_intr  = 116176
  * doorbell   = 715
  VCPU 1
  * incomp_ipi = 2565
  * noaccel= 233061
  * post_intr  = 115765
  * doorbell   = 771
  **

Signed-off-by: Suravee Suthikulpanit 
Cc: Konrad Rzeszutek Wilk 
Cc: Jan Beulich 
Cc: Boris Ostrovsky 
---
 xen/arch/x86/hvm/svm/avic.c| 48 ++
 xen/arch/x86/hvm/svm/svm.c |  1 +
 xen/include/asm-x86/hvm/svm/vmcb.h |  6 +
 3 files changed, 55 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
index faa5e45..1aea724 100644
--- a/xen/arch/x86/hvm/svm/avic.c
+++ b/xen/arch/x86/hvm/svm/avic.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -320,6 +321,8 @@ void svm_avic_vmexit_do_incomp_ipi(struct cpu_user_regs 
*regs)
 u32 id = vmcb->exitinfo2 >> 32;
 u32 index = vmcb->exitinfo2 && 0xFF;
 
+curr->arch.hvm_svm.cnt_avic_incomp_ipi++;
+
 switch ( id )
 {
 case AVIC_INCMP_IPI_ERR_INVALID_INT_TYPE:
@@ -580,6 +583,8 @@ void svm_avic_vmexit_do_noaccel(struct cpu_user_regs *regs)
 u32 offset = vmcb->exitinfo1 & 0xFF0;
 u32 rw = (vmcb->exitinfo1 >> 32) & 0x1;
 
+curr->arch.hvm_svm.cnt_avic_noaccel++;
+
 switch ( offset )
 {
 case APIC_ID:
@@ -654,16 +659,59 @@ void svm_avic_deliver_posted_intr(struct vcpu *v, u8 vec)
 if ( vlapic_test_and_set_vector(vec, >regs->data[APIC_IRR]) )
 return;
 
+v->arch.hvm_svm.cnt_avic_post_intr++;
 /*
  * If vcpu is running on another cpu, hit the doorbell to signal
  * it to process interrupt. Otherwise, kick it.
  */
 if ( v->is_running && (v != current) )
+{
 wrmsrl(AVIC_DOORBELL, cpu_data[v->processor].apicid);
+v->arch.hvm_svm.cnt_avic_doorbell++;
+}
 else
 vcpu_kick(v);
 }
 
+static void avic_dump(unsigned char ch)
+{
+struct domain *d;
+struct vcpu *v;
+
+printk("*** SVM AVIC Statistics **\n");
+
+rcu_read_lock(_read_lock);
+
+for_each_domain ( d )
+{
+if ( !is_hvm_domain(d) )
+continue;
+printk(">>> Domain %d <<<\n", d->domain_id);
+for_each_vcpu ( d, v )
+{
+printk("\tVCPU %d\n", v->vcpu_id);
+printk("\t* incomp_ipi = %u\n",
+   v->arch.hvm_svm.cnt_avic_incomp_ipi);
+printk("\t* noaccel= %u\n",
+   v->arch.hvm_svm.cnt_avic_noaccel);
+printk("\t* post_intr  = %u\n",
+   v->arch.hvm_svm.cnt_avic_post_intr);
+printk("\t* doorbell   = %u\n",
+   v->arch.hvm_svm.cnt_avic_doorbell);
+}
+}
+
+rcu_read_unlock(_read_lock);
+
+printk("**\n");
+
+}
+
+void __init setup_avic_dump(void)
+{
+register_keyhandler('j', avic_dump, "dump SVM AVIC", 1);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 7c0cda0..b8861d8 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1456,6 +1456,7 @@ const struct hvm_function_table * __init start_svm(void)
 }
 
 setup_vmcb_dump();
+setup_avic_dump();
 
 svm_feature_flags = (current_cpu_data.extended_cpuid_level >= 0x800A ?
  cpuid_edx(0x800A) : 0);
diff --git a/xen/include/asm-x86/hvm/svm/vmcb.h 
b/xen/include/asm-x86/hvm/svm/vmcb.h
index 9abf077..e2b810e 100644
--- a/xen/include/asm-x86/hvm/svm/vmcb.h
+++ b/xen/include/asm-x86/hvm/svm/vmcb.h
@@ -552,6 +552,12 @@ struct arch_svm_struct {
 
 struct avic_phy_apic_id_ent *avic_last_phy_id;
 u32 avic_last_ldr;
+
+/* AVIC Statistics */
+u32 cnt_avic_incomp_ipi;
+u32 cnt_avic_noaccel;
+u32 cnt_avic_post_intr;
+u32 cnt_avic_doorbell;
 };
 
 struct vmcb_struct *alloc_vmcb(void);
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel