| Message ID | 20170223094117.7212-2-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 23.02.17 at 10:41, <haozhong.zhang@intel.com> wrote: > @@ -992,10 +993,30 @@ bool_t update_secondary_system_time(struct vcpu *v, > { > XEN_GUEST_HANDLE(vcpu_time_info_t) user_u = v->arch.time_info_guest; > smap_check_policy_t saved_policy; > + bool nested_guest_mode = false; > > if ( guest_handle_is_null(user_u) ) > return 1; > > + /* > + * Must be before all following __copy_field_to_guest() and > + * __copy_to_guest(). > + * > + * Otherwise, if 'v' is in the nested guest mode, paging_gva_to_gfn() called > + * from __copy_field_to_guest() and __copy_to_guest() will treat the target > + * address as L2 gva, and __copy_field_to_guest() and __copy_to_guest() will > + * consequently copy runstate to L2 guest rather than L1 guest. > + * > + * Therefore, we clear the nested guest flag before __copy_field_to_guest() > + * and __copy_to_guest(), and restore the flag after all guest copy. > + */ > + if ( nestedhvm_enabled(v->domain) ) > + { > + nested_guest_mode = nestedhvm_is_n2(v); > + if ( nested_guest_mode ) > + nestedhvm_vcpu_exit_guestmode(v); > + } > + > saved_policy = smap_policy_change(v, SMAP_CHECK_ENABLED); > > /* 1. Update userspace version. */ There is an early exit path right below here. Taking this together with the code and comment redundancy with patch 1, this is a pretty clear sign that you want to rename smap_policy_change() and use the new function, taking care of both issues, in both code paths. Jan
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index 3ad2ab0..cb69dd5 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -24,6 +24,7 @@ #include <xen/symbols.h> #include <xen/keyhandler.h> #include <xen/guest_access.h> +#include <asm/hvm/nestedhvm.h> #include <asm/io.h> #include <asm/msr.h> #include <asm/mpspec.h> @@ -992,10 +993,30 @@ bool_t update_secondary_system_time(struct vcpu *v, { XEN_GUEST_HANDLE(vcpu_time_info_t) user_u = v->arch.time_info_guest; smap_check_policy_t saved_policy; + bool nested_guest_mode = false; if ( guest_handle_is_null(user_u) ) return 1; + /* + * Must be before all following __copy_field_to_guest() and + * __copy_to_guest(). + * + * Otherwise, if 'v' is in the nested guest mode, paging_gva_to_gfn() called + * from __copy_field_to_guest() and __copy_to_guest() will treat the target + * address as L2 gva, and __copy_field_to_guest() and __copy_to_guest() will + * consequently copy runstate to L2 guest rather than L1 guest. + * + * Therefore, we clear the nested guest flag before __copy_field_to_guest() + * and __copy_to_guest(), and restore the flag after all guest copy. + */ + if ( nestedhvm_enabled(v->domain) ) + { + nested_guest_mode = nestedhvm_is_n2(v); + if ( nested_guest_mode ) + nestedhvm_vcpu_exit_guestmode(v); + } + saved_policy = smap_policy_change(v, SMAP_CHECK_ENABLED); /* 1. Update userspace version. */ @@ -1014,6 +1035,9 @@ bool_t update_secondary_system_time(struct vcpu *v, smap_policy_change(v, saved_policy); + if ( unlikely(nested_guest_mode) ) + nestedhvm_vcpu_enter_guestmode(v); + return 1; }
For a HVM domain, if a vcpu is in the nested guest mode, __copy_field_to_guest() and __copy_to_guest() used by update_secondary_system_time() will copy data to L2 guest rather than L1 guest. This commit temporally clears the nested guest flag before all __copy_field_to_guest() and __copy_to_guest() in update_secondary_system_time(), and restores the flag after those guest copy operations. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> Suggested-by: Jan Beulich <jbeulich@suse.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/time.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)