On Sat, Dec 25, 2010 at 12:02:58PM -0800, Philip Guenther wrote:
> Here's a revised diff to correct the handling on amd64 and i386 of the
> MXCSR register in frames that could be altered by users, so that a user
> can't trick the kernel into faulting by trying to load an invalid MXCSR
> value during the return to userspace.
>
> Originally worked out by joshe@; this corrects the timing of the call to
> fpuinit() for the primary CPU on amd64 so that the saved mask value is
> initialized correctly. It also uniformly uses
> __attribute__((aligned(16))) do get a buffer of the correct alignment.
>
> I've been using this on amd64 for a while and have confirmed that a test
> program which previous crashed the box now continues just fine. I'm away
> from my i386 test box so we could use confirmation from someone running an
> i386 that shows FXSR in the cpu0 flags list.
>
> ok?
>
>
> Philip Guenther
>
> Index: sys/arch/amd64/amd64/cpu.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 cpu.c
> --- sys/arch/amd64/amd64/cpu.c 27 Nov 2010 13:03:04 -0000 1.40
> +++ sys/arch/amd64/amd64/cpu.c 25 Dec 2010 19:38:48 -0000
> @@ -365,6 +365,8 @@ cpu_attach(struct device *parent, struct
> void
> cpu_init(struct cpu_info *ci)
> {
> + fpuinit(ci);
> +
> /* configure the CPU if needed */
> if (ci->cpu_setup != NULL)
> (*ci->cpu_setup)(ci);
> @@ -509,7 +511,6 @@ cpu_hatch(void *v)
> cpu_init_idt();
> lapic_set_lvt();
> gdt_init_cpu(ci);
> - fpuinit(ci);
>
> lldt(0);
>
> Index: sys/arch/amd64/amd64/fpu.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/fpu.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 fpu.c
> --- sys/arch/amd64/amd64/fpu.c 29 Sep 2010 15:11:31 -0000 1.21
> +++ sys/arch/amd64/amd64/fpu.c 25 Dec 2010 19:38:48 -0000
> @@ -95,6 +95,8 @@
> void fpudna(struct cpu_info *);
> static int x86fpflags_to_siginfo(u_int32_t);
>
> +uint32_t fpu_mxcsr_mask;
> +
> /*
> * Init the FPU.
> */
> @@ -103,6 +105,16 @@ fpuinit(struct cpu_info *ci)
> {
> lcr0(rcr0() & ~(CR0_EM|CR0_TS));
> fninit();
> + if (CPU_IS_PRIMARY(ci)) {
> + struct fxsave64 fx __attribute__((aligned(16)));
> +
> + bzero(&fx, sizeof(fx));
> + fxsave(&fx);
> + if (fx.fx_mxcsr_mask)
> + fpu_mxcsr_mask = fx.fx_mxcsr_mask;
> + else
> + fpu_mxcsr_mask = __INITIAL_MXCSR_MASK__;
> + }
> lcr0(rcr0() | (CR0_TS));
> }
>
> Index: sys/arch/amd64/amd64/machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
> retrieving revision 1.129
> diff -u -p -r1.129 machdep.c
> --- sys/arch/amd64/amd64/machdep.c 22 Nov 2010 21:07:16 -0000 1.129
> +++ sys/arch/amd64/amd64/machdep.c 25 Dec 2010 19:38:49 -0000
> @@ -658,9 +658,11 @@ sys_sigreturn(struct proc *p, void *v, r
> fpusave_proc(p, 0);
>
> if (ksc.sc_fpstate) {
> - if ((error = copyin(ksc.sc_fpstate,
> - &p->p_addr->u_pcb.pcb_savefpu.fp_fxsave, sizeof (struct
> fxsave64))))
> + struct fxsave64 *fx = &p->p_addr->u_pcb.pcb_savefpu.fp_fxsave;
> +
> + if ((error = copyin(ksc.sc_fpstate, fx, sizeof(*fx))))
> return (error);
> + fx->fx_mxcsr &= fpu_mxcsr_mask;
> p->p_md.md_flags |= MDP_USEDFPU;
> }
>
> Index: sys/arch/amd64/amd64/process_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/process_machdep.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 process_machdep.c
> --- sys/arch/amd64/amd64/process_machdep.c 29 Sep 2010 15:11:31 -0000
> 1.9
> +++ sys/arch/amd64/amd64/process_machdep.c 25 Dec 2010 19:38:50 -0000
> @@ -137,7 +137,7 @@ process_read_fpregs(struct proc *p, stru
> frame->fx_fsw = 0x0000;
> frame->fx_ftw = 0xff;
> frame->fx_mxcsr = __INITIAL_MXCSR__;
> - frame->fx_mxcsr_mask = __INITIAL_MXCSR_MASK__;
> + frame->fx_mxcsr_mask = fpu_mxcsr_mask;
> p->p_md.md_flags |= MDP_USEDFPU;
> }
>
> @@ -198,6 +198,7 @@ process_write_fpregs(struct proc *p, str
> }
>
> memcpy(frame, ®s->fxstate, sizeof(*regs));
> + frame->fx_mxcsr &= fpu_mxcsr_mask;
> return (0);
> }
>
> Index: sys/arch/amd64/include/fpu.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/include/fpu.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 fpu.h
> --- sys/arch/amd64/include/fpu.h 20 Nov 2010 20:11:17 -0000 1.7
> +++ sys/arch/amd64/include/fpu.h 25 Dec 2010 19:38:50 -0000
> @@ -49,6 +49,8 @@ struct savefpu {
> struct trapframe;
> struct cpu_info;
>
> +extern uint32_t fpu_mxcsr_mask;
> +
> void fpuinit(struct cpu_info *);
> void fpudrop(void);
> void fpudiscard(struct proc *);
> Index: sys/arch/i386/i386/machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
> retrieving revision 1.485
> diff -u -p -r1.485 machdep.c
> --- sys/arch/i386/i386/machdep.c 2 Oct 2010 23:31:34 -0000 1.485
> +++ sys/arch/i386/i386/machdep.c 25 Dec 2010 19:38:52 -0000
> @@ -2362,9 +2362,11 @@ sys_sigreturn(struct proc *p, void *v, r
> npxsave_proc(p, 0);
>
> if (context.sc_fpstate) {
> - if ((error = copyin(context.sc_fpstate,
> - &p->p_addr->u_pcb.pcb_savefpu, sizeof (union savefpu))))
> + union savefpu *sfp = &p->p_addr->u_pcb.pcb_savefpu;
> + if ((error = copyin(context.sc_fpstate, sfp, sizeof(*sfp))))
> return (error);
> + if (i386_use_fxsave)
> + sfp->sv_xmm.sv_env.en_mxcsr &= fpu_mxcsr_mask;
> p->p_md.md_flags |= MDP_USEDFPU;
> }
>
> Index: sys/arch/i386/i386/process_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/process_machdep.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 process_machdep.c
> --- sys/arch/i386/i386/process_machdep.c 29 Sep 2010 15:11:31 -0000
> 1.25
> +++ sys/arch/i386/i386/process_machdep.c 25 Dec 2010 19:38:52 -0000
> @@ -137,6 +137,7 @@ process_fninit_xmm(struct savexmm *sxmm)
> memset(sxmm, 0, sizeof(*sxmm));
> sxmm->sv_env.en_cw = __OpenBSD_NPXCW__;
> sxmm->sv_env.en_mxcsr = __INITIAL_MXCSR__;
> + sxmm->sv_env.en_mxcsr_mask = fpu_mxcsr_mask;
> sxmm->sv_env.en_sw = 0x0000;
> sxmm->sv_env.en_tw = 0x00;
> }
> @@ -308,6 +309,7 @@ process_write_fpregs(struct proc *p, str
> /* XXX Yuck. */
> memcpy(&s87, regs, sizeof(*regs));
> process_s87_to_xmm(&s87, &frame->sv_xmm);
> + frame->sv_xmm.sv_env.en_mxcsr &= fpu_mxcsr_mask;
> } else
> memcpy(&frame->sv_87, regs, sizeof(*regs));
>
> Index: sys/arch/i386/include/npx.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/include/npx.h,v
> retrieving revision 1.15
> diff -u -p -r1.15 npx.h
> --- sys/arch/i386/include/npx.h 29 Sep 2010 15:11:31 -0000 1.15
> +++ sys/arch/i386/include/npx.h 25 Dec 2010 19:38:52 -0000
> @@ -96,7 +96,7 @@ struct envxmm {
> uint16_t en_fos; /* FPU Data pointer selector */
> uint16_t en_rsvd2;
> uint32_t en_mxcsr; /* MXCSR Register State */
> - uint32_t en_rsvd3;
> + uint32_t en_mxcsr_mask; /* Mask for valid MXCSR bits (may be 0) */
> };
>
> /* FPU regsters in the extended save format. */
> @@ -142,6 +142,7 @@ struct emcsts {
> * Set Reference, pg. 3-369.
> */
> #define __INITIAL_MXCSR__ 0x1f80
> +#define __INITIAL_MXCSR_MASK__ 0xffbf
>
> /*
> * The standard control word from finit is 0x37F, giving:
> @@ -157,6 +158,8 @@ void process_s87_to_xmm(const struct
>
> struct cpu_info;
> struct trapframe;
> +
> +extern unsigned int fpu_mxcsr_mask;
>
> void npxinit(struct cpu_info *);
> void npxtrap(struct trapframe *);
> Index: sys/arch/i386/isa/npx.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/isa/npx.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 npx.c
> --- sys/arch/i386/isa/npx.c 29 Sep 2010 15:11:31 -0000 1.53
> +++ sys/arch/i386/isa/npx.c 25 Dec 2010 19:38:52 -0000
> @@ -54,6 +54,7 @@
>
> #include <machine/cpu.h>
> #include <machine/intr.h>
> +#include <machine/lock.h>
> #include <machine/npx.h>
> #include <machine/pio.h>
> #include <machine/cpufunc.h>
> @@ -97,6 +98,11 @@
> #define clts() __asm("clts")
> #define stts() lcr0(rcr0() | CR0_TS)
>
> +/*
> + * The mxcsr_mask for this host, taken from fxsave() on the primary CPU
> + */
> +unsigned int fpu_mxcsr_mask;
> +
> int npxintr(void *);
> static int npxprobe1(struct isa_attach_args *);
> static int x86fpflags_to_siginfo(u_int32_t);
> @@ -350,6 +356,16 @@ npxinit(struct cpu_info *ci)
> i386_fpu_fdivbug = 1;
> printf("%s: WARNING: Pentium FDIV bug detected!\n",
> ci->ci_dev.dv_xname);
> + }
> + if (CPU_IS_PRIMARY(ci) && i386_use_fxsave) {
> + struct savexmm xm __attribute__((aligned(16)));
> +
> + bzero(&xm, sizeof(xm));
> + fxsave(&xm);
> + if (xm.sv_env.en_mxcsr_mask)
> + fpu_mxcsr_mask = xm.sv_env.en_mxcsr_mask;
> + else
> + fpu_mxcsr_mask = __INITIAL_MXCSR_MASK__;
> }
> lcr0(rcr0() | (CR0_TS));
> }
>
Appears to work (i.e. boot) on both my amd64 and i386 boxes. The
latter has FXSR in the cpu0 flags. Just in case I am hallucinating,
the i386 dmesg is below.
.... Ken
OpenBSD 4.8-current (GENERIC.MP) #0: Sun Dec 26 08:56:56 EST 2010
r...@acer.westerback.ca:/usr/src/sys/arch/i386/compile/GENERIC.MP
cpu0: Pentium(R) Dual-Core CPU E5400 @ 2.70GHz ("GenuineIntel" 686-class) 2.71
GHz
cpu0:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,XSAVE
real mem = 2683523072 (2559MB)
avail mem = 2629521408 (2507MB)
mainbus0 at root
bios0 at mainbus0: AT/286+ BIOS, date 10/14/09, SMBIOS rev. 2.6 @ 0x9f400 (38
entries)
bios0: vendor American Megatrends Inc. version "P01-A4" date 10/14/2009
bios0: Acer Aspire X1800
acpi0 at bios0: rev 0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP APIC MCFG SLIC WDRT OEMB HPET NVHD AWMI SSDT
acpi0: wakeup devices PS2K(S4) PS2M(S4) NSMB(S4) USB0(S3) USB2(S3) NMAC(S5)
P0P1(S4) HDAC(S4) BR10(S4) BR11(S4) BR12(S4) PWRB(S4)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpimadt0 at acpi0 addr 0xfee00000: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: apic clock running at 200MHz
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Pentium(R) Dual-Core CPU E5400 @ 2.70GHz ("GenuineIntel" 686-class) 2.71
GHz
cpu1:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,XSAVE
ioapic0 at mainbus0: apid 2 pa 0xfec00000, version 11, 24 pins
acpihpet0 at acpi0: 25000000 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus 1 (P0P1)
acpiprt2 at acpi0: bus 2 (BR10)
acpiprt3 at acpi0: bus 3 (BR11)
acpiprt4 at acpi0: bus 4 (BR12)
acpicpu0 at acpi0: PSS
acpicpu1 at acpi0: PSS
acpitz0 at acpi0: critical temperature 125 degC
acpibtn0 at acpi0: PWRB
bios0: ROM list: 0xc0000/0xde00
cpu0: Enhanced SpeedStep 2701 MHz: speeds: 2700, 2003, 1603, 1203 MHz
pci0 at mainbus0 bus 0: configuration mode 1 (bios)
pchb0 at pci0 dev 0 function 0 "NVIDIA MCP73 Host" rev 0xa2
"NVIDIA MCP73 Memory" rev 0xa2 at pci0 dev 0 function 1 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 0 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 1 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 2 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 3 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 4 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 5 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 6 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 2 function 0 not configured
pcib0 at pci0 dev 3 function 0 "NVIDIA MCP73 ISA" rev 0xa2
nviic0 at pci0 dev 3 function 1 "NVIDIA MCP73 SMBus" rev 0xa1
iic0 at nviic0
spdmem0 at iic0 addr 0x50: 2GB DDR2 SDRAM non-parity PC2-6400CL5
spdmem1 at iic0 addr 0x51: 2GB DDR2 SDRAM non-parity PC2-6400CL5
iic1 at nviic0
"eeprom" at iic1 addr 0x55 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 3 function 2 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 3 function 4 not configured
ohci0 at pci0 dev 4 function 0 "NVIDIA MCP73 USB" rev 0xa1: apic 2 int 7 (irq
7), version 1.0, legacy support
ehci0 at pci0 dev 4 function 1 "NVIDIA MCP73 USB" rev 0xa1: apic 2 int 10 (irq
10)
usb0 at ehci0: USB revision 2.0
uhub0 at usb0 "NVIDIA EHCI root hub" rev 2.00/1.00 addr 1
pciide0 at pci0 dev 8 function 0 "NVIDIA MCP73 IDE" rev 0xa1: DMA, channel 0
configured to compatibility, channel 1 configured to compatibility
pciide0: channel 0 disabled (no drives)
pciide0: channel 1 ignored (disabled)
azalia0 at pci0 dev 9 function 0 "NVIDIA MCP73 HD Audio" rev 0xa1: apic 2 int
11 (irq 11)
azalia0: codecs: Realtek ALC888, NVIDIA/0x8001, using Realtek ALC888
audio0 at azalia0
ppb0 at pci0 dev 10 function 0 "NVIDIA MCP73 PCIE" rev 0xa1
pci1 at ppb0 bus 1
ppb1 at pci0 dev 11 function 0 "NVIDIA MCP73 PCIE" rev 0xa1
pci2 at ppb1 bus 2
ppb2 at pci0 dev 12 function 0 "NVIDIA MCP73 PCIE" rev 0xa1
pci3 at ppb2 bus 3
ppb3 at pci0 dev 13 function 0 "NVIDIA MCP73 PCIE" rev 0xa1
pci4 at ppb3 bus 4
vendor "VIA", unknown product 0x3403 (class serial bus subclass Firewire, rev
0x00) at pci4 dev 0 function 0 not configured
ahci0 at pci0 dev 14 function 0 "NVIDIA MCP73 AHCI" rev 0xa2: apic 2 int 5 (irq
5), AHCI 1.1
scsibus0 at ahci0: 32 targets
sd0 at scsibus0 targ 0 lun 0: <ATA, Hitachi HDT72103, ST2O> SCSI3 0/direct fixed
sd0: 305245MB, 512 bytes/sec, 625142448 sec total
cd0 at scsibus0 targ 1 lun 0: <ATAPI, DVD A DH16AASH, SA15> ATAPI 5/cdrom
removable
nfe0 at pci0 dev 15 function 0 "NVIDIA MCP73 LAN" rev 0xa2: apic 2 int 7 (irq
7), address 00:25:11:8e:a6:7c
rgephy0 at nfe0 phy 3: RTL8169S/8110S PHY, rev. 2
vga1 at pci0 dev 16 function 0 "NVIDIA GeForce 7100" rev 0xa2
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
isa0 at pcib0
isadma0 at isa0
com0 at isa0 port 0x3f8/8 irq 4: ns16550a, 16 byte fifo
pckbc0 at isa0 port 0x60/5
pckbd0 at pckbc0 (kbd slot)
pckbc0: using irq 1 for kbd slot
wskbd0 at pckbd0: console keyboard, using wsdisplay0
pcppi0 at isa0 port 0x61
spkr0 at pcppi0
it0 at isa0 port 0x2e/2: IT8718F rev 4, EC port 0xa10
npx0 at isa0 port 0xf0/16: reported by CPUID; using exception 16
usb1 at ohci0: USB revision 1.0
uhub1 at usb1 "NVIDIA OHCI root hub" rev 1.00/1.00 addr 1
mtrr: Pentium Pro MTRR support
umass0 at uhub0 port 6 configuration 1 interface 0 "Generic Mass Storage
Device" rev 2.00/1.00 addr 2
umass0: using SCSI over Bulk-Only
scsibus1 at umass0: 2 targets, initiator 0
sd1 at scsibus1 targ 1 lun 0: <Generic-, Compact Flash, 1.01> SCSI0 0/direct
removable
sd1: drive offline
sd2 at scsibus1 targ 1 lun 1: <Multiple, Flash Reader, 1.05> SCSI0 0/direct
removable
sd2: drive offline
uhidev0 at uhub0 port 6 configuration 1 interface 1 "Generic Mass Storage
Device" rev 2.00/1.00 addr 2
uhidev0: iclass 3/0, 4 report ids
uhid0 at uhidev0 reportid 4: input=7, output=0, feature=0
vscsi0 at root
scsibus2 at vscsi0: 256 targets
softraid0 at root
root on sd0a swap on sd0b dump on sd0b
