Message ID | 20190512083624.8916-1-drjones@redhat.com (mailing list archive) |
---|---|
Headers | show |
Series | target/arm/kvm: enable SVE in guests | expand |
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?
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
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
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
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
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
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
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~
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
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
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?
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
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?
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~
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.
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
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
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.
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