Re: [PATCH] ARM: KVM: iterate over all CPUs for CPU compatibility check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






On Wed, Apr 17, 2013 at 1:01 AM, Andre Przywara <andre.przywara@xxxxxxxxxx> wrote:
On 04/16/2013 06:24 PM, Christoffer Dall wrote:
On Mon, Apr 15, 2013 at 5:26 PM, Geoff Levand <geoff@xxxxxxxxxxxxx> wrote:
On Sun, 2013-04-14 at 21:57 -0700, Christoffer Dall wrote:
On Fri, Apr 12, 2013 at 6:24 AM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
Nak. The fact that one of the CPUs seem to hang is a sure sign that
something is severely broken, and you definitely want to fix that issue,
instead of blindly ignoring it.

Additionally, it seems you're just papering over the issue. You should
be able to exclude the A7 processors, but not completely deny KVM from
running on the hardware.

Marc, I disagree with this nak. If the current kernel breaks boot on a
Big.Little system, we need to take care of that first, and the
proposed patch is a quick way to do so, and it does not stand in the
way of introducing proper Big.Little support in any way, which I'm
sure is going to open up a lot of other interesting questions.

I'm going to take this one.

Since this problem will cause the 3.9 kernel to hang then a workaround
like this should go in.  There isn't enough time to do a proper fix for
3.9, and even if it could be done I think it would be too intrusive to
get merged this late.

That's why I was inclined to take the patch, but as Marc pointed out
the error message is incorrect, so that should be fixed at the very
least. Also I don't think we need the counting logic, just bail out if
we have any CPUs that are not supported.

The counting logic is just to prevent an erroneous hint message. If Cluster 0 is A7, the first CPU checked will fail, the counter is 0 and KVM outputs the "Target CPU not supported!" message.
But if at least one CPU already passed the test, we have a) a b.L system and b) restricting the number of CPUs with maxcpus= would help.
Thus the hint at this point.

I know, but you can't be sure the kernel won't run on some other system having a different configuration, Krait + something else for example, and the error message then makes some wrong assumptions.  Also, I'm not convinced that the first cpus as specified by maxcpus will always be the A15's in which case the error message is also not correct.

Combining this with the somewhat unnecessarily complicated code tells me that this should just be removed all together, in any case.
 

But I am OK with removing both the hint message and the counting altogether, if you want to have an easier patch.

Regards,
Andre.

Marc, since you're the strongest opponent of this patch, are you still
opposed to making sure we don't try to run KVM on Big.Little until
support is properly introduced?

I also cannot see how we can fix the affinity issue easily from within
the kernel, do you have a concrete approach in mind you can share?

Note that this is irrespective of the boot delay issue that you guys
are observing.

-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm



_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux