[PATCH v7 0/9] target/arm/kvm: enable SVE in guests

Andrew Jones posted 9 patches 4 years, 5 months ago
Test FreeBSD passed
Test asan passed
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191024121808.9612-1-drjones@redhat.com
Maintainers: Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Eduardo Habkost <ehabkost@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
There is a newer version of this series
docs/arm-cpu-features.rst | 317 ++++++++++++++++++++++
include/qemu/bitops.h     |   1 +
qapi/machine-target.json  |   6 +-
target/arm/cpu.c          |  25 +-
target/arm/cpu.h          |  21 ++
target/arm/cpu64.c        | 356 ++++++++++++++++++++++++-
target/arm/helper.c       |  10 +-
target/arm/kvm.c          |  25 +-
target/arm/kvm32.c        |   6 +-
target/arm/kvm64.c        | 323 ++++++++++++++++++++---
target/arm/kvm_arm.h      |  39 +++
target/arm/monitor.c      | 158 +++++++++++
tests/Makefile.include    |   5 +-
tests/arm-cpu-features.c  | 540 ++++++++++++++++++++++++++++++++++++++
14 files changed, 1775 insertions(+), 57 deletions(-)
create mode 100644 docs/arm-cpu-features.rst
create mode 100644 tests/arm-cpu-features.c
[PATCH v7 0/9] target/arm/kvm: enable SVE in guests
Posted by Andrew Jones 4 years, 5 months ago
Since Linux kernel v5.2-rc1 KVM has support for enabling SVE in guests.
This series provides the QEMU bits for that enablement. First, we
select existing CPU properties representing features we want to
advertise in addition to the SVE vector lengths and prepare
them for a qmp query. Then we introduce the qmp query, applying
it immediately to those selected features. We also document ARM CPU
features at this time. We next add a qtest for the selected CPU
features that uses the qmp query for its tests - and we continue to
add tests as we add CPU features with the following patches. So then,
once we have the support we need for CPU feature querying and testing,
we add our first SVE CPU feature property, 'sve', which just allows
SVE to be completely enabled/disabled. Following that feature property,
we add all 16 vector length properties along with the input validation
they need and tests to prove the validation works. At this point the
SVE features are still only for TCG, so we provide some patches to
prepare for KVM and then a patch that allows the 'max' CPU type to
enable SVE with KVM, but at first without vector length properties.
After a bit more preparation we add the SVE vector length properties
to the KVM-enabled 'max' CPU type along with the additional input
validation and tests that that needs.  Finally we allow the 'host'
CPU type to also enjoy these properties by simply sharing them with it.

v7:
  - Fixed kvm32 tests and added a couple
  - Picked up r-b's from Beata and t-b's from Masa

v6:
  - Adding missing visit_free() to an error path in
    qmp_query_cpu_model_expansion() in patch 1/9.
  - Rebased on latest master (applied cleanly)
  - Picked up r-b's from Richard and Eric

v5:
  - Now generate an error if vector lengths have been explicitly
    enabled, but SVE is disabled
  - Fixed a bug in sve_zcr_len_for_el()
  - Fixed a bug in kvm_arch_put/get_sve() and brought back the
    put/get of FPSR/FPCR
  - A few document clarifications and added some new sentences
  - Added a couple more tests
  - Added BIT_ULL and use it in the test
  - Removed an unnecessary bitmap search
  - Moved a cpu_max_get_sve_max_vq() hunk from 4/9 to 3/9 and
    added a fix for it in 8/9
  - Picked up some more tags from Eric

v4:
  - Integrated Richard Henderson's rework for the sve property
    validation, in order to do all validating at finalize time
    and save several lines of code.
  - Fixed 'host' cpu SVE default. It was still off by default.
  - Cleaned up #ifdef's for sve_bswap64()
  - Removed redundant KVM_CAP_ARM_SVE extension check in
    kvm_arm_sve_get_vls()
  - Improved the KVM SVE qtest
  - Renamed sve<vl-bits> to sve<N> everywhere
  - Renamed power-of-2 to power-of-two everywhere
  - Picked up some more tags from Richard

Thanks!
drew


Andrew Jones (9):
  target/arm/monitor: Introduce qmp_query_cpu_model_expansion
  tests: arm: Introduce cpu feature tests
  target/arm: Allow SVE to be disabled via a CPU property
  target/arm/cpu64: max cpu: Introduce sve<N> properties
  target/arm/kvm64: Add kvm_arch_get/put_sve
  target/arm/kvm64: max cpu: Enable SVE when available
  target/arm/kvm: scratch vcpu: Preserve input kvm_vcpu_init features
  target/arm/cpu64: max cpu: Support sve properties with KVM
  target/arm/kvm: host cpu: Add support for sve<N> properties

 docs/arm-cpu-features.rst | 317 ++++++++++++++++++++++
 include/qemu/bitops.h     |   1 +
 qapi/machine-target.json  |   6 +-
 target/arm/cpu.c          |  25 +-
 target/arm/cpu.h          |  21 ++
 target/arm/cpu64.c        | 356 ++++++++++++++++++++++++-
 target/arm/helper.c       |  10 +-
 target/arm/kvm.c          |  25 +-
 target/arm/kvm32.c        |   6 +-
 target/arm/kvm64.c        | 323 ++++++++++++++++++++---
 target/arm/kvm_arm.h      |  39 +++
 target/arm/monitor.c      | 158 +++++++++++
 tests/Makefile.include    |   5 +-
 tests/arm-cpu-features.c  | 540 ++++++++++++++++++++++++++++++++++++++
 14 files changed, 1775 insertions(+), 57 deletions(-)
 create mode 100644 docs/arm-cpu-features.rst
 create mode 100644 tests/arm-cpu-features.c

-- 
2.21.0


Re: [PATCH v7 0/9] target/arm/kvm: enable SVE in guests
Posted by Peter Maydell 4 years, 5 months ago
On Thu, 24 Oct 2019 at 13:18, Andrew Jones <drjones@redhat.com> wrote:
>
> Since Linux kernel v5.2-rc1 KVM has support for enabling SVE in guests.
> This series provides the QEMU bits for that enablement. First, we
> select existing CPU properties representing features we want to
> advertise in addition to the SVE vector lengths and prepare
> them for a qmp query. Then we introduce the qmp query, applying
> it immediately to those selected features. We also document ARM CPU
> features at this time. We next add a qtest for the selected CPU
> features that uses the qmp query for its tests - and we continue to
> add tests as we add CPU features with the following patches. So then,
> once we have the support we need for CPU feature querying and testing,
> we add our first SVE CPU feature property, 'sve', which just allows
> SVE to be completely enabled/disabled. Following that feature property,
> we add all 16 vector length properties along with the input validation
> they need and tests to prove the validation works. At this point the
> SVE features are still only for TCG, so we provide some patches to
> prepare for KVM and then a patch that allows the 'max' CPU type to
> enable SVE with KVM, but at first without vector length properties.
> After a bit more preparation we add the SVE vector length properties
> to the KVM-enabled 'max' CPU type along with the additional input
> validation and tests that that needs.  Finally we allow the 'host'
> CPU type to also enjoy these properties by simply sharing them with it.
>



Applied to target-arm.next, thanks.

-- PMM

Re: [PATCH v7 0/9] target/arm/kvm: enable SVE in guests
Posted by Peter Maydell 4 years, 5 months ago
On Thu, 24 Oct 2019 at 14:42, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 24 Oct 2019 at 13:18, Andrew Jones <drjones@redhat.com> wrote:
> >
> > Since Linux kernel v5.2-rc1 KVM has support for enabling SVE in guests.
> > This series provides the QEMU bits for that enablement. First, we
> > select existing CPU properties representing features we want to
> > advertise in addition to the SVE vector lengths and prepare
> > them for a qmp query. Then we introduce the qmp query, applying
> > it immediately to those selected features. We also document ARM CPU
> > features at this time. We next add a qtest for the selected CPU
> > features that uses the qmp query for its tests - and we continue to
> > add tests as we add CPU features with the following patches. So then,
> > once we have the support we need for CPU feature querying and testing,
> > we add our first SVE CPU feature property, 'sve', which just allows
> > SVE to be completely enabled/disabled. Following that feature property,
> > we add all 16 vector length properties along with the input validation
> > they need and tests to prove the validation works. At this point the
> > SVE features are still only for TCG, so we provide some patches to
> > prepare for KVM and then a patch that allows the 'max' CPU type to
> > enable SVE with KVM, but at first without vector length properties.
> > After a bit more preparation we add the SVE vector length properties
> > to the KVM-enabled 'max' CPU type along with the additional input
> > validation and tests that that needs.  Finally we allow the 'host'
> > CPU type to also enjoy these properties by simply sharing them with it.
> >
>
>
>
> Applied to target-arm.next, thanks.

Fails 'make check' on my aarch32-compile-in-chroot-on-aarch64
machine:

(armhf)pmaydell@mustang-maydell:~/qemu/build/all-a32$
QTEST_QEMU_BINARY=arm-softmmu/qemu-system-arm tests/arm-cpu-features
/arm/arm/query-cpu-model-expansion: OK
/arm/arm/kvm/query-cpu-model-expansion: qemu-system-arm: Failed to
retrieve host CPU features
Broken pipe
/home/peter.maydell/qemu/tests/libqtest.c:140: kill_qemu() tried to
terminate QEMU process but encountered exit status 1 (expected 0)
Aborted

Dropping again :-(

thanks
-- PMM

Re: [PATCH v7 0/9] target/arm/kvm: enable SVE in guests
Posted by Andrew Jones 4 years, 5 months ago
On Fri, Oct 25, 2019 at 01:06:26PM +0100, Peter Maydell wrote:
> On Thu, 24 Oct 2019 at 14:42, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Thu, 24 Oct 2019 at 13:18, Andrew Jones <drjones@redhat.com> wrote:
> > >
> > > Since Linux kernel v5.2-rc1 KVM has support for enabling SVE in guests.
> > > This series provides the QEMU bits for that enablement. First, we
> > > select existing CPU properties representing features we want to
> > > advertise in addition to the SVE vector lengths and prepare
> > > them for a qmp query. Then we introduce the qmp query, applying
> > > it immediately to those selected features. We also document ARM CPU
> > > features at this time. We next add a qtest for the selected CPU
> > > features that uses the qmp query for its tests - and we continue to
> > > add tests as we add CPU features with the following patches. So then,
> > > once we have the support we need for CPU feature querying and testing,
> > > we add our first SVE CPU feature property, 'sve', which just allows
> > > SVE to be completely enabled/disabled. Following that feature property,
> > > we add all 16 vector length properties along with the input validation
> > > they need and tests to prove the validation works. At this point the
> > > SVE features are still only for TCG, so we provide some patches to
> > > prepare for KVM and then a patch that allows the 'max' CPU type to
> > > enable SVE with KVM, but at first without vector length properties.
> > > After a bit more preparation we add the SVE vector length properties
> > > to the KVM-enabled 'max' CPU type along with the additional input
> > > validation and tests that that needs.  Finally we allow the 'host'
> > > CPU type to also enjoy these properties by simply sharing them with it.
> > >
> >
> >
> >
> > Applied to target-arm.next, thanks.
> 
> Fails 'make check' on my aarch32-compile-in-chroot-on-aarch64
> machine:

Are there easy-to-follow instructions for setting this environment up
somewhere?

> 
> (armhf)pmaydell@mustang-maydell:~/qemu/build/all-a32$
> QTEST_QEMU_BINARY=arm-softmmu/qemu-system-arm tests/arm-cpu-features
> /arm/arm/query-cpu-model-expansion: OK
> /arm/arm/kvm/query-cpu-model-expansion: qemu-system-arm: Failed to
> retrieve host CPU features
> Broken pipe

I guess the problem is how we're determining if KVM is available, which
is like this

int main(int argc, char **argv)
{
    bool kvm_available = false;

    if (!access("/dev/kvm",  R_OK | W_OK)) {
#if defined(HOST_AARCH64)
        kvm_available = g_str_equal(qtest_get_arch(), "aarch64");
#elif defined(HOST_ARM)
        kvm_available = g_str_equal(qtest_get_arch(), "arm");
#endif
    }


So we need /dev/kvm and the QEMU binary arch type (qemu-system-arm in
this case) needs to match the host arch type. The problem is that
HOST_<type> doesn't imply anything about the actual host arch type.
<type> comes from the configure $ARCH variable, which for 'arm'
comes from the $cpu variable, which for 'arm' comes from whether or
not the compiler defines __arm__, and cross compilers certainly do.
I guess we'd have the same problem in an aarch32-compile-in-chroot-on-
<any-other-type> environment, if a cross compiler is used for the
compiling. I should change the KVM available check to something that
uses the actual host arch type. I assume the following works, but
I don't know if I'm allowed to use uname() in these tests, and, if
not, then I don't know what the right way to get the actual host
type is.


diff --git a/tests/arm-cpu-features.c b/tests/arm-cpu-features.c
index 6b8c48de8aa8..712af2b405fb 100644
--- a/tests/arm-cpu-features.c
+++ b/tests/arm-cpu-features.c
@@ -13,6 +13,7 @@
 #include "libqtest.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
+#include <sys/utsname.h>
 
 /*
  * We expect the SVE max-vq to be 16. Also it must be <= 64
@@ -506,13 +507,16 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
 int main(int argc, char **argv)
 {
     bool kvm_available = false;
+    struct utsname u;
+
+    g_assert(uname(&u) == 0);
 
     if (!access("/dev/kvm",  R_OK | W_OK)) {
-#if defined(HOST_AARCH64)
-        kvm_available = g_str_equal(qtest_get_arch(), "aarch64");
-#elif defined(HOST_ARM)
-        kvm_available = g_str_equal(qtest_get_arch(), "arm");
-#endif
+        if (g_str_equal(u.machine, "aarch64")) {
+            kvm_available = g_str_equal(qtest_get_arch(), "aarch64");
+        } else if (!strncmp(u.machine, "arm", 3)) {
+            kvm_available = g_str_equal(qtest_get_arch(), "arm");
+        }
     }
 
     g_test_init(&argc, &argv, NULL);

> /home/peter.maydell/qemu/tests/libqtest.c:140: kill_qemu() tried to
> terminate QEMU process but encountered exit status 1 (expected 0)
> Aborted
> 
> Dropping again :-(

Sigh...

Thanks,
drew


Re: [PATCH v7 0/9] target/arm/kvm: enable SVE in guests
Posted by Peter Maydell 4 years, 5 months ago
On Fri, 25 Oct 2019 at 14:52, Andrew Jones <drjones@redhat.com> wrote:
>
> On Fri, Oct 25, 2019 at 01:06:26PM +0100, Peter Maydell wrote:
> > Fails 'make check' on my aarch32-compile-in-chroot-on-aarch64
> > machine:
>
> Are there easy-to-follow instructions for setting this environment up
> somewhere?

It's an ancient setup, so not really. But it's just an
Ubuntu 32-bit chroot inside an Ubuntu 64-bit host. I use
schroot to manage it.



> I guess the problem is how we're determining if KVM is available, which
> is like this
>
> int main(int argc, char **argv)
> {
>     bool kvm_available = false;
>
>     if (!access("/dev/kvm",  R_OK | W_OK)) {
> #if defined(HOST_AARCH64)
>         kvm_available = g_str_equal(qtest_get_arch(), "aarch64");
> #elif defined(HOST_ARM)
>         kvm_available = g_str_equal(qtest_get_arch(), "arm");
> #endif
>     }

> So we need /dev/kvm and the QEMU binary arch type (qemu-system-arm in
> this case) needs to match the host arch type. The problem is that
> HOST_<type> doesn't imply anything about the actual host arch type.
> <type> comes from the configure $ARCH variable, which for 'arm'
> comes from the $cpu variable, which for 'arm' comes from whether or
> not the compiler defines __arm__, and cross compilers certainly do.
> I guess we'd have the same problem in an aarch32-compile-in-chroot-on-
> <any-other-type> environment, if a cross compiler is used for the
> compiling. I should change the KVM available check to something that
> uses the actual host arch type. I assume the following works, but
> I don't know if I'm allowed to use uname() in these tests, and, if
> not, then I don't know what the right way to get the actual host
> type is.

This all feels like it's trying to do the wrong thing, ie
replicate the logic within QEMU that decides whether to use
KVM or not. Some possible other approaches:

 * If you want to know whether you can run the qemu binary
   with -accel kvm, then just try that and see if it succeeds.
 * Or use '-accel kvm:tcg' and make the test work so that it
   will pass for both KVM and TCG
 * Or use QMP to query what accelerators are available, assuming
   there's a QMP command for that

thanks
-- PMM

Re: [PATCH v7 0/9] target/arm/kvm: enable SVE in guests
Posted by Andrew Jones 4 years, 5 months ago
Apologies if this mail is messed up. I'm having trouble with a hotel's network and am forced to use a webui.

----- Original Message -----
> On Fri, 25 Oct 2019 at 14:52, Andrew Jones <drjones@redhat.com> wrote:
> >
> > On Fri, Oct 25, 2019 at 01:06:26PM +0100, Peter Maydell wrote:
> > > Fails 'make check' on my aarch32-compile-in-chroot-on-aarch64
> > > machine:
> >
> > Are there easy-to-follow instructions for setting this environment up
> > somewhere?
> 
> It's an ancient setup, so not really. But it's just an
> Ubuntu 32-bit chroot inside an Ubuntu 64-bit host. I use
> schroot to manage it.
> 
> 
> 
> > I guess the problem is how we're determining if KVM is available, which
> > is like this
> >
> > int main(int argc, char **argv)
> > {
> >     bool kvm_available = false;
> >
> >     if (!access("/dev/kvm",  R_OK | W_OK)) {
> > #if defined(HOST_AARCH64)
> >         kvm_available = g_str_equal(qtest_get_arch(), "aarch64");
> > #elif defined(HOST_ARM)
> >         kvm_available = g_str_equal(qtest_get_arch(), "arm");
> > #endif
> >     }
> 
> > So we need /dev/kvm and the QEMU binary arch type (qemu-system-arm in
> > this case) needs to match the host arch type. The problem is that
> > HOST_<type> doesn't imply anything about the actual host arch type.
> > <type> comes from the configure $ARCH variable, which for 'arm'
> > comes from the $cpu variable, which for 'arm' comes from whether or
> > not the compiler defines __arm__, and cross compilers certainly do.
> > I guess we'd have the same problem in an aarch32-compile-in-chroot-on-
> > <any-other-type> environment, if a cross compiler is used for the
> > compiling. I should change the KVM available check to something that
> > uses the actual host arch type. I assume the following works, but
> > I don't know if I'm allowed to use uname() in these tests, and, if
> > not, then I don't know what the right way to get the actual host
> > type is.
> 
> This all feels like it's trying to do the wrong thing, ie
> replicate the logic within QEMU that decides whether to use
> KVM or not. Some possible other approaches:
> 
>  * If you want to know whether you can run the qemu binary
>    with -accel kvm, then just try that and see if it succeeds.

That's what's happening here, but the failure happens in qtest_init(), so
there's no way to avoid the assert.

>  * Or use '-accel kvm:tcg' and make the test work so that it
>    will pass for both KVM and TCG

For the KVM tests, I'd prefer we specifically test the 'host' cpu type,
which requires we specifically test with KVM. That said, we can use
the fallback along with an additional query to make sure 'host' is
ok to use.

>  * Or use QMP to query what accelerators are available, assuming
>    there's a QMP command for that

Looks like we can check for kvm with query-kvm. 

I'll rework patch 2/9 using the fallback and kvm query to ensure
kvm can be used. I can even add a test or two that ensure 'host'
is ok to use.

Thanks,
drew