Jan Beulich
2013-Nov-05 08:46 UTC
[PATCH] x86/HVM: 32-bit IN result must be zero-extended to 64 bits
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;
+ 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
Andrew Cooper
2013-Nov-05 09:53 UTC
Re: [PATCH] x86/HVM: 32-bit IN result must be zero-extended to 64 bits
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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Jan Beulich
2013-Nov-05 10:07 UTC
Re: [PATCH] x86/HVM: 32-bit IN result must be zero-extended to 64 bits
>>> On 05.11.13 at 10:53, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > On 05/11/13 08:46, Jan Beulich wrote: >> @@ -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.In fact I agree - I coded it that way with backportability in mind, but am inclined to change _both_ instances to "rax" here (which will cause obvious build breakage if not adjusted accordingly for 4.2 and earlier).> Either way, > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>Thanks. Jan
Keir Fraser
2013-Nov-05 13:24 UTC
Re: [PATCH] x86/HVM: 32-bit IN result must be zero-extended to 64 bits
On 05/11/2013 08:46, "Jan Beulich" <JBeulich@suse.com> wrote:> Just like for all other operations with 32-bit operand size. > > Signed-off-by: Jan Beulich <jbeulich@suse.com>Acked-by: Keir Fraser <keir@xen.org>