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

Re: [PATCH v2 4/8] x86: Initial support for WRMSRNS


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 12 Jan 2023 13:58:05 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=D26GuqKYX6gPt9im9t3yBAAtCPZw3qa90PikQLoD9uQ=; b=H6U9yEeQFWjs+9MBoUe84yf70a98Q/oYuuEWdYR0HdzejNJVQYMJe+CRkyMJ8wOvEUohg99QRr1Mv0VOee61AhX1CRh12JANJ53FCDepBWkEcufXwwqda4//Q2GYrfDZd0oyGuMYv43Gd3+5vfRMIICY5MQInB0hJYWxrOm57yymKCteQUJT/e45ohmLBOwOiWJKAGU9uW64+6WKc//fAlvarSMuRYZGoq5CCELZNEH2akE9NboXzXbM9es+g+bKql3EZL+LfBw6uJ5Zv4C6La/vnsXN5Zfesr3O+w5gD/v2uGGSKRthjZMf/4VfhM+lZpsH0UlMB9s9pe+yzCDNfg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=oFMYx3rn7wtE7mWuxpi/794htkqQyjJ0vlySlU2qcRTcQJ4FxyNljbMp0E8GS//SQ/y5JAReoJk72WmrjI60I+ZSxVZOPp4lWCns5H2rac63B/kWIENwIMhiU2ghWne59gp+1IpJYS0878JbKnLP6Ba6bXwnXCesCtznfyVd65j3Ufy57lqpiefwaUEGUxnG+Qbn9CL9FE6AdKh6QcCoS3SbwQVnsc1b5v/MK+CJdnbxLTsv/HiqdXZbpASNT7WrNI66qlJqYdoK34sL952pL4LYjkmssevxidDcTuIm4beN9qQJ2AwUywzM9a9QM1l+DtMs9G5i4LvDijWLZFezHw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 12 Jan 2023 12:58:20 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 10.01.2023 18:18, Andrew Cooper wrote:
> WRMSR Non-Serialising is an optimisation intended for cases where an MSR needs
> updating, but architectural serialising properties are not needed.
> 
> In is anticipated that this will apply to most if not all MSRs modified on
> context switch paths.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

This will allow me to drop half of what the respective emulator patch
consists of, which I'm yet to post (but which in turn is sitting on
top of many other already posted emulator patches). Comparing with
that patch, one nit though:

> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -189,6 +189,7 @@ static const char *const str_7a1[32] =
>  
>      [10] = "fzrm",          [11] = "fsrs",
>      [12] = "fsrcs",
> +    /* 18 */                [19] = "wrmsrns",
>  };

We commonly leave a blank line to indicate dis-contiguous entries.

> --- a/xen/arch/x86/include/asm/msr.h
> +++ b/xen/arch/x86/include/asm/msr.h
> @@ -38,6 +38,18 @@ static inline void wrmsrl(unsigned int msr, __u64 val)
>          wrmsr(msr, lo, hi);
>  }
>  
> +/* Non-serialising WRMSR, when available.  Falls back to a serialising 
> WRMSR. */
> +static inline void wrmsr_ns(uint32_t msr, uint32_t lo, uint32_t hi)
> +{
> +    /*
> +     * WRMSR is 2 bytes.  WRMSRNS is 3 bytes.  Pad WRMSR with a redundant CS
> +     * prefix to avoid a trailing NOP.
> +     */
> +    alternative_input(".byte 0x2e; wrmsr",
> +                      ".byte 0x0f,0x01,0xc6", X86_FEATURE_WRMSRNS,
> +                      "c" (msr), "a" (lo), "d" (hi));
> +}

No wrmsrl_ns() and/or wrmsr_ns_safe() variants right away?

Do you have any indications towards a CS prefix being the least risky
one to use here (or in general)? Recognizing that segment prefixes have
gained alternative meaning in certain contexts, I would otherwise wonder
whether an address or operand size prefix wouldn't be more suitable.

Jan



 


Rackspace

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