diff mbox series

[v2,6/8] x86/pv: disallow access to unknown MSRs

Message ID 20200820150835.27440-7-roger.pau@citrix.com (mailing list archive)
State Superseded
Headers show
Series x86: switch default MSR behavior | expand

Commit Message

Roger Pau Monne Aug. 20, 2020, 3:08 p.m. UTC 
Change the catch-all behavior for MSR not explicitly handled. Instead
of allow full read-access to the MSR space and silently dropping
writes return an exception when the MSR is not explicitly handled.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/pv/emul-priv-op.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

Comments

Jan Beulich Aug. 28, 2020, 8:45 a.m. UTC | #1 
On 20.08.2020 17:08, Roger Pau Monne wrote:
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -972,9 +972,10 @@ static int read_msr(unsigned int reg, uint64_t *val,
>          }
>          /* fall through */
>      default:
> +        gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", reg);
> +        break;
> +
>      normal:
> -        /* Everyone can read the MSR space. */
> -        /* gdprintk(XENLOG_WARNING, "Domain attempted RDMSR %08x\n", reg); */
>          if ( rdmsr_safe(reg, *val) )
>              break;
>          return X86EMUL_OKAY;
> @@ -1141,14 +1142,15 @@ static int write_msr(unsigned int reg, uint64_t val,
>          }
>          /* fall through */
>      default:
> -        if ( rdmsr_safe(reg, temp) )
> -            break;
> +        gdprintk(XENLOG_WARNING,
> +                 "WRMSR 0x%08x val 0x%016"PRIx64" unimplemented\n",
> +                 reg, val);
> +        break;
>  
> -        if ( val != temp )
>      invalid:
> -            gdprintk(XENLOG_WARNING,
> -                     "Domain attempted WRMSR %08x from 0x%016"PRIx64" to 0x%016"PRIx64"\n",
> -                     reg, temp, val);
> +        gdprintk(XENLOG_WARNING,
> +                 "Domain attempted WRMSR %08x from 0x%016"PRIx64" to 0x%016"PRIx64"\n",
> +                 reg, temp, val);
>          return X86EMUL_OKAY;
>      }

There some odd mix of whether or no 0x prefixes get logged. I'd
advocate for dropping all of them, but imo at the very least all
three messages should be consistent in this regard.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index bcc1188f6a..d4735b4f06 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -972,9 +972,10 @@  static int read_msr(unsigned int reg, uint64_t *val,
         }
         /* fall through */
     default:
+        gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", reg);
+        break;
+
     normal:
-        /* Everyone can read the MSR space. */
-        /* gdprintk(XENLOG_WARNING, "Domain attempted RDMSR %08x\n", reg); */
         if ( rdmsr_safe(reg, *val) )
             break;
         return X86EMUL_OKAY;
@@ -1141,14 +1142,15 @@  static int write_msr(unsigned int reg, uint64_t val,
         }
         /* fall through */
     default:
-        if ( rdmsr_safe(reg, temp) )
-            break;
+        gdprintk(XENLOG_WARNING,
+                 "WRMSR 0x%08x val 0x%016"PRIx64" unimplemented\n",
+                 reg, val);
+        break;
 
-        if ( val != temp )
     invalid:
-            gdprintk(XENLOG_WARNING,
-                     "Domain attempted WRMSR %08x from 0x%016"PRIx64" to 0x%016"PRIx64"\n",
-                     reg, temp, val);
+        gdprintk(XENLOG_WARNING,
+                 "Domain attempted WRMSR %08x from 0x%016"PRIx64" to 0x%016"PRIx64"\n",
+                 reg, temp, val);
         return X86EMUL_OKAY;
     }