All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-4.8] x86/emul: Move CPUID Faulting fault generation into the emulator
@ 2016-10-26 11:09 Andrew Cooper
  2016-10-26 12:26 ` Jan Beulich
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2016-10-26 11:09 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

In hindsight, this is a better position for it, as it avoids opencoding
hvmemul_inject_hw_exception() in hvmemul_cpuid(), and reduces the requirements
on other ops->cpuid() hooks wanting to implement cpuid faulting in the future.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/hvm/emulate.c             | 11 -----------
 xen/arch/x86/x86_emulate/x86_emulate.c | 10 +++++++++-
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 70c8d44..5b408f8 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1556,18 +1556,7 @@ static int hvmemul_cpuid(
      */
     if ( ctxt->opcode == X86EMUL_OPC(0x0f, 0xa2) &&
          hvm_check_cpuid_faulting(current) )
-    {
-        struct hvm_emulate_ctxt *hvmemul_ctxt =
-            container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
-
-        hvmemul_ctxt->exn_pending = 1;
-        hvmemul_ctxt->trap.vector = TRAP_gp_fault;
-        hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION;
-        hvmemul_ctxt->trap.error_code = 0;
-        hvmemul_ctxt->trap.insn_len = 0;
-
         return X86EMUL_EXCEPTION;
-    }
 
     hvm_funcs.cpuid_intercept(eax, ebx, ecx, edx);
     return X86EMUL_OKAY;
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index a1821d5..0f8d252 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -5011,8 +5011,16 @@ x86_emulate(
         unsigned int eax = _regs.eax, ebx = _regs.ebx;
         unsigned int ecx = _regs.ecx, edx = _regs.edx;
         fail_if(ops->cpuid == NULL);
-        if ( (rc = ops->cpuid(&eax, &ebx, &ecx, &edx, ctxt)) != 0 )
+        switch ( rc = ops->cpuid(&eax, &ebx, &ecx, &edx, ctxt) )
+        {
+        case X86EMUL_OKAY:
+            break;
+        case X86EMUL_EXCEPTION: /* CPUID Faulting active. */
+            generate_exception_if(true, EXC_GP, 0);
+            /* unreachable */
+        default:
             goto done;
+        }
         _regs.eax = eax; _regs.ebx = ebx;
         _regs.ecx = ecx; _regs.edx = edx;
         break;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH for-4.8] x86/emul: Move CPUID Faulting fault generation into the emulator
  2016-10-26 11:09 [PATCH for-4.8] x86/emul: Move CPUID Faulting fault generation into the emulator Andrew Cooper
@ 2016-10-26 12:26 ` Jan Beulich
  2016-10-26 12:58   ` [PATCH v2 " Andrew Cooper
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2016-10-26 12:26 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Wei Liu, Xen-devel

>>> On 26.10.16 at 13:09, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -5011,8 +5011,16 @@ x86_emulate(
>          unsigned int eax = _regs.eax, ebx = _regs.ebx;
>          unsigned int ecx = _regs.ecx, edx = _regs.edx;
>          fail_if(ops->cpuid == NULL);
> -        if ( (rc = ops->cpuid(&eax, &ebx, &ecx, &edx, ctxt)) != 0 )
> +        switch ( rc = ops->cpuid(&eax, &ebx, &ecx, &edx, ctxt) )
> +        {
> +        case X86EMUL_OKAY:
> +            break;
> +        case X86EMUL_EXCEPTION: /* CPUID Faulting active. */
> +            generate_exception_if(true, EXC_GP, 0);
> +            /* unreachable */
> +        default:
>              goto done;
> +        }

Since this makes the cpuid hook different from other hooks (where
a return of X86EMUL_EXCEPTION means an exception was raised,
not that the emulator should raise one), this should be
accompanied by a comment adjustment in x86_emulate.h. Also I
think this would end up easier to read as

        rc = ops->cpuid(&eax, &ebx, &ecx, &edx, ctxt);
        generate_exception_if(rc == X86EMUL_EXCEPTION, EXC_GP, 0);
        if ( rc != X86EMUL_OKAY )
             goto done;

And then I think we should eventually alter X86EMUL_EXCEPTION:
This should be a macro taking both exception type and error code as
arguments, at which point the raising of exceptions could be done
solely by the emulator.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 for-4.8] x86/emul: Move CPUID Faulting fault generation into the emulator
  2016-10-26 12:26 ` Jan Beulich
@ 2016-10-26 12:58   ` Andrew Cooper
  2016-10-26 13:05     ` Wei Liu
  2016-10-26 13:40     ` Jan Beulich
  0 siblings, 2 replies; 5+ messages in thread
From: Andrew Cooper @ 2016-10-26 12:58 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

In hindsight, this is a better position for it, as it avoids opencoding
hvmemul_inject_hw_exception() in hvmemul_cpuid(), and reduces the requirements
on other ops->cpuid() hooks wanting to implement cpuid faulting in the future.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>

v2:
 * Expand the cpuid() hook comment in x86_emulate.h
 * Adjust how the exception is generated
---
 xen/arch/x86/hvm/emulate.c             | 11 -----------
 xen/arch/x86/x86_emulate/x86_emulate.c |  5 ++++-
 xen/arch/x86/x86_emulate/x86_emulate.h |  7 ++++++-
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 70c8d44..5b408f8 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1556,18 +1556,7 @@ static int hvmemul_cpuid(
      */
     if ( ctxt->opcode == X86EMUL_OPC(0x0f, 0xa2) &&
          hvm_check_cpuid_faulting(current) )
-    {
-        struct hvm_emulate_ctxt *hvmemul_ctxt =
-            container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
-
-        hvmemul_ctxt->exn_pending = 1;
-        hvmemul_ctxt->trap.vector = TRAP_gp_fault;
-        hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION;
-        hvmemul_ctxt->trap.error_code = 0;
-        hvmemul_ctxt->trap.insn_len = 0;
-
         return X86EMUL_EXCEPTION;
-    }
 
     hvm_funcs.cpuid_intercept(eax, ebx, ecx, edx);
     return X86EMUL_OKAY;
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index a1821d5..2b087e5 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -5011,7 +5011,10 @@ x86_emulate(
         unsigned int eax = _regs.eax, ebx = _regs.ebx;
         unsigned int ecx = _regs.ecx, edx = _regs.edx;
         fail_if(ops->cpuid == NULL);
-        if ( (rc = ops->cpuid(&eax, &ebx, &ecx, &edx, ctxt)) != 0 )
+        rc = ops->cpuid(&eax, &ebx, &ecx, &edx, ctxt);
+        generate_exception_if(rc == X86EMUL_EXCEPTION,
+                              EXC_GP, 0); /* CPUID Faulting? */
+        if ( rc != X86EMUL_OKAY )
             goto done;
         _regs.eax = eax; _regs.ebx = ebx;
         _regs.ecx = ecx; _regs.edx = edx;
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index 641711e..fc8d234 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -352,7 +352,12 @@ struct x86_emulate_ops
     int (*wbinvd)(
         struct x86_emulate_ctxt *ctxt);
 
-    /* cpuid: Emulate CPUID via given set of EAX-EDX inputs/outputs. */
+    /*
+     * cpuid: Emulate CPUID via given set of EAX-EDX inputs/outputs.
+     *
+     * May return X86EMUL_EXCEPTION, which causes the emulator to inject
+     * #GP[0].  Used to implement CPUID faulting.
+     */
     int (*cpuid)(
         unsigned int *eax,
         unsigned int *ebx,
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 for-4.8] x86/emul: Move CPUID Faulting fault generation into the emulator
  2016-10-26 12:58   ` [PATCH v2 " Andrew Cooper
@ 2016-10-26 13:05     ` Wei Liu
  2016-10-26 13:40     ` Jan Beulich
  1 sibling, 0 replies; 5+ messages in thread
From: Wei Liu @ 2016-10-26 13:05 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Wei Liu, Jan Beulich, Xen-devel

On Wed, Oct 26, 2016 at 01:58:02PM +0100, Andrew Cooper wrote:
> In hindsight, this is a better position for it, as it avoids opencoding
> hvmemul_inject_hw_exception() in hvmemul_cpuid(), and reduces the requirements
> on other ops->cpuid() hooks wanting to implement cpuid faulting in the future.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

Subject to Jan's review / ack:

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> 
> v2:
>  * Expand the cpuid() hook comment in x86_emulate.h
>  * Adjust how the exception is generated
> ---
>  xen/arch/x86/hvm/emulate.c             | 11 -----------
>  xen/arch/x86/x86_emulate/x86_emulate.c |  5 ++++-
>  xen/arch/x86/x86_emulate/x86_emulate.h |  7 ++++++-
>  3 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 70c8d44..5b408f8 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1556,18 +1556,7 @@ static int hvmemul_cpuid(
>       */
>      if ( ctxt->opcode == X86EMUL_OPC(0x0f, 0xa2) &&
>           hvm_check_cpuid_faulting(current) )
> -    {
> -        struct hvm_emulate_ctxt *hvmemul_ctxt =
> -            container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> -
> -        hvmemul_ctxt->exn_pending = 1;
> -        hvmemul_ctxt->trap.vector = TRAP_gp_fault;
> -        hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION;
> -        hvmemul_ctxt->trap.error_code = 0;
> -        hvmemul_ctxt->trap.insn_len = 0;
> -
>          return X86EMUL_EXCEPTION;
> -    }
>  
>      hvm_funcs.cpuid_intercept(eax, ebx, ecx, edx);
>      return X86EMUL_OKAY;
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
> index a1821d5..2b087e5 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -5011,7 +5011,10 @@ x86_emulate(
>          unsigned int eax = _regs.eax, ebx = _regs.ebx;
>          unsigned int ecx = _regs.ecx, edx = _regs.edx;
>          fail_if(ops->cpuid == NULL);
> -        if ( (rc = ops->cpuid(&eax, &ebx, &ecx, &edx, ctxt)) != 0 )
> +        rc = ops->cpuid(&eax, &ebx, &ecx, &edx, ctxt);
> +        generate_exception_if(rc == X86EMUL_EXCEPTION,
> +                              EXC_GP, 0); /* CPUID Faulting? */
> +        if ( rc != X86EMUL_OKAY )
>              goto done;
>          _regs.eax = eax; _regs.ebx = ebx;
>          _regs.ecx = ecx; _regs.edx = edx;
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
> index 641711e..fc8d234 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -352,7 +352,12 @@ struct x86_emulate_ops
>      int (*wbinvd)(
>          struct x86_emulate_ctxt *ctxt);
>  
> -    /* cpuid: Emulate CPUID via given set of EAX-EDX inputs/outputs. */
> +    /*
> +     * cpuid: Emulate CPUID via given set of EAX-EDX inputs/outputs.
> +     *
> +     * May return X86EMUL_EXCEPTION, which causes the emulator to inject
> +     * #GP[0].  Used to implement CPUID faulting.
> +     */
>      int (*cpuid)(
>          unsigned int *eax,
>          unsigned int *ebx,
> -- 
> 2.1.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 for-4.8] x86/emul: Move CPUID Faulting fault generation into the emulator
  2016-10-26 12:58   ` [PATCH v2 " Andrew Cooper
  2016-10-26 13:05     ` Wei Liu
@ 2016-10-26 13:40     ` Jan Beulich
  1 sibling, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2016-10-26 13:40 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Wei Liu, Xen-devel

>>> On 26.10.16 at 14:58, <andrew.cooper3@citrix.com> wrote:
> In hindsight, this is a better position for it, as it avoids opencoding
> hvmemul_inject_hw_exception() in hvmemul_cpuid(), and reduces the 
> requirements
> on other ops->cpuid() hooks wanting to implement cpuid faulting in the future.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-10-26 13:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-26 11:09 [PATCH for-4.8] x86/emul: Move CPUID Faulting fault generation into the emulator Andrew Cooper
2016-10-26 12:26 ` Jan Beulich
2016-10-26 12:58   ` [PATCH v2 " Andrew Cooper
2016-10-26 13:05     ` Wei Liu
2016-10-26 13:40     ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.