>>> On 09.05.16 at 15:15, <andrew.coop...@citrix.com> wrote:
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2222,10 +2222,13 @@ static void svm_invlpga_intercept(
>
> static void svm_invlpg_intercept(unsigned long vaddr)
> {
> - struct vcpu *curr = current;
> HVMTRACE_LONG_2D(INVLPG, 0, TRC_PAR_LONG(vaddr));
> - paging_invlpg(curr, vaddr);
> - svm_asid_g_invlpg(curr, vaddr);
> + paging_invlpg(current, vaddr);
> +}
> +
> +static void svm_invlpg(unsigned long vaddr)
> +{
> + svm_asid_g_invlpg(current, vaddr);
Since paging_invlpg() already gets passed a struct vcpu *, having
it hand it to ->invlpg() would seem quite natural.
> @@ -2432,10 +2432,14 @@ static void vmx_dr_access(unsigned long
> exit_qualification,
>
> static void vmx_invlpg_intercept(unsigned long vaddr)
> {
> - struct vcpu *curr = current;
> HVMTRACE_LONG_2D(INVLPG, /*invlpga=*/ 0, TRC_PAR_LONG(vaddr));
> - if ( paging_invlpg(curr, vaddr) && cpu_has_vmx_vpid )
So the previous dependency on paging_invlpg()'s return value gets
moved into that function (making the call here conditional). While
that's fine for VMX, SVM previously didn't pay attention to that
return value, and hence you're altering behavior in a way not
spelled out in the description (and to be honest I'm also not 100%
certain that change is correct).
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -6478,6 +6478,19 @@ const unsigned long *__init
> get_platform_badpages(unsigned int *array_size)
> return bad_pages;
> }
>
> +bool_t paging_invlpg(struct vcpu *v, unsigned long va)
> +{
> + bool_t invl = paging_mode_external(v->domain)
> + ? is_canonical_address(va) : __addr_ok(va);
> +
> + if ( invl )
> + invl = paging_get_hostmode(v)->invlpg(v, va);
> + if ( invl && is_hvm_domain(v->domain) )
has_hvm_container_domain() (or paging_mode_external(),
implying the former)?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
