Re: [PATCH 2/2] add sysenter/syscall emulation for 32bit compat mode

[Avi Kivity <avi@redhat.com>]

Andre Przywara wrote:
> sysenter/sysexit are not supported on AMD's 32bit compat mode, whereas
> syscall is not supported on Intel's 32bit compat mode. To allow cross
> vendor migration we emulate the missing instructions by setting up the
> processor state according to the other call.
> The sysenter code was originally sketched by Amit Shah, it was completed,
> debugged,  syscall added and made-to-work by Christoph Egger and polished
> up by Andre Przywara.
> Please note that sysret does not need to be emulated, because it will be
> exectued in 64bit mode and returning to 32bit compat mode works on Intel.
>  		++vcpu->stat.insn_emulation;
>  		if (r)  {
> diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
> index 22c765d..41b78fa 100644
> --- a/arch/x86/kvm/x86_emulate.c
> +++ b/arch/x86/kvm/x86_emulate.c
> @@ -32,6 +32,8 @@
>  #include <linux/module.h>
>  #include <asm/kvm_x86_emulate.h>
>  
> +#include "mmu.h"
> +
>   

I think this is unneeded?

> @@ -1985,10 +1992,114 @@ twobyte_insn:
>  			goto cannot_emulate;
>  		}
>  		break;
> +	case 0x05: { /* syscall */
> +		unsigned long cr0 = ctxt->vcpu->arch.cr0;
> +		struct kvm_segment cs, ss;
> +
> +		memset(&cs, 0, sizeof(struct kvm_segment));
> +		memset(&ss, 0, sizeof(struct kvm_segment));
> +
> +		/* inject #UD if
> +		 * 1. we are in real mode
> +		 * 2. protected mode is not enabled
> +		 * 3. LOCK prefix is used
> +		 */
> +		if ((ctxt->mode == X86EMUL_MODE_REAL)
> +			|| (!(cr0 & X86_CR0_PE))
> +			|| (c->lock_prefix)) {
> +			/* we don't need to inject #UD here, because
> +			 * when emulate_instruction() returns something else
> +			 * than EMULATE_DONE, then svm.c:ud_interception()
> +			 * will do that for us.
> +			 */
> +			goto cannot_emulate;
>   

I prefer explicit injection, relying on the caller is tricky and may change.

> +	case 0x07:		/* sysret */
>   

Since we don't emulate sysret, it should be dropped here.

> +			cs.limit = 0xffffffff;
> +			ss.base = 0;
> +			ss.limit = 0xffffffff;
>   

Once is enough.


Please move the code out of the switch and into separate functions.

-- 
error compiling committee.c: too many arguments to function