Hi,
Today's mainline kernel failed to build on powerpc with below error
arch/powerpc/kvm/../../../virt/kvm/eventfd.c: In function ‘kvm_irqfd’:
arch/powerpc/kvm/../../../virt/kvm/eventfd.c:568:19: error:
‘KVM_MAX_IRQ_ROUTES’ undeclared (first use in this function)
if (args->gsi >= KVM_MAX_IRQ_ROUTES)
^
arch/powerpc/kvm/../../../virt/kvm/eventfd.c:568:19: note: each
undeclared identifier is reported only once for each function it appears
in
make[1]: *** [arch/powerpc/kvm/../../../virt/kvm/eventfd.o] Error 1
Machine Type: Power 8 KVM Host
kernel: 4.13.0
gcc : 5.1.1
config: attached.
Possible bad commit is :
commit 36ae3c0a36b7456432fedce38ae2f7bd3e01a563
Author: Jan H. Schönherr <[email protected]>
Date: Thu Sep 7 19:02:48 2017 +0100
KVM: Don't accept obviously wrong gsi values via KVM_IRQFD
We cannot add routes for gsi values >= KVM_MAX_IRQ_ROUTES -- see
kvm_set_irq_routing(). Hence, there is no sense in accepting them
via KVM_IRQFD. Prevent them from entering the system in the first
place.
Signed-off-by: Jan H. Schönherr <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index f2ac53a..c608ab4 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -565,6 +565,8 @@ kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
{
if (args->flags & ~(KVM_IRQFD_FLAG_DEASSIGN |
KVM_IRQFD_FLAG_RESAMPLE))
return -EINVAL;
+ if (args->gsi >= KVM_MAX_IRQ_ROUTES)
+ return -EINVAL;
if (args->flags & KVM_IRQFD_FLAG_DEASSIGN)
return kvm_irqfd_deassign(kvm, args);
--
Regard's
Abdul Haleem
IBM Linux Technology Centre
On 09/16/2017 06:24 PM, Abdul Haleem wrote:
> Hi,
>
> Today's mainline kernel failed to build on powerpc with below error
>
> arch/powerpc/kvm/../../../virt/kvm/eventfd.c: In function ‘kvm_irqfd’:
> arch/powerpc/kvm/../../../virt/kvm/eventfd.c:568:19: error:
> ‘KVM_MAX_IRQ_ROUTES’ undeclared (first use in this function)
> if (args->gsi >= KVM_MAX_IRQ_ROUTES)
Hmm... the header file for that constant is included...
... but it's only defined when CONFIG_HAVE_KVM_IRQ_ROUTING, which is not the case here.
We can either make that "if" conditional on that, or we revert the commit.
The commit was meant the give user space an early indication that something is wrong.
I was only aware of the IRQ routing use case. This indicates that there are other
use cases. To err on the side of caution, I'd favor a revert as the patch is not strictly
needed -- unless someone else can provide more insight on valid values for GSI.
Paolo, do you want me to prepare a revert for the commit below?
Regards
Jan
> Possible bad commit is :
>
> commit 36ae3c0a36b7456432fedce38ae2f7bd3e01a563
> Author: Jan H. Schönherr <[email protected]>
> Date: Thu Sep 7 19:02:48 2017 +0100
>
> KVM: Don't accept obviously wrong gsi values via KVM_IRQFD
>
> We cannot add routes for gsi values >= KVM_MAX_IRQ_ROUTES -- see
> kvm_set_irq_routing(). Hence, there is no sense in accepting them
> via KVM_IRQFD. Prevent them from entering the system in the first
> place.
>
> Signed-off-by: Jan H. Schönherr <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
>
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index f2ac53a..c608ab4 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -565,6 +565,8 @@ kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
> {
> if (args->flags & ~(KVM_IRQFD_FLAG_DEASSIGN |
> KVM_IRQFD_FLAG_RESAMPLE))
> return -EINVAL;
> + if (args->gsi >= KVM_MAX_IRQ_ROUTES)
> + return -EINVAL;
>
> if (args->flags & KVM_IRQFD_FLAG_DEASSIGN)
> return kvm_irqfd_deassign(kvm, args);
>
>
This reverts commit 36ae3c0a36b7456432fedce38ae2f7bd3e01a563.
The commit broke compilation on !CONFIG_HAVE_KVM_IRQ_ROUTING. Also,
there may be cases with CONFIG_HAVE_KVM_IRQ_ROUTING, where larger
gsi values make sense.
As the commit was meant as an early indicator to user space that
something is wrong, reverting just restores the previous behavior
where overly large values are ignored when encountered (without
any direct feedback).
Reported-by: Abdul Haleem <[email protected]>
Signed-off-by: Jan H. Schönherr <[email protected]>
---
virt/kvm/eventfd.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index c608ab4..f2ac53a 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -565,8 +565,6 @@ kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
{
if (args->flags & ~(KVM_IRQFD_FLAG_DEASSIGN | KVM_IRQFD_FLAG_RESAMPLE))
return -EINVAL;
- if (args->gsi >= KVM_MAX_IRQ_ROUTES)
- return -EINVAL;
if (args->flags & KVM_IRQFD_FLAG_DEASSIGN)
return kvm_irqfd_deassign(kvm, args);
--
2.3.1.dirty
"Jan H. Schönherr" <[email protected]> writes:
> This reverts commit 36ae3c0a36b7456432fedce38ae2f7bd3e01a563.
>
> The commit broke compilation on !CONFIG_HAVE_KVM_IRQ_ROUTING. Also,
> there may be cases with CONFIG_HAVE_KVM_IRQ_ROUTING, where larger
> gsi values make sense.
>
> As the commit was meant as an early indicator to user space that
> something is wrong, reverting just restores the previous behavior
> where overly large values are ignored when encountered (without
> any direct feedback).
>
> Reported-by: Abdul Haleem <[email protected]>
> Signed-off-by: Jan H. Schönherr <[email protected]>
> ---
> virt/kvm/eventfd.c | 2 --
> 1 file changed, 2 deletions(-)
Can someone merge this or preferably just send it straight to Linus, the
original patch has broken the powerpc build in mainline.
cheers