Claudio Carvalho <cclaudio@xxxxxxxxxxxxx> writes: > On 7/11/19 9:57 AM, Michael Ellerman wrote: >> Claudio Carvalho <cclaudio@xxxxxxxxxxxxx> writes: >>> From: Ram Pai <linuxram@xxxxxxxxxx> >>> >>> Add the ucall() function, which can be used to make ultravisor calls >>> with varied number of in and out arguments. Ultravisor calls can be made >>> from the host or guests. >>> >>> This copies the implementation of plpar_hcall(). >> .. with quite a few changes? >> >> This is one of the things I'd like to see in a Documentation file, so >> that people can review the implementation vs the specification. > > I will document this (and other things) in a file under Documentation/powerpc. Thanks. >>> +/* API functions */ >>> +#define UCALL_BUFSIZE 4 >> Please don't copy this design from the hcall code, it has led to bugs in >> the past. >> >> See my (still unmerged) attempt to fix this for the hcall case: >> https://patchwork.ozlabs.org/patch/683577/ >> >> Basically instead of asking callers nicely to define a certain sized >> buffer, and them forgetting, define a proper type that has the right size. > > I will keep that in mind. For now I think we don't need that since the v5 > will have ucall_norets() instead. OK. >>> diff --git a/arch/powerpc/kernel/ultravisor.c b/arch/powerpc/kernel/ultravisor.c >>> index dc6021f63c97..02ddf79a9522 100644 >>> --- a/arch/powerpc/kernel/ultravisor.c >>> +++ b/arch/powerpc/kernel/ultravisor.c >>> @@ -8,10 +8,14 @@ >>> #include <linux/init.h> >>> #include <linux/printk.h> >>> #include <linux/string.h> >>> +#include <linux/export.h> >>> >>> #include <asm/ultravisor.h> >>> #include <asm/firmware.h> >>> >>> +/* in ucall.S */ >>> +EXPORT_SYMBOL_GPL(ucall); >> This should be in ucall.S > > Here I'm following the same way that hypercall wrapper symbols are exported. Yeah we used to not be able to export from .S but that was fixed a few years ago. > Last time I tried to export that in ucall.S the linker complained that the > ucall > symbol will not be versioned. Something like this: > https://patchwork.kernel.org/patch/9452759/ I think you just need to include <asm/export.h> in the .S ? cheers
