On Tue, May 14, 2019 at 06:54:03AM +0200, Markus Armbruster wrote: > Andrew Jones <drjo...@redhat.com> writes: > > > On Thu, Apr 18, 2019 at 07:48:09PM +0200, Markus Armbruster wrote: > >> Daniel P. Berrangé <berra...@redhat.com> writes: > >> > >> > On Thu, Apr 18, 2019 at 11:28:41AM +0200, Andrew Jones wrote: > >> >> Hi all, > >> >> > >> >> First some background: > >> >> > >> >> For the userspace side of AArch64 guest SVE support we need to > >> >> expose KVM's allowed vector lengths bitmap to the user and allow > >> >> the user to choose a subset of that bitmap. Since bitmaps are a > >> >> bit awkward to work with then we'll likely want to expose it as > >> >> an array of vector lengths instead. Also, assuming we want to > >> >> expose the lengths as number-of-quadwords (quadword == 128 bits > >> >> for AArch64 and vector lengths must be multiples of quadwords) > >> >> rather than number-of-bits, then an example array (which will > >> >> always be a sequence) might be > >> >> > >> >> [ 8, 16, 32 ] > >> >> > >> >> The user may choose a subsequence, but only through truncation, > >> >> i.e. [ 8, 32 ] is not valid, but [ 8, 16 ] is. > >> >> > >> >> Furthermore, different hosts may support different sequences > >> >> which have the same maximum. For example, if the above sequence > >> >> is for Host_A, then Host_B could be > >> >> > >> >> [ 8, 16, 24, 32 ] > >> >> > >> >> The host must support all lengths in the sequence, which means > >> >> that while Host_A supports 32, since it doesn't support 24 and > >> >> we can only truncate sequences, we must use either [ 8 ] or > >> >> [ 8, 16 ] for a compatible sequence if we intend to migrate > >> >> between the hosts. > >> >> > >> >> Now to the $SUBJECT question: > >> >> > >> >> My feeling is that we should require the sequence to be > >> >> provided on the command line as a cpu property. Something > >> >> like > >> >> > >> >> -cpu host,sve-vl-list=8:16 > >> >> > >> >> (I chose ':' for the delimiter because ',' can't work, but > >> >> if there's a better choice, then that's fine by me.) > >> >> > >> >> Afaict a property list like this will require a new parser, > >> > >> We had 20+ of those when I last counted. Among the more annoying > >> reasons CLI QAPIfication is hard[1]. > >> > >> >> which feels a bit funny since it seems we should already > >> >> have support for this type of thing somewhere in QEMU. So, > >> >> the question is: do we? I see we have array properties, but > >> >> I don't believe that works with the command line. Should we > >> >> only use QMP for this? We already want some QMP in order to > >> >> query the supported vector lengths. Maybe we should use QMP > >> >> to set the selection too? But then what about command line > >> >> support for developers? And if the property is on the command > >> >> line then we don't have to add it to the migration stream. > >> > > >> > You should be able to use arrays from the CLI with QemuOpts by repeating > >> > the same option name many times, though I can't say it is a very > >> > nice approach if you have many values to list as it gets very repetative. > >> > >> Yes, this is one of the ways the current CLI does lists. It's also one > >> of the more annoying reasons CLI QAPIfication is hard[2]. > >> > >> QemuOpts let the last param=value win the stupidest way that could > >> possibly work (I respect that): add to the front of the list, search it > >> front to back. > >> > >> Then somebody discovered that if you search the list manually, you can > >> see them all, and abuse that to get a list-valued param. I'm sure that > >> felt clever at the time. > >> > >> Another way to do lists the funky list feature of string input and opts > >> visitor. Yet another annoying reason CLI QAPIfication is hard[3]. > >> > >> We use the opts visitor's list feature for -numa node,cpus=... Hmm, > >> looks like we even combine it with the "multiple param=value build up a > >> list" technique: -smp node,cpus=0-1,cpus=4-5 denotes [0,1,4,5]. > >> > >> > That's the curse of not having a good CLI syntax for non-scalar data in > >> > QemuOpts & why Markus believes we should switch to JSON for the CLI too > >> > > >> > -cpu host,sve-vl=8,sve-vl=16 > >> > >> We actually have CLI syntax for non-scalar data: dotted keys. Dotted > >> keys are syntactic sugar for JSON. It looks friendlier than JSON for > >> simple cases, then gets uglier as things get more complex, and then it > >> falls apart: it can't quite express all of JSON. > >> > >> Example: sve-vl.0=8,sve-vl.1=16 > >> gets desugared into {"sve": [8, 16]} > >> if the QAPI schema has 'sve': ['int']. > >> > >> The comment at the beginning of util/keyval.c explains it in more > >> detail. > >> > >> It powers -blockdev and -display. Both options accept either JSON or > >> dotted keys. If the option argument starts with '{', it's JSON. > >> Management applications should stick to JSON. > >> > >> > >> [1] Towards a more expressive and introspectable QEMU command line > >> https://www.linux-kvm.org/images/f/f2/Armbru-qapi-cmdline_1.pdf > >> Slide 34 "Backward compatibility" item 1 > >> > >> [2] ibid, item 4 > >> > >> [3] ibid, item 3 > >> > > > > Sorry I forgot to follow up to this earlier. I looked at the examples > > provided and saw they were all for independent command line options, > > rather than command line options like '-cpu' that then accepts additional > > properties. I couldn't see how I could use ',' to separate array members > > when using properties or to use an array property input on the command > > line. > > The argument of -cpu is parsed ad hoc. Unlike QemuOpts and dotted keys, > parse_cpu_option() doesn't seem to support escaping ','. Not that > escaping would be a user-friendly solution. > > > In the end I opted to use a single uint64_t for a bitmap, as 64 is > > big enough for now, > > Do you think it'll remain big enough?
Probably not forever, and TBH I can't even give an estimate for how long. Based on the current state, I "feel" like it'll be quite some time though. I think we can extend this map by adding more ad hoc parsing to -cpu later. If we added dotted key support then each array member could be another bitmap word, for example. > > > and even though passing some hex number on the command > > line isn't user friendly at all, it didn't seem like a long list of a > > repeated parameter was that user friendly either. Of course I'm still open > > to suggestions to try to find the best balance between user friendliness, > > current QEMU command line parsing support, and just getting a bitmap into > > cpu state one way or another. > > I'd ask for consistency with existing practice no matter how flawed if > we had such consistency. > > If I understand your "[PATCH 00/13] target/arm/kvm: enable SVE in > guests" correctly, the bitmap form of [1, 2, 4] is > > -cpu max,sve-vls-map=11 > > Observe bit#0 means 1; better document that clearly. > > If we used dotted keys to produce an intList, we'd do > > -cpu max,sve-vls-map.0=1,sve-vls-map.1=2,sve-vls-map.2=4 > > If the option argument is QAPIfied, we additionally get > > -cpu '{"type": "max", "sve-vls-map": [1, 2, 4]}' > > for free. > > If we did it like -numa (please don't), we'd get something like > > -cpu max,sve-vls-map=1-2,sve-vls-map=4 > > None of the above is exactly a pinnacle of user-friendliness. JSON is > at least ugly in a regular way. Your bitmap encoded in a number is at > least concise. > > If a numerically encoded bitmap is the least bad option here, I wonder > why it's not the least bad option for -numa... Perhaps because there 64 > isn't big enough. > > I'm afraid the numerically encoded bitmap will make its way into the > QAPI schema sooner or later. This will create an unfortunate > inconsistency with the [int] encoding already there. > > Who's going to use sve-vls-map? Humans, or pretty much only machines? My thought is primarily machines. If a human wants to use the command line and SVE, then I'm assuming they'll be happy with sve-max-vq or figuring out a map they like once and then sticking to it. Thanks, drew