Re: [PATCH v2 12/12] target/arm: spe: Add corresponding doc and test.

[Haibo Xu]

On Tue, 8 Sep 2020 at 19:41, Andrew Jones <drjones@redhat.com> wrote:
>
> On Tue, Sep 08, 2020 at 08:13:30AM +0000, Haibo Xu wrote:
> > Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> > ---
> >  docs/system/arm/cpu-features.rst | 20 ++++++++++++++++++++
> >  target/arm/monitor.c             |  2 +-
> >  tests/qtest/arm-cpu-features.c   |  9 +++++++++
> >  3 files changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/system/arm/cpu-features.rst 
> > b/docs/system/arm/cpu-features.rst
> > index 2d5c06cd01..5b81b9a560 100644
> > --- a/docs/system/arm/cpu-features.rst
> > +++ b/docs/system/arm/cpu-features.rst
> > @@ -344,3 +344,23 @@ verbose command lines.  However, the recommended way 
> > to select vector
> >  lengths is to explicitly enable each desired length.  Therefore only
> >  example's (1), (4), and (6) exhibit recommended uses of the properties.
> >
> > +SPE CPU Property
> > +==================
>
> Too many '='
>
> > +
> > +The SPE CPU property `spe` is used to enable or disable the SPE feature,
> > +just as the `pmu` CPU property completely enables or disables the PMU.
> > +
> > +Currently, this property is only available with KVM mode, and is enabled
> > +by default if KVM support it. When KVM is enabled, if the host does not
> > +support SPE, then an error is generated when attempting to enable it.
> > +
> > +Following are 2 examples to use this property:
> > +
> > +  1) Disable SPE::
> > +
> > +     $ qemu-system-aarch64 -M virt,accel=kvm -cpu max,spe=off
> > +
> > +  2) Implicitly enable it with the `host` CPU type if host cpu
> > +     support it::
>
> if the host CPU supports it
>
>
> Actually, I'm not sure we need to document this feature. We didn't bother
> documenting pauth, since there wasn't anything special about it and
> there's nothing special about this feature either.
>

Yes, there is no special treatment for this feature, and it just
follows the syntax
of other vCPU features. Will remove this doc in v3.
Anyway, thanks so much for the review!

Regards,
Haibo

> > +
> > +     $ qemu-system-aarch64 -M virt,accel=kvm -cpu host
> > diff --git a/target/arm/monitor.c b/target/arm/monitor.c
> > index ba6e01abd0..1b8f08988a 100644
> > --- a/target/arm/monitor.c
> > +++ b/target/arm/monitor.c
> > @@ -99,7 +99,7 @@ QEMU_BUILD_BUG_ON(ARM_MAX_VQ > 16);
> >   * then the order that considers those dependencies must be used.
> >   */
> >  static const char *cpu_model_advertised_features[] = {
> > -    "aarch64", "pmu", "sve",
> > +    "aarch64", "pmu", "spe", "sve",
> >      "sve128", "sve256", "sve384", "sve512",
> >      "sve640", "sve768", "sve896", "sve1024", "sve1152", "sve1280",
> >      "sve1408", "sve1536", "sve1664", "sve1792", "sve1920", "sve2048",
> > diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
> > index 77b5e30a9c..4d393fb2e2 100644
> > --- a/tests/qtest/arm-cpu-features.c
> > +++ b/tests/qtest/arm-cpu-features.c
> > @@ -494,6 +494,7 @@ static void test_query_cpu_model_expansion_kvm(const 
> > void *data)
> >
> >      if (g_str_equal(qtest_get_arch(), "aarch64")) {
> >          bool kvm_supports_sve;
> > +        bool kvm_supports_spe;
> >          char max_name[8], name[8];
> >          uint32_t max_vq, vq;
> >          uint64_t vls;
> > @@ -512,8 +513,10 @@ static void test_query_cpu_model_expansion_kvm(const 
> > void *data)
> >              "with KVM on this host", NULL);
> >
> >          assert_has_feature(qts, "host", "sve");
> > +        assert_has_feature(qts, "host", "spe");
> >          resp = do_query_no_props(qts, "host");
> >          kvm_supports_sve = resp_get_feature(resp, "sve");
> > +        kvm_supports_spe = resp_get_feature(resp, "spe");
> >          vls = resp_get_sve_vls(resp);
> >          qobject_unref(resp);
> >
> > @@ -573,10 +576,16 @@ static void test_query_cpu_model_expansion_kvm(const 
> > void *data)
> >          } else {
> >              g_assert(vls == 0);
> >          }
> > +
> > +        if (kvm_supports_spe) {
> > +                assert_set_feature(qts, "host", "spe", false);
> > +                assert_set_feature(qts, "host", "spe", true);
> > +        }
> >      } else {
> >          assert_has_not_feature(qts, "host", "aarch64");
> >          assert_has_not_feature(qts, "host", "pmu");
> >          assert_has_not_feature(qts, "host", "sve");
> > +        assert_has_not_feature(qts, "host", "spe");
> >      }
> >
> >      qtest_quit(qts);
> > --
> > 2.17.1
> >
>
> Otherwise
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>