* [PATCH] KVM: s390: check if kernel irqchip is actually enabled
@ 2020-01-09 13:47 Cornelia Huck
2020-01-09 13:50 ` Cornelia Huck
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Cornelia Huck @ 2020-01-09 13:47 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank
Cc: David Hildenbrand, kvm, linux-s390, Cornelia Huck
On s390, we only allow userspace to create an in-kernel irqchip
if it has first enabled the KVM_CAP_S390_IRQCHIP vm capability.
Let's assume that a userspace that enabled that capability has
created an irqchip as well.
Fixes: 84223598778b ("KVM: s390: irq routing for adapter interrupts.")
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
A more precise check would be to add a field in kvm_arch that tracks
whether an irqchip has actually been created; not sure if that is
really needed.
Found while trying to hunt down QEMU crashes with kvm-irqchip=off;
this is not sufficient, though. I *think* everything but irqfds
should work without kvm-irqchip as well, but have not found the problem
yet.
---
arch/s390/kvm/irq.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/s390/kvm/irq.h b/arch/s390/kvm/irq.h
index 484608c71dd0..30e13d031379 100644
--- a/arch/s390/kvm/irq.h
+++ b/arch/s390/kvm/irq.h
@@ -13,7 +13,7 @@
static inline int irqchip_in_kernel(struct kvm *kvm)
{
- return 1;
+ return !!kvm->arch.use_irqchip;
}
#endif
--
2.21.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: s390: check if kernel irqchip is actually enabled
2020-01-09 13:47 [PATCH] KVM: s390: check if kernel irqchip is actually enabled Cornelia Huck
@ 2020-01-09 13:50 ` Cornelia Huck
2020-01-09 13:57 ` David Hildenbrand
2020-01-09 14:06 ` Christian Borntraeger
2 siblings, 0 replies; 6+ messages in thread
From: Cornelia Huck @ 2020-01-09 13:50 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank; +Cc: David Hildenbrand, kvm, linux-s390
On Thu, 9 Jan 2020 14:47:13 +0100
Cornelia Huck <cohuck@redhat.com> wrote:
> On s390, we only allow userspace to create an in-kernel irqchip
> if it has first enabled the KVM_CAP_S390_IRQCHIP vm capability.
> Let's assume that a userspace that enabled that capability has
> created an irqchip as well.
>
> Fixes: 84223598778b ("KVM: s390: irq routing for adapter interrupts.")
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>
> A more precise check would be to add a field in kvm_arch that tracks
> whether an irqchip has actually been created; not sure if that is
> really needed.
>
> Found while trying to hunt down QEMU crashes with kvm-irqchip=off;
s/kvm-irqchip/kernel_irqchip/
> this is not sufficient, though. I *think* everything but irqfds
> should work without kvm-irqchip as well, but have not found the problem
> yet.
>
> ---
> arch/s390/kvm/irq.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/kvm/irq.h b/arch/s390/kvm/irq.h
> index 484608c71dd0..30e13d031379 100644
> --- a/arch/s390/kvm/irq.h
> +++ b/arch/s390/kvm/irq.h
> @@ -13,7 +13,7 @@
>
> static inline int irqchip_in_kernel(struct kvm *kvm)
> {
> - return 1;
> + return !!kvm->arch.use_irqchip;
> }
>
> #endif
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: s390: check if kernel irqchip is actually enabled
2020-01-09 13:47 [PATCH] KVM: s390: check if kernel irqchip is actually enabled Cornelia Huck
2020-01-09 13:50 ` Cornelia Huck
@ 2020-01-09 13:57 ` David Hildenbrand
2020-01-09 16:46 ` Cornelia Huck
2020-01-09 14:06 ` Christian Borntraeger
2 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2020-01-09 13:57 UTC (permalink / raw)
To: Cornelia Huck, Christian Borntraeger, Janosch Frank; +Cc: kvm, linux-s390
On 09.01.20 14:47, Cornelia Huck wrote:
> On s390, we only allow userspace to create an in-kernel irqchip
> if it has first enabled the KVM_CAP_S390_IRQCHIP vm capability.
> Let's assume that a userspace that enabled that capability has
> created an irqchip as well.
>
> Fixes: 84223598778b ("KVM: s390: irq routing for adapter interrupts.")
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>
> A more precise check would be to add a field in kvm_arch that tracks
> whether an irqchip has actually been created; not sure if that is
> really needed.
>
> Found while trying to hunt down QEMU crashes with kvm-irqchip=off;
> this is not sufficient, though. I *think* everything but irqfds
> should work without kvm-irqchip as well, but have not found the problem
> yet.
>
> ---
> arch/s390/kvm/irq.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/kvm/irq.h b/arch/s390/kvm/irq.h
> index 484608c71dd0..30e13d031379 100644
> --- a/arch/s390/kvm/irq.h
> +++ b/arch/s390/kvm/irq.h
> @@ -13,7 +13,7 @@
>
> static inline int irqchip_in_kernel(struct kvm *kvm)
> {
> - return 1;
> + return !!kvm->arch.use_irqchip;
> }
>
> #endif
>
This function is used on s390x/common code only in
virt/kvm/irqchip.c:kvm_send_userspace_msi()
That function is only used with CONFIG_HAVE_KVM_MSI. That is not
selected for s390x.
What does this patch fix?
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: s390: check if kernel irqchip is actually enabled
2020-01-09 13:57 ` David Hildenbrand
@ 2020-01-09 16:46 ` Cornelia Huck
0 siblings, 0 replies; 6+ messages in thread
From: Cornelia Huck @ 2020-01-09 16:46 UTC (permalink / raw)
To: David Hildenbrand; +Cc: Christian Borntraeger, Janosch Frank, kvm, linux-s390
On Thu, 9 Jan 2020 14:57:43 +0100
David Hildenbrand <david@redhat.com> wrote:
> On 09.01.20 14:47, Cornelia Huck wrote:
> > On s390, we only allow userspace to create an in-kernel irqchip
> > if it has first enabled the KVM_CAP_S390_IRQCHIP vm capability.
> > Let's assume that a userspace that enabled that capability has
> > created an irqchip as well.
> >
> > Fixes: 84223598778b ("KVM: s390: irq routing for adapter interrupts.")
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >
> > A more precise check would be to add a field in kvm_arch that tracks
> > whether an irqchip has actually been created; not sure if that is
> > really needed.
> >
> > Found while trying to hunt down QEMU crashes with kvm-irqchip=off;
> > this is not sufficient, though. I *think* everything but irqfds
> > should work without kvm-irqchip as well, but have not found the problem
> > yet.
> >
> > ---
> > arch/s390/kvm/irq.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/s390/kvm/irq.h b/arch/s390/kvm/irq.h
> > index 484608c71dd0..30e13d031379 100644
> > --- a/arch/s390/kvm/irq.h
> > +++ b/arch/s390/kvm/irq.h
> > @@ -13,7 +13,7 @@
> >
> > static inline int irqchip_in_kernel(struct kvm *kvm)
> > {
> > - return 1;
> > + return !!kvm->arch.use_irqchip;
> > }
> >
> > #endif
> >
>
> This function is used on s390x/common code only in
>
> virt/kvm/irqchip.c:kvm_send_userspace_msi()
>
> That function is only used with CONFIG_HAVE_KVM_MSI. That is not
> selected for s390x.
>
> What does this patch fix?
Currently, only future callers... but maybe this should be an optional
function instead that is only defined by the architectures that can
actually do something useful here? TBH, I'm not completely sure what
this function is actually supposed to indicate...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: s390: check if kernel irqchip is actually enabled
2020-01-09 13:47 [PATCH] KVM: s390: check if kernel irqchip is actually enabled Cornelia Huck
2020-01-09 13:50 ` Cornelia Huck
2020-01-09 13:57 ` David Hildenbrand
@ 2020-01-09 14:06 ` Christian Borntraeger
2020-01-09 16:49 ` Cornelia Huck
2 siblings, 1 reply; 6+ messages in thread
From: Christian Borntraeger @ 2020-01-09 14:06 UTC (permalink / raw)
To: Cornelia Huck, Janosch Frank; +Cc: David Hildenbrand, kvm, linux-s390
On 09.01.20 14:47, Cornelia Huck wrote:
> On s390, we only allow userspace to create an in-kernel irqchip
> if it has first enabled the KVM_CAP_S390_IRQCHIP vm capability.
> Let's assume that a userspace that enabled that capability has
> created an irqchip as well.
>
> Fixes: 84223598778b ("KVM: s390: irq routing for adapter interrupts.")
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>
> A more precise check would be to add a field in kvm_arch that tracks
> whether an irqchip has actually been created; not sure if that is
> really needed.
I think this is semantically wrong. We always have in-kernel irq handling.
It is actually not possible to not use it. So I understand where you are coming
from but this feels kind of wrong.
>
> Found while trying to hunt down QEMU crashes with kvm-irqchip=off;
> this is not sufficient, though. I *think* everything but irqfds
> should work without kvm-irqchip as well, but have not found the problem
> yet.
>
> ---
> arch/s390/kvm/irq.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/kvm/irq.h b/arch/s390/kvm/irq.h
> index 484608c71dd0..30e13d031379 100644
> --- a/arch/s390/kvm/irq.h
> +++ b/arch/s390/kvm/irq.h
> @@ -13,7 +13,7 @@
>
> static inline int irqchip_in_kernel(struct kvm *kvm)
> {
> - return 1;
> + return !!kvm->arch.use_irqchip;
> }
>
> #endif
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: s390: check if kernel irqchip is actually enabled
2020-01-09 14:06 ` Christian Borntraeger
@ 2020-01-09 16:49 ` Cornelia Huck
0 siblings, 0 replies; 6+ messages in thread
From: Cornelia Huck @ 2020-01-09 16:49 UTC (permalink / raw)
To: Christian Borntraeger; +Cc: Janosch Frank, David Hildenbrand, kvm, linux-s390
On Thu, 9 Jan 2020 15:06:22 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> On 09.01.20 14:47, Cornelia Huck wrote:
> > On s390, we only allow userspace to create an in-kernel irqchip
> > if it has first enabled the KVM_CAP_S390_IRQCHIP vm capability.
> > Let's assume that a userspace that enabled that capability has
> > created an irqchip as well.
> >
> > Fixes: 84223598778b ("KVM: s390: irq routing for adapter interrupts.")
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >
> > A more precise check would be to add a field in kvm_arch that tracks
> > whether an irqchip has actually been created; not sure if that is
> > really needed.
>
> I think this is semantically wrong. We always have in-kernel irq handling.
> It is actually not possible to not use it. So I understand where you are coming
> from but this feels kind of wrong.
You cannot actually call create_irqchip if not enabled, though... as I
said in my other reply, the intended semantics here are a bit unclear.
>
> >
> > Found while trying to hunt down QEMU crashes with kvm-irqchip=off;
> > this is not sufficient, though. I *think* everything but irqfds
> > should work without kvm-irqchip as well, but have not found the problem
> > yet.
> >
> > ---
> > arch/s390/kvm/irq.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/s390/kvm/irq.h b/arch/s390/kvm/irq.h
> > index 484608c71dd0..30e13d031379 100644
> > --- a/arch/s390/kvm/irq.h
> > +++ b/arch/s390/kvm/irq.h
> > @@ -13,7 +13,7 @@
> >
> > static inline int irqchip_in_kernel(struct kvm *kvm)
> > {
> > - return 1;
> > + return !!kvm->arch.use_irqchip;
> > }
> >
> > #endif
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-09 16:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-09 13:47 [PATCH] KVM: s390: check if kernel irqchip is actually enabled Cornelia Huck
2020-01-09 13:50 ` Cornelia Huck
2020-01-09 13:57 ` David Hildenbrand
2020-01-09 16:46 ` Cornelia Huck
2020-01-09 14:06 ` Christian Borntraeger
2020-01-09 16:49 ` Cornelia Huck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox