Re: [Xen-devel] [PATCH v4 2/2] xen: use SYMBOL when required

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 9 Jan 2019, Jan Beulich wrote:
> >>> On 08.01.19 at 19:08, <sstabellini@xxxxxxxxxx> wrote:
> > On Tue, 8 Jan 2019, Stefano Stabellini wrote:
> >> So, this is what I am going to do: I'll send a series update according
> >> to your suggestion, with SYMBOL returning the native pointer type. As I
> >> wrote earlier, although weaker, it is still an improvement from my point
> >> of view.
> > 
> > There is a problem with this though I didn't foresee :-(
> > 
> > The native type of _start is not char* -- it is char[]. So I cannot
> > actually return the native type from SYMBOL because I cannot cast to
> > (char[]). I didn't notice it until I actually tried it.
> > 
> > See the implementation of RELOC_HIDE:
> > 
> >   #define RELOC_HIDE(ptr, off)                    \
> >     ({ unsigned long __ptr;                       \
> >       __asm__ ("" : "=r"(__ptr) : "0"(ptr));      \
> >       (typeof(ptr)) (__ptr + (off)); })
> > 
> > It casts to the type at the end, the error is:
> > 
> >   error: cast specifies array type
> >        (typeof(ptr)) (__ptr + (off)); })
> > 
> > We have a few options:
> > 
> > 1) use unsigned long as in this version of the series (the Linux kernel
> > also uses this technique)
> > Sorry if I insist, it is still the best I think :-)
> > 
> > 2) casts the parameters of SYMBOL to the corresponding pointer type
> > For instance:
> >   SYMBOL((char *)_start)
> >   SYMBOL((struct alt_instr *)__alt_instructions_end)
> > This works, but it is ugly, I would say it makes the code worse than
> > option 1)
> > 
> > 2) always return void* from SYMBOL
> > I don't think it is a good idea to use void* as a workaround here
> > 
> > 3) pass the desired return type to SYMBOL
> > For instance:
> >   SYMBOL(_start, char *)
> >   SYMBOL(__alt_instructions_end, struct alt_instr *)
> > Then SYMBOL would automatically cast the return type to char * and
> > struct alt_instr * according to the second parameter.
> > 
> > Do you have any other suggestions?
> 
> 4) 
> 
> #define RELOC_HIDE(ptr, off)                    \
>     ({ unsigned long ptr_;                       \
>       __asm__ ("" : "=r"(ptr_) : "0" (ptr));      \
>       (typeof(*(ptr)) *) (ptr_ + (off)); })
> 
> or, if not suitable for RELOC_HIDE() itself, clone the macro into one
> that fits SYMBOL()'s needs.

OK. I still don't think that this is a good idea. Nonetheless, I have
just sent a patch series that uses this trick in the implementation of
SYMBOL to return the native type.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel