Reworking our support for platform specific functions

[Christopher James Halse Rogers]

I, too, like the idea of a generalised approach. I just think that this 
is insufficiently generalised :).

We're talking about extensions here, but again we're talking about 
throwing void* around. It's entirely possible to add a mechanism for 
the platform (or other code) to register extra protobuf handlers.

I look forward to discussing this in Washington :)

On Tue, Oct 14, 2014 at 12:57 PM, Daniel van Vugt 
<daniel.van.vugt at canonical.com> wrote:
> Liking the idea of a generalised approach... Although on first glance 
> this does not sound like a real function name:
>    mir_connection_platform_operation()
> I know we want to be generic, but that sounds somehow overly generic 
> with "platform operation". Maybe we need a better function name 
> there. Not "ioctl" :)
> 
> Also, to generalise you would probably not use a parameter named 
> "DataAndFds" as "Fds" is too specific. The structure would need to 
> contain a size and pointer (void*) field, although they may as well 
> be parameters instead of a struct. And Protobuf could use opaque type 
> "bytes".
> 
> Finally instead of MesaPlatformOperationType, you could just make 
> "type" a UUID string. That has a few advantages:
>   1. Adding new (private-ish) operations does not require public 
> header changes.
>   2. An operation can be made portable and understood by multiple 
> platforms if need be.
>   3. Operation types can be removed and retired without lingering or 
> leaving a hole in any enum definition.
> 
> Then again, enum/ints are beautifully simple.
> 
> 
> On 13/10/14 19:37, Alexandros Frantzis wrote:
>> Hi all,
>> 
>> the need has arisen to add another platform-specific function, in 
>> order
>> to resolve bug LP: #1379266. This addition triggered some concerns 
>> about
>> the way we implement such functions.
>> 
>> The problem
>> -----------
>> 
>> In our current implementation, adding a platform-specific function
>> causes platform-specific details to leak into many layers from 
>> client to
>> server. For example, for the existing drm_auth_magic() call we have:
>> 
>> Client API: mir_connection_drm_auth_magic(int magic, ...)
>> Client    : MirConnection::drm_auth_magic(...)
>> Protobuf  : rpc drm_auth_magic(DRMMagic) returns (DRMAuthMagicStatus)
>> Server    : SessionMediator::drm_auth_magic(), mg::DRMAuthenticator
>> 
>> This approach has a few drawbacks:
>> 
>> 1. Platform-specific details leak in code that should be platform
>>     agnostic.
>> 2. It can't support platform-specific functions for 3rd party 
>> platforms.
>> 3. It's burdensome to add a new platform-specific functions (I guess
>>     this is a problem for our API functions in general).
>> 
>> Proposed solution
>> -----------------
>> 
>> To solve the issues mentioned above we need to provide a way to make 
>> an
>> opaque (as far as the rest of Mir is concerned) request to the 
>> platform:
>> 
>> Client API: mir_connection_platform_operation(type, DataAndFds 
>> request, ...);
>> Client    : MirConnection::platform_operation(type, ...)
>> Protobuf  : rpc platform_operation(type, DataAndFds) returns 
>> (DataAndFds)
>> Server    : SessionMediator::platform_operation()
>> Platform  : DataAndFds 
>> PlatformIpcOperations::platform_operation(type, DataAndFds)
>> 
>> Each platform should expose a header with its operation types and
>> expected parameters types. For mesa it could be something like:
>> 
>> enum MesaPlatformOperationType { drm_auth_magic, drm_get_auth_fd };
>> struct DRMAuthMagicRequest { int magic; }
>> struct DRMAuthMagicReply { int status; }
>> ...
>> 
>> And a request would be:
>> 
>> DRMAuthMagicRequest request = { magic };
>> 
>> void platform_operation_callback(
>>      MirConnection* connection, DataAndFds const* reply, void* 
>> context)
>> {
>>      struct DRMAuthMagicReply const* magic_reply = reply->data;
>>      ...
>> }
>> 
>> mir_connection_platform_operation(
>>      drm_auth_magic, request, platform_operation_callback);
>> 
>> With this approach:
>> 
>> 1. No platform-specific details leak into unrelated layers. Only the
>>     platform knows about these details and exposes them to other 
>> interested
>>     parties.
>> 
>> 2+3. It's trivial for 3rd party platforms to expose 
>> platform-specific operations
>> 
>> This proposal involves changes and ABI bumps to a few layers, so I 
>> would
>> appreciate some early feedback and discussion.
>> 
>> Thoughts?
>> 
>> Thanks,
>> Alexandros
>> 
> 
> --
> Mir-devel mailing list
> Mir-devel at lists.ubuntu.com
> Modify settings or unsubscribe at: 
> https://lists.ubuntu.com/mailman/listinfo/mir-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/mir-devel/attachments/20141014/7380d445/attachment.html>