Re: [Xen-devel] [PATCH v9 3/5] x86/ioreq server: Handle read-modify-write cases for p2m_ioreq_server pages.

2017-03-22 Thread Jan Beulich
>>> On 21.03.17 at 03:52,  wrote:
> @@ -226,6 +249,17 @@ static int hvmemul_do_io(
>  vio->io_req.state = STATE_IOREQ_NONE;
>  break;
>  }
> +
> +/*
> + * This is part of a read-modify-write instruction.
> + * Emulate the read part so we have the value cached.

s/cached/available/ ?

Other than that
Reviewed-by: Jan Beulich 

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


[Xen-devel] [PATCH v9 3/5] x86/ioreq server: Handle read-modify-write cases for p2m_ioreq_server pages.

2017-03-20 Thread Yu Zhang
In ept_handle_violation(), write violations are also treated as
read violations. And when a VM is accessing a write-protected
address with read-modify-write instructions, the read emulation
process is triggered first.

For p2m_ioreq_server pages, current ioreq server only forwards
the write operations to the device model. Therefore when such page
is being accessed by a read-modify-write instruction, the read
operations should be emulated here in hypervisor. This patch provides
such a handler to copy the data to the buffer.

Note: MMIOs with p2m_mmio_dm type do not need such special treatment
because both reads and writes will go to the device mode.

Signed-off-by: Paul Durrant 
Signed-off-by: Yu Zhang 
---
Cc: Paul Durrant 
Cc: Jan Beulich 
Cc: Andrew Cooper 

changes in v3: 
  - According to comments from Jan: clarify comments in hvmemul_do_io().

changes in v2: 
  - According to comments from Jan: rename mem_ops to ioreq_server_ops.
  - According to comments from Jan: use hvm_copy_from_guest_phys() in
ioreq_server_read(), instead of do it by myself.
---
 xen/arch/x86/hvm/emulate.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 37139e6..52c726e 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -94,6 +94,26 @@ static const struct hvm_io_handler null_handler = {
 .ops = _ops
 };
 
+static int ioreq_server_read(const struct hvm_io_handler *io_handler,
+uint64_t addr,
+uint32_t size,
+uint64_t *data)
+{
+if ( hvm_copy_from_guest_phys(data, addr, size) != HVMCOPY_okay )
+return X86EMUL_UNHANDLEABLE;
+
+return X86EMUL_OKAY;
+}
+
+static const struct hvm_io_ops ioreq_server_ops = {
+.read = ioreq_server_read,
+.write = null_write
+};
+
+static const struct hvm_io_handler ioreq_server_handler = {
+.ops = _server_ops
+};
+
 static int hvmemul_do_io(
 bool_t is_mmio, paddr_t addr, unsigned long *reps, unsigned int size,
 uint8_t dir, bool_t df, bool_t data_is_addr, uintptr_t data)
@@ -193,6 +213,9 @@ static int hvmemul_do_io(
  *   - If the domain ioreq_server is NULL, assume there is a
  *   race between the unbinding of ioreq server and guest fault
  *   so re-try the instruction.
+ *
+ *   - If the accesss is a read, this could be part of a
+ *   read-modify-write instruction, emulate the read first.
  */
 struct hvm_ioreq_server *s = NULL;
 p2m_type_t p2mt = p2m_invalid;
@@ -226,6 +249,17 @@ static int hvmemul_do_io(
 vio->io_req.state = STATE_IOREQ_NONE;
 break;
 }
+
+/*
+ * This is part of a read-modify-write instruction.
+ * Emulate the read part so we have the value cached.
+ */
+if ( dir == IOREQ_READ )
+{
+rc = hvm_process_io_intercept(_server_handler, );
+vio->io_req.state = STATE_IOREQ_NONE;
+break;
+}
 }
 }
 
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel