Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1371568 > unrolled thread

Re: [PART1 RFC v3 07/12] svm: Add interrupt injection via AVIC

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PART1 RFC v3 07/12] svm: Add interrupt injection via AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-04-05 15:30 +0200
    Re: [PART1 RFC v3 07/12] svm: Add interrupt injection via AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-04-05 18:00 +0200

#1371568 — Re: [PART1 RFC v3 07/12] svm: Add interrupt injection via AVIC

Hi Paolo,

On 3/18/16 17:22, Paolo Bonzini wrote:
>
>
> On 18/03/2016 07:09, Suravee Suthikulpanit wrote:
>> This patch introduces a new mechanism to inject interrupt using AVIC.
>> Since VINTR is not supported when enable AVIC, we need to inject
>> interrupt via APIC backing page instead.
>>
>> This patch also adds support for AVIC doorbell, which is used by
>> KVM to signal a running vcpu to check IRR for injected interrupts.
>>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>
> Looks good, but I think it breaks nested virtualization.  See below.
>
>> [...]
>> @@ -2877,8 +2895,10 @@ static int clgi_interception(struct vcpu_svm *svm)
>>   	disable_gif(svm);
>>
>>   	/* After a CLGI no interrupts should come */
>> -	svm_clear_vintr(svm);
>> -	svm->vmcb->control.int_ctl &= ~V_IRQ_MASK;
>> +	if (!svm_vcpu_avic_enabled(svm)) {
>> +		svm_clear_vintr(svm);
>> +		svm->vmcb->control.int_ctl &= ~V_IRQ_MASK;
>> +	}
>
> This is for nested virtualization.  Unless you support nested AVIC, the
> L2 guest should run without AVIC (i.e. IsRunning should be false) and
> use the old VINTR mechanism.

I see. I am not planning to supported nested AVIC at the L2 level for 
the moment. If it is alright, I would like to get the basic AVIC and 
IOMMU in first (unless you have a different opinion).

In that case, I think I should also make sure to not expose AVIC CPUID 
to the guest VM.

>> [...]
>> @@ -3904,6 +3942,9 @@ static void enable_irq_window(struct kvm_vcpu *vcpu)
>>   	 * get that intercept, this function will be called again though and
>>   	 * we'll get the vintr intercept.
>>   	 */
>> +	if (svm_vcpu_avic_enabled(svm))
>> +		return;
>
> Same here.

If I make change so that we do not expose the AVIC CPUID to the L1 
guest, then the L1 KVM driver should not be setting up AVIC for the L2 
vcpus. And, in this case, the svm_vcpu_avic_enabled(svm) should return 
false. I've not tested with nested VM. I will give that a try.

Thanks,
Suravee

[toc] | [next] | [standalone]

#1371753

Hi Paolo,

On 4/5/16 20:26, Suravee Suthikulpanit wrote:
>>> @@ -2877,8 +2895,10 @@ static int clgi_interception(struct vcpu_svm
>>> *svm)
>>>       disable_gif(svm);
>>>
>>>       /* After a CLGI no interrupts should come */
>>> -    svm_clear_vintr(svm);
>>> -    svm->vmcb->control.int_ctl &= ~V_IRQ_MASK;
>>> +    if (!svm_vcpu_avic_enabled(svm)) {
>>> +        svm_clear_vintr(svm);
>>> +        svm->vmcb->control.int_ctl &= ~V_IRQ_MASK;
>>> +    }
>>
>> This is for nested virtualization.  Unless you support nested AVIC, the
>> L2 guest should run without AVIC (i.e. IsRunning should be false) and
>> use the old VINTR mechanism.
>
> I see. I am not planning to supported nested AVIC at the L2 level for
> the moment. If it is alright, I would like to get the basic AVIC and
> IOMMU in first (unless you have a different opinion).
>
> In that case, I think I should also make sure to not expose AVIC CPUID
> to the guest VM.

Actually, it should have already not set the AVIC CPUID in the L1 guest.

Thanks,
Suravee

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web