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