Re: [PATCH v4 0/3] KVM: arm/arm64: Clean up some obsolete code
On 5 November 2015 at 06:50, Pavel Fedin > You know, since we are talking about this... This definitely > has something to do with the reset, and... Looks like nobody > resets vGIC/vTimer, unless the userland does it explicitly by > resetting every register by hand. This is how KVM in-kernel device reset is supposed to work, yes. thanks -- PMM -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 0/3] KVM: arm/arm64: Clean up some obsolete code
Hello! > Actually, I seem to have been just incredibly unlucky with my test > cycles, because I eventually reproduced the bug without your patches. Or lucky, without "un" :) > I'm going to take this version of the series because that's what I > reviewed and tested. It's OK, as i wrote, v5 is no different from v4 actually, just 0001 bisected. And making it was useful because it helped me to make sure once again that i haven't messed anything up. > Sorry for the noise. It's OK, thank you very much for putting efforts into testing and cooperation. You know, since we are talking about this... This definitely has something to do with the reset, and... Looks like nobody resets vGIC/vTimer, unless the userland does it explicitly by resetting every register by hand. I know, there is no global "reset" function for the whole VM. But, at least we have reset ioctl for vCPU. What if we hook up vGIC/vTimer there, and reset at least per-CPU objects (CPU interface + redist + timer) at this point? P.S. I've seen your PULL, and it is missing a little thing that could be good for 4.4 too. I've fixed one more bug recently, it reproduces on CP15-timer-less boards like Exynos: http://www.spinics.net/lists/kvm/msg122746.html. Just to make sure that you don't miss it. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/3] KVM: arm/arm64: Clean up some obsolete code
On Tue, Nov 03, 2015 at 12:44:54PM +0300, Pavel Fedin wrote: > Hello! > > > By this time i'll make a very minimal version of patch 0001, for you to > > test it. If we have > > problems with current 0001, which we > > cannot solve quickly, we could stick to that version then, which will > > provide the necessary > > changes to plug in LPIs, yet with > > minimal changes (it will only remove vgic_irq_lr_map). > > I guess i should have done it before. Or, i could even respin v5, with > > current 0001 split up. > > This should make it easier to bisect > > the problem. > > So, i have just sent v5, conditions are the same as before. It is OK to stop > at any point, and actually you should be able to > easily throw away 0003 and apply just 1, 2, 4. The minimum needed thing for > LPIs introduction is 0001. > You can also stick to v4 if the problem does not get triggered by its first > patch, if you prefer reduced commit log. > Actually, I seem to have been just incredibly unlucky with my test cycles, because I eventually reproduced the bug without your patches. I'm going to take this version of the series because that's what I reviewed and tested. Sorry for the noise. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 0/3] KVM: arm/arm64: Clean up some obsolete code
Hello! > By this time i'll make a very minimal version of patch 0001, for you to test > it. If we have > problems with current 0001, which we > cannot solve quickly, we could stick to that version then, which will provide > the necessary > changes to plug in LPIs, yet with > minimal changes (it will only remove vgic_irq_lr_map). > I guess i should have done it before. Or, i could even respin v5, with > current 0001 split up. > This should make it easier to bisect > the problem. So, i have just sent v5, conditions are the same as before. It is OK to stop at any point, and actually you should be able to easily throw away 0003 and apply just 1, 2, 4. The minimum needed thing for LPIs introduction is 0001. You can also stick to v4 if the problem does not get triggered by its first patch, if you prefer reduced commit log. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 0/3] KVM: arm/arm64: Clean up some obsolete code
Hello! > I ran this through my test scripts and I'm now quite sure that there's > some breakage in here. > > One of my tests is running two VMs in parallel, each booting up, running > hackbench, and then doing reboot (from within the guest), and just > repeating like that. > > I've run your patches in the above config 100 times, and every time, > the rebooting VMs got stuck before 50 reboots. > > Without these patches, I could run the above config 100 times, and every > time, the rebooting VMs passed 200 reboots. Huh, the description looks like some problem with vgic_retire_disabled_irqs(). By the way, during reboot, who does call it? The only call i see is in vgic_handle_enable_reg(), which obviously just processes emulated register accesses... And the only thing i know is that in case of GICv2 the userland resets vGIC manually by resetting each register to its default value (therefore all ENABLER are set to 0). At least qemu does this, and i'm not sure about kvmtool. And in case of vGICv3 nobody can do this because there's no API to set registers yet. So, could we be rebooting with interrupts enabled or something like that? So: what kind of container are you running and what vGIC version? Does this problem reproduce with both vGICv2 and vGICv3? By this time i'll make a very minimal version of patch 0001, for you to test it. If we have problems with current 0001, which we cannot solve quickly, we could stick to that version then, which will provide the necessary changes to plug in LPIs, yet with minimal changes (it will only remove vgic_irq_lr_map). I guess i should have done it before. Or, i could even respin v5, with current 0001 split up. This should make it easier to bisect the problem. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/3] KVM: arm/arm64: Clean up some obsolete code
On Tue, Oct 27, 2015 at 11:37:28AM +0300, Pavel Fedin wrote: > Current KVM code has lots of old redundancies, which can be cleaned up. > This patchset is actually a better alternative to > http://www.spinics.net/lists/arm-kernel/msg430726.html, which allows to > keep piggy-backed LRs. The idea is based on the fact that our code also > maintains LR state in elrsr, and this information is enough to track LR > usage. > > In case of problems this series can be applied partially, each patch is > a complete refactoring step on its own. > > Thanks to Andre Przywara for pinpointing some 4.3+ specifics. > > This version has been tested on SMDK5410 development board > (Exynos5410 SoC). I ran this through my test scripts and I'm now quite sure that there's some breakage in here. One of my tests is running two VMs in parallel, each booting up, running hackbench, and then doing reboot (from within the guest), and just repeating like that. I've run your patches in the above config 100 times, and every time, the rebooting VMs got stuck before 50 reboots. Without these patches, I could run the above config 100 times, and every time, the rebooting VMs passed 200 reboots. I'll try to test each patch individually soon. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
