Re: [PATCH v9 09/13] Add xentrace to vmware_port

[Don Slutz <dslutz@verizon.com>]

On 02/23/15 11:57, Jan Beulich wrote:
>>>> On 17.02.15 at 00:05, <dslutz@verizon.com> wrote:
>> @@ -55,8 +56,9 @@ int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
>>          uint64_t value;
>>          struct vcpu *curr = current;
>>          struct domain *d = curr->domain;
>> +        uint16_t cmd = regs->_ecx;
>>  
>> -        switch ( regs->_ecx & 0xffff )
>> +        switch ( cmd )
> 
> This surely doesn't belong here.
> 

Ah, this version was missing the diff:

@@ -116,11 +118,20 @@ static int vmport_ioport(int dir, uint32_t port,
uint32_t bytes, uint32_t *val)
             /* Let backing DM handle */
             return X86EMUL_VMPORT_SEND;
         }
+        HVMTRACE_ND7(VMPORT_HANDLED, 0, 0/*cycles*/, 7,
+                     cmd, new_eax, regs->_ebx, regs->_ecx,
+                     regs->_edx, regs->_esi, regs->_edi);
         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);
+        if ( dir == IOREQ_READ )
+            *val = ~0u;
+    }

     return X86EMUL_OKAY;
 }

So, should cmd be in this patch or patch #5 (xen: Add vmware_port
support) where you said:

>> +    uint16_t cmd = regs->rcx;
>
> As you already have most other variables needed only inside the if()
> below declared in that scope, please be consistent with this one.
> Albeit the value of this variable is questionable anyway - it's being
> used exactly once.

?

   -Don Slutz

> Jan
>