kvm: Add ioctl for gathering debug counters

KVM exposes debug counters through individual debugfs files. Monitoring these counters requires debugfs to be enabled/accessible for the application, which might not be always the case. Additionally, periodic monitoring multiple debugfs files from userspace requires multiple file open/read/close + atoi conversion operations, which is not very efficient.

Let's expose new interface to userspace for garhering these statistics with one ioctl.

Two new ioctl methods are added:
- KVM_GET_SUPPORTED_DEBUGFS_STAT : Returns list of available counter names. Names correspond to the debugfs file names
- KVM_GET_DEBUGFS_VALUES : Returns list of u64 values each corresponding to a value described in KVM_GET_SUPPORTED_DEBUGFS_STAT.

Userspace application can read counter description once using KVM_GET_SUPPORTED_DEBUGFS_STAT and periodically invoke the KVM_GET_DEBUGFS_VALUES to get value update.

Signed-off-by Milan Pandurov <[email protected]>

Quoted 16 lines · Milan Pandurov

KVM exposes debug counters through individual debugfs files. Monitoring these counters requires debugfs to be enabled/accessible for the application, which might not be always the case. Additionally, periodic monitoring multiple debugfs files from userspace requires multiple file open/read/close + atoi conversion operations, which is not very efficient.

Let's expose new interface to userspace for garhering these statistics with one ioctl.

Two new ioctl methods are added:
  - KVM_GET_SUPPORTED_DEBUGFS_STAT : Returns list of available counter names. Names correspond to the debugfs file names
  - KVM_GET_DEBUGFS_VALUES : Returns list of u64 values each corresponding to a value described in KVM_GET_SUPPORTED_DEBUGFS_STAT.

Quoted 5 lines

Userspace application can read counter description once using KVM_GET_SUPPORTED_DEBUGFS_STAT and periodically invoke the KVM_GET_DEBUGFS_VALUES to get value update.

Signed-off-by Milan Pandurov <[email protected]>

Thanks a lot! I really love the idea to get stats directly from the kvm vm fd owner. This is so much better than poking at files from a random unrelated debug or stat file system.

I have a few comments overall though:

1)

This is an interface that requires a lot of logic and buffers from user space to retrieve individual, explicit counters. What if I just wanted to monitor the number of exits on every user space exit?

Also, we're suddenly making the debugfs names a full ABI, because through this interface we only identify the individual stats through their names. That means we can not remove stats or change their names, because people may rely on them, no? Thining about this again, maybe they already are an ABI because people rely on them in debugfs though?

I see two alternatives to this approach here:

a) ONE_REG

We can just add a new DEBUG arch in ONE_REG and expose ONE_REG per VM as well (if we really have to?). That gives us explicit identifiers for each stat with an explicit path to introduce new ones with very unique identifiers.

That would give us a very easily structured approach to retrieve individual counters.

b) part of the mmap'ed vcpu struct

We could simply move the counters into the shared struct that we expose to user space via mmap. IIRC we only have that per-vcpu, but then again most counters are per-vcpu anyway, so we would push the aggregation to user space.

For per-vm ones, maybe we can just add another mmap'ed shared page for the vm fd?

2) vcpu counters

Most of the counters count on vcpu granularity, but debugfs only gives us a full VM view. Whatever we do to improve the situation, we should definitely try to build something that allows us to get the counters per vcpu (as well).

The main purpose of these counters is monitoring. It can be quite important to know that only a single vCPU is going wild, compared to all of them for example.

Thanks,

Alex

Quoted 25 lines · Alexander Graf

Quoted 16 lines · Milan Pandurov

KVM exposes debug counters through individual debugfs files. Monitoring these counters requires debugfs to be enabled/accessible for the application, which might not be always the case. Additionally, periodic monitoring multiple debugfs files from userspace requires multiple file open/read/close + atoi conversion operations, which is not very efficient.

Let's expose new interface to userspace for garhering these statistics with one ioctl.

Two new ioctl methods are added:
  - KVM_GET_SUPPORTED_DEBUGFS_STAT : Returns list of available counter names. Names correspond to the debugfs file names
  - KVM_GET_DEBUGFS_VALUES : Returns list of u64 values each corresponding to a value described in KVM_GET_SUPPORTED_DEBUGFS_STAT.

Quoted 5 lines

Userspace application can read counter description once using KVM_GET_SUPPORTED_DEBUGFS_STAT and periodically invoke the KVM_GET_DEBUGFS_VALUES to get value update.

Signed-off-by Milan Pandurov <[email protected]>

Quoted 8 lines

Thanks a lot! I really love the idea to get stats directly from the kvm vm fd owner. This is so much better than poking at files from a random unrelated debug or stat file system.

I have a few comments overall though:

1)

This is an interface that requires a lot of logic and buffers from user space to retrieve individual, explicit counters. What if I just wanted to monitor the number of exits on every user space exit?

In case we want to cover such latency sensitive use cases solution b), with mmap'ed structs you suggested, would be a way to go, IMO.

Quoted 18 lines

Also, we're suddenly making the debugfs names a full ABI, because through this interface we only identify the individual stats through their names. That means we can not remove stats or change their names, because people may rely on them, no? Thining about this again, maybe they already are an ABI because people rely on them in debugfs though?

I see two alternatives to this approach here:

a) ONE_REG

We can just add a new DEBUG arch in ONE_REG and expose ONE_REG per VM as well (if we really have to?). That gives us explicit identifiers for each stat with an explicit path to introduce new ones with very unique identifiers.

That would give us a very easily structured approach to retrieve individual counters.

b) part of the mmap'ed vcpu struct

We could simply move the counters into the shared struct that we expose to user space via mmap. IIRC we only have that per-vcpu, but then again most counters are per-vcpu anyway, so we would push the aggregation to user space.

For per-vm ones, maybe we can just add another mmap'ed shared page for the vm fd?

Quoted 5 lines

2) vcpu counters

Most of the counters count on vcpu granularity, but debugfs only gives us a full VM view. Whatever we do to improve the situation, we should definitely try to build something that allows us to get the counters per vcpu (as well).

The main purpose of these counters is monitoring. It can be quite important to know that only a single vCPU is going wild, compared to all of them for example.

I agree, exposing per vcpu counters can be useful. I guess it didn't make much sense exposing them through debugfs so aggregation was done in kernel. However if we chose to go with approach 1-b) mmap counters struct in userspace, we could do this.

Quoted 14 lines · On 15.01.20 15:43, [email protected] wrote:

Quoted 10 lines

Quoted 6 lines

Let's expose new interface to userspace for garhering these statistics with one ioctl.

Userspace application can read counter description once using KVM_GET_SUPPORTED_DEBUGFS_STAT and periodically invoke the KVM_GET_DEBUGFS_VALUES to get value update.

This is an interface that requires a lot of logic and buffers from user space to retrieve individual, explicit counters. What if I just wanted to monitor the number of exits on every user space exit?

In case we want to cover such latency sensitive use cases solution b), with mmap'ed structs you suggested, would be a way to go, IMO.

Quoted 6 lines

Quoted 6 lines

Also, we're suddenly making the debugfs names a full ABI, because through this interface we only identify the individual stats through their names. That means we can not remove stats or change their names, because people may rely on them, no? Thining about this again, maybe they already are an ABI because people rely on them in debugfs though?

Quoted 8 lines

Quoted 8 lines

I see two alternatives to this approach here:

a) ONE_REG

We can just add a new DEBUG arch in ONE_REG and expose ONE_REG per VM as well (if we really have to?). That gives us explicit identifiers for each stat with an explicit path to introduce new ones with very unique identifiers.

Quoted 1 line

Quoted 1 line

b) part of the mmap'ed vcpu struct

Quoted 7 lines

Quoted 7 lines

2) vcpu counters

Most of the counters count on vcpu granularity, but debugfs only gives us a full VM view. Whatever we do to improve the situation, we should definitely try to build something that allows us to get the counters per vcpu (as well).

Quoted 8 lines

Quoted 3 lines

The main purpose of these counters is monitoring. It can be quite important to know that only a single vCPU is going wild, compared to all of them for example.

I agree, exposing per vcpu counters can be useful. I guess it didn't make much sense exposing them through debugfs so aggregation was done in kernel. However if we chose to go with approach 1-b) mmap counters struct in userspace, we could do this.

The reason I dislike the debugfs/statfs approach is that it generates a completely separate permission and access paths to the stats. That's great for full system monitoring, but really bad when you have multiple individual tenants on a single host.

Quoted 15 lines · Alexander Graf

Quoted 14 lines · On 15.01.20 15:43, [email protected] wrote:

Quoted 10 lines

Quoted 6 lines

Let's expose new interface to userspace for garhering these statistics with one ioctl.

Userspace application can read counter description once using KVM_GET_SUPPORTED_DEBUGFS_STAT and periodically invoke the KVM_GET_DEBUGFS_VALUES to get value update.

This is an interface that requires a lot of logic and buffers from user space to retrieve individual, explicit counters. What if I just wanted to monitor the number of exits on every user space exit?

In case we want to cover such latency sensitive use cases solution b), with mmap'ed structs you suggested, would be a way to go, IMO.

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Also, we're suddenly making the debugfs names a full ABI, because through this interface we only identify the individual stats through their names. That means we can not remove stats or change their names, because people may rely on them, no? Thining about this again, maybe they already are an ABI because people rely on them in debugfs though?

In theory not, in practice I have treated them as a kind of "soft" ABI:
if the meaning changes you should rename them, and removing them is fine, but you shouldn't for example change the unit of measure (which is not hard since they are all counters :) but perhaps you could have nanoseconds vs TSC cycles in the future for some counters).

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

I see two alternatives to this approach here:

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

a) ONE_REG

We can just add a new DEBUG arch in ONE_REG and expose ONE_REG per VM as well (if we really have to?). That gives us explicit identifiers for each stat with an explicit path to introduce new ones with very unique identifiers.

ONE_REG would force us to define constants for each counter, and would make it hard to retire them. I don't like this.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

b) part of the mmap'ed vcpu struct

Same here. Even if we say the semantics of the struct would be exposed to userspace via KVM_GET_SUPPORTED_DEBUGFS_STAT, someone might end up getting this wrong and expecting a particular layout. Milan's proposed API has the big advantage of being hard to get wrong for userspace. And pushing the aggregation to userspace is not a huge chore, but it's still a chore.

So unless someone has a usecase for latency-sensitive monitoring I'd prefer to keep it simple (usually these kind of stats can even make sense if you gather them over relatively large period of time, because then you'll probably use something else like tracepoints to actually pinpoint what's going on).

Quoted 17 lines

Quoted 16 lines

Quoted 11 lines

2) vcpu counters

Most of the counters count on vcpu granularity, but debugfs only gives us a full VM view. Whatever we do to improve the situation, we should definitely try to build something that allows us to get the counters per vcpu (as well).

The main purpose of these counters is monitoring. It can be quite important to know that only a single vCPU is going wild, compared to all of them for example.

I agree, exposing per vcpu counters can be useful. I guess it didn't make much sense exposing them through debugfs so aggregation was done in kernel. However if we chose to go with approach 1-b) mmap counters struct in userspace, we could do this.

Quoted 4 lines

The reason I dislike the debugfs/statfs approach is that it generates a completely separate permission and access paths to the stats. That's great for full system monitoring, but really bad when you have multiple individual tenants on a single host.

I agree, anything in sysfs is complementary to vmfd/vcpufd access.

Quoted 31 lines · Paolo Bonzini

Quoted 15 lines · Alexander Graf

Quoted 14 lines · On 15.01.20 15:43, [email protected] wrote:

Quoted 10 lines

Quoted 6 lines

Let's expose new interface to userspace for garhering these statistics with one ioctl.

Userspace application can read counter description once using KVM_GET_SUPPORTED_DEBUGFS_STAT and periodically invoke the KVM_GET_DEBUGFS_VALUES to get value update.

This is an interface that requires a lot of logic and buffers from user space to retrieve individual, explicit counters. What if I just wanted to monitor the number of exits on every user space exit?

In case we want to cover such latency sensitive use cases solution b), with mmap'ed structs you suggested, would be a way to go, IMO.

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Also, we're suddenly making the debugfs names a full ABI, because through this interface we only identify the individual stats through their names. That means we can not remove stats or change their names, because people may rely on them, no? Thining about this again, maybe they already are an ABI because people rely on them in debugfs though?

In theory not, in practice I have treated them as a kind of "soft" ABI:
if the meaning changes you should rename them, and removing them is fine, but you shouldn't for example change the unit of measure (which is not hard since they are all counters :) but perhaps you could have nanoseconds vs TSC cycles in the future for some counters).

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

I see two alternatives to this approach here:

Quoted 8 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

a) ONE_REG

We can just add a new DEBUG arch in ONE_REG and expose ONE_REG per VM as well (if we really have to?). That gives us explicit identifiers for each stat with an explicit path to introduce new ones with very unique identifiers.

ONE_REG would force us to define constants for each counter, and would make it hard to retire them.  I don't like this.

Why does it make it hard to retire them? We would just return -EINVAL on retrieval, like we do for any other non-supported ONE_REG.

It's the same as a file not existing in debugfs/statfs. Or an entry in the array of this patch to disappear.

Quoted 10 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

b) part of the mmap'ed vcpu struct

Same here.  Even if we say the semantics of the struct would be exposed to userspace via KVM_GET_SUPPORTED_DEBUGFS_STAT, someone might end up getting this wrong and expecting a particular layout.  Milan's proposed API has the big advantage of being hard to get wrong for userspace.  And pushing the aggregation to userspace is not a huge chore, but it's still a chore.

Quoted 5 lines

So unless someone has a usecase for latency-sensitive monitoring I'd prefer to keep it simple (usually these kind of stats can even make sense if you gather them over relatively large period of time, because then you'll probably use something else like tracepoints to actually pinpoint what's going on).

I tend to agree. Fetching them via an explicit call into the kernel is definitely the safer route.

Quoted 22 lines

Quoted 21 lines

Quoted 8 lines

Quoted 8 lines

2) vcpu counters

Most of the counters count on vcpu granularity, but debugfs only gives us a full VM view. Whatever we do to improve the situation, we should definitely try to build something that allows us to get the counters per vcpu (as well).

Quoted 8 lines

Quoted 3 lines

The main purpose of these counters is monitoring. It can be quite important to know that only a single vCPU is going wild, compared to all of them for example.

I agree, exposing per vcpu counters can be useful. I guess it didn't make much sense exposing them through debugfs so aggregation was done in kernel. However if we chose to go with approach 1-b) mmap counters struct in userspace, we could do this.

The reason I dislike the debugfs/statfs approach is that it generates a completely separate permission and access paths to the stats. That's great for full system monitoring, but really bad when you have multiple individual tenants on a single host.

Quoted 1 line

I agree, anything in sysfs is complementary to vmfd/vcpufd access.

Cool :).

So we only need to agree on ONE_REG vs. this patch mostly?

Alex

Quoted 87 lines · Alexander Graf

Quoted 31 lines · Paolo Bonzini

Quoted 15 lines · Alexander Graf

Quoted 14 lines · On 15.01.20 15:43, [email protected] wrote:

Quoted 10 lines

Quoted 6 lines

Let's expose new interface to userspace for garhering these statistics with one ioctl.

Userspace application can read counter description once using KVM_GET_SUPPORTED_DEBUGFS_STAT and periodically invoke the KVM_GET_DEBUGFS_VALUES to get value update.

This is an interface that requires a lot of logic and buffers from user space to retrieve individual, explicit counters. What if I just wanted to monitor the number of exits on every user space exit?

In case we want to cover such latency sensitive use cases solution b), with mmap'ed structs you suggested, would be a way to go, IMO.

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Also, we're suddenly making the debugfs names a full ABI, because through this interface we only identify the individual stats through their names. That means we can not remove stats or change their names, because people may rely on them, no? Thining about this again, maybe they already are an ABI because people rely on them in debugfs though?

In theory not, in practice I have treated them as a kind of "soft" ABI:
if the meaning changes you should rename them, and removing them is fine, but you shouldn't for example change the unit of measure (which is not hard since they are all counters :) but perhaps you could have nanoseconds vs TSC cycles in the future for some counters).

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

I see two alternatives to this approach here:

Quoted 8 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

a) ONE_REG

We can just add a new DEBUG arch in ONE_REG and expose ONE_REG per VM as well (if we really have to?). That gives us explicit identifiers for each stat with an explicit path to introduce new ones with very unique identifiers.

ONE_REG would force us to define constants for each counter, and would make it hard to retire them.  I don't like this.

Why does it make it hard to retire them? We would just return -EINVAL on retrieval, like we do for any other non-supported ONE_REG.

It's the same as a file not existing in debugfs/statfs. Or an entry in the array of this patch to disappear.

Quoted 10 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

b) part of the mmap'ed vcpu struct

Same here.  Even if we say the semantics of the struct would be exposed to userspace via KVM_GET_SUPPORTED_DEBUGFS_STAT, someone might end up getting this wrong and expecting a particular layout.  Milan's proposed API has the big advantage of being hard to get wrong for userspace.  And pushing the aggregation to userspace is not a huge chore, but it's still a chore.

Quoted 5 lines

So unless someone has a usecase for latency-sensitive monitoring I'd prefer to keep it simple (usually these kind of stats can even make sense if you gather them over relatively large period of time, because then you'll probably use something else like tracepoints to actually pinpoint what's going on).

I tend to agree. Fetching them via an explicit call into the kernel is definitely the safer route.

Quoted 22 lines

Quoted 21 lines

Quoted 8 lines

Quoted 8 lines

2) vcpu counters

Most of the counters count on vcpu granularity, but debugfs only gives us a full VM view. Whatever we do to improve the situation, we should definitely try to build something that allows us to get the counters per vcpu (as well).

Quoted 8 lines

Quoted 3 lines

The main purpose of these counters is monitoring. It can be quite important to know that only a single vCPU is going wild, compared to all of them for example.

I agree, exposing per vcpu counters can be useful. I guess it didn't make much sense exposing them through debugfs so aggregation was done in kernel. However if we chose to go with approach 1-b) mmap counters struct in userspace, we could do this.

The reason I dislike the debugfs/statfs approach is that it generates a completely separate permission and access paths to the stats. That's great for full system monitoring, but really bad when you have multiple individual tenants on a single host.

Quoted 5 lines

Quoted 1 line

I agree, anything in sysfs is complementary to vmfd/vcpufd access.

Cool :).

So we only need to agree on ONE_REG vs. this patch mostly?

What about extending KVM_GET_SUPPORTED_DEBUGFS_STAT with some additional information like the data type and unit? ONE_REG exposes additional semantics about data.

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

ONE_REG would force us to define constants for each counter, and would make it hard to retire them.  I don't like this.

Why does it make it hard to retire them? We would just return -EINVAL on retrieval, like we do for any other non-supported ONE_REG.

It's the same as a file not existing in debugfs/statfs. Or an entry in the array of this patch to disappear.

It's not a problem of exposing the type information - we have that today by implicitly saying "every counter is 64bit".

The thing I'm worried about is that we keep inventing these special purpose interfaces that really do nothing but transfer numbers in one way or another. ONE_REG's purpose was to unify them. Debug counters really are the same story.

Quoted 8 lines · Alexander Graf

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

ONE_REG would force us to define constants for each counter, and would make it hard to retire them.  I don't like this.

Why does it make it hard to retire them? We would just return -EINVAL on retrieval, like we do for any other non-supported ONE_REG.

It's the same as a file not existing in debugfs/statfs. Or an entry in the array of this patch to disappear.

The devil is in the details. For example, would you retire uapi/ constants and cause programs to fail compilation? Or do you keep the obsolete constants forever? Also, fixing the mapping from ONE_REG number to stat would mean a switch statement (or loop of some kind---a switch statement is basically an unrolled binary search) to access the stats. Instead returning the id in KVM_GET_SUPPORTED_DEBUGFS_STAT would simplify returning the stats to a simple copy_to_user.

Of course, some of the complexity would be punted to userspace. But userspace is much closer to the humans that ultimately look at the stats, so the question is: does userspace really care about knowing which stat is which, or do they just care about having a name that will ultimately be consumed by humans down the pipe? If the latter (which is also my gut feeling), that is also a point against ONE_REG.

Quoted 7 lines

It's not a problem of exposing the type information - we have that today by implicitly saying "every counter is 64bit".

The thing I'm worried about is that we keep inventing these special purpose interfaces that really do nothing but transfer numbers in one way or another. ONE_REG's purpose was to unify them. Debug counters really are the same story.

See above: I am not sure they are the same story because their consumers might be very different from registers. Registers are generally consumed by programs (to migrate VMs, for example) and only occasionally by humans, while stats are meant to be consumed by humans. We may disagree on whether this justifies a completely different API...

Quoted 5 lines

See above: I am not sure they are the same story because their consumers might be very different from registers.  Registers are generally consumed by programs (to migrate VMs, for example) and only occasionally by humans, while stats are meant to be consumed by humans.  We may disagree on whether this justifies a completely different API...

I don't fully agree on the "human" part here.

At the end of the day, you
want stats because you want to act on stats. Ideally, you want to do that fully automatically. Let me give you a few examples:

1) insn_emulation_fail triggers

You may want to feed all the failures into a database to check whether there is something wrong in the emulator.

2) (remote_)tlb_flush beyond certain threshold

If you see that you're constantly flushing remote TLBs, there's a good chance that you found a workload that may need tuning in KVM. You want to gather those stats across your full fleet of hosts, so that for the few occasions when you hit it, you can work with the actual VM owners to potentially improve their performance

3) exits beyond certain threshold

You know roughly how many exits your fleet would usually see, so you can configure an upper threshold on that. When you now have an automated way to notify you when the threshold is exceeded, you can check what that particular guest did to raise so many exits.

... and I'm sure there's room for a lot more potential stats that could be useful to gather to determine the health of a KVM environment, such as a "vcpu steal time" one or a "maximum time between two VMENTERS while the guest was in running state".

All of these should eventually feed into something bigger that collects the numbers across your full VM fleet, so that a human can take actions based on them. However, that means the values are no longer directly impacting a human, they need to feed into machines. And for that, exact, constant identifiers make much more sense

Quoted 7 lines · Alexander Graf

Quoted 5 lines

See above: I am not sure they are the same story because their consumers might be very different from registers.  Registers are generally consumed by programs (to migrate VMs, for example) and only occasionally by humans, while stats are meant to be consumed by humans.  We may disagree on whether this justifies a completely different API...

I don't fully agree on the "human" part here.

I agree it's not entirely about humans, but in general it's going to be rules and queries on monitoring tools, where 1) the monitoring tools' output is generally not KVM-specific, 2) the rules and queries will be written by humans.

So if the kernel produces insn_emulation_fail, the plugin for the monitoring tool will just log kvm.insn_emulation_fail. If the kernel produces 0x10042, the plugin will have to convert it and then log it. This is why I'm not sure that providing strings is actually less work for userspace.

I think we're in agreement then, just leaning onto the other side of the same fence. My take is that if I don't know whether a string is necessary, I'd rather not have a string :).

I guess as long as we do get stat information out per-vm as well as per-vcpu through vmfd and vcpufd, I'm happy overall.

So how strongly do you feel about the string based approach?

PS: You could btw easily add a "give me the string for a ONE_REG id" interface in KVM to translate from 0x10042 to "insn_emulation_fail" :).

Quoted 3 lines · Alexander Graf

I think we're in agreement then, just leaning onto the other side of the same fence. My take is that if I don't know whether a string is necessary, I'd rather not have a string :).

And for me it's if I don't know whether a #define is necessary, I'd rather not have a #define. So yeah we agree on everything except the userspace API (which is no small thing, but it's a start).

Quoted 4 lines

I guess as long as we do get stat information out per-vm as well as per-vcpu through vmfd and vcpufd, I'm happy overall.

So how strongly do you feel about the string based approach?

I like it, of course.

Quoted 2 lines

PS: You could btw easily add a "give me the string for a ONE_REG id" interface in KVM to translate from 0x10042 to "insn_emulation_fail" :).

That could actually be somewhat useful for VCPU registers as well (give me the string and type, and a list of valid ONE_REG ids). If that was

the case, of course it would be fine for me to use ONE_REG on a VM. The part which I don't like is having to make all ONE_REG part of the userspace ABI/API.

Quoted 3 lines

the case, of course it would be fine for me to use ONE_REG on a VM.  The part which I don't like is having to make all ONE_REG part of the userspace ABI/API.

We don't have all of ONE_REG as part of the user space ABI today.

But I like the idea of a ONE_REG query interface that gives you the list of available registers and a string representation of them. It would make programming kvm from Python so easy!

Quoted 4 lines · Alexander Graf

Quoted 3 lines

the case, of course it would be fine for me to use ONE_REG on a VM.  The part which I don't like is having to make all ONE_REG part of the userspace ABI/API.

We don't have all of ONE_REG as part of the user space ABI today.

Still those that exist cannot change their id. This makes them a part of the userspace ABI.

Quoted 3 lines

But I like the idea of a ONE_REG query interface that gives you the list of available registers and a string representation of them. It would make programming kvm from Python so easy!

Yeah, wouldn't it? Milan, what do you think about it?

Paolo

Quoted 4 lines

Paolo

I agree, extending the API with GET_AVAILABLE_ONE_REGS (and possibly a bitmask argument to narrow down which type of registers userspace is interested in) is a clean solution. We won't require userspace to rely on constants in compile time if it doesn't need to.

Only concern is that now we need to have some kind of datastructure for keeping the mappings between all available ONE_REG IDs and their strings/descriptions. Additionally enforcing that newly added ONE_REGs always get added to that mapping, is also necessary.

Quoted 8 lines · On 23/01/20 16:27, [email protected] wrote:

Quoted 3 lines

Paolo

I agree, extending the API with GET_AVAILABLE_ONE_REGS (and possibly a bitmask argument to narrow down which type of registers userspace is interested in) is a clean solution. We won't require userspace to rely on constants in compile time if it doesn't need to.

Quoted 4 lines

Only concern is that now we need to have some kind of datastructure for keeping the mappings between all available ONE_REG IDs and their strings/descriptions. Additionally enforcing that newly added ONE_REGs always get added to that mapping, is also necessary.

For now just do the implementation for VM ONE_REGs. We'll worry about the existing VCPU registers later.

Paolo