Re: [PATCH qemu v14] spapr: Implement Open Firmware client interface

[David Gibson]

On Wed, Mar 10, 2021 at 12:55:07PM +1100, Alexey Kardashevskiy wrote:
> 
> 
> On 10/03/2021 01:00, BALATON Zoltan wrote:
> > On Tue, 9 Mar 2021, Alexey Kardashevskiy wrote:
> > > On 09/03/2021 16:29, David Gibson wrote:
> > > > > > > +struct ClientArchitectureSupportClass {
> > > > > > > +    InterfaceClass parent;
> > > > > > > +    target_ulong (*cas)(CPUState *cs, target_ulong vec);
> > > > > > > +    void (*quiesce)(void);
> > > > > > 
> > > > > > Is there actually any real connection of quiesce behaviour to cas
> > > > > > behaviour?  Basically, I'm wondering if this is not so much about
> > > > > > client-architecture-support fundamentally as just about
> > > > > > machine-specific parts of the VOF behaviour.  Which would be fine, 
> > > > > > but
> > > > > > suggests a different name for the interface.
> > > > > 
> > > > > The most canonical way would be having 2 interfaces.
> > > > 
> > > > Why?  I don't see any reason these shouldn't be a single interface, it
> > > > just has a bad name.
> > > 
> > > I renamed it to SpaprVofInterface for now.
> > > 
> > > 
> > > > [snip]
> > > > > > > +typedef int size_t;
> > > > > > > +typedef void client(void);
> > > > > > > +
> > > > > > > +/* globals */
> > > > > > > +extern void _prom_entry(void); /* OF CI entry point
> > > > > > > (i.e. this firmware) */
> > > > > > > +
> > > > > > > +void do_boot(unsigned long addr, unsigned long r3,
> > > > > > > unsigned long r4);
> > > > > > > +
> > > > > > > +/* libc */
> > > > > > > +int strlen(const char *s);
> > > > > > > +int strcmp(const char *s1, const char *s2);
> > > > > > > +void *memcpy(void *dest, const void *src, size_t n);
> > > > > > > +int memcmp(const void *ptr1, const void *ptr2, size_t n);
> > > > > > > +void *memmove(void *dest, const void *src, size_t n);
> > > > > > > +void *memset(void *dest, int c, size_t size);
> > > > > > > +
> > > > > > > +/* Prom */
> > > > > > > +typedef unsigned long prom_arg_t;
> > > > > > > +int call_prom(const char *service, int nargs, int nret, ...);
> > > > > > 
> > > > > > AIUI this isn't so much about calling the PROM, since this *is* the
> > > > > > PROM code, but rather about calling the parts that are implemented 
> > > > > > on
> > > > > > the qemu side.  Different names might clarify that.
> > > > > 
> > > > > "call_ci"?
> > > > 
> > > > Works for me.
> > > 
> > > call_ci() it is then.
> > > 
> > > About builtins such as memcmp() - turns out these are not really
> > > builtins as they are not inlined and gcc/ld still want to link
> > > against libc which is trickier for such firmware (not quite sure how
> > > to do this and keep it small and not pull other libc stuff in), gcc
> > > just knows about them a bit more. This is different from, for
> > > example, __builtin_ctz which is inlined. So I am keeping my libc.o
> > > for now.
> > 
> > Do they really want libc or they are in libgcc for which there's
> > --static-libgcc I think to avoid needing it in runtime but not sure what
> > clang has for these.
> 
> I was getting "unresolved symbol `memcmp`" when I tried calling memcmp() or
> __builtin_memcmp() and "-lc" did not help (installed some multilib packages,

Yeah, you'll need -lgcc rather than -lc, libgcc is the one with the
builtin helpers.

> did not help either). I figured if I cannot get it compile in 3 minutes, I
> should not probably be posting this and better off simply keeping the
> existing small libc.

Fair point.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature