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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2934 bytes --]

On Tue, Mar 09, 2021 at 06:28:44PM +1100, 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.

It doesn't really have anything to do with PAPR, though.  Well, I
guess the CAS part does, but quiesce doesn't.  I'd suggest
"VofMachineInterface" - it represents where VOF needs to interact with
machine type specifics.

> > [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.
> 
> 
> 

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]