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

2023-04-06 Thread Andrew Cooper
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 
> Reviewed-by: Jan Beulich 
>
> 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



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

2023-04-06 Thread Jan Beulich
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 

Reviewed-by: Jan Beulich 

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



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

2023-04-06 Thread Andrew Cooper
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 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
---
 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