| Message ID | 1445357947-6022-4-git-send-email-christoffer.dall@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Oct 20, 2015 at 03:51:05PM -0400, Paolo Bonzini wrote: > Should this be "select" or "depends on"? Not a blocker, can always be fixed in 4.4. > Hmm, I don't know actually. I trusted Arnd to make the right call and given Marc's ack as well, I didn't pay too much attention to that particular detail. Arnd, any comments? Thanks, -Christoffer > > > -----Original Message----- > From: Christoffer Dall [christoffer.dall@linaro.org] > Received: martedì, 20 ott 2015, 18:18 > To: Paolo Bonzini [pbonzini@redhat.com]; kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org > CC: Marc Zyngier [marc.zyngier@arm.com]; Arnd Bergmann [arnd@arndb.de]; Christoffer Dall [christoffer.dall@linaro.org] > Subject: [GIT PULL 3/6] KVM: arm: use GIC support unconditionally > > From: Arnd Bergmann <arnd@arndb.de> > > The vgic code on ARM is built for all configurations that enable KVM, > but the parent_data field that it references is only present when > CONFIG_IRQ_DOMAIN_HIERARCHY is set: > > virt/kvm/arm/vgic.c: In function 'kvm_vgic_map_phys_irq': > virt/kvm/arm/vgic.c:1781:13: error: 'struct irq_data' has no member named 'parent_data' > > This flag is implied by the GIC driver, and indeed the VGIC code only > makes sense if a GIC is present. This changes the CONFIG_KVM symbol > to always select GIC, which avoids the issue. > > Fixes: 662d9715840 ("arm/arm64: KVM: Kill CONFIG_KVM_ARM_{VGIC,TIMER}") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Acked-by: Marc Zyngier <marc.zyngier@arm.com> > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> > --- > arch/arm/kvm/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig > index 210ecca..356970f 100644 > --- a/arch/arm/kvm/Kconfig > +++ b/arch/arm/kvm/Kconfig > @@ -21,6 +21,7 @@ config KVM > depends on MMU && OF > select PREEMPT_NOTIFIERS > select ANON_INODES > + select ARM_GIC > select HAVE_KVM_CPU_RELAX_INTERCEPT > select HAVE_KVM_ARCH_TLB_FLUSH_ALL > select KVM_MMIO > -- > 2.1.2.330.g565301e.dirty >
On Tuesday 20 October 2015 15:51:05 Paolo Bonzini wrote:
> Should this be "select" or "depends on"? Not a blocker, can always be fixed in 4.4.
We have lots of 'select ARM_GIC' in the tree for platforms that use one, using
'depends on' will limit KVM support to being available only if at least one
of them is being used.
The only platform I can think of that uses ARMv7ve without actually having
a GIC is BCM2836 (Raspberry Pi 2). Can we actually run KVM on a platform
like that? If so, 'depends on' might be better, otherwise let's stay with
'select'.
Note that ARM_GIC is not a user-visible option, you can only turn it on
by picking one or more platforms that have a GIC.
Arnd
On Wed, Oct 21, 2015 at 03:45:20PM +0200, Arnd Bergmann wrote: > On Tuesday 20 October 2015 15:51:05 Paolo Bonzini wrote: > > Should this be "select" or "depends on"? Not a blocker, can always be fixed in 4.4. > > We have lots of 'select ARM_GIC' in the tree for platforms that use one, using > 'depends on' will limit KVM support to being available only if at least one > of them is being used. > > The only platform I can think of that uses ARMv7ve without actually having > a GIC is BCM2836 (Raspberry Pi 2). Can we actually run KVM on a platform > like that? If so, 'depends on' might be better, otherwise let's stay with > 'select'. Yes you can, just without the VGIC and the timer - you have to emulate that in userspace. Samsung also has a broken platform where they integrated things incorrectly, so you cannot use the VGIC, but that platform support is out of tree, so I can't see if it uses the GIC in general or not. I'm a bit confused why using 'depends on' in this case helps anythign? (I know, I suck at dealing with the config system) Thanks, -Christoffer
On Wednesday 21 October 2015 15:58:44 Christoffer Dall wrote: > On Wed, Oct 21, 2015 at 03:45:20PM +0200, Arnd Bergmann wrote: > > On Tuesday 20 October 2015 15:51:05 Paolo Bonzini wrote: > > > Should this be "select" or "depends on"? Not a blocker, can always be fixed in 4.4. > > > > We have lots of 'select ARM_GIC' in the tree for platforms that use one, using > > 'depends on' will limit KVM support to being available only if at least one > > of them is being used. > > > > The only platform I can think of that uses ARMv7ve without actually having > > a GIC is BCM2836 (Raspberry Pi 2). Can we actually run KVM on a platform > > like that? If so, 'depends on' might be better, otherwise let's stay with > > 'select'. > > Yes you can, just without the VGIC and the timer - you have to emulate > that in userspace. Samsung also has a broken platform where they > integrated things incorrectly, so you cannot use the VGIC, but that > platform support is out of tree, so I can't see if it uses the GIC in > general or not. Ok, my patch should be fine then. > I'm a bit confused why using 'depends on' in this case helps anythign? > > (I know, I suck at dealing with the config system) Generally speaking, 'select' causes more problems than 'depends on', in particular when you get conflicting requirements (A selects B, B depends on C, but A can be enabled without C). However, symbols that only have 'select' and no 'depends on', and also are not user-visible, are not problematic. This is the case here. Arnd
Hello! > The only platform I can think of that uses ARMv7ve without actually having > a GIC is BCM2836 (Raspberry Pi 2). Can we actually run KVM on a platform > like that? We can, with two limitations: 1. GIC has to be emulated in software. I have recently fixed support for this. The only problem here would be that KVM currently refuses to initialize if there's no vGIC, but it is easy to fix, i posted patches for this too. 2. We cannot emulate CP15 timer, because accessing virtual timer registers cannot be trapped to HYP. However, it is possible to trap physical timer access, but a small KVM API extension is needed for this. Currently it is possible to run qemu vexpress model in this mode, because it has another, memory-mapped timer. It is only necessary to either remove CP15 timer from guest device tree, or disable support in guest .config. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia
diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig index 210ecca..356970f 100644 --- a/arch/arm/kvm/Kconfig +++ b/arch/arm/kvm/Kconfig @@ -21,6 +21,7 @@ config KVM depends on MMU && OF select PREEMPT_NOTIFIERS select ANON_INODES + select ARM_GIC select HAVE_KVM_CPU_RELAX_INTERCEPT select HAVE_KVM_ARCH_TLB_FLUSH_ALL select KVM_MMIO