[Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests

Andrew Jones posted 13 patches 4 years, 10 months ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu failed
Test checkpatch failed
Test asan passed
Failed in applying to current master (apply log)
There is a newer version of this series
linux-headers/asm-arm64/kvm.h         |  41 +++
linux-headers/asm-arm64/sve_context.h |  53 ++++
linux-headers/linux/kvm.h             |   5 +
qapi/target.json                      |  34 +++
scripts/update-linux-headers.sh       |   3 +
target/arm/cpu.c                      |   5 +
target/arm/cpu.h                      |   9 +
target/arm/cpu64.c                    |  81 ++++-
target/arm/helper.c                   |  10 +-
target/arm/kvm.c                      |   5 +
target/arm/kvm64.c                    | 418 ++++++++++++++++++++++----
target/arm/kvm_arm.h                  |  32 ++
target/arm/monitor.c                  | 106 +++++++
tests/qmp-cmd-test.c                  |   1 +
14 files changed, 732 insertions(+), 71 deletions(-)
create mode 100644 linux-headers/asm-arm64/sve_context.h
[Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrew Jones 4 years, 10 months ago
With the recent KVM guest SVE support pull request [1] KVM will be
ready for guests with SVE. This series provides the QEMU bits for
that enablement. The series starts with the bits needed for the KVM
SVE ioctls. Then it enables the arm 'max'cpu type, which with TCG
already supports SVE, to also support SVE when using KVM. Next
a new QMP query is added that allows users to ask which vector
lengths are supported by the host, allowing them to select a valid
set of vectors for the guest. In order to select those vectors a
new property 'sve-vls-map' is added to the 'max' cpu type, and then
also to the 'host' cpu type. The table below shows the resulting user
interfaces.

   CPU type | accel | sve-max-vq | sve-vls-map
   -------------------------------------------
 1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
 2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
 3)    host | kvm   |  N/A       |  $VLS_MAP

Where for (1) $MAX_VQ sets the maximum vq and smaller vqs are
all supported when $VLS_MAP is zero, or when the vqs are selected
in $VLS_MAP.

(2) is the same as (1) except KVM has the final say on what
vqs are valid.

(3) doesn't accept sve-max-vq because a guest that uses this
property without sve-vls-map cannot be safely migrated.

There is never any need to provide both properties, but if both
are provided then they are checked for consistency.

The QMP query returns a list of valid vq lists. For example, if
a guest can use vqs 1, 2, 3, and 4, then the following list will
be returned

 [ [ 1 ], [ 1, 2 ], [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ]

Another example might be 1, 2, 4, as the architecture states 3
is optional. In that case the list would be

 [ [ 1 ], [ 1, 2 ], [ 1, 2, 4 ] ]

This may look redundant, but it's necessary to provide a future-
proof query, because while KVM currently requires vector sets to
be strict truncations of the full valid vector set, that may change
at some point. Additionally, TCG doesn't have this restriction so
more vector sets can be returned that aren't so easily derived from
the full set already. (Note, this series doesn't do that yet. See
the TODO below.)

And now for what might be a bit more controversial; how we input
the valid vector set with sve-vls-map. Well, sve-vls-map is a
64-bit bitmap, which is admittedly not user friendly and also not
the same size as KVM's vls bitmap (which is 8 64-bit words). Here's
the justification:

 1) We want to use the QEMU command line in order for the information
    to be migrated without needing to add more VM state.
 2) It's not easy/pretty to input arrays on the QEMU command line.
 3) We already need to use the QMP query to get a valid set, which
    isn't user friendly either, meaning we're already in libvirt
    territory.
 4) A 64-bit map (supporting up to 8192-bit vectors) is likely big
    enough for quite some time (currently KVM and TCG only support
    2048-bit vectors).
 5) If user friendliness is needed more than migratability then
    the 'max' cpu type can be used with the sve-max-vq property.
 6) It's possible to probe the full valid vector set from the
    command line by using something like sve-vls-map=0xffff and
    then, when it fails, the error message will state the correct
    map, e.g. 0xb.

TODO:
 1) More testing. Initial testing looks good, and I'm doing more
    now, but wanted to get the series out for review in parallel.
 2) Extension to target/arm/arch_dump.c for SVE state. I'll try
    to get to this early next week.
 3) Modify the QMP query to output all valid vector sets for
    TCG, rather than just the ones derived by truncation as
    is required by KVM.

[1] https://www.spinics.net/lists/kvm-arm/msg35844.html

Thanks!

Andrew Jones (13):
  target/arm/kvm64: fix error returns
  update-linux-headers: Add sve_context.h to asm-arm64
  HACK: linux header update
  target/arm/kvm: Move the get/put of fpsimd registers out
  target/arm/kvm: Add kvm_arch_get/put_sve
  target/arm/kvm: max cpu: Enable SVE when available
  target/arm/kvm: max cpu: Allow sve max vector length setting
  target/arm/monitor: Add query-sve-vector-lengths
  target/arm/kvm: Export kvm_arm_get_sve_vls
  target/arm/monitor: kvm: only return valid sve vector sets
  target/arm/cpu64: max cpu: Introduce sve-vls-map
  target/arm/kvm: max cpu: Add support for sve-vls-map
  target/arm/kvm: host cpu: Add support for sve-vls-map

 linux-headers/asm-arm64/kvm.h         |  41 +++
 linux-headers/asm-arm64/sve_context.h |  53 ++++
 linux-headers/linux/kvm.h             |   5 +
 qapi/target.json                      |  34 +++
 scripts/update-linux-headers.sh       |   3 +
 target/arm/cpu.c                      |   5 +
 target/arm/cpu.h                      |   9 +
 target/arm/cpu64.c                    |  81 ++++-
 target/arm/helper.c                   |  10 +-
 target/arm/kvm.c                      |   5 +
 target/arm/kvm64.c                    | 418 ++++++++++++++++++++++----
 target/arm/kvm_arm.h                  |  32 ++
 target/arm/monitor.c                  | 106 +++++++
 tests/qmp-cmd-test.c                  |   1 +
 14 files changed, 732 insertions(+), 71 deletions(-)
 create mode 100644 linux-headers/asm-arm64/sve_context.h

-- 
2.20.1


Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Peter Maydell 4 years, 10 months ago
On Sun, 12 May 2019 at 09:36, Andrew Jones <drjones@redhat.com> wrote:
>
> With the recent KVM guest SVE support pull request [1] KVM will be
> ready for guests with SVE. This series provides the QEMU bits for
> that enablement. The series starts with the bits needed for the KVM
> SVE ioctls. Then it enables the arm 'max'cpu type, which with TCG
> already supports SVE, to also support SVE when using KVM. Next
> a new QMP query is added that allows users to ask which vector
> lengths are supported by the host, allowing them to select a valid
> set of vectors for the guest. In order to select those vectors a
> new property 'sve-vls-map' is added to the 'max' cpu type, and then
> also to the 'host' cpu type. The table below shows the resulting user
> interfaces.
>
>    CPU type | accel | sve-max-vq | sve-vls-map
>    -------------------------------------------
>  1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
>  2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
>  3)    host | kvm   |  N/A       |  $VLS_MAP
>
> Where for (1) $MAX_VQ sets the maximum vq and smaller vqs are
> all supported when $VLS_MAP is zero, or when the vqs are selected
> in $VLS_MAP.
>
> (2) is the same as (1) except KVM has the final say on what
> vqs are valid.
>
> (3) doesn't accept sve-max-vq because a guest that uses this
> property without sve-vls-map cannot be safely migrated.

Is this "migrated between two hosts with the same host CPU
type but with different kernel versions which expose different
subsets of the host's permitted vector lengths" ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrew Jones 4 years, 10 months ago
On Mon, May 13, 2019 at 10:52:06AM +0100, Peter Maydell wrote:
> On Sun, 12 May 2019 at 09:36, Andrew Jones <drjones@redhat.com> wrote:
> >
> > With the recent KVM guest SVE support pull request [1] KVM will be
> > ready for guests with SVE. This series provides the QEMU bits for
> > that enablement. The series starts with the bits needed for the KVM
> > SVE ioctls. Then it enables the arm 'max'cpu type, which with TCG
> > already supports SVE, to also support SVE when using KVM. Next
> > a new QMP query is added that allows users to ask which vector
> > lengths are supported by the host, allowing them to select a valid
> > set of vectors for the guest. In order to select those vectors a
> > new property 'sve-vls-map' is added to the 'max' cpu type, and then
> > also to the 'host' cpu type. The table below shows the resulting user
> > interfaces.
> >
> >    CPU type | accel | sve-max-vq | sve-vls-map
> >    -------------------------------------------
> >  1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
> >  2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
> >  3)    host | kvm   |  N/A       |  $VLS_MAP
> >
> > Where for (1) $MAX_VQ sets the maximum vq and smaller vqs are
> > all supported when $VLS_MAP is zero, or when the vqs are selected
> > in $VLS_MAP.
> >
> > (2) is the same as (1) except KVM has the final say on what
> > vqs are valid.
> >
> > (3) doesn't accept sve-max-vq because a guest that uses this
> > property without sve-vls-map cannot be safely migrated.
> 
> Is this "migrated between two hosts with the same host CPU
> type but with different kernel versions which expose different
> subsets of the host's permitted vector lengths" ?
>

That's one example. Also, I see attempting to make/use only migrate-safe
cpu properties for our primary KVM cpu type ('host') as a step in the
right direction to eventually making a more migratable KVM cpu type. But
I do see that (3) is a bit of an oxymoron considering the 'host' cpu
type isn't really migrate-safe unless the hosts are identical (which
should technically require the same host kernel too) anyway.

Thanks,
drew

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrea Bolognani 4 years, 10 months ago
On Sun, 2019-05-12 at 10:36 +0200, Andrew Jones wrote:
[...]
>    CPU type | accel | sve-max-vq | sve-vls-map
>    -------------------------------------------
>  1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
>  2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
>  3)    host | kvm   |  N/A       |  $VLS_MAP
> 
> Where for (1) $MAX_VQ sets the maximum vq and smaller vqs are
> all supported when $VLS_MAP is zero, or when the vqs are selected
> in $VLS_MAP.

I'm a bit confused by the nomenclature here. VL clearly stands for
Vector Length, but what does VQ stand for? You seem to be using the
two terms pretty much interchangeably throughout the cover letter.

[...]
> There is never any need to provide both properties, but if both
> are provided then they are checked for consistency.

I would personally just error out when both are provided.

> The QMP query returns a list of valid vq lists. For example, if
> a guest can use vqs 1, 2, 3, and 4, then the following list will
> be returned
> 
>  [ [ 1 ], [ 1, 2 ], [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ]
> 
> Another example might be 1, 2, 4, as the architecture states 3
> is optional. In that case the list would be
> 
>  [ [ 1 ], [ 1, 2 ], [ 1, 2, 4 ] ]

I think the proposed QMP command is problematic, because it reports
the valid vector lengths for either KVM or TCG based on which
accelerator is currently enabled: it should report all information
at all times instead, similarly to how query-gic-capabilities does
it.

[...]
> And now for what might be a bit more controversial; how we input
> the valid vector set with sve-vls-map. Well, sve-vls-map is a
> 64-bit bitmap, which is admittedly not user friendly and also not
> the same size as KVM's vls bitmap (which is 8 64-bit words). Here's
> the justification:
> 
>  1) We want to use the QEMU command line in order for the information
>     to be migrated without needing to add more VM state.
>  2) It's not easy/pretty to input arrays on the QEMU command line.
>  3) We already need to use the QMP query to get a valid set, which
>     isn't user friendly either, meaning we're already in libvirt
>     territory.
>  4) A 64-bit map (supporting up to 8192-bit vectors) is likely big
>     enough for quite some time (currently KVM and TCG only support
>     2048-bit vectors).
>  5) If user friendliness is needed more than migratability then
>     the 'max' cpu type can be used with the sve-max-vq property.
>  6) It's possible to probe the full valid vector set from the
>     command line by using something like sve-vls-map=0xffff and
>     then, when it fails, the error message will state the correct
>     map, e.g. 0xb.

I don't have a problem with having to use a bitmap internally,
though libvirt will clearly want to expose a more approachable
interface to users.

However, QMP reporting the information in the current format means
we'd have to build an additional parser on top of the bitmap handling
and conversion routines we'll clearly need to make this work; plus it
just feels weird that the information reported by QMP can't be used
on the command line without going through some tranformation first.

Wouldn't it make more sense to both accept and report bitmaps?

-- 
Andrea Bolognani / Red Hat / Virtualization


Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Dave Martin 4 years, 10 months ago
On Mon, May 13, 2019 at 10:32:46AM +0100, Andrea Bolognani wrote:
> On Sun, 2019-05-12 at 10:36 +0200, Andrew Jones wrote:
> [...]
> >    CPU type | accel | sve-max-vq | sve-vls-map
> >    -------------------------------------------
> >  1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
> >  2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
> >  3)    host | kvm   |  N/A       |  $VLS_MAP
> > 
> > Where for (1) $MAX_VQ sets the maximum vq and smaller vqs are
> > all supported when $VLS_MAP is zero, or when the vqs are selected
> > in $VLS_MAP.
> 
> I'm a bit confused by the nomenclature here. VL clearly stands for
> Vector Length, but what does VQ stand for? You seem to be using the
> two terms pretty much interchangeably throughout the cover letter.

From the Linux end, "vector length" or VL refers to the size of a vector
register, either in no particular unit or in bytes.

"VQ" refers specifically to the vector length in 128-bit quadwords.

In some situations, neither terminology is obviously better than the
other, such as in the way KVM_REG_ARM64_SVE_VLS is encoded.

> [...]
> > There is never any need to provide both properties, but if both
> > are provided then they are checked for consistency.
> 
> I would personally just error out when both are provided.
> 
> > The QMP query returns a list of valid vq lists. For example, if
> > a guest can use vqs 1, 2, 3, and 4, then the following list will
> > be returned
> > 
> >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ]
> > 
> > Another example might be 1, 2, 4, as the architecture states 3
> > is optional. In that case the list would be
> > 
> >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 4 ] ]
> 
> I think the proposed QMP command is problematic, because it reports
> the valid vector lengths for either KVM or TCG based on which
> accelerator is currently enabled: it should report all information
> at all times instead, similarly to how query-gic-capabilities does
> it.

I wonder if this is premature flexibility?

The size of these lists is going to get cumbersome if the architecture
is ever extended.  Even today, we might need over 100 items in this
(nested) list.  If this is to be presented to the user this will be
far from friendly, it could get much worse if the architecutre changes
in future to allow larger vectors or more flexible virtualisation.

Could we just have a list of supported vector lengths and a possibly
empty list of additional capabilities that describe what kinds of
flexibility are allowed?

So, for example, we might support vector lengths of 1, 2, 4 and 8
quadwords, with the the ability to clamp the max vector length the
guest sees: the kernel ABI guarantees that you can do this, even
if you can't disable/enable each individual vector length independently.

So, [ 1, 2, 4, 8 ] seems sufficient to describe this in a forwards
compatible way.

Some day, we might report { "independent", [ 1, 2, 4, 8, 16, 32, ... ] }

I'm guessing about the data representation here.

[...]

Cheers
---Dave

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrew Jones 4 years, 10 months ago
On Mon, May 13, 2019 at 12:15:56PM +0100, Dave Martin wrote:
> On Mon, May 13, 2019 at 10:32:46AM +0100, Andrea Bolognani wrote:
> > On Sun, 2019-05-12 at 10:36 +0200, Andrew Jones wrote:
> > [...]
> > >    CPU type | accel | sve-max-vq | sve-vls-map
> > >    -------------------------------------------
> > >  1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
> > >  2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
> > >  3)    host | kvm   |  N/A       |  $VLS_MAP
> > > 
> > > Where for (1) $MAX_VQ sets the maximum vq and smaller vqs are
> > > all supported when $VLS_MAP is zero, or when the vqs are selected
> > > in $VLS_MAP.
> > 
> > I'm a bit confused by the nomenclature here. VL clearly stands for
> > Vector Length, but what does VQ stand for? You seem to be using the
> > two terms pretty much interchangeably throughout the cover letter.
> 
> From the Linux end, "vector length" or VL refers to the size of a vector
> register, either in no particular unit or in bytes.
> 
> "VQ" refers specifically to the vector length in 128-bit quadwords.
> 
> In some situations, neither terminology is obviously better than the
> other, such as in the way KVM_REG_ARM64_SVE_VLS is encoded.
> 
> > [...]
> > > There is never any need to provide both properties, but if both
> > > are provided then they are checked for consistency.
> > 
> > I would personally just error out when both are provided.
> > 
> > > The QMP query returns a list of valid vq lists. For example, if
> > > a guest can use vqs 1, 2, 3, and 4, then the following list will
> > > be returned
> > > 
> > >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ]
> > > 
> > > Another example might be 1, 2, 4, as the architecture states 3
> > > is optional. In that case the list would be
> > > 
> > >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 4 ] ]
> > 
> > I think the proposed QMP command is problematic, because it reports
> > the valid vector lengths for either KVM or TCG based on which
> > accelerator is currently enabled: it should report all information
> > at all times instead, similarly to how query-gic-capabilities does
> > it.
> 
> I wonder if this is premature flexibility?
> 
> The size of these lists is going to get cumbersome if the architecture
> is ever extended.  Even today, we might need over 100 items in this
> (nested) list.  If this is to be presented to the user this will be
> far from friendly, it could get much worse if the architecutre changes
> in future to allow larger vectors or more flexible virtualisation.
> 
> Could we just have a list of supported vector lengths and a possibly
> empty list of additional capabilities that describe what kinds of
> flexibility are allowed?
> 
> So, for example, we might support vector lengths of 1, 2, 4 and 8
> quadwords, with the the ability to clamp the max vector length the
> guest sees: the kernel ABI guarantees that you can do this, even
> if you can't disable/enable each individual vector length independently.
> 
> So, [ 1, 2, 4, 8 ] seems sufficient to describe this in a forwards
> compatible way.
> 
> Some day, we might report { "independent", [ 1, 2, 4, 8, 16, 32, ... ] }
> 
> I'm guessing about the data representation here.
> 

I think that could work, and something along those lines even crossed my
mind. Let's see what libvirt folk say. I'm not overly concerned about
user friendless here though, as users aren't running QMP commands and
parsing json by hand too much.

Thanks,
drew


Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Dave Martin 4 years, 10 months ago
On Mon, May 13, 2019 at 01:38:57PM +0100, Andrew Jones wrote:
> On Mon, May 13, 2019 at 12:15:56PM +0100, Dave Martin wrote:
> > On Mon, May 13, 2019 at 10:32:46AM +0100, Andrea Bolognani wrote:
> > > On Sun, 2019-05-12 at 10:36 +0200, Andrew Jones wrote:

[...]

> > > > The QMP query returns a list of valid vq lists. For example, if
> > > > a guest can use vqs 1, 2, 3, and 4, then the following list will
> > > > be returned
> > > > 
> > > >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ]
> > > > 
> > > > Another example might be 1, 2, 4, as the architecture states 3
> > > > is optional. In that case the list would be
> > > > 
> > > >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 4 ] ]
> > > 
> > > I think the proposed QMP command is problematic, because it reports
> > > the valid vector lengths for either KVM or TCG based on which
> > > accelerator is currently enabled: it should report all information
> > > at all times instead, similarly to how query-gic-capabilities does
> > > it.
> > 
> > I wonder if this is premature flexibility?
> > 
> > The size of these lists is going to get cumbersome if the architecture
> > is ever extended.  Even today, we might need over 100 items in this
> > (nested) list.  If this is to be presented to the user this will be
> > far from friendly, it could get much worse if the architecutre changes
> > in future to allow larger vectors or more flexible virtualisation.
> > 
> > Could we just have a list of supported vector lengths and a possibly
> > empty list of additional capabilities that describe what kinds of
> > flexibility are allowed?
> > 
> > So, for example, we might support vector lengths of 1, 2, 4 and 8
> > quadwords, with the the ability to clamp the max vector length the
> > guest sees: the kernel ABI guarantees that you can do this, even
> > if you can't disable/enable each individual vector length independently.
> > 
> > So, [ 1, 2, 4, 8 ] seems sufficient to describe this in a forwards
> > compatible way.
> > 
> > Some day, we might report { "independent", [ 1, 2, 4, 8, 16, 32, ... ] }
> > 
> > I'm guessing about the data representation here.
> > 
> 
> I think that could work, and something along those lines even crossed my
> mind. Let's see what libvirt folk say. I'm not overly concerned about
> user friendless here though, as users aren't running QMP commands and
> parsing json by hand too much.

One concern I have is that if the architecture ever supports masking out
vector lengths individually, the size of this description becomes
something like O(2^n) in the maximum vector length, instead of O(n^2).

That could be very painful to deal with...

Cheers
---Dave

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrew Jones 4 years, 10 months ago
On Mon, May 13, 2019 at 11:32:46AM +0200, Andrea Bolognani wrote:
> On Sun, 2019-05-12 at 10:36 +0200, Andrew Jones wrote:
> [...]
> >    CPU type | accel | sve-max-vq | sve-vls-map
> >    -------------------------------------------
> >  1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
> >  2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
> >  3)    host | kvm   |  N/A       |  $VLS_MAP
> > 
> > Where for (1) $MAX_VQ sets the maximum vq and smaller vqs are
> > all supported when $VLS_MAP is zero, or when the vqs are selected
> > in $VLS_MAP.
> 
> I'm a bit confused by the nomenclature here. VL clearly stands for
> Vector Length, but what does VQ stand for? You seem to be using the
> two terms pretty much interchangeably throughout the cover letter.

As Dave pointed out, they're both lengths, but VQ specifically points
out that the unit is 'Q'uadwords. We could use VQS instead of VLS,
"Vector Lengths" sounds better.

> 
> [...]
> > There is never any need to provide both properties, but if both
> > are provided then they are checked for consistency.
> 
> I would personally just error out when both are provided.

I'm fine with that if nobody else objects.

> 
> > The QMP query returns a list of valid vq lists. For example, if
> > a guest can use vqs 1, 2, 3, and 4, then the following list will
> > be returned
> > 
> >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ]
> > 
> > Another example might be 1, 2, 4, as the architecture states 3
> > is optional. In that case the list would be
> > 
> >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 4 ] ]
> 
> I think the proposed QMP command is problematic, because it reports
> the valid vector lengths for either KVM or TCG based on which
> accelerator is currently enabled: it should report all information
> at all times instead, similarly to how query-gic-capabilities does
> it.

OK, and then with a flag stating which is when then. Dave points out
we may want to reduce the list to a single set and then add flags
to indicate what can be done with it in order to derive other sets.
What do you think about that?

> 
> [...]
> > And now for what might be a bit more controversial; how we input
> > the valid vector set with sve-vls-map. Well, sve-vls-map is a
> > 64-bit bitmap, which is admittedly not user friendly and also not
> > the same size as KVM's vls bitmap (which is 8 64-bit words). Here's
> > the justification:
> > 
> >  1) We want to use the QEMU command line in order for the information
> >     to be migrated without needing to add more VM state.
> >  2) It's not easy/pretty to input arrays on the QEMU command line.
> >  3) We already need to use the QMP query to get a valid set, which
> >     isn't user friendly either, meaning we're already in libvirt
> >     territory.
> >  4) A 64-bit map (supporting up to 8192-bit vectors) is likely big
> >     enough for quite some time (currently KVM and TCG only support
> >     2048-bit vectors).
> >  5) If user friendliness is needed more than migratability then
> >     the 'max' cpu type can be used with the sve-max-vq property.
> >  6) It's possible to probe the full valid vector set from the
> >     command line by using something like sve-vls-map=0xffff and
> >     then, when it fails, the error message will state the correct
> >     map, e.g. 0xb.
> 
> I don't have a problem with having to use a bitmap internally,
> though libvirt will clearly want to expose a more approachable
> interface to users.
> 
> However, QMP reporting the information in the current format means
> we'd have to build an additional parser on top of the bitmap handling
> and conversion routines we'll clearly need to make this work; plus it
> just feels weird that the information reported by QMP can't be used
> on the command line without going through some tranformation first.
> 
> Wouldn't it make more sense to both accept and report bitmaps?

If we eventually need more than one word for the bitmap then it'll
require parsing and bitmap composition code in libvirt anyway. I
was thinking by pointing out each bit separately that we could
boundlessly grow the list without having to change anything in
libvirt later.

Thanks,
drew

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrea Bolognani 4 years, 10 months ago
On Mon, 2019-05-13 at 14:36 +0200, Andrew Jones wrote:
> On Mon, May 13, 2019 at 11:32:46AM +0200, Andrea Bolognani wrote:
> > On Sun, 2019-05-12 at 10:36 +0200, Andrew Jones wrote:
> > [...]
> > >    CPU type | accel | sve-max-vq | sve-vls-map
> > >    -------------------------------------------
> > >  1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
> > >  2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
> > >  3)    host | kvm   |  N/A       |  $VLS_MAP
> > > 
> > > Where for (1) $MAX_VQ sets the maximum vq and smaller vqs are
> > > all supported when $VLS_MAP is zero, or when the vqs are selected
> > > in $VLS_MAP.
> > 
> > I'm a bit confused by the nomenclature here. VL clearly stands for
> > Vector Length, but what does VQ stand for? You seem to be using the
> > two terms pretty much interchangeably throughout the cover letter.
> 
> As Dave pointed out, they're both lengths, but VQ specifically points
> out that the unit is 'Q'uadwords. We could use VQS instead of VLS,
> "Vector Lengths" sounds better.

Alright, it makes sense - once you've managed to figure out what
exactly a "quadword" is, at least :)

Since we expect management applications to use QMP to discover what
vector lengths are supported and then provide an explicit map, I
think it's fair to say that the ability to specify a single maximum
vector length is going to be exclusively used as a convenience for
command line users.

In that light, I think it would be reasonable for the usage to look
along the lines of

  -cpu host,sve-vl-map=0xd # machine-friendly variant
  -cpu max,sve-vl-max=512  # user-friendly variant

with documentation clearly pointing out that the two options expect
completely different formats - but that was the case even before,
so we would have had to document that anyway.

> > > The QMP query returns a list of valid vq lists. For example, if
> > > a guest can use vqs 1, 2, 3, and 4, then the following list will
> > > be returned
> > > 
> > >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ]
> > > 
> > > Another example might be 1, 2, 4, as the architecture states 3
> > > is optional. In that case the list would be
> > > 
> > >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 4 ] ]
> > 
> > I think the proposed QMP command is problematic, because it reports
> > the valid vector lengths for either KVM or TCG based on which
> > accelerator is currently enabled: it should report all information
> > at all times instead, similarly to how query-gic-capabilities does
> > it.
> 
> OK, and then with a flag stating which is when then.

Correct.

> Dave points out
> we may want to reduce the list to a single set and then add flags
> to indicate what can be done with it in order to derive other sets.
> What do you think about that?

So right now all that can be done is truncating the list by removing
an arbitrary number of elements from the end, right? Eg. if you have
[ 1, 2, 4 ] you can use either that or [ 1, 2 ] or [ 1 ]. But in the
future you might also be able to mask single elements in the middle
of the list, thus enabling things like [ 1, 4 ].

That doesn't sound very complicated to support in libvirt, though I
have to say that I'm not a big fan of this proposal because as far as
I can see it basically means implementing the very same logic twice,
once in QEMU and then once more in libvirt.

> > [...]
> > > And now for what might be a bit more controversial; how we input
> > > the valid vector set with sve-vls-map. Well, sve-vls-map is a
> > > 64-bit bitmap, which is admittedly not user friendly and also not
> > > the same size as KVM's vls bitmap (which is 8 64-bit words). Here's
> > > the justification:
> > > 
> > >  1) We want to use the QEMU command line in order for the information
> > >     to be migrated without needing to add more VM state.
> > >  2) It's not easy/pretty to input arrays on the QEMU command line.
> > >  3) We already need to use the QMP query to get a valid set, which
> > >     isn't user friendly either, meaning we're already in libvirt
> > >     territory.
> > >  4) A 64-bit map (supporting up to 8192-bit vectors) is likely big
> > >     enough for quite some time (currently KVM and TCG only support
> > >     2048-bit vectors).
> > >  5) If user friendliness is needed more than migratability then
> > >     the 'max' cpu type can be used with the sve-max-vq property.
> > >  6) It's possible to probe the full valid vector set from the
> > >     command line by using something like sve-vls-map=0xffff and
> > >     then, when it fails, the error message will state the correct
> > >     map, e.g. 0xb.
> > 
> > I don't have a problem with having to use a bitmap internally,
> > though libvirt will clearly want to expose a more approachable
> > interface to users.
> > 
> > However, QMP reporting the information in the current format means
> > we'd have to build an additional parser on top of the bitmap handling
> > and conversion routines we'll clearly need to make this work; plus it
> > just feels weird that the information reported by QMP can't be used
> > on the command line without going through some tranformation first.
> > 
> > Wouldn't it make more sense to both accept and report bitmaps?
> 
> If we eventually need more than one word for the bitmap then it'll
> require parsing and bitmap composition code in libvirt anyway. I
> was thinking by pointing out each bit separately that we could
> boundlessly grow the list without having to change anything in
> libvirt later.

If the size of the bitmap on the KVM side is 512 bits, why don't we
just make it that size on the QEMU side too from the start?

-- 
Andrea Bolognani / Red Hat / Virtualization


Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrew Jones 4 years, 10 months ago
On Tue, May 14, 2019 at 02:29:51PM +0200, Andrea Bolognani wrote:
> On Mon, 2019-05-13 at 14:36 +0200, Andrew Jones wrote:
> > On Mon, May 13, 2019 at 11:32:46AM +0200, Andrea Bolognani wrote:
> > > On Sun, 2019-05-12 at 10:36 +0200, Andrew Jones wrote:
> > > [...]
> > > >    CPU type | accel | sve-max-vq | sve-vls-map
> > > >    -------------------------------------------
> > > >  1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
> > > >  2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
> > > >  3)    host | kvm   |  N/A       |  $VLS_MAP
> > > > 
> > > > Where for (1) $MAX_VQ sets the maximum vq and smaller vqs are
> > > > all supported when $VLS_MAP is zero, or when the vqs are selected
> > > > in $VLS_MAP.
> > > 
> > > I'm a bit confused by the nomenclature here. VL clearly stands for
> > > Vector Length, but what does VQ stand for? You seem to be using the
> > > two terms pretty much interchangeably throughout the cover letter.
> > 
> > As Dave pointed out, they're both lengths, but VQ specifically points
> > out that the unit is 'Q'uadwords. We could use VQS instead of VLS,
> > "Vector Lengths" sounds better.
> 
> Alright, it makes sense - once you've managed to figure out what
> exactly a "quadword" is, at least :)
> 
> Since we expect management applications to use QMP to discover what
> vector lengths are supported and then provide an explicit map, I
> think it's fair to say that the ability to specify a single maximum
> vector length is going to be exclusively used as a convenience for
> command line users.
> 
> In that light, I think it would be reasonable for the usage to look
> along the lines of
> 
>   -cpu host,sve-vl-map=0xd # machine-friendly variant
>   -cpu max,sve-vl-max=512  # user-friendly variant

We already have sve-max-vq, so I'm not sure we want to rename it.

> 
> with documentation clearly pointing out that the two options expect
> completely different formats - but that was the case even before,
> so we would have had to document that anyway.
> 
> > > > The QMP query returns a list of valid vq lists. For example, if
> > > > a guest can use vqs 1, 2, 3, and 4, then the following list will
> > > > be returned
> > > > 
> > > >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ]
> > > > 
> > > > Another example might be 1, 2, 4, as the architecture states 3
> > > > is optional. In that case the list would be
> > > > 
> > > >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 4 ] ]
> > > 
> > > I think the proposed QMP command is problematic, because it reports
> > > the valid vector lengths for either KVM or TCG based on which
> > > accelerator is currently enabled: it should report all information
> > > at all times instead, similarly to how query-gic-capabilities does
> > > it.
> > 
> > OK, and then with a flag stating which is when then.
> 
> Correct.
> 
> > Dave points out
> > we may want to reduce the list to a single set and then add flags
> > to indicate what can be done with it in order to derive other sets.
> > What do you think about that?
> 
> So right now all that can be done is truncating the list by removing
> an arbitrary number of elements from the end, right? Eg. if you have
> [ 1, 2, 4 ] you can use either that or [ 1, 2 ] or [ 1 ]. But in the
> future you might also be able to mask single elements in the middle
> of the list, thus enabling things like [ 1, 4 ].
> 
> That doesn't sound very complicated to support in libvirt, though I
> have to say that I'm not a big fan of this proposal because as far as
> I can see it basically means implementing the very same logic twice,
> once in QEMU and then once more in libvirt.

So if big tables of bits aren't a problem for QMP queries, then I'll
just leave the design as it is.

> 
> > > [...]
> > > > And now for what might be a bit more controversial; how we input
> > > > the valid vector set with sve-vls-map. Well, sve-vls-map is a
> > > > 64-bit bitmap, which is admittedly not user friendly and also not
> > > > the same size as KVM's vls bitmap (which is 8 64-bit words). Here's
> > > > the justification:
> > > > 
> > > >  1) We want to use the QEMU command line in order for the information
> > > >     to be migrated without needing to add more VM state.
> > > >  2) It's not easy/pretty to input arrays on the QEMU command line.
> > > >  3) We already need to use the QMP query to get a valid set, which
> > > >     isn't user friendly either, meaning we're already in libvirt
> > > >     territory.
> > > >  4) A 64-bit map (supporting up to 8192-bit vectors) is likely big
> > > >     enough for quite some time (currently KVM and TCG only support
> > > >     2048-bit vectors).
> > > >  5) If user friendliness is needed more than migratability then
> > > >     the 'max' cpu type can be used with the sve-max-vq property.
> > > >  6) It's possible to probe the full valid vector set from the
> > > >     command line by using something like sve-vls-map=0xffff and
> > > >     then, when it fails, the error message will state the correct
> > > >     map, e.g. 0xb.
> > > 
> > > I don't have a problem with having to use a bitmap internally,
> > > though libvirt will clearly want to expose a more approachable
> > > interface to users.
> > > 
> > > However, QMP reporting the information in the current format means
> > > we'd have to build an additional parser on top of the bitmap handling
> > > and conversion routines we'll clearly need to make this work; plus it
> > > just feels weird that the information reported by QMP can't be used
> > > on the command line without going through some tranformation first.
> > > 
> > > Wouldn't it make more sense to both accept and report bitmaps?
> > 
> > If we eventually need more than one word for the bitmap then it'll
> > require parsing and bitmap composition code in libvirt anyway. I
> > was thinking by pointing out each bit separately that we could
> > boundlessly grow the list without having to change anything in
> > libvirt later.
> 
> If the size of the bitmap on the KVM side is 512 bits, why don't we
> just make it that size on the QEMU side too from the start?

I'd still only want to input 64-bits on the command line, otherwise
we get into inputting arrays, which isn't easy. KVM's interface is
meant for expansion, but it won't be using most of those bits for
quite some time either.

Thanks,
drew

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrea Bolognani 4 years, 10 months ago
On Tue, 2019-05-14 at 14:53 +0200, Andrew Jones wrote:
> On Tue, May 14, 2019 at 02:29:51PM +0200, Andrea Bolognani wrote:
> > Since we expect management applications to use QMP to discover what
> > vector lengths are supported and then provide an explicit map, I
> > think it's fair to say that the ability to specify a single maximum
> > vector length is going to be exclusively used as a convenience for
> > command line users.
> > 
> > In that light, I think it would be reasonable for the usage to look
> > along the lines of
> > 
> >   -cpu host,sve-vl-map=0xd # machine-friendly variant
> >   -cpu max,sve-vl-max=512  # user-friendly variant
> 
> We already have sve-max-vq, so I'm not sure we want to rename it.

Oh, I didn't realize that was the case. And of course it already
takes a number of quadwords as argument, I suppose? That's pretty
unfortunate :(

Perhaps we could consider deprecating it in favor of a user-friendly
variant that's actually suitable for regular humans, like the one I
suggest above?

[...]
> > > Dave points out
> > > we may want to reduce the list to a single set and then add flags
> > > to indicate what can be done with it in order to derive other sets.
> > > What do you think about that?
> > 
> > So right now all that can be done is truncating the list by removing
> > an arbitrary number of elements from the end, right? Eg. if you have
> > [ 1, 2, 4 ] you can use either that or [ 1, 2 ] or [ 1 ]. But in the
> > future you might also be able to mask single elements in the middle
> > of the list, thus enabling things like [ 1, 4 ].
> > 
> > That doesn't sound very complicated to support in libvirt, though I
> > have to say that I'm not a big fan of this proposal because as far as
> > I can see it basically means implementing the very same logic twice,
> > once in QEMU and then once more in libvirt.
> 
> So if big tables of bits aren't a problem for QMP queries, then I'll
> just leave the design as it is.

I thought about it a bit more and perhaps the simplified design is
better after all.

Whatever the interface looks like on the QEMU side, we're going to
want to offer libvirt users two options for configuring vector
lengths: listing all desired vector lengths explicitly (basically
sev-vl-map but more verbose and readable) and providing just the
biggest desired vector length (like in sev-max-vq).

In the latter case, we'll want to expand the user-provided value
into an explicit list anyway in order to guarantee guest ABI
stability, and doing so when a single bitmap has been obtained via
QMP seems like it would be more manageable.

Sorry for the flip-flop, but after all isn't this exactly what
upstream design discussion is supposed to be all about? :)

[...]
> > If the size of the bitmap on the KVM side is 512 bits, why don't we
> > just make it that size on the QEMU side too from the start?
> 
> I'd still only want to input 64-bits on the command line, otherwise
> we get into inputting arrays, which isn't easy. KVM's interface is
> meant for expansion, but it won't be using most of those bits for
> quite some time either.

I'm probably missing something entirely obvious, but couldn't you
just have a single, possibly fairly massive (up to 128 hex digits if
I'm not mistaken) value on the command line and just work with that
one, no arrays necessary?

-- 
Andrea Bolognani / Red Hat / Virtualization


Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrew Jones 4 years, 10 months ago
On Tue, May 14, 2019 at 06:03:09PM +0200, Andrea Bolognani wrote:
> I thought about it a bit more and perhaps the simplified design is
> better after all.
> 
> Whatever the interface looks like on the QEMU side, we're going to
> want to offer libvirt users two options for configuring vector
> lengths: listing all desired vector lengths explicitly (basically
> sev-vl-map but more verbose and readable) and providing just the
> biggest desired vector length (like in sev-max-vq).
> 
> In the latter case, we'll want to expand the user-provided value
> into an explicit list anyway in order to guarantee guest ABI
> stability, and doing so when a single bitmap has been obtained via
> QMP seems like it would be more manageable.
> 
> Sorry for the flip-flop, but after all isn't this exactly what
> upstream design discussion is supposed to be all about? :)

Yup, no problem. I'm actually liking the idea of the single map plus
flags. We won't need two implementations (QEMU and libvirt), we'll
only need one (libvirt). The QEMU QMP side will only need to state
what should be implemented using the flags. Also, as we already
agreed, we need TCG and KVM flags anyway, so we're already in flag
land.

> 
> [...]
> > > If the size of the bitmap on the KVM side is 512 bits, why don't we
> > > just make it that size on the QEMU side too from the start?
> > 
> > I'd still only want to input 64-bits on the command line, otherwise
> > we get into inputting arrays, which isn't easy. KVM's interface is
> > meant for expansion, but it won't be using most of those bits for
> > quite some time either.
> 
> I'm probably missing something entirely obvious, but couldn't you
> just have a single, possibly fairly massive (up to 128 hex digits if
> I'm not mistaken) value on the command line and just work with that
> one, no arrays necessary?
>

We could, and I like the idea. It just hadn't crossed my mind due to
implementation tunnel vision.

Thanks,
drew

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Richard Henderson 4 years, 10 months ago
On 5/14/19 9:03 AM, Andrea Bolognani wrote:
> On Tue, 2019-05-14 at 14:53 +0200, Andrew Jones wrote:
>> We already have sve-max-vq, so I'm not sure we want to rename it.
> 
> Oh, I didn't realize that was the case. And of course it already
> takes a number of quadwords as argument, I suppose? That's pretty
> unfortunate :(
> 
> Perhaps we could consider deprecating it in favor of a user-friendly
> variant that's actually suitable for regular humans, like the one I
> suggest above?

Why is =4 less user-friendly than =512?

I don't actually see "total bits in vector" as more user-friendly than "number
of quadwords" when it comes to non-powers-of-2 like =7 vs =896 or =13 vs =1664.


r~

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrea Bolognani 4 years, 10 months ago
On Tue, 2019-05-14 at 13:14 -0700, Richard Henderson wrote:
> On 5/14/19 9:03 AM, Andrea Bolognani wrote:
> > On Tue, 2019-05-14 at 14:53 +0200, Andrew Jones wrote:
> > > We already have sve-max-vq, so I'm not sure we want to rename it.
> > 
> > Oh, I didn't realize that was the case. And of course it already
> > takes a number of quadwords as argument, I suppose? That's pretty
> > unfortunate :(
> > 
> > Perhaps we could consider deprecating it in favor of a user-friendly
> > variant that's actually suitable for regular humans, like the one I
> > suggest above?
> 
> Why is =4 less user-friendly than =512?
> 
> I don't actually see "total bits in vector" as more user-friendly than "number
> of quadwords" when it comes to non-powers-of-2 like =7 vs =896 or =13 vs =1664.

I would wager most people are intimately familiar with bits, bytes
and multiples due to having to work with them daily. Quadwords, not
so much.

-- 
Andrea Bolognani / Red Hat / Virtualization


Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Dave Martin 4 years, 10 months ago
On Wed, May 15, 2019 at 09:03:58AM +0100, Andrea Bolognani wrote:
> On Tue, 2019-05-14 at 13:14 -0700, Richard Henderson wrote:
> > On 5/14/19 9:03 AM, Andrea Bolognani wrote:
> > > On Tue, 2019-05-14 at 14:53 +0200, Andrew Jones wrote:
> > > > We already have sve-max-vq, so I'm not sure we want to rename it.
> > > 
> > > Oh, I didn't realize that was the case. And of course it already
> > > takes a number of quadwords as argument, I suppose? That's pretty
> > > unfortunate :(
> > > 
> > > Perhaps we could consider deprecating it in favor of a user-friendly
> > > variant that's actually suitable for regular humans, like the one I
> > > suggest above?
> > 
> > Why is =4 less user-friendly than =512?
> > 
> > I don't actually see "total bits in vector" as more user-friendly than "number
> > of quadwords" when it comes to non-powers-of-2 like =7 vs =896 or =13 vs =1664.
> 
> I would wager most people are intimately familiar with bits, bytes
> and multiples due to having to work with them daily. Quadwords, not
> so much.

Generally I tend to agree.  For kvmtool I leaned torward quadwords
purely because

	16,32,48,64,80,96,112,128,144,160,176,192,208

is a big pain to type compared with

	1,2,3,4,5,6,7,8,9,10,11,12,13

Even though I prefer to specify vector lengths in bytes everywhere else
in the Linux user API (precisely to avoid the confusion you object to).

This isn't finalised yet for kvmtool -- I need to rework the patches
and may not include it at all initially: kvmtool doesn't support
migration, which is the main usecase for being able to specify an exact
set of vector lengths AFAICT.

Since this is otherwise only useful for migration, experimentation or
machine-driven configuration, a bitmask

	0x1fff

as some have suggested may well be a pragmatic alternative for kvmtool.

Cheers
---Dave

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrea Bolognani 4 years, 10 months ago
On Wed, 2019-05-15 at 12:14 +0100, Dave Martin wrote:
> On Wed, May 15, 2019 at 09:03:58AM +0100, Andrea Bolognani wrote:
> > On Tue, 2019-05-14 at 13:14 -0700, Richard Henderson wrote:
> > > Why is =4 less user-friendly than =512?
> > > 
> > > I don't actually see "total bits in vector" as more user-friendly than "number
> > > of quadwords" when it comes to non-powers-of-2 like =7 vs =896 or =13 vs =1664.
> > 
> > I would wager most people are intimately familiar with bits, bytes
> > and multiples due to having to work with them daily. Quadwords, not
> > so much.
> 
> Generally I tend to agree.  For kvmtool I leaned torward quadwords
> purely because
> 
> 	16,32,48,64,80,96,112,128,144,160,176,192,208
> 
> is a big pain to type compared with
> 
> 	1,2,3,4,5,6,7,8,9,10,11,12,13
> 
> Even though I prefer to specify vector lengths in bytes everywhere else
> in the Linux user API (precisely to avoid the confusion you object to).
> 
> This isn't finalised yet for kvmtool -- I need to rework the patches
> and may not include it at all initially: kvmtool doesn't support
> migration, which is the main usecase for being able to specify an exact
> set of vector lengths AFAICT.
> 
> Since this is otherwise only useful for migration, experimentation or
> machine-driven configuration, a bitmask
> 
> 	0x1fff
> 
> as some have suggested may well be a pragmatic alternative for kvmtool.

Just to be clear, I have suggested using bits (or bytes or megabytes
depending on the exact value) only for the command-line-user-oriented
sve-vl-max option, which would take a single value.

For interoperation with the management layer, on the other hand,
using a bitmap is perfectly fine, and whether the values encoded
within are expressed in quadwords or whatever other format is largely
irrelevant, so long as it it's properly documented of course.

-- 
Andrea Bolognani / Red Hat / Virtualization


Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Dave Martin 4 years, 10 months ago
On Wed, May 15, 2019 at 12:28:20PM +0100, Andrea Bolognani wrote:
> On Wed, 2019-05-15 at 12:14 +0100, Dave Martin wrote:
> > On Wed, May 15, 2019 at 09:03:58AM +0100, Andrea Bolognani wrote:
> > > On Tue, 2019-05-14 at 13:14 -0700, Richard Henderson wrote:
> > > > Why is =4 less user-friendly than =512?
> > > > 
> > > > I don't actually see "total bits in vector" as more user-friendly than "number
> > > > of quadwords" when it comes to non-powers-of-2 like =7 vs =896 or =13 vs =1664.
> > > 
> > > I would wager most people are intimately familiar with bits, bytes
> > > and multiples due to having to work with them daily. Quadwords, not
> > > so much.
> > 
> > Generally I tend to agree.  For kvmtool I leaned torward quadwords
> > purely because
> > 
> > 	16,32,48,64,80,96,112,128,144,160,176,192,208
> > 
> > is a big pain to type compared with
> > 
> > 	1,2,3,4,5,6,7,8,9,10,11,12,13
> > 
> > Even though I prefer to specify vector lengths in bytes everywhere else
> > in the Linux user API (precisely to avoid the confusion you object to).
> > 
> > This isn't finalised yet for kvmtool -- I need to rework the patches
> > and may not include it at all initially: kvmtool doesn't support
> > migration, which is the main usecase for being able to specify an exact
> > set of vector lengths AFAICT.
> > 
> > Since this is otherwise only useful for migration, experimentation or
> > machine-driven configuration, a bitmask
> > 
> > 	0x1fff
> > 
> > as some have suggested may well be a pragmatic alternative for kvmtool.
> 
> Just to be clear, I have suggested using bits (or bytes or megabytes
> depending on the exact value) only for the command-line-user-oriented
> sve-vl-max option, which would take a single value.
> 
> For interoperation with the management layer, on the other hand,
> using a bitmap is perfectly fine, and whether the values encoded
> within are expressed in quadwords or whatever other format is largely
> irrelevant, so long as it it's properly documented of course.

Seems fair.

Cheers
---Dave

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Richard Henderson 4 years, 10 months ago
On 5/12/19 1:36 AM, Andrew Jones wrote:
>    CPU type | accel | sve-max-vq | sve-vls-map
>    -------------------------------------------
>  1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
>  2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
>  3)    host | kvm   |  N/A       |  $VLS_MAP

This doesn't seem right.  Why is -cpu host not whatever the host supports?  It
certainly has been so far.  I really don't see how -cpu max makes any sense for
kvm.


> The QMP query returns a list of valid vq lists. For example, if
> a guest can use vqs 1, 2, 3, and 4, then the following list will
> be returned
> 
>  [ [ 1 ], [ 1, 2 ], [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ]
> 
> Another example might be 1, 2, 4, as the architecture states 3
> is optional. In that case the list would be
> 
>  [ [ 1 ], [ 1, 2 ], [ 1, 2, 4 ] ]
> 
> This may look redundant, but it's necessary to provide a future-
> proof query, because while KVM currently requires vector sets to
> be strict truncations of the full valid vector set, that may change
> at some point.

How and why would that make sense?

Real hardware is going to support one set of vector lengths.  Whether VQ=3 is
valid or not is not going to depend on the maximum VQ, surely.

I'll also note that if we want to support the theoretical
beyond-current-architecture maximum VQ=512, such that migration works
seemlessly with current hardware, then we're going to have to change the
migration format.

So far I'm supporting only the current architecture maximum VQ=16.  Which
seemed plenty, given that the first round of hardware only supports VQ=4.



r~

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Andrew Jones 4 years, 10 months ago
On Mon, May 13, 2019 at 11:46:29AM -0700, Richard Henderson wrote:
> On 5/12/19 1:36 AM, Andrew Jones wrote:
> >    CPU type | accel | sve-max-vq | sve-vls-map
> >    -------------------------------------------
> >  1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
> >  2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
> >  3)    host | kvm   |  N/A       |  $VLS_MAP
> 
> This doesn't seem right.  Why is -cpu host not whatever the host supports?  It
> certainly has been so far. 

-cpu host can support whatever the host (hardware + KVM ) supports, but if
a user doesn't want to expose all of it to the guest, then the user doesn't
have to. For example, the host cpu may have a PMU, but the guest doesn't
necessarily get one (-cpu host,pmu=off).

> I really don't see how -cpu max makes any sense for
> kvm.
>

It's already supported for kvm. This series just extends that support
to match tcg's sve support. The reason it's supported is that you can
then use '-cpu max' along with '-machine accel=kvm:tcg' in a command
line and it'll just work.
 
> 
> > The QMP query returns a list of valid vq lists. For example, if
> > a guest can use vqs 1, 2, 3, and 4, then the following list will
> > be returned
> > 
> >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ]
> > 
> > Another example might be 1, 2, 4, as the architecture states 3
> > is optional. In that case the list would be
> > 
> >  [ [ 1 ], [ 1, 2 ], [ 1, 2, 4 ] ]
> > 
> > This may look redundant, but it's necessary to provide a future-
> > proof query, because while KVM currently requires vector sets to
> > be strict truncations of the full valid vector set, that may change
> > at some point.
> 
> How and why would that make sense?
> 
> Real hardware is going to support one set of vector lengths.

The guest's view of the hardware will be a single set. That set can be
different for different guests though, within the constraints of the
architecture and KVM or TCG implementations.

> Whether VQ=3 is
> valid or not is not going to depend on the maximum VQ, surely.

Exactly. That's why we need a way to explicitly state what is supported.
We can't assume anything from the max VQ alone. Additionally, while
strict truncation is required now for KVM, things may be more flexible
later. TCG is already more flexible. For TCG, all sets that at least
include all the power-of-2 lengths up to the maximum VQ are valid, as
the architecture states. Plus, all the sets that can be derived by adding
one ore more optional lengths to those sets are also valid. Listing each
of them allows management software to know what's going to work and what's
not without having to know all the rules itself.

> 
> I'll also note that if we want to support the theoretical
> beyond-current-architecture maximum VQ=512, such that migration works
> seemlessly with current hardware, then we're going to have to change the
> migration format.
> 
> So far I'm supporting only the current architecture maximum VQ=16.  Which
> seemed plenty, given that the first round of hardware only supports VQ=4.
> 

I agree. The changes won't be small to QEMU, but hopefully we can design
a QMP query that won't need to change, saving libvirt some pain.

Thanks,
drew

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests
Posted by Peter Maydell 4 years, 10 months ago
On Mon, 13 May 2019 at 19:46, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 5/12/19 1:36 AM, Andrew Jones wrote:
> >    CPU type | accel | sve-max-vq | sve-vls-map
> >    -------------------------------------------
> >  1)     max | tcg   |  $MAX_VQ   |  $VLS_MAP
> >  2)     max | kvm   |  $MAX_VQ   |  $VLS_MAP
> >  3)    host | kvm   |  N/A       |  $VLS_MAP
>
> This doesn't seem right.  Why is -cpu host not whatever the host supports?  It
> certainly has been so far.  I really don't see how -cpu max makes any sense for
> kvm.

The point of '-cpu max' is that it works and gives you the
best thing QEMU can support regardless of what accelerator
is in use. This means that you don't need to do tedious
workarounds like "if KVM then -cpu host else -cpu somethingelse".

thanks
-- PMM