lkml.org 
[lkml]   [2009]   [Apr]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] deal with interrupt shadow state for emulated instruction
Glauber Costa wrote:
> we currently unblock shadow interrupt state when we skip an instruction,
> but failing to do so when we actually emulate one. This blocks interrupts
> in key instruction blocks, in particular sti; hlt; sequences
>
> If the instruction emulated is an sti, we have to block shadow interrupts.
> The same goes for mov ss. pop ss also needs it, but we don't currently
> emulate it. For sequences of two or more instructions of the same type
> among those instructions, only the first one has this effect.
>
> Without this patch, I cannot boot gpxe option roms at vmx machines.
> This is described at https://bugzilla.redhat.com/show_bug.cgi?id=494469
>
>

We'll defer this until after Gleb's patchset, since that's much bigger.

> +#define X86_SHADOW_INT_MOV_SS 1
> +#define X86_SHADOW_INT_STI 2
> +
> struct x86_emulate_ctxt {
> /* Register state before/after emulation. */
> struct kvm_vcpu *vcpu;
> @@ -152,6 +155,10 @@ struct x86_emulate_ctxt {
> int mode;
> u32 cs_base;
>
> + /* interruptibility state, as a result of execution of STI or MOV SS */
> + int interruptibility;
> + int movss_int_flag, movss_int_flag_old;
> +
>

bit masks are traditionally unsigned.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0bb4131..b1fc8b6 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2364,7 +2364,7 @@ int emulate_instruction(struct kvm_vcpu *vcpu,
> u16 error_code,
> int emulation_type)
> {
> - int r;
> + int r, shadow_mask;
> struct decode_cache *c;
>
> kvm_clear_exception_queue(vcpu);
> @@ -2412,8 +2412,12 @@ int emulate_instruction(struct kvm_vcpu *vcpu,
> }
> }
>
> + vcpu->arch.emulate_ctxt.interruptibility = 0;
> r = x86_emulate_insn(&vcpu->arch.emulate_ctxt, &emulate_ops);
>
> + shadow_mask = vcpu->arch.emulate_ctxt.interruptibility;
> + kvm_x86_ops->interrupt_shadow_mask(vcpu, shadow_mask);
> +
>

Emulation may have failed, in which case you don't want to update the
interrupt shadow mask.

> if (vcpu->arch.pio.string)
> return EMULATE_DO_MMIO;
>
> diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
> index d7c9f6f..1369a2e 100644
> --- a/arch/x86/kvm/x86_emulate.c
> +++ b/arch/x86/kvm/x86_emulate.c
> @@ -1360,6 +1360,10 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
> int io_dir_in;
> int rc = 0;
>
> + ctxt->movss_int_flag_old = ctxt->movss_int_flag;
> +
> + ctxt->movss_int_flag = 0;
>

This seem to be internal to the emulator. However, instructions may be
executed outside the emulator, invalidating movss_int_flag. But see below.

> @@ -1610,6 +1614,14 @@ special_insn:
>
> sel = c->src.val;
> if (c->modrm_reg <= 5) {
> + if (c->modrm_reg == VCPU_SREG_SS) {
> + if (ctxt->movss_int_flag_old)
> + ctxt->interruptibility |=
> + X86_SHADOW_INT_MOV_SS;
> + else
> + ctxt->movss_int_flag = 1;
> + }
>

The comment about repeating 'mov ss' in the manual has that wonderful
word in it, May. That means we're perfectly allowed to ignore it and
just set the flag unconditionally.

I doubt we'll ever see a repeated 'mov ss', once is more than enough.



--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.



\
 
 \ /
  Last update: 2009-04-14 11:37    [W:0.090 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site