x86/tdx: Port I/O emulation fixes
Subsystems (4)
Kiryl Shutsemau (Meta)
maintainercover x86/tdx: Port I/O emulation fixes
This series addresses two technical inaccuracies in the TDX guest port I/O emulation code reported by Borys Tsyrulnikov.
The first patch fixes an off-by-one error in the GENMASK() macro usage where the mask was being calculated as one bit too wide (e.g., 9 bits for an 8-bit operation).
The second patch ensures that 32-bit port I/O operations (INL) correctly zero-extend the result to the full 64-bit RAX register, as required by
the x86 architecture. Currently, the emulation preserves the upper 32
bits of RAX during such operations.
Both issues were introduced in the initial implementation of the runtime hypercalls for port I/O.
Kiryl Shutsemau (Meta) (2):
x86/tdx: Fix off-by-one in port I/O handling
x86/tdx: Fix zero-extension for 32-bit port I/O
arch/x86/coco/tdx/tdx.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
Signature · 1 line
2.51.2
Kiryl Shutsemau (Meta)
maintainerPATCH 1/2 x86/tdx: Fix off-by-one in port I/O handling
handle_in() and handle_out() in arch/x86/coco/tdx/tdx.c use:
u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
GENMASK(h, l) includes bit h. For size=1 (INB), this produces GENMASK(8, 0) = 0x1FF (9 bits) instead of GENMASK(7, 0) = 0xFF (8 bits). The mask is one bit too wide for all I/O sizes.
Fix the mask calculation.
--- arch/x86/coco/tdx/tdx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/coco/tdx/tdx.c +2 -2
@@ -693,7 +693,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port) .r13 = PORT_READ, .r14 = port, }; -u64 mask =GENMASK (BITS_PER_BYTE * size,0 ); +u64 mask =GENMASK (BITS_PER_BYTE * size -1 ,0 );bool success;/*
@@ -713,7 +713,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port)static bool handle_out (struct pt_regs *regs,int size,int port) { -u64 mask =GENMASK (BITS_PER_BYTE * size,0 ); +u64 mask =GENMASK (BITS_PER_BYTE * size -1 ,0 );/* * Emulate the I/O write via hypercall. More info about ABI can be found
Signature · 1 line
2.51.2
Re: [PATCH 1/2] x86/tdx: Fix off-by-one in port I/O handling
Hi Kirill,
Quoted 8 lines · Kiryl Shutsemau (Meta)
handle_in()andhandle_out()inarch/x86/coco/tdx/tdx.cuse:u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
GENMASK(h, l)includes bit h. For size=1 (INB), this producesGENMASK(8, 0)=0x1FF(9 bits) instead ofGENMASK(7, 0)=0xFF(8 bits). The mask is one bit too wide for all I/O sizes.
Quoted 7 lines
Fix the mask calculation.
Fixes 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")Reported-by Borys Tsyrulnikov <[email protected]>Signed-off-by Kiryl Shutsemau (Meta) <[email protected]>---
LGTM. Can you include a link to the bug report or related discussion in the commit log? It will help understand the impact of this issue.
Quoted 25 lines
arch/x86/coco/tdx/tdx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)arch/x86/coco/tdx/tdx.c +2
-2@@ -693,7 +693,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port) .r13 = PORT_READ, .r14 = port, }; -u64 mask =GENMASK (BITS_PER_BYTE * size,0 ); +u64 mask =GENMASK (BITS_PER_BYTE * size -1 ,0 );bool success;/* @@ -713,7 +713,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port)static bool handle_out (struct pt_regs *regs,int size,int port) { -u64 mask =GENMASK (BITS_PER_BYTE * size,0 ); +u64 mask =GENMASK (BITS_PER_BYTE * size -1 ,0 );/* * Emulate the I/O write via hypercall. More info about ABI can be found
Signature · 2 lines
Sathyanarayanan Kuppuswamy Linux Kernel Developer
Kiryl Shutsemau
maintainerRe: [PATCH 1/2] x86/tdx: Fix off-by-one in port I/O handling
Quoted 13 lines · Kuppuswamy Sathyanarayanan
Hi Kirill,
Quoted 10 lines · Kiryl Shutsemau (Meta)
handle_in()andhandle_out()inarch/x86/coco/tdx/tdx.cuse:u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
GENMASK(h, l)includes bit h. For size=1 (INB), this producesGENMASK(8, 0)=0x1FF(9 bits) instead ofGENMASK(7, 0)=0xFF(8 bits). The mask is one bit too wide for all I/O sizes.Fix the mask calculation.
Quoted 8 lines
Quoted 5 lines
Fixes 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")Reported-by Borys Tsyrulnikov <[email protected]>Signed-off-by Kiryl Shutsemau (Meta) <[email protected]>---
LGTM. Can you include a link to the bug report or related discussion in the commit log? It will help understand the impact of this issue.
It is relevant for both.
Signature · 1 line
Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH 1/2] x86/tdx: Fix off-by-one in port I/O handling
In reply to Kiryl Shutsemau (Meta)Quoted 8 lines · Kiryl Shutsemau (Meta)
handle_in()andhandle_out()inarch/x86/coco/tdx/tdx.cuse:u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
GENMASK(h, l)includes bit h. For size=1 (INB), this producesGENMASK(8, 0)=0x1FF(9 bits) instead ofGENMASK(7, 0)=0xFF(8 bits). The mask is one bit too wide for all I/O sizes.
Quoted 6 lines
Fix the mask calculation.
Fixes 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")Reported-by Borys Tsyrulnikov <[email protected]>Signed-off-by Kiryl Shutsemau (Meta) <[email protected]>
Kiryl Shutsemau (Meta)
maintainerPATCH 2/2 x86/tdx: Fix zero-extension for 32-bit port I/O
In reply to Kiryl Shutsemau (Meta)According to x86 architecture rules, 32-bit operations zero-extend the result to 64 bits. The current implementation of handle_in() only masks the lower 32 bits, which preserves the upper 32 bits of RAX when a 32-bit port IN instruction is emulated.
Update handle_in() to zero out the entire RAX register when the I/O size is 4 bytes to ensure correct zero-extension. For smaller sizes (1 or 2 bytes), continue to preserve the unaffected upper bits.
--- arch/x86/coco/tdx/tdx.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
arch/x86/coco/tdx/tdx.c +11 -2
@@ -703,8 +703,17 @@ static bool handle_in(struct pt_regs *regs, int size, int port)*/ success = !__tdx_hypercall (&args); -/* Update part of the register affected by the emulated instruction */ - regs->ax &= ~mask; +/* +* Update part of the register affected by the emulated instruction. +* +* 32-bit operands generate a 32-bit result, zero-extended to a 64-bit +* result. +*/ +if (size <4 ) + regs->ax &= ~mask; +else + regs->ax =0 ; +if (success) regs->ax |= args.r11 & mask;
Signature · 1 line
2.51.2
Re: [PATCH 2/2] x86/tdx: Fix zero-extension for 32-bit port I/O
Hi Kiril,
Quoted 9 lines · Kiryl Shutsemau (Meta)
According to x86 architecture rules, 32-bit operations zero-extend the result to 64 bits. The current implementation of
handle_in()only masks the lower 32 bits, which preserves the upper 32 bits of RAX when a 32-bit port IN instruction is emulated.Update
handle_in()to zero out the entire RAX register when the I/O size is 4 bytes to ensure correct zero-extension. For smaller sizes (1 or 2 bytes), continue to preserve the unaffected upper bits.
Quoted 5 lines
Fixes 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")Reported-by Borys Tsyrulnikov <[email protected]>Signed-off-by Kiryl Shutsemau (Meta) <[email protected]>---
If you have bug or discussion link, please include it.
Quoted 13 lines
arch/x86/coco/tdx/tdx.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)arch/x86/coco/tdx/tdx.c +0
-2@@ -703,8 +703,17 @@ static bool handle_in(struct pt_regs *regs, int size, int port)*/ success = !__tdx_hypercall (&args); -/* Update part of the register affected by the emulated instruction */ - regs->ax &= ~mask;
arch/x86/coco/tdx/tdx.c static bool handle_in(struct pt_regs *regs, int size, int port) · line 715
Quoted 10 lines
arch/x86/coco/tdx/tdx.c +10
-0+/* +* Update part of the register affected by the emulated instruction. +* +* 32-bit operands generate a 32-bit result, zero-extended to a 64-bit +* result. +*/ +if (size <4 ) + regs->ax &= ~mask; +else + regs->ax =0 ;
The logic would be more readable as:
if (size == 4) regs->ax = 0; else regs->ax &= ~mask;
Quoted 4 lines
arch/x86/coco/tdx/tdx.c +1
-0+if (success) regs->ax |= args.r11 & mask;
Signature · 2 lines
Sathyanarayanan Kuppuswamy Linux Kernel Developer
Re: [PATCH 2/2] x86/tdx: Fix zero-extension for 32-bit port I/O
In reply to Kiryl Shutsemau (Meta)Quoted 3 lines · Kiryl Shutsemau (Meta)
According to x86 architecture rules, 32-bit operations zero-extend the result to 64 bits.
FWIW, the relevant part in the SDM seems to be:
Chapter 3.4.1.1 General-Purpose Registers in 64-Bit Mode
...
* 32-bit operands generate a 32-bit result, zero-extended to a 64 bit result in the destination general-purpose register.
Quoted 4 lines
The current implementation of
handle_in()only masks the lower 32 bits, which preserves the upper 32 bits of RAX when a 32-bit port IN instruction is emulated.
Quoted 8 lines
Update
handle_in()to zero out the entire RAX register when the I/O size is 4 bytes to ensure correct zero-extension. For smaller sizes (1 or 2 bytes), continue to preserve the unaffected upper bits.Fixes 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")Reported-by Borys Tsyrulnikov <[email protected]>Signed-off-by Kiryl Shutsemau (Meta) <[email protected]>
Quoted 28 lines
--- arch/x86/coco/tdx/tdx.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)arch/x86/coco/tdx/tdx.c +11
-2@@ -703,8 +703,17 @@ static bool handle_in(struct pt_regs *regs, int size, int port)*/ success = !__tdx_hypercall (&args); -/* Update part of the register affected by the emulated instruction */ - regs->ax &= ~mask; +/* +* Update part of the register affected by the emulated instruction. +* +* 32-bit operands generate a 32-bit result, zero-extended to a 64-bit +* result. +*/ +if (size <4 ) + regs->ax &= ~mask; +else + regs->ax =0 ; +if (success) regs->ax |= args.r11 & mask;