Re: Value of eax register after BIOS interrupt call from boot(8)

2019-11-09 Thread Philip Guenther
On Friday, November 8, 2019, Theo de Raadt  wrote:

> Philip Guenther  wrote:
>
> > No, it should be the other way, moving the “clear NT flag” block down
> after
> > the “save registers into save area” block
>
> Ah.
>
> Index: arch/amd64/stand/libsa/gidt.S
> ===
> RCS file: /cvs/src/sys/arch/amd64/stand/libsa/gidt.S,v
> retrieving revision 1.11
> diff -u -p -u -r1.11 gidt.S
> --- arch/amd64/stand/libsa/gidt.S   27 Oct 2012 15:43:42 -
> 1.11
> +++ arch/amd64/stand/libsa/gidt.S   9 Nov 2019 06:50:57 -
> @@ -423,14 +423,6 @@ intno  = . - 1
> movl%edx, 0x9*4(%esp)
> movb%bh , 0xe*4(%esp)
>
> -   /* clear NT flag in eflags */
> -   /* Martin Fredriksson  */
> -   pushf
> -   pop %eax
> -   and $0xbfff, %eax
> -   push%eax
> -   popf
> -
> /* save registers into save area */
> movl%eax, _C_LABEL(BIOS_regs)+BIOSR_AX
> movl%ecx, _C_LABEL(BIOS_regs)+BIOSR_CX
> @@ -438,6 +430,13 @@ intno  = . - 1
> movl%ebp, _C_LABEL(BIOS_regs)+BIOSR_BP
> movl%esi, _C_LABEL(BIOS_regs)+BIOSR_SI
> movl%edi, _C_LABEL(BIOS_regs)+BIOSR_DI
> +
> +   /* clear NT flag in eflags */
> +   pushf
> +   pop %eax
> +   and $0xbfff, %eax
> +   push%eax
> +   popf
>
> pop %gs
> pop %fs
> Index: arch/i386/stand/libsa/gidt.S
> ===
> RCS file: /cvs/src/sys/arch/i386/stand/libsa/gidt.S,v
> retrieving revision 1.36
> diff -u -p -u -r1.36 gidt.S
> --- arch/i386/stand/libsa/gidt.S31 Oct 2012 13:55:58 -
> 1.36
> +++ arch/i386/stand/libsa/gidt.S9 Nov 2019 06:51:29 -
> @@ -426,14 +426,6 @@ intno  = . - 1
> movl%edx, 0x9*4(%esp)
> movb%bh , 0xe*4(%esp)
>
> -   /* clear NT flag in eflags */
> -   /* Martin Fredriksson  */
> -   pushf
> -   pop %eax
> -   and $0xbfff, %eax
> -   push%eax
> -   popf
> -
> /* save registers into save area */
> movl%eax, _C_LABEL(BIOS_regs)+BIOSR_AX
> movl%ecx, _C_LABEL(BIOS_regs)+BIOSR_CX
> @@ -441,6 +433,13 @@ intno  = . - 1
> movl%ebp, _C_LABEL(BIOS_regs)+BIOSR_BP
> movl%esi, _C_LABEL(BIOS_regs)+BIOSR_SI
> movl%edi, _C_LABEL(BIOS_regs)+BIOSR_DI
> +
> +   /* clear NT flag in eflags */
> +   pushf
> +   pop %eax
> +   and $0xbfff, %eax
> +   push%eax
> +   popf
>
> pop %gs
> pop %f
>

Ok guenther@


Re: Value of eax register after BIOS interrupt call from boot(8)

2019-11-09 Thread Julius Zint
> 
> Index: arch/amd64/stand/libsa/gidt.S
> ===
> RCS file: /cvs/src/sys/arch/amd64/stand/libsa/gidt.S,v
> retrieving revision 1.11
> diff -u -p -u -r1.11 gidt.S
> --- arch/amd64/stand/libsa/gidt.S 27 Oct 2012 15:43:42 -  1.11
> +++ arch/amd64/stand/libsa/gidt.S 9 Nov 2019 06:50:57 -
> @@ -423,14 +423,6 @@ intno= . - 1
>   movl%edx, 0x9*4(%esp)
>   movb%bh , 0xe*4(%esp)
> 
> - /* clear NT flag in eflags */
> - /* Martin Fredriksson  */
> - pushf
> - pop %eax
> - and $0xbfff, %eax
> - push%eax
> - popf
> -
>   /* save registers into save area */
>   movl%eax, _C_LABEL(BIOS_regs)+BIOSR_AX
>   movl%ecx, _C_LABEL(BIOS_regs)+BIOSR_CX
> @@ -438,6 +430,13 @@ intno= . - 1
>   movl%ebp, _C_LABEL(BIOS_regs)+BIOSR_BP
>   movl%esi, _C_LABEL(BIOS_regs)+BIOSR_SI
>   movl%edi, _C_LABEL(BIOS_regs)+BIOSR_DI
> +
> + /* clear NT flag in eflags */
> + pushf
> + pop %eax
> + and $0xbfff, %eax
> + push%eax
> + popf
> 
>   pop %gs
>   pop %fs
> Index: arch/i386/stand/libsa/gidt.S
> ===
> RCS file: /cvs/src/sys/arch/i386/stand/libsa/gidt.S,v
> retrieving revision 1.36
> diff -u -p -u -r1.36 gidt.S
> --- arch/i386/stand/libsa/gidt.S  31 Oct 2012 13:55:58 -  1.36
> +++ arch/i386/stand/libsa/gidt.S  9 Nov 2019 06:51:29 -
> @@ -426,14 +426,6 @@ intno= . - 1
>   movl%edx, 0x9*4(%esp)
>   movb%bh , 0xe*4(%esp)
> 
> - /* clear NT flag in eflags */
> - /* Martin Fredriksson  */
> - pushf
> - pop %eax
> - and $0xbfff, %eax
> - push%eax
> - popf
> -
>   /* save registers into save area */
>   movl%eax, _C_LABEL(BIOS_regs)+BIOSR_AX
>   movl%ecx, _C_LABEL(BIOS_regs)+BIOSR_CX
> @@ -441,6 +433,13 @@ intno= . - 1
>   movl%ebp, _C_LABEL(BIOS_regs)+BIOSR_BP
>   movl%esi, _C_LABEL(BIOS_regs)+BIOSR_SI
>   movl%edi, _C_LABEL(BIOS_regs)+BIOSR_DI
> +
> + /* clear NT flag in eflags */
> + pushf
> + pop %eax
> + and $0xbfff, %eax
> + push%eax
> + popf
> 
>   pop %gs
>   pop %fs

These changes work for me. As part of my Masterthesis im working on a
measured boot with OpenBSD and to communicate with the TPM from within boot
(8) BIOS calls are a convenient way to do so.

Thanks for the fast response

Julius




Re: Value of eax register after BIOS interrupt call from boot(8)

2019-11-08 Thread Theo de Raadt
Philip Guenther  wrote:

> No, it should be the other way, moving the “clear NT flag” block down after
> the “save registers into save area” block

Ah.

Index: arch/amd64/stand/libsa/gidt.S
===
RCS file: /cvs/src/sys/arch/amd64/stand/libsa/gidt.S,v
retrieving revision 1.11
diff -u -p -u -r1.11 gidt.S
--- arch/amd64/stand/libsa/gidt.S   27 Oct 2012 15:43:42 -  1.11
+++ arch/amd64/stand/libsa/gidt.S   9 Nov 2019 06:50:57 -
@@ -423,14 +423,6 @@ intno  = . - 1
movl%edx, 0x9*4(%esp)
movb%bh , 0xe*4(%esp)
 
-   /* clear NT flag in eflags */
-   /* Martin Fredriksson  */
-   pushf
-   pop %eax
-   and $0xbfff, %eax
-   push%eax
-   popf
-
/* save registers into save area */
movl%eax, _C_LABEL(BIOS_regs)+BIOSR_AX
movl%ecx, _C_LABEL(BIOS_regs)+BIOSR_CX
@@ -438,6 +430,13 @@ intno  = . - 1
movl%ebp, _C_LABEL(BIOS_regs)+BIOSR_BP
movl%esi, _C_LABEL(BIOS_regs)+BIOSR_SI
movl%edi, _C_LABEL(BIOS_regs)+BIOSR_DI
+
+   /* clear NT flag in eflags */
+   pushf
+   pop %eax
+   and $0xbfff, %eax
+   push%eax
+   popf
 
pop %gs
pop %fs
Index: arch/i386/stand/libsa/gidt.S
===
RCS file: /cvs/src/sys/arch/i386/stand/libsa/gidt.S,v
retrieving revision 1.36
diff -u -p -u -r1.36 gidt.S
--- arch/i386/stand/libsa/gidt.S31 Oct 2012 13:55:58 -  1.36
+++ arch/i386/stand/libsa/gidt.S9 Nov 2019 06:51:29 -
@@ -426,14 +426,6 @@ intno  = . - 1
movl%edx, 0x9*4(%esp)
movb%bh , 0xe*4(%esp)
 
-   /* clear NT flag in eflags */
-   /* Martin Fredriksson  */
-   pushf
-   pop %eax
-   and $0xbfff, %eax
-   push%eax
-   popf
-
/* save registers into save area */
movl%eax, _C_LABEL(BIOS_regs)+BIOSR_AX
movl%ecx, _C_LABEL(BIOS_regs)+BIOSR_CX
@@ -441,6 +433,13 @@ intno  = . - 1
movl%ebp, _C_LABEL(BIOS_regs)+BIOSR_BP
movl%esi, _C_LABEL(BIOS_regs)+BIOSR_SI
movl%edi, _C_LABEL(BIOS_regs)+BIOSR_DI
+
+   /* clear NT flag in eflags */
+   pushf
+   pop %eax
+   and $0xbfff, %eax
+   push%eax
+   popf
 
pop %gs
pop %fs



Re: Value of eax register after BIOS interrupt call from boot(8)

2019-11-08 Thread Philip Guenther
On Friday, November 8, 2019, Theo de Raadt  wrote:

> Philip Guenther  wrote:
>
> > Since we're unlikely to do _more_ with BIOS calls in the boot loader, my
> > inclination would be to eliminate the structure value and the code that
> > sets it (incorrectly).  Opinions?
>
> I dunno, my crystal ball provides a more cynical outlook.
>
> How about we just repair by swapping the blocks as you propose, then
> noone gets surprised down the road if they try to use the bios-interface
> API's full functionality.
>
> The bootblocks don't shrink, but they don't grow either.
>
> Is this the right diff?  I'm deleting the name which is in the commitlogs
> since that isn't our style.

...

> --- sys/arch/amd64/stand/libsa/gidt.S   27 Oct 2012 15:43:42 -
> 1.11
>
+++ sys/arch/amd64/stand/libsa/gidt.S   9 Nov 2019 03:57:11 -
> @@ -417,19 +417,18 @@ intno = . - 1
> .byte   0xb8
>  2: .long   0x90909090
>
> -   /* pass BIOS return values back to caller */
> -   movl%eax, 0xb*4(%esp)
> -   movl%ecx, 0xa*4(%esp)
> -   movl%edx, 0x9*4(%esp)
> -   movb%bh , 0xe*4(%esp)
> -
> /* clear NT flag in eflags */
> -   /* Martin Fredriksson  */
> pushf
> pop %eax
> and $0xbfff, %eax
> push%eax
> popf


No, it should be the other way, moving the “clear NT flag” block down after
the “save registers into save area” block

Philip


Re: Value of eax register after BIOS interrupt call from boot(8)

2019-11-08 Thread Theo de Raadt
Philip Guenther  wrote:

> Since we're unlikely to do _more_ with BIOS calls in the boot loader, my
> inclination would be to eliminate the structure value and the code that
> sets it (incorrectly).  Opinions?

I dunno, my crystal ball provides a more cynical outlook.

How about we just repair by swapping the blocks as you propose, then
noone gets surprised down the road if they try to use the bios-interface
API's full functionality.

The bootblocks don't shrink, but they don't grow either.

Is this the right diff?  I'm deleting the name which is in the commitlogs
since that isn't our style.

Index: sys/arch/amd64/stand/libsa/gidt.S
===
RCS file: /cvs/src/sys/arch/amd64/stand/libsa/gidt.S,v
retrieving revision 1.11
diff -u -p -u -r1.11 gidt.S
--- sys/arch/amd64/stand/libsa/gidt.S   27 Oct 2012 15:43:42 -  1.11
+++ sys/arch/amd64/stand/libsa/gidt.S   9 Nov 2019 03:57:11 -
@@ -417,19 +417,18 @@ intno = . - 1
.byte   0xb8
 2: .long   0x90909090
 
-   /* pass BIOS return values back to caller */
-   movl%eax, 0xb*4(%esp)
-   movl%ecx, 0xa*4(%esp)
-   movl%edx, 0x9*4(%esp)
-   movb%bh , 0xe*4(%esp)
-
/* clear NT flag in eflags */
-   /* Martin Fredriksson  */
pushf
pop %eax
and $0xbfff, %eax
push%eax
popf
+
+   /* pass BIOS return values back to caller */
+   movl%eax, 0xb*4(%esp)
+   movl%ecx, 0xa*4(%esp)
+   movl%edx, 0x9*4(%esp)
+   movb%bh , 0xe*4(%esp)
 
/* save registers into save area */
movl%eax, _C_LABEL(BIOS_regs)+BIOSR_AX
Index: sys/arch/i386/stand/libsa/gidt.S
===
RCS file: /cvs/src/sys/arch/i386/stand/libsa/gidt.S,v
retrieving revision 1.36
diff -u -p -u -r1.36 gidt.S
--- sys/arch/i386/stand/libsa/gidt.S31 Oct 2012 13:55:58 -  1.36
+++ sys/arch/i386/stand/libsa/gidt.S9 Nov 2019 03:56:56 -
@@ -420,19 +420,18 @@ intno = . - 1
.byte   0xb8
 2: .long   0x90909090
 
-   /* pass BIOS return values back to caller */
-   movl%eax, 0xb*4(%esp)
-   movl%ecx, 0xa*4(%esp)
-   movl%edx, 0x9*4(%esp)
-   movb%bh , 0xe*4(%esp)
-
/* clear NT flag in eflags */
-   /* Martin Fredriksson  */
pushf
pop %eax
and $0xbfff, %eax
push%eax
popf
+
+   /* pass BIOS return values back to caller */
+   movl%eax, 0xb*4(%esp)
+   movl%ecx, 0xa*4(%esp)
+   movl%edx, 0x9*4(%esp)
+   movb%bh , 0xe*4(%esp)
 
/* save registers into save area */
movl%eax, _C_LABEL(BIOS_regs)+BIOSR_AX



Re: Value of eax register after BIOS interrupt call from boot(8)

2019-11-07 Thread Philip Guenther
On Thu, Nov 7, 2019 at 9:31 AM Julius Zint  wrote:

> the following code snipped is from sys/arch/amd64/stand/libsa/gidt.S
>
> /* pass BIOS return values back to caller */
> movl%eax, 0xb*4(%esp)
> movl%ecx, 0xa*4(%esp)
> movl%edx, 0x9*4(%esp)
> movb%bh , 0xe*4(%esp)
>
> /* clear NT flag in eflags */
> /* Martin Fredriksson  */
> pushf
> pop %eax
> and $0xbfff, %eax
> push%eax
> popf
>
> /* save registers into save area */
> movl%eax, _C_LABEL(BIOS_regs)+BIOSR_AX
> movl%ecx, _C_LABEL(BIOS_regs)+BIOSR_CX
> movl%edx, _C_LABEL(BIOS_regs)+BIOSR_DX
> movl%ebp, _C_LABEL(BIOS_regs)+BIOSR_BP
> movl%esi, _C_LABEL(BIOS_regs)+BIOSR_SI
> movl%edi, _C_LABEL(BIOS_regs)+BIOSR_DI
>
> These instructions are being executed after a BIOS interrupt. If i read
> correctly, than (BIOS_regs)+BIOSR_AX contains the contents of the eflags
> processor register and not of %eax. Is this intended or should it contain
> the value of %eax?
>

Yeah, it looks like it's in the wrong order.  The trick, of course, is that
nothing actually examines BIOS_regs.biosr_ax, so the fact that the wrong
value is saved there hasn't mattered.

Since we're unlikely to do _more_ with BIOS calls in the boot loader, my
inclination would be to eliminate the structure value and the code that
sets it (incorrectly).  Opinions?


Philip Guenther


Value of eax register after BIOS interrupt call from boot(8)

2019-11-07 Thread Julius Zint
Hi misc,

the following code snipped is from sys/arch/amd64/stand/libsa/gidt.S

/* pass BIOS return values back to caller */
movl%eax, 0xb*4(%esp)
movl%ecx, 0xa*4(%esp)
movl%edx, 0x9*4(%esp)
movb%bh , 0xe*4(%esp)

/* clear NT flag in eflags */
/* Martin Fredriksson  */
pushf
pop %eax
and $0xbfff, %eax
push%eax
popf

/* save registers into save area */
movl%eax, _C_LABEL(BIOS_regs)+BIOSR_AX
movl%ecx, _C_LABEL(BIOS_regs)+BIOSR_CX
movl%edx, _C_LABEL(BIOS_regs)+BIOSR_DX
movl%ebp, _C_LABEL(BIOS_regs)+BIOSR_BP
movl%esi, _C_LABEL(BIOS_regs)+BIOSR_SI
movl%edi, _C_LABEL(BIOS_regs)+BIOSR_DI

These instructions are being executed after a BIOS interrupt. If i read
correctly, than (BIOS_regs)+BIOSR_AX contains the contents of the eflags
processor register and not of %eax. Is this intended or should it contain
the value of %eax?

Kind regards

Julius