[PATCH] x86/svm: Provide EXITINFO decodes for Exceptions/NPT intercepts

Andrew Cooper posted 1 patch 1 year ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230406132708.2251000-1-andrew.cooper3@citrix.com
xen/arch/x86/hvm/svm/nestedsvm.c        |  9 +++------
xen/arch/x86/hvm/svm/svm.c              | 23 ++++++++++-------------
xen/arch/x86/include/asm/hvm/svm/vmcb.h | 10 ++++++++++
3 files changed, 23 insertions(+), 19 deletions(-)
[PATCH] x86/svm: Provide EXITINFO decodes for Exceptions/NPT intercepts
Posted by Andrew Cooper 1 year ago
Exceptions and NPT intercepts almost have the same layout, but NPT has bits
above 31 in the error code, and the name for exitinfo2 really does want
distinguishing between cr2 and gpa.

In nsvm_vcpu_vmexit_inject() rearrange VMEXIT_NPF to fall through instead of
repeating the exitinfo1 write.  Use the fallthrough pseudo keyword instead of
a comment.

In VMEXIT_NPF, as we're editing the printk() anyway, switch to using the newer
domain_crash() form.

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: Wei Liu <wl@xen.org>
---
 xen/arch/x86/hvm/svm/nestedsvm.c        |  9 +++------
 xen/arch/x86/hvm/svm/svm.c              | 23 ++++++++++-------------
 xen/arch/x86/include/asm/hvm/svm/vmcb.h | 10 ++++++++++
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 2003f28f66f4..51e437e73a20 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -824,14 +824,11 @@ nsvm_vcpu_vmexit_inject(struct vcpu *v, struct cpu_user_regs *regs,
                 ns_vmcb->exit_int_info = ns_vmcb->event_inj;
             break;
         case VMEXIT_EXCEPTION_PF:
-            ns_vmcb->_cr2 = ns_vmcb->exitinfo2;
-            /* fall through */
+            ns_vmcb->_cr2 = ns_vmcb->ei.exc.cr2;
+            fallthrough;
         case VMEXIT_NPF:
-            /* PF error code */
-            ns_vmcb->exitinfo1 = svm->ns_vmexit.exitinfo1;
-            /* fault address */
             ns_vmcb->exitinfo2 = svm->ns_vmexit.exitinfo2;
-            break;
+            fallthrough;
         case VMEXIT_EXCEPTION_NP:
         case VMEXIT_EXCEPTION_SS:
         case VMEXIT_EXCEPTION_GP:
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 1b32ef77b643..17359047b396 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2808,9 +2808,9 @@ void svm_vmexit_handler(void)
 
     case VMEXIT_EXCEPTION_PF:
     {
-        unsigned long va;
-        va = vmcb->exitinfo2;
-        regs->error_code = vmcb->exitinfo1;
+        unsigned long va = vmcb->ei.exc.cr2;
+
+        regs->error_code = vmcb->ei.exc.ec;
         HVM_DBG_LOG(DBG_LEVEL_VMMU,
                     "eax=%lx, ebx=%lx, ecx=%lx, edx=%lx, esi=%lx, edi=%lx",
                     regs->rax, regs->rbx, regs->rcx,
@@ -2838,7 +2838,7 @@ void svm_vmexit_handler(void)
 
     case VMEXIT_EXCEPTION_AC:
         HVMTRACE_1D(TRAP, X86_EXC_AC);
-        hvm_inject_hw_exception(X86_EXC_AC, vmcb->exitinfo1);
+        hvm_inject_hw_exception(X86_EXC_AC, vmcb->ei.exc.ec);
         break;
 
     case VMEXIT_EXCEPTION_UD:
@@ -3051,18 +3051,15 @@ void svm_vmexit_handler(void)
     case VMEXIT_NPF:
         if ( cpu_has_svm_decode )
             v->arch.hvm.svm.cached_insn_len = vmcb->guest_ins_len & 0xf;
-        rc = vmcb->exitinfo1 & PFEC_page_present
-             ? p2m_pt_handle_deferred_changes(vmcb->exitinfo2) : 0;
+        rc = vmcb->ei.npt.ec & PFEC_page_present
+             ? p2m_pt_handle_deferred_changes(vmcb->ei.npt.gpa) : 0;
         if ( rc == 0 )
             /* If no recal adjustments were being made - handle this fault */
-            svm_do_nested_pgfault(v, regs, vmcb->exitinfo1, vmcb->exitinfo2);
+            svm_do_nested_pgfault(v, regs, vmcb->ei.npt.ec, vmcb->ei.npt.gpa);
         else if ( rc < 0 )
-        {
-            printk(XENLOG_G_ERR
-                   "%pv: Error %d handling NPF (gpa=%08lx ec=%04lx)\n",
-                   v, rc, vmcb->exitinfo2, vmcb->exitinfo1);
-            domain_crash(v->domain);
-        }
+            domain_crash(v->domain,
+                         "%pv: Error %d handling NPF (gpa=%08lx ec=%04lx)\n",
+                         v, rc, vmcb->ei.npt.gpa, vmcb->ei.npt.ec);
         v->arch.hvm.svm.cached_insn_len = 0;
         break;
 
diff --git a/xen/arch/x86/include/asm/hvm/svm/vmcb.h b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
index 6cd1c4cfe4fa..05b3fb82bd12 100644
--- a/xen/arch/x86/include/asm/hvm/svm/vmcb.h
+++ b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
@@ -436,6 +436,12 @@ struct vmcb_struct {
             uint64_t exitinfo2; /* offset 0x80 */
         };
         union {
+            struct {
+                uint32_t ec; /* #NP, #SS, #GP, #PF, #AC */
+                uint32_t :32;
+
+                uint64_t cr2; /* #PF */
+            } exc;
             struct {
                 bool     in:1;
                 bool     :1;
@@ -455,6 +461,10 @@ struct vmcb_struct {
                 uint64_t :59;
                 bool     mov_insn:1; /* MOV, as opposed to LMSW, CLTS, etc */
             } mov_cr;
+            struct {
+                uint64_t ec;
+                uint64_t gpa;
+            } npt;
             struct {
                 uint16_t sel;
                 uint64_t :48;
-- 
2.30.2


Re: [PATCH] x86/svm: Provide EXITINFO decodes for Exceptions/NPT intercepts
Posted by Jan Beulich 1 year ago
On 06.04.2023 15:27, Andrew Cooper wrote:
> Exceptions and NPT intercepts almost have the same layout, but NPT has bits
> above 31 in the error code, and the name for exitinfo2 really does want
> distinguishing between cr2 and gpa.
> 
> In nsvm_vcpu_vmexit_inject() rearrange VMEXIT_NPF to fall through instead of
> repeating the exitinfo1 write.  Use the fallthrough pseudo keyword instead of
> a comment.
> 
> In VMEXIT_NPF, as we're editing the printk() anyway, switch to using the newer
> domain_crash() form.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

with one remark / suggestion:

> @@ -455,6 +461,10 @@ struct vmcb_struct {
>                  uint64_t :59;
>                  bool     mov_insn:1; /* MOV, as opposed to LMSW, CLTS, etc */
>              } mov_cr;
> +            struct {
> +                uint64_t ec;
> +                uint64_t gpa;
> +            } npt;

Maybe better "npf" than "npt", as it's describing the exit/fault?

Jan
Re: [PATCH] x86/svm: Provide EXITINFO decodes for Exceptions/NPT intercepts
Posted by Andrew Cooper 1 year ago
On 06/04/2023 3:28 pm, Jan Beulich wrote:
> On 06.04.2023 15:27, Andrew Cooper wrote:
>> Exceptions and NPT intercepts almost have the same layout, but NPT has bits
>> above 31 in the error code, and the name for exitinfo2 really does want
>> distinguishing between cr2 and gpa.
>>
>> In nsvm_vcpu_vmexit_inject() rearrange VMEXIT_NPF to fall through instead of
>> repeating the exitinfo1 write.  Use the fallthrough pseudo keyword instead of
>> a comment.
>>
>> In VMEXIT_NPF, as we're editing the printk() anyway, switch to using the newer
>> domain_crash() form.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> with one remark / suggestion:
>
>> @@ -455,6 +461,10 @@ struct vmcb_struct {
>>                  uint64_t :59;
>>                  bool     mov_insn:1; /* MOV, as opposed to LMSW, CLTS, etc */
>>              } mov_cr;
>> +            struct {
>> +                uint64_t ec;
>> +                uint64_t gpa;
>> +            } npt;
> Maybe better "npf" than "npt", as it's describing the exit/fault?

Oh yes, of course.  That is what I'd intended to put here.

Thanks.

~Andrew