| Message ID | 20200303122240.27013-1-aisaila@bitdefender.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V6] x86/altp2m: Hypercall to set altp2m view visibility | expand |
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 <aisaila@bitdefender.com> Hypervisor parts Reviewed-by: Jan Beulich <jbeulich@suse.com>
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 <aisaila@bitdefender.com> > --- > CC: Ian Jackson <ian.jackson@eu.citrix.com> > CC: Wei Liu <wl@xen.org> > CC: Andrew Cooper <andrew.cooper3@citrix.com> > CC: George Dunlap <George.Dunlap@eu.citrix.com> > CC: Jan Beulich <jbeulich@suse.com> > CC: Julien Grall <julien@xen.org> > CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: "Roger Pau Monné" <roger.pau@citrix.com> > CC: Jun Nakajima <jun.nakajima@intel.com> > CC: Kevin Tian <kevin.tian@intel.com> > --- > 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; > }; >
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 <aisaila@bitdefender.com> >> --- >> CC: Ian Jackson <ian.jackson@eu.citrix.com> >> CC: Wei Liu <wl@xen.org> >> CC: Andrew Cooper <andrew.cooper3@citrix.com> >> CC: George Dunlap <George.Dunlap@eu.citrix.com> >> CC: Jan Beulich <jbeulich@suse.com> >> CC: Julien Grall <julien@xen.org> >> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >> CC: Stefano Stabellini <sstabellini@kernel.org> >> CC: "Roger Pau Monné" <roger.pau@citrix.com> >> CC: Jun Nakajima <jun.nakajima@intel.com> >> CC: Kevin Tian <kevin.tian@intel.com> >> --- >> 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
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 <aisaila@bitdefender.com> >>> --- >>> CC: Ian Jackson <ian.jackson@eu.citrix.com> >>> CC: Wei Liu <wl@xen.org> >>> CC: Andrew Cooper <andrew.cooper3@citrix.com> >>> CC: George Dunlap <George.Dunlap@eu.citrix.com> >>> CC: Jan Beulich <jbeulich@suse.com> >>> CC: Julien Grall <julien@xen.org> >>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >>> CC: Stefano Stabellini <sstabellini@kernel.org> >>> CC: "Roger Pau Monné" <roger.pau@citrix.com> >>> CC: Jun Nakajima <jun.nakajima@intel.com> >>> CC: Kevin Tian <kevin.tian@intel.com> >>> --- >>> 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
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 <aisaila@bitdefender.com> > --- > CC: Ian Jackson <ian.jackson@eu.citrix.com> > CC: Wei Liu <wl@xen.org> > CC: Andrew Cooper <andrew.cooper3@citrix.com> > CC: George Dunlap <George.Dunlap@eu.citrix.com> > CC: Jan Beulich <jbeulich@suse.com> > CC: Julien Grall <julien@xen.org> > CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: "Roger Pau Monné" <roger.pau@citrix.com> > CC: Jun Nakajima <jun.nakajima@intel.com> > CC: Kevin Tian <kevin.tian@intel.com> > --- > 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; > }; >
> From: Alexandru Stefan ISAILA <aisaila@bitdefender.com> > 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@bitdefender.com> > --- > CC: Ian Jackson <ian.jackson@eu.citrix.com> > CC: Wei Liu <wl@xen.org> > CC: Andrew Cooper <andrew.cooper3@citrix.com> > CC: George Dunlap <George.Dunlap@eu.citrix.com> > CC: Jan Beulich <jbeulich@suse.com> > CC: Julien Grall <julien@xen.org> > CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: "Roger Pau Monné" <roger.pau@citrix.com> > CC: Jun Nakajima <jun.nakajima@intel.com> > CC: Kevin Tian <kevin.tian@intel.com> > --- > 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
Hi Kevin and sorry for the long reply time, On 10.03.2020 04:04, sTian, Kevin wrote: >> From: Alexandru Stefan ISAILA <aisaila@bitdefender.com> >> 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
> From: Isaila Alexandru <aisaila@bitdefender.com> > 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 <aisaila@bitdefender.com> > >> 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
On 27.03.2020 04:30, Tian, Kevin wrote: >> From: Isaila Alexandru <aisaila@bitdefender.com> >> 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 <aisaila@bitdefender.com> >>>> 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
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; };
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 <aisaila@bitdefender.com> --- CC: Ian Jackson <ian.jackson@eu.citrix.com> CC: Wei Liu <wl@xen.org> CC: Andrew Cooper <andrew.cooper3@citrix.com> CC: George Dunlap <George.Dunlap@eu.citrix.com> CC: Jan Beulich <jbeulich@suse.com> CC: Julien Grall <julien@xen.org> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: "Roger Pau Monné" <roger.pau@citrix.com> CC: Jun Nakajima <jun.nakajima@intel.com> CC: Kevin Tian <kevin.tian@intel.com> --- 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(-)