Re: [Xen-devel] [PATCH] x86/vmx: fix vmx_is_singlestep_supported return value
On 28.08.15 at 04:01, kevin.t...@intel.com wrote:
From: Jan Beulich [mailto:jbeul...@suse.com]
Sent: Tuesday, August 25, 2015 3:59 PM
Tamas K Lengyel tamas.leng...@zentific.com 08/25/15 1:51 AM
@@ -1768,7 +1768,7 @@ static void vmx_enable_msr_exit_interception(struct
domain *d)
static bool_t vmx_is_singlestep_supported(void)
{
-return cpu_has_monitor_trap_flag;
+return cpu_has_monitor_trap_flag ? 1 : 0;
Prevailing style would tend towards !!cpu_has_monitor_trap_flag
Yeap, you are right. If the maintainers prefer I can resend with that style.
This could easily be adjusted upon commit. What I'm wondering is whether this
is the right place to fix it: Wouldn't it be better for the cpu_has_* macros
themselves to be adjusted so other (future) users won't fall into the same
trap
(vmx_virtual_intr_delivery_enabled() is a good second example bogusly using
int as its return type, and once adjusted to bool_t it would break)?
I'm OK with original patch. Above example can be taken care when changing
return type.
Okay, I'll commit it the way it is, despite not being convinced of the
approach of waiting for the next one to fall into the same trap.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vmx: fix vmx_is_singlestep_supported return value
From: Tamas K Lengyel [mailto:ta...@tklengyel.com] Sent: Tuesday, August 25, 2015 3:56 AM The function supposed to return a boolean but instead it returned the value 0x800 which is the Intel internal flag for MTF. This has caused various checks using this function to falsely report no MTF capability. Signed-off-by: Tamas K Lengyel tleng...@novetta.com Acked-by: Kevin Tian kevin.t...@intel.com Thanks, Kevin ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vmx: fix vmx_is_singlestep_supported return value
From: Jan Beulich [mailto:jbeul...@suse.com]
Sent: Tuesday, August 25, 2015 3:59 PM
Tamas K Lengyel tamas.leng...@zentific.com 08/25/15 1:51 AM
@@ -1768,7 +1768,7 @@ static void vmx_enable_msr_exit_interception(struct
domain *d)
static bool_t vmx_is_singlestep_supported(void)
{
-return cpu_has_monitor_trap_flag;
+return cpu_has_monitor_trap_flag ? 1 : 0;
Prevailing style would tend towards !!cpu_has_monitor_trap_flag
Yeap, you are right. If the maintainers prefer I can resend with that style.
This could easily be adjusted upon commit. What I'm wondering is whether this
is the right place to fix it: Wouldn't it be better for the cpu_has_* macros
themselves to be adjusted so other (future) users won't fall into the same
trap
(vmx_virtual_intr_delivery_enabled() is a good second example bogusly using
int as its return type, and once adjusted to bool_t it would break)?
I'm OK with original patch. Above example can be taken care when changing
return type.
Thanks
Kevin
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vmx: fix vmx_is_singlestep_supported return value
On Tue, Aug 25, 2015 at 01:58:42AM -0600, Jan Beulich wrote: [...] (vmx_virtual_intr_delivery_enabled() is a good second example bogusly using int as its return type, and once adjusted to bool_t it would break)? VMX maintainers, I think Jan is waiting for reply from you for that particular question. We need reply / confirmation from you to get a fix for this issue. Wei. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vmx: fix vmx_is_singlestep_supported return value
Tamas K Lengyel tamas.leng...@zentific.com 08/25/15 1:51 AM
@@ -1768,7 +1768,7 @@ static void vmx_enable_msr_exit_interception(struct
domain *d)
static bool_t vmx_is_singlestep_supported(void)
{
-return cpu_has_monitor_trap_flag;
+return cpu_has_monitor_trap_flag ? 1 : 0;
Prevailing style would tend towards !!cpu_has_monitor_trap_flag
Yeap, you are right. If the maintainers prefer I can resend with that style.
This could easily be adjusted upon commit. What I'm wondering is whether this
is the right place to fix it: Wouldn't it be better for the cpu_has_* macros
themselves to be adjusted so other (future) users won't fall into the same trap
(vmx_virtual_intr_delivery_enabled() is a good second example bogusly using
int as its return type, and once adjusted to bool_t it would break)?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vmx: fix vmx_is_singlestep_supported return value
On Mon, Aug 24, 2015 at 03:55:33PM -0400, Tamas K Lengyel wrote:
The function supposed to return a boolean but instead it returned
the value 0x800 which is the Intel internal flag for MTF. This has
caused various checks using this function to falsely report no MTF
capability.
Signed-off-by: Tamas K Lengyel tleng...@novetta.com
Release-acked-by: Wei Liu wei.l...@citrix.com
---
xen/arch/x86/hvm/vmx/vmx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 999defe..35bcd79 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1768,7 +1768,7 @@ static void vmx_enable_msr_exit_interception(struct
domain *d)
static bool_t vmx_is_singlestep_supported(void)
{
-return cpu_has_monitor_trap_flag;
+return cpu_has_monitor_trap_flag ? 1 : 0;
}
static void vmx_vcpu_update_eptp(struct vcpu *v)
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86/vmx: fix vmx_is_singlestep_supported return value
The function supposed to return a boolean but instead it returned
the value 0x800 which is the Intel internal flag for MTF. This has
caused various checks using this function to falsely report no MTF
capability.
Signed-off-by: Tamas K Lengyel tleng...@novetta.com
---
xen/arch/x86/hvm/vmx/vmx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 999defe..35bcd79 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1768,7 +1768,7 @@ static void vmx_enable_msr_exit_interception(struct
domain *d)
static bool_t vmx_is_singlestep_supported(void)
{
-return cpu_has_monitor_trap_flag;
+return cpu_has_monitor_trap_flag ? 1 : 0;
}
static void vmx_vcpu_update_eptp(struct vcpu *v)
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vmx: fix vmx_is_singlestep_supported return value
On 24/08/2015 20:55, Tamas K Lengyel wrote:
The function supposed to return a boolean but instead it returned
the value 0x800 which is the Intel internal flag for MTF. This has
caused various checks using this function to falsely report no MTF
capability.
Ouch. Given than bool_t is current signed char, that won't be of much use.
Signed-off-by: Tamas K Lengyel tleng...@novetta.com
---
xen/arch/x86/hvm/vmx/vmx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 999defe..35bcd79 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1768,7 +1768,7 @@ static void vmx_enable_msr_exit_interception(struct
domain *d)
static bool_t vmx_is_singlestep_supported(void)
{
-return cpu_has_monitor_trap_flag;
+return cpu_has_monitor_trap_flag ? 1 : 0;
Prevailing style would tend towards !!cpu_has_monitor_trap_flag
Either way, Reviewed-by: Andrew Cooper andrew.coop...@citrix.com
}
static void vmx_vcpu_update_eptp(struct vcpu *v)
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vmx: fix vmx_is_singlestep_supported return value
@@ -1768,7 +1768,7 @@ static void vmx_enable_msr_exit_interception(struct
domain *d)
static bool_t vmx_is_singlestep_supported(void)
{
-return cpu_has_monitor_trap_flag;
+return cpu_has_monitor_trap_flag ? 1 : 0;
Prevailing style would tend towards !!cpu_has_monitor_trap_flag
Yeap, you are right. If the maintainers prefer I can resend with that style.
Either way, Reviewed-by: Andrew Cooper andrew.coop...@citrix.com
Thanks!
Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
