|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility
> From: Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>
> Sent: Tuesday, March 3, 2020 8:23 PM
>
> At this moment a guest can call vmfunc to change the altp2m view. This
> should be limited in order to avoid any unwanted view switch.
I look forward to more elaboration of the motivation, especially for one
who doesn't track altp2m closely like me. For example, do_altp2m_op
mentions three modes: external, internal, coordinated. Then is this patch
trying to limit the view switch in all three modes or just one of them?
from the definition clearly external disallows guest to change any view
(then why do we want per-view visibility control) while the latter two
both allows guest to switch the view. later you noted some exception
with mixed (internal) mode. then is this restriction pushed just for
limited (coordinated) mode?
btw I'm not sure why altp2m invents two names per mode, and their
mapping looks a bit weird. e.g. isn't 'coordinated' mode sound more
like 'mixed' mode?
>
> The new xc_altp2m_set_visibility() solves this by making views invisible
> to vmfunc.
if one doesn't want to make view visible to vmfunc, why can't he just
avoids registering the view at the first place? Are you aiming for a
scenario that dom0 may register 10 views, with 5 views visible to
vmfunc with the other 5 views switched by dom0 itself?
> This is done by having a separate arch.altp2m_working_eptp that is
> populated and made invalid in the same places as altp2m_eptp. This is
> written to EPTP_LIST_ADDR.
> The views are made in/visible by marking them with INVALID_MFN or
> copying them back from altp2m_eptp.
> To have consistency the visibility also applies to
> p2m_switch_domain_altp2m_by_id().
>
> Note: If altp2m mode is set to mixed the guest is able to change the view
> visibility and then call vmfunc.
>
> Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
> ---
> CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> CC: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
> Changes since V5:
> - Change idx type from uint16_t to unsigned int
> - Add rc var and dropped the err return from p2m_get_suppress_ve().
>
> Changes since V4:
> - Move p2m specific things from hvm to p2m.c
> - Add comment for altp2m_idx bounds check
> - Add altp2m_list_lock/unlock().
>
> Changes since V3:
> - Change var name form altp2m_idx to idx to shorten line length
> - Add bounds check for idx
> - Update commit message
> - Add comment in xenctrl.h.
>
> Changes since V2:
> - Drop hap_enabled() check
> - Reduce the indentation depth in hvm.c
> - Fix assignment indentation
> - Drop pad2.
>
> Changes since V1:
> - Drop double view from title.
> ---
> tools/libxc/include/xenctrl.h | 7 +++++++
> tools/libxc/xc_altp2m.c | 24 +++++++++++++++++++++++
> xen/arch/x86/hvm/hvm.c | 14 ++++++++++++++
> xen/arch/x86/hvm/vmx/vmx.c | 2 +-
> xen/arch/x86/mm/hap/hap.c | 15 +++++++++++++++
> xen/arch/x86/mm/p2m-ept.c | 1 +
> xen/arch/x86/mm/p2m.c | 34 +++++++++++++++++++++++++++++++--
> xen/include/asm-x86/domain.h | 1 +
> xen/include/asm-x86/p2m.h | 4 ++++
> xen/include/public/hvm/hvm_op.h | 9 +++++++++
> 10 files changed, 108 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index fc6e57a1a0..2e6e652678 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1943,6 +1943,13 @@ int xc_altp2m_change_gfn(xc_interface *handle,
> uint32_t domid,
> xen_pfn_t new_gfn);
> int xc_altp2m_get_vcpu_p2m_idx(xc_interface *handle, uint32_t domid,
> uint32_t vcpuid, uint16_t *p2midx);
> +/*
> + * Set view visibility for xc_altp2m_switch_to_view and vmfunc.
> + * Note: If altp2m mode is set to mixed the guest is able to change the view
> + * visibility and then call vmfunc.
> + */
> +int xc_altp2m_set_visibility(xc_interface *handle, uint32_t domid,
> + uint16_t view_id, bool visible);
>
> /**
> * Mem paging operations.
> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
> index 46fb725806..6987c9541f 100644
> --- a/tools/libxc/xc_altp2m.c
> +++ b/tools/libxc/xc_altp2m.c
> @@ -410,3 +410,27 @@ int xc_altp2m_get_vcpu_p2m_idx(xc_interface
> *handle, uint32_t domid,
> xc_hypercall_buffer_free(handle, arg);
> return rc;
> }
> +
> +int xc_altp2m_set_visibility(xc_interface *handle, uint32_t domid,
> + uint16_t view_id, bool visible)
> +{
> + int rc;
> +
> + DECLARE_HYPERCALL_BUFFER(xen_hvm_altp2m_op_t, arg);
> +
> + arg = xc_hypercall_buffer_alloc(handle, arg, sizeof(*arg));
> + if ( arg == NULL )
> + return -1;
> +
> + arg->version = HVMOP_ALTP2M_INTERFACE_VERSION;
> + arg->cmd = HVMOP_altp2m_set_visibility;
> + arg->domain = domid;
> + arg->u.set_visibility.altp2m_idx = view_id;
> + arg->u.set_visibility.visible = visible;
> +
> + rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
> + HYPERCALL_BUFFER_AS_ARG(arg));
> +
> + xc_hypercall_buffer_free(handle, arg);
> + return rc;
> +}
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index db5d7b4d30..7e631e30dd 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4564,6 +4564,7 @@ static int do_altp2m_op(
> case HVMOP_altp2m_get_mem_access:
> case HVMOP_altp2m_change_gfn:
> case HVMOP_altp2m_get_p2m_idx:
> + case HVMOP_altp2m_set_visibility:
> break;
>
> default:
> @@ -4841,6 +4842,19 @@ static int do_altp2m_op(
> break;
> }
>
> + case HVMOP_altp2m_set_visibility:
> + {
> + unsigned int idx = a.u.set_visibility.altp2m_idx;
> +
> + if ( a.u.set_visibility.pad )
> + rc = -EINVAL;
> + else if ( !altp2m_active(d) )
> + rc = -EOPNOTSUPP;
> + else
> + rc = p2m_set_altp2m_view_visibility(d, idx,
> + a.u.set_visibility.visible);
> + }
> +
> default:
> ASSERT_UNREACHABLE();
> }
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index d265ed46ad..bb44ef39a1 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2140,7 +2140,7 @@ static void vmx_vcpu_update_vmfunc_ve(struct
> vcpu *v)
> {
> v->arch.hvm.vmx.secondary_exec_control |= mask;
> __vmwrite(VM_FUNCTION_CONTROL,
> VMX_VMFUNC_EPTP_SWITCHING);
> - __vmwrite(EPTP_LIST_ADDR, virt_to_maddr(d->arch.altp2m_eptp));
> + __vmwrite(EPTP_LIST_ADDR, virt_to_maddr(d-
> >arch.altp2m_working_eptp));
>
> if ( cpu_has_vmx_virt_exceptions )
> {
> diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> index 3d93f3451c..5969ec8922 100644
> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -488,8 +488,17 @@ int hap_enable(struct domain *d, u32 mode)
> goto out;
> }
>
> + if ( (d->arch.altp2m_working_eptp = alloc_xenheap_page()) == NULL )
> + {
> + rv = -ENOMEM;
> + goto out;
> + }
> +
> for ( i = 0; i < MAX_EPTP; i++ )
> + {
> d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
> + d->arch.altp2m_working_eptp[i] = mfn_x(INVALID_MFN);
> + }
>
> for ( i = 0; i < MAX_ALTP2M; i++ )
> {
> @@ -523,6 +532,12 @@ void hap_final_teardown(struct domain *d)
> d->arch.altp2m_eptp = NULL;
> }
>
> + if ( d->arch.altp2m_working_eptp )
> + {
> + free_xenheap_page(d->arch.altp2m_working_eptp);
> + d->arch.altp2m_working_eptp = NULL;
> + }
> +
> for ( i = 0; i < MAX_ALTP2M; i++ )
> p2m_teardown(d->arch.altp2m_p2m[i]);
> }
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index eb0f0edfef..6539ca619b 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -1368,6 +1368,7 @@ void p2m_init_altp2m_ept(struct domain *d,
> unsigned int i)
> ept = &p2m->ept;
> ept->mfn = pagetable_get_pfn(p2m_get_pagetable(p2m));
> d->arch.altp2m_eptp[array_index_nospec(i, MAX_EPTP)] = ept->eptp;
> + d->arch.altp2m_working_eptp[array_index_nospec(i, MAX_EPTP)] = ept-
> >eptp;
> }
>
> unsigned int p2m_find_altp2m_by_eptp(struct domain *d, uint64_t eptp)
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 3719deae77..0677691783 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -2516,6 +2516,7 @@ void p2m_flush_altp2m(struct domain *d)
> {
> p2m_reset_altp2m(d, i, ALTP2M_DEACTIVATE);
> d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
> + d->arch.altp2m_working_eptp[i] = mfn_x(INVALID_MFN);
> }
>
> altp2m_list_unlock(d);
> @@ -2635,7 +2636,9 @@ int p2m_destroy_altp2m_by_id(struct domain *d,
> unsigned int idx)
> {
> p2m_reset_altp2m(d, idx, ALTP2M_DEACTIVATE);
> d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] =
> - mfn_x(INVALID_MFN);
> + mfn_x(INVALID_MFN);
> + d->arch.altp2m_working_eptp[array_index_nospec(idx, MAX_EPTP)]
> =
> + mfn_x(INVALID_MFN);
> rc = 0;
> }
> }
> @@ -2662,7 +2665,7 @@ int p2m_switch_domain_altp2m_by_id(struct
> domain *d, unsigned int idx)
> rc = -EINVAL;
> altp2m_list_lock(d);
>
> - if ( d->arch.altp2m_eptp[idx] != mfn_x(INVALID_MFN) )
> + if ( d->arch.altp2m_working_eptp[idx] != mfn_x(INVALID_MFN) )
> {
> for_each_vcpu( d, v )
> if ( idx != vcpu_altp2m(v).p2midx )
> @@ -3146,6 +3149,33 @@ int p2m_get_suppress_ve(struct domain *d, gfn_t
> gfn, bool *suppress_ve,
>
> return rc;
> }
> +
> +int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int
> altp2m_idx,
> + uint8_t visible)
> +{
> + int rc = 0;
> +
> + altp2m_list_lock(d);
> +
> + /*
> + * Eptp index is correlated with altp2m index and should not exceed
> + * min(MAX_ALTP2M, MAX_EPTP).
> + */
> + if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> + d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
> + mfn_x(INVALID_MFN) )
> + rc = -EINVAL;
> + else if ( visible )
> + d->arch.altp2m_working_eptp[array_index_nospec(altp2m_idx,
> MAX_EPTP)] =
> + d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)];
> + else
> + d->arch.altp2m_working_eptp[array_index_nospec(altp2m_idx,
> MAX_EPTP)] =
> + mfn_x(INVALID_MFN);
> +
> + altp2m_list_unlock(d);
> +
> + return rc;
> +}
> #endif
>
> /*
> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
> index 105adf96eb..800e12eae5 100644
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -327,6 +327,7 @@ struct arch_domain
> struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
> mm_lock_t altp2m_list_lock;
> uint64_t *altp2m_eptp;
> + uint64_t *altp2m_working_eptp;
> #endif
>
> /* NB. protected by d->event_lock and by irq_desc[irq].lock */
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index 0cf531abb7..0f7ec4a9f6 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -897,6 +897,10 @@ int p2m_change_altp2m_gfn(struct domain *d,
> unsigned int idx,
> int p2m_altp2m_propagate_change(struct domain *d, gfn_t gfn,
> mfn_t mfn, unsigned int page_order,
> p2m_type_t p2mt, p2m_access_t p2ma);
> +
> +/* Set a specific p2m view visibility */
> +int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int idx,
> + uint8_t visible);
> #else
> struct p2m_domain *p2m_get_altp2m(struct vcpu *v);
> static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx) {}
> diff --git a/xen/include/public/hvm/hvm_op.h
> b/xen/include/public/hvm/hvm_op.h
> index b599d3cbd0..870ec52060 100644
> --- a/xen/include/public/hvm/hvm_op.h
> +++ b/xen/include/public/hvm/hvm_op.h
> @@ -318,6 +318,12 @@ struct xen_hvm_altp2m_get_vcpu_p2m_idx {
> uint16_t altp2m_idx;
> };
>
> +struct xen_hvm_altp2m_set_visibility {
> + uint16_t altp2m_idx;
> + uint8_t visible;
> + uint8_t pad;
> +};
> +
> struct xen_hvm_altp2m_op {
> uint32_t version; /* HVMOP_ALTP2M_INTERFACE_VERSION */
> uint32_t cmd;
> @@ -350,6 +356,8 @@ struct xen_hvm_altp2m_op {
> #define HVMOP_altp2m_get_p2m_idx 14
> /* Set the "Supress #VE" bit for a range of pages */
> #define HVMOP_altp2m_set_suppress_ve_multi 15
> +/* Set visibility for a given altp2m view */
> +#define HVMOP_altp2m_set_visibility 16
> domid_t domain;
> uint16_t pad1;
> uint32_t pad2;
> @@ -367,6 +375,7 @@ struct xen_hvm_altp2m_op {
> struct xen_hvm_altp2m_suppress_ve_multi suppress_ve_multi;
> struct xen_hvm_altp2m_vcpu_disable_notify disable_notify;
> struct xen_hvm_altp2m_get_vcpu_p2m_idx get_vcpu_p2m_idx;
> + struct xen_hvm_altp2m_set_visibility set_visibility;
> uint8_t pad[64];
> } u;
> };
> --
> 2.17.1
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |