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]

Message-ID: <20150706103755.GC11590@cbox>
Date:	Mon, 6 Jul 2015 12:37:55 +0200
From:	Christoffer Dall <christoffer.dall@...aro.org>
To:	Andre Przywara <andre.przywara@....com>
Cc:	"pbonzini@...hat.com" <pbonzini@...hat.com>,
	Pavel Fedin <p.fedin@...sung.com>,
	'Eric Auger' <eric.auger@...aro.org>,
	"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

On Mon, Jul 06, 2015 at 11:05:26AM +0100, Andre Przywara wrote:
> Hi Christoffer,
> 
> On 06/07/15 10:30, Christoffer Dall wrote:
> > On Mon, Jul 06, 2015 at 09:30:20AM +0100, Andre Przywara wrote:
> >> Hi Pavel,
> >>
> >> On 06/07/15 07:42, Pavel Fedin wrote:
> >>>  Hello!
> >>>
> >>>> I like this approach, but it runs into problems:
> >>>> As you read above the current documentation says that the flags field
> >>>> must be zero and the current KVM_SET_GSI_ROUTING handler bails out if it
> >>>> isn't. So userland would need to know whether it's safe to set that
> >>>> field.
> >>>
> >>>  This problem does not exist because:
> >>> a) Older platforms do not need this flag, so they expect to get zero.
> >>> b) ARM64 + GICv3 platform cannot work without this flag.
> >>>
> >>>  This is perfectly OK combination IMO. Userland just knows, whether it needs to supply device ID or
> >>> not. For example, my modified qemu now has kvm_msi_flags global variable which defaults to 0. ITS
> >>> code, then, if activated, changes it to KVM_MSI_VALID_DEVID, and qemu starts supplying device IDs to
> >>> the related calls.
> >>
> >> Well, I had this solution before in kvmtool: If ARM && ITS then set the
> >> flag. But I wasn't really happy with this, as the IRQ routing, setup and
> >> injection code is rather architecture agnostic (implementing the generic
> >> KVM interface), so spraying in some architecture hacks sounded not very
> >> elegant.
> >> Also as the flag describes a rather generic feature (provide an unique
> >> device ID), I'd rather avoid to make this an ARM hack.
> >>
> >> That being said this is not a show stopper for me, so if the others are
> >> happy with this, I will go down your road.
> >>
> > There must be some way for userspace to discover if it's valid to set
> > the flag or not; either through a well-defined error-code probing
> > mechanism for KVM_SET_GSI_ROUTING or through advertising a capability.
> 
> Right, makes sense, I was wondering about this requirement earlier, but
> couldn't find really good prior art in the code (KVM_GET_VCPU_EVENTS
> seems to be bad example).
> So I think we get along with a new VM specific capability like
> KVM_CAP_MSIS_REQUIRE_DEVID. This isn't strictly a "capability" (as it's
> more a requirement), but I guess it fits here anyway. It has to be
> per-VM, as a GICv2M guest does not need it, but an ITS guest does.
> We can use this very flag for both the KVM_SIGNAL_MSI and the
> KVM_SET_GSI_ROUTING ioctl, so interface churn is kept minimal.
> 
> Does that make sense?
> 
> Actually I have implemented this already last week, I will send it out
> along with a v2 of the ITS emulation later this week.
> 
I don't view it as 'the kernel requires this' but as 'the kernel will
not complain with arbitrary error code if you set the devid flag'
capability, and it's up to userspace (as usual) to provide the correct
arguments for things to work, and up to the kernel to ensure we don't
crash the system etc.

Thus, if you want to advertise it as a capability, I would rather call
it KVM_CAP_MSI_DEVID.

The question is if userspace code that sets the devid flag will anyway
depend on some discovery mechanism of whether or not the kernel supports
arm64 irqfd etc. and if so, can we be sure to add the required support
at once in the kernel so that EINVAL never means 'you set the flags
field on the ioctl on an old kernel'?

This smells an awful lot like a capability to me.

-Christoffer
--
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/