Re: [PATCH V8] x86/altp2m: Hypercall to set altp2m view visibility
On 13.04.2020 08:51, Alexandru Isaila wrote:
> @@ -4786,6 +4787,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();
> }
Coverity points out that there's a break statement missing here.
Which makes me wonder how you would have successfully tested this
on a debug build. Please submit a fix (Coverity ID: 1461759).
Jan
Re: [PATCH V8] x86/altp2m: Hypercall to set altp2m view visibility
On Mon, Apr 13, 2020 at 09:51:13AM +0300, Alexandru Isaila wrote:
[...]
> ---
> tools/libxc/include/xenctrl.h | 7 +++
> tools/libxc/xc_altp2m.c | 24 +++
Acked-by: Wei Liu
> 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 58fa931de1..5f25c5a6d4 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
Ping: [PATCH V8] x86/altp2m: Hypercall to set altp2m view visibility
Hi,
I need a review for the tools bits in this patch.
Thanks,
Alex
On 13.04.2020 09:51, Alexandru Isaila wrote:
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.
The new xc_altp2m_set_visibility() solves this by making views invisible
to vmfunc.
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().
The usage of this hypercall is aimed at dom0 having a logic with a number of
views
created and at some time there is a need to be sure that only some of the views
can be switched, saving the rest and making them visible when the time
is right.
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
Reviewed-by: Jan Beulich
Reviewed-by: Kevin Tian
---
CC: Ian Jackson
CC: Wei Liu
CC: Andrew Cooper
CC: George Dunlap
CC: Jan Beulich
CC: Julien Grall
CC: Stefano Stabellini
CC: "Roger Pau Monné"
CC: Jun Nakajima
CC: Kevin Tian
---
Changes since V7:
- Change altp2m_working_eptp to altp2m_visible_eptp
- Rebase.
Changes since V6:
- Update commit message.
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 58fa931de1..5f25c5a6d4 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 827c5fa89d..6f6f3f73a8 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4509,6 +4509,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;
[PATCH V8] x86/altp2m: Hypercall to set altp2m view visibility
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.
The new xc_altp2m_set_visibility() solves this by making views invisible
to vmfunc.
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().
The usage of this hypercall is aimed at dom0 having a logic with a number of
views
created and at some time there is a need to be sure that only some of the views
can be switched, saving the rest and making them visible when the time
is right.
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
Reviewed-by: Jan Beulich
Reviewed-by: Kevin Tian
---
CC: Ian Jackson
CC: Wei Liu
CC: Andrew Cooper
CC: George Dunlap
CC: Jan Beulich
CC: Julien Grall
CC: Stefano Stabellini
CC: "Roger Pau Monné"
CC: Jun Nakajima
CC: Kevin Tian
---
Changes since V7:
- Change altp2m_working_eptp to altp2m_visible_eptp
- Rebase.
Changes since V6:
- Update commit message.
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 58fa931de1..5f25c5a6d4 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 827c5fa89d..6f6f3f73a8 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4509,6 +4509,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:
@@ -4786,6 +4787,19 @@ static int do_altp2m_op(
break;
}
+case HVMOP_altp2m_set_visibility:
+{
+unsigned
