[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH Remus v2 04/10] tools/libxc: introduce DECLARE_HYPERCALL_BUFFER_USER_POINTER

On 05/12/2015 04:19 PM, Ian Campbell wrote:
On Tue, 2015-05-12 at 15:18 +0800, Yang Hongyang wrote:

On 05/11/2015 07:53 PM, Ian Campbell wrote:
On Fri, 2015-05-08 at 17:33 +0800, Yang Hongyang wrote:
Define a user pointer that can access the hypercall buffer data
Useful when you only need to access the hypercall buffer data

Can you expand on this please, I think the context is a hypercall buffer
passed as an argument or as a member of a struct. Please describe why
are not usable here.

There are cases that we only need to use the hypercall buffer data, and do
not use the xc_hypercall_buffer_t struct.

DECLARE_HYPERCALL_BUFFER_ARGUMENT will only define a xc_hypercall_buffer_t,
while DECLARE_HYPERCALL_BUFFER_SHADOW do define a user pointer that can allow
us to access the hypercall buffer data but it also define a
xc_hypercall_buffer_t that we don't use, the compiler will report arg unused

The cases for example:
In send_all_pages(), we only need to use the haypercall buffer data which is a
dirty bitmap, we set the dirty bitmap to all dirty and call send_dirty_pages,
we will not use the xc_hypercall_buffer_t and hypercall to retrive the dirty
In send_some_pages(), we will also only need to use the dirty_bitmap. the
retrive dirty bitmap hypercall are done by the caller.

Thanks. Please include the bulk of this description in the commit

However, perhaps we should just mark the xc_hypercall_buffer_t as
potentially unused, by tagging it with  __attribute__((unused)), in some
or all of the DECLARE_HYPERCALL_* variants. Then I think you could use

If __attribute__((unused)) won't cause other problems here, I also prefer
to add this to DECLARE_HYPERCALL_BUFFER_SHADOW instead of create another
macro, I think only DECLARE_HYPERCALL_BUFFER_SHADOW may need to set this
attribute because this is a shadow, we might not use the xc_hypercall_buffer_t
which already allocated.

My argument here would be that the xc_hypercall_buffer_t created here is
a hidden part of the internal hcall buf infrastructure and should be
transparent to the user's code. The user's code only interacts with it
via the magic macros which take the "user pointer" name, not the mangled
xc_hypercall_buffer_t's name.

I'm wary of creating too many accessors macros, or of creating too many
back doors around the attempts to steer code away from using regular
memory to pass to hypercalls.




Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.