Re: [Xen-devel] [PATCH v2 1/2] vm_event: Sanitize vm_event response handling

[Razvan Cojocaru]

On 09/15/16 19:51, Tamas K Lengyel wrote:
> Setting response flags in vm_event are only ever safe if the vCPUs are paused.
> To reflect this we move all checks within the if block that already checks
> whether this is the case. Checks that are only supported on one architecture
> we relocate the bitmask operations to the arch-specific handlers to avoid
> the overhead on architectures that don't support it.
> 
> Furthermore, we clean-up the emulation checks so it more clearly represents 
> the
> decision-logic when emulation should take place. As part of this we also
> set the stage to allow emulation in response to other types of events, not 
> just
> mem_access violations.
> 
> Signed-off-by: Tamas K Lengyel 
> Acked-by: George Dunlap 
> ---
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Razvan Cojocaru 
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> 
> v2: use bool instead of bool_t

Acked-by: Razvan Cojocaru 


Thanks,
Razvan

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

[Xen-devel] [PATCH v2 1/2] vm_event: Sanitize vm_event response handling

[Tamas K Lengyel]

Setting response flags in vm_event are only ever safe if the vCPUs are paused.
To reflect this we move all checks within the if block that already checks
whether this is the case. Checks that are only supported on one architecture
we relocate the bitmask operations to the arch-specific handlers to avoid
the overhead on architectures that don't support it.

Furthermore, we clean-up the emulation checks so it more clearly represents the
decision-logic when emulation should take place. As part of this we also
set the stage to allow emulation in response to other types of events, not just
mem_access violations.

Signed-off-by: Tamas K Lengyel 
Acked-by: George Dunlap 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Razvan Cojocaru 
Cc: Stefano Stabellini 
Cc: Julien Grall 

v2: use bool instead of bool_t
---
 xen/arch/x86/mm/p2m.c  | 79 +++---
 xen/arch/x86/vm_event.c| 35 ++-
 xen/common/vm_event.c  | 53 ++--
 xen/include/asm-arm/p2m.h  |  3 +-
 xen/include/asm-arm/vm_event.h |  9 -
 xen/include/asm-x86/p2m.h  |  2 +-
 xen/include/asm-x86/vm_event.h |  5 ++-
 xen/include/xen/mem_access.h   | 12 ---
 8 files changed, 111 insertions(+), 87 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 7d14c3b..faffc2a 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1588,62 +1588,55 @@ void p2m_mem_paging_resume(struct domain *d, 
vm_event_response_t *rsp)
 }
 }
 
-void p2m_mem_access_emulate_check(struct vcpu *v,
+bool p2m_mem_access_emulate_check(struct vcpu *v,
   const vm_event_response_t *rsp)
 {
-/* Mark vcpu for skipping one instruction upon rescheduling. */
-if ( rsp->flags & VM_EVENT_FLAG_EMULATE )
-{
-xenmem_access_t access;
-bool_t violation = 1;
-const struct vm_event_mem_access *data = >u.mem_access;
+xenmem_access_t access;
+bool violation = 1;
+const struct vm_event_mem_access *data = >u.mem_access;
 
-if ( p2m_get_mem_access(v->domain, _gfn(data->gfn), ) == 0 )
+if ( p2m_get_mem_access(v->domain, _gfn(data->gfn), ) == 0 )
+{
+switch ( access )
 {
-switch ( access )
-{
-case XENMEM_access_n:
-case XENMEM_access_n2rwx:
-default:
-violation = data->flags & MEM_ACCESS_RWX;
-break;
+case XENMEM_access_n:
+case XENMEM_access_n2rwx:
+default:
+violation = data->flags & MEM_ACCESS_RWX;
+break;
 
-case XENMEM_access_r:
-violation = data->flags & MEM_ACCESS_WX;
-break;
+case XENMEM_access_r:
+violation = data->flags & MEM_ACCESS_WX;
+break;
 
-case XENMEM_access_w:
-violation = data->flags & MEM_ACCESS_RX;
-break;
+case XENMEM_access_w:
+violation = data->flags & MEM_ACCESS_RX;
+break;
 
-case XENMEM_access_x:
-violation = data->flags & MEM_ACCESS_RW;
-break;
+case XENMEM_access_x:
+violation = data->flags & MEM_ACCESS_RW;
+break;
 
-case XENMEM_access_rx:
-case XENMEM_access_rx2rw:
-violation = data->flags & MEM_ACCESS_W;
-break;
+case XENMEM_access_rx:
+case XENMEM_access_rx2rw:
+violation = data->flags & MEM_ACCESS_W;
+break;
 
-case XENMEM_access_wx:
-violation = data->flags & MEM_ACCESS_R;
-break;
+case XENMEM_access_wx:
+violation = data->flags & MEM_ACCESS_R;
+break;
 
-case XENMEM_access_rw:
-violation = data->flags & MEM_ACCESS_X;
-break;
+case XENMEM_access_rw:
+violation = data->flags & MEM_ACCESS_X;
+break;
 
-case XENMEM_access_rwx:
-violation = 0;
-break;
-}
+case XENMEM_access_rwx:
+violation = 0;
+break;
 }
-
-v->arch.vm_event->emulate_flags = violation ? rsp->flags : 0;
-
-if ( (rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA) )
-v->arch.vm_event->emul_read_data = rsp->data.emul_read_data;
 }
+
+return violation;
 }
 
 void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index e938ca3..343b9c8 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -18,6 +18,7 @@
  * License along with this program; If not, see .
  */