|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2] x86/HVM: fix preemption handling in do_hvm_op()
>Just like previously done for some mem-op hypercalls, undo preemption
>using the interface structures (altering it in ways the caller may not
>expect) and replace it by storing the continuation point in the high bits of
>sub-
>operation argument.
>
>This also changes the "nr" fields of struct xen_hvm_track_dirty_vram
>(operation already limited to 1Gb worth of pages) and struct
>xen_hvm_modified_memory to be only 32 bits wide, consistent with those of
>struct xen_set_mem{type,access}. If that's not acceptable for some reason,
>we'd need to shrink the HVMOP_op_bits (while still enforcing the [then
>higher] limit resulting from the need to be able to encode the continuation).
>
>Whether (and if so how) to adjust xc_hvm_track_dirty_vram(),
>xc_hvm_modified_memory(), xc_hvm_set_mem_type(), and
>xc_hvm_set_mem_access() to reflect the 32-bit restriction on "nr" is
>unclear: If the APIs need to remain stable, all four functions should probably
>check that there was no truncation. Preferably their parameters would be
>changed to uint32_t or unsigned int, though.
I am adding mem_access support to PV domains. As part of that I am adding a
more generic xc_set/get_mem_access() APIs and deprecating the
xc_hvm_set/get_mem_access() APIs. At the moment I have the
xc_hvm_set/get_mem_access() APIs calling the newer ones. I am also folding the
mem_access ops under XENMEM_access_op and deprecating the HVMOPs. Maybe as part
of this I can change the nr parameter to uint32_t. Please advice and I will
make it part of the patch which should be ready in a couple of weeks.
Thanks,
Aravindh
>As a minor cleanup, along with introducing the switch-wide "pfn" the
>redundant "d" is also being converted to a switch-wide one.
>
>Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
>--- a/xen/arch/x86/hvm/hvm.c
>+++ b/xen/arch/x86/hvm/hvm.c
>@@ -4050,20 +4050,25 @@ static int hvm_replace_event_channel(str
> return 0;
> }
>
>+#define HVMOP_op_bits 32
>+
> long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
>
> {
> struct domain *curr_d = current->domain;
>+ unsigned long start_iter = op >> HVMOP_op_bits;
> long rc = 0;
>
>- switch ( op )
>+ switch ( op &= ((1UL << HVMOP_op_bits) - 1) )
> {
>+ struct domain *d;
>+ unsigned long pfn;
>+
> case HVMOP_set_param:
> case HVMOP_get_param:
> {
> struct xen_hvm_param a;
> struct hvm_ioreq_page *iorp;
>- struct domain *d;
> struct vcpu *v;
>
> if ( copy_from_guest(&a, arg, 1) ) @@ -4327,7 +4332,6 @@ long
>do_hvm_op(unsigned long op, XEN_GUE
> case HVMOP_track_dirty_vram:
> {
> struct xen_hvm_track_dirty_vram a;
>- struct domain *d;
>
> if ( copy_from_guest(&a, arg, 1) )
> return -EFAULT;
>@@ -4368,7 +4372,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
> case HVMOP_modified_memory:
> {
> struct xen_hvm_modified_memory a;
>- struct domain *d;
>
> if ( copy_from_guest(&a, arg, 1) )
> return -EFAULT;
>@@ -4386,7 +4389,8 @@ long do_hvm_op(unsigned long op, XEN_GUE
> goto param_fail3;
>
> rc = -EINVAL;
>- if ( ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
>+ if ( a.nr < start_iter ||
>+ ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
> ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
> goto param_fail3;
>
>@@ -4394,9 +4398,8 @@ long do_hvm_op(unsigned long op, XEN_GUE
> if ( !paging_mode_log_dirty(d) )
> goto param_fail3;
>
>- while ( a.nr > 0 )
>+ for ( pfn = a.first_pfn + start_iter; a.nr > start_iter; ++pfn
>+ )
> {
>- unsigned long pfn = a.first_pfn;
> struct page_info *page;
>
> page = get_page_from_gfn(d, pfn, NULL, P2M_UNSHARE); @@ -
>4409,16 +4412,13 @@ long do_hvm_op(unsigned long op, XEN_GUE
> put_page(page);
> }
>
>- a.first_pfn++;
>- a.nr--;
>+ ++pfn;
>+ ++start_iter;
>
> /* Check for continuation if it's not the last interation */
>- if ( a.nr > 0 && hypercall_preempt_check() )
>+ if ( a.nr > start_iter && hypercall_preempt_check() )
> {
>- if ( __copy_to_guest(arg, &a, 1) )
>- rc = -EFAULT;
>- else
>- rc = -EAGAIN;
>+ rc = -EAGAIN;
> break;
> }
> }
>@@ -4431,7 +4431,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
> case HVMOP_get_mem_type:
> {
> struct xen_hvm_get_mem_type a;
>- struct domain *d;
> p2m_type_t t;
>
> if ( copy_from_guest(&a, arg, 1) ) @@ -4475,7 +4474,6 @@ long
>do_hvm_op(unsigned long op, XEN_GUE
> case HVMOP_set_mem_type:
> {
> struct xen_hvm_set_mem_type a;
>- struct domain *d;
>
> /* Interface types to internal p2m types */
> static const p2m_type_t memtype[] = { @@ -4500,20 +4498,19 @@ long
>do_hvm_op(unsigned long op, XEN_GUE
> goto param_fail4;
>
> rc = -EINVAL;
>- if ( ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
>+ if ( a.nr < start_iter ||
>+ ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
> ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
> goto param_fail4;
>
> if ( a.hvmmem_type >= ARRAY_SIZE(memtype) )
> goto param_fail4;
>
>- while ( a.nr )
>+ for ( pfn = a.first_pfn + start_iter; a.nr > start_iter; ++pfn
>+ )
> {
>- unsigned long pfn = a.first_pfn;
>- p2m_type_t t;
>- p2m_type_t nt;
>- mfn_t mfn;
>- mfn = get_gfn_unshare(d, pfn, &t);
>+ p2m_type_t t, nt;
>+
>+ get_gfn_unshare(d, pfn, &t);
> if ( p2m_is_paging(t) )
> {
> put_gfn(d, pfn);
>@@ -4550,16 +4547,13 @@ long do_hvm_op(unsigned long op, XEN_GUE
> }
> put_gfn(d, pfn);
>
>- a.first_pfn++;
>- a.nr--;
>+ ++pfn;
>+ ++start_iter;
>
> /* Check for continuation if it's not the last interation */
>- if ( a.nr > 0 && hypercall_preempt_check() )
>+ if ( a.nr > start_iter && hypercall_preempt_check() )
> {
>- if ( __copy_to_guest(arg, &a, 1) )
>- rc = -EFAULT;
>- else
>- rc = -EAGAIN;
>+ rc = -EAGAIN;
> goto param_fail4;
> }
> }
>@@ -4574,7 +4568,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
> case HVMOP_set_mem_access:
> {
> struct xen_hvm_set_mem_access a;
>- struct domain *d;
>
> if ( copy_from_guest(&a, arg, 1) )
> return -EFAULT;
>@@ -4593,19 +4586,17 @@ long do_hvm_op(unsigned long op, XEN_GUE
>
> rc = -EINVAL;
> if ( (a.first_pfn != ~0ull) &&
>- (((a.first_pfn + a.nr - 1) < a.first_pfn) ||
>+ (a.nr < start_iter ||
>+ ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
> ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d))) )
> goto param_fail5;
>
>- rc = p2m_set_mem_access(d, a.first_pfn, a.nr, a.hvmmem_access);
>+ rc = p2m_set_mem_access(d, a.first_pfn + start_iter, a.nr -
>start_iter,
>+ a.hvmmem_access);
> if ( rc > 0 )
> {
>- a.first_pfn += a.nr - rc;
>- a.nr = rc;
>- if ( __copy_to_guest(arg, &a, 1) )
>- rc = -EFAULT;
>- else
>- rc = -EAGAIN;
>+ start_iter = a.nr - rc;
>+ rc = -EAGAIN;
> }
>
> param_fail5:
>@@ -4616,7 +4607,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
> case HVMOP_get_mem_access:
> {
> struct xen_hvm_get_mem_access a;
>- struct domain *d;
> hvmmem_access_t access;
>
> if ( copy_from_guest(&a, arg, 1) ) @@ -4653,7 +4643,6 @@ long
>do_hvm_op(unsigned long op, XEN_GUE
> case HVMOP_pagetable_dying:
> {
> struct xen_hvm_pagetable_dying a;
>- struct domain *d;
>
> if ( copy_from_guest(&a, arg, 1) )
> return -EFAULT;
>@@ -4706,7 +4695,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
> case HVMOP_inject_trap:
> {
> xen_hvm_inject_trap_t tr;
>- struct domain *d;
> struct vcpu *v;
>
> if ( copy_from_guest(&tr, arg, 1 ) ) @@ -4754,8 +4742,9 @@ long
>do_hvm_op(unsigned long op, XEN_GUE
> }
>
> if ( rc == -EAGAIN )
>- rc = hypercall_create_continuation(
>- __HYPERVISOR_hvm_op, "lh", op, arg);
>+ rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, "lh",
>+ op | (start_iter << HVMOP_op_bits),
>+ arg);
>
> return rc;
> }
>--- a/xen/include/public/hvm/hvm_op.h
>+++ b/xen/include/public/hvm/hvm_op.h
>@@ -90,10 +90,10 @@ typedef enum {
> struct xen_hvm_track_dirty_vram {
> /* Domain to be tracked. */
> domid_t domid;
>+ /* Number of pages to track. */
>+ uint32_t nr;
> /* First pfn to track. */
> uint64_aligned_t first_pfn;
>- /* Number of pages to track. */
>- uint64_aligned_t nr;
> /* OUT variable. */
> /* Dirty bitmap buffer. */
> XEN_GUEST_HANDLE_64(uint8) dirty_bitmap; @@ -106,10 +106,10 @@
>DEFINE_XEN_GUEST_HANDLE(xen_hvm_track_di
> struct xen_hvm_modified_memory {
> /* Domain to be updated. */
> domid_t domid;
>+ /* Number of pages. */
>+ uint32_t nr;
> /* First pfn. */
> uint64_aligned_t first_pfn;
>- /* Number of pages. */
>- uint64_aligned_t nr;
> };
> typedef struct xen_hvm_modified_memory xen_hvm_modified_memory_t;
>DEFINE_XEN_GUEST_HANDLE(xen_hvm_modified_memory_t);
>
>
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@xxxxxxxxxxxxx
>http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |