diff mbox series

[1/2] x86/hvm: Rearrange the logic in hvmemul_{read,write}_cr()

Message ID 20250325174109.267974-2-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series x86/emul: Emulate %cr8 accesses | expand

Commit Message

Andrew Cooper March 25, 2025, 5:41 p.m. UTC 
In hvmemul_read_cr(), make the TRACE()/X86EMUL_OKAY path common in preparation
for adding a %cr8 case.  Use a local 'val' variable instead of always
operating on a deferenced pointer.

In both, calculate curr once.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Petr Beneš <w1benny@gmail.com>

v2:
 * New
---
 xen/arch/x86/hvm/emulate.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Jan Beulich March 26, 2025, 11:16 a.m. UTC | #1 
On 25.03.2025 18:41, Andrew Cooper wrote:
> In hvmemul_read_cr(), make the TRACE()/X86EMUL_OKAY path common in preparation
> for adding a %cr8 case.  Use a local 'val' variable instead of always
> operating on a deferenced pointer.
> 
> In both, calculate curr once.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with two suggestions:

> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -2273,23 +2273,30 @@ static int cf_check hvmemul_write_io(
>  
>  static int cf_check hvmemul_read_cr(
>      unsigned int reg,
> -    unsigned long *val,
> +    unsigned long *_val,

Could I talk you into avoiding the leading underscore here, by using e.g.
"pval" or "valp"?

>      struct x86_emulate_ctxt *ctxt)
>  {
> +    struct vcpu *curr = current;

This can be pointer-to-const?

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index dbf6b5543adf..172ed55bd5e7 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -2273,23 +2273,30 @@  static int cf_check hvmemul_write_io(
 
 static int cf_check hvmemul_read_cr(
     unsigned int reg,
-    unsigned long *val,
+    unsigned long *_val,
     struct x86_emulate_ctxt *ctxt)
 {
+    struct vcpu *curr = current;
+    unsigned long val;
+
     switch ( reg )
     {
     case 0:
     case 2:
     case 3:
     case 4:
-        *val = current->arch.hvm.guest_cr[reg];
-        TRACE(TRC_HVM_CR_READ64, reg, *val, *val >> 32);
-        return X86EMUL_OKAY;
-    default:
+        val = curr->arch.hvm.guest_cr[reg];
         break;
+
+    default:
+        return X86EMUL_UNHANDLEABLE;
     }
 
-    return X86EMUL_UNHANDLEABLE;
+    TRACE(TRC_HVM_CR_READ64, reg, val, val >> 32);
+
+    *_val = val;
+
+    return X86EMUL_OKAY;
 }
 
 static int cf_check hvmemul_write_cr(
@@ -2297,6 +2304,7 @@  static int cf_check hvmemul_write_cr(
     unsigned long val,
     struct x86_emulate_ctxt *ctxt)
 {
+    struct vcpu *curr = current;
     int rc;
 
     TRACE(TRC_HVM_CR_WRITE64, reg, val, val >> 32);
@@ -2307,13 +2315,13 @@  static int cf_check hvmemul_write_cr(
         break;
 
     case 2:
-        current->arch.hvm.guest_cr[2] = val;
+        curr->arch.hvm.guest_cr[2] = val;
         rc = X86EMUL_OKAY;
         break;
 
     case 3:
     {
-        bool noflush = hvm_pcid_enabled(current) && (val & X86_CR3_NOFLUSH);
+        bool noflush = hvm_pcid_enabled(curr) && (val & X86_CR3_NOFLUSH);
 
         if ( noflush )
             val &= ~X86_CR3_NOFLUSH;