From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86/HVM: 32-bit IN result must be zero-extended to 64 bits Date: Tue, 5 Nov 2013 09:53:55 +0000 Message-ID: <5278C033.2000408@citrix.com> References: <5278BE8D02000078000FF631@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2164895068133079388==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VddKi-0005Nw-5n for xen-devel@lists.xenproject.org; Tue, 05 Nov 2013 09:54:00 +0000 In-Reply-To: <5278BE8D02000078000FF631@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============2164895068133079388== Content-Type: multipart/alternative; boundary="------------000404000405000700060102" --------------000404000405000700060102 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit On 05/11/13 08:46, Jan Beulich wrote: > Just like for all other operations with 32-bit operand size. > > Signed-off-by: Jan Beulich > > --- a/xen/arch/x86/hvm/io.c > +++ b/xen/arch/x86/hvm/io.c > @@ -221,13 +221,15 @@ int handle_mmio_with_translation(unsigne > return handle_mmio(); > } > > -int handle_pio(uint16_t port, int size, int dir) > +int handle_pio(uint16_t port, unsigned int size, int dir) > { > struct vcpu *curr = current; > struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io; > unsigned long data, reps = 1; > int rc; > > + ASSERT((size - 1) < 4 && size != 3); > + > if ( dir == IOREQ_WRITE ) > data = guest_cpu_user_regs()->eax; > > @@ -237,7 +239,12 @@ int handle_pio(uint16_t port, int size, > { > case X86EMUL_OKAY: > if ( dir == IOREQ_READ ) > - memcpy(&guest_cpu_user_regs()->eax, &data, vio->io_size); > + { > + if ( size == 4 ) /* Needs zero extension. */ > + guest_cpu_user_regs()->eax = (uint32_t)data; This would be a rather more clear as guest_cpu_user_regs()->rax, with makes the 0-extension more obvious than remembering that eax is a 64bit value in Xen. However, I guess consistency with the surrounding code is important, and it is spelt out in a comment. Either way, Reviewed-by: Andrew Cooper > + else > + memcpy(&guest_cpu_user_regs()->eax, &data, size); > + } > break; > case X86EMUL_RETRY: > if ( vio->io_state != HVMIO_awaiting_completion ) > --- a/xen/include/asm-x86/hvm/io.h > +++ b/xen/include/asm-x86/hvm/io.h > @@ -119,7 +119,7 @@ void send_timeoffset_req(unsigned long t > void send_invalidate_req(void); > int handle_mmio(void); > int handle_mmio_with_translation(unsigned long gva, unsigned long gpfn); > -int handle_pio(uint16_t port, int size, int dir); > +int handle_pio(uint16_t port, unsigned int size, int dir); > void hvm_interrupt_post(struct vcpu *v, int vector, int type); > void hvm_io_assist(void); > void hvm_dpci_eoi(struct domain *d, unsigned int guest_irq, > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------000404000405000700060102 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
On 05/11/13 08:46, Jan Beulich wrote:
Just like for all other operations with 32-bit operand size.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -221,13 +221,15 @@ int handle_mmio_with_translation(unsigne
     return handle_mmio();
 }
 
-int handle_pio(uint16_t port, int size, int dir)
+int handle_pio(uint16_t port, unsigned int size, int dir)
 {
     struct vcpu *curr = current;
     struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
     unsigned long data, reps = 1;
     int rc;
 
+    ASSERT((size - 1) < 4 && size != 3);
+
     if ( dir == IOREQ_WRITE )
         data = guest_cpu_user_regs()->eax;
 
@@ -237,7 +239,12 @@ int handle_pio(uint16_t port, int size, 
     {
     case X86EMUL_OKAY:
         if ( dir == IOREQ_READ )
-            memcpy(&guest_cpu_user_regs()->eax, &data, vio->io_size);
+        {
+            if ( size == 4 ) /* Needs zero extension. */
+                guest_cpu_user_regs()->eax = (uint32_t)data;

This would be a rather more clear as guest_cpu_user_regs()->rax, with makes the 0-extension more obvious than remembering that eax is a 64bit value in Xen.  However, I guess consistency with the surrounding code is important, and it is spelt out in a comment.

Either way,

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

+            else
+                memcpy(&guest_cpu_user_regs()->eax, &data, size);
+        }
         break;
     case X86EMUL_RETRY:
         if ( vio->io_state != HVMIO_awaiting_completion )
--- a/xen/include/asm-x86/hvm/io.h
+++ b/xen/include/asm-x86/hvm/io.h
@@ -119,7 +119,7 @@ void send_timeoffset_req(unsigned long t
 void send_invalidate_req(void);
 int handle_mmio(void);
 int handle_mmio_with_translation(unsigned long gva, unsigned long gpfn);
-int handle_pio(uint16_t port, int size, int dir);
+int handle_pio(uint16_t port, unsigned int size, int dir);
 void hvm_interrupt_post(struct vcpu *v, int vector, int type);
 void hvm_io_assist(void);
 void hvm_dpci_eoi(struct domain *d, unsigned int guest_irq,






_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

--------------000404000405000700060102-- --===============2164895068133079388== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============2164895068133079388==--