|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v11 9/9] Add xentrace to vmware_port
On 06/04/15 07:20, George Dunlap wrote:
> On 05/22/2015 04:50 PM, Don Slutz wrote:
>> Also added missing TRAP_DEBUG & VLAPIC.
>>
>> Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
>> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>> ---
>> v11:
>> No change
>>
>> v10:
>> Added Acked-by: Ian Campbell
>> Added back in the trace point calls.
>>
>> Why is cmd in this patch?
>> Because the trace points use it.
>>
>> v9:
>> Dropped unneed VMPORT_UNHANDLED, VMPORT_DECODE.
>>
>> v7:
>> Dropped some of the new traces.
>> Added HVMTRACE_ND7.
>>
>> v6:
>> Dropped the attempt to use svm_nextrip_insn_length via
>> __get_instruction_length (added in v2). Just always look
>> at upto 15 bytes on AMD.
>>
>> v5:
>> exitinfo1 is used twice.
>> Fixed.
>>
>> tools/xentrace/formats | 5 +++++
>> xen/arch/x86/hvm/io.c | 3 +++
>> xen/arch/x86/hvm/vmware/vmport.c | 17 ++++++++++++++---
>> xen/include/asm-x86/hvm/trace.h | 22 ++++++++++++++++++++++
>> xen/include/public/trace.h | 3 +++
>> 5 files changed, 47 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/xentrace/formats b/tools/xentrace/formats
>> index 5d7b72a..eec65f4 100644
>> --- a/tools/xentrace/formats
>> +++ b/tools/xentrace/formats
>> @@ -79,6 +79,11 @@
>> 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 ]
>> +0x00082024 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) TRAP_DEBUG [
>> exit_qualification = 0x%(1)08x ]
>> +0x00082025 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VLAPIC
>> +0x00082026 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_HANDLED [ cmd =
>> %(1)d eax = 0x%(2)08x ebx = 0x%(3)08x ecx = 0x%(4)08x edx = 0x%(5)08x esi =
>> 0x%(6)08x edi = 0x%(7)08x ]
>> +0x00082027 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_IGNORED [ port =
>> %(1)d eax = 0x%(2)08x ebx = 0x%(3)08x ecx = 0x%(4)08x edx = 0x%(5)08x esi =
>> 0x%(6)08x edi = 0x%(7)08x ]
>> +0x00082028 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_QEMU [ eax =
>> 0x%(1)08x ebx = 0x%(2)08x ecx = 0x%(3)08x edx = 0x%(4)08x esi = 0x%(5)08x
>> edi = 0x%(6)08x ]
>>
>> 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/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
>> index 7684cf0..6a9cfb0 100644
>> --- a/xen/arch/x86/hvm/io.c
>> +++ b/xen/arch/x86/hvm/io.c
>> @@ -206,6 +206,9 @@ void hvm_io_assist(ioreq_t *p)
>> regs->_edx = vr->edx;
>> regs->_esi = vr->esi;
>> regs->_edi = vr->edi;
>> + HVMTRACE_ND(VMPORT_QEMU, 0, 1/*cycles*/, 6,
>> + p->data, regs->_ebx, regs->_ecx,
>> + regs->_edx, regs->_esi, regs->_edi);
>> }
>> }
>> if ( vio->io_size == 4 ) /* Needs zero extension. */
>> diff --git a/xen/arch/x86/hvm/vmware/vmport.c
>> b/xen/arch/x86/hvm/vmware/vmport.c
>> index 36e3f1b..3c3ccd4 100644
>> --- a/xen/arch/x86/hvm/vmware/vmport.c
>> +++ b/xen/arch/x86/hvm/vmware/vmport.c
>> @@ -16,6 +16,7 @@
>> #include <xen/lib.h>
>> #include <asm/hvm/hvm.h>
>> #include <asm/hvm/support.h>
>> +#include <asm/hvm/trace.h>
>>
>> #include "backdoor_def.h"
>>
>> @@ -35,6 +36,7 @@ static int vmport_ioport(int dir, uint32_t port, uint32_t
>> bytes, uint32_t *val)
>> if ( port == BDOOR_PORT && regs->_eax == BDOOR_MAGIC )
>> {
>> uint32_t new_eax = ~0u;
>> + uint16_t cmd = regs->_ecx;
>> uint64_t value;
>> struct vcpu *curr = current;
>> struct domain *currd = curr->domain;
>> @@ -45,7 +47,7 @@ static int vmport_ioport(int dir, uint32_t port, uint32_t
>> bytes, uint32_t *val)
>> * leaving the high 32-bits unchanged, unlike what one would
>> * expect to happen.
>> */
>> - switch ( regs->_ecx & 0xffff )
>> + switch ( cmd )
>> {
>> case BDOOR_CMD_GETMHZ:
>> new_eax = currd->arch.tsc_khz / 1000;
>> @@ -123,11 +125,20 @@ static int vmport_ioport(int dir, uint32_t port,
>> uint32_t bytes, uint32_t *val)
>> /* Let backing DM handle */
>> return X86EMUL_UNHANDLEABLE;
>> }
>> + HVMTRACE_ND7(VMPORT_HANDLED, 0, 0/*cycles*/, 7,
>> + cmd, new_eax, regs->_ebx, regs->_ecx,
>> + regs->_edx, regs->_esi, regs->_edi);
>
> Do you need to log edi as well? It looks like it's not used.
I guess not, but since there are VMware port commands that do use edi, a
future add might need it. I find it simpler to have this and the QEMU
case above the same but will change it if you want.
>
>> if ( dir == IOREQ_READ )
>> *val = new_eax;
>> }
>> - else if ( dir == IOREQ_READ )
>> - *val = ~0u;
>> + else
>> + {
>> + HVMTRACE_ND7(VMPORT_IGNORED, 0, 0/*cycles*/, 7,
>> + port, regs->_eax, regs->_ebx, regs->_ecx,
>> + regs->_edx, regs->_esi, regs->_edi);
>
> And do you need to log all the registers here? It seems like port +
> regs->_ecx would be enough to tell you why it got ignored.
>
The min would be port and regs->_eax. QEMU is the one that cares about
regs->_ecx.
Happy to make this change. Will wait until response from above.
>> + if ( dir == IOREQ_READ )
>> + *val = ~0u;
>> + }
>>
>> return X86EMUL_OKAY;
>> }
>> diff --git a/xen/include/asm-x86/hvm/trace.h
>> b/xen/include/asm-x86/hvm/trace.h
>> index de802a6..0ad805f 100644
>> --- a/xen/include/asm-x86/hvm/trace.h
>> +++ b/xen/include/asm-x86/hvm/trace.h
>> @@ -54,6 +54,9 @@
>> #define DO_TRC_HVM_TRAP DEFAULT_HVM_MISC
>> #define DO_TRC_HVM_TRAP_DEBUG DEFAULT_HVM_MISC
>> #define DO_TRC_HVM_VLAPIC DEFAULT_HVM_MISC
>> +#define DO_TRC_HVM_VMPORT_HANDLED DEFAULT_HVM_IO
>> +#define DO_TRC_HVM_VMPORT_IGNORED DEFAULT_HVM_IO
>> +#define DO_TRC_HVM_VMPORT_QEMU DEFAULT_HVM_IO
>>
>>
>> #define TRC_PAR_LONG(par) ((par)&0xFFFFFFFF),((par)>>32)
>> @@ -83,6 +86,25 @@
>> } \
>> } while(0)
>>
>> +#define HVMTRACE_ND7(evt, modifier, cycles, count, d1, d2, d3, d4, d5, d6,
>> d7) \
>> + do { \
>> + if ( unlikely(tb_init_done) && DO_TRC_HVM_ ## evt ) \
>> + { \
>> + struct { \
>> + u32 d[7]; \
>> + } _d; \
>> + _d.d[0]=(d1); \
>> + _d.d[1]=(d2); \
>> + _d.d[2]=(d3); \
>> + _d.d[3]=(d4); \
>> + _d.d[4]=(d5); \
>> + _d.d[5]=(d6); \
>> + _d.d[6]=(d7); \
>> + __trace_var(TRC_HVM_ ## evt | (modifier), cycles, \
>> + sizeof(*_d.d) * count, &_d); \
>> + } \
>> + } while(0)
>
> If you reduced the registers as mentioned above, you wouldn't need this
> here either.
>
Yes. However with out this, I did not understand that a trace could
have 7 32bit arguments. Happy to go either way depending on changes above.
-Don Slutz
> -George
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |