[PATCH v4 2/2] add icswx support

[Tseng-Hui (Frank) Lin]

On Sat, 2011-03-05 at 07:26 +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2011-03-04 at 11:29 -0600, Tseng-Hui (Frank) Lin wrote:
> > On Fri, 2011-03-04 at 12:02 +1100, Benjamin Herrenschmidt wrote:
> > > On Wed, 2011-03-02 at 11:20 -0600, Tseng-Hui (Frank) Lin wrote:
> > > 
> > > > +#define CPU_FTR_ICSWX                  LONG_ASM_CONST(0x1000000000000000)
> > > 
> > > Do we want a userspace visible feature as well ? Or some other way to
> > > inform userspace that we support icswx ?
> > > 
> > Does a user space program really need to know about icswx? Only
> > coprocessor drivers need to know about icswx. Shouldn't user space
> > programs talk to the coprocessor drivers instead? 
> 
> Well, I don't know how you use icswx on P7+, but on Prism it's
> definitely issued directly by userspace.
> 
OK. You've got a point. I wasn't aware of Prism. HFI device driver is
currently the only icswx user on P7. Could you point me to more
information about how Prism uses icswx from user space?

> > Thought about that. However, multiple threads can call use_cop() at the
> > same time. Without the spinlock being setup in advance, how do I
> > guarantee allocating struct copro_data and modifying the pointer in the
> > mm_context to be atomic?
> 
> You don't need to. You allocate and initialize the structure, and you
> compare & swap the pointer. If somebody beat you, you trash your copy. 
> 
Is atomic_cmpxchg() the one to do the trick?

> > > I'm not sure I totally get the point of having an ifdef here. Can't you
> > > make it unconditional ? Or do you expect distros to turn that off in
> > > which case what's the point ?
> > > 
> > There is only one coprocessor, HFI, using icswx at this moment. The lazy
> > switching makes sense. However, in the future, if more types of
> > coprocessors are added, the lazy switching may actually be a bad idea.
> > This option allows users to turn off the lazy switching.
> 
> No user in real life plays with kernel config options. Care to explain
> why the lazy switching would be a problem ?
> 
The lazy switching checks the shadow variable first before setting ACOP
register. This saves mtspr() only if the new value is the same as
current. If there are several coprocessors on the system, the ACOP
register may have to be changed frequently. In that case, the lazy
switching will not save time. In extreme case when the ACOP register
needs to be changed every time, it actually slows down the execution by
the additional shadow variable checking.

> > Same concern as above. I need something initialized in advance to
> > guarantee allocating memory and updating the pointer are safe when it
> > happens in use_cop().
> 
> No you don't, see above.
> 
> Cheers,
> Ben.
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev