linux-kernel - Re: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 6 Jul 2015 16:52:41 +0100
From:	Andre Przywara <andre.przywara@....com>
To:	Eric Auger <eric.auger@...aro.org>,
	Pavel Fedin <p.fedin@...sung.com>,
	"'Paolo Bonzini'" <pbonzini@...hat.com>,
	"'Christoffer Dall'" <christoffer.dall@...aro.org>
Cc:	"eric.auger@...com" <eric.auger@...com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Marc Zyngier <Marc.Zyngier@....com>,
	"kvmarm@...ts.cs.columbia.edu" <kvmarm@...ts.cs.columbia.edu>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi

Salut Eric,

....

>>  ITS code in qemu just does:
>>
>> ---cut ---
>>     msi_supported = true;
>>     kvm_msi_flags = KVM_MSI_VALID_DEVID;
>>     kvm_msi_via_irqfd_allowed = kvm_has_gsi_routing();
>>     kvm_gsi_routing_allowed = kvm_msi_via_irqfd_allowed;
>> --- cut ---
>>
>>  I set KVM_MSI_VALID_DEVID unconditionally here just because it will never be checked if
>> kvm_msi_via_irqfd_allowed is false, it's just qemu specifics. The more canonical form would perhaps
>> be:
>> --- cut ---
>> if (kvm_has_gsi_routing()) {
>>     kvm_msi_flags = KVM_MSI_VALID_DEVID;
> Personally I prefer a capability rather than hardcoding a global
> variable value in the qemu interrupt controller code. All the more so
> typically there is KVM GSI routing cap that could/should? be queried
> instead of hardcoding the value I think.
> 
> So not sure whether we eventually concluded;-)
> - introduce a KVM_CAP_MSI_DEVID capability? All OK except Pavel not
> convinced?

OK for me.

> - userspaces puts the devid in struct kvm_irq_routing_msi pad field:
> consensus (we do not intrduce a new kvm_irq_routing_ext_msi)

OK for me.

> - userspace tells it conveyed a devid by setting
> A) the kvm_irq_routing_entry's field?

You mean kvm_irq_routing_entry's "flags" here?

> B) the kvm_irq_routing_entry's type

So personally I don't like it so much to use the generic flags field to
specify the meaning within one particular type only. Using a new type
instead seems to be more consistent, reusing an existing struct for that
sounds even better.
As written before (and coded in my branch) we can collapse that into the
existing MSI type while translating that into the kernel internal
routing structure to make the kernel code changes minimal.

> no consensus. If there is a cap, does it really matter?

I guess not. But I prefer the new type anyway, as it also has a known
error path for older kernels.

Cheers,
Andre.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/