Re: [Xen-devel] [PATCH v3] VT-d PI: track the vcpu number in pi blocking list

[Jan Beulich]

>>> On 22.06.17 at 07:16,  wrote:
> On Fri, Jun 16, 2017 at 08:34:20AM -0600, Jan Beulich wrote:
> On 24.05.17 at 08:56,  wrote:
>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>> @@ -82,6 +82,7 @@ static int vmx_vmfunc_intercept(struct cpu_user_regs 
>>> *regs);
>>>  struct vmx_pi_blocking_vcpu {
>>>  struct list_head list;
>>>  spinlock_t   lock;
>>> +uint32_t counter;
>>
>>Is there any reason for this to be fixed width? Other than that the
> 
> I will use 'int' instead of 'uint32_t'.

Can the counter go negative? I don't think so, in which case you
really want to use "unsigned int".

Jan


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

Re: [Xen-devel] [PATCH v3] VT-d PI: track the vcpu number in pi blocking list

[Chao Gao]

On Fri, Jun 16, 2017 at 08:34:20AM -0600, Jan Beulich wrote:
 On 24.05.17 at 08:56,  wrote:
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -82,6 +82,7 @@ static int vmx_vmfunc_intercept(struct cpu_user_regs 
>> *regs);
>>  struct vmx_pi_blocking_vcpu {
>>  struct list_head list;
>>  spinlock_t   lock;
>> +uint32_t counter;
>
>Is there any reason for this to be fixed width? Other than that the

I will use 'int' instead of 'uint32_t'.

>non-tracing parts look fine, but I really wonder whether the
>introduction of the counter and the tracing wouldn't better be split.

Will split them apart.

Thanks
chao

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

Re: [Xen-devel] [PATCH v3] VT-d PI: track the vcpu number in pi blocking list

[Jan Beulich]

>>> On 24.05.17 at 08:56,  wrote:
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -82,6 +82,7 @@ static int vmx_vmfunc_intercept(struct cpu_user_regs *regs);
>  struct vmx_pi_blocking_vcpu {
>  struct list_head list;
>  spinlock_t   lock;
> +uint32_t counter;

Is there any reason for this to be fixed width? Other than that the
non-tracing parts look fine, but I really wonder whether the
introduction of the counter and the tracing wouldn't better be split.

Jan


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

[Xen-devel] [PATCH v3] VT-d PI: track the vcpu number in pi blocking list

[Chao Gao]

This patch adds a field, counter, in struct vmx_pi_blocking_vcpu to track
how many entries are on the pi blocking list. In order to analyze list
operation frequence and obtain the list length, add some relevant events
to xentrace and some associated code in xenalyze. As to xenalyze, for
removing from pi list can happen in various contexts (interrupt context,
and non-interrupt context) and be done by the vcpu itself or others, some of
the contexts would incur that toplevel_assert_check() fails. To bypass this
check, this patch adds a new type TRC_HVM_ASYNC and for this new type event,
the context would not be checked.

Signed-off-by: Chao Gao 
---
 tools/xentrace/formats  |   3 +
 tools/xentrace/xenalyze.c   | 154 +---
 xen/arch/x86/hvm/vmx/vmx.c  |  33 +++--
 xen/include/asm-x86/hvm/trace.h |   1 +
 xen/include/public/trace.h  |   5 ++
 5 files changed, 182 insertions(+), 14 deletions(-)

diff --git a/tools/xentrace/formats b/tools/xentrace/formats
index 8b31780..54e0b11 100644
--- a/tools/xentrace/formats
+++ b/tools/xentrace/formats
@@ -125,6 +125,9 @@
 0x00082020  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  INTR_WINDOW [ value = 
0x%(1)08x ]
 0x00082021  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  NPF [ gpa = 
0x%(2)08x%(1)08x mfn = 0x%(4)08x%(3)08x qual = 0x%(5)04x p2mt = 0x%(6)04x ]
 0x00082023  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  TRAP[ vector = 
0x%(1)02x ]
+0x00082026  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  PI_LIST_ADD [ domid = 
0x%(1)04x vcpu = 0x%(2)04x, pcpu = 0x%(3)04x, #entry = 0x%(4)04x ]
+
+0x00088001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  ASYNC_PI_LIST_DEL [ domid = 
0x%(1)04x vcpu = 0x%(2)04x ]
 
 0x0010f001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  page_grant_map  [ domid = 
%(1)d ]
 0x0010f002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  page_grant_unmap[ domid = 
%(1)d ]
diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index fa608ad..fbc2429 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -296,6 +296,7 @@ struct symbol_struct {
 };
 
 void error(enum error_level l, struct record_info *ri);
+struct vcpu_data * vcpu_find(int did, int vid);
 
 void parse_symbol_file(char *fn) {
 unsigned long long last_addr = 0;
@@ -944,6 +945,7 @@ enum {
 HVM_EVENT_TRAP,
 HVM_EVENT_TRAP_DEBUG,
 HVM_EVENT_VLAPIC,
+HVM_EVENT_PI_LIST_ADD,
 HVM_EVENT_HANDLER_MAX
 };
 char * hvm_event_handler_name[HVM_EVENT_HANDLER_MAX] = {
@@ -979,13 +981,15 @@ char * hvm_event_handler_name[HVM_EVENT_HANDLER_MAX] = {
 "realmode_emulate",
 "trap",
 "trap_debug",
-"vlapic"
+"vlapic",
+"pi_list_add",
 };
 
 enum {
 HVM_VOL_VMENTRY,
 HVM_VOL_VMEXIT,
 HVM_VOL_HANDLER,
+HVM_VOL_ASYNC,
 HVM_VOL_MAX
 };
 
@@ -1012,6 +1016,7 @@ char *hvm_vol_name[HVM_VOL_MAX] = {
 [HVM_VOL_VMENTRY]="vmentry",
 [HVM_VOL_VMEXIT] ="vmexit",
 [HVM_VOL_HANDLER]="handler",
+[HVM_VOL_ASYNC]="async",
 };
 
 enum {
@@ -1337,6 +1342,9 @@ struct hvm_data {
 struct {
 struct io_address *mmio, *pio;
 } io;
+struct {
+int pi_list_add, pi_list_del;
+} pi;
 } summary;
 
 /* In-flight accumulation information */
@@ -1391,6 +1399,9 @@ struct hvm_data {
 
 /* Historical info */
 tsc_t last_rdtsc;
+
+/* Destination pcpu of posted interrupt's wakeup interrupt */
+int pi_cpu;
 };
 
 enum {
@@ -1457,6 +1468,8 @@ void init_hvm_data(struct hvm_data *h, struct vcpu_data 
*v) {
 }
 for(i=0; isummary.guest_interrupt[i].count=0;
+
+h->pi_cpu = -1;
 }
 
 /* PV data */
@@ -1766,6 +1779,14 @@ char * toplevel_name[TOPLEVEL_MAX] = {
 [TOPLEVEL_HW]="hw",
 };
 
+enum {
+SUBLEVEL_HVM_ENTRYEXIT=1,
+SUBLEVEL_HVM_HANDLER,
+SUBLEVEL_HVM_EMUL,
+SUBLEVEL_HVM_ASYNC,
+SUBLEVEL_HVM_MAX=SUBLEVEL_HVM_ASYNC+1,
+};
+
 struct trace_volume {
 unsigned long long toplevel[TOPLEVEL_MAX];
 unsigned long long sched_verbose;
@@ -1852,6 +1873,9 @@ struct pcpu_info {
 tsc_t tsc;
 struct cycle_summary idle, running, lost;
 } time;
+
+/* Posted Interrupt List Length */
+int pi_list_length;
 };
 
 void __fill_in_record_info(struct pcpu_info *p);
@@ -4726,6 +4750,71 @@ void hvm_generic_dump(struct record_info *ri, char * 
prefix)
 printf(" ]\n");
 }
 
+void hvm_pi_list_add_process(struct record_info *ri, struct hvm_data *h)
+{
+struct {
+int did;
+int vid;
+int pcpu;
+int len;
+} *data;
+struct vcpu_data *v;
+
+data = (typeof(data))ri->rec.u.tsc.data;
+v = vcpu_find(data->did, data->vid);
+if ( !v->hvm.init )
+init_hvm_data(>hvm, v);
+
+if ( opt.dump_all )
+printf("d%uv%u is added to pi blocking list of pcpu%u. "
+   "The list length is now %d\n",
+   data->did, data->vid, data->pcpu, data->len);
+
+v->hvm.pi_cpu =