From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>, Keir Fraser <keir@xen.org>
Subject: Re: [PATCH 1/5] x86/HVM: properly handle backward string instruction emulation
Date: Tue, 8 Oct 2013 17:36:48 +0100 [thread overview]
Message-ID: <525434A0.6050500@citrix.com> (raw)
In-Reply-To: <5249915002000078000F8077@nat28.tlf.novell.com>
[-- Attachment #1.1: Type: text/plain, Size: 7828 bytes --]
On 30/09/13 13:57, Jan Beulich wrote:
> Multiplying a signed 32-bit quantity with an unsigned 32-bit quantity
> produces an unsigned 32-bit result, yet for emulation of backward
> string instructions we need the result sign extended before getting
> added to the base address.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> --- a/xen/arch/x86/hvm/intercept.c
> +++ b/xen/arch/x86/hvm/intercept.c
> @@ -48,7 +48,7 @@ static int hvm_mmio_access(struct vcpu *
> hvm_mmio_write_t write_handler)
> {
> unsigned long data;
> - int rc = X86EMUL_OKAY, i, sign = p->df ? -1 : 1;
> + int rc = X86EMUL_OKAY, i, step = p->df ? -p->size : p->size;
>
> if ( !p->data_is_ptr )
> {
> @@ -68,13 +68,10 @@ static int hvm_mmio_access(struct vcpu *
> {
> int ret;
>
> - rc = read_handler(v, p->addr + (sign * i * p->size), p->size,
> - &data);
> + rc = read_handler(v, p->addr + step * i, p->size, &data);
> if ( rc != X86EMUL_OKAY )
> break;
> - ret = hvm_copy_to_guest_phys(p->data + (sign * i * p->size),
> - &data,
> - p->size);
> + ret = hvm_copy_to_guest_phys(p->data + step * i, &data, p->size);
> if ( (ret == HVMCOPY_gfn_paged_out) ||
> (ret == HVMCOPY_gfn_shared) )
> {
> @@ -87,8 +84,7 @@ static int hvm_mmio_access(struct vcpu *
> {
> for ( i = 0; i < p->count; i++ )
> {
> - switch ( hvm_copy_from_guest_phys(&data,
> - p->data + sign * i * p->size,
> + switch ( hvm_copy_from_guest_phys(&data, p->data + step * i,
> p->size) )
> {
> case HVMCOPY_okay:
> @@ -109,8 +105,7 @@ static int hvm_mmio_access(struct vcpu *
> }
> if ( rc != X86EMUL_OKAY )
> break;
> - rc = write_handler(v, p->addr + (sign * i * p->size), p->size,
> - data);
> + rc = write_handler(v, p->addr + step * i, p->size, data);
> if ( rc != X86EMUL_OKAY )
> break;
> }
> @@ -142,7 +137,7 @@ int hvm_mmio_intercept(ioreq_t *p)
>
> static int process_portio_intercept(portio_action_t action, ioreq_t *p)
> {
> - int rc = X86EMUL_OKAY, i, sign = p->df ? -1 : 1;
> + int rc = X86EMUL_OKAY, i, step = p->df ? -p->size : p->size;
> uint32_t data;
>
> if ( !p->data_is_ptr )
> @@ -167,8 +162,7 @@ static int process_portio_intercept(port
> rc = action(IOREQ_READ, p->addr, p->size, &data);
> if ( rc != X86EMUL_OKAY )
> break;
> - (void)hvm_copy_to_guest_phys(p->data + sign*i*p->size,
> - &data, p->size);
> + (void)hvm_copy_to_guest_phys(p->data + step * i, &data, p->size);
> }
> }
> else /* p->dir == IOREQ_WRITE */
> @@ -176,8 +170,7 @@ static int process_portio_intercept(port
> for ( i = 0; i < p->count; i++ )
> {
> data = 0;
> - switch ( hvm_copy_from_guest_phys(&data,
> - p->data + sign * i * p->size,
> + switch ( hvm_copy_from_guest_phys(&data, p->data + step * i,
> p->size) )
> {
> case HVMCOPY_okay:
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -294,7 +294,7 @@ void hvm_io_assist(void)
>
> static int dpci_ioport_read(uint32_t mport, ioreq_t *p)
> {
> - int i, sign = p->df ? -1 : 1;
> + int i, step = p->df ? -p->size : p->size;
> uint32_t data = 0;
>
> for ( i = 0; i < p->count; i++ )
> @@ -317,8 +317,7 @@ static int dpci_ioport_read(uint32_t mpo
> if ( p->data_is_ptr )
> {
> int ret;
> - ret = hvm_copy_to_guest_phys(p->data + (sign * i * p->size), &data,
> - p->size);
> + ret = hvm_copy_to_guest_phys(p->data + step * i, &data, p->size);
> if ( (ret == HVMCOPY_gfn_paged_out) ||
> (ret == HVMCOPY_gfn_shared) )
> return X86EMUL_RETRY;
> @@ -332,7 +331,7 @@ static int dpci_ioport_read(uint32_t mpo
>
> static int dpci_ioport_write(uint32_t mport, ioreq_t *p)
> {
> - int i, sign = p->df ? -1 : 1;
> + int i, step = p->df ? -p->size : p->size;
> uint32_t data;
>
> for ( i = 0; i < p->count; i++ )
> @@ -340,8 +339,7 @@ static int dpci_ioport_write(uint32_t mp
> data = p->data;
> if ( p->data_is_ptr )
> {
> - switch ( hvm_copy_from_guest_phys(&data,
> - p->data + sign * i * p->size,
> + switch ( hvm_copy_from_guest_phys(&data, p->data + step * i,
> p->size) )
> {
> case HVMCOPY_okay:
> --- a/xen/arch/x86/hvm/stdvga.c
> +++ b/xen/arch/x86/hvm/stdvga.c
> @@ -467,15 +467,17 @@ static uint32_t read_data;
> static int mmio_move(struct hvm_hw_stdvga *s, ioreq_t *p)
> {
> int i;
> - int sign = p->df ? -1 : 1;
> + uint64_t addr = p->addr;
> p2m_type_t p2mt;
> struct domain *d = current->domain;
>
> if ( p->data_is_ptr )
> {
> + uint64_t data = p->data, tmp;
> + int step = p->df ? -p->size : p->size;
> +
> if ( p->dir == IOREQ_READ )
> {
> - uint64_t addr = p->addr, data = p->data, tmp;
> for ( i = 0; i < p->count; i++ )
> {
> tmp = stdvga_mem_read(addr, p->size);
> @@ -498,13 +500,12 @@ static int mmio_move(struct hvm_hw_stdvg
> ASSERT(!dp);
> stdvga_mem_write(data, tmp, p->size);
> }
> - data += sign * p->size;
> - addr += sign * p->size;
> + data += step;
> + addr += step;
> }
> }
> else
> {
> - uint32_t addr = p->addr, data = p->data, tmp;
> for ( i = 0; i < p->count; i++ )
> {
> if ( hvm_copy_from_guest_phys(&tmp, data, p->size) !=
> @@ -523,31 +524,18 @@ static int mmio_move(struct hvm_hw_stdvg
> tmp = stdvga_mem_read(data, p->size);
> }
> stdvga_mem_write(addr, tmp, p->size);
> - data += sign * p->size;
> - addr += sign * p->size;
> + data += step;
> + addr += step;
> }
> }
> }
> else
> {
> + ASSERT(p->count == 1);
> if ( p->dir == IOREQ_READ )
> - {
> - uint32_t addr = p->addr;
> - for ( i = 0; i < p->count; i++ )
> - {
> - p->data = stdvga_mem_read(addr, p->size);
> - addr += sign * p->size;
> - }
> - }
> + p->data = stdvga_mem_read(addr, p->size);
> else
> - {
> - uint32_t addr = p->addr;
> - for ( i = 0; i < p->count; i++ )
> - {
> - stdvga_mem_write(addr, p->data, p->size);
> - addr += sign * p->size;
> - }
> - }
> + stdvga_mem_write(addr, p->data, p->size);
> }
>
> read_data = p->data;
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 8763 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2013-10-08 16:36 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-30 12:51 [PATCH 0/5] x86/HVM: XSA-63 follow-ups Jan Beulich
2013-09-30 12:57 ` [PATCH 1/5] x86/HVM: properly handle backward string instruction emulation Jan Beulich
2013-10-08 16:36 ` Andrew Cooper [this message]
2013-09-30 12:57 ` [PATCH 2/5] x86/HVM: fix direct PCI port I/O emulation retry and error handling Jan Beulich
2013-10-08 18:13 ` Andrew Cooper
2013-09-30 12:58 ` [PATCH 3/5] x86/HVM: don't ignore hvm_copy_to_guest_phys() errors during I/O intercept Jan Beulich
2013-10-08 18:20 ` Andrew Cooper
2013-10-09 7:48 ` Jan Beulich
2013-09-30 12:58 ` [PATCH 4/5] x86/HVM: properly deal with hvm_copy_*_guest_phys() errors Jan Beulich
2013-09-30 13:07 ` Andrew Cooper
2013-09-30 12:59 ` [PATCH 5/5] x86/HVM: cache emulated instruction for retry processing Jan Beulich
2013-10-10 11:35 ` Andrew Cooper
2013-12-18 8:36 ` Zhang, Yang Z
2013-12-18 8:48 ` Jan Beulich
2013-12-18 9:40 ` Zhang, Yang Z
2013-12-18 10:53 ` Jan Beulich
2013-12-24 11:29 ` Zhang, Yang Z
2014-01-07 8:28 ` Jan Beulich
2014-01-07 8:54 ` Zhang, Yang Z
2014-01-07 9:43 ` Egger, Christoph
2014-01-08 5:50 ` Zhang, Yang Z
2014-01-09 12:19 ` Egger, Christoph
2014-01-16 4:42 ` Zhang, Yang Z
2014-01-16 8:23 ` Jan Beulich
2014-01-17 2:53 ` Zhang, Yang Z
2013-10-08 15:10 ` Ping: [PATCH 0/5] x86/HVM: XSA-63 follow-ups Jan Beulich
2013-10-14 7:29 ` Keir Fraser
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=525434A0.6050500@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=keir@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.