[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 3/4] x86: Allow non-faulting accesses to non-emulated MSRs if policy permits this


  • To: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 18 Feb 2021 12:24:17 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=243NAMITV7mJMRT4NUohsQfhdOtzxg0uW+vW8Q3KYzE=; b=NVg5qKWA1eJbOYAHCCLHachEmpm7UUTioxDi5xfFfqB73x1+olHzI6zo3kQygIJEKKg5mq6IEDLYzI4o+1YyGPRGF4W+zERc72erCV+PcDhlAi/+9FLi5fB3NsP1nswAt0b7fDRj8tygpOtsn3Wz8w6f3EiVzHvm+kqXHiOfutOc7LLhMB58luDR3kCVOwoDGqY9PO5IUrIsEOd5U8tIxeyhfiUdB5LwS3Y01VBbSEI91nCzCtaTN/4boRu0xMbbs3Q7OK2Qrwu+cvhuzOeaORBgk70xikBgPGsoe7M3x6G2WR7Z/ZmCl8StL2oUlaWSTkPxSbSMjmdCYLF/59xmaw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fZgJRhg9EpoUgu4RfKAhXZ6Hrw/d2yXje6PP2NR/rOrYBr7yZmobeKlY2yYndxCVRQi6qLB8Mbvyzf4QdBqjxGHrIrYyM60fwMUk3k12zvw3w2uqhvAMN4j1E7Y+DNmmV20jC9eprqNDO+UYCO3bVVHVxvTF15/kJO3V436p3M3gU/BEvybs39lcujoEV0+lZoiAdi/8pnmUUh+IC5J6lKQinXir2CjfTw9CsiCwAUuiGlceNgVj48G19yeKyc6k8NUGW2GMqo9bEar3pZNCY56eP+l31Hp061QRNSZKxnoScFSEX9xglbobQV1Ax929S5mU1F9RKf2dQG0MFpEr6A==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: <xen-devel@xxxxxxxxxxxxxxxxxxxx>, <iwj@xxxxxxxxxxxxxx>, <wl@xxxxxxx>, <anthony.perard@xxxxxxxxxx>, <jbeulich@xxxxxxxx>, <andrew.cooper3@xxxxxxxxxx>, <jun.nakajima@xxxxxxxxx>, <kevin.tian@xxxxxxxxx>
  • Delivery-date: Thu, 18 Feb 2021 11:24:35 +0000
  • Ironport-sdr: h6fTCPB80k32DTNHQbdaQqGpFDsyJ1wYhcIPv7kcdGIFCwKgEogHXcHhU1hQtNZ+Y1f03qJkl2 JVmw8Wp3iyPkbCeW+Z34qZ2+/P9cx6NPXfDL8jQew1NSb9qv2rVro565KAGzT+5Dz38RjrOkYU eiGZmWp5r338EPXc27X9gZXeSzEj1YzHvqCF00t5h0toMO+o7IYuiP/KeEF++KrhOC2HhPscH9 uP5Oq3THpw4PyhARTPhXF6pt7vbthcfU5T3skgIhVF0/ovaioqYATpR+piv6d4MzdPcC3nxxLG B4U=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Jan 20, 2021 at 05:49:11PM -0500, Boris Ostrovsky wrote:
> Starting with commit 84e848fd7a16 ("x86/hvm: disallow access to unknown MSRs")
> accesses to unhandled MSRs result in #GP sent to the guest. This caused a
> regression for Solaris who tries to acccess MSR_RAPL_POWER_UNIT and (unlike,
> for example, Linux) does not catch exceptions when accessing MSRs that
> potentially may not be present.
> 
> Instead of special-casing RAPL registers we decide what to do when any
> non-emulated MSR is accessed based on ignore_msrs field of msr_policy.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> ---
> Changes in v2:
> * define x86_emul_guest_msr_access() and use it to determine whether emulated
>   instruction is rd/wrmsr.
> * Don't use ignore_msrs for MSR accesses that are not guest's rd/wrmsr.
> * Clear @val for writes too in guest_unhandled_msr()
> 
>  xen/arch/x86/hvm/svm/svm.c             | 10 ++++------
>  xen/arch/x86/hvm/vmx/vmx.c             | 10 ++++------
>  xen/arch/x86/msr.c                     | 28 ++++++++++++++++++++++++++++
>  xen/arch/x86/pv/emul-priv-op.c         | 10 ++++++----
>  xen/arch/x86/x86_emulate/x86_emulate.h |  6 ++++++
>  xen/include/asm-x86/msr.h              |  3 +++
>  6 files changed, 51 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index b819897a4a9f..7b59885b2619 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1965,8 +1965,8 @@ static int svm_msr_read_intercept(unsigned int msr, 
> uint64_t *msr_content)
>          break;
>  
>      default:
> -        gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", msr);
> -        goto gpf;
> +        if ( guest_unhandled_msr(v, msr, msr_content, false, true) )
> +            goto gpf;
>      }
>  
>      HVM_DBG_LOG(DBG_LEVEL_MSR, "returns: ecx=%x, msr_value=%"PRIx64,
> @@ -2151,10 +2151,8 @@ static int svm_msr_write_intercept(unsigned int msr, 
> uint64_t msr_content)
>          break;
>  
>      default:
> -        gdprintk(XENLOG_WARNING,
> -                 "WRMSR 0x%08x val 0x%016"PRIx64" unimplemented\n",
> -                 msr, msr_content);
> -        goto gpf;
> +        if ( guest_unhandled_msr(v, msr, &msr_content, true, true) )
> +            goto gpf;
>      }
>  
>      return X86EMUL_OKAY;
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 2d4475ee3de2..87baca57d33f 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3017,8 +3017,8 @@ static int vmx_msr_read_intercept(unsigned int msr, 
> uint64_t *msr_content)
>              break;
>          }
>  
> -        gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", msr);
> -        goto gp_fault;
> +        if ( guest_unhandled_msr(curr, msr, msr_content, false, true) )
> +            goto gp_fault;
>      }
>  
>  done:
> @@ -3319,10 +3319,8 @@ static int vmx_msr_write_intercept(unsigned int msr, 
> uint64_t msr_content)
>               is_last_branch_msr(msr) )
>              break;
>  
> -        gdprintk(XENLOG_WARNING,
> -                 "WRMSR 0x%08x val 0x%016"PRIx64" unimplemented\n",
> -                 msr, msr_content);
> -        goto gp_fault;
> +        if ( guest_unhandled_msr(v, msr, &msr_content, true, true) )
> +            goto gp_fault;
>      }

I think this could be done in hvm_msr_read_intercept instead of having
to call guest_unhandled_msr from each vendor specific handler?

Oh, I see, that's likely done to differentiate between guest MSR
accesses and emulator ones? I'm not sure we really need to make a
difference between guests MSR accesses and emulator ones, surely in
the past they would be treated equally?

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.