Re: [PATCH 2/3] KVM: x86: Save&restore interrupt shadow mask

Marcelo Tosatti
2010-02-17T03:50:03Z

On Mon, Feb 15, 2010 at 10:45:42AM +0100, Jan Kiszka wrote:
> The interrupt shadow created by STI or MOV-SS-like operations is part of
> the VCPU state and must be preserved across migration. Transfer it in
> the spare padding field of kvm_vcpu_events.interrupt.
>
> As a side effect we now have to make vmx_set_interrupt_shadow robust
> against both shadow types being set. Give MOV SS a higher priority and
> skip STI in that case to avoid that VMX throws a fault on next entry.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> Documentation/kvm/api.txt | 11 ++++++++++-
> arch/x86/include/asm/kvm.h | 3 ++-
> arch/x86/kvm/vmx.c | 2 +-
> arch/x86/kvm/x86.c | 12 ++++++++++--
> include/linux/kvm.h | 1 +
> 5 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
> index c6416a3..8770b67 100644
> --- a/Documentation/kvm/api.txt
> +++ b/Documentation/kvm/api.txt
> @@ -656,6 +656,7 @@ struct kvm_clock_data {
> 4.29 KVM_GET_VCPU_EVENTS
>
> Capability: KVM_CAP_VCPU_EVENTS
> +Extended by: KVM_CAP_INTR_SHADOW
> Architectures: x86
> Type: vm ioctl
> Parameters: struct kvm_vcpu_event (out)
> @@ -676,7 +677,7 @@ struct kvm_vcpu_events {
> __u8 injected;
> __u8 nr;
> __u8 soft;
> - __u8 pad;
> + __u8 shadow;
> } interrupt;
> struct {
> __u8 injected;
> @@ -688,9 +689,13 @@ struct kvm_vcpu_events {
> __u32 flags;
> };
>
> +KVM_VCPUEVENT_VALID_SHADOW may be set in the flags field to signal that
> +interrupt.shadow contains a valid state. Otherwise, this field is undefined.
> +
> 4.30 KVM_SET_VCPU_EVENTS
>
> Capability: KVM_CAP_VCPU_EVENTS
> +Extended by: KVM_CAP_INTR_SHADOW
> Architectures: x86
> Type: vm ioctl
> Parameters: struct kvm_vcpu_event (in)
> @@ -709,6 +714,10 @@ current in-kernel state. The bits are:
> KVM_VCPUEVENT_VALID_NMI_PENDING - transfer nmi.pending to the kernel
> KVM_VCPUEVENT_VALID_SIPI_VECTOR - transfer sipi_vector
>
> +If KVM_CAP_INTR_SHADOW is available, KVM_VCPUEVENT_VALID_SHADOW can be set in
> +the flags field to signal that interrupt.shadow contains a valid state and
> +shall be written into the VCPU.
> +
>
> 5. The kvm_run structure
>
> diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
> index f46b79f..dc6cd24 100644
> --- a/arch/x86/include/asm/kvm.h
> +++ b/arch/x86/include/asm/kvm.h
> @@ -257,6 +257,7 @@ struct kvm_reinject_control {
> /* When set in flags, include corresponding fields on KVM_SET_VCPU_EVENTS */
> #define KVM_VCPUEVENT_VALID_NMI_PENDING 0x00000001
> #define KVM_VCPUEVENT_VALID_SIPI_VECTOR 0x00000002
> +#define KVM_VCPUEVENT_VALID_SHADOW 0x00000004
>
> /* for KVM_GET/SET_VCPU_EVENTS */
> struct kvm_vcpu_events {
> @@ -271,7 +272,7 @@ struct kvm_vcpu_events {
> __u8 injected;
> __u8 nr;
> __u8 soft;
> - __u8 pad;
> + __u8 shadow;
> } interrupt;
> struct {
> __u8 injected;
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index f82b072..0fa74d0 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -854,7 +854,7 @@ static void vmx_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask)
>
> if (mask & X86_SHADOW_INT_MOV_SS)
> interruptibility |= GUEST_INTR_STATE_MOV_SS;
> - if (mask & X86_SHADOW_INT_STI)
> + else if (mask & X86_SHADOW_INT_STI)
> interruptibility |= GUEST_INTR_STATE_STI;
>
> if ((interruptibility != interruptibility_old))
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 50d1d2a..60e6341 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2132,6 +2132,9 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
> vcpu->arch.interrupt.pending && !vcpu->arch.interrupt.soft;
> events->interrupt.nr = vcpu->arch.interrupt.nr;
> events->interrupt.soft = 0;
> + events->interrupt.shadow =
> + !!kvm_x86_ops->get_interrupt_shadow(vcpu,
> + X86_SHADOW_INT_MOV_SS | X86_SHADOW_INT_STI);
>
> events->nmi.injected = vcpu->arch.nmi_injected;
> events->nmi.pending = vcpu->arch.nmi_pending;
> @@ -2140,7 +2143,8 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
> events->sipi_vector = vcpu->arch.sipi_vector;
>
> events->flags = (KVM_VCPUEVENT_VALID_NMI_PENDING
> - | KVM_VCPUEVENT_VALID_SIPI_VECTOR);
> + | KVM_VCPUEVENT_VALID_SIPI_VECTOR
> + | KVM_VCPUEVENT_VALID_SHADOW);
>
> vcpu_put(vcpu);
> }
> @@ -2149,7 +2153,8 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
> struct kvm_vcpu_events *events)
> {
> if (events->flags & ~(KVM_VCPUEVENT_VALID_NMI_PENDING
> - | KVM_VCPUEVENT_VALID_SIPI_VECTOR))
> + | KVM_VCPUEVENT_VALID_SIPI_VECTOR
> + | KVM_VCPUEVENT_VALID_SHADOW))
> return -EINVAL;
>
> vcpu_load(vcpu);
> @@ -2164,6 +2169,9 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
> vcpu->arch.interrupt.soft = events->interrupt.soft;
> if (vcpu->arch.interrupt.pending && irqchip_in_kernel(vcpu->kvm))
> kvm_pic_clear_isr_ack(vcpu->kvm);
> + if (events->flags & KVM_VCPUEVENT_VALID_SHADOW)
> + kvm_x86_ops->set_interrupt_shadow(vcpu,
> + events->interrupt.shadow ? X86_SHADOW_INT_MOV_SS : 0);

Its hackish to transform blocking-by-sti into blocking-by-mov-ss (sti
does not block debug exceptions for the next instruction).

Any special reason you are doing this?

Also, as Avi mentioned it would be better to avoid this. Is it not
possible to disallow migration while interrupt shadow is present?