| Message ID | 20221028150243.34514-1-lyan@digtalocean.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/i386/cpu: disable PERFCORE for AMD when cpu.pmu is off | expand |
Liang Yan <lyan@digitalocean.com> writes: > With cpu.pmu=off, perfctr_core could still be seen in an AMD guest cpuid. > By further digging, I found cpu.perfctr_core did the trick. However, > considering the 'enable_pmu' in KVM could work on both Intel and AMD, > we may add AMD PMU control under 'enabe_pmu' in QEMU too. > > This change will overide the property 'perfctr_ctr' and change the AMD PMU > to off by default. > > Signed-off-by: Liang Yan <lyan@digtalocean.com> > --- > target/i386/cpu.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 22b681ca37..edf5413c90 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -5706,6 +5706,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > *ecx |= 1 << 1; /* CmpLegacy bit */ > } > } > + > + if (!cpu->enable_pmu) { > + *ecx &= ~CPUID_EXT3_PERFCORE; > + } > break; > case 0x80000002: > case 0x80000003: I may be missing something but my first impression is that this will make CPUID_EXT3_PERFCORE bit disappear when a !enable_pmu VM is migrated from an old QEMU (pre-patch) to a new one. If so, then additional precautions should be taking against that (e.g. tying the change to CPU/machine model versions, for example).
Hey Vitaly, On 10/31/22 6:07 AM, Vitaly Kuznetsov wrote: > Liang Yan <lyan@digitalocean.com> writes: > >> With cpu.pmu=off, perfctr_core could still be seen in an AMD guest cpuid. >> By further digging, I found cpu.perfctr_core did the trick. However, >> considering the 'enable_pmu' in KVM could work on both Intel and AMD, >> we may add AMD PMU control under 'enabe_pmu' in QEMU too. >> >> This change will overide the property 'perfctr_ctr' and change the AMD PMU >> to off by default. >> >> Signed-off-by: Liang Yan <lyan@digtalocean.com> >> --- >> target/i386/cpu.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index 22b681ca37..edf5413c90 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -5706,6 +5706,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, >> *ecx |= 1 << 1; /* CmpLegacy bit */ >> } >> } >> + >> + if (!cpu->enable_pmu) { >> + *ecx &= ~CPUID_EXT3_PERFCORE; >> + } >> break; >> case 0x80000002: >> case 0x80000003: > I may be missing something but my first impression is that this will > make CPUID_EXT3_PERFCORE bit disappear when a !enable_pmu VM is migrated > from an old QEMU (pre-patch) to a new one. If so, then additional > precautions should be taking against that (e.g. tying the change to > CPU/machine model versions, for example). > Thanks for the reply, it is a quite good point. I was struggled with it a little bit earlier because cpu.pmu has such operation for Intel CPU. After further talk with AMD people, I noticed that AMD PMU is more than perfctr_core, it has some legacy counters in use. I will dig a little further and send a v2 with extra cpu counters and live migration compatibility. Regards, Liang
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 22b681ca37..edf5413c90 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5706,6 +5706,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *ecx |= 1 << 1; /* CmpLegacy bit */ } } + + if (!cpu->enable_pmu) { + *ecx &= ~CPUID_EXT3_PERFCORE; + } break; case 0x80000002: case 0x80000003:
With cpu.pmu=off, perfctr_core could still be seen in an AMD guest cpuid. By further digging, I found cpu.perfctr_core did the trick. However, considering the 'enable_pmu' in KVM could work on both Intel and AMD, we may add AMD PMU control under 'enabe_pmu' in QEMU too. This change will overide the property 'perfctr_ctr' and change the AMD PMU to off by default. Signed-off-by: Liang Yan <lyan@digtalocean.com> --- target/i386/cpu.c | 4 ++++ 1 file changed, 4 insertions(+)