RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Eduardo, Jan I will update tsc deadline timer patch (at qemu-kvm side) recently. Have you made a final agreement of the issue 'KVM_CAP_TSC_DEADLINE_TIMER' vs. 'GET_SUPPORTED_CPUID'? Thanks, Jinsong Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) On Tue, Apr 24, 2012 at 06:06:55PM +0200, Jan Kiszka wrote: On 2012-04-23 22:02, Eduardo Habkost wrote: On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote: However, that was how I interpreted this GET_SUPPORTED_CPUID. In fact, it is used as kernel or hardware does not _prevent_ already. And in that sense, it's ok to enable even features that are not in kernel/hardware hands. We should point out this fact in the documentation. I see GET_SUPPORTED_CPUID as just a what userspace can enable because the kernel and the hardware support it (= don't prevent it), as long as userspace has the required support (meaning A+B). It's a bit like KVM_CHECK_EXTENSION, but with the nice feature that that the capabilities map directly to CPUID bits. So, it's not clear to me: now you are OK with adding TSC_DEADLINE to GET_SUPPORTED_CPUID? But we still have the issue of -cpu host not knowing what can be safely enabled (without userspace feature-specific setup code), or not. Do you have any suggestion for that? Avi, do you have any suggestion? First of all, I bet this was already broken with the introduction of x2apic. So TSC deadline won't make it worse. I guess we need to address this in userspace, first by masking those features out, later by actually emulating them. I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. And I still don't know the answer to: - How to precisely define the groups (A) and (B)? - requires additional code only if migration is required qualifies as (B) or (A)? Re: documentation, isn't the following paragraph (already present on api.txt) sufficient? The entries returned are the host cpuid as returned by the cpuid instruction, with unknown or unsupported features masked out. Some features (for example, x2apic), may not be present in the host cpu, but are exposed by kvm if it can emulate them efficiently. That suggests such features are always emulated - which is not true. They are either emulated, or nothing _prevents_ their emulation by user space. Well... it's a bit more complicated than that: the current semantics are a bit more than doesn't prevent, as in theory every single feature can be emulated by userspace, without any help from the kernel. So, if doesn't prevent were the only criteria, the kernel would set every single feature bit on GET_SUPPORTED_CPUID, making it not very useful. At least in the case of x2apic, the kernel is using GET_SUPPORTED_CPUID to expose a _capability_ too: when x2apic is present on GET_SUPPORTED_CPUID, userspace knows that in addition to not preventing the feature from being enabled, the kernel is now able to emulate x2apic (if proper setup is made by userspace). A kernel that can't emulate x2apic (even if userspace was allowed to emulate it completely in userspace) would never have x2apic enabled on GET_SUPPORTED_CPUID. Like I said previously, in the end GET_SUPPORTED_CPUID is
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On Thu, Jun 14, 2012 at 07:02:03PM +, Liu, Jinsong wrote: Eduardo, Jan I will update tsc deadline timer patch (at qemu-kvm side) recently. Have you made a final agreement of the issue 'KVM_CAP_TSC_DEADLINE_TIMER' vs. 'GET_SUPPORTED_CPUID'? I don't think there's a final agreement, but I was convinced later that it's probably better to _not_ have TSC-deadline on GET_SUPPORTED_CPUID, at least not by default. Even if this is changed in the future, it's a good idea to make qemu support the KVM_CAP_TSC_DEADLINE_TIMER method if running on older kernels, anyway. Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) On Tue, Apr 24, 2012 at 06:06:55PM +0200, Jan Kiszka wrote: On 2012-04-23 22:02, Eduardo Habkost wrote: On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote: However, that was how I interpreted this GET_SUPPORTED_CPUID. In fact, it is used as kernel or hardware does not _prevent_ already. And in that sense, it's ok to enable even features that are not in kernel/hardware hands. We should point out this fact in the documentation. I see GET_SUPPORTED_CPUID as just a what userspace can enable because the kernel and the hardware support it (= don't prevent it), as long as userspace has the required support (meaning A+B). It's a bit like KVM_CHECK_EXTENSION, but with the nice feature that that the capabilities map directly to CPUID bits. So, it's not clear to me: now you are OK with adding TSC_DEADLINE to GET_SUPPORTED_CPUID? But we still have the issue of -cpu host not knowing what can be safely enabled (without userspace feature-specific setup code), or not. Do you have any suggestion for that? Avi, do you have any suggestion? First of all, I bet this was already broken with the introduction of x2apic. So TSC deadline won't make it worse. I guess we need to address this in userspace, first by masking those features out, later by actually emulating them. I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. And I still don't know the answer to: - How to precisely define the groups (A) and (B)? - requires additional code only if migration is required qualifies as (B) or (A)? Re: documentation, isn't the following paragraph (already present on api.txt) sufficient? The entries returned are the host cpuid as returned by the cpuid instruction, with unknown or unsupported features masked out. Some features (for example, x2apic), may not be present in the host cpu, but are exposed by kvm if it can emulate them efficiently. That suggests such features are always emulated - which is not true. They are either emulated, or nothing _prevents_ their emulation by user space. Well... it's a bit more complicated than that: the current semantics are a bit more than doesn't prevent, as in theory every single feature can be emulated by userspace, without any help from the kernel. So, if doesn't prevent were the only criteria, the kernel would set every single feature bit on GET_SUPPORTED_CPUID, making it not very useful. At least in the case of x2apic, the kernel is using
RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Eduardo Habkost wrote: On Thu, Jun 14, 2012 at 07:02:03PM +, Liu, Jinsong wrote: Eduardo, Jan I will update tsc deadline timer patch (at qemu-kvm side) recently. Have you made a final agreement of the issue 'KVM_CAP_TSC_DEADLINE_TIMER' vs. 'GET_SUPPORTED_CPUID'? I don't think there's a final agreement, but I was convinced later that it's probably better to _not_ have TSC-deadline on GET_SUPPORTED_CPUID, at least not by default. Even if this is changed in the future, it's a good idea to make qemu support the KVM_CAP_TSC_DEADLINE_TIMER method if running on older kernels, anyway. OK, so I will coding based on current KVM_CAP_TSC_DEADLINE_TIMER method. Thanks for clarifying! Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) On Tue, Apr 24, 2012 at 06:06:55PM +0200, Jan Kiszka wrote: On 2012-04-23 22:02, Eduardo Habkost wrote: On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote: However, that was how I interpreted this GET_SUPPORTED_CPUID. In fact, it is used as kernel or hardware does not _prevent_ already. And in that sense, it's ok to enable even features that are not in kernel/hardware hands. We should point out this fact in the documentation. I see GET_SUPPORTED_CPUID as just a what userspace can enable because the kernel and the hardware support it (= don't prevent it), as long as userspace has the required support (meaning A+B). It's a bit like KVM_CHECK_EXTENSION, but with the nice feature that that the capabilities map directly to CPUID bits. So, it's not clear to me: now you are OK with adding TSC_DEADLINE to GET_SUPPORTED_CPUID? But we still have the issue of -cpu host not knowing what can be safely enabled (without userspace feature-specific setup code), or not. Do you have any suggestion for that? Avi, do you have any suggestion? First of all, I bet this was already broken with the introduction of x2apic. So TSC deadline won't make it worse. I guess we need to address this in userspace, first by masking those features out, later by actually emulating them. I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. And I still don't know the answer to: - How to precisely define the groups (A) and (B)? - requires additional code only if migration is required qualifies as (B) or (A)? Re: documentation, isn't the following paragraph (already present on api.txt) sufficient? The entries returned are the host cpuid as returned by the cpuid instruction, with unknown or unsupported features masked out. Some features (for example, x2apic), may not be present in the host cpu, but are exposed by kvm if it can emulate them efficiently. That suggests such features are always emulated - which is not true. They are either emulated, or nothing _prevents_ their emulation by user space. Well... it's a bit more complicated than that: the current semantics are a bit more than doesn't prevent, as in theory every single feature can be emulated by userspace, without any help from the kernel. So, if doesn't prevent were the only criteria, the kernel would set every single feature bit on GET_SUPPORTED_CPUID, making it not very useful. At least in the case of x2apic,
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On Wed, May 09, 2012 at 04:38:02PM -0300, Eduardo Habkost wrote: On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote: On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:51, Gleb Natapov wrote: On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:14, Gleb Natapov g...@redhat.com wrote: On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote: On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? How kernel should know that userspace does not emulate it? You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. Same here. Well, technically both of those features can't be implemented in userspace right now since MSRs are terminated in the kernel, but I Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case. You mean terminating MSRs in kernel does not sound like the greatest design? I do not disagree. That is why IMO kernel can't filter out TSC-deadline and x2apic like you suggest. I still don't see why it can't. Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet. Now, we implement TSC-deadline in the kernel. We still filter TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide an interface to user space that says call me to enable TSC-deadline CPUID, but only if you're using the in-kernel apic We have that interface already, it is called KVM_SET_CPUID. :-) New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic. Old user space doesn't call that ioctl. First of all we already have TSC-deadline in GET_SUPORTED_CPUID without any additional ioctls. And
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On 05/10/2012 02:53 PM, Gleb Natapov wrote: On Wed, May 09, 2012 at 04:38:02PM -0300, Eduardo Habkost wrote: On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote: On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:51, Gleb Natapov wrote: On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:14, Gleb Natapovg...@redhat.com wrote: On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote: On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? How kernel should know that userspace does not emulate it? You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. Same here. Well, technically both of those features can't be implemented in userspace right now since MSRs are terminated in the kernel, but I Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case. You mean terminating MSRs in kernel does not sound like the greatest design? I do not disagree. That is why IMO kernel can't filter out TSC-deadline and x2apic like you suggest. I still don't see why it can't. Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet. Now, we implement TSC-deadline in the kernel. We still filter TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide an interface to user space that says call me to enable TSC-deadline CPUID, but only if you're using the in-kernel apic We have that interface already, it is called KVM_SET_CPUID. :-) New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic. Old user space doesn't call that ioctl. First of all we already have TSC-deadline in GET_SUPORTED_CPUID without any additional ioctls. And second I do not see why we need additional iostls here. We don't have TSC-deadline set today (and that's what started this thread), but we have x2apic. So what you say is true for x2apic, anyway. Hmm, strange. But s/TSC-deadline/x2apic/ and the point stands :) Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not from KVM_SET_CPUID? For those two features it may
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On Thu, May 10, 2012 at 03:21:41PM +0200, Alexander Graf wrote: On 05/10/2012 02:53 PM, Gleb Natapov wrote: On Wed, May 09, 2012 at 04:38:02PM -0300, Eduardo Habkost wrote: On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote: On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:51, Gleb Natapov wrote: On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:14, Gleb Natapovg...@redhat.com wrote: On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote: On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? How kernel should know that userspace does not emulate it? You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. Same here. Well, technically both of those features can't be implemented in userspace right now since MSRs are terminated in the kernel, but I Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case. You mean terminating MSRs in kernel does not sound like the greatest design? I do not disagree. That is why IMO kernel can't filter out TSC-deadline and x2apic like you suggest. I still don't see why it can't. Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet. Now, we implement TSC-deadline in the kernel. We still filter TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide an interface to user space that says call me to enable TSC-deadline CPUID, but only if you're using the in-kernel apic We have that interface already, it is called KVM_SET_CPUID. :-) New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic. Old user space doesn't call that ioctl. First of all we already have TSC-deadline in GET_SUPORTED_CPUID without any additional ioctls. And second I do not see why we need additional iostls here. We don't have TSC-deadline set today (and that's what started this thread), but we have x2apic. So what you say is true for x2apic, anyway. Hmm, strange. But s/TSC-deadline/x2apic/ and the point stands :) Hmm, so may be I
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On Thu, May 10, 2012 at 04:39:45PM +0300, Gleb Natapov wrote: On Thu, May 10, 2012 at 03:21:41PM +0200, Alexander Graf wrote: On 05/10/2012 02:53 PM, Gleb Natapov wrote: On Wed, May 09, 2012 at 04:38:02PM -0300, Eduardo Habkost wrote: On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote: On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:51, Gleb Natapov wrote: On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:14, Gleb Natapovg...@redhat.com wrote: On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote: On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? How kernel should know that userspace does not emulate it? You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. Same here. Well, technically both of those features can't be implemented in userspace right now since MSRs are terminated in the kernel, but I Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case. You mean terminating MSRs in kernel does not sound like the greatest design? I do not disagree. That is why IMO kernel can't filter out TSC-deadline and x2apic like you suggest. I still don't see why it can't. Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet. Now, we implement TSC-deadline in the kernel. We still filter TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide an interface to user space that says call me to enable TSC-deadline CPUID, but only if you're using the in-kernel apic We have that interface already, it is called KVM_SET_CPUID. :-) New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic. Old user space doesn't call that ioctl. First of all we already have TSC-deadline in GET_SUPORTED_CPUID without any additional ioctls. And second I do not see why we need additional iostls here. We don't have TSC-deadline
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On 05/07/2012 08:21 PM, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? Sorry for the delay, the easy answers first: I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. The purpose of -cpu host is to let guests use ISA features that the host CPU provides. Those would not need any KVM/QEMU intervention, because they work out of the box. Things like AES or SSE4.2, which are used by Linux and glibc, for instance. Users would expect those to be usable, and actually we only need to set the bits and are done. A second goal is to get rid of the awkward and artificial family/model/stepping settings and just let the guest see the actual CPU model. This has some strings attached, though, but other virtualization solution do it the same way and they can cope with it. A third thing currently not really addressed are the more informational bits like cache size and organization and TLB layout. Those are properties of the (core) CPU (except shared L3, cache maybe) and apply to guests as well. I don't see any reason why this should not be exposed to the guest. From the top of my head I don't know any prominent user (just glibc reading the cache size, but not really using it), but I guess there are applications which benefit from it. What clearly is not the intention of -cpu host is to provide access to every feature a certain CPU model provides. So things which require emulation or hypervisor intervention are not in the primary use case. That does not mean that they don't need to implemented, but that was not the purpose of -cpu host and they should be handled independently. Regards, Andre. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote: On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? How kernel should know that userspace does not emulate it? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. Same here. Well, technically both of those features can't be implemented in userspace right now since MSRs are terminated in the kernel, but I wouldn't make it into ABI. -- Gleb. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On 09.05.2012, at 10:14, Gleb Natapov g...@redhat.com wrote: On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote: On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? How kernel should know that userspace does not emulate it? You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. Same here. Well, technically both of those features can't be implemented in userspace right now since MSRs are terminated in the kernel, but I Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case. wouldn't make it into ABI. -- Gleb. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:14, Gleb Natapov g...@redhat.com wrote: On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote: On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? How kernel should know that userspace does not emulate it? You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. Same here. Well, technically both of those features can't be implemented in userspace right now since MSRs are terminated in the kernel, but I Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case. You mean terminating MSRs in kernel does not sound like the greatest design? I do not disagree. That is why IMO kernel can't filter out TSC-deadline and x2apic like you suggest. wouldn't make it into ABI. -- Gleb. -- Gleb. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:51, Gleb Natapov wrote: On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:14, Gleb Natapov g...@redhat.com wrote: On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote: On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? How kernel should know that userspace does not emulate it? You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. Same here. Well, technically both of those features can't be implemented in userspace right now since MSRs are terminated in the kernel, but I Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case. You mean terminating MSRs in kernel does not sound like the greatest design? I do not disagree. That is why IMO kernel can't filter out TSC-deadline and x2apic like you suggest. I still don't see why it can't. Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet. Now, we implement TSC-deadline in the kernel. We still filter TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide an interface to user space that says call me to enable TSC-deadline CPUID, but only if you're using the in-kernel apic New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic. Old user space doesn't call that ioctl. First of all we already have TSC-deadline in GET_SUPORTED_CPUID without any additional ioctls. And second I do not see why we need additional iostls here. Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not from KVM_SET_CPUID? For those two features it may make sense indeed. Not sure there won't be others that are not dependent on irq chip presence. You propose to add additional ioctls to enable them if they appear? So at the end all bits in GET_SUPPORTED_CPUID are consistent with what user
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On 05/09/2012 11:38 AM, Gleb Natapov wrote: On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:51, Gleb Natapov wrote: On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:14, Gleb Natapovg...@redhat.com wrote: On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote: On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? How kernel should know that userspace does not emulate it? You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. Same here. Well, technically both of those features can't be implemented in userspace right now since MSRs are terminated in the kernel, but I Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case. You mean terminating MSRs in kernel does not sound like the greatest design? I do not disagree. That is why IMO kernel can't filter out TSC-deadline and x2apic like you suggest. I still don't see why it can't. Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet. Now, we implement TSC-deadline in the kernel. We still filter TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide an interface to user space that says call me to enable TSC-deadline CPUID, but only if you're using the in-kernel apic New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic. Old user space doesn't call that ioctl. First of all we already have TSC-deadline in GET_SUPORTED_CPUID without any additional ioctls. And second I do not see why we need additional iostls here. Yeah, some times our ABI is already broken :(. What a shame... Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not from KVM_SET_CPUID? For those two features it may make sense indeed. Not sure there won't be others that are not dependent on irq chip presence. You propose to add additional ioctls to enable them if they appear? That's the only backwards compatible way to design this without putting a plethora of knowledge into user space I can see, yes. So at the end all bits in
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote: On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:51, Gleb Natapov wrote: On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:14, Gleb Natapov g...@redhat.com wrote: On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote: On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? How kernel should know that userspace does not emulate it? You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. Same here. Well, technically both of those features can't be implemented in userspace right now since MSRs are terminated in the kernel, but I Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case. You mean terminating MSRs in kernel does not sound like the greatest design? I do not disagree. That is why IMO kernel can't filter out TSC-deadline and x2apic like you suggest. I still don't see why it can't. Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet. Now, we implement TSC-deadline in the kernel. We still filter TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide an interface to user space that says call me to enable TSC-deadline CPUID, but only if you're using the in-kernel apic We have that interface already, it is called KVM_SET_CPUID. :-) New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic. Old user space doesn't call that ioctl. First of all we already have TSC-deadline in GET_SUPORTED_CPUID without any additional ioctls. And second I do not see why we need additional iostls here. We don't have TSC-deadline set today (and that's what started this thread), but we have x2apic. So what you say is true for x2apic, anyway. Hmm, so may be I
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On 09.05.2012, at 21:38, Eduardo Habkost wrote: On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote: On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:51, Gleb Natapov wrote: On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote: On 09.05.2012, at 10:14, Gleb Natapov g...@redhat.com wrote: On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote: On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? How kernel should know that userspace does not emulate it? You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. Same here. Well, technically both of those features can't be implemented in userspace right now since MSRs are terminated in the kernel, but I Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case. You mean terminating MSRs in kernel does not sound like the greatest design? I do not disagree. That is why IMO kernel can't filter out TSC-deadline and x2apic like you suggest. I still don't see why it can't. Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet. Now, we implement TSC-deadline in the kernel. We still filter TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide an interface to user space that says call me to enable TSC-deadline CPUID, but only if you're using the in-kernel apic We have that interface already, it is called KVM_SET_CPUID. :-) New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic. Old user space doesn't call that ioctl. First of all we already have TSC-deadline in GET_SUPORTED_CPUID without any additional ioctls. And second I do not see why we need additional iostls here. We don't have TSC-deadline set today (and that's what started this thread), but we have x2apic. So what you say is true for x2apic, anyway. Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not from KVM_SET_CPUID? For those two features it may make
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. - x2apic: ditto. I am not sure about XSAVE: an old qemu version would call kvm_put_fpu() instead of kvm_put_xsave() on kvm_arch_put_registers(), but I don't know if this would have unexpected side-effects or not. I wouldn't be surprised if we find many other cases, as even the GET_SUPPORTED_CPUID documentation is explicit about that: Userspace can use the information returned by this ioctl [GET_SUPPORTED_CPUID] to construct cpuid information (for KVM_SET_CPUID2) that is consistent with hardware, kernel, and userspace capabilities, [...] ^^ All features I'm aware of work fine (without migration, but that one is moot for -cpu host anyway) as long as the host kvm implementation is fine with it (GET_SUPPORTED_CPUID). So they'd be category A. So, you would argue that GET_SUPPORTED_CPUID should include only features of type A? That's the opposite of what we have today. Maybe we could change the semantics to type-A-only if we define type A as: - Don't require any extra userspace support except for: - Migration support. - Enabling the in-kernel irqchip. If we agree on that semantics, -cpu host could safely enable all the fetures returned by GET_SUPPORTED_CPUID blindly, after making sure that migration support is disabled and the in-kernel irqchip is enabled. Then all type-B features will require defining a KVM_CAP_* capability instead of using GET_SUPPORTED_CPUID. It's the opposite of the direction I was proposing earlier in this thread, but it is starting to look like a better idea (otherwise -cpu host would never be reliable). If we agree on that semantics, it won't require any code change on the current code, just a documentation update. -- Eduardo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On 08.05.2012, at 22:14, Eduardo Habkost wrote: On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? - TSC-deadline: can't be enabled unless userspace takes care to enable the in-kernel irqchip. The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. I am not sure about XSAVE: an old qemu version would call kvm_put_fpu() instead of kvm_put_xsave() on kvm_arch_put_registers(), but I don't know if this would have unexpected side-effects or not. Then XSAVE awareness should be manually enabled by user space. That's what we have ENABLE_CAP for :). Do ENABLE_CAP(XSAVE) - get XSAVE as a bit in GET_SUPPORTED_CPUID. I wouldn't be surprised if we find many other cases, as even the GET_SUPPORTED_CPUID documentation is explicit about that: Userspace can use the information returned by this ioctl [GET_SUPPORTED_CPUID] to construct cpuid information (for KVM_SET_CPUID2) that is consistent with hardware, kernel, and userspace capabilities, [...] ^^ Yeah, so the intent is that kvm is aware of all the bits user space would care about. All features I'm aware of work fine (without migration, but that one is moot for -cpu host anyway) as long as the host kvm implementation is fine with it (GET_SUPPORTED_CPUID). So they'd be category A. So, you would argue that GET_SUPPORTED_CPUID should include only features of type A? That's the opposite of what we have today. Maybe we could change the semantics to type-A-only if we define type A as: - Don't require any extra userspace support except for: - Migration support. - Enabling the in-kernel irqchip. If we agree on that semantics, -cpu host could safely enable all the fetures returned by GET_SUPPORTED_CPUID blindly, after making sure that migration support is disabled and the in-kernel irqchip is enabled. Then all type-B features will require defining a KVM_CAP_* capability instead not instead. In addition. Define a KVM_CAP_ and do an ENABLE_CAP on that one to have it exposed. of using GET_SUPPORTED_CPUID. It's the opposite of the direction I was proposing earlier in this thread, but it is starting to look like a better idea (otherwise -cpu host would never be reliable). If we agree on that semantics, it won't require any code change on the current code, just a documentation update. Life is simple, eh? :) Alex -- To unsubscribe from this list: send the line unsubscribe
Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. -- Eduardo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Semantics of -cpu host (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)
On 07.05.2012, at 20:21, Eduardo Habkost wrote: Andre? Are you able to help to answer the question below? I would like to clarify what's the expected behavior of -cpu host to be able to continue working on it. I believe the code will need to be fixed on either case, but first we need to figure out what are the expectations/requirements, to know _which_ changes will be needed. On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: (CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) [...] I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. Can you think of any feature that'd go into category B? All features I'm aware of work fine (without migration, but that one is moot for -cpu host anyway) as long as the host kvm implementation is fine with it (GET_SUPPORTED_CPUID). So they'd be category A. Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On 2012-04-23 22:02, Eduardo Habkost wrote: On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote: On 2012-04-23 16:48, Eduardo Habkost wrote: Trying to summarize the points above: Groups (A) and (B) are: A) a feature that KVM supports and emulate by itself and can be enabled by userspace blindly, without requiring any additional userspace code to work. B) a feature that KVM supports but need support from userspace to work. We have to differentiate those two groups somehow, otherwise -cpu host will always risk being unstable (in case we can't identify group (B) and end up enabling a feature that will break) or useless (if group (A) is considered always empty). (If you think this two-group model is not sufficient, please let me know.) Note that I am discussing two things above: - Whether GET_SUPPORTED_CPUID should expose only features from group (A), or group (B) too. - One problem here is that today GET_SUPPORTED_CPUIDS have many examples of (B) features inside it. Should we stop doing that? We have exactly two for the category that I was concerned about: TSC deadline and X2APIC. The latter is already exposed unconditionally, even if the kernel does not provide emulation. So, you are right, TSC deadline is not a new scenario. Oh, that explains why you were seing it differently: if the kernel doesn't control anything about the feature exposure, there was no obvious need to add it to GET_SUPPORTED_CPUID. - TSC-deadline is the first case where we are _not_ doing that. It is the first CPU feature in KVM that can be enabled by userspace (as long as userspace does the proper setup), but it's not included on GET_SUPPORTED_CPUIDs. - Even the current documentation implies that group (B) is included: This ioctl returns x86 cpuid features which are supported by both the hardware and kvm. Userspace can use the information returned by this ioctl to construct cpuid information (for KVM_SET_CPUID2) that is consistent with hardware, kernel, and userspace capabilities, and ^^ with user requirements (for example, the user may wish to constrain cpuid to emulate older hardware, or for feature consistency across a cluster). In the specific case of TSC-deadline, I consider Qemu knowing that TSC-deadline can be enabled only if in-kernel irqchip is enabled as an userpace capability. - How to precisely define the groups (A) and (B)? - requires additional code only if migration is required qualifies as (B) or (A)? - requires in-kernel irqchip to be enabled to work qualifies as (B), doesn't it? My problem is that features like X2APIC and TSC deadline are exposed by the kernel without contributing to them (if in-kernel irqchip is off). They are not simply exposed by the kernel: they are enabled by userspace, after userspace deciding whether it's safe or not (based on multiple factors). However, that was how I interpreted this GET_SUPPORTED_CPUID. In fact, it is used as kernel or hardware does not _prevent_ already. And in that sense, it's ok to enable even features that are not in kernel/hardware hands. We should point out this fact in the documentation. I see GET_SUPPORTED_CPUID as just a what userspace can enable because the kernel and the hardware support it (= don't prevent it), as long as userspace has the required support (meaning A+B). It's a bit like KVM_CHECK_EXTENSION, but with the nice feature that that the capabilities map directly to CPUID bits. So, it's not clear to me: now you are OK with adding TSC_DEADLINE to GET_SUPPORTED_CPUID? But we still have the issue of -cpu host not knowing what can be safely enabled (without userspace feature-specific setup code), or not. Do you have any suggestion for that? Avi, do you have any suggestion? First of all, I bet this was already broken with the introduction of x2apic. So TSC deadline won't make it worse. I guess we need to address this in userspace, first by masking those features out, later by actually emulating them. And I still don't know the answer to: - How to precisely define the groups (A) and (B)? - requires additional code only if migration is required qualifies as (B) or (A)? Re: documentation, isn't the following paragraph (already present on api.txt) sufficient? The entries returned are the host cpuid as returned by the cpuid instruction, with unknown or unsupported features masked out. Some features (for example, x2apic), may not be present in the host cpu, but are exposed by kvm if it can emulate them efficiently. That suggests such features are always emulated - which is not true. They are either emulated, or nothing _prevents_ their emulation by user space. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
(CCing Andre Przywara, in case he can help to clarify what's the expected meaning of -cpu host) On Tue, Apr 24, 2012 at 06:06:55PM +0200, Jan Kiszka wrote: On 2012-04-23 22:02, Eduardo Habkost wrote: On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote: However, that was how I interpreted this GET_SUPPORTED_CPUID. In fact, it is used as kernel or hardware does not _prevent_ already. And in that sense, it's ok to enable even features that are not in kernel/hardware hands. We should point out this fact in the documentation. I see GET_SUPPORTED_CPUID as just a what userspace can enable because the kernel and the hardware support it (= don't prevent it), as long as userspace has the required support (meaning A+B). It's a bit like KVM_CHECK_EXTENSION, but with the nice feature that that the capabilities map directly to CPUID bits. So, it's not clear to me: now you are OK with adding TSC_DEADLINE to GET_SUPPORTED_CPUID? But we still have the issue of -cpu host not knowing what can be safely enabled (without userspace feature-specific setup code), or not. Do you have any suggestion for that? Avi, do you have any suggestion? First of all, I bet this was already broken with the introduction of x2apic. So TSC deadline won't make it worse. I guess we need to address this in userspace, first by masking those features out, later by actually emulating them. I am not sure I understand what you are proposing. Let me explain the use case I am thinking about: - Feature FOO is of type (A) (e.g. just a new instruction set that doesn't require additional userspace support) - User has a Qemu vesion that doesn't know anything about feature FOO - User gets a new CPU that supports feature FOO - User gets a new kernel that supports feature FOO (i.e. has FOO in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User expects to get feature FOO enabled if using -cpu host, without upgrading Qemu. The problem here is: to support the above use-case, userspace need a probing mechanism that can differentiate _new_ (previously unknown) features that are in group (A) (safe to blindly enable) from features that are in group (B) (that can't be enabled without an userspace upgrade). In short, it becomes a problem if we consider the following case: - Feature BAR is of type (B) (it can't be enabled without extra userspace support) - User has a Qemu version that doesn't know anything about feature BAR - User gets a new CPU that supports feature BAR - User gets a new kernel that supports feature BAR (i.e. has BAR in GET_SUPPORTED_CPUID) - User does _not_ upgrade Qemu. - User simply shouldn't get feature BAR enabled, even if using -cpu host, otherwise Qemu would break. If userspace always limited itself to features it knows about, it would be really easy to implement the feature without any new probing mechanism from the kernel. But that's not how I think users expect -cpu host to work. Maybe I am wrong, I don't know. I am CCing Andre, who introduced the -cpu host feature, in case he can explain what's the expected semantics on the cases above. And I still don't know the answer to: - How to precisely define the groups (A) and (B)? - requires additional code only if migration is required qualifies as (B) or (A)? Re: documentation, isn't the following paragraph (already present on api.txt) sufficient? The entries returned are the host cpuid as returned by the cpuid instruction, with unknown or unsupported features masked out. Some features (for example, x2apic), may not be present in the host cpu, but are exposed by kvm if it can emulate them efficiently. That suggests such features are always emulated - which is not true. They are either emulated, or nothing _prevents_ their emulation by user space. Well... it's a bit more complicated than that: the current semantics are a bit more than doesn't prevent, as in theory every single feature can be emulated by userspace, without any help from the kernel. So, if doesn't prevent were the only criteria, the kernel would set every single feature bit on GET_SUPPORTED_CPUID, making it not very useful. At least in the case of x2apic, the kernel is using GET_SUPPORTED_CPUID to expose a _capability_ too: when x2apic is present on GET_SUPPORTED_CPUID, userspace knows that in addition to not preventing the feature from being enabled, the kernel is now able to emulate x2apic (if proper setup is made by userspace). A kernel that can't emulate x2apic (even if userspace was allowed to emulate it completely in userspace) would never have x2apic enabled on GET_SUPPORTED_CPUID. Like I said previously, in the end GET_SUPPORTED_CPUID is just a capability querying mechanism like KVM_CHECK_EXTENSION (where each extension have a specific kernel-capability meaning), but with the nice feature of being automatically mapped to CPUID bits (with no need for additional KVM_CAP_* = CPUID translation in
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On Sat, Apr 21, 2012 at 09:23:50AM +0200, Jan Kiszka wrote: On 2012-04-20 17:36, Eduardo Habkost wrote: On Fri, Apr 20, 2012 at 05:19:17PM +0200, Jan Kiszka wrote: On 2012-04-20 17:00, Eduardo Habkost wrote: On Fri, Apr 20, 2012 at 12:12:38PM +0200, Jan Kiszka wrote: On 2012-04-19 22:03, Eduardo Habkost wrote: Jan/Avi: ping? I would like to get this ABI detail clarified so it can be implemented the right way on Qemu and KVM. My proposal is to simply add tsc-deadline to the data returned by GET_SUPPORTED_CPUID, making KVM_CAP_TSC_DEADLINE_TIMER unnecessary. IIRC, there were problems with this model to exclude that the feature gets reported this way without ensuring that in-kernel irqchip is actually activated. Please browse the discussions, it should be documented there. The flag was never added to GET_SUPPORTED_CPUID on any of the git repositories I have checked, and I don't see that being done anywhere on my KVM mailing list archives, either. So I don't see how we could have had issues with GET_SUPPORTED_CPUID, if it was never present on the code. What was present on the code before the fix, was coded that unconditinally enabled the flag even if Qemu never asked for it, and that really was wrong. GET_SUPPORTED_CPUID doesn't enable any flag: it just tells userspace that the hardware and KVM supports the feature (and, surprise, this is exactly what KVM_CAP_TSC_DEADLINE_TIMER means too, isn't it?). It's up to userspace to enable the CPUID bits according to user requirements and userspace capabilities (in other words: only when userspace knows it is safe because the in-kernel irqchip is enabled). If the above is not what GET_SUPPORTED_CPUID means, I would like to get that clarified, so I can submit an updated to KVM API documentation. Does old userspace filter out flags from GET_SUPPORTED_CPUID that it does not understand? It's even more strict than that: it only enables what was explicitly enabled on the CPU model asked by the user. But: The only exception is -cpu host, that tries to enable everything, even flags Qemu never heard of, and that is something that may require some changes on the API design (or at least documentation clarifications). Today -cpu host can't differentiate (A) a feature that KVM supports and emulate by itself and can be enabled without any support from userspace and (B) a feature that KVM supports but need support from userspace to be enabled. I am sure we will be able to find multiple examples of (B) inside the flags returned by GET_SUPPORTED_CPUID today. The problem remains that exposing TSC_DEADLINE via SUPPORTED_CPUID is wrong in case userspace doesn't select the in-kernel APIC. The kernel does _nothing_ about emulating the flag if userspace provides the APIC, so it must not expose this feature. Even if this had no practical impact (but it has: old qemu[-kvm] + userspace APIC + -cpu host), it would still be semantically broken. How is that different from any other feature that requires userspace to do the right thing to make the feature work? GET_SUPPORTED_CPUID just tells userspace that the flag is supported, but userspace has to be sure it will really work, before enabling it. In other words, I always assumed that features from the (B) group were always included on GET_SUPPORTED_CPUID, too. Yes, that risks breaking -cpu host, so we will probably need a better interface to let -cpu host know what can be enabled or not, anyway. We could decide to never add any new flag to GET_SUPPORTED_CPUID if it requires additional userspace support to work, from now on, and create new KVM_CAP_* flags for them. But, we really want to do that for almost every new CPUID feature bit in the future? Most CPU features do not depend on our in-kernel irqchips. But if there is something to simplify in retrieving information about such conditional features, let's do it. But many CPU features on GET_SUPPORTED_CPUID depend on other userspace capabilities, need userspace to do the right thing to make it work, and won't work if userspace doesn't do what's required. Why configuring the in-kernel irqchip is special? We also have the problem of defining what requires support from userspace to work means: I am not sure this has the same meaning for everybody. Many new features require userspace support only for migration, and nothing else, but some users don't need migration to work. Do those features qualify as (A), or as (B)? Works for most user also means breaks for some. And that is a bug, isn't it? It is, surely, but -cpu host is the only case where CPU features are enabled blindly, and migration is not expected to be available when using -cpu host, so in that case breaks only migration would still mean works for all (becasue except for -cpu host, Qemu won't enable any feature if it still doesn't have the proper migration support working).
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On 2012-04-23 16:48, Eduardo Habkost wrote: Trying to summarize the points above: Groups (A) and (B) are: A) a feature that KVM supports and emulate by itself and can be enabled by userspace blindly, without requiring any additional userspace code to work. B) a feature that KVM supports but need support from userspace to work. We have to differentiate those two groups somehow, otherwise -cpu host will always risk being unstable (in case we can't identify group (B) and end up enabling a feature that will break) or useless (if group (A) is considered always empty). (If you think this two-group model is not sufficient, please let me know.) Note that I am discussing two things above: - Whether GET_SUPPORTED_CPUID should expose only features from group (A), or group (B) too. - One problem here is that today GET_SUPPORTED_CPUIDS have many examples of (B) features inside it. Should we stop doing that? We have exactly two for the category that I was concerned about: TSC deadline and X2APIC. The latter is already exposed unconditionally, even if the kernel does not provide emulation. So, you are right, TSC deadline is not a new scenario. - TSC-deadline is the first case where we are _not_ doing that. It is the first CPU feature in KVM that can be enabled by userspace (as long as userspace does the proper setup), but it's not included on GET_SUPPORTED_CPUIDs. - Even the current documentation implies that group (B) is included: This ioctl returns x86 cpuid features which are supported by both the hardware and kvm. Userspace can use the information returned by this ioctl to construct cpuid information (for KVM_SET_CPUID2) that is consistent with hardware, kernel, and userspace capabilities, and ^^ with user requirements (for example, the user may wish to constrain cpuid to emulate older hardware, or for feature consistency across a cluster). In the specific case of TSC-deadline, I consider Qemu knowing that TSC-deadline can be enabled only if in-kernel irqchip is enabled as an userpace capability. - How to precisely define the groups (A) and (B)? - requires additional code only if migration is required qualifies as (B) or (A)? - requires in-kernel irqchip to be enabled to work qualifies as (B), doesn't it? My problem is that features like X2APIC and TSC deadline are exposed by the kernel without contributing to them (if in-kernel irqchip is off). However, that was how I interpreted this GET_SUPPORTED_CPUID. In fact, it is used as kernel or hardware does not _prevent_ already. And in that sense, it's ok to enable even features that are not in kernel/hardware hands. We should point out this fact in the documentation. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote: On 2012-04-23 16:48, Eduardo Habkost wrote: Trying to summarize the points above: Groups (A) and (B) are: A) a feature that KVM supports and emulate by itself and can be enabled by userspace blindly, without requiring any additional userspace code to work. B) a feature that KVM supports but need support from userspace to work. We have to differentiate those two groups somehow, otherwise -cpu host will always risk being unstable (in case we can't identify group (B) and end up enabling a feature that will break) or useless (if group (A) is considered always empty). (If you think this two-group model is not sufficient, please let me know.) Note that I am discussing two things above: - Whether GET_SUPPORTED_CPUID should expose only features from group (A), or group (B) too. - One problem here is that today GET_SUPPORTED_CPUIDS have many examples of (B) features inside it. Should we stop doing that? We have exactly two for the category that I was concerned about: TSC deadline and X2APIC. The latter is already exposed unconditionally, even if the kernel does not provide emulation. So, you are right, TSC deadline is not a new scenario. Oh, that explains why you were seing it differently: if the kernel doesn't control anything about the feature exposure, there was no obvious need to add it to GET_SUPPORTED_CPUID. - TSC-deadline is the first case where we are _not_ doing that. It is the first CPU feature in KVM that can be enabled by userspace (as long as userspace does the proper setup), but it's not included on GET_SUPPORTED_CPUIDs. - Even the current documentation implies that group (B) is included: This ioctl returns x86 cpuid features which are supported by both the hardware and kvm. Userspace can use the information returned by this ioctl to construct cpuid information (for KVM_SET_CPUID2) that is consistent with hardware, kernel, and userspace capabilities, and ^^ with user requirements (for example, the user may wish to constrain cpuid to emulate older hardware, or for feature consistency across a cluster). In the specific case of TSC-deadline, I consider Qemu knowing that TSC-deadline can be enabled only if in-kernel irqchip is enabled as an userpace capability. - How to precisely define the groups (A) and (B)? - requires additional code only if migration is required qualifies as (B) or (A)? - requires in-kernel irqchip to be enabled to work qualifies as (B), doesn't it? My problem is that features like X2APIC and TSC deadline are exposed by the kernel without contributing to them (if in-kernel irqchip is off). They are not simply exposed by the kernel: they are enabled by userspace, after userspace deciding whether it's safe or not (based on multiple factors). However, that was how I interpreted this GET_SUPPORTED_CPUID. In fact, it is used as kernel or hardware does not _prevent_ already. And in that sense, it's ok to enable even features that are not in kernel/hardware hands. We should point out this fact in the documentation. I see GET_SUPPORTED_CPUID as just a what userspace can enable because the kernel and the hardware support it (= don't prevent it), as long as userspace has the required support (meaning A+B). It's a bit like KVM_CHECK_EXTENSION, but with the nice feature that that the capabilities map directly to CPUID bits. So, it's not clear to me: now you are OK with adding TSC_DEADLINE to GET_SUPPORTED_CPUID? But we still have the issue of -cpu host not knowing what can be safely enabled (without userspace feature-specific setup code), or not. Do you have any suggestion for that? Avi, do you have any suggestion? And I still don't know the answer to: - How to precisely define the groups (A) and (B)? - requires additional code only if migration is required qualifies as (B) or (A)? Re: documentation, isn't the following paragraph (already present on api.txt) sufficient? The entries returned are the host cpuid as returned by the cpuid instruction, with unknown or unsupported features masked out. Some features (for example, x2apic), may not be present in the host cpu, but are exposed by kvm if it can emulate them efficiently. -- Eduardo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On 2012-04-20 17:36, Eduardo Habkost wrote: On Fri, Apr 20, 2012 at 05:19:17PM +0200, Jan Kiszka wrote: On 2012-04-20 17:00, Eduardo Habkost wrote: On Fri, Apr 20, 2012 at 12:12:38PM +0200, Jan Kiszka wrote: On 2012-04-19 22:03, Eduardo Habkost wrote: Jan/Avi: ping? I would like to get this ABI detail clarified so it can be implemented the right way on Qemu and KVM. My proposal is to simply add tsc-deadline to the data returned by GET_SUPPORTED_CPUID, making KVM_CAP_TSC_DEADLINE_TIMER unnecessary. IIRC, there were problems with this model to exclude that the feature gets reported this way without ensuring that in-kernel irqchip is actually activated. Please browse the discussions, it should be documented there. The flag was never added to GET_SUPPORTED_CPUID on any of the git repositories I have checked, and I don't see that being done anywhere on my KVM mailing list archives, either. So I don't see how we could have had issues with GET_SUPPORTED_CPUID, if it was never present on the code. What was present on the code before the fix, was coded that unconditinally enabled the flag even if Qemu never asked for it, and that really was wrong. GET_SUPPORTED_CPUID doesn't enable any flag: it just tells userspace that the hardware and KVM supports the feature (and, surprise, this is exactly what KVM_CAP_TSC_DEADLINE_TIMER means too, isn't it?). It's up to userspace to enable the CPUID bits according to user requirements and userspace capabilities (in other words: only when userspace knows it is safe because the in-kernel irqchip is enabled). If the above is not what GET_SUPPORTED_CPUID means, I would like to get that clarified, so I can submit an updated to KVM API documentation. Does old userspace filter out flags from GET_SUPPORTED_CPUID that it does not understand? It's even more strict than that: it only enables what was explicitly enabled on the CPU model asked by the user. But: The only exception is -cpu host, that tries to enable everything, even flags Qemu never heard of, and that is something that may require some changes on the API design (or at least documentation clarifications). Today -cpu host can't differentiate (A) a feature that KVM supports and emulate by itself and can be enabled without any support from userspace and (B) a feature that KVM supports but need support from userspace to be enabled. I am sure we will be able to find multiple examples of (B) inside the flags returned by GET_SUPPORTED_CPUID today. The problem remains that exposing TSC_DEADLINE via SUPPORTED_CPUID is wrong in case userspace doesn't select the in-kernel APIC. The kernel does _nothing_ about emulating the flag if userspace provides the APIC, so it must not expose this feature. Even if this had no practical impact (but it has: old qemu[-kvm] + userspace APIC + -cpu host), it would still be semantically broken. We could decide to never add any new flag to GET_SUPPORTED_CPUID if it requires additional userspace support to work, from now on, and create new KVM_CAP_* flags for them. But, we really want to do that for almost every new CPUID feature bit in the future? Most CPU features do not depend on our in-kernel irqchips. But if there is something to simplify in retrieving information about such conditional features, let's do it. We also have the problem of defining what requires support from userspace to work means: I am not sure this has the same meaning for everybody. Many new features require userspace support only for migration, and nothing else, but some users don't need migration to work. Do those features qualify as (A), or as (B)? Works for most user also means breaks for some. And that is a bug, isn't it? Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On 2012-04-19 22:03, Eduardo Habkost wrote: Jan/Avi: ping? I would like to get this ABI detail clarified so it can be implemented the right way on Qemu and KVM. My proposal is to simply add tsc-deadline to the data returned by GET_SUPPORTED_CPUID, making KVM_CAP_TSC_DEADLINE_TIMER unnecessary. IIRC, there were problems with this model to exclude that the feature gets reported this way without ensuring that in-kernel irqchip is actually activated. Please browse the discussions, it should be documented there. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On Fri, Apr 20, 2012 at 12:12:38PM +0200, Jan Kiszka wrote: On 2012-04-19 22:03, Eduardo Habkost wrote: Jan/Avi: ping? I would like to get this ABI detail clarified so it can be implemented the right way on Qemu and KVM. My proposal is to simply add tsc-deadline to the data returned by GET_SUPPORTED_CPUID, making KVM_CAP_TSC_DEADLINE_TIMER unnecessary. IIRC, there were problems with this model to exclude that the feature gets reported this way without ensuring that in-kernel irqchip is actually activated. Please browse the discussions, it should be documented there. The flag was never added to GET_SUPPORTED_CPUID on any of the git repositories I have checked, and I don't see that being done anywhere on my KVM mailing list archives, either. So I don't see how we could have had issues with GET_SUPPORTED_CPUID, if it was never present on the code. What was present on the code before the fix, was coded that unconditinally enabled the flag even if Qemu never asked for it, and that really was wrong. GET_SUPPORTED_CPUID doesn't enable any flag: it just tells userspace that the hardware and KVM supports the feature (and, surprise, this is exactly what KVM_CAP_TSC_DEADLINE_TIMER means too, isn't it?). It's up to userspace to enable the CPUID bits according to user requirements and userspace capabilities (in other words: only when userspace knows it is safe because the in-kernel irqchip is enabled). If the above is not what GET_SUPPORTED_CPUID means, I would like to get that clarified, so I can submit an updated to KVM API documentation. -- Eduardo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On 2012-04-20 17:00, Eduardo Habkost wrote: On Fri, Apr 20, 2012 at 12:12:38PM +0200, Jan Kiszka wrote: On 2012-04-19 22:03, Eduardo Habkost wrote: Jan/Avi: ping? I would like to get this ABI detail clarified so it can be implemented the right way on Qemu and KVM. My proposal is to simply add tsc-deadline to the data returned by GET_SUPPORTED_CPUID, making KVM_CAP_TSC_DEADLINE_TIMER unnecessary. IIRC, there were problems with this model to exclude that the feature gets reported this way without ensuring that in-kernel irqchip is actually activated. Please browse the discussions, it should be documented there. The flag was never added to GET_SUPPORTED_CPUID on any of the git repositories I have checked, and I don't see that being done anywhere on my KVM mailing list archives, either. So I don't see how we could have had issues with GET_SUPPORTED_CPUID, if it was never present on the code. What was present on the code before the fix, was coded that unconditinally enabled the flag even if Qemu never asked for it, and that really was wrong. GET_SUPPORTED_CPUID doesn't enable any flag: it just tells userspace that the hardware and KVM supports the feature (and, surprise, this is exactly what KVM_CAP_TSC_DEADLINE_TIMER means too, isn't it?). It's up to userspace to enable the CPUID bits according to user requirements and userspace capabilities (in other words: only when userspace knows it is safe because the in-kernel irqchip is enabled). If the above is not what GET_SUPPORTED_CPUID means, I would like to get that clarified, so I can submit an updated to KVM API documentation. Does old userspace filter out flags from GET_SUPPORTED_CPUID that it does not understand? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On Fri, Apr 20, 2012 at 05:19:17PM +0200, Jan Kiszka wrote: On 2012-04-20 17:00, Eduardo Habkost wrote: On Fri, Apr 20, 2012 at 12:12:38PM +0200, Jan Kiszka wrote: On 2012-04-19 22:03, Eduardo Habkost wrote: Jan/Avi: ping? I would like to get this ABI detail clarified so it can be implemented the right way on Qemu and KVM. My proposal is to simply add tsc-deadline to the data returned by GET_SUPPORTED_CPUID, making KVM_CAP_TSC_DEADLINE_TIMER unnecessary. IIRC, there were problems with this model to exclude that the feature gets reported this way without ensuring that in-kernel irqchip is actually activated. Please browse the discussions, it should be documented there. The flag was never added to GET_SUPPORTED_CPUID on any of the git repositories I have checked, and I don't see that being done anywhere on my KVM mailing list archives, either. So I don't see how we could have had issues with GET_SUPPORTED_CPUID, if it was never present on the code. What was present on the code before the fix, was coded that unconditinally enabled the flag even if Qemu never asked for it, and that really was wrong. GET_SUPPORTED_CPUID doesn't enable any flag: it just tells userspace that the hardware and KVM supports the feature (and, surprise, this is exactly what KVM_CAP_TSC_DEADLINE_TIMER means too, isn't it?). It's up to userspace to enable the CPUID bits according to user requirements and userspace capabilities (in other words: only when userspace knows it is safe because the in-kernel irqchip is enabled). If the above is not what GET_SUPPORTED_CPUID means, I would like to get that clarified, so I can submit an updated to KVM API documentation. Does old userspace filter out flags from GET_SUPPORTED_CPUID that it does not understand? It's even more strict than that: it only enables what was explicitly enabled on the CPU model asked by the user. But: The only exception is -cpu host, that tries to enable everything, even flags Qemu never heard of, and that is something that may require some changes on the API design (or at least documentation clarifications). Today -cpu host can't differentiate (A) a feature that KVM supports and emulate by itself and can be enabled without any support from userspace and (B) a feature that KVM supports but need support from userspace to be enabled. I am sure we will be able to find multiple examples of (B) inside the flags returned by GET_SUPPORTED_CPUID today. We could decide to never add any new flag to GET_SUPPORTED_CPUID if it requires additional userspace support to work, from now on, and create new KVM_CAP_* flags for them. But, we really want to do that for almost every new CPUID feature bit in the future? We also have the problem of defining what requires support from userspace to work means: I am not sure this has the same meaning for everybody. Many new features require userspace support only for migration, and nothing else, but some users don't need migration to work. Do those features qualify as (A), or as (B)? -- Eduardo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Jan/Avi: ping? I would like to get this ABI detail clarified so it can be implemented the right way on Qemu and KVM. My proposal is to simply add tsc-deadline to the data returned by GET_SUPPORTED_CPUID, making KVM_CAP_TSC_DEADLINE_TIMER unnecessary. On Fri, Mar 23, 2012 at 02:17:52PM +, Liu, Jinsong wrote: Eduardo Habkost wrote: On Fri, Mar 23, 2012 at 03:49:27AM +, Liu, Jinsong wrote: Eduardo Habkost wrote: [1] From Documentation/virtual/kvm/api.txt: KVM_GET_SUPPORTED_CPUID [...] This ioctl returns x86 cpuid features which are supported by both the hardware and kvm. Userspace can use the information returned by this ioctl to construct cpuid information (for KVM_SET_CPUID2) that is consistent with hardware, kernel, and userspace capabilities, and with ^^ user requirements (for example, the user may wish to constrain cpuid to emulate older hardware, or for feature consistency across a cluster). The fixbug patch is implemented by Jan and Avi, I reply per my understanding. No problem. I hope Jan or Avi can clarify this. I think for tsc deadline timer feature, KVM_CAP_TSC_DEADLINE_TIMER is slightly better than KVM_GET_SUPPORTED_CPUID. If use KVM_GET_SUPPORTED_CPUID, it means tsc deadline features bind to host cpuid, while it fact it could be pure software emulated by kvm (though currently it implemented as bound to hareware). For the sake of extension, it choose KVM_CAP_TSC_DEADLINE_TIMER. There's no requirement for GET_SUPPORTED_CPUID to be a subset of the host CPU features. If KVM can completely emulate the feature by software, then it can return the feature on GET_SUPPORTED_CPUID even if the host CPU doesn't have the feature. That's the case for x2apic, for example (see commit 0d1de2d901f4ba0972a3886496a44fb1d3300dbd). Jan/Avi, Could you elaborate more your thought? Thanks, Jinsong -- Eduardo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Eduardo Habkost wrote:
On Fri, Mar 09, 2012 at 09:52:29PM +0100, Jan Kiszka wrote:
On 2012-03-09 20:09, Liu, Jinsong wrote:
Jan Kiszka wrote:
On 2012-03-09 19:27, Liu, Jinsong wrote:
Jan Kiszka wrote:
On 2012-03-06 08:49, Liu, Jinsong wrote:
Jan,
Any comments? I feel some confused about your point 'disable
cpuid feature for older machine types by default': are you
planning a common approach for this common issue, or, you just
ask me a specific solution for the tsc deadline timer case?
I think a generic solution for this can be as simple as passing a
feature exclusion mask to cpu_init. You could simple append a
string of -feature1,-feature2 to the cpu model that is
specified on creation. And that string could be defined in the
compat machine descriptions. Does this make sense?
Jan, to prevent misunderstanding, I elaborate my understanding of
your points below (if any misunderstanding please point out to
me): = Your target is, to migrate from A(old
qemu) to B(new qemu) by
1. at A: qemu-version-A [-cpu whatever] // currently the
default machine type is pc-A
2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1
-feature2
B run new qemu-version-B (w/ new features 'feature1' and
'feature2'), but when B runs w/ compat '-machine pc-A', vm should
not see 'feature1' and 'feature2', so commandline append string to
cpu model '-cpu whatever -feature1 -feature2' to hidden new
feature1 and feature2 to vm, hence vm can see same cpuid features
(at B) as those at A (which means, no feature1, no feature2)
=
If my understanding of your thoughts is right, I think currently
qemu has satisfied your target, code refer to
pc_cpus_init(cpu_model) .. cpu_init(cpu_model)
-- cpu_x86_register(*env, cpu_model)
-- cpu_x86_find_by_name(*def, cpu_model) // parse '+/-
features', generate feature masks plus_features... // and
minus_features...(this is feature exclusion masks you want)
I think your point 'define in the compat machine description' is
unnecessary.
The user would have to specify the new feature as exclusions
*manually* on the command line if -machine pc-A doesn't inject them
*automatically*. So it is necessary to enhance qemu in this regard.
... You suggest 'append a string of -feature1,-feature2 to the
cpu model that is specified on creation' at your last email. Could
you tell me other way user exclude features? I only know qemu
command line :-(
I was thinking of something like
diff --git a/hw/boards.h b/hw/boards.h
index 667177d..2bae071 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -28,6 +28,7 @@ typedef struct QEMUMachine {
int is_default;
const char *default_machine_opts;
GlobalProperty *compat_props;
+const char *compat_cpu_features;
struct QEMUMachine *next;
} QEMUMachine;
diff --git a/hw/pc.c b/hw/pc.c
index bb9867b..4d11559 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -949,8 +949,9 @@ static CPUState *pc_new_cpu(const char
*cpu_model) return env; }
-void pc_cpus_init(const char *cpu_model)
+void pc_cpus_init(const char *cpu_model, const char
*append_features) { +char *model_and_features;
int i;
/* init CPUs */
@@ -961,10 +962,13 @@ void pc_cpus_init(const char *cpu_model)
cpu_model = qemu32;
#endif
}
+model_and_features = g_strconcat(cpu_model, ,,
append_features, NULL);
for(i = 0; i smp_cpus; i++) {
-pc_new_cpu(cpu_model);
+pc_new_cpu(model_and_features);
}
+
+g_free(model_and_features);
}
void pc_memory_init(MemoryRegion *system_memory,
However, getting machine.compat_cpu_features to pc_cpus_init is
rather
ugly. And we will have CPU devices with real properties soon. Then
the compat feature string could be passed that way, without changing
any
machine init function.
What if one cpudef had the wrong flags set but another cpudef was
correct, and we had to fix it on Qemu 1.1 for only one model? What if
the user _really_ wanted to edit the config file to add or remove a
given flag?
I think the best approach would be:
- Having versioned CPU model names;
- Specifying per-machine-type aliases.
See also the [libvirt] Modern CPU models cannot be used with libvirt
for related discussion.
The config file would look like:
[cpudef]
name = Westmere-1.0
features = [...] # no tsc-deadline
[...]
[cpudef]
name = Westmere-1.1
# so we don't have to copy everything from Westmere-1.0 manually:
base_cpudef = Westemere-1.0
# we could simply copy extend:
features = [...] tsc-deadline
# or, even better, if we had a append mechanism. e.g.:
#features_append = tsc-deadline
Then, on the machine-type table:
- Machine-types pc-1.0 and older would have:
.cpudef_aliases = {
{Westmere, Westmere-1.0},
[...]
}
- Machine-type pc-1.1 would have:
.cpudef_aliases = {
{Westmere, Westmere-1.1},
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On Fri, Mar 23, 2012 at 03:49:27AM +, Liu, Jinsong wrote: Eduardo Habkost wrote: [1] From Documentation/virtual/kvm/api.txt: KVM_GET_SUPPORTED_CPUID [...] This ioctl returns x86 cpuid features which are supported by both the hardware and kvm. Userspace can use the information returned by this ioctl to construct cpuid information (for KVM_SET_CPUID2) that is consistent with hardware, kernel, and userspace capabilities, and with ^^ user requirements (for example, the user may wish to constrain cpuid to emulate older hardware, or for feature consistency across a cluster). The fixbug patch is implemented by Jan and Avi, I reply per my understanding. No problem. I hope Jan or Avi can clarify this. I think for tsc deadline timer feature, KVM_CAP_TSC_DEADLINE_TIMER is slightly better than KVM_GET_SUPPORTED_CPUID. If use KVM_GET_SUPPORTED_CPUID, it means tsc deadline features bind to host cpuid, while it fact it could be pure software emulated by kvm (though currently it implemented as bound to hareware). For the sake of extension, it choose KVM_CAP_TSC_DEADLINE_TIMER. There's no requirement for GET_SUPPORTED_CPUID to be a subset of the host CPU features. If KVM can completely emulate the feature by software, then it can return the feature on GET_SUPPORTED_CPUID even if the host CPU doesn't have the feature. That's the case for x2apic, for example (see commit 0d1de2d901f4ba0972a3886496a44fb1d3300dbd). -- Eduardo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Eduardo Habkost wrote: On Fri, Mar 23, 2012 at 03:49:27AM +, Liu, Jinsong wrote: Eduardo Habkost wrote: [1] From Documentation/virtual/kvm/api.txt: KVM_GET_SUPPORTED_CPUID [...] This ioctl returns x86 cpuid features which are supported by both the hardware and kvm. Userspace can use the information returned by this ioctl to construct cpuid information (for KVM_SET_CPUID2) that is consistent with hardware, kernel, and userspace capabilities, and with ^^ user requirements (for example, the user may wish to constrain cpuid to emulate older hardware, or for feature consistency across a cluster). The fixbug patch is implemented by Jan and Avi, I reply per my understanding. No problem. I hope Jan or Avi can clarify this. I think for tsc deadline timer feature, KVM_CAP_TSC_DEADLINE_TIMER is slightly better than KVM_GET_SUPPORTED_CPUID. If use KVM_GET_SUPPORTED_CPUID, it means tsc deadline features bind to host cpuid, while it fact it could be pure software emulated by kvm (though currently it implemented as bound to hareware). For the sake of extension, it choose KVM_CAP_TSC_DEADLINE_TIMER. There's no requirement for GET_SUPPORTED_CPUID to be a subset of the host CPU features. If KVM can completely emulate the feature by software, then it can return the feature on GET_SUPPORTED_CPUID even if the host CPU doesn't have the feature. That's the case for x2apic, for example (see commit 0d1de2d901f4ba0972a3886496a44fb1d3300dbd). Jan/Avi, Could you elaborate more your thought? Thanks, Jinsong-- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Eduardo Habkost wrote:
On Tue, Mar 20, 2012 at 12:53:57PM +, Liu, Jinsong wrote:
Rik van Riel wrote:
On 03/09/2012 01:27 PM, Liu, Jinsong wrote:
As for 'tsc deadline' feature exposing, my patch (as attached) just
obey qemu general cpuid exposing method, and also satisfied your
target I think.
One question.
Why is TSC_DEADLINE not exposed in the cpuid allowed feature
bits in do_cpuid_ent() in arch/x86/kvm/x86.c ?
/* cpuid 1.ecx */
const u32 kvm_supported_word4_x86_features =
F(XMM3) | F(PCLMULQDQ) | 0 /* DTES64, MONITOR */ |
0 /* DS-CPL, VMX, SMX, EST */ |
0 /* TM2 */ | F(SSSE3) | 0 /* CNXT-ID */ | 0 /*
Reserved */ | F(FMA) | F(CX16) | 0 /* xTPR Update,
PDCM */ | 0 /* Reserved, DCA */ | F(XMM4_1) |
F(XMM4_2) | F(X2APIC) | F(MOVBE) | F(POPCNT) |
0 /* Reserved*/ | F(AES) | F(XSAVE) | 0 /* OSXSAVE
*/ | F(AVX) | F(F16C) | F(RDRAND);
Would it make sense to expose F(TSC_DEADLINE) above?
Or is there something truly special about tsc deadline
that means it should be different from everything else?
Because the feature depends on KVM_CREATE_IRQCHIP, which we cannot
guarantee will be called, we expose it via a
KVM_CAP_TSC_DEADLINE_TIMER and not KVM_GET_SUPPORTED_CPUID.
We have many other features that depend on proper support from
userspace otherwise they wouldn't work, but are listed on
GET_SUPPORTED_CPUID, don't we? Why is TSC-deadline special?
GET_SUPPORTED_CPUID just means KVM supports it as long as userspace
supports it too and enables it, it doesn't mean CPUID bit that will
be enabled by default[1].
Refer changeset 4d25a066b69fb749a39d0d4c610689dd765a0b0e.
That changeset was necessary because the kernel was doing this on
update_cpu
if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL
best-function == 0x1) {
best-ecx |= bit(X86_FEATURE_TSC_DEADLINE_TIMER);
And that was really wrong, because it enabled the bit unconditionally.
But I don't understand why KVM_CAP_TSC_DEADLINE_TIMER was created if
we already have KVM_GET_SUPPORTED_CPUID to tell userspace which bits
are supported by KVM.
Yes, exactly. That's why we need this patch.
[1] From Documentation/virtual/kvm/api.txt:
KVM_GET_SUPPORTED_CPUID
[...]
This ioctl returns x86 cpuid features which are supported by both the
hardware and kvm. Userspace can use the information returned by this
ioctl to construct cpuid information (for KVM_SET_CPUID2) that is
consistent with hardware, kernel, and userspace capabilities, and with
^^
user requirements (for example, the user may wish to constrain cpuid
to emulate older hardware, or for feature consistency across a
cluster).
The fixbug patch is implemented by Jan and Avi, I reply per my understanding.
I think for tsc deadline timer feature, KVM_CAP_TSC_DEADLINE_TIMER is slightly
better than KVM_GET_SUPPORTED_CPUID. If use KVM_GET_SUPPORTED_CPUID, it means
tsc deadline features bind to host cpuid, while it fact it could be pure
software emulated by kvm (though currently it implemented as bound to
hareware). For the sake of extension, it choose KVM_CAP_TSC_DEADLINE_TIMER.
Thanks,
Jinsong
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On Tue, Mar 20, 2012 at 12:53:57PM +, Liu, Jinsong wrote:
Rik van Riel wrote:
On 03/09/2012 01:27 PM, Liu, Jinsong wrote:
As for 'tsc deadline' feature exposing, my patch (as attached) just
obey qemu general cpuid exposing method, and also satisfied your
target I think.
One question.
Why is TSC_DEADLINE not exposed in the cpuid allowed feature
bits in do_cpuid_ent() in arch/x86/kvm/x86.c ?
/* cpuid 1.ecx */
const u32 kvm_supported_word4_x86_features =
F(XMM3) | F(PCLMULQDQ) | 0 /* DTES64, MONITOR */ |
0 /* DS-CPL, VMX, SMX, EST */ |
0 /* TM2 */ | F(SSSE3) | 0 /* CNXT-ID */ | 0 /*
Reserved */ |
F(FMA) | F(CX16) | 0 /* xTPR Update, PDCM */ |
0 /* Reserved, DCA */ | F(XMM4_1) |
F(XMM4_2) | F(X2APIC) | F(MOVBE) | F(POPCNT) |
0 /* Reserved*/ | F(AES) | F(XSAVE) | 0 /* OSXSAVE
*/ | F(AVX) |
F(F16C) | F(RDRAND);
Would it make sense to expose F(TSC_DEADLINE) above?
Or is there something truly special about tsc deadline
that means it should be different from everything else?
Because the feature depends on KVM_CREATE_IRQCHIP, which we cannot guarantee
will be called, we expose it via a KVM_CAP_TSC_DEADLINE_TIMER and not
KVM_GET_SUPPORTED_CPUID.
We have many other features that depend on proper support from userspace
otherwise they wouldn't work, but are listed on GET_SUPPORTED_CPUID,
don't we? Why is TSC-deadline special?
GET_SUPPORTED_CPUID just means KVM supports it as long as userspace
supports it too and enables it, it doesn't mean CPUID bit that will be
enabled by default[1].
Refer changeset 4d25a066b69fb749a39d0d4c610689dd765a0b0e.
That changeset was necessary because the kernel was doing this on
update_cpu
if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL
best-function == 0x1) {
best-ecx |= bit(X86_FEATURE_TSC_DEADLINE_TIMER);
And that was really wrong, because it enabled the bit unconditionally.
But I don't understand why KVM_CAP_TSC_DEADLINE_TIMER was created if we
already have KVM_GET_SUPPORTED_CPUID to tell userspace which bits are
supported by KVM.
[1] From Documentation/virtual/kvm/api.txt:
KVM_GET_SUPPORTED_CPUID
[...]
This ioctl returns x86 cpuid features which are supported by both the
hardware and kvm. Userspace can use the information returned by this
ioctl to construct cpuid information (for KVM_SET_CPUID2) that is
consistent with hardware, kernel, and userspace capabilities, and with
^^
user requirements (for example, the user may wish to constrain cpuid to
emulate older hardware, or for feature consistency across a cluster).
--
Eduardo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On Fri, Mar 09, 2012 at 09:52:29PM +0100, Jan Kiszka wrote:
On 2012-03-09 20:09, Liu, Jinsong wrote:
Jan Kiszka wrote:
On 2012-03-09 19:27, Liu, Jinsong wrote:
Jan Kiszka wrote:
On 2012-03-06 08:49, Liu, Jinsong wrote:
Jan,
Any comments? I feel some confused about your point 'disable cpuid
feature for older machine types by default': are you planning a
common approach for this common issue, or, you just ask me a
specific solution for the tsc deadline timer case?
I think a generic solution for this can be as simple as passing a
feature exclusion mask to cpu_init. You could simple append a string
of -feature1,-feature2 to the cpu model that is specified on
creation. And that string could be defined in the compat machine
descriptions. Does this make sense?
Jan, to prevent misunderstanding, I elaborate my understanding of
your points below (if any misunderstanding please point out to me):
=
Your target is, to migrate from A(old qemu) to B(new qemu) by
1. at A: qemu-version-A [-cpu whatever] // currently the
default machine type is pc-A
2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1
-feature2
B run new qemu-version-B (w/ new features 'feature1' and
'feature2'), but when B runs w/ compat '-machine pc-A', vm should
not see 'feature1' and 'feature2', so commandline append string to
cpu model '-cpu whatever -feature1 -feature2' to hidden new feature1
and feature2 to vm, hence vm can see same cpuid features (at B) as
those at A (which means, no feature1, no feature2)
=
If my understanding of your thoughts is right, I think currently
qemu has satisfied your target, code refer to
pc_cpus_init(cpu_model) .. cpu_init(cpu_model)
-- cpu_x86_register(*env, cpu_model)
-- cpu_x86_find_by_name(*def, cpu_model) // parse '+/-
features', generate feature masks plus_features... // and
minus_features...(this is feature exclusion masks you want)
I think your point 'define in the compat machine description' is
unnecessary.
The user would have to specify the new feature as exclusions
*manually* on the command line if -machine pc-A doesn't inject them
*automatically*. So it is necessary to enhance qemu in this regard.
... You suggest 'append a string of -feature1,-feature2 to the cpu model
that is specified on creation' at your last email.
Could you tell me other way user exclude features? I only know qemu command
line :-(
I was thinking of something like
diff --git a/hw/boards.h b/hw/boards.h
index 667177d..2bae071 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -28,6 +28,7 @@ typedef struct QEMUMachine {
int is_default;
const char *default_machine_opts;
GlobalProperty *compat_props;
+const char *compat_cpu_features;
struct QEMUMachine *next;
} QEMUMachine;
diff --git a/hw/pc.c b/hw/pc.c
index bb9867b..4d11559 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -949,8 +949,9 @@ static CPUState *pc_new_cpu(const char *cpu_model)
return env;
}
-void pc_cpus_init(const char *cpu_model)
+void pc_cpus_init(const char *cpu_model, const char *append_features)
{
+char *model_and_features;
int i;
/* init CPUs */
@@ -961,10 +962,13 @@ void pc_cpus_init(const char *cpu_model)
cpu_model = qemu32;
#endif
}
+model_and_features = g_strconcat(cpu_model, ,, append_features, NULL);
for(i = 0; i smp_cpus; i++) {
-pc_new_cpu(cpu_model);
+pc_new_cpu(model_and_features);
}
+
+g_free(model_and_features);
}
void pc_memory_init(MemoryRegion *system_memory,
However, getting machine.compat_cpu_features to pc_cpus_init is rather
ugly. And we will have CPU devices with real properties soon. Then the
compat feature string could be passed that way, without changing any
machine init function.
What if one cpudef had the wrong flags set but another cpudef was
correct, and we had to fix it on Qemu 1.1 for only one model? What if
the user _really_ wanted to edit the config file to add or remove a
given flag?
I think the best approach would be:
- Having versioned CPU model names;
- Specifying per-machine-type aliases.
See also the [libvirt] Modern CPU models cannot be used with libvirt
for related discussion.
The config file would look like:
[cpudef]
name = Westmere-1.0
features = [...] # no tsc-deadline
[...]
[cpudef]
name = Westmere-1.1
# so we don't have to copy everything from Westmere-1.0 manually:
base_cpudef = Westemere-1.0
# we could simply copy extend:
features = [...] tsc-deadline
# or, even better, if we had a append mechanism. e.g.:
#features_append = tsc-deadline
Then, on the machine-type table:
- Machine-types pc-1.0 and older would have:
.cpudef_aliases = {
{Westmere, Westmere-1.0},
[...]
}
- Machine-type pc-1.1
RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Jan Kiszka wrote:
On 2012-03-09 20:09, Liu, Jinsong wrote:
Jan Kiszka wrote:
On 2012-03-09 19:27, Liu, Jinsong wrote:
Jan Kiszka wrote:
On 2012-03-06 08:49, Liu, Jinsong wrote:
Jan,
Any comments? I feel some confused about your point 'disable
cpuid feature for older machine types by default': are you
planning a common approach for this common issue, or, you just
ask me a specific solution for the tsc deadline timer case?
I think a generic solution for this can be as simple as passing a
feature exclusion mask to cpu_init. You could simple append a
string of -feature1,-feature2 to the cpu model that is
specified on creation. And that string could be defined in the
compat machine descriptions. Does this make sense?
Jan, to prevent misunderstanding, I elaborate my understanding of
your points below (if any misunderstanding please point out to
me): = Your target is, to migrate from A(old
qemu) to B(new qemu) by
1. at A: qemu-version-A [-cpu whatever] // currently the
default machine type is pc-A
2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1
-feature2
B run new qemu-version-B (w/ new features 'feature1' and
'feature2'), but when B runs w/ compat '-machine pc-A', vm should
not see 'feature1' and 'feature2', so commandline append string to
cpu model '-cpu whatever -feature1 -feature2' to hidden new
feature1 and feature2 to vm, hence vm can see same cpuid features
(at B) as those at A (which means, no feature1, no feature2)
=
If my understanding of your thoughts is right, I think currently
qemu has satisfied your target, code refer to
pc_cpus_init(cpu_model) .. cpu_init(cpu_model)
-- cpu_x86_register(*env, cpu_model)
-- cpu_x86_find_by_name(*def, cpu_model) // parse '+/-
features', generate feature masks plus_features... // and
minus_features...(this is feature exclusion masks you want)
I think your point 'define in the compat machine description' is
unnecessary.
The user would have to specify the new feature as exclusions
*manually* on the command line if -machine pc-A doesn't inject them
*automatically*. So it is necessary to enhance qemu in this regard.
... You suggest 'append a string of -feature1,-feature2 to the cpu
model that is specified on creation' at your last email. Could you
tell me other way user exclude features? I only know qemu command
line :-(
I was thinking of something like
diff --git a/hw/boards.h b/hw/boards.h
index 667177d..2bae071 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -28,6 +28,7 @@ typedef struct QEMUMachine {
int is_default;
const char *default_machine_opts;
GlobalProperty *compat_props;
+const char *compat_cpu_features;
struct QEMUMachine *next;
} QEMUMachine;
diff --git a/hw/pc.c b/hw/pc.c
index bb9867b..4d11559 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -949,8 +949,9 @@ static CPUState *pc_new_cpu(const char *cpu_model)
return env;
}
-void pc_cpus_init(const char *cpu_model)
+void pc_cpus_init(const char *cpu_model, const char *append_features)
{
+char *model_and_features;
int i;
/* init CPUs */
@@ -961,10 +962,13 @@ void pc_cpus_init(const char *cpu_model)
cpu_model = qemu32;
#endif
}
+model_and_features = g_strconcat(cpu_model, ,,
append_features, NULL);
for(i = 0; i smp_cpus; i++) {
-pc_new_cpu(cpu_model);
+pc_new_cpu(model_and_features);
}
+
+g_free(model_and_features);
}
void pc_memory_init(MemoryRegion *system_memory,
However, getting machine.compat_cpu_features to pc_cpus_init is rather
ugly. And we will have CPU devices with real properties soon. Then the
compat feature string could be passed that way, without changing any
machine init function.
Andreas, do you expect CPU devices to be ready for qemu 1.1? We would
need them to pass a feature exclusion mask from machine.compat_props
to
the (x86) CPU init code.
cpu devices is just another format of current cpu_model. It helps nothing to
our problem.
Again, the point is, by what method the feature exclusion mask can be
generated, if user not give hint manually?
Thanks,
Jinsong
Well, given that introducing some intermediate solution for this would
be complex and hacky and that there is a way to configure tsc_deadline
for old machines away, though only an explicit one, I could live with
postponing the feature mask after the CPU device conversion. But the
last word will have the maintainers.
Jan
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Jan Kiszka wrote: On 2012-03-06 08:49, Liu, Jinsong wrote: Jan, Any comments? I feel some confused about your point 'disable cpuid feature for older machine types by default': are you planning a common approach for this common issue, or, you just ask me a specific solution for the tsc deadline timer case? I think a generic solution for this can be as simple as passing a feature exclusion mask to cpu_init. You could simple append a string of -feature1,-feature2 to the cpu model that is specified on creation. And that string could be defined in the compat machine descriptions. Does this make sense? Jan, to prevent misunderstanding, I elaborate my understanding of your points below (if any misunderstanding please point out to me): = Your target is, to migrate from A(old qemu) to B(new qemu) by 1. at A: qemu-version-A [-cpu whatever] // currently the default machine type is pc-A 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1 -feature2 B run new qemu-version-B (w/ new features 'feature1' and 'feature2'), but when B runs w/ compat '-machine pc-A', vm should not see 'feature1' and 'feature2', so commandline append string to cpu model '-cpu whatever -feature1 -feature2' to hidden new feature1 and feature2 to vm, hence vm can see same cpuid features (at B) as those at A (which means, no feature1, no feature2) = If my understanding of your thoughts is right, I think currently qemu has satisfied your target, code refer to pc_cpus_init(cpu_model) .. cpu_init(cpu_model) -- cpu_x86_register(*env, cpu_model) -- cpu_x86_find_by_name(*def, cpu_model) // parse '+/- features', generate feature masks plus_features... // and minus_features...(this is feature exclusion masks you want) I think your point 'define in the compat machine description' is unnecessary. As for 'tsc deadline' feature exposing, my patch (as attached) just obey qemu general cpuid exposing method, and also satisfied your target I think. Thanks, Jinsong 0001-Expose-tsc-deadline-timer-feature-to-guest.patch Description: 0001-Expose-tsc-deadline-timer-feature-to-guest.patch
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On 2012-03-09 19:27, Liu, Jinsong wrote: Jan Kiszka wrote: On 2012-03-06 08:49, Liu, Jinsong wrote: Jan, Any comments? I feel some confused about your point 'disable cpuid feature for older machine types by default': are you planning a common approach for this common issue, or, you just ask me a specific solution for the tsc deadline timer case? I think a generic solution for this can be as simple as passing a feature exclusion mask to cpu_init. You could simple append a string of -feature1,-feature2 to the cpu model that is specified on creation. And that string could be defined in the compat machine descriptions. Does this make sense? Jan, to prevent misunderstanding, I elaborate my understanding of your points below (if any misunderstanding please point out to me): = Your target is, to migrate from A(old qemu) to B(new qemu) by 1. at A: qemu-version-A [-cpu whatever] // currently the default machine type is pc-A 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1 -feature2 B run new qemu-version-B (w/ new features 'feature1' and 'feature2'), but when B runs w/ compat '-machine pc-A', vm should not see 'feature1' and 'feature2', so commandline append string to cpu model '-cpu whatever -feature1 -feature2' to hidden new feature1 and feature2 to vm, hence vm can see same cpuid features (at B) as those at A (which means, no feature1, no feature2) = If my understanding of your thoughts is right, I think currently qemu has satisfied your target, code refer to pc_cpus_init(cpu_model) .. cpu_init(cpu_model) -- cpu_x86_register(*env, cpu_model) -- cpu_x86_find_by_name(*def, cpu_model) // parse '+/- features', generate feature masks plus_features... // and minus_features...(this is feature exclusion masks you want) I think your point 'define in the compat machine description' is unnecessary. The user would have to specify the new feature as exclusions *manually* on the command line if -machine pc-A doesn't inject them *automatically*. So it is necessary to enhance qemu in this regard. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Jan Kiszka wrote: On 2012-03-09 19:27, Liu, Jinsong wrote: Jan Kiszka wrote: On 2012-03-06 08:49, Liu, Jinsong wrote: Jan, Any comments? I feel some confused about your point 'disable cpuid feature for older machine types by default': are you planning a common approach for this common issue, or, you just ask me a specific solution for the tsc deadline timer case? I think a generic solution for this can be as simple as passing a feature exclusion mask to cpu_init. You could simple append a string of -feature1,-feature2 to the cpu model that is specified on creation. And that string could be defined in the compat machine descriptions. Does this make sense? Jan, to prevent misunderstanding, I elaborate my understanding of your points below (if any misunderstanding please point out to me): = Your target is, to migrate from A(old qemu) to B(new qemu) by 1. at A: qemu-version-A [-cpu whatever] // currently the default machine type is pc-A 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1 -feature2 B run new qemu-version-B (w/ new features 'feature1' and 'feature2'), but when B runs w/ compat '-machine pc-A', vm should not see 'feature1' and 'feature2', so commandline append string to cpu model '-cpu whatever -feature1 -feature2' to hidden new feature1 and feature2 to vm, hence vm can see same cpuid features (at B) as those at A (which means, no feature1, no feature2) = If my understanding of your thoughts is right, I think currently qemu has satisfied your target, code refer to pc_cpus_init(cpu_model) .. cpu_init(cpu_model) -- cpu_x86_register(*env, cpu_model) -- cpu_x86_find_by_name(*def, cpu_model) // parse '+/- features', generate feature masks plus_features... // and minus_features...(this is feature exclusion masks you want) I think your point 'define in the compat machine description' is unnecessary. The user would have to specify the new feature as exclusions *manually* on the command line if -machine pc-A doesn't inject them *automatically*. So it is necessary to enhance qemu in this regard. ... You suggest 'append a string of -feature1,-feature2 to the cpu model that is specified on creation' at your last email. Could you tell me other way user exclude features? I only know qemu command line :-( Thanks, Jinsong-- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Liu, Jinsong wrote: Jan Kiszka wrote: On 2012-03-06 08:49, Liu, Jinsong wrote: Jan, Any comments? I feel some confused about your point 'disable cpuid feature for older machine types by default': are you planning a common approach for this common issue, or, you just ask me a specific solution for the tsc deadline timer case? I think a generic solution for this can be as simple as passing a feature exclusion mask to cpu_init. You could simple append a string of -feature1,-feature2 to the cpu model that is specified on creation. And that string could be defined in the compat machine descriptions. Does this make sense? Jan, to prevent misunderstanding, I elaborate my understanding of your points below (if any misunderstanding please point out to me): = Your target is, to migrate from A(old qemu) to B(new qemu) by 1. at A: qemu-version-A [-cpu whatever] // currently the default machine type is pc-A 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1 -feature2 B run new qemu-version-B (w/ new features 'feature1' and 'feature2'), but when B runs w/ compat '-machine pc-A', vm should not see 'feature1' and 'feature2', so commandline append string to cpu model '-cpu whatever -feature1 -feature2' to hidden new feature1 and feature2 to vm, hence vm can see same cpuid features (at B) as those at A (which means, no feature1, no feature2) = BTW, any misunderstanding or something wrong about my understanding of your target? please help me confirm. I want to make sure we are talking same thing. Thanks, Jinsong If my understanding of your thoughts is right, I think currently qemu has satisfied your target, code refer to pc_cpus_init(cpu_model) .. cpu_init(cpu_model) -- cpu_x86_register(*env, cpu_model) -- cpu_x86_find_by_name(*def, cpu_model) // parse '+/- features', generate feature masks plus_features... // and minus_features...(this is feature exclusion masks you want) I think your point 'define in the compat machine description' is unnecessary. As for 'tsc deadline' feature exposing, my patch (as attached) just obey qemu general cpuid exposing method, and also satisfied your target I think. Thanks, Jinsong -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On 2012-03-09 20:09, Liu, Jinsong wrote:
Jan Kiszka wrote:
On 2012-03-09 19:27, Liu, Jinsong wrote:
Jan Kiszka wrote:
On 2012-03-06 08:49, Liu, Jinsong wrote:
Jan,
Any comments? I feel some confused about your point 'disable cpuid
feature for older machine types by default': are you planning a
common approach for this common issue, or, you just ask me a
specific solution for the tsc deadline timer case?
I think a generic solution for this can be as simple as passing a
feature exclusion mask to cpu_init. You could simple append a string
of -feature1,-feature2 to the cpu model that is specified on
creation. And that string could be defined in the compat machine
descriptions. Does this make sense?
Jan, to prevent misunderstanding, I elaborate my understanding of
your points below (if any misunderstanding please point out to me):
=
Your target is, to migrate from A(old qemu) to B(new qemu) by
1. at A: qemu-version-A [-cpu whatever] // currently the
default machine type is pc-A
2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1
-feature2
B run new qemu-version-B (w/ new features 'feature1' and
'feature2'), but when B runs w/ compat '-machine pc-A', vm should
not see 'feature1' and 'feature2', so commandline append string to
cpu model '-cpu whatever -feature1 -feature2' to hidden new feature1
and feature2 to vm, hence vm can see same cpuid features (at B) as
those at A (which means, no feature1, no feature2)
=
If my understanding of your thoughts is right, I think currently
qemu has satisfied your target, code refer to
pc_cpus_init(cpu_model) .. cpu_init(cpu_model)
-- cpu_x86_register(*env, cpu_model)
-- cpu_x86_find_by_name(*def, cpu_model) // parse '+/-
features', generate feature masks plus_features... // and
minus_features...(this is feature exclusion masks you want)
I think your point 'define in the compat machine description' is
unnecessary.
The user would have to specify the new feature as exclusions
*manually* on the command line if -machine pc-A doesn't inject them
*automatically*. So it is necessary to enhance qemu in this regard.
... You suggest 'append a string of -feature1,-feature2 to the cpu model
that is specified on creation' at your last email.
Could you tell me other way user exclude features? I only know qemu command
line :-(
I was thinking of something like
diff --git a/hw/boards.h b/hw/boards.h
index 667177d..2bae071 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -28,6 +28,7 @@ typedef struct QEMUMachine {
int is_default;
const char *default_machine_opts;
GlobalProperty *compat_props;
+const char *compat_cpu_features;
struct QEMUMachine *next;
} QEMUMachine;
diff --git a/hw/pc.c b/hw/pc.c
index bb9867b..4d11559 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -949,8 +949,9 @@ static CPUState *pc_new_cpu(const char *cpu_model)
return env;
}
-void pc_cpus_init(const char *cpu_model)
+void pc_cpus_init(const char *cpu_model, const char *append_features)
{
+char *model_and_features;
int i;
/* init CPUs */
@@ -961,10 +962,13 @@ void pc_cpus_init(const char *cpu_model)
cpu_model = qemu32;
#endif
}
+model_and_features = g_strconcat(cpu_model, ,, append_features, NULL);
for(i = 0; i smp_cpus; i++) {
-pc_new_cpu(cpu_model);
+pc_new_cpu(model_and_features);
}
+
+g_free(model_and_features);
}
void pc_memory_init(MemoryRegion *system_memory,
However, getting machine.compat_cpu_features to pc_cpus_init is rather
ugly. And we will have CPU devices with real properties soon. Then the
compat feature string could be passed that way, without changing any
machine init function.
Andreas, do you expect CPU devices to be ready for qemu 1.1? We would
need them to pass a feature exclusion mask from machine.compat_props to
the (x86) CPU init code.
Well, given that introducing some intermediate solution for this would
be complex and hacky and that there is a way to configure tsc_deadline
for old machines away, though only an explicit one, I could live with
postponing the feature mask after the CPU device conversion. But the
last word will have the maintainers.
Jan
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am 09.03.2012 21:52, schrieb Jan Kiszka: Andreas, do you expect CPU devices to be ready for qemu 1.1? We would need them to pass a feature exclusion mask from machine.compat_props to the (x86) CPU init code. I was sure hoping to! Marcelo and Avi both going into vacation have let me down with the kvmclock patch that you wanted to go through uq/master. http://patchwork.ozlabs.org/patch/141721/ After it didn't make it into Avi's next PULL, I resubmitted it as part of the qom-user series. That's still on the list. Review and tags appreciated. http://patchwork.ozlabs.org/patch/144529/ Also on the list is a patch for a new object_class_get_list() function for sensible -cpu ? handling. http://patchwork.ozlabs.org/patch/143076/ I have been waiting for progress on these prerequisites before posting huge follow-on batches, but I can spam the list right away... The remaining issue was how to solve the struct CPU vs. CPU() issue - my solution on qom-cpu branch is to free the identifier CPUState in a quite invasive but half-scripted renaming. Based on struct CPUState, all targets are long successfully converted and keep needing to be rebased whenever someone moves code to a new microblaze file, adds a new arm board or in-kernel device using CPUState, etc. Since there's a configure change involved in *-user QOM support, virtually all files get rebuilt on each rebase and that takes quite some time, so getting that in soonish would be appreciated. http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-cpu Andreas - -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.18 (GNU/Linux) iQIcBAEBAgAGBQJPWqljAAoJEPou0S0+fgE/u+kQAInqv/OYlL2zU+hyeKXdJcZ4 RJ9Ne7J1+MOYcOI0p2T4lTImawHmZbF1HQyM9gTYv3EcGoSLm/SgYnHhFOulvPaE dbpuwk3KPHU+1ekvMVhEX89F62rXvMTz8JxO7hSmNAwIU2fIeKKWEAo0tED2tY16 OC2ExnE46+xLpandGJmfnx8vUJ1/PaO6fpd/FLOZbACT75zmdDxjDpF85e6NNsct ysMQ3kNMWcc0RfTU02dPgrhCPc5irEbalaX6BtOnquVQZDH4ypwzKHOZpMfoTF4u QWyxy+NaOVMVBHgHJeGek0wQ4nbihIhrQAn+Hg/h0P+NPuxQ8s7zX4xwqlYwhAHn cdDfN1sVbIvRWkGCXSRJR5eIkPKV+81Afm7/pL26eNlVtSrVV9UnsOJyfyDrtFs/ tSE90n8ZecjARGXUaACboZRalt7Pxl4EGWJwBhsNd5+3xaYvc9/WuTYP1FK1nltk KiBETQXnSY5aTh13erKLjbtYGR5YexXiLIOXvnBD0qQ/LDryHNjslgM8HcShrx4S U9LFAqEq78fgpCXZlzwtnzed341Wuerw3z4jELrSW8GH+R21jMfT9L8lwaHexabh 7YlulgdSpsmCxzhHOUCMeUTFFY6TlPxVk2NhEhQxlzDXdDmm6nd67R9qJIf7d42z CPzkIX+OpKBmorMQREIg =a+ZQ -END PGP SIGNATURE- -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
On 2012-03-06 08:49, Liu, Jinsong wrote: Jan, Any comments? I feel some confused about your point 'disable cpuid feature for older machine types by default': are you planning a common approach for this common issue, or, you just ask me a specific solution for the tsc deadline timer case? I think a generic solution for this can be as simple as passing a feature exclusion mask to cpu_init. You could simple append a string of -feature1,-feature2 to the cpu model that is specified on creation. And that string could be defined in the compat machine descriptions. Does this make sense? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Jan,
Any comments? I feel some confused about your point 'disable cpuid feature for
older machine types by default': are you planning a common approach for this
common issue, or, you just ask me a specific solution for the tsc deadline
timer case?
Thanks,
Jinsong
Liu, Jinsong wrote:
My point is that
qemu-version-A [-cpu whatever]
should provide the same VM as
qemu-version-B -machine pc-A [-cpu whatever]
specifically if you leave out the cpu specification.
So the compat machine could establish a feature mask (e.g. append
some -tsc_deadline in this case). But, indeed, we need a new
channel for this.
Yes, if such requirement need to be satisfied, I agree we need a new
channel to solve this kind of common issue.
As for tsc deadline timer feature exposing, I write an updated patch
as attached. 1). It exposes tsc deadline timer feature to guest if
in-kernel irqchip is used and kvm has emulated tsc deadline timer;
2). It also authorizes user to control the feature exposing via a cpu
feature flag;
Thanks,
Jinsong
From 5b7d5f459b621686e78e437010ce34748bcb9e8e Mon Sep 17 00:00:00 2001
From: Liu, Jinsong jinsong@intel.com
Date: Wed, 29 Feb 2012 01:53:15 +0800
Subject: [PATCH] Expose tsc deadline timer feature to guest
It exposes tsc deadline timer feature to guest if in-kernel irqchip
is used
and kvm has emulated tsc deadline timer.
It also authorizes user to control the feature exposing via a cpu
feature flag.
Signed-off-by: Liu, Jinsong jinsong@intel.com
---
target-i386/cpu.h |1 +
target-i386/cpuid.c |2 +-
target-i386/kvm.c |4
3 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index d92be5d..3409afe 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -399,6 +399,7 @@
#define CPUID_EXT_X2APIC (1 21)
#define CPUID_EXT_MOVBE(1 22)
#define CPUID_EXT_POPCNT (1 23)
+#define CPUID_EXT_TSC_DEADLINE_TIMER (1 24)
#define CPUID_EXT_XSAVE(1 26)
#define CPUID_EXT_OSXSAVE (1 27)
#define CPUID_EXT_HYPERVISOR (1 31)
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index b9bfeaf..ac4b79c 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -50,7 +50,7 @@ static const char *ext_feature_name[] = {
fma, cx16, xtpr, pdcm,
NULL, NULL, dca, sse4.1|sse4_1,
sse4.2|sse4_2, x2apic, movbe, popcnt,
-NULL, aes, xsave, osxsave,
+tsc_deadline, aes, xsave, osxsave,
avx, NULL, NULL, hypervisor,
};
static const char *ext2_feature_name[] = {
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7079e87..2639699 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -370,6 +370,10 @@ int kvm_arch_init_vcpu(CPUState *env)
i = env-cpuid_ext_features CPUID_EXT_HYPERVISOR;
env-cpuid_ext_features = kvm_arch_get_supported_cpuid(s, 1, 0,
R_ECX); env-cpuid_ext_features |= i;
+if (!kvm_irqchip_in_kernel() ||
+!kvm_check_extension(s, KVM_CAP_TSC_DEADLINE_TIMER)) {
+env-cpuid_ext_features = ~CPUID_EXT_TSC_DEADLINE_TIMER;
+}
env-cpuid_ext2_features = kvm_arch_get_supported_cpuid(s,
0x8001, 0, R_EDX);
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
