Re: [PATCH 4/9] Implement GIF, clgi and stgi v3
Alexander Graf wrote: Hmm yes, this is a problem. So this optimization will not work. We need other ways to optimize :) Well it would work for the KVM-in-KVM case, where we know that VMRUN is always triggered with IF=1 and V_INTR=1. The only case that hack fails is when we have IF=0 and V_INTR=1. Everything else should work just fine. And in this case we would simply issue some VMEXITs 0x60, so no big deal IMHO. It should be worth the tradeoff of making most VMMs a lot faster. There should be a compile-option to enable the "correct" behavior though. If we join that with the VMLOAD and VMSAVE hack there would be only the VMRUN and DR exits left. That sounds like a really good improvement where I wouldn't mind to break some specs :-). Maybe a hypercall, so it can be enabled on a guest-by-guest basis. I must say that if we do guest-specific hacking this way, a paravirt approach doesn't look so bad. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/9] Implement GIF, clgi and stgi v3
Joerg Roedel wrote: I had another possible idea for performance improvement here. Since we only inject normal interrupts and exceptions (and not NMI and such) we can patch clgi to cli and stgi to sti to save these two intercepts in the guests vmrun path. Any objections/problems with this? The sequence 'clgi; sti' will break. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/9] Implement GIF, clgi and stgi v3
On 25.09.2008, at 23:27, Joerg Roedel wrote: On Thu, Sep 25, 2008 at 09:55:27PM +0200, Alexander Graf wrote: On 25.09.2008, at 20:47, Joerg Roedel wrote: I had another possible idea for performance improvement here. Since we only inject normal interrupts and exceptions (and not NMI and such) we can patch clgi to cli and stgi to sti to save these two intercepts in the guests vmrun path. Any objections/problems with this? How do we know if we're allowed to inject interrupts with V_INTR set? Usually IF is on and GIF is off when entering the VM in KVM, so we allow interrupts to arrive even when IF is clear in the guest... Hmm yes, this is a problem. So this optimization will not work. We need other ways to optimize :) Well it would work for the KVM-in-KVM case, where we know that VMRUN is always triggered with IF=1 and V_INTR=1. The only case that hack fails is when we have IF=0 and V_INTR=1. Everything else should work just fine. And in this case we would simply issue some VMEXITs 0x60, so no big deal IMHO. It should be worth the tradeoff of making most VMMs a lot faster. There should be a compile-option to enable the "correct" behavior though. If we join that with the VMLOAD and VMSAVE hack there would be only the VMRUN and DR exits left. That sounds like a really good improvement where I wouldn't mind to break some specs :-). Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/9] Implement GIF, clgi and stgi v3
On Thu, Sep 25, 2008 at 09:55:27PM +0200, Alexander Graf wrote: > > On 25.09.2008, at 20:47, Joerg Roedel wrote: > > >I had another possible idea for performance improvement here. Since we > >only inject normal interrupts and exceptions (and not NMI and such) we > >can patch clgi to cli and stgi to sti to save these two intercepts in > >the guests vmrun path. > >Any objections/problems with this? > > How do we know if we're allowed to inject interrupts with V_INTR set? > Usually IF is on and GIF is off when entering the VM in KVM, so we > allow interrupts to arrive even when IF is clear in the guest... Hmm yes, this is a problem. So this optimization will not work. We need other ways to optimize :) Joerg -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/9] Implement GIF, clgi and stgi v3
On 25.09.2008, at 20:47, Joerg Roedel wrote:
I had another possible idea for performance improvement here. Since we
only inject normal interrupts and exceptions (and not NMI and such) we
can patch clgi to cli and stgi to sti to save these two intercepts in
the guests vmrun path.
Any objections/problems with this?
How do we know if we're allowed to inject interrupts with V_INTR set?
Usually IF is on and GIF is off when entering the VM in KVM, so we
allow interrupts to arrive even when IF is clear in the guest...
On Wed, Sep 17, 2008 at 03:41:21PM +0200, Alexander Graf wrote:
This patch implements the GIF flag and the clgi and stgi
instructions that
set this flag. Only if the flag is set (default), interrupts can be
received by
the CPU.
To keep the information about that somewhere, this patch adds a new
hidden
flags vector. that is used to store information that does not go
into the
vmcb, but is SVM specific.
v2 moves the hflags to x86 generic code
v3 makes use of the new permission helper
Signed-off-by: Alexander Graf <[EMAIL PROTECTED]>
---
arch/x86/kvm/svm.c | 42 ++
+---
include/asm-x86/kvm_host.h |3 +++
2 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index c72e728..469ecc5 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -612,6 +612,8 @@ static void init_vmcb(struct vcpu_svm *svm)
save->cr4 = 0;
}
force_new_asid(&svm->vcpu);
+
+ svm->vcpu.arch.hflags = HF_GIF_MASK;
}
static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
@@ -1227,6 +1229,36 @@ static int nested_svm_do(struct vcpu_svm *svm,
return retval;
}
+static int stgi_interception(struct vcpu_svm *svm, struct kvm_run
*kvm_run)
+{
+ if (nested_svm_check_permissions(svm))
+ return 1;
+
+ svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
+ skip_emulated_instruction(&svm->vcpu);
+
+ svm->vcpu.arch.hflags |= HF_GIF_MASK;
+
+ return 1;
+}
+
+static int clgi_interception(struct vcpu_svm *svm, struct kvm_run
*kvm_run)
+{
+ if (nested_svm_check_permissions(svm))
+ return 1;
+
+ svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
+ skip_emulated_instruction(&svm->vcpu);
+
+ svm->vcpu.arch.hflags &= ~HF_GIF_MASK;
+
+ /* After a CLGI no interrupts should come */
+ svm_clear_vintr(svm);
+ svm->vmcb->control.int_ctl &= ~V_IRQ_MASK;
+
+ return 1;
+}
+
static int invalid_op_interception(struct vcpu_svm *svm,
struct kvm_run *kvm_run)
{
@@ -1521,8 +1553,8 @@ static int (*svm_exit_handlers[])(struct
vcpu_svm *svm,
[SVM_EXIT_VMMCALL] = vmmcall_interception,
[SVM_EXIT_VMLOAD] = invalid_op_interception,
[SVM_EXIT_VMSAVE] = invalid_op_interception,
- [SVM_EXIT_STGI] = invalid_op_interception,
- [SVM_EXIT_CLGI] = invalid_op_interception,
+ [SVM_EXIT_STGI] = stgi_interception,
+ [SVM_EXIT_CLGI] = clgi_interception,
[SVM_EXIT_SKINIT] = invalid_op_interception,
[SVM_EXIT_WBINVD] = emulate_on_interception,
[SVM_EXIT_MONITOR] = invalid_op_interception,
@@ -1669,6 +1701,9 @@ static void svm_intr_assist(struct kvm_vcpu
*vcpu)
if (!kvm_cpu_has_interrupt(vcpu))
goto out;
+ if (!(svm->vcpu.arch.hflags & HF_GIF_MASK))
+ goto out;
+
if (!(vmcb->save.rflags & X86_EFLAGS_IF) ||
(vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) ||
(vmcb->control.event_inj & SVM_EVTINJ_VALID)) {
@@ -1720,7 +1755,8 @@ static void do_interrupt_requests(struct
kvm_vcpu *vcpu,
svm->vcpu.arch.interrupt_window_open =
(!(control->int_state & SVM_INTERRUPT_SHADOW_MASK) &&
-(svm->vmcb->save.rflags & X86_EFLAGS_IF));
+(svm->vmcb->save.rflags & X86_EFLAGS_IF) &&
+(svm->vcpu.arch.hflags & HF_GIF_MASK));
if (svm->vcpu.arch.interrupt_window_open && svm-
>vcpu.arch.irq_summary)
/*
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 982b6b2..3e25004 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -245,6 +245,7 @@ struct kvm_vcpu_arch {
unsigned long cr3;
unsigned long cr4;
unsigned long cr8;
+ u32 hflags;
u64 pdptrs[4]; /* pae */
u64 shadow_efer;
u64 apic_base;
@@ -734,6 +735,8 @@ enum {
TASK_SWITCH_GATE = 3,
};
+#define HF_GIF_MASK(1 << 0)
+
/*
* Hardware virtualization extension instructions may fault if a
* reboot turns off virtualization while processes are running.
--
1.5.6
--
To unsubscribe from
Re: [PATCH 4/9] Implement GIF, clgi and stgi v3
I had another possible idea for performance improvement here. Since we
only inject normal interrupts and exceptions (and not NMI and such) we
can patch clgi to cli and stgi to sti to save these two intercepts in
the guests vmrun path.
Any objections/problems with this?
On Wed, Sep 17, 2008 at 03:41:21PM +0200, Alexander Graf wrote:
> This patch implements the GIF flag and the clgi and stgi instructions that
> set this flag. Only if the flag is set (default), interrupts can be received
> by
> the CPU.
>
> To keep the information about that somewhere, this patch adds a new hidden
> flags vector. that is used to store information that does not go into the
> vmcb, but is SVM specific.
>
> v2 moves the hflags to x86 generic code
> v3 makes use of the new permission helper
>
> Signed-off-by: Alexander Graf <[EMAIL PROTECTED]>
> ---
> arch/x86/kvm/svm.c | 42 +++---
> include/asm-x86/kvm_host.h |3 +++
> 2 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index c72e728..469ecc5 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -612,6 +612,8 @@ static void init_vmcb(struct vcpu_svm *svm)
> save->cr4 = 0;
> }
> force_new_asid(&svm->vcpu);
> +
> + svm->vcpu.arch.hflags = HF_GIF_MASK;
> }
>
> static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
> @@ -1227,6 +1229,36 @@ static int nested_svm_do(struct vcpu_svm *svm,
> return retval;
> }
>
> +static int stgi_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
> +{
> + if (nested_svm_check_permissions(svm))
> + return 1;
> +
> + svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
> + skip_emulated_instruction(&svm->vcpu);
> +
> + svm->vcpu.arch.hflags |= HF_GIF_MASK;
> +
> + return 1;
> +}
> +
> +static int clgi_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
> +{
> + if (nested_svm_check_permissions(svm))
> + return 1;
> +
> + svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
> + skip_emulated_instruction(&svm->vcpu);
> +
> + svm->vcpu.arch.hflags &= ~HF_GIF_MASK;
> +
> + /* After a CLGI no interrupts should come */
> + svm_clear_vintr(svm);
> + svm->vmcb->control.int_ctl &= ~V_IRQ_MASK;
> +
> + return 1;
> +}
> +
> static int invalid_op_interception(struct vcpu_svm *svm,
> struct kvm_run *kvm_run)
> {
> @@ -1521,8 +1553,8 @@ static int (*svm_exit_handlers[])(struct vcpu_svm *svm,
> [SVM_EXIT_VMMCALL] = vmmcall_interception,
> [SVM_EXIT_VMLOAD] = invalid_op_interception,
> [SVM_EXIT_VMSAVE] = invalid_op_interception,
> - [SVM_EXIT_STGI] = invalid_op_interception,
> - [SVM_EXIT_CLGI] = invalid_op_interception,
> + [SVM_EXIT_STGI] = stgi_interception,
> + [SVM_EXIT_CLGI] = clgi_interception,
> [SVM_EXIT_SKINIT] = invalid_op_interception,
> [SVM_EXIT_WBINVD] = emulate_on_interception,
> [SVM_EXIT_MONITOR] = invalid_op_interception,
> @@ -1669,6 +1701,9 @@ static void svm_intr_assist(struct kvm_vcpu *vcpu)
> if (!kvm_cpu_has_interrupt(vcpu))
> goto out;
>
> + if (!(svm->vcpu.arch.hflags & HF_GIF_MASK))
> + goto out;
> +
> if (!(vmcb->save.rflags & X86_EFLAGS_IF) ||
> (vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) ||
> (vmcb->control.event_inj & SVM_EVTINJ_VALID)) {
> @@ -1720,7 +1755,8 @@ static void do_interrupt_requests(struct kvm_vcpu *vcpu,
>
> svm->vcpu.arch.interrupt_window_open =
> (!(control->int_state & SVM_INTERRUPT_SHADOW_MASK) &&
> - (svm->vmcb->save.rflags & X86_EFLAGS_IF));
> + (svm->vmcb->save.rflags & X86_EFLAGS_IF) &&
> + (svm->vcpu.arch.hflags & HF_GIF_MASK));
>
> if (svm->vcpu.arch.interrupt_window_open && svm->vcpu.arch.irq_summary)
> /*
> diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
> index 982b6b2..3e25004 100644
> --- a/include/asm-x86/kvm_host.h
> +++ b/include/asm-x86/kvm_host.h
> @@ -245,6 +245,7 @@ struct kvm_vcpu_arch {
> unsigned long cr3;
> unsigned long cr4;
> unsigned long cr8;
> + u32 hflags;
> u64 pdptrs[4]; /* pae */
> u64 shadow_efer;
> u64 apic_base;
> @@ -734,6 +735,8 @@ enum {
> TASK_SWITCH_GATE = 3,
> };
>
> +#define HF_GIF_MASK (1 << 0)
> +
> /*
> * Hardware virtualization extension instructions may fault if a
> * reboot turns off virtualization while processes are running.
> --
> 1.5.6
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel
[PATCH 4/9] Implement GIF, clgi and stgi v3
This patch implements the GIF flag and the clgi and stgi instructions that
set this flag. Only if the flag is set (default), interrupts can be received by
the CPU.
To keep the information about that somewhere, this patch adds a new hidden
flags vector. that is used to store information that does not go into the
vmcb, but is SVM specific.
v2 moves the hflags to x86 generic code
v3 makes use of the new permission helper
Signed-off-by: Alexander Graf <[EMAIL PROTECTED]>
---
arch/x86/kvm/svm.c | 42 +++---
include/asm-x86/kvm_host.h |3 +++
2 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index c72e728..469ecc5 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -612,6 +612,8 @@ static void init_vmcb(struct vcpu_svm *svm)
save->cr4 = 0;
}
force_new_asid(&svm->vcpu);
+
+ svm->vcpu.arch.hflags = HF_GIF_MASK;
}
static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
@@ -1227,6 +1229,36 @@ static int nested_svm_do(struct vcpu_svm *svm,
return retval;
}
+static int stgi_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
+{
+ if (nested_svm_check_permissions(svm))
+ return 1;
+
+ svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
+ skip_emulated_instruction(&svm->vcpu);
+
+ svm->vcpu.arch.hflags |= HF_GIF_MASK;
+
+ return 1;
+}
+
+static int clgi_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
+{
+ if (nested_svm_check_permissions(svm))
+ return 1;
+
+ svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
+ skip_emulated_instruction(&svm->vcpu);
+
+ svm->vcpu.arch.hflags &= ~HF_GIF_MASK;
+
+ /* After a CLGI no interrupts should come */
+ svm_clear_vintr(svm);
+ svm->vmcb->control.int_ctl &= ~V_IRQ_MASK;
+
+ return 1;
+}
+
static int invalid_op_interception(struct vcpu_svm *svm,
struct kvm_run *kvm_run)
{
@@ -1521,8 +1553,8 @@ static int (*svm_exit_handlers[])(struct vcpu_svm *svm,
[SVM_EXIT_VMMCALL] = vmmcall_interception,
[SVM_EXIT_VMLOAD] = invalid_op_interception,
[SVM_EXIT_VMSAVE] = invalid_op_interception,
- [SVM_EXIT_STGI] = invalid_op_interception,
- [SVM_EXIT_CLGI] = invalid_op_interception,
+ [SVM_EXIT_STGI] = stgi_interception,
+ [SVM_EXIT_CLGI] = clgi_interception,
[SVM_EXIT_SKINIT] = invalid_op_interception,
[SVM_EXIT_WBINVD] = emulate_on_interception,
[SVM_EXIT_MONITOR] = invalid_op_interception,
@@ -1669,6 +1701,9 @@ static void svm_intr_assist(struct kvm_vcpu *vcpu)
if (!kvm_cpu_has_interrupt(vcpu))
goto out;
+ if (!(svm->vcpu.arch.hflags & HF_GIF_MASK))
+ goto out;
+
if (!(vmcb->save.rflags & X86_EFLAGS_IF) ||
(vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) ||
(vmcb->control.event_inj & SVM_EVTINJ_VALID)) {
@@ -1720,7 +1755,8 @@ static void do_interrupt_requests(struct kvm_vcpu *vcpu,
svm->vcpu.arch.interrupt_window_open =
(!(control->int_state & SVM_INTERRUPT_SHADOW_MASK) &&
-(svm->vmcb->save.rflags & X86_EFLAGS_IF));
+(svm->vmcb->save.rflags & X86_EFLAGS_IF) &&
+(svm->vcpu.arch.hflags & HF_GIF_MASK));
if (svm->vcpu.arch.interrupt_window_open && svm->vcpu.arch.irq_summary)
/*
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 982b6b2..3e25004 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -245,6 +245,7 @@ struct kvm_vcpu_arch {
unsigned long cr3;
unsigned long cr4;
unsigned long cr8;
+ u32 hflags;
u64 pdptrs[4]; /* pae */
u64 shadow_efer;
u64 apic_base;
@@ -734,6 +735,8 @@ enum {
TASK_SWITCH_GATE = 3,
};
+#define HF_GIF_MASK(1 << 0)
+
/*
* Hardware virtualization extension instructions may fault if a
* reboot turns off virtualization while processes are running.
--
1.5.6
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
