[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 2/2] xen/arm32: implement VFP context switch
On Mon, 2013-06-03 at 12:14 +0100, Julien Grall wrote: > On 05/31/2013 04:54 PM, Ian Campbell wrote: > > > On Fri, 2013-05-31 at 16:27 +0100, Julien Grall wrote: > >> On 05/31/2013 03:12 PM, Ian Campbell wrote: > >> > >>> On Thu, 2013-05-30 at 17:01 +0100, Julien Grall wrote: > >>>> +void vfp_save_state(struct vcpu *v) > >>>> +{ > >>>> + uint32_t tmp; > >>>> + > >>>> + v->arch.vfp.fpexc = READ_CP32(FPEXC); > >>> > >>> The docs seem to call for reading this via an explicit VMRS > >>> instruction. > >>> > >>> Looking at the ARM ARM this seems to be an alias for the encoding of an > >>> MRC instruction corresponding to reading FPEXC as you have done. Did you > >>> have a reference for that aliasing? (I'm not finding it in the ARM ARM). > >> > >> > >> I didn't find anything on the aliasing. I looked at the linux VFP > >> include (arch/arm/include/asm/vfp.h). > > > > I wonder if it would be better to do this via VFP specific macros? > > > Except in this code we don't need the VFP macros. I don't see specific > reason to use it here. > > > Probably not. As you can see I agree ;-) > > > >>> Are you avoiding the mnemonic to avoid issues with binutils providing > >>> the instruction? > >> > >> This mnemonic can only be used if VFP is enabled by gcc. I think it's > >> safer to use mrc if we don't want to use VFP on the other part of XEN. > > > > Agreed. > > > >>> Do you know what happens to the values of d0..d31 and FPSCR etc when > >> > >>> FPEXC.EN is disabled? I'm wondering if we can avoid saving the registers > >>> in that case, we'd still need to restore to prevent leaking the last > >>> guests state if the VCPU enabled exceptions. I suppose it's not worth it > >>> unless we implement a more full featured lazy saving regime. > >> > >> > >> I didn't find something about the state of the registers when VFP is > >> disabled. I think the registers is not clobbered. > >> Linux seems to save vfp registers at each context switch only if VFP is > >> enabled. But we cannot rely on that for the other distributions. > > > > Linux likely traps if the process then goes on to use VFP, at which > > point it can clear/restore the registers as necessary. We could do > > something similar using HCPTR -- that's what I meant by lazy saving (and > > restore). > > I will not have time before 4.3 to implement lazy context switch. I > think we can postpone this part and see later. Yes, sorry I should have been clearer -- not worth it until/if we implement lazy switching post 4.3 is what I meant to say. > > >> > >>>> +} > >>>> + > >>>> +void vfp_restore_state(struct vcpu *v) > >>>> +{ > >>> > >>> Some of the same comments (tmps, barriers etc) apply to restore as to > >>> save. > >>> > >>> Is there any chance that any of these loads could cause an FP fault? > >>> e.g. if the guest had an FP exception pending when we saved it, could > >>> reloading the register retrigger it? > >> > >> I don't know. > > > > Hrm :-/ > > > > If there are no invalid encodings for d0..d31 then it should just be a > > case of checking the ARM ARM for FPINST* and FPSCR. > > > For FPINST*: > "Any value read from a Floating-Point Instruction Register can be > written back to the same register. This means > context switch and debugger software can save and restore Floating-Point > Instruction Register values." Good. > For FPSCR: > "Writes to the FPSCR can have side-effects on various aspects of > processor operation. All of these side-effects are > synchronous to the FPSCR write. This means they are guaranteed not to be > visible to earlier instructions in the > execution stream, and they are guaranteed to be visible to later > instructions in the execution stream." > > Except if I missed something in FPSCR encoding, we are safe during the > restore step. Yes, sounds like it. Thanks. > >>> The register resets to 0x0 which is OK but it might be wise to trap all > >>> accesses the coprocessors which we haven't implemented ctx switching > >>> for? So at least we find out about missing ones instead of silently > >>> leaking information between guests and/or corrupting their state. > >> > >> > >> What about sending an undefined instruction to the guest if the > >> coprocessor is not implemented? > >> It seems that some software (sshd, ntpdate) which are using libcrypto, > >> are trying to access to the cryptographic coprocessor even if it doesn't > >> exist. > > > > Urk. We should either ctx switch it properly or we should hide it > > properly (from all the feature registers) and inject undef. > > > I will try to write a patch for that when I will have time. Thank you. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |