Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility
On 27.03.2020 04:30, Tian, Kevin wrote: From: Isaila Alexandru Sent: Tuesday, March 24, 2020 6:46 PM Hi Kevin and sorry for the long reply time, On 10.03.2020 04:04, sTian, Kevin wrote: From: Alexandru Stefan ISAILA 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? As you stated, there are some exceptions with mixed (internal) mode. This restriction is clearly used for coordinated mode but it also restricts view switching in the external mode as well. I had a good example to start with, let's say we have one external agent in dom0 that uses view1 and view2 and the logic requires the switch between the views. At this point VMFUNC is available to the guest so with a simple asm code it can witch to view 0. At this time the external agent is not aware that the view has switched and further more view0 was not supposed to be in the main logic so it crashes. This example can be extended to any number of views. I hope it can paint a more clear picture of what this patch is trying to achive. Can VMFUNC be hidden and disabled when external mode is being used? or is it because the mode can be dynamically switched after a VM is launched so you have to restrict the views in this way? Like you said, there is a problem if the mode is dynamically switched. 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? Yes that is true, it si a bit weird. 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? That is one scenario, another can be that dom0 has a number of views created and in some time it wants to be sure that only some of the views can be switched, saving the rest and making them visible when the time is right. Sure the example given up is another example. Can you update the patch description and resend? I'll take another look then. Ok, I will update the description for the next version. Thanks, Alex
Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility
> From: Isaila Alexandru > Sent: Tuesday, March 24, 2020 6:46 PM > > > Hi Kevin and sorry for the long reply time, > > On 10.03.2020 04:04, sTian, Kevin wrote: > >> From: Alexandru Stefan ISAILA > >> 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? > > > > As you stated, there are some exceptions with mixed (internal) mode. > This restriction is clearly used for coordinated mode but it also > restricts view switching in the external mode as well. I had a good > example to start with, let's say we have one external agent in dom0 that > uses view1 and view2 and the logic requires the switch between the > views. At this point VMFUNC is available to the guest so with a simple > asm code it can witch to view 0. At this time the external agent is not > aware that the view has switched and further more view0 was not supposed > to be in the main logic so it crashes. This example can be extended to > any number of views. I hope it can paint a more clear picture of what > this patch is trying to achive. Can VMFUNC be hidden and disabled when external mode is being used? or is it because the mode can be dynamically switched after a VM is launched so you have to restrict the views in this way? > > > 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? > > Yes that is true, it si a bit weird. > > > > >> > >> 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? > > That is one scenario, another can be that dom0 has a number of views > created and in some time it wants to be sure that only some of the views > can be switched, saving the rest and making them visible when the time > is right. Sure the example given up is another example. > Can you update the patch description and resend? I'll take another look then. Thanks Kevin
Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility
Hi Kevin and sorry for the long reply time, On 10.03.2020 04:04, sTian, Kevin wrote: From: Alexandru Stefan ISAILA 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? As you stated, there are some exceptions with mixed (internal) mode. This restriction is clearly used for coordinated mode but it also restricts view switching in the external mode as well. I had a good example to start with, let's say we have one external agent in dom0 that uses view1 and view2 and the logic requires the switch between the views. At this point VMFUNC is available to the guest so with a simple asm code it can witch to view 0. At this time the external agent is not aware that the view has switched and further more view0 was not supposed to be in the main logic so it crashes. This example can be extended to any number of views. I hope it can paint a more clear picture of what this patch is trying to achive. 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? Yes that is true, it si a bit weird. 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? That is one scenario, another can be that dom0 has a number of views created and in some time it wants to be sure that only some of the views can be switched, saving the rest and making them visible when the time is right. Sure the example given up is another example. Regards, Alex
Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility
> From: Alexandru Stefan ISAILA
> 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
> ---
> CC: Ian Jackson
> CC: Wei Liu
> CC: Andrew Cooper
> CC: George Dunlap
> CC: Jan Beulich
> CC: Julien Grall
> CC: Konrad Rzeszutek Wilk
> CC: Stefano Stabellini
> CC: "Roger Pau Monné"
> CC: Jun Nakajima
> CC: Kevin Tian
> ---
> 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 =
Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility
Hi,
Any thoughts on this patch are appreciated.
Thanks,
Alex
On 03.03.2020 14:23, Alexandru Stefan 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().
>
> 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
> ---
> CC: Ian Jackson
> CC: Wei Liu
> CC: Andrew Cooper
> CC: George Dunlap
> CC: Jan Beulich
> CC: Julien Grall
> CC: Konrad Rzeszutek Wilk
> CC: Stefano Stabellini
> CC: "Roger Pau Monné"
> CC: Jun Nakajima
> CC: Kevin Tian
> ---
> 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 =
Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility
On 04.03.2020 16:07, Jan Beulich wrote: > On 04.03.2020 14:57, Alexandru Stefan ISAILA wrote: >> Hi George, >> >> This is a kind reminder if you can take a look at this patch when you >> have the time. > > Are you perhaps not aware of the recent maintainer change on > xen/arch/x86/mm/? What you need to go hunt is ... > >> On 03.03.2020 14:23, Alexandru Stefan 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(). >>> >>> 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 >>> --- >>> CC: Ian Jackson >>> CC: Wei Liu >>> CC: Andrew Cooper >>> CC: George Dunlap >>> CC: Jan Beulich >>> CC: Julien Grall >>> CC: Konrad Rzeszutek Wilk >>> CC: Stefano Stabellini >>> CC: "Roger Pau Monné" >>> CC: Jun Nakajima >>> CC: Kevin Tian >>> --- >>> 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 +++ > > ... a tool stack ack and ... > >>>xen/arch/x86/hvm/hvm.c | 14 ++ >>>xen/arch/x86/hvm/vmx/vmx.c | 2 +- > > ... and a VMX one, also for ... > >>>xen/arch/x86/mm/hap/hap.c | 15 +++ >>>xen/arch/x86/mm/p2m-ept.c | 1 + > > ... this. > Ok, tanks for this, I just saw the changes on the maintainers. Alex ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility
On 04.03.2020 14:57, Alexandru Stefan ISAILA wrote: > Hi George, > > This is a kind reminder if you can take a look at this patch when you > have the time. Are you perhaps not aware of the recent maintainer change on xen/arch/x86/mm/? What you need to go hunt is ... > On 03.03.2020 14:23, Alexandru Stefan 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(). >> >> 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 >> --- >> CC: Ian Jackson >> CC: Wei Liu >> CC: Andrew Cooper >> CC: George Dunlap >> CC: Jan Beulich >> CC: Julien Grall >> CC: Konrad Rzeszutek Wilk >> CC: Stefano Stabellini >> CC: "Roger Pau Monné" >> CC: Jun Nakajima >> CC: Kevin Tian >> --- >> 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 +++ ... a tool stack ack and ... >> xen/arch/x86/hvm/hvm.c | 14 ++ >> xen/arch/x86/hvm/vmx/vmx.c | 2 +- ... and a VMX one, also for ... >> xen/arch/x86/mm/hap/hap.c | 15 +++ >> xen/arch/x86/mm/p2m-ept.c | 1 + ... this. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility
Hi George,
This is a kind reminder if you can take a look at this patch when you
have the time.
Thanks,
Alex
On 03.03.2020 14:23, Alexandru Stefan 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().
>
> 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
> ---
> CC: Ian Jackson
> CC: Wei Liu
> CC: Andrew Cooper
> CC: George Dunlap
> CC: Jan Beulich
> CC: Julien Grall
> CC: Konrad Rzeszutek Wilk
> CC: Stefano Stabellini
> CC: "Roger Pau Monné"
> CC: Jun Nakajima
> CC: Kevin Tian
> ---
> 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:
> +{
>
Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility
On 03.03.2020 13:23, Alexandru Stefan 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(). > > 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 Hypervisor parts Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH V6] 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().
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
---
CC: Ian Jackson
CC: Wei Liu
CC: Andrew Cooper
CC: George Dunlap
CC: Jan Beulich
CC: Julien Grall
CC: Konrad Rzeszutek Wilk
CC: Stefano Stabellini
CC: "Roger Pau Monné"
CC: Jun Nakajima
CC: Kevin Tian
---
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
