Re: [Linux-ia64] Re: [PATCH] head.S fix for unusual load addrs

[Jack Steiner <steiner@sgi.com>]

> 
> > The patch looks mostly fine to me.  I'm not too fond of the __tpa()
> > and __imva() macros, but that may be mainly a matter of preference.
> > What I definitely don't like is that the casting seems rather confused
> > and that the patch is adding __tpa() when we already have ia64_tpa().
> > As an example of the casting issues: __imva() returns a long, but at
> > times it's cast to "unsigned long" which doesn't make a lot of sense
> > (for assignments, anyhow).  Moreover, we should stick to the Linux
> > principle that kernel-space pointers have a type of "void *".
> 
> I can get rid of the __tpa() easily by using ia64_tpa() ... I'll
> fix the type-casting fiasco while  making those changes.
> 
> Do you have some direction for the __imva() macro?  The acronym
> stands for "Identity Mapped Virtual Address", and its purpose is
> to provide the region 7 address for an object, so that the existing
> code can continue to work without a whole lot of run-on changes.
> If you just don't like the name, then it's easy to change to something
> else.  If it's the typecast issue, then I can switch over to void *.
> If it just makes your head hurt coping with schizophrenic dual
> mapping that the kernel gets with this patch, then you'll just have
> to take a couple of aspirin and look at the patch again in the
> morning :-)

You can always replace __imva(x) with  __va(ia64_tpa(x)). This looks
very strange but is equivalent.  The disadvantage is that it obscures
the conversion that is taking place.

Seem to me that a special macro (any name is fine) makes it easier to
understand the reason for the conversion.




> 
> -Tony
>  
> 


-- 
Thanks

Jack Steiner    (651-683-5302)   (vnet 233-5302)      steiner@sgi.com