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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).