linux-kernel - Re: [PATCH v5 18/23] KVM: arm64: GICv4.1: Add direct injection capability to SGI registers

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 linux-cve-announce 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:   Fri, 20 Mar 2020 10:05:21 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Auger Eric <eric.auger@...hat.com>
Cc:     linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Jason Cooper <jason@...edaemon.net>,
        Robert Richter <rrichter@...vell.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Zenghui Yu <yuzenghui@...wei.com>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        Suzuki K Poulose <suzuki.poulose@....com>
Subject: Re: [PATCH v5 18/23] KVM: arm64: GICv4.1: Add direct injection
 capability to SGI registers

Hi Eric,

On 2020-03-20 08:11, Auger Eric wrote:
> Hi Marc,
> On 3/4/20 9:33 PM, Marc Zyngier wrote:
>> Most of the GICv3 emulation code that deals with SGIs now has to be
>> aware of the v4.1 capabilities in order to benefit from it.
>> 
>> Add such support, keyed on the interrupt having the hw flag set and
>> being a SGI.
>> 
>> Signed-off-by: Marc Zyngier <maz@...nel.org>
>> ---
>>  virt/kvm/arm/vgic/vgic-mmio-v3.c | 15 +++++-
>>  virt/kvm/arm/vgic/vgic-mmio.c    | 88 
>> ++++++++++++++++++++++++++++++--
>>  2 files changed, 96 insertions(+), 7 deletions(-)
>> 

[...]

>> @@ -113,7 +125,21 @@ void vgic_mmio_write_senable(struct kvm_vcpu 
>> *vcpu,
>>  		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
>> 
>>  		raw_spin_lock_irqsave(&irq->irq_lock, flags);
>> -		if (vgic_irq_is_mapped_level(irq)) {
>> +		if (irq->hw && vgic_irq_is_sgi(irq->intid)) {
>> +			if (!irq->enabled) {
>> +				struct irq_data *data;
>> +
>> +				irq->enabled = true;
>> +				data = &irq_to_desc(irq->host_irq)->irq_data;
>> +				while (irqd_irq_disabled(data))
>> +					enable_irq(irq->host_irq);
> could you explain me why the while() is requested?

Ah, interesting question: disable_irq() (and its variants) can nest. 
This
means that if you have done two disable_irq(), you must do two 
enable_irq()
to get back to the interrupt being enabled.

The locking should ensure that this nesting doesn't happen, but I'm 
paranoid
(see the GICv4.0 doorbell handling). It also makes it easier to reason 
about
the initial state.

[...]

> Reviewed-by: Eric Auger <eric.auger@...hat.com>

Thanks!

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