diff mbox series

x86/mwait-idle: fix ubsan warning

Message ID f0ac3890b5e2e1e98bfd3fe5fffcf3c3c031e12c.1704388276.git.tamas.lengyel@intel.com (mailing list archive)
State New, archived
Headers show
Series x86/mwait-idle: fix ubsan warning | expand

Commit Message

Tamas K Lengyel Jan. 4, 2024, 5:13 p.m. UTC
Fix warning:
(XEN) UBSAN: Undefined behaviour in arch/x86/cpu/mwait-idle.c:1300:44
(XEN) left shift of 15 by 28 places cannot be represented in type 'int'

Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
Fixes: 5a211704e88 ("mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled")
---
 xen/arch/x86/include/asm/mwait.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andrew Cooper Jan. 4, 2024, 5:26 p.m. UTC | #1
On 04/01/2024 5:13 pm, Tamas K Lengyel wrote:
> Fix warning:
> (XEN) UBSAN: Undefined behaviour in arch/x86/cpu/mwait-idle.c:1300:44
> (XEN) left shift of 15 by 28 places cannot be represented in type 'int'
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
> Fixes: 5a211704e88 ("mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled")

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

It's perhaps worth saying this is in sklh_idle_state_table_update()
which is why it only manifests on a single CPU.  Happy to adjust on commit.

All other uses of this constant shift right first, and then mask, hence
why they don't trip UBSAN.

~Andrew
Jan Beulich Jan. 5, 2024, 7:33 a.m. UTC | #2
On 04.01.2024 18:13, Tamas K Lengyel wrote:
> Fix warning:
> (XEN) UBSAN: Undefined behaviour in arch/x86/cpu/mwait-idle.c:1300:44
> (XEN) left shift of 15 by 28 places cannot be represented in type 'int'
> 
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
> Fixes: 5a211704e88 ("mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled")

No matter that I appreciate the change, I think this wants fixing by a
patch to the (Linux) original, which we'd then import (like we do for
other changes, including the one referenced by the Fixes: tag).

Jan
Tamas K Lengyel Jan. 5, 2024, 4:09 p.m. UTC | #3
On Fri, Jan 5, 2024 at 2:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 04.01.2024 18:13, Tamas K Lengyel wrote:
> > Fix warning:
> > (XEN) UBSAN: Undefined behaviour in arch/x86/cpu/mwait-idle.c:1300:44
> > (XEN) left shift of 15 by 28 places cannot be represented in type 'int'
> >
> > Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
> > Fixes: 5a211704e88 ("mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled")
>
> No matter that I appreciate the change, I think this wants fixing by a
> patch to the (Linux) original, which we'd then import (like we do for
> other changes, including the one referenced by the Fixes: tag).

Feel free to submit it to other projects if the same issue applies to
them. I only ran into this with Xen and can only test it with Xen.

Tamas
Andrew Cooper Jan. 5, 2024, 7:38 p.m. UTC | #4
On 05/01/2024 4:09 pm, Tamas K Lengyel wrote:
> On Fri, Jan 5, 2024 at 2:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 04.01.2024 18:13, Tamas K Lengyel wrote:
>>> Fix warning:
>>> (XEN) UBSAN: Undefined behaviour in arch/x86/cpu/mwait-idle.c:1300:44
>>> (XEN) left shift of 15 by 28 places cannot be represented in type 'int'
>>>
>>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
>>> Fixes: 5a211704e88 ("mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled")
>> No matter that I appreciate the change, I think this wants fixing by a
>> patch to the (Linux) original, which we'd then import (like we do for
>> other changes, including the one referenced by the Fixes: tag).
> Feel free to submit it to other projects if the same issue applies to
> them. I only ran into this with Xen and can only test it with Xen.

Linux is affected by this, but a fix to Linux won't apply to Xen because
Xen already diverged from Linux in this function.

~Andrew
diff mbox series

Patch

diff --git a/xen/arch/x86/include/asm/mwait.h b/xen/arch/x86/include/asm/mwait.h
index f377d9f..9298f98 100644
--- a/xen/arch/x86/include/asm/mwait.h
+++ b/xen/arch/x86/include/asm/mwait.h
@@ -4,7 +4,7 @@ 
 #include <xen/types.h>
 
 #define MWAIT_SUBSTATE_MASK		0xf
-#define MWAIT_CSTATE_MASK		0xf
+#define MWAIT_CSTATE_MASK		0xfU
 #define MWAIT_SUBSTATE_SIZE		4
 
 #define CPUID_MWAIT_LEAF		5