Messages in this thread | | | Date | Sat, 7 Sep 2019 14:26:10 -0700 | From | Ricardo Neri <> | Subject | Re: [PATCH] x86/umip: Add emulation for 64-bit processes |
| |
On Thu, Sep 05, 2019 at 04:22:21PM -0700, Brendan Shanks wrote: > Add emulation of the sgdt, sidt, and smsw instructions for 64-bit > processes. > > Wine users have encountered a number of 64-bit Windows games that use > these instructions (particularly sgdt), and were crashing when run on > UMIP-enabled systems.
Emulation support for 64-bit processes was not initially included because no use cases had been identified. Brendan has found one.
Here is the relevant e-mail thread: https://lkml.org/lkml/2017/1/26/12
FWIW,
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Only one minor comment below...
> > Originally-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > Signed-off-by: Brendan Shanks <bshanks@codeweavers.com> > --- > arch/x86/kernel/umip.c | 55 +++++++++++++++++++++++++----------------- > 1 file changed, 33 insertions(+), 22 deletions(-) > > diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c > index 5b345add550f..1812e95d2f55 100644 > --- a/arch/x86/kernel/umip.c > +++ b/arch/x86/kernel/umip.c > @@ -51,9 +51,7 @@ > * The instruction smsw is emulated to return the value that the register CR0 > * has at boot time as set in the head_32. > * > - * Also, emulation is provided only for 32-bit processes; 64-bit processes > - * that attempt to use the instructions that UMIP protects will receive the > - * SIGSEGV signal issued as a consequence of the general protection fault. > + * Emulation is provided for both 32-bit and 64-bit processes. > * > * Care is taken to appropriately emulate the results when segmentation is > * used. That is, rather than relying on USER_DS and USER_CS, the function > @@ -63,17 +61,18 @@ > * application uses a local descriptor table. > */ > > -#define UMIP_DUMMY_GDT_BASE 0xfffe0000 > -#define UMIP_DUMMY_IDT_BASE 0xffff0000 > +#define UMIP_DUMMY_GDT_BASE 0xfffffffffffe0000ULL > +#define UMIP_DUMMY_IDT_BASE 0xffffffffffff0000ULL > > /* > * The SGDT and SIDT instructions store the contents of the global descriptor > * table and interrupt table registers, respectively. The destination is a > * memory operand of X+2 bytes. X bytes are used to store the base address of > - * the table and 2 bytes are used to store the limit. In 32-bit processes, the > - * only processes for which emulation is provided, X has a value of 4. > + * the table and 2 bytes are used to store the limit. In 32-bit processes X > + * has a value of 4, in 64-bit processes X has a value of 8. > */ > -#define UMIP_GDT_IDT_BASE_SIZE 4 > +#define UMIP_GDT_IDT_BASE_SIZE_64BIT 8 > +#define UMIP_GDT_IDT_BASE_SIZE_32BIT 4 > #define UMIP_GDT_IDT_LIMIT_SIZE 2 > > #define UMIP_INST_SGDT 0 /* 0F 01 /0 */ > @@ -189,6 +188,7 @@ static int identify_insn(struct insn *insn) > * @umip_inst: A constant indicating the instruction to emulate > * @data: Buffer into which the dummy result is stored > * @data_size: Size of the emulated result > + * @x86_64: true if process is 64-bit, false otherwise > * > * Emulate an instruction protected by UMIP and provide a dummy result. The > * result of the emulation is saved in @data. The size of the results depends > @@ -202,11 +202,8 @@ static int identify_insn(struct insn *insn) > * 0 on success, -EINVAL on error while emulating. > */ > static int emulate_umip_insn(struct insn *insn, int umip_inst, > - unsigned char *data, int *data_size) > + unsigned char *data, int *data_size, bool x86_64) > { > - unsigned long dummy_base_addr, dummy_value; > - unsigned short dummy_limit = 0; > - > if (!data || !data_size || !insn) > return -EINVAL; > /* > @@ -219,6 +216,9 @@ static int emulate_umip_insn(struct insn *insn, int umip_inst, > * is always returned irrespective of the operand size. > */ > if (umip_inst == UMIP_INST_SGDT || umip_inst == UMIP_INST_SIDT) { > + u64 dummy_base_addr; > + u16 dummy_limit = 0; > + > /* SGDT and SIDT do not use registers operands. */ > if (X86_MODRM_MOD(insn->modrm.value) == 3) > return -EINVAL; > @@ -228,13 +228,24 @@ static int emulate_umip_insn(struct insn *insn, int umip_inst, > else > dummy_base_addr = UMIP_DUMMY_IDT_BASE; > > - *data_size = UMIP_GDT_IDT_LIMIT_SIZE + UMIP_GDT_IDT_BASE_SIZE;
Maybe a blank line here?
Thanks and BR, Ricardo
| |