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

[xen master] x86/msr: Rework rdmsr_safe() using asm goto()



commit 16de294bed6e4817a7275fa11cf8d203c009a1f5
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Fri Apr 18 17:09:40 2025 +0100
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Fri Aug 22 23:34:07 2025 +0100

    x86/msr: Rework rdmsr_safe() using asm goto()
    
    ... on capable toolchains.
    
    This avoids needing to hold rc in a register across the RDMSR, and in most
    cases removes direct testing and branching based on rc, as the fault label 
can
    be rearranged to directly land on the out-of-line block.
    
    There is a subtle difference in behaviour.  The old behaviour would, on 
fault,
    still produce 0's and write to val.
    
    The new behaviour only writes val on success, and write_msr() is the only
    place where this matters.  Move temp out of switch() scope and initialise it
    to 0.
    
    While it is possible to retain the old behaviour, it's not great to do so
    because the compiler cannot optimise out the dead store when it's not to a
    local function variable, and Xen has a lot of msr infrastructure which 
passes
    a val pointer through the call-tree.  We have compilers with ASM_GOTO_OUTPUT
    in CI, and they do notice.
    
    Resolves: https://gitlab.com/xen-project/xen/-/work_items/217
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/include/asm/msr.h | 18 ++++++++++++++++++
 xen/arch/x86/pv/emul-priv-op.c |  3 +--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h
index 70e6796a45..901770555b 100644
--- a/xen/arch/x86/include/asm/msr.h
+++ b/xen/arch/x86/include/asm/msr.h
@@ -51,6 +51,23 @@ static inline void wrmsrns(uint32_t msr, uint64_t val)
 static inline int rdmsr_safe(unsigned int msr, uint64_t *val)
 {
     uint64_t lo, hi;
+
+#ifdef CONFIG_CC_HAS_ASM_GOTO_OUTPUT
+    asm_inline goto (
+        "1: rdmsr\n\t"
+        _ASM_EXTABLE(1b, %l[fault])
+        : "=a" (lo), "=d" (hi)
+        : "c" (msr)
+        :
+        : fault );
+
+    *val = lo | (hi << 32);
+
+    return 0;
+
+ fault:
+    return -EFAULT;
+#else
     int rc;
 
     asm_inline volatile (
@@ -68,6 +85,7 @@ static inline int rdmsr_safe(unsigned int msr, uint64_t *val)
     *val = lo | (hi << 32);
 
     return rc;
+#endif
 }
 
 /* wrmsr with exception handling */
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index f46aaf2a3b..225d4cff03 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -1028,6 +1028,7 @@ static int cf_check write_msr(
     struct vcpu *curr = current;
     const struct domain *currd = curr->domain;
     const struct cpu_policy *cp = currd->arch.cpu_policy;
+    uint64_t temp = 0;
     bool vpmu_msr = false;
     int ret;
 
@@ -1041,8 +1042,6 @@ static int cf_check write_msr(
 
     switch ( reg )
     {
-        uint64_t temp;
-
     case MSR_FS_BASE:
     case MSR_GS_BASE:
     case MSR_SHADOW_GS_BASE:
--
generated by git-patchbot for /home/xen/git/xen.git#master



 


Rackspace

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