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

RE: [PATCH 1/5] x86/asm: Rename FS/GS base helpers


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: "Tian, Kevin" <kevin.tian@xxxxxxxxx>
  • Date: Tue, 15 Sep 2020 02:50:39 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.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=QaFKVSxcGH4AloHmvyWJ9EWJtZxA5/6bEq1bzmj5hnI=; b=NeDrL1nFJKiv0q77+t81pdaqJ8ys3kOYuAarbTOxcn84R86zUIC5iKOZoVouWfFqoHTb5KQhXKYi54vPtKAldZphyQeQ2LN5NydwZPVH9Ir8Tq/O1oHGp6EA/UUQH0JmxzjBGv9p3jSHatdQgxWxj9hO7siTYCs88pQByQHrcCypgueO0/3FffqhZ6/V0/0QYJVIo+QWYr+fM/YGMsKjF/PK9BzBhOEvbr5Pwvdhka1VftGbjuzgppxF295kGKoP2JdAsX2QHWoCMkNihAU3NrbKFG6ZCkFNErQSWn5kAIlLKDG4cGjdFCfXM+8+tFzEs1o/fFEP4Vn9lOkJNqKhkQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gHPafHqSLHCDHn7A35vDJSrpciCRZ5TATU4pGAmJCz7Z+H8U/FgfY963pCj/5sUE8OV9A5jE3M/Eh6/izXlYpk5cOM6r2flO6oe1uOhMky3V+ecJLMkEAhv7DU7Z1fUI69qDfX+9XeXxeFa5xhzfEwkJlm4druRx6uWqBZV30S0b4cr+4EqxOzpW4KtSIHMiifH7eXWXIYwyCf0yRh8lSB6bABlmYhws7P7vqARuf4E6d4UpT4IDhfLi6pDT05rRfdbmWOjxzkZz+ZmmJrAUITRqwnxHabCpiNfyC40zx/UoxSVMzwRbu91HcQEjr5z6218dBwAZspYhLmqMgo8Fvg==
  • Authentication-results: citrix.com; dkim=none (message not signed) header.d=none;citrix.com; dmarc=none action=none header.from=intel.com;
  • Cc: Jan Beulich <JBeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, "Nakajima, Jun" <jun.nakajima@xxxxxxxxx>
  • Delivery-date: Tue, 15 Sep 2020 02:50:51 +0000
  • Dlp-product: dlpe-windows
  • Dlp-reaction: no-action
  • Dlp-version: 11.5.1.3
  • Ironport-sdr: VLXT7lEbMzK4f7W5d3fKRT8+Pr3gm6I/Z6s/Kym/Xp1IPfYQMSaJUtYYIdRCcSHTYIsn30hdVc dYjxWd1vpdAQ==
  • Ironport-sdr: ebgO7U2Q2caSzDDaDQ5LdkFguloA16JDYYx3HB3PUqnnnCRK3pcptax8iYPhkoIImInLlapt0V vtAqw+R9mVjQ==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHWho/3RQMV/6pO4EqtH9V6HxfzYqlpCMLg
  • Thread-topic: [PATCH 1/5] x86/asm: Rename FS/GS base helpers

> From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Sent: Wednesday, September 9, 2020 5:59 PM
> 
> They are currently named after the FSGSBASE instructions, but are not thin
> wrappers around said instructions, and therefore do not accurately reflect
> the
> logic they perform, especially when it comes to functioning safely in non
> FSGSBASE context.
> 
> Rename them to {read,write}_{fs,gs}_base() to avoid confusion.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>

> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> CC: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
>  xen/arch/x86/domain.c          | 10 +++++-----
>  xen/arch/x86/hvm/vmx/vmx.c     |  8 ++++----
>  xen/arch/x86/pv/domain.c       |  2 +-
>  xen/arch/x86/pv/emul-priv-op.c | 14 +++++++-------
>  xen/arch/x86/x86_64/mm.c       |  8 ++++----
>  xen/arch/x86/x86_64/traps.c    |  6 +++---
>  xen/include/asm-x86/msr.h      | 12 ++++++------
>  7 files changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index e8e91cf080..2271bee36a 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1581,9 +1581,9 @@ static void load_segments(struct vcpu *n)
> 
>      if ( !fs_gs_done && !compat )
>      {
> -        wrfsbase(n->arch.pv.fs_base);
> -        wrgsshadow(n->arch.pv.gs_base_kernel);
> -        wrgsbase(n->arch.pv.gs_base_user);
> +        write_fs_base(n->arch.pv.fs_base);
> +        write_gs_shadow(n->arch.pv.gs_base_kernel);
> +        write_gs_base(n->arch.pv.gs_base_user);
> 
>          /* If in kernel mode then switch the GS bases around. */
>          if ( (n->arch.flags & TF_kernel_mode) )
> @@ -1710,9 +1710,9 @@ static void save_segments(struct vcpu *v)
> 
>      if ( !is_pv_32bit_vcpu(v) )
>      {
> -        unsigned long gs_base = rdgsbase();
> +        unsigned long gs_base = read_gs_base();
> 
> -        v->arch.pv.fs_base = rdfsbase();
> +        v->arch.pv.fs_base = read_fs_base();
>          if ( v->arch.flags & TF_kernel_mode )
>              v->arch.pv.gs_base_kernel = gs_base;
>          else
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index c4b40bf3cb..d26e102970 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -512,12 +512,12 @@ static void vmx_save_guest_msrs(struct vcpu *v)
>       * We cannot cache SHADOW_GS_BASE while the VCPU runs, as it can
>       * be updated at any time via SWAPGS, which we cannot trap.
>       */
> -    v->arch.hvm.vmx.shadow_gs = rdgsshadow();
> +    v->arch.hvm.vmx.shadow_gs = read_gs_shadow();
>  }
> 
>  static void vmx_restore_guest_msrs(struct vcpu *v)
>  {
> -    wrgsshadow(v->arch.hvm.vmx.shadow_gs);
> +    write_gs_shadow(v->arch.hvm.vmx.shadow_gs);
>      wrmsrl(MSR_STAR,           v->arch.hvm.vmx.star);
>      wrmsrl(MSR_LSTAR,          v->arch.hvm.vmx.lstar);
>      wrmsrl(MSR_SYSCALL_MASK,   v->arch.hvm.vmx.sfmask);
> @@ -2958,7 +2958,7 @@ static int vmx_msr_read_intercept(unsigned int
> msr, uint64_t *msr_content)
>          break;
> 
>      case MSR_SHADOW_GS_BASE:
> -        *msr_content = rdgsshadow();
> +        *msr_content = read_gs_shadow();
>          break;
> 
>      case MSR_STAR:
> @@ -3190,7 +3190,7 @@ static int vmx_msr_write_intercept(unsigned int
> msr, uint64_t msr_content)
>          else if ( msr == MSR_GS_BASE )
>              __vmwrite(GUEST_GS_BASE, msr_content);
>          else
> -            wrgsshadow(msr_content);
> +            write_gs_shadow(msr_content);
> 
>          break;
> 
> diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
> index 44e4ea2582..663e76c773 100644
> --- a/xen/arch/x86/pv/domain.c
> +++ b/xen/arch/x86/pv/domain.c
> @@ -452,7 +452,7 @@ void toggle_guest_mode(struct vcpu *v)
>       * Update the cached value of the GS base about to become inactive, as a
>       * subsequent context switch won't bother re-reading it.
>       */
> -    gs_base = rdgsbase();
> +    gs_base = read_gs_base();
>      if ( v->arch.flags & TF_kernel_mode )
>          v->arch.pv.gs_base_kernel = gs_base;
>      else
> diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-
> op.c
> index a192160f84..9dd1d59423 100644
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -511,10 +511,10 @@ static int read_segment(enum x86_segment seg,
>              reg->base = 0;
>              break;
>          case x86_seg_fs:
> -            reg->base = rdfsbase();
> +            reg->base = read_fs_base();
>              break;
>          case x86_seg_gs:
> -            reg->base = rdgsbase();
> +            reg->base = read_gs_base();
>              break;
>          }
> 
> @@ -871,13 +871,13 @@ static int read_msr(unsigned int reg, uint64_t *val,
>      case MSR_FS_BASE:
>          if ( is_pv_32bit_domain(currd) )
>              break;
> -        *val = rdfsbase();
> +        *val = read_fs_base();
>          return X86EMUL_OKAY;
> 
>      case MSR_GS_BASE:
>          if ( is_pv_32bit_domain(currd) )
>              break;
> -        *val = rdgsbase();
> +        *val = read_gs_base();
>          return X86EMUL_OKAY;
> 
>      case MSR_SHADOW_GS_BASE:
> @@ -993,19 +993,19 @@ static int write_msr(unsigned int reg, uint64_t val,
>      case MSR_FS_BASE:
>          if ( is_pv_32bit_domain(currd) || !is_canonical_address(val) )
>              break;
> -        wrfsbase(val);
> +        write_fs_base(val);
>          return X86EMUL_OKAY;
> 
>      case MSR_GS_BASE:
>          if ( is_pv_32bit_domain(currd) || !is_canonical_address(val) )
>              break;
> -        wrgsbase(val);
> +        write_gs_base(val);
>          return X86EMUL_OKAY;
> 
>      case MSR_SHADOW_GS_BASE:
>          if ( is_pv_32bit_domain(currd) || !is_canonical_address(val) )
>              break;
> -        wrgsshadow(val);
> +        write_gs_shadow(val);
>          curr->arch.pv.gs_base_user = val;
>          return X86EMUL_OKAY;
> 
> diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
> index b69cf2dc4f..0d11a9f500 100644
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -1030,7 +1030,7 @@ long do_set_segment_base(unsigned int which,
> unsigned long base)
>      {
>      case SEGBASE_FS:
>          if ( is_canonical_address(base) )
> -            wrfsbase(base);
> +            write_fs_base(base);
>          else
>              ret = -EINVAL;
>          break;
> @@ -1038,7 +1038,7 @@ long do_set_segment_base(unsigned int which,
> unsigned long base)
>      case SEGBASE_GS_USER:
>          if ( is_canonical_address(base) )
>          {
> -            wrgsshadow(base);
> +            write_gs_shadow(base);
>              v->arch.pv.gs_base_user = base;
>          }
>          else
> @@ -1047,7 +1047,7 @@ long do_set_segment_base(unsigned int which,
> unsigned long base)
> 
>      case SEGBASE_GS_KERNEL:
>          if ( is_canonical_address(base) )
> -            wrgsbase(base);
> +            write_gs_base(base);
>          else
>              ret = -EINVAL;
>          break;
> @@ -1096,7 +1096,7 @@ long do_set_segment_base(unsigned int which,
> unsigned long base)
>                         : [flat] "r" (FLAT_USER_DS32) );
> 
>          /* Update the cache of the inactive base, as read from the GDT/LDT. 
> */
> -        v->arch.pv.gs_base_user = rdgsbase();
> +        v->arch.pv.gs_base_user = read_gs_base();
> 
>          asm volatile ( safe_swapgs );
>          break;
> diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
> index 93af0c5e87..4f262122b7 100644
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -47,9 +47,9 @@ static void read_registers(struct cpu_user_regs *regs,
> unsigned long crs[8])
>      regs->es = read_sreg(es);
>      regs->fs = read_sreg(fs);
>      regs->gs = read_sreg(gs);
> -    crs[5] = rdfsbase();
> -    crs[6] = rdgsbase();
> -    crs[7] = rdgsshadow();
> +    crs[5] = read_fs_base();
> +    crs[6] = read_gs_base();
> +    crs[7] = read_gs_shadow();
>  }
> 
>  static void _show_registers(
> diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h
> index 5c44c79600..5e141ac5a5 100644
> --- a/xen/include/asm-x86/msr.h
> +++ b/xen/include/asm-x86/msr.h
> @@ -156,7 +156,7 @@ static inline unsigned long __rdgsbase(void)
>      return base;
>  }
> 
> -static inline unsigned long rdfsbase(void)
> +static inline unsigned long read_fs_base(void)
>  {
>      unsigned long base;
> 
> @@ -168,7 +168,7 @@ static inline unsigned long rdfsbase(void)
>      return base;
>  }
> 
> -static inline unsigned long rdgsbase(void)
> +static inline unsigned long read_gs_base(void)
>  {
>      unsigned long base;
> 
> @@ -180,7 +180,7 @@ static inline unsigned long rdgsbase(void)
>      return base;
>  }
> 
> -static inline unsigned long rdgsshadow(void)
> +static inline unsigned long read_gs_shadow(void)
>  {
>      unsigned long base;
> 
> @@ -196,7 +196,7 @@ static inline unsigned long rdgsshadow(void)
>      return base;
>  }
> 
> -static inline void wrfsbase(unsigned long base)
> +static inline void write_fs_base(unsigned long base)
>  {
>      if ( read_cr4() & X86_CR4_FSGSBASE )
>  #ifdef HAVE_AS_FSGSBASE
> @@ -208,7 +208,7 @@ static inline void wrfsbase(unsigned long base)
>          wrmsrl(MSR_FS_BASE, base);
>  }
> 
> -static inline void wrgsbase(unsigned long base)
> +static inline void write_gs_base(unsigned long base)
>  {
>      if ( read_cr4() & X86_CR4_FSGSBASE )
>  #ifdef HAVE_AS_FSGSBASE
> @@ -220,7 +220,7 @@ static inline void wrgsbase(unsigned long base)
>          wrmsrl(MSR_GS_BASE, base);
>  }
> 
> -static inline void wrgsshadow(unsigned long base)
> +static inline void write_gs_shadow(unsigned long base)
>  {
>      if ( read_cr4() & X86_CR4_FSGSBASE )
>      {
> --
> 2.11.0


 


Rackspace

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