lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 30 Aug 2017 16:36:06 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     Christoffer Dall <cdall@...aro.org>
Cc:     linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        kvmarm@...ts.cs.columbia.edu, kvm@...r.kernel.org,
        Christoffer Dall <christoffer.dall@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Eric Auger <eric.auger@...hat.com>,
        Shanker Donthineni <shankerd@...eaurora.org>,
        Mark Rutland <mark.rutland@....com>,
        Shameerali Kolothum Thodi 
        <shameerali.kolothum.thodi@...wei.com>
Subject: Re: [PATCH v3 51/59] KVM: arm/arm64: GICv4: Add doorbell interrupt
 handling

On 28/08/17 19:18, Christoffer Dall wrote:
> On Mon, Jul 31, 2017 at 06:26:29PM +0100, Marc Zyngier wrote:
>> When a vPE is not running, a VLPI being made pending results in a
>> doorbell interrupt being delivered. Let's handle this interrupt
>> and update the pending_last flag that indicates that VLPIs are
>> pending. The corresponding vcpu is also kicked into action.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@....com>
>> ---
>>  virt/kvm/arm/vgic/vgic-v4.c | 34 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c
>> index 534d3051a078..6af3cde6d7d4 100644
>> --- a/virt/kvm/arm/vgic/vgic-v4.c
>> +++ b/virt/kvm/arm/vgic/vgic-v4.c
>> @@ -21,6 +21,19 @@
>>  
>>  #include "vgic.h"
>>  
>> +static irqreturn_t vgic_v4_doorbell_handler(int irq, void *info)
>> +{
>> +	struct kvm_vcpu *vcpu = info;
>> +
>> +	if (!kvm_vgic_vcpu_pending_irq(vcpu)) {
>> +		vcpu->arch.vgic_cpu.vgic_v3.its_vpe.pending_last = true;
>> +		kvm_make_request(KVM_REQ_IRQ_PENDING, vcpu);
>> +		kvm_vcpu_kick(vcpu);
>> +	}
> 
> Can this ever fire while vgic_v4_init() is running and before te rest of
> the system has been properly initialized with some entertaining results
> to follow?  (I'm not sure if spurious doorbell non-resident vPE
> interrupts is a thing or not).

It could if you only had this patch. The following patch makes sure that
the interrupt does not get enabled at request time, meaning it will only
get enabled when the vcpu will eventually block.

And yes, spurious doorbells are a real thing. And they suck.

> 
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>>  int vgic_v4_init(struct kvm *kvm)
>>  {
>>  	struct vgic_dist *dist = &kvm->arch.vgic;
>> @@ -57,16 +70,37 @@ int vgic_v4_init(struct kvm *kvm)
>>  		return ret;
>>  	}
>>  
>> +	kvm_for_each_vcpu(i, vcpu, kvm) {
>> +		int irq = dist->its_vm.vpes[i]->irq;
>> +
>> +		ret = request_irq(irq, vgic_v4_doorbell_handler,
>> +				  0, "vcpu", vcpu);
>> +		if (ret) {
>> +			kvm_err("failed to allocate vcpu IRQ%d\n", irq);
>> +			dist->its_vm.nr_vpes = i;
> 
> That's a neat trick for the error handling.  Might deserve a tiny
> comment.

Ah, yes, will do.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Powered by blists - more mailing lists