Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[Ricardo Neri]

On Thu, 2017-01-26 at 09:05 -0800, Andy Lutomirski wrote:
> On Wed, Jan 25, 2017 at 9:50 PM, Ricardo Neri
>  wrote:
> > On Wed, 2017-01-25 at 13:58 -0800, Andy Lutomirski wrote:
> >> On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri
> >>  wrote:
> >> > Tasks running in virtual-8086 mode will use 16-bit addressing form
> >> > encodings as described in the Intel 64 and IA-32 Architecture Software
> >> > Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings
> >> > differ in several ways from the 32-bit/64-bit addressing form encodings:
> >> > the r/m part of the ModRM byte points to different registers and, in some
> >> > cases, addresses can be indicated by the addition of the value of two
> >> > registers. Also, there is no support for SiB bytes. Thus, a separate
> >> > function is needed to parse this form of addressing.
> >> >
> >> > Furthermore, virtual-8086 mode tasks will use real-mode addressing. This
> >> > implies that the segment selectors do not point to a segment descriptor
> >> > but are used to compute logical addresses. Hence, there is a need to
> >> > add support to compute addresses using the segment selectors. If segment-
> >> > override prefixes are present in the instructions, they take precedence.
> >> >
> >> > Lastly, it is important to note that when a tasks is running in virtual-
> >> > 8086 mode and an interrupt/exception occurs, the CPU pushes to the stack
> >> > the segment selectors for ds, es, fs and gs. These are accesible via the
> >> > struct kernel_vm86_regs rather than pt_regs.
> >> >
> >> > Code for 16-bit addressing encodings is likely to be used only by 
> >> > virtual-
> >> > 8086 mode tasks. Thus, this code is wrapped to be built only if the
> >> > option CONFIG_VM86 is selected.
> >>
> >> That's not true.  It's used in 16-bit protected mode, too.  And there
> >> are (ugh!) six possibilities:
> >
> > Thanks for the clarification. I will enable the decoding of addresses
> > for 16-bit as well... and test the emulation code.
> >>
> >>  - Normal 32-bit protected mode.  This should already work.
> >>  - Normal 64-bit protected mode.  This should also already work.  (I
> >> forget whether a 16-bit SS is either illegal or has no effect in this
> >> case.)
> >
> > For these two cases I am just taking the effective address that the user
> > space application provides, given that the segment selectors were set
> > beforehand (and with a base of 0).
> 
> What do you mean by the base being zero?  User code can set a nonzero
> DS base if it wants.  In 64-bit mode (user_64bit_mode(regs)), the base
> is ignored unless there's an fs or gs prefix, and in 32-bit mode the
> base is never ignored.

Yes, I take this back. At the time of writing I was thinking about the
__USER_CS and _USER_DS descriptors. You ar right, the base is not
ignored.
> 
> >
> >>  - Virtual 8086 mode
> >
> > In this case I calculate the linear address as:
> >  (segment_select << 4) + effective address.
> >
> >>  - Normal 16-bit protected mode, used by DOSEMU and Wine.  (16-bit CS,
> >> 16-bit address segment)
> >>  - 16-bit CS, 32-bit address segment.  IIRC this might be used by some
> >> 32-bit DOS programs to call BIOS.
> >>  - 32-bit CS, 16-bit address segment.  I don't know whether anything uses 
> >> this.
> >
> > In all these protected modes, are you referring to the size in bits of
> > the base address of in the descriptor selected in the CS register? In
> > such a case I would need to get the base address and add it to the
> > effective address given in the operands of the instructions, right?
> 
> No, I'm referring to the D/B bit.  I'm a bit fuzzy on exactly how the
> instruction encoding works, but I think that 16-bit x86 code is
> encoded just like real mode code except that the selectors are used
> for real.

I see. I have the logic to differentiate between 16-bit and 32-bit
addresses. What I am missing is code to look at the value of this bit
and any potential operand overrides. I will work on adding this.
> 
> >> size, but I suspect you'll need to handle 16-bit CS.
> >
> > Unless I am missing what is special with the 16-bit base address, I only
> > would need to add that base address to whatever effective address (aka,
> > offset) is encoded in the ModRM and displacement bytes.
> 
> Exactly.  (And make sure the instruction decoder can decode 16-bit
> instructions correctly.)

Will do. I have tested my 16-bit decoding extensively. I think it will
work.


Thanks and BR,
Ricardo

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[Ricardo Neri]

On Thu, 2017-01-26 at 09:05 -0800, Andy Lutomirski wrote:
> On Wed, Jan 25, 2017 at 9:50 PM, Ricardo Neri
>  wrote:
> > On Wed, 2017-01-25 at 13:58 -0800, Andy Lutomirski wrote:
> >> On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri
> >>  wrote:
> >> > Tasks running in virtual-8086 mode will use 16-bit addressing form
> >> > encodings as described in the Intel 64 and IA-32 Architecture Software
> >> > Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings
> >> > differ in several ways from the 32-bit/64-bit addressing form encodings:
> >> > the r/m part of the ModRM byte points to different registers and, in some
> >> > cases, addresses can be indicated by the addition of the value of two
> >> > registers. Also, there is no support for SiB bytes. Thus, a separate
> >> > function is needed to parse this form of addressing.
> >> >
> >> > Furthermore, virtual-8086 mode tasks will use real-mode addressing. This
> >> > implies that the segment selectors do not point to a segment descriptor
> >> > but are used to compute logical addresses. Hence, there is a need to
> >> > add support to compute addresses using the segment selectors. If segment-
> >> > override prefixes are present in the instructions, they take precedence.
> >> >
> >> > Lastly, it is important to note that when a tasks is running in virtual-
> >> > 8086 mode and an interrupt/exception occurs, the CPU pushes to the stack
> >> > the segment selectors for ds, es, fs and gs. These are accesible via the
> >> > struct kernel_vm86_regs rather than pt_regs.
> >> >
> >> > Code for 16-bit addressing encodings is likely to be used only by 
> >> > virtual-
> >> > 8086 mode tasks. Thus, this code is wrapped to be built only if the
> >> > option CONFIG_VM86 is selected.
> >>
> >> That's not true.  It's used in 16-bit protected mode, too.  And there
> >> are (ugh!) six possibilities:
> >
> > Thanks for the clarification. I will enable the decoding of addresses
> > for 16-bit as well... and test the emulation code.
> >>
> >>  - Normal 32-bit protected mode.  This should already work.
> >>  - Normal 64-bit protected mode.  This should also already work.  (I
> >> forget whether a 16-bit SS is either illegal or has no effect in this
> >> case.)
> >
> > For these two cases I am just taking the effective address that the user
> > space application provides, given that the segment selectors were set
> > beforehand (and with a base of 0).
> 
> What do you mean by the base being zero?  User code can set a nonzero
> DS base if it wants.  In 64-bit mode (user_64bit_mode(regs)), the base
> is ignored unless there's an fs or gs prefix, and in 32-bit mode the
> base is never ignored.

Yes, I take this back. At the time of writing I was thinking about the
__USER_CS and _USER_DS descriptors. You ar right, the base is not
ignored.
> 
> >
> >>  - Virtual 8086 mode
> >
> > In this case I calculate the linear address as:
> >  (segment_select << 4) + effective address.
> >
> >>  - Normal 16-bit protected mode, used by DOSEMU and Wine.  (16-bit CS,
> >> 16-bit address segment)
> >>  - 16-bit CS, 32-bit address segment.  IIRC this might be used by some
> >> 32-bit DOS programs to call BIOS.
> >>  - 32-bit CS, 16-bit address segment.  I don't know whether anything uses 
> >> this.
> >
> > In all these protected modes, are you referring to the size in bits of
> > the base address of in the descriptor selected in the CS register? In
> > such a case I would need to get the base address and add it to the
> > effective address given in the operands of the instructions, right?
> 
> No, I'm referring to the D/B bit.  I'm a bit fuzzy on exactly how the
> instruction encoding works, but I think that 16-bit x86 code is
> encoded just like real mode code except that the selectors are used
> for real.

I see. I have the logic to differentiate between 16-bit and 32-bit
addresses. What I am missing is code to look at the value of this bit
and any potential operand overrides. I will work on adding this.
> 
> >> size, but I suspect you'll need to handle 16-bit CS.
> >
> > Unless I am missing what is special with the 16-bit base address, I only
> > would need to add that base address to whatever effective address (aka,
> > offset) is encoded in the ModRM and displacement bytes.
> 
> Exactly.  (And make sure the instruction decoder can decode 16-bit
> instructions correctly.)

Will do. I have tested my 16-bit decoding extensively. I think it will
work.


Thanks and BR,
Ricardo

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings (fwd)

[Ricardo Neri]

On Wed, 2017-01-25 at 22:49 +0100, Julia Lawall wrote:
> Check the type of seg on line 267.

Ah! I missed that. It makes sense. I will fix this issue.

Thanks and BR,
Ricardo
> 
> julia
> 
> -- Forwarded message --
> Date: Thu, 26 Jan 2017 05:24:40 +0800
> From: kbuild test robot <fengguang...@intel.com>
> To: kbu...@01.org
> Cc: Julia Lawall <julia.law...@lip6.fr>
> Subject: Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit
> addressing encodings
> 
> In-Reply-To: <20170125202353.101059-6-ricardo.neri-calde...@linux.intel.com>
> 
> Hi Ricardo,
> 
> [auto build test WARNING on tip/auto-latest]
> [also build test WARNING on v4.10-rc5 next-20170125]
> [cannot apply to tip/x86/core]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Ricardo-Neri/x86-Enable-User-Mode-Instruction-Prevention/20170126-043345
> :: branch date: 51 minutes ago
> :: commit date: 51 minutes ago
> 
> >> arch/x86/lib/insn-kernel.c:267:6-9: WARNING: Unsigned expression compared 
> >> with zero: seg < 0
> 
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 574de0de238ef30c816985006f02f7a1dbba92aa
> vim +267 arch/x86/lib/insn-kernel.c
> 
> 574de0de Ricardo Neri 2017-01-25  251 addr = (seg << 4) + 
> regs_get_register(regs, addr_offset1);
> 574de0de Ricardo Neri 2017-01-25  252 } else {
> 574de0de Ricardo Neri 2017-01-25  253 ret = 
> get_reg_offset_16(insn, regs, _offset1,
> 574de0de Ricardo Neri 2017-01-25  254 
> _offset2);
> 574de0de Ricardo Neri 2017-01-25  255 if (ret < 0)
> 574de0de Ricardo Neri 2017-01-25  256 goto out_err;
> 574de0de Ricardo Neri 2017-01-25  257 /*
> 574de0de Ricardo Neri 2017-01-25  258  * Don't fail on 
> invalid offset values. They might be invalid
> 574de0de Ricardo Neri 2017-01-25  259  * because they are not 
> supported. Instead, use them in the
> 574de0de Ricardo Neri 2017-01-25  260  * calculation only if 
> they contain a valid value.
> 574de0de Ricardo Neri 2017-01-25  261  */
> 574de0de Ricardo Neri 2017-01-25  262 if (addr_offset1 >= 0)
> 574de0de Ricardo Neri 2017-01-25  263 addr1 = 
> regs_get_register(regs, addr_offset1);
> 574de0de Ricardo Neri 2017-01-25  264 if (addr_offset2 >= 0)
> 574de0de Ricardo Neri 2017-01-25  265 addr2 = 
> regs_get_register(regs, addr_offset2);
> 574de0de Ricardo Neri 2017-01-25  266 seg = 
> __get_segment_selector_16(regs, insn, addr_offset1);
> 574de0de Ricardo Neri 2017-01-25 @267 if (seg < 0)
> 574de0de Ricardo Neri 2017-01-25  268 goto out_err;
> 574de0de Ricardo Neri 2017-01-25  269 addr =  (seg << 4) + 
> addr1 + addr2;
> 574de0de Ricardo Neri 2017-01-25  270 }
> 574de0de Ricardo Neri 2017-01-25  271 addr += 
> insn->displacement.value;
> 574de0de Ricardo Neri 2017-01-25  272
> 574de0de Ricardo Neri 2017-01-25  273 return (void __user *)addr;
> 574de0de Ricardo Neri 2017-01-25  274  out_err:
> 574de0de Ricardo Neri 2017-01-25  275 return (void __user *)-1;
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings (fwd)

[Ricardo Neri]

On Wed, 2017-01-25 at 22:49 +0100, Julia Lawall wrote:
> Check the type of seg on line 267.

Ah! I missed that. It makes sense. I will fix this issue.

Thanks and BR,
Ricardo
> 
> julia
> 
> -- Forwarded message --
> Date: Thu, 26 Jan 2017 05:24:40 +0800
> From: kbuild test robot 
> To: kbu...@01.org
> Cc: Julia Lawall 
> Subject: Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit
> addressing encodings
> 
> In-Reply-To: <20170125202353.101059-6-ricardo.neri-calde...@linux.intel.com>
> 
> Hi Ricardo,
> 
> [auto build test WARNING on tip/auto-latest]
> [also build test WARNING on v4.10-rc5 next-20170125]
> [cannot apply to tip/x86/core]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Ricardo-Neri/x86-Enable-User-Mode-Instruction-Prevention/20170126-043345
> :: branch date: 51 minutes ago
> :: commit date: 51 minutes ago
> 
> >> arch/x86/lib/insn-kernel.c:267:6-9: WARNING: Unsigned expression compared 
> >> with zero: seg < 0
> 
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 574de0de238ef30c816985006f02f7a1dbba92aa
> vim +267 arch/x86/lib/insn-kernel.c
> 
> 574de0de Ricardo Neri 2017-01-25  251 addr = (seg << 4) + 
> regs_get_register(regs, addr_offset1);
> 574de0de Ricardo Neri 2017-01-25  252 } else {
> 574de0de Ricardo Neri 2017-01-25  253 ret = 
> get_reg_offset_16(insn, regs, _offset1,
> 574de0de Ricardo Neri 2017-01-25  254 
> _offset2);
> 574de0de Ricardo Neri 2017-01-25  255 if (ret < 0)
> 574de0de Ricardo Neri 2017-01-25  256 goto out_err;
> 574de0de Ricardo Neri 2017-01-25  257 /*
> 574de0de Ricardo Neri 2017-01-25  258  * Don't fail on 
> invalid offset values. They might be invalid
> 574de0de Ricardo Neri 2017-01-25  259  * because they are not 
> supported. Instead, use them in the
> 574de0de Ricardo Neri 2017-01-25  260  * calculation only if 
> they contain a valid value.
> 574de0de Ricardo Neri 2017-01-25  261  */
> 574de0de Ricardo Neri 2017-01-25  262 if (addr_offset1 >= 0)
> 574de0de Ricardo Neri 2017-01-25  263 addr1 = 
> regs_get_register(regs, addr_offset1);
> 574de0de Ricardo Neri 2017-01-25  264 if (addr_offset2 >= 0)
> 574de0de Ricardo Neri 2017-01-25  265 addr2 = 
> regs_get_register(regs, addr_offset2);
> 574de0de Ricardo Neri 2017-01-25  266 seg = 
> __get_segment_selector_16(regs, insn, addr_offset1);
> 574de0de Ricardo Neri 2017-01-25 @267 if (seg < 0)
> 574de0de Ricardo Neri 2017-01-25  268 goto out_err;
> 574de0de Ricardo Neri 2017-01-25  269 addr =  (seg << 4) + 
> addr1 + addr2;
> 574de0de Ricardo Neri 2017-01-25  270 }
> 574de0de Ricardo Neri 2017-01-25  271 addr += 
> insn->displacement.value;
> 574de0de Ricardo Neri 2017-01-25  272
> 574de0de Ricardo Neri 2017-01-25  273 return (void __user *)addr;
> 574de0de Ricardo Neri 2017-01-25  274  out_err:
> 574de0de Ricardo Neri 2017-01-25  275 return (void __user *)-1;
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[Andy Lutomirski]

On Wed, Jan 25, 2017 at 9:50 PM, Ricardo Neri
 wrote:
> On Wed, 2017-01-25 at 13:58 -0800, Andy Lutomirski wrote:
>> On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri
>>  wrote:
>> > Tasks running in virtual-8086 mode will use 16-bit addressing form
>> > encodings as described in the Intel 64 and IA-32 Architecture Software
>> > Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings
>> > differ in several ways from the 32-bit/64-bit addressing form encodings:
>> > the r/m part of the ModRM byte points to different registers and, in some
>> > cases, addresses can be indicated by the addition of the value of two
>> > registers. Also, there is no support for SiB bytes. Thus, a separate
>> > function is needed to parse this form of addressing.
>> >
>> > Furthermore, virtual-8086 mode tasks will use real-mode addressing. This
>> > implies that the segment selectors do not point to a segment descriptor
>> > but are used to compute logical addresses. Hence, there is a need to
>> > add support to compute addresses using the segment selectors. If segment-
>> > override prefixes are present in the instructions, they take precedence.
>> >
>> > Lastly, it is important to note that when a tasks is running in virtual-
>> > 8086 mode and an interrupt/exception occurs, the CPU pushes to the stack
>> > the segment selectors for ds, es, fs and gs. These are accesible via the
>> > struct kernel_vm86_regs rather than pt_regs.
>> >
>> > Code for 16-bit addressing encodings is likely to be used only by virtual-
>> > 8086 mode tasks. Thus, this code is wrapped to be built only if the
>> > option CONFIG_VM86 is selected.
>>
>> That's not true.  It's used in 16-bit protected mode, too.  And there
>> are (ugh!) six possibilities:
>
> Thanks for the clarification. I will enable the decoding of addresses
> for 16-bit as well... and test the emulation code.
>>
>>  - Normal 32-bit protected mode.  This should already work.
>>  - Normal 64-bit protected mode.  This should also already work.  (I
>> forget whether a 16-bit SS is either illegal or has no effect in this
>> case.)
>
> For these two cases I am just taking the effective address that the user
> space application provides, given that the segment selectors were set
> beforehand (and with a base of 0).

What do you mean by the base being zero?  User code can set a nonzero
DS base if it wants.  In 64-bit mode (user_64bit_mode(regs)), the base
is ignored unless there's an fs or gs prefix, and in 32-bit mode the
base is never ignored.

>
>>  - Virtual 8086 mode
>
> In this case I calculate the linear address as:
>  (segment_select << 4) + effective address.
>
>>  - Normal 16-bit protected mode, used by DOSEMU and Wine.  (16-bit CS,
>> 16-bit address segment)
>>  - 16-bit CS, 32-bit address segment.  IIRC this might be used by some
>> 32-bit DOS programs to call BIOS.
>>  - 32-bit CS, 16-bit address segment.  I don't know whether anything uses 
>> this.
>
> In all these protected modes, are you referring to the size in bits of
> the base address of in the descriptor selected in the CS register? In
> such a case I would need to get the base address and add it to the
> effective address given in the operands of the instructions, right?

No, I'm referring to the D/B bit.  I'm a bit fuzzy on exactly how the
instruction encoding works, but I think that 16-bit x86 code is
encoded just like real mode code except that the selectors are used
for real.

>> size, but I suspect you'll need to handle 16-bit CS.
>
> Unless I am missing what is special with the 16-bit base address, I only
> would need to add that base address to whatever effective address (aka,
> offset) is encoded in the ModRM and displacement bytes.

Exactly.  (And make sure the instruction decoder can decode 16-bit
instructions correctly.)

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[Andy Lutomirski]

On Wed, Jan 25, 2017 at 9:50 PM, Ricardo Neri
 wrote:
> On Wed, 2017-01-25 at 13:58 -0800, Andy Lutomirski wrote:
>> On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri
>>  wrote:
>> > Tasks running in virtual-8086 mode will use 16-bit addressing form
>> > encodings as described in the Intel 64 and IA-32 Architecture Software
>> > Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings
>> > differ in several ways from the 32-bit/64-bit addressing form encodings:
>> > the r/m part of the ModRM byte points to different registers and, in some
>> > cases, addresses can be indicated by the addition of the value of two
>> > registers. Also, there is no support for SiB bytes. Thus, a separate
>> > function is needed to parse this form of addressing.
>> >
>> > Furthermore, virtual-8086 mode tasks will use real-mode addressing. This
>> > implies that the segment selectors do not point to a segment descriptor
>> > but are used to compute logical addresses. Hence, there is a need to
>> > add support to compute addresses using the segment selectors. If segment-
>> > override prefixes are present in the instructions, they take precedence.
>> >
>> > Lastly, it is important to note that when a tasks is running in virtual-
>> > 8086 mode and an interrupt/exception occurs, the CPU pushes to the stack
>> > the segment selectors for ds, es, fs and gs. These are accesible via the
>> > struct kernel_vm86_regs rather than pt_regs.
>> >
>> > Code for 16-bit addressing encodings is likely to be used only by virtual-
>> > 8086 mode tasks. Thus, this code is wrapped to be built only if the
>> > option CONFIG_VM86 is selected.
>>
>> That's not true.  It's used in 16-bit protected mode, too.  And there
>> are (ugh!) six possibilities:
>
> Thanks for the clarification. I will enable the decoding of addresses
> for 16-bit as well... and test the emulation code.
>>
>>  - Normal 32-bit protected mode.  This should already work.
>>  - Normal 64-bit protected mode.  This should also already work.  (I
>> forget whether a 16-bit SS is either illegal or has no effect in this
>> case.)
>
> For these two cases I am just taking the effective address that the user
> space application provides, given that the segment selectors were set
> beforehand (and with a base of 0).

What do you mean by the base being zero?  User code can set a nonzero
DS base if it wants.  In 64-bit mode (user_64bit_mode(regs)), the base
is ignored unless there's an fs or gs prefix, and in 32-bit mode the
base is never ignored.

>
>>  - Virtual 8086 mode
>
> In this case I calculate the linear address as:
>  (segment_select << 4) + effective address.
>
>>  - Normal 16-bit protected mode, used by DOSEMU and Wine.  (16-bit CS,
>> 16-bit address segment)
>>  - 16-bit CS, 32-bit address segment.  IIRC this might be used by some
>> 32-bit DOS programs to call BIOS.
>>  - 32-bit CS, 16-bit address segment.  I don't know whether anything uses 
>> this.
>
> In all these protected modes, are you referring to the size in bits of
> the base address of in the descriptor selected in the CS register? In
> such a case I would need to get the base address and add it to the
> effective address given in the operands of the instructions, right?

No, I'm referring to the D/B bit.  I'm a bit fuzzy on exactly how the
instruction encoding works, but I think that 16-bit x86 code is
encoded just like real mode code except that the selectors are used
for real.

>> size, but I suspect you'll need to handle 16-bit CS.
>
> Unless I am missing what is special with the 16-bit base address, I only
> would need to add that base address to whatever effective address (aka,
> offset) is encoded in the ModRM and displacement bytes.

Exactly.  (And make sure the instruction decoder can decode 16-bit
instructions correctly.)

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[Ricardo Neri]

On Wed, 2017-01-25 at 13:58 -0800, Andy Lutomirski wrote:
> On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri
>  wrote:
> > Tasks running in virtual-8086 mode will use 16-bit addressing form
> > encodings as described in the Intel 64 and IA-32 Architecture Software
> > Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings
> > differ in several ways from the 32-bit/64-bit addressing form encodings:
> > the r/m part of the ModRM byte points to different registers and, in some
> > cases, addresses can be indicated by the addition of the value of two
> > registers. Also, there is no support for SiB bytes. Thus, a separate
> > function is needed to parse this form of addressing.
> >
> > Furthermore, virtual-8086 mode tasks will use real-mode addressing. This
> > implies that the segment selectors do not point to a segment descriptor
> > but are used to compute logical addresses. Hence, there is a need to
> > add support to compute addresses using the segment selectors. If segment-
> > override prefixes are present in the instructions, they take precedence.
> >
> > Lastly, it is important to note that when a tasks is running in virtual-
> > 8086 mode and an interrupt/exception occurs, the CPU pushes to the stack
> > the segment selectors for ds, es, fs and gs. These are accesible via the
> > struct kernel_vm86_regs rather than pt_regs.
> >
> > Code for 16-bit addressing encodings is likely to be used only by virtual-
> > 8086 mode tasks. Thus, this code is wrapped to be built only if the
> > option CONFIG_VM86 is selected.
> 
> That's not true.  It's used in 16-bit protected mode, too.  And there
> are (ugh!) six possibilities:

Thanks for the clarification. I will enable the decoding of addresses
for 16-bit as well... and test the emulation code. 
> 
>  - Normal 32-bit protected mode.  This should already work.
>  - Normal 64-bit protected mode.  This should also already work.  (I
> forget whether a 16-bit SS is either illegal or has no effect in this
> case.)

For these two cases I am just taking the effective address that the user
space application provides, given that the segment selectors were set
beforehand (and with a base of 0).

copy_to/from_user(kernel_buf, effective_address, sizeof(kernel_buf));

>  - Virtual 8086 mode

In this case I calculate the linear address as:
 (segment_select << 4) + effective address.

>  - Normal 16-bit protected mode, used by DOSEMU and Wine.  (16-bit CS,
> 16-bit address segment)
>  - 16-bit CS, 32-bit address segment.  IIRC this might be used by some
> 32-bit DOS programs to call BIOS.
>  - 32-bit CS, 16-bit address segment.  I don't know whether anything uses 
> this.

In all these protected modes, are you referring to the size in bits of
the base address of in the descriptor selected in the CS register? In
such a case I would need to get the base address and add it to the
effective address given in the operands of the instructions, right?
> 
> I don't know if anything you're doing cares about SS's, DS's, etc.

For 16-bit addressing encodings, I use DS for the BX, DI and SI
registers, and no register at all. I use SS for BP and SP. This can be
overridden by the segment selector prefixes. For 32/64-bit, I need to
implement segment selector awareness.

> size, but I suspect you'll need to handle 16-bit CS.

Unless I am missing what is special with the 16-bit base address, I only
would need to add that base address to whatever effective address (aka,
offset) is encoded in the ModRM and displacement bytes.

At this moment my implementation is able to decode the ModRM, SiB and
displacement bytes for 16-bit and 32/64-bit address encodings.

Thanks and BR,
Ricardo

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[Ricardo Neri]

On Wed, 2017-01-25 at 13:58 -0800, Andy Lutomirski wrote:
> On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri
>  wrote:
> > Tasks running in virtual-8086 mode will use 16-bit addressing form
> > encodings as described in the Intel 64 and IA-32 Architecture Software
> > Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings
> > differ in several ways from the 32-bit/64-bit addressing form encodings:
> > the r/m part of the ModRM byte points to different registers and, in some
> > cases, addresses can be indicated by the addition of the value of two
> > registers. Also, there is no support for SiB bytes. Thus, a separate
> > function is needed to parse this form of addressing.
> >
> > Furthermore, virtual-8086 mode tasks will use real-mode addressing. This
> > implies that the segment selectors do not point to a segment descriptor
> > but are used to compute logical addresses. Hence, there is a need to
> > add support to compute addresses using the segment selectors. If segment-
> > override prefixes are present in the instructions, they take precedence.
> >
> > Lastly, it is important to note that when a tasks is running in virtual-
> > 8086 mode and an interrupt/exception occurs, the CPU pushes to the stack
> > the segment selectors for ds, es, fs and gs. These are accesible via the
> > struct kernel_vm86_regs rather than pt_regs.
> >
> > Code for 16-bit addressing encodings is likely to be used only by virtual-
> > 8086 mode tasks. Thus, this code is wrapped to be built only if the
> > option CONFIG_VM86 is selected.
> 
> That's not true.  It's used in 16-bit protected mode, too.  And there
> are (ugh!) six possibilities:

Thanks for the clarification. I will enable the decoding of addresses
for 16-bit as well... and test the emulation code. 
> 
>  - Normal 32-bit protected mode.  This should already work.
>  - Normal 64-bit protected mode.  This should also already work.  (I
> forget whether a 16-bit SS is either illegal or has no effect in this
> case.)

For these two cases I am just taking the effective address that the user
space application provides, given that the segment selectors were set
beforehand (and with a base of 0).

copy_to/from_user(kernel_buf, effective_address, sizeof(kernel_buf));

>  - Virtual 8086 mode

In this case I calculate the linear address as:
 (segment_select << 4) + effective address.

>  - Normal 16-bit protected mode, used by DOSEMU and Wine.  (16-bit CS,
> 16-bit address segment)
>  - 16-bit CS, 32-bit address segment.  IIRC this might be used by some
> 32-bit DOS programs to call BIOS.
>  - 32-bit CS, 16-bit address segment.  I don't know whether anything uses 
> this.

In all these protected modes, are you referring to the size in bits of
the base address of in the descriptor selected in the CS register? In
such a case I would need to get the base address and add it to the
effective address given in the operands of the instructions, right?
> 
> I don't know if anything you're doing cares about SS's, DS's, etc.

For 16-bit addressing encodings, I use DS for the BX, DI and SI
registers, and no register at all. I use SS for BP and SP. This can be
overridden by the segment selector prefixes. For 32/64-bit, I need to
implement segment selector awareness.

> size, but I suspect you'll need to handle 16-bit CS.

Unless I am missing what is special with the 16-bit base address, I only
would need to add that base address to whatever effective address (aka,
offset) is encoded in the ModRM and displacement bytes.

At this moment my implementation is able to decode the ModRM, SiB and
displacement bytes for 16-bit and 32/64-bit address encodings.

Thanks and BR,
Ricardo

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[H. Peter Anvin]

Buchbinder ,Colin Ian King 
,Lorenzo Stoakes ,Qiaowei Ren 
,Arnaldo Carvalho de Melo ,Adrian 
Hunter ,Kees Cook ,Thomas 
Garnier ,Dmitry Vyukov 
From: h...@zytor.com
Message-ID: <18e8698f-6c60-4b98-ae73-c371184c5...@zytor.com>

On January 25, 2017 1:58:27 PM PST, Andy Lutomirski  wrote:
>On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri
> wrote:
>> Tasks running in virtual-8086 mode will use 16-bit addressing form
>> encodings as described in the Intel 64 and IA-32 Architecture
>Software
>> Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing
>encodings
>> differ in several ways from the 32-bit/64-bit addressing form
>encodings:
>> the r/m part of the ModRM byte points to different registers and, in
>some
>> cases, addresses can be indicated by the addition of the value of two
>> registers. Also, there is no support for SiB bytes. Thus, a separate
>> function is needed to parse this form of addressing.
>>
>> Furthermore, virtual-8086 mode tasks will use real-mode addressing.
>This
>> implies that the segment selectors do not point to a segment
>descriptor
>> but are used to compute logical addresses. Hence, there is a need to
>> add support to compute addresses using the segment selectors. If
>segment-
>> override prefixes are present in the instructions, they take
>precedence.
>>
>> Lastly, it is important to note that when a tasks is running in
>virtual-
>> 8086 mode and an interrupt/exception occurs, the CPU pushes to the
>stack
>> the segment selectors for ds, es, fs and gs. These are accesible via
>the
>> struct kernel_vm86_regs rather than pt_regs.
>>
>> Code for 16-bit addressing encodings is likely to be used only by
>virtual-
>> 8086 mode tasks. Thus, this code is wrapped to be built only if the
>> option CONFIG_VM86 is selected.
>
>That's not true.  It's used in 16-bit protected mode, too.  And there
>are (ugh!) six possibilities:
>
> - Normal 32-bit protected mode.  This should already work.
> - Normal 64-bit protected mode.  This should also already work.  (I
>forget whether a 16-bit SS is either illegal or has no effect in this
>case.)
> - Virtual 8086 mode
> - Normal 16-bit protected mode, used by DOSEMU and Wine.  (16-bit CS,
>16-bit address segment)
> - 16-bit CS, 32-bit address segment.  IIRC this might be used by some
>32-bit DOS programs to call BIOS.
>- 32-bit CS, 16-bit address segment.  I don't know whether anything
>uses this.
>
>I don't know if anything you're doing cares about SS's, DS's, etc.
>size, but I suspect you'll need to handle 16-bit CS.

Only the CS bitness matters for the purpose of addressing modes; the SS bitness 
(which has no effect in 64-bit mode) only matters for implicit stack references 
unless I'm completely out to sea.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[H. Peter Anvin]

Buchbinder ,Colin Ian King 
,Lorenzo Stoakes ,Qiaowei Ren 
,Arnaldo Carvalho de Melo ,Adrian 
Hunter ,Kees Cook ,Thomas 
Garnier ,Dmitry Vyukov 
From: h...@zytor.com
Message-ID: <18e8698f-6c60-4b98-ae73-c371184c5...@zytor.com>

On January 25, 2017 1:58:27 PM PST, Andy Lutomirski  wrote:
>On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri
> wrote:
>> Tasks running in virtual-8086 mode will use 16-bit addressing form
>> encodings as described in the Intel 64 and IA-32 Architecture
>Software
>> Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing
>encodings
>> differ in several ways from the 32-bit/64-bit addressing form
>encodings:
>> the r/m part of the ModRM byte points to different registers and, in
>some
>> cases, addresses can be indicated by the addition of the value of two
>> registers. Also, there is no support for SiB bytes. Thus, a separate
>> function is needed to parse this form of addressing.
>>
>> Furthermore, virtual-8086 mode tasks will use real-mode addressing.
>This
>> implies that the segment selectors do not point to a segment
>descriptor
>> but are used to compute logical addresses. Hence, there is a need to
>> add support to compute addresses using the segment selectors. If
>segment-
>> override prefixes are present in the instructions, they take
>precedence.
>>
>> Lastly, it is important to note that when a tasks is running in
>virtual-
>> 8086 mode and an interrupt/exception occurs, the CPU pushes to the
>stack
>> the segment selectors for ds, es, fs and gs. These are accesible via
>the
>> struct kernel_vm86_regs rather than pt_regs.
>>
>> Code for 16-bit addressing encodings is likely to be used only by
>virtual-
>> 8086 mode tasks. Thus, this code is wrapped to be built only if the
>> option CONFIG_VM86 is selected.
>
>That's not true.  It's used in 16-bit protected mode, too.  And there
>are (ugh!) six possibilities:
>
> - Normal 32-bit protected mode.  This should already work.
> - Normal 64-bit protected mode.  This should also already work.  (I
>forget whether a 16-bit SS is either illegal or has no effect in this
>case.)
> - Virtual 8086 mode
> - Normal 16-bit protected mode, used by DOSEMU and Wine.  (16-bit CS,
>16-bit address segment)
> - 16-bit CS, 32-bit address segment.  IIRC this might be used by some
>32-bit DOS programs to call BIOS.
>- 32-bit CS, 16-bit address segment.  I don't know whether anything
>uses this.
>
>I don't know if anything you're doing cares about SS's, DS's, etc.
>size, but I suspect you'll need to handle 16-bit CS.

Only the CS bitness matters for the purpose of addressing modes; the SS bitness 
(which has no effect in 64-bit mode) only matters for implicit stack references 
unless I'm completely out to sea.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[Andy Lutomirski]

On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri
 wrote:
> Tasks running in virtual-8086 mode will use 16-bit addressing form
> encodings as described in the Intel 64 and IA-32 Architecture Software
> Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings
> differ in several ways from the 32-bit/64-bit addressing form encodings:
> the r/m part of the ModRM byte points to different registers and, in some
> cases, addresses can be indicated by the addition of the value of two
> registers. Also, there is no support for SiB bytes. Thus, a separate
> function is needed to parse this form of addressing.
>
> Furthermore, virtual-8086 mode tasks will use real-mode addressing. This
> implies that the segment selectors do not point to a segment descriptor
> but are used to compute logical addresses. Hence, there is a need to
> add support to compute addresses using the segment selectors. If segment-
> override prefixes are present in the instructions, they take precedence.
>
> Lastly, it is important to note that when a tasks is running in virtual-
> 8086 mode and an interrupt/exception occurs, the CPU pushes to the stack
> the segment selectors for ds, es, fs and gs. These are accesible via the
> struct kernel_vm86_regs rather than pt_regs.
>
> Code for 16-bit addressing encodings is likely to be used only by virtual-
> 8086 mode tasks. Thus, this code is wrapped to be built only if the
> option CONFIG_VM86 is selected.

That's not true.  It's used in 16-bit protected mode, too.  And there
are (ugh!) six possibilities:

 - Normal 32-bit protected mode.  This should already work.
 - Normal 64-bit protected mode.  This should also already work.  (I
forget whether a 16-bit SS is either illegal or has no effect in this
case.)
 - Virtual 8086 mode
 - Normal 16-bit protected mode, used by DOSEMU and Wine.  (16-bit CS,
16-bit address segment)
 - 16-bit CS, 32-bit address segment.  IIRC this might be used by some
32-bit DOS programs to call BIOS.
 - 32-bit CS, 16-bit address segment.  I don't know whether anything uses this.

I don't know if anything you're doing cares about SS's, DS's, etc.
size, but I suspect you'll need to handle 16-bit CS.

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[Andy Lutomirski]

On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri
 wrote:
> Tasks running in virtual-8086 mode will use 16-bit addressing form
> encodings as described in the Intel 64 and IA-32 Architecture Software
> Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings
> differ in several ways from the 32-bit/64-bit addressing form encodings:
> the r/m part of the ModRM byte points to different registers and, in some
> cases, addresses can be indicated by the addition of the value of two
> registers. Also, there is no support for SiB bytes. Thus, a separate
> function is needed to parse this form of addressing.
>
> Furthermore, virtual-8086 mode tasks will use real-mode addressing. This
> implies that the segment selectors do not point to a segment descriptor
> but are used to compute logical addresses. Hence, there is a need to
> add support to compute addresses using the segment selectors. If segment-
> override prefixes are present in the instructions, they take precedence.
>
> Lastly, it is important to note that when a tasks is running in virtual-
> 8086 mode and an interrupt/exception occurs, the CPU pushes to the stack
> the segment selectors for ds, es, fs and gs. These are accesible via the
> struct kernel_vm86_regs rather than pt_regs.
>
> Code for 16-bit addressing encodings is likely to be used only by virtual-
> 8086 mode tasks. Thus, this code is wrapped to be built only if the
> option CONFIG_VM86 is selected.

That's not true.  It's used in 16-bit protected mode, too.  And there
are (ugh!) six possibilities:

 - Normal 32-bit protected mode.  This should already work.
 - Normal 64-bit protected mode.  This should also already work.  (I
forget whether a 16-bit SS is either illegal or has no effect in this
case.)
 - Virtual 8086 mode
 - Normal 16-bit protected mode, used by DOSEMU and Wine.  (16-bit CS,
16-bit address segment)
 - 16-bit CS, 32-bit address segment.  IIRC this might be used by some
32-bit DOS programs to call BIOS.
 - 32-bit CS, 16-bit address segment.  I don't know whether anything uses this.

I don't know if anything you're doing cares about SS's, DS's, etc.
size, but I suspect you'll need to handle 16-bit CS.

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings (fwd)

[Julia Lawall]

Check the type of seg on line 267.

julia

-- Forwarded message --
Date: Thu, 26 Jan 2017 05:24:40 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit
    addressing encodings

In-Reply-To: <20170125202353.101059-6-ricardo.neri-calde...@linux.intel.com>

Hi Ricardo,

[auto build test WARNING on tip/auto-latest]
[also build test WARNING on v4.10-rc5 next-20170125]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Ricardo-Neri/x86-Enable-User-Mode-Instruction-Prevention/20170126-043345
:: branch date: 51 minutes ago
:: commit date: 51 minutes ago

>> arch/x86/lib/insn-kernel.c:267:6-9: WARNING: Unsigned expression compared 
>> with zero: seg < 0

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 574de0de238ef30c816985006f02f7a1dbba92aa
vim +267 arch/x86/lib/insn-kernel.c

574de0de Ricardo Neri 2017-01-25  251   addr = (seg << 4) + 
regs_get_register(regs, addr_offset1);
574de0de Ricardo Neri 2017-01-25  252   } else {
574de0de Ricardo Neri 2017-01-25  253   ret = get_reg_offset_16(insn, 
regs, _offset1,
574de0de Ricardo Neri 2017-01-25  254   
_offset2);
574de0de Ricardo Neri 2017-01-25  255   if (ret < 0)
574de0de Ricardo Neri 2017-01-25  256   goto out_err;
574de0de Ricardo Neri 2017-01-25  257   /*
574de0de Ricardo Neri 2017-01-25  258* Don't fail on invalid offset 
values. They might be invalid
574de0de Ricardo Neri 2017-01-25  259* because they are not 
supported. Instead, use them in the
574de0de Ricardo Neri 2017-01-25  260* calculation only if they 
contain a valid value.
574de0de Ricardo Neri 2017-01-25  261*/
574de0de Ricardo Neri 2017-01-25  262   if (addr_offset1 >= 0)
574de0de Ricardo Neri 2017-01-25  263   addr1 = 
regs_get_register(regs, addr_offset1);
574de0de Ricardo Neri 2017-01-25  264   if (addr_offset2 >= 0)
574de0de Ricardo Neri 2017-01-25  265   addr2 = 
regs_get_register(regs, addr_offset2);
574de0de Ricardo Neri 2017-01-25  266   seg = 
__get_segment_selector_16(regs, insn, addr_offset1);
574de0de Ricardo Neri 2017-01-25 @267   if (seg < 0)
574de0de Ricardo Neri 2017-01-25  268   goto out_err;
574de0de Ricardo Neri 2017-01-25  269   addr =  (seg << 4) + addr1 + 
addr2;
574de0de Ricardo Neri 2017-01-25  270   }
574de0de Ricardo Neri 2017-01-25  271   addr += insn->displacement.value;
574de0de Ricardo Neri 2017-01-25  272
574de0de Ricardo Neri 2017-01-25  273   return (void __user *)addr;
574de0de Ricardo Neri 2017-01-25  274  out_err:
574de0de Ricardo Neri 2017-01-25  275   return (void __user *)-1;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings (fwd)

[Julia Lawall]

Check the type of seg on line 267.

julia

-- Forwarded message --
Date: Thu, 26 Jan 2017 05:24:40 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit
addressing encodings

In-Reply-To: <20170125202353.101059-6-ricardo.neri-calde...@linux.intel.com>

Hi Ricardo,

[auto build test WARNING on tip/auto-latest]
[also build test WARNING on v4.10-rc5 next-20170125]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Ricardo-Neri/x86-Enable-User-Mode-Instruction-Prevention/20170126-043345
:: branch date: 51 minutes ago
:: commit date: 51 minutes ago

>> arch/x86/lib/insn-kernel.c:267:6-9: WARNING: Unsigned expression compared 
>> with zero: seg < 0

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 574de0de238ef30c816985006f02f7a1dbba92aa
vim +267 arch/x86/lib/insn-kernel.c

574de0de Ricardo Neri 2017-01-25  251   addr = (seg << 4) + 
regs_get_register(regs, addr_offset1);
574de0de Ricardo Neri 2017-01-25  252   } else {
574de0de Ricardo Neri 2017-01-25  253   ret = get_reg_offset_16(insn, 
regs, _offset1,
574de0de Ricardo Neri 2017-01-25  254   
_offset2);
574de0de Ricardo Neri 2017-01-25  255   if (ret < 0)
574de0de Ricardo Neri 2017-01-25  256   goto out_err;
574de0de Ricardo Neri 2017-01-25  257   /*
574de0de Ricardo Neri 2017-01-25  258* Don't fail on invalid offset 
values. They might be invalid
574de0de Ricardo Neri 2017-01-25  259* because they are not 
supported. Instead, use them in the
574de0de Ricardo Neri 2017-01-25  260* calculation only if they 
contain a valid value.
574de0de Ricardo Neri 2017-01-25  261*/
574de0de Ricardo Neri 2017-01-25  262   if (addr_offset1 >= 0)
574de0de Ricardo Neri 2017-01-25  263   addr1 = 
regs_get_register(regs, addr_offset1);
574de0de Ricardo Neri 2017-01-25  264   if (addr_offset2 >= 0)
574de0de Ricardo Neri 2017-01-25  265   addr2 = 
regs_get_register(regs, addr_offset2);
574de0de Ricardo Neri 2017-01-25  266   seg = 
__get_segment_selector_16(regs, insn, addr_offset1);
574de0de Ricardo Neri 2017-01-25 @267   if (seg < 0)
574de0de Ricardo Neri 2017-01-25  268   goto out_err;
574de0de Ricardo Neri 2017-01-25  269   addr =  (seg << 4) + addr1 + 
addr2;
574de0de Ricardo Neri 2017-01-25  270   }
574de0de Ricardo Neri 2017-01-25  271   addr += insn->displacement.value;
574de0de Ricardo Neri 2017-01-25  272
574de0de Ricardo Neri 2017-01-25  273   return (void __user *)addr;
574de0de Ricardo Neri 2017-01-25  274  out_err:
574de0de Ricardo Neri 2017-01-25  275   return (void __user *)-1;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

[v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[Ricardo Neri]

Tasks running in virtual-8086 mode will use 16-bit addressing form
encodings as described in the Intel 64 and IA-32 Architecture Software
Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings
differ in several ways from the 32-bit/64-bit addressing form encodings:
the r/m part of the ModRM byte points to different registers and, in some
cases, addresses can be indicated by the addition of the value of two
registers. Also, there is no support for SiB bytes. Thus, a separate
function is needed to parse this form of addressing.

Furthermore, virtual-8086 mode tasks will use real-mode addressing. This
implies that the segment selectors do not point to a segment descriptor
but are used to compute logical addresses. Hence, there is a need to
add support to compute addresses using the segment selectors. If segment-
override prefixes are present in the instructions, they take precedence.

Lastly, it is important to note that when a tasks is running in virtual-
8086 mode and an interrupt/exception occurs, the CPU pushes to the stack
the segment selectors for ds, es, fs and gs. These are accesible via the
struct kernel_vm86_regs rather than pt_regs.

Code for 16-bit addressing encodings is likely to be used only by virtual-
8086 mode tasks. Thus, this code is wrapped to be built only if the
option CONFIG_VM86 is selected.

Cc: Dave Hansen 
Cc: Adam Buchbinder 
Cc: Colin Ian King 
Cc: Lorenzo Stoakes 
Cc: Qiaowei Ren 
Cc: Arnaldo Carvalho de Melo 
Cc: Masami Hiramatsu 
Cc: Adrian Hunter 
Cc: Kees Cook 
Cc: Thomas Garnier 
Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Cc: Dmitry Vyukov 
Cc: Ravi V. Shankar 
Cc: x...@kernel.org
Signed-off-by: Ricardo Neri 
---
 arch/x86/lib/insn-kernel.c | 192 +
 1 file changed, 192 insertions(+)

diff --git a/arch/x86/lib/insn-kernel.c b/arch/x86/lib/insn-kernel.c
index 267cab4..10afdf3 100644
--- a/arch/x86/lib/insn-kernel.c
+++ b/arch/x86/lib/insn-kernel.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 
 enum reg_type {
REG_TYPE_RM = 0,
@@ -95,6 +96,194 @@ static int get_reg_offset(struct insn *insn, struct pt_regs 
*regs,
return regoff[regno];
 }
 
+#ifdef CONFIG_VM86
+
+/*
+ * Obtain the segment selector to use based on any prefixes in the instruction
+ * or in the offset of the register given by the r/m part of the ModRM byte. 
The
+ * register offset is as found in struct pt_regs.
+ */
+static unsigned short __get_segment_selector_16(struct pt_regs *regs,
+   struct insn *insn, int regoff)
+{
+   int i;
+
+   struct kernel_vm86_regs *vm86regs = (struct kernel_vm86_regs *)regs;
+
+   /*
+* If not in virtual-8086 mode, the segment selector is not used
+* to compute addresses but to select the segment descriptor. Return
+* 0 to ease the computation of address.
+*/
+   if (!v8086_mode(regs))
+   return 0;
+
+   insn_get_prefixes(insn);
+
+   /* Check first if we have selector overrides */
+   for (i = 0; i < insn->prefixes.nbytes; i++) {
+   switch (insn->prefixes.bytes[i]) {
+   /*
+* Code and stack segment selector register are saved in all
+* processor modes. Thus, it makes sense to take them
+* from pt_regs.
+*/
+   case 0x2e:
+   return (unsigned short)regs->cs;
+   case 0x36:
+   return (unsigned short)regs->ss;
+   /*
+* The rest of the segment selector registers are only saved
+* in virtual-8086 mode. Thus, we must obtain them from the
+* vm86 register structure.
+*/
+   case 0x3e:
+   return vm86regs->ds;
+   case 0x26:
+   return vm86regs->es;
+   case 0x64:
+   return vm86regs->fs;
+   case 0x65:
+   return vm86regs->gs;
+   /*
+* No default action needed. We simply did not find any
+* relevant prefixes.
+*/
+   }
+   }
+
+   /*
+* If no overrides, use default selectors as described in the
+* Intel documentationn.
+*/
+   switch (regoff) {
+   case -EINVAL: /* no register involved in address computation */
+   case offsetof(struct pt_regs, bx):
+   case offsetof(struct pt_regs, di):
+   case offsetof(struct pt_regs, si):
+   return 

[v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

[Ricardo Neri]

Tasks running in virtual-8086 mode will use 16-bit addressing form
encodings as described in the Intel 64 and IA-32 Architecture Software
Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings
differ in several ways from the 32-bit/64-bit addressing form encodings:
the r/m part of the ModRM byte points to different registers and, in some
cases, addresses can be indicated by the addition of the value of two
registers. Also, there is no support for SiB bytes. Thus, a separate
function is needed to parse this form of addressing.

Furthermore, virtual-8086 mode tasks will use real-mode addressing. This
implies that the segment selectors do not point to a segment descriptor
but are used to compute logical addresses. Hence, there is a need to
add support to compute addresses using the segment selectors. If segment-
override prefixes are present in the instructions, they take precedence.

Lastly, it is important to note that when a tasks is running in virtual-
8086 mode and an interrupt/exception occurs, the CPU pushes to the stack
the segment selectors for ds, es, fs and gs. These are accesible via the
struct kernel_vm86_regs rather than pt_regs.

Code for 16-bit addressing encodings is likely to be used only by virtual-
8086 mode tasks. Thus, this code is wrapped to be built only if the
option CONFIG_VM86 is selected.

Cc: Dave Hansen 
Cc: Adam Buchbinder 
Cc: Colin Ian King 
Cc: Lorenzo Stoakes 
Cc: Qiaowei Ren 
Cc: Arnaldo Carvalho de Melo 
Cc: Masami Hiramatsu 
Cc: Adrian Hunter 
Cc: Kees Cook 
Cc: Thomas Garnier 
Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Cc: Dmitry Vyukov 
Cc: Ravi V. Shankar 
Cc: x...@kernel.org
Signed-off-by: Ricardo Neri 
---
 arch/x86/lib/insn-kernel.c | 192 +
 1 file changed, 192 insertions(+)

diff --git a/arch/x86/lib/insn-kernel.c b/arch/x86/lib/insn-kernel.c
index 267cab4..10afdf3 100644
--- a/arch/x86/lib/insn-kernel.c
+++ b/arch/x86/lib/insn-kernel.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 
 enum reg_type {
REG_TYPE_RM = 0,
@@ -95,6 +96,194 @@ static int get_reg_offset(struct insn *insn, struct pt_regs 
*regs,
return regoff[regno];
 }
 
+#ifdef CONFIG_VM86
+
+/*
+ * Obtain the segment selector to use based on any prefixes in the instruction
+ * or in the offset of the register given by the r/m part of the ModRM byte. 
The
+ * register offset is as found in struct pt_regs.
+ */
+static unsigned short __get_segment_selector_16(struct pt_regs *regs,
+   struct insn *insn, int regoff)
+{
+   int i;
+
+   struct kernel_vm86_regs *vm86regs = (struct kernel_vm86_regs *)regs;
+
+   /*
+* If not in virtual-8086 mode, the segment selector is not used
+* to compute addresses but to select the segment descriptor. Return
+* 0 to ease the computation of address.
+*/
+   if (!v8086_mode(regs))
+   return 0;
+
+   insn_get_prefixes(insn);
+
+   /* Check first if we have selector overrides */
+   for (i = 0; i < insn->prefixes.nbytes; i++) {
+   switch (insn->prefixes.bytes[i]) {
+   /*
+* Code and stack segment selector register are saved in all
+* processor modes. Thus, it makes sense to take them
+* from pt_regs.
+*/
+   case 0x2e:
+   return (unsigned short)regs->cs;
+   case 0x36:
+   return (unsigned short)regs->ss;
+   /*
+* The rest of the segment selector registers are only saved
+* in virtual-8086 mode. Thus, we must obtain them from the
+* vm86 register structure.
+*/
+   case 0x3e:
+   return vm86regs->ds;
+   case 0x26:
+   return vm86regs->es;
+   case 0x64:
+   return vm86regs->fs;
+   case 0x65:
+   return vm86regs->gs;
+   /*
+* No default action needed. We simply did not find any
+* relevant prefixes.
+*/
+   }
+   }
+
+   /*
+* If no overrides, use default selectors as described in the
+* Intel documentationn.
+*/
+   switch (regoff) {
+   case -EINVAL: /* no register involved in address computation */
+   case offsetof(struct pt_regs, bx):
+   case offsetof(struct pt_regs, di):
+   case offsetof(struct pt_regs, si):
+   return vm86regs->ds;
+   case offsetof(struct pt_regs, bp):
+   case offsetof(struct pt_regs, sp):
+   return (unsigned short)regs->ss;
+   /* ax, cx, dx are not valid registers for 16-bit addressing*/
+   default:
+   return -EINVAL;
+   }
+}
+
+/*
+ * Obtain offsets from pt_regs to the two registers indicated by the
+ * r/m