[Xen-devel] [PATCH] ARM: xen: unexport HYPERVISOR_platform_op function

Arnd Bergmann posted 1 patch 2 weeks ago
Failed in applying to current master (apply log)
arch/arm/xen/enlighten.c | 1 -
1 file changed, 1 deletion(-)

[Xen-devel] [PATCH] ARM: xen: unexport HYPERVISOR_platform_op function

Posted by Arnd Bergmann 2 weeks ago
HYPERVISOR_platform_op() is an inline function and should not
be exported. Since commit 15bfc2348d54 ("modpost: check for
static EXPORT_SYMBOL* functions"), this causes a warning:

WARNING: "HYPERVISOR_platform_op" [vmlinux] is a static EXPORT_SYMBOL_GPL

Remove the extraneous export.

Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/xen/enlighten.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 1e57692552d9..845c528acf24 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -437,7 +437,6 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_memory_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_physdev_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_tmem_op);
-EXPORT_SYMBOL_GPL(HYPERVISOR_platform_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_multicall);
 EXPORT_SYMBOL_GPL(HYPERVISOR_vm_assist);
 EXPORT_SYMBOL_GPL(HYPERVISOR_dm_op);
-- 
2.20.0


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

Re: [Xen-devel] [PATCH] ARM: xen: unexport HYPERVISOR_platform_op function

Posted by Andrew Cooper 2 weeks ago
On 06/09/2019 16:39, Arnd Bergmann wrote:
> HYPERVISOR_platform_op() is an inline function and should not
> be exported. Since commit 15bfc2348d54 ("modpost: check for
> static EXPORT_SYMBOL* functions"), this causes a warning:
>
> WARNING: "HYPERVISOR_platform_op" [vmlinux] is a static EXPORT_SYMBOL_GPL
>
> Remove the extraneous export.
>
> Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Something is wonky.  That symbol is (/ really ought to be) in the
hypercall page and most definitely not inline.

Which tree is that changeset from?  I can't find the SHA.

I hate to open a separate can of worms, but why are they tagged GPL? 
The Xen hypercall ABI, like the Linux syscall ABI, are specifically not
GPL.  Xen has as something very similar to (and probably derived from)
the Linux-syscall-note exception.

~Andrew

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

Re: [Xen-devel] [PATCH] ARM: xen: unexport HYPERVISOR_platform_op function

Posted by Jan Beulich 2 weeks ago
On 06.09.2019 17:55, Andrew Cooper wrote:
> On 06/09/2019 16:39, Arnd Bergmann wrote:
>> HYPERVISOR_platform_op() is an inline function and should not
>> be exported. Since commit 15bfc2348d54 ("modpost: check for
>> static EXPORT_SYMBOL* functions"), this causes a warning:
>>
>> WARNING: "HYPERVISOR_platform_op" [vmlinux] is a static EXPORT_SYMBOL_GPL
>>
>> Remove the extraneous export.
>>
>> Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Something is wonky.  That symbol is (/ really ought to be) in the
> hypercall page and most definitely not inline.

It's HYPERVISOR_platform_op_raw that's in the hypercall page afaics.

Jan

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

Re: [Xen-devel] [PATCH] ARM: xen: unexport HYPERVISOR_platform_op function

Posted by Arnd Bergmann 2 weeks ago
On Fri, Sep 6, 2019 at 5:55 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 06/09/2019 16:39, Arnd Bergmann wrote:
> > HYPERVISOR_platform_op() is an inline function and should not
> > be exported. Since commit 15bfc2348d54 ("modpost: check for
> > static EXPORT_SYMBOL* functions"), this causes a warning:
> >
> > WARNING: "HYPERVISOR_platform_op" [vmlinux] is a static EXPORT_SYMBOL_GPL
> >
> > Remove the extraneous export.
> >
> > Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Something is wonky.  That symbol is (/ really ought to be) in the
> hypercall page and most definitely not inline.
>
> Which tree is that changeset from?  I can't find the SHA.

This is from linux-next, I think from the kbuild tree.

       Arnd

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

Re: [Xen-devel] [PATCH] ARM: xen: unexport HYPERVISOR_platform_op function

Posted by Andrew Cooper 2 weeks ago
On 06/09/2019 17:00, Arnd Bergmann wrote:
> On Fri, Sep 6, 2019 at 5:55 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 06/09/2019 16:39, Arnd Bergmann wrote:
>>> HYPERVISOR_platform_op() is an inline function and should not
>>> be exported. Since commit 15bfc2348d54 ("modpost: check for
>>> static EXPORT_SYMBOL* functions"), this causes a warning:
>>>
>>> WARNING: "HYPERVISOR_platform_op" [vmlinux] is a static EXPORT_SYMBOL_GPL
>>>
>>> Remove the extraneous export.
>>>
>>> Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> Something is wonky.  That symbol is (/ really ought to be) in the
>> hypercall page and most definitely not inline.
>>
>> Which tree is that changeset from?  I can't find the SHA.
> This is from linux-next, I think from the kbuild tree.

Thanks.

Julien/Stefano: Why are any of these hypercalls out-of-line?  ARM
doesn't use the hypercall page, and there is no argument translation
(not even in arm32 as there are no 5-argument hypercalls declared).

They'd surely be easier to implement with a few static inlines and some
common code, than to try and replicate the x86 side hypercall_page
interface ?

~Andrew

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

Re: [Xen-devel] [PATCH] ARM: xen: unexport HYPERVISOR_platform_op function

Posted by Julien Grall 1 week ago
Hi Andrew,

On 9/6/19 6:20 PM, Andrew Cooper wrote:
> On 06/09/2019 17:00, Arnd Bergmann wrote:
>> On Fri, Sep 6, 2019 at 5:55 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>> On 06/09/2019 16:39, Arnd Bergmann wrote:
>>>> HYPERVISOR_platform_op() is an inline function and should not
>>>> be exported. Since commit 15bfc2348d54 ("modpost: check for
>>>> static EXPORT_SYMBOL* functions"), this causes a warning:
>>>>
>>>> WARNING: "HYPERVISOR_platform_op" [vmlinux] is a static EXPORT_SYMBOL_GPL
>>>>
>>>> Remove the extraneous export.
>>>>
>>>> Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>> Something is wonky.  That symbol is (/ really ought to be) in the
>>> hypercall page and most definitely not inline.
>>>
>>> Which tree is that changeset from?  I can't find the SHA.
>> This is from linux-next, I think from the kbuild tree.
> 
> Thanks.
> 
> Julien/Stefano: Why are any of these hypercalls out-of-line?  ARM
> doesn't use the hypercall page, and there is no argument translation
> (not even in arm32 as there are no 5-argument hypercalls declared).

I am not sure how the hypercall page makes things different. You still 
have to store the arguments in the correct register so...

> 
> They'd surely be easier to implement with a few static inlines and some
> common code, than to try and replicate the x86 side hypercall_page
> interface ?

... I don't think they will be easier to implement with a few static 
inlines. The implementation will likely end up to be similar to 
arch/x86/asm/xen/hypercall.h.

Furthermore, one of the downside of per-arch static inline is it is more 
difficult to ensure the prototype match for all the architectures. 
Although, it might be possible to make them common by only requesting 
per-arch to implement HYPERCALL_N(...).

So I think the code is better as it is.

While looking at the code, I also realized that the implementation of 
HYPERCALL_dm_op might be incorrect for Arm32. Similarly do privcmd call, 
I think dm_op call should enable user access as they will be used by 
userspace.

We don't use dm_op on Arm so far, hence why I think this was unnoticed. 
I will see if I can reproduce it and send a patch.

Cheers,

-- 
Julien Grall

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