diff mbox series

[v8] arm/kvm: Enable support for KVM_ARM_VCPU_PMU_V3_FILTER

Message ID 20240312074849.71475-1-shahuang@redhat.com (mailing list archive)
State New, archived
Headers show
Series [v8] arm/kvm: Enable support for KVM_ARM_VCPU_PMU_V3_FILTER | expand

Commit Message

Shaoqin Huang March 12, 2024, 7:48 a.m. UTC
The KVM_ARM_VCPU_PMU_V3_FILTER provides the ability to let the VMM decide
which PMU events are provided to the guest. Add a new option
`kvm-pmu-filter` as -cpu sub-option to set the PMU Event Filtering.
Without the filter, all PMU events are exposed from host to guest by
default. The usage of the new sub-option can be found from the updated
document (docs/system/arm/cpu-features.rst).

Here is an example which shows how to use the PMU Event Filtering, when
we launch a guest by use kvm, add such command line:

  # qemu-system-aarch64 \
        -accel kvm \
        -cpu host,kvm-pmu-filter="D:0x11-0x11"

Since the first action is deny, we have a global allow policy. This
filters out the cycle counter (event 0x11 being CPU_CYCLES).

And then in guest, use the perf to count the cycle:

  # perf stat sleep 1

   Performance counter stats for 'sleep 1':

              1.22 msec task-clock                       #    0.001 CPUs utilized
                 1      context-switches                 #  820.695 /sec
                 0      cpu-migrations                   #    0.000 /sec
                55      page-faults                      #   45.138 K/sec
   <not supported>      cycles
           1128954      instructions
            227031      branches                         #  186.323 M/sec
              8686      branch-misses                    #    3.83% of all branches

       1.002492480 seconds time elapsed

       0.001752000 seconds user
       0.000000000 seconds sys

As we can see, the cycle counter has been disabled in the guest, but
other pmu events do still work.

Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
---
v7->v8:
  - Add qtest for kvm-pmu-filter.
  - Do the kvm-pmu-filter syntax checking up-front in the kvm_pmu_filter_set()
  function. And store the filter information at there. When kvm_pmu_filter_get()
  reconstitute it.

v6->v7:
  - Check return value of sscanf.
  - Improve the check condition.

v5->v6:
  - Commit message improvement.
  - Remove some unused code.
  - Collect Reviewed-by, thanks Sebastian.
  - Use g_auto(Gstrv) to replace the gchar **.          [Eric]

v4->v5:
  - Change the kvm-pmu-filter as a -cpu sub-option.     [Eric]
  - Comment tweak.                                      [Gavin]
  - Rebase to the latest branch.

v3->v4:
  - Fix the wrong check for pmu_filter_init.            [Sebastian]
  - Fix multiple alignment issue.                       [Gavin]
  - Report error by warn_report() instead of error_report(), and don't use
  abort() since the PMU Event Filter is an add-on and best-effort feature.
                                                        [Gavin]
  - Add several missing {  } for single line of code.   [Gavin]
  - Use the g_strsplit() to replace strtok().           [Gavin]

v2->v3:
  - Improve commits message, use kernel doc wording, add more explaination on
    filter example, fix some typo error.                [Eric]
  - Add g_free() in kvm_arch_set_pmu_filter() to prevent memory leak. [Eric]
  - Add more precise error message report.              [Eric]
  - In options doc, add pmu-filter rely on KVM_ARM_VCPU_PMU_V3_FILTER support in
    KVM.                                                [Eric]

v1->v2:
  - Add more description for allow and deny meaning in 
    commit message.                                     [Sebastian]
  - Small improvement.                                  [Sebastian]
---
 docs/system/arm/cpu-features.rst |  23 +++++++
 target/arm/arm-qmp-cmds.c        |   2 +-
 target/arm/cpu.h                 |   3 +
 target/arm/kvm.c                 | 115 +++++++++++++++++++++++++++++++
 tests/qtest/arm-cpu-features.c   |  51 ++++++++++++++
 5 files changed, 193 insertions(+), 1 deletion(-)

Comments

Eric Auger March 19, 2024, 3:23 p.m. UTC | #1
Hi Shaoqin,

On 3/12/24 08:48, Shaoqin Huang wrote:
> The KVM_ARM_VCPU_PMU_V3_FILTER provides the ability to let the VMM decide
> which PMU events are provided to the guest. Add a new option
> `kvm-pmu-filter` as -cpu sub-option to set the PMU Event Filtering.
> Without the filter, all PMU events are exposed from host to guest by
> default. The usage of the new sub-option can be found from the updated
> document (docs/system/arm/cpu-features.rst).
> 
> Here is an example which shows how to use the PMU Event Filtering, when
> we launch a guest by use kvm, add such command line:
> 
>   # qemu-system-aarch64 \
>         -accel kvm \
>         -cpu host,kvm-pmu-filter="D:0x11-0x11"
> 
> Since the first action is deny, we have a global allow policy. This
> filters out the cycle counter (event 0x11 being CPU_CYCLES).
> 
> And then in guest, use the perf to count the cycle:
> 
>   # perf stat sleep 1
> 
>    Performance counter stats for 'sleep 1':
> 
>               1.22 msec task-clock                       #    0.001 CPUs utilized
>                  1      context-switches                 #  820.695 /sec
>                  0      cpu-migrations                   #    0.000 /sec
>                 55      page-faults                      #   45.138 K/sec
>    <not supported>      cycles
>            1128954      instructions
>             227031      branches                         #  186.323 M/sec
>               8686      branch-misses                    #    3.83% of all branches
> 
>        1.002492480 seconds time elapsed
> 
>        0.001752000 seconds user
>        0.000000000 seconds sys
> 
> As we can see, the cycle counter has been disabled in the guest, but
> other pmu events do still work.
> 
> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> ---
> v7->v8:
>   - Add qtest for kvm-pmu-filter.
>   - Do the kvm-pmu-filter syntax checking up-front in the kvm_pmu_filter_set()
>   function. And store the filter information at there. When kvm_pmu_filter_get()
>   reconstitute it.
> 
> v6->v7:
>   - Check return value of sscanf.
>   - Improve the check condition.
> 
> v5->v6:
>   - Commit message improvement.
>   - Remove some unused code.
>   - Collect Reviewed-by, thanks Sebastian.
>   - Use g_auto(Gstrv) to replace the gchar **.          [Eric]
> 
> v4->v5:
>   - Change the kvm-pmu-filter as a -cpu sub-option.     [Eric]
>   - Comment tweak.                                      [Gavin]
>   - Rebase to the latest branch.
> 
> v3->v4:
>   - Fix the wrong check for pmu_filter_init.            [Sebastian]
>   - Fix multiple alignment issue.                       [Gavin]
>   - Report error by warn_report() instead of error_report(), and don't use
>   abort() since the PMU Event Filter is an add-on and best-effort feature.
>                                                         [Gavin]
>   - Add several missing {  } for single line of code.   [Gavin]
>   - Use the g_strsplit() to replace strtok().           [Gavin]
> 
> v2->v3:
>   - Improve commits message, use kernel doc wording, add more explaination on
>     filter example, fix some typo error.                [Eric]
>   - Add g_free() in kvm_arch_set_pmu_filter() to prevent memory leak. [Eric]
>   - Add more precise error message report.              [Eric]
>   - In options doc, add pmu-filter rely on KVM_ARM_VCPU_PMU_V3_FILTER support in
>     KVM.                                                [Eric]
> 
> v1->v2:
>   - Add more description for allow and deny meaning in 
>     commit message.                                     [Sebastian]
>   - Small improvement.                                  [Sebastian]
> ---
>  docs/system/arm/cpu-features.rst |  23 +++++++
>  target/arm/arm-qmp-cmds.c        |   2 +-
>  target/arm/cpu.h                 |   3 +
>  target/arm/kvm.c                 | 115 +++++++++++++++++++++++++++++++
>  tests/qtest/arm-cpu-features.c   |  51 ++++++++++++++
>  5 files changed, 193 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst
> index a5fb929243..f3930f34b3 100644
> --- a/docs/system/arm/cpu-features.rst
> +++ b/docs/system/arm/cpu-features.rst
> @@ -204,6 +204,29 @@ the list of KVM VCPU features and their descriptions.
>    the guest scheduler behavior and/or be exposed to the guest
>    userspace.
>  
> +``kvm-pmu-filter``
> +  By default kvm-pmu-filter is disabled. This means that by default all PMU
> +  events will be exposed to guest.
> +
> +  KVM implements PMU Event Filtering to prevent a guest from being able to
> +  sample certain events. It depends on the KVM_ARM_VCPU_PMU_V3_FILTER
> +  attribute supported in KVM. It has the following format:
> +
> +  kvm-pmu-filter="{A,D}:start-end[;{A,D}:start-end...]"
> +
> +  The A means "allow" and D means "deny", start is the first event of the
> +  range and the end is the last one. The first registered range defines
> +  the global policy (global ALLOW if the first action is DENY, global DENY
> +  if the first action is ALLOW). The start and end only support hexadecimal
> +  format. For example:
> +
> +  kvm-pmu-filter="A:0x11-0x11;A:0x23-0x3a;D:0x30-0x30"
> +
> +  Since the first action is allow, we have a global deny policy. It
> +  will allow event 0x11 (the cycle counter), events 0x23 to 0x3a are
> +  also allowed except the event 0x30 which is denied, and all the other
> +  events are denied.
> +
>  TCG VCPU Features
>  =================
>  
> diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
> index 2250cd7ddf..36df2e4820 100644
> --- a/target/arm/arm-qmp-cmds.c
> +++ b/target/arm/arm-qmp-cmds.c
> @@ -94,7 +94,7 @@ static const char *cpu_model_advertised_features[] = {
>      "sve128", "sve256", "sve384", "sve512",
>      "sve640", "sve768", "sve896", "sve1024", "sve1152", "sve1280",
>      "sve1408", "sve1536", "sve1664", "sve1792", "sve1920", "sve2048",
> -    "kvm-no-adjvtime", "kvm-steal-time",
> +    "kvm-no-adjvtime", "kvm-steal-time", "kvm-pmu-filter",
>      "pauth", "pauth-impdef", "pauth-qarma3",
>      NULL
>  };
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 63f31e0d98..b810a80e67 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -948,6 +948,9 @@ struct ArchCPU {
>  
>      /* KVM steal time */
>      OnOffAuto kvm_steal_time;
> +
> +    /* KVM PMU Filter */
> +    GArray *kvm_pmu_filter;
>  #endif /* CONFIG_KVM */
>  
>      /* Uniprocessor system with MP extensions */
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 81813030a5..7f62fad029 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -496,6 +496,72 @@ static void kvm_steal_time_set(Object *obj, bool value, Error **errp)
>      ARM_CPU(obj)->kvm_steal_time = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
>  }
>  
> +static char *kvm_pmu_filter_get(Object *obj, Error **errp)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +    g_autoptr(GString) pmu_filter = g_string_new(NULL);
> +    struct kvm_pmu_event_filter *filter;
> +    char action;
> +    int i;
> +
> +    if (!cpu->kvm_pmu_filter) {
> +        return NULL;
> +    }
> +
> +    for (i = 0; i < cpu->kvm_pmu_filter->len; i++) {
> +        filter = &g_array_index(cpu->kvm_pmu_filter,
> +                                struct kvm_pmu_event_filter, i);
> +        if (i) {
> +            g_string_append_c(pmu_filter, ';');
> +        }
> +        action = filter->action == KVM_PMU_EVENT_ALLOW ? 'A' : 'D';
> +        g_string_append_printf(pmu_filter, "%c:0x%hx-0x%hx", action,
> +                               filter->base_event,
> +                               filter->base_event + filter->nevents - 1);
> +    }
> +
> +    return g_strdup(pmu_filter->str);
> +}
> +
> +static void kvm_pmu_filter_set(Object *obj, const char *pmu_filter,
> +                               Error **errp)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +    struct kvm_pmu_event_filter filter;
> +    g_auto(GStrv) event_filters;
> +    int i;
> +
> +    if (cpu->kvm_pmu_filter) {
> +        g_array_free(cpu->kvm_pmu_filter, true);
> +    }
> +
> +    cpu->kvm_pmu_filter = g_array_new(false, false,
> +                                      sizeof(struct kvm_pmu_event_filter));
> +
> +    event_filters = g_strsplit(pmu_filter, ";", -1);
> +    for (i = 0; event_filters[i]; i++) {
> +        unsigned short start = 0, end = 0;
> +        char act;
> +
> +        if (sscanf(event_filters[i], "%c:%hx-%hx", &act, &start, &end) != 3) {
> +            warn_report("Skipping invalid PMU filter %s", event_filters[i]);
> +            continue;
> +        }
> +
> +        if ((act != 'A' && act != 'D') || start > end) {
> +            warn_report("Skipping invalid PMU filter %s", event_filters[i]);
> +            continue;
if both conditions above lead to the same outcome, I think you can merge
"or" them.

on v7 Peter suggested we should rather fail if the user passes us an
option that can't be applied. This sounds even more sensible now we
handle things on "set". Then you can use error_setg on errp and return.
> +        }
> +
> +        filter.base_event = start;
> +        filter.nevents = end - start + 1;
> +        filter.action = (act == 'A') ? KVM_PMU_EVENT_ALLOW :
> +                                       KVM_PMU_EVENT_DENY;
> +
> +        g_array_append_vals(cpu->kvm_pmu_filter, &filter, 1);
> +    }
> +}
> +
>  /* KVM VCPU properties should be prefixed with "kvm-". */
>  void kvm_arm_add_vcpu_properties(ARMCPU *cpu)
>  {
> @@ -517,6 +583,12 @@ void kvm_arm_add_vcpu_properties(ARMCPU *cpu)
>                               kvm_steal_time_set);
>      object_property_set_description(obj, "kvm-steal-time",
>                                      "Set off to disable KVM steal time.");
> +
> +    object_property_add_str(obj, "kvm-pmu-filter", kvm_pmu_filter_get,
> +                            kvm_pmu_filter_set);
> +    object_property_set_description(obj, "kvm-pmu-filter",
> +                                    "PMU Event Filtering description for "
> +                                    "guest PMU. (default: NULL, disabled)");
>  }
>  
>  bool kvm_arm_pmu_supported(void)
> @@ -1706,6 +1778,47 @@ static bool kvm_arm_set_device_attr(ARMCPU *cpu, struct kvm_device_attr *attr,
>      return true;
>  }
>  
> +static void kvm_arm_pmu_filter_init(ARMCPU *cpu)
> +{
> +    static bool pmu_filter_init;
> +    struct kvm_device_attr attr = {
> +        .group      = KVM_ARM_VCPU_PMU_V3_CTRL,
> +        .attr       = KVM_ARM_VCPU_PMU_V3_FILTER,
> +    };
> +    int i;
> +
> +    /*
> +     * The filter only needs to be initialized through one vcpu ioctl and it
> +     * will affect all other vcpu in the vm.
> +     * It can be referred from kernel commit d7eec2360e3 ("KVM: arm64: Add PMU
> +     * event filtering infrastructure"):
> +     * Note that although the ioctl is per-vcpu, the map of allowed events is
> +     * global to the VM (it can be setup from any vcpu until the vcpu PMU is
> +     * initialized).
Personnally I would just say:

although the ioctl is per-vcpu,  the map of allowed events is global to
the VM (and can be setup from any vcpu until the vcpu PMU is initialized
> +     */
> +    if (pmu_filter_init) {
> +        return;
> +    } else {
> +        pmu_filter_init = true;
> +    }
> +
> +    if (!cpu->kvm_pmu_filter) {
> +        return;
> +    }
> +    if (kvm_vcpu_ioctl(CPU(cpu), KVM_HAS_DEVICE_ATTR, &attr)) {
> +        error_report("KVM doesn't support the PMU Event Filter!");
> +        return;
> +    }
> +
> +    for (i = 0; i < cpu->kvm_pmu_filter->len; i++) {
> +        attr.addr = (uint64_t)&g_array_index(cpu->kvm_pmu_filter,
> +                                             struct kvm_pmu_event_filter, i);
> +        if (!kvm_arm_set_device_attr(cpu, &attr, "PMU_V3_FILTER")) {
> +            break;
I think Peter requested it to fail. Here it doesn't.
> +        }
> +    }
> +}
> +
>  void kvm_arm_pmu_init(ARMCPU *cpu)
>  {
>      struct kvm_device_attr attr = {
> @@ -1716,6 +1829,8 @@ void kvm_arm_pmu_init(ARMCPU *cpu)
>      if (!cpu->has_pmu) {
>          return;
>      }
> +
> +    kvm_arm_pmu_filter_init(cpu);
>      if (!kvm_arm_set_device_attr(cpu, &attr, "PMU")) {
>          error_report("failed to init PMU");
>          abort();
> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
> index a8a4c668ad..60a5e32eb8 100644
> --- a/tests/qtest/arm-cpu-features.c
> +++ b/tests/qtest/arm-cpu-features.c
> @@ -127,6 +127,17 @@ static bool resp_get_feature(QDict *resp, const char *feature)
>      return qdict_get_bool(props, feature);
>  }
>  
> +static const char *resp_get_feature_str(QDict *resp, const char *feature)
> +{
> +    QDict *props;
> +
> +    g_assert(resp);
> +    g_assert(resp_has_props(resp));
> +    props = resp_get_props(resp);
> +    g_assert(qdict_get(props, feature));
> +    return qdict_get_str(props, feature);
> +}
> +
>  #define assert_has_feature(qts, cpu_type, feature)                     \
>  ({                                                                     \
>      QDict *_resp = do_query_no_props(qts, cpu_type);                   \
> @@ -156,6 +167,18 @@ static bool resp_get_feature(QDict *resp, const char *feature)
>      g_assert(qdict_get_bool(_props, feature) == (expected_value));     \
>  })
>  
> +#define resp_assert_feature_str(resp, feature, expected_value)         \
> +({                                                                     \
> +    QDict *_props;                                                     \
> +                                                                       \
> +    g_assert(_resp);                                                   \
> +    g_assert(resp_has_props(_resp));                                   \
> +    _props = resp_get_props(_resp);                                    \
> +    g_assert(qdict_get(_props, feature));                              \
> +    g_assert_cmpstr(qdict_get_str(_props, feature),                    \
> +                    ==, (expected_value));                             \
> +})
> +
>  #define assert_feature(qts, cpu_type, feature, expected_value)         \
>  ({                                                                     \
>      QDict *_resp;                                                      \
> @@ -177,6 +200,17 @@ static bool resp_get_feature(QDict *resp, const char *feature)
>      qobject_unref(_resp);                                              \
>  })
>  
> +#define assert_set_feature_str(qts, cpu_type, feature, value)          \
> +({                                                                     \
> +    const char *_fmt = "{ %s: %s }";                                   \
> +    QDict *_resp;                                                      \
> +                                                                       \
> +    _resp = do_query(qts, cpu_type, _fmt, feature, value);             \
> +    g_assert(_resp);                                                   \
> +    resp_assert_feature_str(_resp, feature, value);                    \
> +    qobject_unref(_resp);                                              \
> +})
> +
>  #define assert_has_feature_enabled(qts, cpu_type, feature)             \
>      assert_feature(qts, cpu_type, feature, true)
>  
> @@ -461,6 +495,7 @@ static void test_query_cpu_model_expansion(const void *data)
>  
>      assert_has_not_feature(qts, "max", "kvm-no-adjvtime");
>      assert_has_not_feature(qts, "max", "kvm-steal-time");
> +    assert_has_not_feature(qts, "max", "kvm-pmu-filter");
>  
>      if (g_str_equal(qtest_get_arch(), "aarch64")) {
>          assert_has_feature_enabled(qts, "max", "aarch64");
> @@ -508,6 +543,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
>      assert_set_feature(qts, "host", "kvm-no-adjvtime", false);
>  
>      if (g_str_equal(qtest_get_arch(), "aarch64")) {
> +        const char *kvm_supports_pmu_filter;
>          bool kvm_supports_steal_time;
>          bool kvm_supports_sve;
>          char max_name[8], name[8];
> @@ -546,15 +582,29 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
>           * because this instance of KVM doesn't support them. Test that the
>           * features are present, and, when enabled, issue further tests.
>           */
> +        assert_has_feature(qts, "host", "kvm-pmu-filter");
>          assert_has_feature(qts, "host", "kvm-steal-time");
>          assert_has_feature(qts, "host", "sve");
>  
>          resp = do_query_no_props(qts, "host");
> +        kvm_supports_pmu_filter = resp_get_feature_str(resp, "kvm-pmu-filter");
>          kvm_supports_steal_time = resp_get_feature(resp, "kvm-steal-time");
>          kvm_supports_sve = resp_get_feature(resp, "sve");
>          vls = resp_get_sve_vls(resp);
>          qobject_unref(resp);
>  
> +        if (kvm_supports_pmu_filter) {
> +            assert_set_feature_str(qts, "host", "kvm-pmu-filter", "");
> +            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
> +                                   "A:0x11-0x11");
> +            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
> +                                   "D:0x11-0x11");
> +            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
> +                                   "A:0x11-0x11;A:0x12-0x20");
> +            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
> +                                   "D:0x11-0x11;A:0x12-0x20;D:0x12-0x15");
Just to double check this set the filter and checks the filter is
applied, is that correct?
I see you set some ranges of events. Are you sure those events are
supported on host PMU and won't create a failure on setting the PMU filter?

Thanks

Eric
> +        }
> +
>          if (kvm_supports_steal_time) {
>              /* If we have steal-time then we should be able to toggle it. */
>              assert_set_feature(qts, "host", "kvm-steal-time", false);
> @@ -622,6 +672,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
>          assert_has_not_feature(qts, "host", "pmu");
>          assert_has_not_feature(qts, "host", "sve");
>          assert_has_not_feature(qts, "host", "kvm-steal-time");
> +        assert_has_not_feature(qts, "host", "kvm-pmu-filter");
>      }
>  
>      qtest_quit(qts);
Daniel P. Berrangé March 22, 2024, 2:53 p.m. UTC | #2
On Tue, Mar 12, 2024 at 03:48:49AM -0400, Shaoqin Huang wrote:
> The KVM_ARM_VCPU_PMU_V3_FILTER provides the ability to let the VMM decide
> which PMU events are provided to the guest. Add a new option
> `kvm-pmu-filter` as -cpu sub-option to set the PMU Event Filtering.
> Without the filter, all PMU events are exposed from host to guest by
> default. The usage of the new sub-option can be found from the updated
> document (docs/system/arm/cpu-features.rst).
> 
> Here is an example which shows how to use the PMU Event Filtering, when
> we launch a guest by use kvm, add such command line:
> 
>   # qemu-system-aarch64 \
>         -accel kvm \
>         -cpu host,kvm-pmu-filter="D:0x11-0x11"

I mistakenly sent some comments to the older v7 (despite this v8 already
existing) about the design of this syntax So for linking up the threads:

 https://lists.nongnu.org/archive/html/qemu-devel/2024-03/msg04703.html

With regards,
Daniel
Shaoqin Huang March 25, 2024, 5:35 a.m. UTC | #3
Hi Daniel,

Thanks for your reviewing. I see your comments in the v7.

I have some doubts about what you said about the QAPI. Do you want me to 
convert the current design into the QAPI parsing like the 
IOThreadVirtQueueMapping? And we need to add new json definition in the 
qapi/ directory?

Thanks,
Shaoqin

On 3/22/24 22:53, Daniel P. Berrangé wrote:
> On Tue, Mar 12, 2024 at 03:48:49AM -0400, Shaoqin Huang wrote:
>> The KVM_ARM_VCPU_PMU_V3_FILTER provides the ability to let the VMM decide
>> which PMU events are provided to the guest. Add a new option
>> `kvm-pmu-filter` as -cpu sub-option to set the PMU Event Filtering.
>> Without the filter, all PMU events are exposed from host to guest by
>> default. The usage of the new sub-option can be found from the updated
>> document (docs/system/arm/cpu-features.rst).
>>
>> Here is an example which shows how to use the PMU Event Filtering, when
>> we launch a guest by use kvm, add such command line:
>>
>>    # qemu-system-aarch64 \
>>          -accel kvm \
>>          -cpu host,kvm-pmu-filter="D:0x11-0x11"
> 
> I mistakenly sent some comments to the older v7 (despite this v8 already
> existing) about the design of this syntax So for linking up the threads:
> 
>   https://lists.nongnu.org/archive/html/qemu-devel/2024-03/msg04703.html
> 
> With regards,
> Daniel
Daniel P. Berrangé March 25, 2024, 8:55 a.m. UTC | #4
On Mon, Mar 25, 2024 at 01:35:58PM +0800, Shaoqin Huang wrote:
> Hi Daniel,
> 
> Thanks for your reviewing. I see your comments in the v7.
> 
> I have some doubts about what you said about the QAPI. Do you want me to
> convert the current design into the QAPI parsing like the
> IOThreadVirtQueueMapping? And we need to add new json definition in the
> qapi/ directory?

Yes, you would define a type in the qapi dir similar to how is
done for IOThreadVirtQueueMapping, and then you can use that
in the property setter method.


With regards,
Daniel
Shaoqin Huang March 29, 2024, 3:45 a.m. UTC | #5
Hi Daniel,

On 3/25/24 16:55, Daniel P. Berrangé wrote:
> On Mon, Mar 25, 2024 at 01:35:58PM +0800, Shaoqin Huang wrote:
>> Hi Daniel,
>>
>> Thanks for your reviewing. I see your comments in the v7.
>>
>> I have some doubts about what you said about the QAPI. Do you want me to
>> convert the current design into the QAPI parsing like the
>> IOThreadVirtQueueMapping? And we need to add new json definition in the
>> qapi/ directory?

I have defined the QAPI for kvm-pmu-filter like below:

+##
+# @FilterAction:
+#
+# The Filter Action
+#
+# @a: Allow
+#
+# @d: Disallow
+#
+# Since: 9.0
+##
+{ 'enum': 'FilterAction',
+  'data': [ 'a', 'd' ] }
+
+##
+# @SingleFilter:
+#
+# Lazy
+#
+# @action: the action
+#
+# @start: the start
+#
+# @end: the end
+#
+# Since: 9.0
+##
+
+{ 'struct': 'SingleFilter',
+ 'data': { 'action': 'FilterAction', 'start': 'int', 'end': 'int' } }
+
+##
+# @KVMPMUFilter:
+#
+# Lazy
+#
+# @filter: the filter
+#
+# Since: 9.0
+##
+
+{ 'struct': 'KVMPMUFilter',
+  'data': { 'filter': ['SingleFilter'] }}

And I guess I can use it by adding code like below:

--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -1206,3 +1206,35 @@ const PropertyInfo 
qdev_prop_iothread_vq_mapping_list = {
      .set = set_iothread_vq_mapping_list,
      .release = release_iothread_vq_mapping_list,
  };
+
+/* --- kvm-pmu-filter ---*/
+
+static void get_kvm_pmu_filter(Object *obj, Visitor *v,
+        const char *name, void *opaque, Error **errp)
+{
+    KVMPMUFilter **prop_ptr = object_field_prop_ptr(obj, opaque);
+
+    visit_type_KVMPMUFilter(v, name, prop_ptr, errp);
+}
+
+static void set_kvm_pmu_filter(Object *obj, Visitor *v,
+        const char *name, void *opaque, Error **errp)
+{
+    KVMPMUFilter **prop_ptr = object_field_prop_ptr(obj, opaque);
+    KVMPMUFilter *list;
+
+    printf("running the %s\n", __func__);
+    if (!visit_type_KVMPMUFilter(v, name, &list, errp)) {
+        return;
+    }
+
+    printf("The name is %s\n", name);
+    *prop_ptr = list;
+}
+
+const PropertyInfo qdev_prop_kvm_pmu_filter = {
+    .name = "KVMPMUFilter",
+    .description = "der der",
+    .get = get_kvm_pmu_filter,
+    .set = set_kvm_pmu_filter,
+};

+#define DEFINE_PROP_KVM_PMU_FILTER(_name, _state, _field) \
+    DEFINE_PROP(_name, _state, _field, qdev_prop_kvm_pmu_filter, \
+                KVMPMUFilter *)

--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2439,6 +2441,7 @@ static Property arm_cpu_properties[] = {
                          mp_affinity, ARM64_AFFINITY_INVALID),
      DEFINE_PROP_INT32("node-id", ARMCPU, node_id, CPU_UNSET_NUMA_NODE_ID),
      DEFINE_PROP_INT32("core-count", ARMCPU, core_count, -1),
+    DEFINE_PROP_KVM_PMU_FILTER("kvm-pmu-filter", ARMCPU, kvm_pmu_filter),
      DEFINE_PROP_END_OF_LIST()
  };

And I guess I can use the new json format input like below:

qemu-system-aarch64 \
	-cpu host, '{"filter": [{"action": "a", "start": 0x10, "end": "0x11"}]}'

But it doesn't work. It seems like because the -cpu option doesn't 
support json format parameter.

Maybe I'm wrong. So I want to double check with if the -cpu option 
support json format nowadays?

If the -cpu option doesn't support json format, how I can use the QAPI 
for kvm-pmu-filter property?

Thanks,
Shaoqin

> 
> Yes, you would define a type in the qapi dir similar to how is
> done for IOThreadVirtQueueMapping, and then you can use that
> in the property setter method.
> 
> 
> With regards,
> Daniel
Kevin Wolf April 2, 2024, 1:01 p.m. UTC | #6
Am 29.03.2024 um 04:45 hat Shaoqin Huang geschrieben:
> Hi Daniel,
> 
> On 3/25/24 16:55, Daniel P. Berrangé wrote:
> > On Mon, Mar 25, 2024 at 01:35:58PM +0800, Shaoqin Huang wrote:
> > > Hi Daniel,
> > > 
> > > Thanks for your reviewing. I see your comments in the v7.
> > > 
> > > I have some doubts about what you said about the QAPI. Do you want me to
> > > convert the current design into the QAPI parsing like the
> > > IOThreadVirtQueueMapping? And we need to add new json definition in the
> > > qapi/ directory?
> 
> I have defined the QAPI for kvm-pmu-filter like below:
> 
> +##
> +# @FilterAction:
> +#
> +# The Filter Action
> +#
> +# @a: Allow
> +#
> +# @d: Disallow
> +#
> +# Since: 9.0
> +##
> +{ 'enum': 'FilterAction',
> +  'data': [ 'a', 'd' ] }
> +
> +##
> +# @SingleFilter:
> +#
> +# Lazy
> +#
> +# @action: the action
> +#
> +# @start: the start
> +#
> +# @end: the end
> +#
> +# Since: 9.0
> +##
> +
> +{ 'struct': 'SingleFilter',
> + 'data': { 'action': 'FilterAction', 'start': 'int', 'end': 'int' } }
> +
> +##
> +# @KVMPMUFilter:
> +#
> +# Lazy
> +#
> +# @filter: the filter
> +#
> +# Since: 9.0
> +##
> +
> +{ 'struct': 'KVMPMUFilter',
> +  'data': { 'filter': ['SingleFilter'] }}
> 
> And I guess I can use it by adding code like below:
> 
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -1206,3 +1206,35 @@ const PropertyInfo qdev_prop_iothread_vq_mapping_list
> = {
>      .set = set_iothread_vq_mapping_list,
>      .release = release_iothread_vq_mapping_list,
>  };
> +
> +/* --- kvm-pmu-filter ---*/
> +
> +static void get_kvm_pmu_filter(Object *obj, Visitor *v,
> +        const char *name, void *opaque, Error **errp)
> +{
> +    KVMPMUFilter **prop_ptr = object_field_prop_ptr(obj, opaque);
> +
> +    visit_type_KVMPMUFilter(v, name, prop_ptr, errp);
> +}
> +
> +static void set_kvm_pmu_filter(Object *obj, Visitor *v,
> +        const char *name, void *opaque, Error **errp)
> +{
> +    KVMPMUFilter **prop_ptr = object_field_prop_ptr(obj, opaque);
> +    KVMPMUFilter *list;
> +
> +    printf("running the %s\n", __func__);
> +    if (!visit_type_KVMPMUFilter(v, name, &list, errp)) {
> +        return;
> +    }
> +
> +    printf("The name is %s\n", name);
> +    *prop_ptr = list;
> +}
> +
> +const PropertyInfo qdev_prop_kvm_pmu_filter = {
> +    .name = "KVMPMUFilter",
> +    .description = "der der",
> +    .get = get_kvm_pmu_filter,
> +    .set = set_kvm_pmu_filter,
> +};
> 
> +#define DEFINE_PROP_KVM_PMU_FILTER(_name, _state, _field) \
> +    DEFINE_PROP(_name, _state, _field, qdev_prop_kvm_pmu_filter, \
> +                KVMPMUFilter *)
> 
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2439,6 +2441,7 @@ static Property arm_cpu_properties[] = {
>                          mp_affinity, ARM64_AFFINITY_INVALID),
>      DEFINE_PROP_INT32("node-id", ARMCPU, node_id, CPU_UNSET_NUMA_NODE_ID),
>      DEFINE_PROP_INT32("core-count", ARMCPU, core_count, -1),
> +    DEFINE_PROP_KVM_PMU_FILTER("kvm-pmu-filter", ARMCPU, kvm_pmu_filter),
>      DEFINE_PROP_END_OF_LIST()
>  };
> 
> And I guess I can use the new json format input like below:
> 
> qemu-system-aarch64 \
> 	-cpu host, '{"filter": [{"action": "a", "start": 0x10, "end": "0x11"}]}'
> 
> But it doesn't work. It seems like because the -cpu option doesn't
> support json format parameter.
> 
> Maybe I'm wrong. So I want to double check with if the -cpu option
> support json format nowadays?

As far as I can see, -cpu doesn't support JSON yet. But even if it did,
your command line would be invalid because the 'host,' part isn't JSON.

> If the -cpu option doesn't support json format, how I can use the QAPI
> for kvm-pmu-filter property?

This would probably mean QAPIfying all CPUs first, which sounds like a
major effort.

Kevin
Shaoqin Huang April 9, 2024, 1:57 a.m. UTC | #7
Hi Kevin,

On 4/2/24 21:01, Kevin Wolf wrote:
>> Maybe I'm wrong. So I want to double check with if the -cpu option
>> support json format nowadays?
> As far as I can see, -cpu doesn't support JSON yet. But even if it did,
> your command line would be invalid because the 'host,' part isn't JSON.
> 

Thanks for answering my question. I guess I should still keep the 
current implementation, and to transform the property in the future when 
the -cpu option support JSON format.

Thanks,
Shaoqin

>> If the -cpu option doesn't support json format, how I can use the QAPI
>> for kvm-pmu-filter property?
> This would probably mean QAPIfying all CPUs first, which sounds like a
> major effort.
Shaoqin Huang April 9, 2024, 2:43 a.m. UTC | #8
Hi Eric,

On 3/19/24 23:23, Eric Auger wrote:
>> +        if (kvm_supports_pmu_filter) {
>> +            assert_set_feature_str(qts, "host", "kvm-pmu-filter", "");
>> +            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
>> +                                   "A:0x11-0x11");
>> +            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
>> +                                   "D:0x11-0x11");
>> +            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
>> +                                   "A:0x11-0x11;A:0x12-0x20");
>> +            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
>> +                                   "D:0x11-0x11;A:0x12-0x20;D:0x12-0x15");
> Just to double check this set the filter and checks the filter is
> applied, is that correct?
> I see you set some ranges of events. Are you sure those events are
> supported on host PMU and won't create a failure on setting the PMU filter?

What I test here is that checking if the PMU Filter parser is right 
which I write in the kvm_pmu_filter_set/get function, I don't test any 
KVM side things like if the PMU event is supported by host.

Thanks,
Shaoqin

> 
> Thanks
> 
> Eric
Daniel P. Berrangé April 15, 2024, 5:31 p.m. UTC | #9
On Tue, Apr 02, 2024 at 03:01:50PM +0200, Kevin Wolf wrote:
> Am 29.03.2024 um 04:45 hat Shaoqin Huang geschrieben:
> > Hi Daniel,
> > 
> > On 3/25/24 16:55, Daniel P. Berrangé wrote:
> > > On Mon, Mar 25, 2024 at 01:35:58PM +0800, Shaoqin Huang wrote:
> > > > Hi Daniel,
> > > > 
> > > > Thanks for your reviewing. I see your comments in the v7.
> > > > 
> > > > I have some doubts about what you said about the QAPI. Do you want me to
> > > > convert the current design into the QAPI parsing like the
> > > > IOThreadVirtQueueMapping? And we need to add new json definition in the
> > > > qapi/ directory?
> > 
> > I have defined the QAPI for kvm-pmu-filter like below:

> > @@ -2439,6 +2441,7 @@ static Property arm_cpu_properties[] = {
> >                          mp_affinity, ARM64_AFFINITY_INVALID),
> >      DEFINE_PROP_INT32("node-id", ARMCPU, node_id, CPU_UNSET_NUMA_NODE_ID),
> >      DEFINE_PROP_INT32("core-count", ARMCPU, core_count, -1),
> > +    DEFINE_PROP_KVM_PMU_FILTER("kvm-pmu-filter", ARMCPU, kvm_pmu_filter),
> >      DEFINE_PROP_END_OF_LIST()
> >  };
> > 
> > And I guess I can use the new json format input like below:
> > 
> > qemu-system-aarch64 \
> > 	-cpu host, '{"filter": [{"action": "a", "start": 0x10, "end": "0x11"}]}'
> > 
> > But it doesn't work. It seems like because the -cpu option doesn't
> > support json format parameter.
> > 
> > Maybe I'm wrong. So I want to double check with if the -cpu option
> > support json format nowadays?
> 
> As far as I can see, -cpu doesn't support JSON yet. But even if it did,
> your command line would be invalid because the 'host,' part isn't JSON.
> 
> > If the -cpu option doesn't support json format, how I can use the QAPI
> > for kvm-pmu-filter property?
> 
> This would probably mean QAPIfying all CPUs first, which sounds like a
> major effort.

I wonder if we can do a half-way house where we parse the JSON and
turn it into regular QemuOpts internally, and then just use QAPI
parsing for the filter property. IOW, publically give the illusion
that -cpu has been QAPI-ified, but without actually doing the hard
part yet. The idea being to avoid inventing a new cli syntax that
has no analogue to QAPI.
With regards,
Daniel
diff mbox series

Patch

diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst
index a5fb929243..f3930f34b3 100644
--- a/docs/system/arm/cpu-features.rst
+++ b/docs/system/arm/cpu-features.rst
@@ -204,6 +204,29 @@  the list of KVM VCPU features and their descriptions.
   the guest scheduler behavior and/or be exposed to the guest
   userspace.
 
+``kvm-pmu-filter``
+  By default kvm-pmu-filter is disabled. This means that by default all PMU
+  events will be exposed to guest.
+
+  KVM implements PMU Event Filtering to prevent a guest from being able to
+  sample certain events. It depends on the KVM_ARM_VCPU_PMU_V3_FILTER
+  attribute supported in KVM. It has the following format:
+
+  kvm-pmu-filter="{A,D}:start-end[;{A,D}:start-end...]"
+
+  The A means "allow" and D means "deny", start is the first event of the
+  range and the end is the last one. The first registered range defines
+  the global policy (global ALLOW if the first action is DENY, global DENY
+  if the first action is ALLOW). The start and end only support hexadecimal
+  format. For example:
+
+  kvm-pmu-filter="A:0x11-0x11;A:0x23-0x3a;D:0x30-0x30"
+
+  Since the first action is allow, we have a global deny policy. It
+  will allow event 0x11 (the cycle counter), events 0x23 to 0x3a are
+  also allowed except the event 0x30 which is denied, and all the other
+  events are denied.
+
 TCG VCPU Features
 =================
 
diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
index 2250cd7ddf..36df2e4820 100644
--- a/target/arm/arm-qmp-cmds.c
+++ b/target/arm/arm-qmp-cmds.c
@@ -94,7 +94,7 @@  static const char *cpu_model_advertised_features[] = {
     "sve128", "sve256", "sve384", "sve512",
     "sve640", "sve768", "sve896", "sve1024", "sve1152", "sve1280",
     "sve1408", "sve1536", "sve1664", "sve1792", "sve1920", "sve2048",
-    "kvm-no-adjvtime", "kvm-steal-time",
+    "kvm-no-adjvtime", "kvm-steal-time", "kvm-pmu-filter",
     "pauth", "pauth-impdef", "pauth-qarma3",
     NULL
 };
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 63f31e0d98..b810a80e67 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -948,6 +948,9 @@  struct ArchCPU {
 
     /* KVM steal time */
     OnOffAuto kvm_steal_time;
+
+    /* KVM PMU Filter */
+    GArray *kvm_pmu_filter;
 #endif /* CONFIG_KVM */
 
     /* Uniprocessor system with MP extensions */
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 81813030a5..7f62fad029 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -496,6 +496,72 @@  static void kvm_steal_time_set(Object *obj, bool value, Error **errp)
     ARM_CPU(obj)->kvm_steal_time = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
 }
 
+static char *kvm_pmu_filter_get(Object *obj, Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+    g_autoptr(GString) pmu_filter = g_string_new(NULL);
+    struct kvm_pmu_event_filter *filter;
+    char action;
+    int i;
+
+    if (!cpu->kvm_pmu_filter) {
+        return NULL;
+    }
+
+    for (i = 0; i < cpu->kvm_pmu_filter->len; i++) {
+        filter = &g_array_index(cpu->kvm_pmu_filter,
+                                struct kvm_pmu_event_filter, i);
+        if (i) {
+            g_string_append_c(pmu_filter, ';');
+        }
+        action = filter->action == KVM_PMU_EVENT_ALLOW ? 'A' : 'D';
+        g_string_append_printf(pmu_filter, "%c:0x%hx-0x%hx", action,
+                               filter->base_event,
+                               filter->base_event + filter->nevents - 1);
+    }
+
+    return g_strdup(pmu_filter->str);
+}
+
+static void kvm_pmu_filter_set(Object *obj, const char *pmu_filter,
+                               Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+    struct kvm_pmu_event_filter filter;
+    g_auto(GStrv) event_filters;
+    int i;
+
+    if (cpu->kvm_pmu_filter) {
+        g_array_free(cpu->kvm_pmu_filter, true);
+    }
+
+    cpu->kvm_pmu_filter = g_array_new(false, false,
+                                      sizeof(struct kvm_pmu_event_filter));
+
+    event_filters = g_strsplit(pmu_filter, ";", -1);
+    for (i = 0; event_filters[i]; i++) {
+        unsigned short start = 0, end = 0;
+        char act;
+
+        if (sscanf(event_filters[i], "%c:%hx-%hx", &act, &start, &end) != 3) {
+            warn_report("Skipping invalid PMU filter %s", event_filters[i]);
+            continue;
+        }
+
+        if ((act != 'A' && act != 'D') || start > end) {
+            warn_report("Skipping invalid PMU filter %s", event_filters[i]);
+            continue;
+        }
+
+        filter.base_event = start;
+        filter.nevents = end - start + 1;
+        filter.action = (act == 'A') ? KVM_PMU_EVENT_ALLOW :
+                                       KVM_PMU_EVENT_DENY;
+
+        g_array_append_vals(cpu->kvm_pmu_filter, &filter, 1);
+    }
+}
+
 /* KVM VCPU properties should be prefixed with "kvm-". */
 void kvm_arm_add_vcpu_properties(ARMCPU *cpu)
 {
@@ -517,6 +583,12 @@  void kvm_arm_add_vcpu_properties(ARMCPU *cpu)
                              kvm_steal_time_set);
     object_property_set_description(obj, "kvm-steal-time",
                                     "Set off to disable KVM steal time.");
+
+    object_property_add_str(obj, "kvm-pmu-filter", kvm_pmu_filter_get,
+                            kvm_pmu_filter_set);
+    object_property_set_description(obj, "kvm-pmu-filter",
+                                    "PMU Event Filtering description for "
+                                    "guest PMU. (default: NULL, disabled)");
 }
 
 bool kvm_arm_pmu_supported(void)
@@ -1706,6 +1778,47 @@  static bool kvm_arm_set_device_attr(ARMCPU *cpu, struct kvm_device_attr *attr,
     return true;
 }
 
+static void kvm_arm_pmu_filter_init(ARMCPU *cpu)
+{
+    static bool pmu_filter_init;
+    struct kvm_device_attr attr = {
+        .group      = KVM_ARM_VCPU_PMU_V3_CTRL,
+        .attr       = KVM_ARM_VCPU_PMU_V3_FILTER,
+    };
+    int i;
+
+    /*
+     * The filter only needs to be initialized through one vcpu ioctl and it
+     * will affect all other vcpu in the vm.
+     * It can be referred from kernel commit d7eec2360e3 ("KVM: arm64: Add PMU
+     * event filtering infrastructure"):
+     * Note that although the ioctl is per-vcpu, the map of allowed events is
+     * global to the VM (it can be setup from any vcpu until the vcpu PMU is
+     * initialized).
+     */
+    if (pmu_filter_init) {
+        return;
+    } else {
+        pmu_filter_init = true;
+    }
+
+    if (!cpu->kvm_pmu_filter) {
+        return;
+    }
+    if (kvm_vcpu_ioctl(CPU(cpu), KVM_HAS_DEVICE_ATTR, &attr)) {
+        error_report("KVM doesn't support the PMU Event Filter!");
+        return;
+    }
+
+    for (i = 0; i < cpu->kvm_pmu_filter->len; i++) {
+        attr.addr = (uint64_t)&g_array_index(cpu->kvm_pmu_filter,
+                                             struct kvm_pmu_event_filter, i);
+        if (!kvm_arm_set_device_attr(cpu, &attr, "PMU_V3_FILTER")) {
+            break;
+        }
+    }
+}
+
 void kvm_arm_pmu_init(ARMCPU *cpu)
 {
     struct kvm_device_attr attr = {
@@ -1716,6 +1829,8 @@  void kvm_arm_pmu_init(ARMCPU *cpu)
     if (!cpu->has_pmu) {
         return;
     }
+
+    kvm_arm_pmu_filter_init(cpu);
     if (!kvm_arm_set_device_attr(cpu, &attr, "PMU")) {
         error_report("failed to init PMU");
         abort();
diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index a8a4c668ad..60a5e32eb8 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -127,6 +127,17 @@  static bool resp_get_feature(QDict *resp, const char *feature)
     return qdict_get_bool(props, feature);
 }
 
+static const char *resp_get_feature_str(QDict *resp, const char *feature)
+{
+    QDict *props;
+
+    g_assert(resp);
+    g_assert(resp_has_props(resp));
+    props = resp_get_props(resp);
+    g_assert(qdict_get(props, feature));
+    return qdict_get_str(props, feature);
+}
+
 #define assert_has_feature(qts, cpu_type, feature)                     \
 ({                                                                     \
     QDict *_resp = do_query_no_props(qts, cpu_type);                   \
@@ -156,6 +167,18 @@  static bool resp_get_feature(QDict *resp, const char *feature)
     g_assert(qdict_get_bool(_props, feature) == (expected_value));     \
 })
 
+#define resp_assert_feature_str(resp, feature, expected_value)         \
+({                                                                     \
+    QDict *_props;                                                     \
+                                                                       \
+    g_assert(_resp);                                                   \
+    g_assert(resp_has_props(_resp));                                   \
+    _props = resp_get_props(_resp);                                    \
+    g_assert(qdict_get(_props, feature));                              \
+    g_assert_cmpstr(qdict_get_str(_props, feature),                    \
+                    ==, (expected_value));                             \
+})
+
 #define assert_feature(qts, cpu_type, feature, expected_value)         \
 ({                                                                     \
     QDict *_resp;                                                      \
@@ -177,6 +200,17 @@  static bool resp_get_feature(QDict *resp, const char *feature)
     qobject_unref(_resp);                                              \
 })
 
+#define assert_set_feature_str(qts, cpu_type, feature, value)          \
+({                                                                     \
+    const char *_fmt = "{ %s: %s }";                                   \
+    QDict *_resp;                                                      \
+                                                                       \
+    _resp = do_query(qts, cpu_type, _fmt, feature, value);             \
+    g_assert(_resp);                                                   \
+    resp_assert_feature_str(_resp, feature, value);                    \
+    qobject_unref(_resp);                                              \
+})
+
 #define assert_has_feature_enabled(qts, cpu_type, feature)             \
     assert_feature(qts, cpu_type, feature, true)
 
@@ -461,6 +495,7 @@  static void test_query_cpu_model_expansion(const void *data)
 
     assert_has_not_feature(qts, "max", "kvm-no-adjvtime");
     assert_has_not_feature(qts, "max", "kvm-steal-time");
+    assert_has_not_feature(qts, "max", "kvm-pmu-filter");
 
     if (g_str_equal(qtest_get_arch(), "aarch64")) {
         assert_has_feature_enabled(qts, "max", "aarch64");
@@ -508,6 +543,7 @@  static void test_query_cpu_model_expansion_kvm(const void *data)
     assert_set_feature(qts, "host", "kvm-no-adjvtime", false);
 
     if (g_str_equal(qtest_get_arch(), "aarch64")) {
+        const char *kvm_supports_pmu_filter;
         bool kvm_supports_steal_time;
         bool kvm_supports_sve;
         char max_name[8], name[8];
@@ -546,15 +582,29 @@  static void test_query_cpu_model_expansion_kvm(const void *data)
          * because this instance of KVM doesn't support them. Test that the
          * features are present, and, when enabled, issue further tests.
          */
+        assert_has_feature(qts, "host", "kvm-pmu-filter");
         assert_has_feature(qts, "host", "kvm-steal-time");
         assert_has_feature(qts, "host", "sve");
 
         resp = do_query_no_props(qts, "host");
+        kvm_supports_pmu_filter = resp_get_feature_str(resp, "kvm-pmu-filter");
         kvm_supports_steal_time = resp_get_feature(resp, "kvm-steal-time");
         kvm_supports_sve = resp_get_feature(resp, "sve");
         vls = resp_get_sve_vls(resp);
         qobject_unref(resp);
 
+        if (kvm_supports_pmu_filter) {
+            assert_set_feature_str(qts, "host", "kvm-pmu-filter", "");
+            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
+                                   "A:0x11-0x11");
+            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
+                                   "D:0x11-0x11");
+            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
+                                   "A:0x11-0x11;A:0x12-0x20");
+            assert_set_feature_str(qts, "host", "kvm-pmu-filter",
+                                   "D:0x11-0x11;A:0x12-0x20;D:0x12-0x15");
+        }
+
         if (kvm_supports_steal_time) {
             /* If we have steal-time then we should be able to toggle it. */
             assert_set_feature(qts, "host", "kvm-steal-time", false);
@@ -622,6 +672,7 @@  static void test_query_cpu_model_expansion_kvm(const void *data)
         assert_has_not_feature(qts, "host", "pmu");
         assert_has_not_feature(qts, "host", "sve");
         assert_has_not_feature(qts, "host", "kvm-steal-time");
+        assert_has_not_feature(qts, "host", "kvm-pmu-filter");
     }
 
     qtest_quit(qts);