Zhao Liu <zhao1....@intel.com> writes:
> ...
>
>> > diff --git a/qemu-options.hx b/qemu-options.hx
>> > index dc694a99a30a..51a7c61ce0b0 100644
>> > --- a/qemu-options.hx
>> > +++ b/qemu-options.hx
>> > @@ -232,7 +232,8 @@ DEF("accel", HAS_ARG, QEMU_OPTION_accel,
>> > " eager-split-size=n (KVM Eager Page Split chunk size,
>> > default 0, disabled. ARM only)\n"
>> > "
>> > notify-vmexit=run|internal-error|disable,notify-window=n (enable notify VM
>> > exit and set notify window, x86 only)\n"
>> > " thread=single|multi (enable multi-threaded TCG)\n"
>> > - " device=path (KVM device path, default /dev/kvm)\n",
>> > QEMU_ARCH_ALL)
>> > + " device=path (KVM device path, default /dev/kvm)\n"
>> > + " pmu-filter=id (configure KVM PMU filter)\n",
>> > QEMU_ARCH_ALL)
>>
>> As we'll see below, this property is actually available only for i386.
>> Other target-specific properties document this like "x86 only". Please
>> do that for this one, too.
>
> Thanks! I'll change QEMU_ARCH_ALL to QEMU_ARCH_I386.
That would be wrong :)
QEMU_ARCH_ALL is the last argument passed to macro DEF(). It applies to
the entire option, in this case -accel.
I'd like you to mark the option parameter as "(x86 only)", like
notify-vmexit right above, and several more elsewhere.
>> As far as I can tell, the kvm-pmu-filter object needs to be activated
>> with -accel pmu-filter=... to do anything. Correct?
>
> Yes,
>
>> You can create any number of kvm-pmu-filter objects, but only one of
>> them can be active. Correct?
>
> Yes! I'll try to report error when user repeats to set this object, or
> mention this rule in doc.
Creating kvm-pmu-filter objects without using them should be harmless,
shouldn't it? I think users can already create other kinds of unused
objects.
>> > +
>> > +static int kvm_install_pmu_event_filter(KVMState *s)
>> > +{
>> > + struct kvm_pmu_event_filter *kvm_filter;
>> > + KVMPMUFilter *filter = s->pmu_filter;
>> > + int ret;
>> > +
>> > + kvm_filter = g_malloc0(sizeof(struct kvm_pmu_event_filter) +
>> > + filter->nevents * sizeof(uint64_t));
>>
>> Should we use sizeof(filter->events[0])?
>
> No, here I'm trying to constructing the memory accepted in kvm interface
> (with the specific layout), which is not the same as the KVMPMUFilter
> object.
You're right. What about sizeof(kvm_filter->events[0])?
[...]
