Re: [XEN PATCH v4] x86/monitor: Add new monitor event to catch I/O instructions

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 21/03/2023 11:06 am, Jan Beulich wrote:
> On 21.03.2023 11:51, Andrew Cooper wrote:
>> On 20/03/2023 9:56 pm, Dmitry Isaykin wrote:
>>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>>> index 00b531f76c..0b7a302928 100644
>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>> @@ -4560,8 +4560,24 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>>>          break;
>>>  
>>>      case EXIT_REASON_IO_INSTRUCTION:
>>> +    {
>>> +        unsigned int port, bytes;
>>> +        bool in, str;
>>> +        int rc;
>>> +
>>>          __vmread(EXIT_QUALIFICATION, &exit_qualification);
>>> -        if ( exit_qualification & 0x10 )
>>> +
>>> +        port = (exit_qualification >> 16) & 0xFFFF;
>>> +        bytes = (exit_qualification & 0x07) + 1;
>>> +        in = (exit_qualification & 0x08);
>>> +        str = (exit_qualification & 0x10);
>>> +        rc = hvm_monitor_io(port, bytes, in, str);
>>> +        if ( rc < 0 )
>>> +            goto exit_and_crash;
>>> +        if ( rc )
>>> +            break;
>>> +
>>> +        if ( str )
>>>          {
>>>              /* INS, OUTS */
>>>              if ( !hvm_emulate_one_insn(x86_insn_is_portio, "port I/O") )
>>> @@ -4570,13 +4586,11 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>>>          else
>>>          {
>>>              /* IN, OUT */
>>> -            uint16_t port = (exit_qualification >> 16) & 0xFFFF;
>>> -            int bytes = (exit_qualification & 0x07) + 1;
>>> -            int dir = (exit_qualification & 0x08) ? IOREQ_READ : 
>>> IOREQ_WRITE;
>>> -            if ( handle_pio(port, bytes, dir) )
>>> +            if ( handle_pio(port, bytes, in ? IOREQ_READ : IOREQ_WRITE) )
>>>                  update_guest_eip(); /* Safe: IN, OUT */
>>>          }
>>>          break;
>>> +    }
>> Sorry for the delay.  I've got the Intel side sorted now too with
>> https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=f71f8e95c34fedb0d9ae21a100bfa9f012543abf
>>
>> The rebase is:
>>
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index 78ac9ece6ff2..7233e805a905 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -4578,6 +4578,14 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>>          __vmread(EXIT_QUALIFICATION, &io_qual.raw);
>>          bytes = io_qual.size + 1;
>>  
>> +        rc = hvm_monitor_io(io_qual.port, bytes,
>> +                            io_qual.in ? IOREQ_READ : IOREQ_WRITE,
> Here the conditional operator needs dropping; it just "io_qual.in" which
> wants passing.

Oh, of course.  In which case the delta is even smaller:

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 78ac9ece6ff2..076752d9e84b 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4578,6 +4578,12 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
         __vmread(EXIT_QUALIFICATION, &io_qual.raw);
         bytes = io_qual.size + 1;
 
+        rc = hvm_monitor_io(io_qual.port, bytes, io_qual.in, io_qual.str);
+        if ( rc < 0 )
+            goto exit_and_crash;
+        if ( rc )
+            break;
+
         if ( io_qual.str )
         {
             if ( !hvm_emulate_one_insn(x86_insn_is_portio, "port I/O") )


~Andrew