On Fri, Oct 14, 2016 at 09:48:28AM +0200, Andrew Jones wrote: > On Thu, Oct 13, 2016 at 05:19:37PM +0100, Peter Maydell wrote: > > On 4 October 2016 at 22:38, Wei Huang <w...@redhat.com> wrote: > > > This patchset adds a pmu=[on/off] option to enable/disable vPMU support > > > for guest VM. There are several reasons to justify this option. First, > > > vPMU can be problematic for cross-migration between different SoC as perf > > > counters are architecture-dependent. It is more flexible to have an option > > > to turn it on/off. Secondly this option matches the "pmu" option as > > > supported in libvirt. To make sure backward compatible, a PMU-related > > > property is added to mach-virt machine types. > > > > > > The following are testing results with this patchset. Other combinations > > > should have similar results: > > > CONFIG (qemu-system-aarch64) vPMU WARNING > > > -M virt-2.8/virt,accel=kvm -cpu host NO NO > > > > What's the rationale for defaulting to no-pmu? Other cpu features > > we have tend to default to present unless you ask not to have them. > > The feature needs to be managed in order for users to be able to trade-off > features exposed vs. migratability. If the lack of an explicit pmu=on > means either you get a PMU, when it's available, or you don't, when it's > not, then managing the feature becomes more difficult. Also, libvirt > already manages this cpu property for x86, and there it defaults off.
We discussed this today with Andrea. He says libvirt doesn't mind the default being on. x86 defaults pmu off, ppc defaults pmu on. I guess we might as well stick with on, it should make things easier. > > Maybe we should revisit the other features that default on, but may > be turned off, to make sure they make sense. Can you point them out? > I took a quick look, but it appears the cpu property defaults are all > pretty much determined by vms->secure, which defaults to false. > > > > > > -M virt-2.8/virt,accel=kvm -cpu host,pmu=off NO NO > > > -M virt-2.8/virt,accel=kvm -cpu host,pmu=on YES NO > > > -M virt-2.7,accel=kvm -cpu host YES NO > > > -M virt-2.7,accel=kvm -cpu host,pmu=off NO NO > > > -M virt-2.7,accel=kvm -cpu host,pmu=on YES NO > > > > in virt-2.7 you always got a pmu regardless, so why does pmu=off here give > > you no pmu? > > Hmm, I think you're right that the property should just be a nop, or > even invalid (causing an error) for 2.7 and down. Hmm... a no-op property wouldn't be exactly compatible. An error message stating the property isn't allowed would be. That's not how QEMU currently handles compat though. compat is only supported for going from an older- version host to a newer-version host (and back). In those cases the command line is older-version compatible and works in both places. Starting on a newer-version, even when using a compat machine type, doesn't guarantee we can go to an older, because, for example, the command line may include properties that don't exist there. We could address this, if we really want. We'd need to keep the auto tri-state to detect that the user set the property. If they did, then we should complain and fail to start, never allowing virt-2.7 instantiations to have the pmu property on the command line. I don't know if it's worth it though. > > > > > > -M virt-2.6,accel=kvm -cpu host NO NO > > > -M virt-2.6,accel=kvm -cpu host,pmu=off NO NO > > > -M virt-2.6,accel=kvm -cpu host,pmu=on YES NO > > > > > > -M virt-2.8/virt,accel=tcg -cpu cortex-a57 NO NO > > > -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=off NO NO > > > -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=on NO "No KVM" > > > -M virt-2.7,accel=tcg -cpu cortex-a57 NO NO > > > -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=off NO NO > > > -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=on NO "No KVM" > > > -M virt-2.6,accel=tcg -cpu cortex-a57 NO NO > > > -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=off NO NO > > > -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=on NO "No KVM" > > > > This patchset doesn't actually change whether the pmu is present > > or not under TCG so this is all a little bit odd. > > The goal here is to allow the property on the command line, i.e. not > error and fail, when using tcg. Otherwise more than just s/kvm/tcg/ > would be required when switching, and accel=kvm:tcg wouldn't work either. > > > > > > -M virt-2.8/virt,accel=tcg -cpu cortex-a15 NO NO > > > -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=off NO "No PMU > > > property" > > > -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=on NO "No PMU > > > property" > > > -M virt-2.7,accel=tcg -cpu cortex-a15 NO NO > > > -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=off NO "No PMU > > > property" > > > -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=on NO "No PMU > > > property" > > > -M virt-2.6,accel=tcg -cpu cortex-a15 NO NO > > > -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=off NO "No PMU > > > property" > > > -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=on NO "No PMU > > > property" > > > > > > * "NO KVM" msg > > > warning: pmu can't be enabled without KVM acceleration > > > * "No PMU property" msg > > > can't apply global cortex-a15-arm-cpu.pmu=off: Property '.pmu' not > > > found > > > > I'm really not a fan of the tristate has_pmu, so I'm hoping > > there's a setup which lets us avoid it... > > Probably keeping PMU on no matter what for 2.7 and down will allow > the removal of the tristate. If we switch to default on, then the only way to turn it off is with pmu=off added to the command line. So we'll know if pmu=off is given and we can just ignore it for virt-2.7. If we don't care about keeping the property off the virt-2.7 command line then there's no need for a tri-state. Thanks, drew
