[PATCH 0/2] KVM: SVM: Virtual GIF

[PATCH 0/2] KVM: SVM: Virtual GIF

[Janakarajan Natarajan]

This patchset adds support for the Virtual GIF (VGIF) feature. This
feature allows the STGI and CLGI instructions to be executed in the
Guest Mode and not require a #VMEXIT. With this, virtual interrupts
can be controlled in the Guest Mode while still allowing physical
interrupts to be intercepted by the hypervisor.

In order to provide this ability, two new bits are added to the VMCB
at offset 60h:

* Bit 9  - VGIF value
	 : 0 -> Virtual interrupts are masked
	 : 1 -> Virtual interrupts are unmasked

* Bit 25 - AMD Virtual GIF enabled for this guest
	 : 0 -> Disabled
	 : 1 -> Enabled

When a VMRUN is executed and Bit 25 is set, the processor uses Bit 9
as the starting value of the virtual GIF. It then provides masking
capability for when virtual interrupts are taken. Bit 9 is writeable
by the hypervisor and loaded on VMRUN and saved on #VMEXIT. STGI/CLGI
executed in the Guest Mode sets or clear the virtual GIF.

The advantage of this feature will be the greatly reduced number of
world switches to support the STGI and CLGI instructions by the
outermost hypervisor at Current Privilege Level (CPL) 0.

This has been tested with Xen, Hyper-V and KVM as the nested hypervisor.

Janakarajan Natarajan (2):
  KVM: SVM: Add Virtual GIF feature definition
  KVM: SVM: Enable Virtual GIF feature

 arch/x86/include/asm/cpufeatures.h |  1 +
 arch/x86/include/asm/svm.h         |  6 +++++
 arch/x86/kvm/svm.c                 | 47 ++++++++++++++++++++++++++++++++------
 3 files changed, 47 insertions(+), 7 deletions(-)

-- 
2.7.4

[PATCH 1/2] KVM: SVM: Add Virtual GIF feature definition

[Janakarajan Natarajan]

Define a new cpufeature definition for Virtual GIF.

Signed-off-by: Janakarajan Natarajan <Janakarajan.Natarajan@amd.com>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index ca3c48c..58e7211 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -287,6 +287,7 @@
 #define X86_FEATURE_PFTHRESHOLD (15*32+12) /* pause filter threshold */
 #define X86_FEATURE_AVIC	(15*32+13) /* Virtual Interrupt Controller */
 #define X86_FEATURE_VIRTUAL_VMLOAD_VMSAVE (15*32+15) /* Virtual VMLOAD VMSAVE */
+#define X86_FEATURE_V_GIF	(15*32+16) /* Virtual GIF */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx), word 16 */
 #define X86_FEATURE_AVX512VBMI  (16*32+ 1) /* AVX512 Vector Bit Manipulation instructions*/
-- 
2.7.4

Re: [PATCH 1/2] KVM: SVM: Add Virtual GIF feature definition

[Borislav Petkov]

On Wed, Aug 16, 2017 at 10:54:49AM -0500, Janakarajan Natarajan wrote:
> Define a new cpufeature definition for Virtual GIF.

"Define ... definition" ?

> 
> Signed-off-by: Janakarajan Natarajan <Janakarajan.Natarajan@amd.com>
> ---
>  arch/x86/include/asm/cpufeatures.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index ca3c48c..58e7211 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -287,6 +287,7 @@
>  #define X86_FEATURE_PFTHRESHOLD (15*32+12) /* pause filter threshold */
>  #define X86_FEATURE_AVIC	(15*32+13) /* Virtual Interrupt Controller */
>  #define X86_FEATURE_VIRTUAL_VMLOAD_VMSAVE (15*32+15) /* Virtual VMLOAD VMSAVE */
> +#define X86_FEATURE_V_GIF	(15*32+16) /* Virtual GIF */

PPR calls it vGIF so make that X86_FEATURE_VGIF.

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [PATCH 1/2] KVM: SVM: Add Virtual GIF feature definition

[Natarajan, Janakarajan]

On 8/16/2017 12:36 PM, Borislav Petkov wrote:
> On Wed, Aug 16, 2017 at 10:54:49AM -0500, Janakarajan Natarajan wrote:
>> Define a new cpufeature definition for Virtual GIF.
> "Define ... definition" ?
>
>> Signed-off-by: Janakarajan Natarajan <Janakarajan.Natarajan@amd.com>
>> ---
>>   arch/x86/include/asm/cpufeatures.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index ca3c48c..58e7211 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -287,6 +287,7 @@
>>   #define X86_FEATURE_PFTHRESHOLD (15*32+12) /* pause filter threshold */
>>   #define X86_FEATURE_AVIC	(15*32+13) /* Virtual Interrupt Controller */
>>   #define X86_FEATURE_VIRTUAL_VMLOAD_VMSAVE (15*32+15) /* Virtual VMLOAD VMSAVE */
>> +#define X86_FEATURE_V_GIF	(15*32+16) /* Virtual GIF */
> PPR calls it vGIF so make that X86_FEATURE_VGIF.

I will send a v2 with the changes.

>
> Thanks.
>

[PATCH 2/2] KVM: SVM: Enable Virtual GIF feature

[Janakarajan Natarajan]

Enable the Virtual GIF feature. This is done by setting bit 25 at position
60h in the vmcb.

With this feature enabled, the processor uses bit 9 at position 60h as the
virtual GIF when executing STGI/CLGI instructions.

Since the execution of STGI by the L1 hypervisor does not cause a return to
the outermost (L0) hypervisor, the enable_irq_window and enable_nmi_window
are modified.

The IRQ and NMI windows will be opened even if GIF is not set, under the
assumption that on resuming the L1 hypervisor the IRQ and NMI will be
held pending until the processor executes the STGI instruction.

Signed-off-by: Janakarajan Natarajan <Janakarajan.Natarajan@amd.com>
---
 arch/x86/include/asm/svm.h |  6 ++++++
 arch/x86/kvm/svm.c         | 47 +++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 58fffe7..14835dd 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -107,6 +107,9 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
 #define V_IRQ_SHIFT 8
 #define V_IRQ_MASK (1 << V_IRQ_SHIFT)
 
+#define V_GIF_SHIFT 9
+#define V_GIF_MASK (1 << V_GIF_SHIFT)
+
 #define V_INTR_PRIO_SHIFT 16
 #define V_INTR_PRIO_MASK (0x0f << V_INTR_PRIO_SHIFT)
 
@@ -116,6 +119,9 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
 #define V_INTR_MASKING_SHIFT 24
 #define V_INTR_MASKING_MASK (1 << V_INTR_MASKING_SHIFT)
 
+#define V_GIF_ENABLE_SHIFT 25
+#define V_GIF_ENABLE_MASK (1 << V_GIF_ENABLE_SHIFT)
+
 #define AVIC_ENABLE_SHIFT 31
 #define AVIC_ENABLE_MASK (1 << AVIC_ENABLE_SHIFT)
 
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 1107626..e6ddabb 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -280,6 +280,10 @@ module_param(avic, int, S_IRUGO);
 static int vls = true;
 module_param(vls, int, 0444);
 
+/* enable/disable Virtual GIF */
+static int vgif = true;
+module_param(vgif, int, 0444);
+
 /* AVIC VM ID bit masks and lock */
 static DECLARE_BITMAP(avic_vm_id_bitmap, AVIC_VM_ID_NR);
 static DEFINE_SPINLOCK(avic_vm_id_lock);
@@ -479,19 +483,33 @@ static inline void clr_intercept(struct vcpu_svm *svm, int bit)
 	recalc_intercepts(svm);
 }
 
+static inline bool vgif_enabled(struct vcpu_svm *svm)
+{
+	return !!(svm->vmcb->control.int_ctl & V_GIF_ENABLE_MASK);
+}
+
 static inline void enable_gif(struct vcpu_svm *svm)
 {
-	svm->vcpu.arch.hflags |= HF_GIF_MASK;
+	if (vgif_enabled(svm))
+		svm->vmcb->control.int_ctl |= V_GIF_MASK;
+	else
+		svm->vcpu.arch.hflags |= HF_GIF_MASK;
 }
 
 static inline void disable_gif(struct vcpu_svm *svm)
 {
-	svm->vcpu.arch.hflags &= ~HF_GIF_MASK;
+	if (vgif_enabled(svm))
+		svm->vmcb->control.int_ctl &= ~V_GIF_MASK;
+	else
+		svm->vcpu.arch.hflags &= ~HF_GIF_MASK;
 }
 
 static inline bool gif_set(struct vcpu_svm *svm)
 {
-	return !!(svm->vcpu.arch.hflags & HF_GIF_MASK);
+	if (vgif_enabled(svm))
+		return !!(svm->vmcb->control.int_ctl & V_GIF_MASK);
+	else
+		return !!(svm->vcpu.arch.hflags & HF_GIF_MASK);
 }
 
 static unsigned long iopm_base;
@@ -1108,6 +1126,13 @@ static __init int svm_hardware_setup(void)
 		}
 	}
 
+	if (vgif) {
+		if (!boot_cpu_has(X86_FEATURE_V_GIF))
+			vgif = false;
+		else
+			pr_info("Virtual GIF supported\n");
+	}
+
 	return 0;
 
 err:
@@ -1305,6 +1330,12 @@ static void init_vmcb(struct vcpu_svm *svm)
 		svm->vmcb->control.virt_ext |= VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK;
 	}
 
+	if (vgif) {
+		clr_intercept(svm, INTERCEPT_STGI);
+		clr_intercept(svm, INTERCEPT_CLGI);
+		svm->vmcb->control.int_ctl |= V_GIF_ENABLE_MASK;
+	}
+
 	mark_all_dirty(svm->vmcb);
 
 	enable_gif(svm);
@@ -4686,9 +4717,11 @@ static void enable_irq_window(struct kvm_vcpu *vcpu)
 	 * In case GIF=0 we can't rely on the CPU to tell us when GIF becomes
 	 * 1, because that's a separate STGI/VMRUN intercept.  The next time we
 	 * get that intercept, this function will be called again though and
-	 * we'll get the vintr intercept.
+	 * we'll get the vintr intercept. However, if the VGIF feature is
+	 * enabled, the STGI interception will not occur. Enable the irq
+	 * window under the assumption that the hardware will set the GIF.
 	 */
-	if (gif_set(svm) && nested_svm_intr(svm)) {
+	if ((vgif_enabled(svm) || gif_set(svm)) && nested_svm_intr(svm)) {
 		svm_set_vintr(svm);
 		svm_inject_irq(svm, 0x0);
 	}
@@ -4702,8 +4735,8 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu)
 	    == HF_NMI_MASK)
 		return; /* IRET will cause a vm exit */
 
-	if ((svm->vcpu.arch.hflags & HF_GIF_MASK) == 0)
-		return; /* STGI will cause a vm exit */
+	if (!vgif_enabled(svm) && !gif_set(svm))
+		return; /* STGI will cause a vm exit or HW will set VGIF*/
 
 	if (svm->nested.exit_required)
 		return; /* we're not going to run the guest yet */
-- 
2.7.4

Re: [PATCH 2/2] KVM: SVM: Enable Virtual GIF feature

[Radim Krcmar]

2017-08-16 10:54-0500, Janakarajan Natarajan:
> Enable the Virtual GIF feature. This is done by setting bit 25 at position
> 60h in the vmcb.
> 
> With this feature enabled, the processor uses bit 9 at position 60h as the
> virtual GIF when executing STGI/CLGI instructions.
> 
> Since the execution of STGI by the L1 hypervisor does not cause a return to
> the outermost (L0) hypervisor, the enable_irq_window and enable_nmi_window
> are modified.
> 
> The IRQ and NMI windows will be opened even if GIF is not set, under the
> assumption that on resuming the L1 hypervisor the IRQ and NMI will be
> held pending until the processor executes the STGI instruction.
> 
> Signed-off-by: Janakarajan Natarajan <Janakarajan.Natarajan@amd.com>
> ---
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> @@ -4702,8 +4735,8 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu)
>  	    == HF_NMI_MASK)
>  		return; /* IRET will cause a vm exit */
>  
> -	if ((svm->vcpu.arch.hflags & HF_GIF_MASK) == 0)
> -		return; /* STGI will cause a vm exit */
> +	if (!vgif_enabled(svm) && !gif_set(svm))
> +		return; /* STGI will cause a vm exit or HW will set VGIF*/

Why don't we enable STGI interception to get notified that the window
has opened?  (I doubt that single stepping would be faster ...)

Thanks.

Re: [PATCH 2/2] KVM: SVM: Enable Virtual GIF feature

[Natarajan, Janakarajan]

On 8/16/2017 2:53 PM, Radim Krcmar wrote:
> 2017-08-16 10:54-0500, Janakarajan Natarajan:
>> Enable the Virtual GIF feature. This is done by setting bit 25 at position
>> 60h in the vmcb.
>>
>> With this feature enabled, the processor uses bit 9 at position 60h as the
>> virtual GIF when executing STGI/CLGI instructions.
>>
>> Since the execution of STGI by the L1 hypervisor does not cause a return to
>> the outermost (L0) hypervisor, the enable_irq_window and enable_nmi_window
>> are modified.
>>
>> The IRQ and NMI windows will be opened even if GIF is not set, under the
>> assumption that on resuming the L1 hypervisor the IRQ and NMI will be
>> held pending until the processor executes the STGI instruction.
>>
>> Signed-off-by: Janakarajan Natarajan <Janakarajan.Natarajan@amd.com>
>> ---
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> @@ -4702,8 +4735,8 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu)
>>   	    == HF_NMI_MASK)
>>   		return; /* IRET will cause a vm exit */
>>   
>> -	if ((svm->vcpu.arch.hflags & HF_GIF_MASK) == 0)
>> -		return; /* STGI will cause a vm exit */
>> +	if (!vgif_enabled(svm) && !gif_set(svm))
>> +		return; /* STGI will cause a vm exit or HW will set VGIF*/
> Why don't we enable STGI interception to get notified that the window
> has opened?  (I doubt that single stepping would be faster ...)

It would defeat the purpose of having vGIF - execute STGI/CLGI and not 
have a world-switch. Plus
it would be like implementing the vGIF feature halfway, allowing only 
CLGI to take advantage of the
hardware.

>
> Thanks.

Re: [PATCH 2/2] KVM: SVM: Enable Virtual GIF feature

[Paolo Bonzini]

----- Original Message -----
> From: "Janakarajan Natarajan" <Janakarajan.Natarajan@amd.com>
> To: "Radim Krcmar" <rkrcmar@redhat.com>
> Cc: kvm@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, "Paolo Bonzini" <pbonzini@redhat.com>, "Joerg
> Roedel" <joro@8bytes.org>, "Andy Lutomirski" <luto@kernel.org>, "Tony Luck" <tony.luck@intel.com>, "Borislav Petkov"
> <bp@suse.de>, "Thomas Gleixner" <tglx@linutronix.de>, "Ingo Molnar" <mingo@redhat.com>, "H . Peter Anvin"
> <hpa@zytor.com>, "Yazen Ghannam" <Yazen.Ghannam@amd.com>
> Sent: Thursday, August 17, 2017 12:04:10 AM
> Subject: Re: [PATCH 2/2] KVM: SVM: Enable Virtual GIF feature
> 
> On 8/16/2017 2:53 PM, Radim Krcmar wrote:
> > 2017-08-16 10:54-0500, Janakarajan Natarajan:
> >> Enable the Virtual GIF feature. This is done by setting bit 25 at position
> >> 60h in the vmcb.
> >>
> >> With this feature enabled, the processor uses bit 9 at position 60h as the
> >> virtual GIF when executing STGI/CLGI instructions.
> >>
> >> Since the execution of STGI by the L1 hypervisor does not cause a return
> >> to
> >> the outermost (L0) hypervisor, the enable_irq_window and enable_nmi_window
> >> are modified.
> >>
> >> The IRQ and NMI windows will be opened even if GIF is not set, under the
> >> assumption that on resuming the L1 hypervisor the IRQ and NMI will be
> >> held pending until the processor executes the STGI instruction.
> >>
> >> Signed-off-by: Janakarajan Natarajan <Janakarajan.Natarajan@amd.com>
> >> ---
> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> >> @@ -4702,8 +4735,8 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu)
> >>   	    == HF_NMI_MASK)
> >>   		return; /* IRET will cause a vm exit */
> >>   
> >> -	if ((svm->vcpu.arch.hflags & HF_GIF_MASK) == 0)
> >> -		return; /* STGI will cause a vm exit */
> >> +	if (!vgif_enabled(svm) && !gif_set(svm))
> >> +		return; /* STGI will cause a vm exit or HW will set VGIF*/
> > Why don't we enable STGI interception to get notified that the window
> > has opened?  (I doubt that single stepping would be faster ...)
> 
> It would defeat the purpose of having vGIF - execute STGI/CLGI and not
> have a world-switch. Plus it would be like implementing the vGIF feature
> halfway, allowing only CLGI to take advantage of the hardware.

No, only enable the STGI intercept to enable the NMI window.  Normally
you'd still run with free STGI.

This is because if you do not return here, you enter singlestepping mode
where each instruction causes a world switch.

Paolo