On 2016-02-23 at 13:16 Michael Taufen <[email protected]> wrote: > I was a little surprised by the braces one. A lack of braces around > if statement bodies always felt a bit fragile to me. And there was > the whole Apple "goto fail" thing a couple years ago... But if that's > a hard and fast rule for us then I'll roll with it.
Please do. It's part of ours (and Linux's) coding rules. > I just matched the style of the existing aligns/other attributes in > the codebase, so I figure it's ok to ignore those warnings. > > I'll switch the newlines to ; for the symbols. Thanks. All in all, the patches look pretty good. I think the fixup stuff won't work though. Comments below: > From 8b3891864c4cca3795d1bebb9e6f8ab999fc80fa Mon Sep 17 00:00:00 2001 > From: Michael Taufen <[email protected]> > Date: Mon, 22 Feb 2016 14:27:02 -0800 > Subject: Add load, safe load, read xcr0 functions > +static inline int safe_lxcr0(uint64_t xcr0) > +{ > + int err = 0; > + uint32_t eax, edx; > + > + edx = xcr0 >> 32; > + eax = xcr0; > + asm volatile("1: xsetbv\n" > + "2: \n" > + ".section .fixup, \"ax\"\n" > + "3: mov %4, %0\n" > + " jmp 2b \n" > + ".previous" > + : "=r" (err) > + : "a"(eax), "c" (0), "d"(edx), > + "i" (-EINVAL), "0" (err)); > + > + return err; > +} I think this needs more to work as a fixup table entry. Take a look at __read_msr_asm. You need the _ASM_EXTABLE macro, so that the fixup table knows about the 1: and 3: labels. While you're at it, you should also have ASM_STAC and ASM_CLAC (even though they are empty now). x86/uaccess.h has those #defines. You might be able to just #include that. A potentially nicer fix would be to have another commit, before this one, that moves those #defines to somewhere else, so they are accessible. (Might need to do this due to #include loops) > From 44fc3cbbfe39b3f48aa9504ee56c4f824b5b020a Mon Sep 17 00:00:00 2001 > From: Michael Taufen <[email protected]> > Date: Mon, 22 Feb 2016 14:55:52 -0800 > Subject: fp state save, restore, and error handling > diff --git a/kern/arch/x86/trap.h b/kern/arch/x86/trap.h > -/* TODO: this can trigger a GP fault if MXCSR reserved bits are set. Callers > - * will need to handle intercepting the kernel fault. */ > -static inline void restore_fp_state(struct ancillary_state *silly) > +static inline int restore_fp_state(struct ancillary_state *silly) > { > - asm volatile("fxrstor %0" : : "m"(*silly)); > + int err = 0; > + uint32_t eax, edx; > + > + edx = x86_default_xcr0 >> 32; > + eax = x86_default_xcr0; > + asm volatile("1: xrstor64 %1\n" > + "2: \n" > + ".section .fixup, \"ax\"\n" > + "3: mov %4, %0\n" > + " jmp 2b \n" > + ".previous" > + : "=r" (err) > + : "m"(*silly), "a"(eax), "d"(edx), > + "i" (-EINVAL), "0" (err)); > + > + return err; > } Same deal here (need more for the fixup to work). Barret -- You received this message because you are subscribed to the Google Groups "Akaros" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. For more options, visit https://groups.google.com/d/optout.
