| Message ID | 57888eef1c5c21337ddad384250bc5f55829ac67.1448874023.git.p.fedin@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; -- 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
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 -- 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
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). -- 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
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 -- 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
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
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 -- 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
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 -- 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
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 -- 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
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;
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(-)