[PATCH v5 0/2] KVM: arm/arm64: Allow to use KVM without in-kernel irqchip

[PATCH v5 0/2] KVM: arm/arm64: Allow to use KVM without in-kernel irqchip

[Pavel Fedin]

This patch set brings back functionality which was broken in v4.0.
Unfortunately, currently it is impossible to take advantage of virtual
architected timer in this case, therefore guest, running in such
restricted mode, has to use some memory-mapped timer. But it is still
better than nothing.

Patch 0002 needs to be verified on PowerPC architecture, because i've
got an impression that KVM_CAP_IRQCHIP is forgotten there.

v4 => v5:
- Tested on top of kvmarm/next
- Dropped already applied part
- Fixed minor checkpatch issues

v3 => v4:
- Revert back to using switch on kvm_vgic_hyp_init() return code. I decided
  to leave 'vgic_present = false' statement because it helps to understand
  the code.

v2 => v3:
- Improved commit messages, added references to commits where the respective
  functionality was broken
- Explicitly specify that the solution currently affects only vGIC and has
  nothing to do with timer.
- Fixed code style according to previous notes
- Removed ARM64 save/restore patch introduced in v2 because it was already
  obsolete for linux-next
- Modify KVM_CAP_IRQFD handling in correct place

v1 => v2:
- Do not use defensive approach in patch 0001. Use correct conditions in
  callers instead
- Added ARM64-specific code, without which attempt to run a VM ends in a
  HYP crash because of unset vGIC save/restore function pointers



Pavel Fedin (2):
  arm/arm64: KVM: Detect vGIC presence at runtime
  KVM: Make KVM_CAP_IRQFD dependent on KVM_CAP_IRQCHIP

 arch/arm/kvm/arm.c  | 22 ++++++++++++++++++++--
 virt/kvm/kvm_main.c |  6 ++++--
 2 files changed, 24 insertions(+), 4 deletions(-)

-- 
2.4.4

[PATCH v5 1/2] arm/arm64: KVM: Detect vGIC presence at runtime

[Pavel Fedin]

Before commit 662d9715840aef44dcb573b0f9fab9e8319c868a
("arm/arm64: KVM: Kill CONFIG_KVM_ARM_{VGIC,TIMER}") is was possible to
compile the kernel without vGIC and vTimer support. Commit message says
about possibility to detect vGIC support in runtime, but this has never
been implemented.

This patch introdices runtime check, restoring the lost functionality.
It again allows to use KVM on hardware without vGIC. Interrupt
controller has to be emulated in userspace in this case.

-ENODEV return code from probe function means there's no GIC at all.
-ENXIO happens when, for example, there is GIC node in the device tree,
but it does not specify vGIC resources. Normally this means that vGIC
hardware is defunct. Any other error code is still treated as full stop
because it might mean some really serious problems.

This patch does not touch any virtual timer code, suggesting that timer
hardware is actually in place. Normally on boards in question it is true,
however since vGIC is missing, it is impossible to correctly utilize
interrupts from the virtual timer. Since virtual timer handling is in
active redevelopment now, handling in it userspace is out of scope at
the moment. The guest is currently suggested to use some memory-mapped
timer which can be emulated in userspace.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 arch/arm/kvm/arm.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index eab83b2..d581756 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -61,6 +61,8 @@ static atomic64_t kvm_vmid_gen = ATOMIC64_INIT(1);
 static u8 kvm_next_vmid;
 static DEFINE_SPINLOCK(kvm_vmid_lock);
 
+static bool vgic_present;
+
 static void kvm_arm_set_running_vcpu(struct kvm_vcpu *vcpu)
 {
 	BUG_ON(preemptible());
@@ -132,7 +134,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	kvm->arch.vmid_gen = 0;
 
 	/* The maximum number of VCPUs is limited by the host's GIC model */
-	kvm->arch.max_vcpus = kvm_vgic_get_max_vcpus();
+	kvm->arch.max_vcpus = vgic_present ?
+				kvm_vgic_get_max_vcpus() : KVM_MAX_VCPUS;
 
 	return ret;
 out_free_stage2_pgd:
@@ -172,6 +175,8 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	int r;
 	switch (ext) {
 	case KVM_CAP_IRQCHIP:
+		r = vgic_present;
+		break;
 	case KVM_CAP_IOEVENTFD:
 	case KVM_CAP_DEVICE_CTRL:
 	case KVM_CAP_USER_MEMORY:
@@ -918,6 +923,8 @@ static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
 
 	switch (dev_id) {
 	case KVM_ARM_DEVICE_VGIC_V2:
+		if (!vgic_present)
+			return -ENXIO;
 		return kvm_vgic_addr(kvm, type, &dev_addr->addr, true);
 	default:
 		return -ENODEV;
@@ -932,6 +939,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 
 	switch (ioctl) {
 	case KVM_CREATE_IRQCHIP: {
+		if (!vgic_present)
+			return -ENXIO;
 		return kvm_vgic_create(kvm, KVM_DEV_TYPE_ARM_VGIC_V2);
 	}
 	case KVM_ARM_SET_DEVICE_ADDR: {
@@ -1116,8 +1125,17 @@ static int init_hyp_mode(void)
 	 * Init HYP view of VGIC
 	 */
 	err = kvm_vgic_hyp_init();
-	if (err)
+	switch (err) {
+	case 0:
+		vgic_present = true;
+		break;
+	case -ENODEV:
+	case -ENXIO:
+		vgic_present = false;
+		break;
+	default:
 		goto out_free_context;
+	}
 
 	/*
 	 * Init HYP architected timer support
-- 
2.4.4

Re: [PATCH v5 1/2] arm/arm64: KVM: Detect vGIC presence at runtime

[Alexander Graf]

On 30.11.15 10:40, Pavel Fedin wrote:
> Before commit 662d9715840aef44dcb573b0f9fab9e8319c868a
> ("arm/arm64: KVM: Kill CONFIG_KVM_ARM_{VGIC,TIMER}") is was possible to
> compile the kernel without vGIC and vTimer support. Commit message says
> about possibility to detect vGIC support in runtime, but this has never
> been implemented.
> 
> This patch introdices runtime check, restoring the lost functionality.
> It again allows to use KVM on hardware without vGIC. Interrupt
> controller has to be emulated in userspace in this case.
> 
> -ENODEV return code from probe function means there's no GIC at all.
> -ENXIO happens when, for example, there is GIC node in the device tree,
> but it does not specify vGIC resources. Normally this means that vGIC
> hardware is defunct. Any other error code is still treated as full stop
> because it might mean some really serious problems.
> 
> This patch does not touch any virtual timer code, suggesting that timer
> hardware is actually in place. Normally on boards in question it is true,
> however since vGIC is missing, it is impossible to correctly utilize
> interrupts from the virtual timer. Since virtual timer handling is in

So effectively all we'd need is to set CNTHCTL_EL2.EL1PCEN to 0 for
guests that have no in-kernel irqchip, no? We should then trap on all
timer accesses and be able to emulate them in user space where we can
inject IRQs into an emulated GIC again.


Alex

Re: [PATCH v5 1/2] arm/arm64: KVM: Detect vGIC presence at runtime

[Peter Maydell]

On 21 April 2016 at 22:41, Alexander Graf <agraf@suse.de> wrote:
> So effectively all we'd need is to set CNTHCTL_EL2.EL1PCEN to 0 for
> guests that have no in-kernel irqchip, no? We should then trap on all
> timer accesses and be able to emulate them in user space where we can
> inject IRQs into an emulated GIC again.

You'd still need to define a new userspace ABI for "we stopped
for a system register trap" and the userspace code to emulate
the timers as part of KVM rather than as part of TCG, which seems
like a lot of effort for a mode that you really don't want to be
using...

For GICv3 it gets trickier again because the interface between
the interrupt controller and the CPU is no longer as simple
as "an FIQ line and an IRQ line". (In particular the interrupt
controller needs to know the CPU's current exception level and
security state to know if it should raise IRQ or FIQ, which means
that it needs to be told every time the CPU changes EL. I haven't
yet figured out if I should model this in the QEMU emulated GICv3
by having a backdoor callback function for this or by biting
the bullet and putting the GICv3 cpu interface really in the
CPU with a properly modelled equivalent of the stream protocol...)

thanks
-- PMM

Re: [PATCH v5 1/2] arm/arm64: KVM: Detect vGIC presence at runtime

[Alexander Graf]

On 22.04.16 00:04, Peter Maydell wrote:
> On 21 April 2016 at 22:41, Alexander Graf <agraf@suse.de> wrote:
>> So effectively all we'd need is to set CNTHCTL_EL2.EL1PCEN to 0 for
>> guests that have no in-kernel irqchip, no? We should then trap on all
>> timer accesses and be able to emulate them in user space where we can
>> inject IRQs into an emulated GIC again.
> 
> You'd still need to define a new userspace ABI for "we stopped
> for a system register trap" and the userspace code to emulate
> the timers as part of KVM rather than as part of TCG, which seems
> like a lot of effort for a mode that you really don't want to be
> using...

It might make sense to have a generic "system register couldn't get
handled" exit code anyway. If nothing else, at least to display
unhandled registers in the qemu context where they appear rather than in
the kernel log (which a guest could deliberately clutter as it stands
today).

With such an interface in place, the rest would only be a few lines of glue.

> For GICv3 it gets trickier again because the interface between
> the interrupt controller and the CPU is no longer as simple
> as "an FIQ line and an IRQ line". (In particular the interrupt
> controller needs to know the CPU's current exception level and
> security state to know if it should raise IRQ or FIQ, which means
> that it needs to be told every time the CPU changes EL. I haven't
> yet figured out if I should model this in the QEMU emulated GICv3
> by having a backdoor callback function for this or by biting
> the bullet and putting the GICv3 cpu interface really in the
> CPU with a properly modelled equivalent of the stream protocol...)

We moved the lapic into target-i386 as well, no? Given that it really is
very close to the cpu these days it might not be a bad idea.


Alex

Re: [PATCH v5 1/2] arm/arm64: KVM: Detect vGIC presence at runtime

[Peter Maydell]

On 21 April 2016 at 23:35, Alexander Graf <agraf@suse.de> wrote:
> It might make sense to have a generic "system register couldn't get
> handled" exit code anyway. If nothing else, at least to display
> unhandled registers in the qemu context where they appear rather than in
> the kernel log (which a guest could deliberately clutter as it stands
> today).

Unhandled sysregs UNDEF so they get logged by the guest if anywhere,
I think.

thanks
-- PMM

Re: [PATCH v5 1/2] arm/arm64: KVM: Detect vGIC presence at runtime

[Marc Zyngier]

On 21/04/16 22:41, Alexander Graf wrote:
> 
> 
> On 30.11.15 10:40, Pavel Fedin wrote:
>> Before commit 662d9715840aef44dcb573b0f9fab9e8319c868a
>> ("arm/arm64: KVM: Kill CONFIG_KVM_ARM_{VGIC,TIMER}") is was possible to
>> compile the kernel without vGIC and vTimer support. Commit message says
>> about possibility to detect vGIC support in runtime, but this has never
>> been implemented.
>>
>> This patch introdices runtime check, restoring the lost functionality.
>> It again allows to use KVM on hardware without vGIC. Interrupt
>> controller has to be emulated in userspace in this case.
>>
>> -ENODEV return code from probe function means there's no GIC at all.
>> -ENXIO happens when, for example, there is GIC node in the device tree,
>> but it does not specify vGIC resources. Normally this means that vGIC
>> hardware is defunct. Any other error code is still treated as full stop
>> because it might mean some really serious problems.
>>
>> This patch does not touch any virtual timer code, suggesting that timer
>> hardware is actually in place. Normally on boards in question it is true,
>> however since vGIC is missing, it is impossible to correctly utilize
>> interrupts from the virtual timer. Since virtual timer handling is in
> 
> So effectively all we'd need is to set CNTHCTL_EL2.EL1PCEN to 0 for
> guests that have no in-kernel irqchip, no? We should then trap on all
> timer accesses and be able to emulate them in user space where we can
> inject IRQs into an emulated GIC again.

That's for the counter. The timer is already trapped.

That very nice, until you realize that Linux guests use the virtual
timer, not the physical one. Yes, you can hack that. And at that point,
you might as well have a different timer altogether, which solves the
problem entirely, without having to forward anything to userspace.

Thanks,

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

[PATCH v5 2/2] KVM: Make KVM_CAP_IRQFD dependent on KVM_CAP_IRQCHIP

[Pavel Fedin]

Now at least ARM is able to determine whether the machine has
virtualization support for irqchip or not at runtime. Obviously,
irqfd requires irqchip.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 virt/kvm/kvm_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 7873d6d..a057d5e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2716,13 +2716,15 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
 	case KVM_CAP_INTERNAL_ERROR_DATA:
 #ifdef CONFIG_HAVE_KVM_MSI
 	case KVM_CAP_SIGNAL_MSI:
+		/* Fallthrough */
 #endif
+	case KVM_CAP_CHECK_EXTENSION_VM:
+		return 1;
 #ifdef CONFIG_HAVE_KVM_IRQFD
 	case KVM_CAP_IRQFD:
 	case KVM_CAP_IRQFD_RESAMPLE:
+		return kvm_vm_ioctl_check_extension(kvm, KVM_CAP_IRQCHIP);
 #endif
-	case KVM_CAP_CHECK_EXTENSION_VM:
-		return 1;
 #ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
 	case KVM_CAP_IRQ_ROUTING:
 		return KVM_MAX_IRQ_ROUTES;
-- 
2.4.4

Re: [PATCH v5 2/2] KVM: Make KVM_CAP_IRQFD dependent on KVM_CAP_IRQCHIP

[Cornelia Huck]

On Mon, 30 Nov 2015 12:40:45 +0300
Pavel Fedin <p.fedin@samsung.com> wrote:

> Now at least ARM is able to determine whether the machine has
> virtualization support for irqchip or not at runtime. Obviously,
> irqfd requires irqchip.
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> ---
>  virt/kvm/kvm_main.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 7873d6d..a057d5e 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2716,13 +2716,15 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
>  	case KVM_CAP_INTERNAL_ERROR_DATA:
>  #ifdef CONFIG_HAVE_KVM_MSI
>  	case KVM_CAP_SIGNAL_MSI:
> +		/* Fallthrough */
>  #endif
> +	case KVM_CAP_CHECK_EXTENSION_VM:
> +		return 1;
>  #ifdef CONFIG_HAVE_KVM_IRQFD
>  	case KVM_CAP_IRQFD:
>  	case KVM_CAP_IRQFD_RESAMPLE:
> +		return kvm_vm_ioctl_check_extension(kvm, KVM_CAP_IRQCHIP);

This won't work for s390, as it doesn't have KVM_CAP_IRQCHIP but
KVM_CAP_S390_IRQCHIP (which needs to be enabled).

>  #endif
> -	case KVM_CAP_CHECK_EXTENSION_VM:
> -		return 1;
>  #ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
>  	case KVM_CAP_IRQ_ROUTING:
>  		return KVM_MAX_IRQ_ROUTES;

RE: [PATCH v5 2/2] KVM: Make KVM_CAP_IRQFD dependent on KVM_CAP_IRQCHIP

[Pavel Fedin]

 Hello!

> >  	case KVM_CAP_INTERNAL_ERROR_DATA:
> >  #ifdef CONFIG_HAVE_KVM_MSI
> >  	case KVM_CAP_SIGNAL_MSI:
> > +		/* Fallthrough */
> >  #endif
> > +	case KVM_CAP_CHECK_EXTENSION_VM:
> > +		return 1;
> >  #ifdef CONFIG_HAVE_KVM_IRQFD
> >  	case KVM_CAP_IRQFD:
> >  	case KVM_CAP_IRQFD_RESAMPLE:
> > +		return kvm_vm_ioctl_check_extension(kvm, KVM_CAP_IRQCHIP);
> 
> This won't work for s390, as it doesn't have KVM_CAP_IRQCHIP but
> KVM_CAP_S390_IRQCHIP (which needs to be enabled).

 Thank you for the note, i didn't know about irqchip-specific capability codes. There's the same issue with PowerPC, now i
understand why there's no KVM_CAP_IRQCHIP for them. Because they have KVM_CAP_IRQ_MPIC and KVM_CAP_IRQ_XICS, similar to S390.
 But isn't it just weird? I understand that perhaps we have some real need to distinguish between different irqchip types, but
shouldn't the kernel also publish KVM_CAP_IRQCHIP, which stands just for "we support some irqchip virtualization"?
 May be we should just add this for PowerPC and S390, to make things less ambiguous?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [PATCH v5 2/2] KVM: Make KVM_CAP_IRQFD dependent on KVM_CAP_IRQCHIP

[Cornelia Huck]

On Mon, 30 Nov 2015 14:56:38 +0300
Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
> 
> > >  	case KVM_CAP_INTERNAL_ERROR_DATA:
> > >  #ifdef CONFIG_HAVE_KVM_MSI
> > >  	case KVM_CAP_SIGNAL_MSI:
> > > +		/* Fallthrough */
> > >  #endif
> > > +	case KVM_CAP_CHECK_EXTENSION_VM:
> > > +		return 1;
> > >  #ifdef CONFIG_HAVE_KVM_IRQFD
> > >  	case KVM_CAP_IRQFD:
> > >  	case KVM_CAP_IRQFD_RESAMPLE:
> > > +		return kvm_vm_ioctl_check_extension(kvm, KVM_CAP_IRQCHIP);
> > 
> > This won't work for s390, as it doesn't have KVM_CAP_IRQCHIP but
> > KVM_CAP_S390_IRQCHIP (which needs to be enabled).
> 
>  Thank you for the note, i didn't know about irqchip-specific capability codes. There's the same issue with PowerPC, now i
> understand why there's no KVM_CAP_IRQCHIP for them. Because they have KVM_CAP_IRQ_MPIC and KVM_CAP_IRQ_XICS, similar to S390.
>  But isn't it just weird? I understand that perhaps we have some real need to distinguish between different irqchip types, but
> shouldn't the kernel also publish KVM_CAP_IRQCHIP, which stands just for "we support some irqchip virtualization"?
>  May be we should just add this for PowerPC and S390, to make things less ambiguous?

Note that we explicitly need to _enable_ the s390 cap (for
compatibility). I'd need to recall the exact details but I came to the
conclusion back than that I could not simply enable KVM_CAP_IRQCHIP for
s390 (and current qemu would fail to enable the s390 cap if we started
advertising KVM_CAP_IRQCHIP now).

RE: [PATCH v5 2/2] KVM: Make KVM_CAP_IRQFD dependent on KVM_CAP_IRQCHIP

[Pavel Fedin]

 Hello!

> >  Thank you for the note, i didn't know about irqchip-specific capability codes. There's the
> > same issue with PowerPC, now i
> > understand why there's no KVM_CAP_IRQCHIP for them. Because they have KVM_CAP_IRQ_MPIC and
> > KVM_CAP_IRQ_XICS, similar to S390.
> >  But isn't it just weird? I understand that perhaps we have some real need to distinguish
> > between different irqchip types, but
> > shouldn't the kernel also publish KVM_CAP_IRQCHIP, which stands just for "we support some
> > irqchip virtualization"?
> >  May be we should just add this for PowerPC and S390, to make things less ambiguous?
> 
> Note that we explicitly need to _enable_ the s390 cap (for
> compatibility). I'd need to recall the exact details but I came to the
> conclusion back than that I could not simply enable KVM_CAP_IRQCHIP for
> s390 (and current qemu would fail to enable the s390 cap if we started
> advertising KVM_CAP_IRQCHIP now).

 OMG... I've looked at the code, what a mess...
 If i was implementing this, i'd simply introduce kvm_vm_enable_cap(s, KVM_CAP_IRQCHIP, 0),
which would be allowed to fail with -ENOSYS, so that backwards compatibility is kept and an existing API is reused... But, well,
it's already impossible to unscramble an egg... :)
 Ok, i think in current situation we could choose one of these ways (both are based on the fact that it's obvious that irqfd require
IRQCHIP).
 a) I look for an alternate way to report KVM_CAP_IRQFD dynamically, and maybe PowerPC and S390 follow this way.
 b) I simply drop it as it is, because current qemu knows about the dependency and does not try to use irqfd without irqchip,
because there's simply no use for them. But, well, perhaps there would be an exception in vhost, i don't remember testing it.
 So what shall we do?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [PATCH v5 2/2] KVM: Make KVM_CAP_IRQFD dependent on KVM_CAP_IRQCHIP

[Cornelia Huck]

On Mon, 30 Nov 2015 15:41:20 +0300
Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
> 
> > >  Thank you for the note, i didn't know about irqchip-specific capability codes. There's the
> > > same issue with PowerPC, now i
> > > understand why there's no KVM_CAP_IRQCHIP for them. Because they have KVM_CAP_IRQ_MPIC and
> > > KVM_CAP_IRQ_XICS, similar to S390.
> > >  But isn't it just weird? I understand that perhaps we have some real need to distinguish
> > > between different irqchip types, but
> > > shouldn't the kernel also publish KVM_CAP_IRQCHIP, which stands just for "we support some
> > > irqchip virtualization"?
> > >  May be we should just add this for PowerPC and S390, to make things less ambiguous?
> > 
> > Note that we explicitly need to _enable_ the s390 cap (for
> > compatibility). I'd need to recall the exact details but I came to the
> > conclusion back than that I could not simply enable KVM_CAP_IRQCHIP for
> > s390 (and current qemu would fail to enable the s390 cap if we started
> > advertising KVM_CAP_IRQCHIP now).
> 
>  OMG... I've looked at the code, what a mess...
>  If i was implementing this, i'd simply introduce kvm_vm_enable_cap(s, KVM_CAP_IRQCHIP, 0),
> which would be allowed to fail with -ENOSYS, so that backwards compatibility is kept and an existing API is reused... But, well,
> it's already impossible to unscramble an egg... :)
>  Ok, i think in current situation we could choose one of these ways (both are based on the fact that it's obvious that irqfd require
> IRQCHIP).
>  a) I look for an alternate way to report KVM_CAP_IRQFD dynamically, and maybe PowerPC and S390 follow this way.

The thing is: _when_ can you report KVM_CAP_IRQFD? It obviously
requires an irqchip; but if you need some configuration/enablement
beforehand, you'll get different values depending on when you retrieve
the cap. So does KVM_CAP_IRQFD mean "irqfds are available in principle"
or "everything has been setup for usage of irqfds"? I'd assume the
former.

>  b) I simply drop it as it is, because current qemu knows about the dependency and does not try to use irqfd without irqchip,
> because there's simply no use for them. But, well, perhaps there would be an exception in vhost, i don't remember testing it.

Wouldn't an irqfd emulation cover vhost?

>  So what shall we do?

RE: [PATCH v5 2/2] KVM: Make KVM_CAP_IRQFD dependent on KVM_CAP_IRQCHIP

[Pavel Fedin]

 Hello!

> >  b) I simply drop it as it is, because current qemu knows about the dependency and does not
> try to use irqfd without irqchip,
> > because there's simply no use for them. But, well, perhaps there would be an exception in
> vhost, i don't remember testing it.
> 
> Wouldn't an irqfd emulation cover vhost?

 Of course it would. At least it should, but perhaps will need some minor tweaks.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

RE: [PATCH v5 2/2] KVM: Make KVM_CAP_IRQFD dependent on KVM_CAP_IRQCHIP

[Pavel Fedin]

 Hello!

> >  b) I simply drop it as it is, because current qemu knows about the dependency and does not
> try to use irqfd without irqchip,
> > because there's simply no use for them. But, well, perhaps there would be an exception in
> vhost, i don't remember testing it.
> 
> Wouldn't an irqfd emulation cover vhost?

 I've just tested, and no, it does not cause any problems with qemu. It happens to correctly detect that the whole thing is not
running and falls back to not using vhost. This is output from my qemu:
--- cut ---
2015-12-01T11:03:16.135724Z qemu-system-arm: Error binding guest notifier: 11
2015-12-01T11:03:16.135849Z qemu-system-arm: unable to start vhost net: 11: falling back on userspace virtio
--- cut ---

 So, the resume is: we just drop this patch and only N1 remains.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


> -----Original Message-----
> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On Behalf Of Cornelia Huck
> Sent: Monday, November 30, 2015 5:38 PM
> To: Pavel Fedin
> Cc: kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org; 'Marc Zyngier'; 'Christoffer Dall';
> 'Gleb Natapov'; 'Paolo Bonzini'
> Subject: Re: [PATCH v5 2/2] KVM: Make KVM_CAP_IRQFD dependent on KVM_CAP_IRQCHIP
> 
> On Mon, 30 Nov 2015 15:41:20 +0300
> Pavel Fedin <p.fedin@samsung.com> wrote:
> 
> >  Hello!
> >
> > > >  Thank you for the note, i didn't know about irqchip-specific capability codes. There's
> the
> > > > same issue with PowerPC, now i
> > > > understand why there's no KVM_CAP_IRQCHIP for them. Because they have KVM_CAP_IRQ_MPIC
> and
> > > > KVM_CAP_IRQ_XICS, similar to S390.
> > > >  But isn't it just weird? I understand that perhaps we have some real need to
> distinguish
> > > > between different irqchip types, but
> > > > shouldn't the kernel also publish KVM_CAP_IRQCHIP, which stands just for "we support
> some
> > > > irqchip virtualization"?
> > > >  May be we should just add this for PowerPC and S390, to make things less ambiguous?
> > >
> > > Note that we explicitly need to _enable_ the s390 cap (for
> > > compatibility). I'd need to recall the exact details but I came to the
> > > conclusion back than that I could not simply enable KVM_CAP_IRQCHIP for
> > > s390 (and current qemu would fail to enable the s390 cap if we started
> > > advertising KVM_CAP_IRQCHIP now).
> >
> >  OMG... I've looked at the code, what a mess...
> >  If i was implementing this, i'd simply introduce kvm_vm_enable_cap(s, KVM_CAP_IRQCHIP, 0),
> > which would be allowed to fail with -ENOSYS, so that backwards compatibility is kept and an
> existing API is reused... But, well,
> > it's already impossible to unscramble an egg... :)
> >  Ok, i think in current situation we could choose one of these ways (both are based on the
> fact that it's obvious that irqfd require
> > IRQCHIP).
> >  a) I look for an alternate way to report KVM_CAP_IRQFD dynamically, and maybe PowerPC and
> S390 follow this way.
> 
> The thing is: _when_ can you report KVM_CAP_IRQFD? It obviously
> requires an irqchip; but if you need some configuration/enablement
> beforehand, you'll get different values depending on when you retrieve
> the cap. So does KVM_CAP_IRQFD mean "irqfds are available in principle"
> or "everything has been setup for usage of irqfds"? I'd assume the
> former.
> 
> >  b) I simply drop it as it is, because current qemu knows about the dependency and does not
> try to use irqfd without irqchip,
> > because there's simply no use for them. But, well, perhaps there would be an exception in
> vhost, i don't remember testing it.
> 
> Wouldn't an irqfd emulation cover vhost?
> 
> >  So what shall we do?
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] KVM: Make KVM_CAP_IRQFD dependent on KVM_CAP_IRQCHIP

[Paolo Bonzini]

On 30/11/2015 15:38, Cornelia Huck wrote:
> It obviously
> requires an irqchip; but if you need some configuration/enablement
> beforehand, you'll get different values depending on when you retrieve
> the cap. So does KVM_CAP_IRQFD mean "irqfds are available in principle"
> or "everything has been setup for usage of irqfds"? I'd assume the
> former.

It should be the former, yes.

Paolo