On Sat, Dec 22, 2012 at 12:30:35PM -0700, David Ahern wrote: > On 12/17/12 3:20 AM, Gleb Natapov wrote: > >Does the regression happen because of commit 20b279ddb38c. If it does I > >think it is safe to revert it. KVM disables PEBS during guest entry now, so > >VMs shouldn't be blowing up (they do not in my testing) and if they still > >do we can disable the counter that has PEBS enabled on a guest entry too. > >Yes, if user runs "perf record -e cycles:ppG" he will not know that > >kernel ignored :pp modifier (with 20b279ddb38c he will get an error), but > >at least old binaries will continue working and new binaries can do the > >checking in userspace. > > > > Your patch alone was not enough. Start here: > https://lkml.org/lkml/2012/7/12/3 > I cannot reproduce this failure. I reverted 20b279ddb38c and ran "perf record -e cycles:ppG" while guest was running. Admittedly I ran the test for a short time, but without disabling PEBS during the guest entry this was enough to crash a guest.
The difference between "perf record -e cycles:ppG" and "perf record -e cycles:ppH" from KVM point of view is that for ppH PMU counter and PEBS will be disabled during a guest entry for ppG only PEBS will be disabled, so may be my testing is not enough and if counter remains enabled PEBS write can eventually overshoot guest entry. In this case we can treat ppG and ppH the same during guest entry and disable both counter and PEBS. > And from your response: > https://lkml.org/lkml/2012/7/12/337 > > "Do not run perf kvm. It does not set exclude_guest and :p and :pp > is not compatible with guest profiling and should be disallowed. > Again Peter's patch takes care of this." > I stand by this :) It should be disallowed as in "user should get a warning that he does something wrong and hist settings will be ignored". Unfortunately the way it was implemented breaks old perf binaries and keeping them running is more important than warning users about something that never worked anyway. New perf binary can do the check in userspace. Kernel should still disallow configuration that may crash a guest, but not in a way that breaks the userspace that does not set exclude_guest. What about forcing exclude_guest on an event that has precise flag set without reporting error to userspace? > 20b279ddb38c is Peter's patch -- kernel side enforcement that > exclude_guest needs to be set when using precise mode. > > David -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
