From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45844) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bT9P7-0007v8-NJ for qemu-devel@nongnu.org; Fri, 29 Jul 2016 11:08:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bT9P5-0004PQ-M1 for qemu-devel@nongnu.org; Fri, 29 Jul 2016 11:08:48 -0400 References: <1469723896-28049-1-git-send-email-wei@redhat.com> From: Wei Huang Message-ID: Date: Fri, 29 Jul 2016 10:08:37 -0500 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC 1/1] arm64: add an option to turn on/off vpmu support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-arm , Andrew Jones , Shannon Zhao , Andrea Bolognani , QEMU Developers On 07/29/2016 02:57 AM, Peter Maydell wrote: > On 28 July 2016 at 17:38, Wei Huang wrote: >> This patch adds a pmu=[on/off] option to enable/disable vpmu support >> in guest vm. There are several reasons to justify this option. First >> vpmu can be problematic for cross-migration between different SoC as >> perf counters is architecture-dependent. It is more flexible to >> have an option to turn it on/off. Secondly it matches the -cpu pmu >> option in libivrt. This patch has been tested on both DT/ACPI modes. > > > What particular two systems are you trying to migrate between? One example: APM's Mustang has 5 perf counters while AMD's Seattle has 7 counters. > In general we don't support migrating between different CPU > types at the moment, so the PMU sholud be the same on both ends. > > (If we ever do get to supporting cross-cpu-type migration > then it will probably involve a very long and detailed command > line to specify exactly a whole lot of things like pmu yes/no, > number of hw breakpoints/watchpoints, and everything else that > can differ between implementations.) > > That said, I don't have any objection to making the PMU > presence controllable (especially if we have similar > control on x86). > >> --- a/target-arm/cpu.h >> +++ b/target-arm/cpu.h >> @@ -579,8 +579,9 @@ struct ARMCPU { >> bool powered_off; >> /* CPU has security extension */ >> bool has_el3; >> - /* CPU has PMU (Performance Monitor Unit) */ >> - bool has_pmu; >> + >> + /* CPU has vPMU (Performance Monitor Unit) support */ >> + bool enable_pmu; > > Why rename the flag? has_foo is what we use for other features, > as you can see in the context of this bit of the patch. I will fix it. Maybe follow the suggestion Drew's suggestion, keeping has_pmu and add another option for turning it on/off. > >> >> /* CPU has memory protection unit */ >> bool has_mpu; > > thanks > -- PMM >