Re: [PATCH v2 07/30] KVM: Provide x86_emulate_ctxt callback to get current cpl

[Andre Przywara <andre.przywara@amd.com>]

Gleb,

what is the purpose of this patch? Is this a preparation for something 
upcoming? I don't see a reason to change this, in my eyes it is not a 
simplification.

Regards,
Andre.


Gleb Natapov wrote:
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> ---
>  arch/x86/include/asm/kvm_emulate.h |    1 +
>  arch/x86/kvm/emulate.c             |   15 ++++++++-------
>  arch/x86/kvm/x86.c                 |    6 ++++++
>  3 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
> index 0c5caa4..b048fd2 100644
> --- a/arch/x86/include/asm/kvm_emulate.h
> +++ b/arch/x86/include/asm/kvm_emulate.h
> @@ -110,6 +110,7 @@ struct x86_emulate_ops {
>  				struct kvm_vcpu *vcpu);
>  	ulong (*get_cr)(int cr, struct kvm_vcpu *vcpu);
>  	void (*set_cr)(int cr, ulong val, struct kvm_vcpu *vcpu);
> +	int (*cpl)(struct kvm_vcpu *vcpu);
>  };
>  
>  /* Type, address-of, and value of an instruction's operand. */
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 5e2fa61..8bd0557 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1257,7 +1257,7 @@ static int emulate_popf(struct x86_emulate_ctxt *ctxt,
>  	int rc;
>  	unsigned long val, change_mask;
>  	int iopl = (ctxt->eflags & X86_EFLAGS_IOPL) >> IOPL_SHIFT;
> -	int cpl = kvm_x86_ops->get_cpl(ctxt->vcpu);
> +	int cpl = ops->cpl(ctxt->vcpu);
>  
>  	rc = emulate_pop(ctxt, ops, &val, len);
>  	if (rc != X86EMUL_CONTINUE)
> @@ -1758,7 +1758,8 @@ emulate_sysexit(struct x86_emulate_ctxt *ctxt)
>  	return X86EMUL_CONTINUE;
>  }
>  
> -static bool emulator_bad_iopl(struct x86_emulate_ctxt *ctxt)
> +static bool emulator_bad_iopl(struct x86_emulate_ctxt *ctxt,
> +			      struct x86_emulate_ops *ops)
>  {
>  	int iopl;
>  	if (ctxt->mode == X86EMUL_MODE_REAL)
> @@ -1766,7 +1767,7 @@ static bool emulator_bad_iopl(struct x86_emulate_ctxt *ctxt)
>  	if (ctxt->mode == X86EMUL_MODE_VM86)
>  		return true;
>  	iopl = (ctxt->eflags & X86_EFLAGS_IOPL) >> IOPL_SHIFT;
> -	return kvm_x86_ops->get_cpl(ctxt->vcpu) > iopl;
> +	return ops->cpl(ctxt->vcpu) > iopl;
>  }
>  
>  static bool emulator_io_port_access_allowed(struct x86_emulate_ctxt *ctxt,
> @@ -1803,7 +1804,7 @@ static bool emulator_io_permited(struct x86_emulate_ctxt *ctxt,
>  				 struct x86_emulate_ops *ops,
>  				 u16 port, u16 len)
>  {
> -	if (emulator_bad_iopl(ctxt))
> +	if (emulator_bad_iopl(ctxt, ops))
>  		if (!emulator_io_port_access_allowed(ctxt, ops, port, len))
>  			return false;
>  	return true;
> @@ -1842,7 +1843,7 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
>  	}
>  
>  	/* Privileged instruction can be executed only in CPL=0 */
> -	if ((c->d & Priv) && kvm_x86_ops->get_cpl(ctxt->vcpu)) {
> +	if ((c->d & Priv) && ops->cpl(ctxt->vcpu)) {
>  		kvm_inject_gp(ctxt->vcpu, 0);
>  		goto done;
>  	}
> @@ -2378,7 +2379,7 @@ special_insn:
>  		c->dst.type = OP_NONE;	/* Disable writeback. */
>  		break;
>  	case 0xfa: /* cli */
> -		if (emulator_bad_iopl(ctxt))
> +		if (emulator_bad_iopl(ctxt, ops))
>  			kvm_inject_gp(ctxt->vcpu, 0);
>  		else {
>  			ctxt->eflags &= ~X86_EFLAGS_IF;
> @@ -2386,7 +2387,7 @@ special_insn:
>  		}
>  		break;
>  	case 0xfb: /* sti */
> -		if (emulator_bad_iopl(ctxt))
> +		if (emulator_bad_iopl(ctxt, ops))
>  			kvm_inject_gp(ctxt->vcpu, 0);
>  		else {
>  			toggle_interruptibility(ctxt, KVM_X86_SHADOW_INT_STI);
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b08f8a1..3f2a8d3 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3426,6 +3426,11 @@ static void emulator_set_cr(int cr, unsigned long val, struct kvm_vcpu *vcpu)
>  	}
>  }
>  
> +static int emulator_get_cpl(struct kvm_vcpu *vcpu)
> +{
> +	return kvm_x86_ops->get_cpl(vcpu);
> +}
> +
>  static struct x86_emulate_ops emulate_ops = {
>  	.read_std            = kvm_read_guest_virt_system,
>  	.fetch               = kvm_fetch_guest_virt,
> @@ -3434,6 +3439,7 @@ static struct x86_emulate_ops emulate_ops = {
>  	.cmpxchg_emulated    = emulator_cmpxchg_emulated,
>  	.get_cr              = emulator_get_cr,
>  	.set_cr              = emulator_set_cr,
> +	.cpl                 = emulator_get_cpl,
>  };
>  
>  static void cache_all_regs(struct kvm_vcpu *vcpu)


-- 
Andre Przywara
AMD-OSRC (Dresden)
Tel: x29712