<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jul 29, 2019 at 9:58 AM Eugene Syromiatnikov <<a href="mailto:esyr@redhat.com" target="_blank">esyr@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This is just a cursory review, since I haven't looked at in-kernel<br>
DRM ioctl implementation yet.<br>
<br>
On Sat, Jul 27, 2019 at 11:57:16PM +0800, Zhibin Li wrote:<br>
<br>
> (drm_version, drm_unique, drm_get_map, drm_get_client, drm_get_stats,<br>
> drm_add_map, drm_add_bufs, drm_mark_bufs, drm_info_bufs, drm_map_bufs,<br>
> drm_free_bufs, drm_rm_map, drm_sarea_ctx, drm_res_ctx, drm_agp_enable,<br>
> drm_agp_info, drm_agp_alloc, drm_agp_free, drm_mode_get_connector,<br>
> drm_scatter_gather, drm_wait_vblank, drm_mode_getplaneresources,<br>
> drm_mode_add_fb2, drm_mode_obj_getproperties, drm_mode_obj_setproperty<br>
> drm_agp_bind): New functions.<br>
> (drm_ioctl_mpers): Use them.<br>
<br>
I don't thing the list of functions is required for a new file GNU-style<br>
changelog entry. </blockquote><div> </div><div>May I ask when this is changed? I guess I'm not following up. </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> (print_drm_iowr, drm_decode_number, drm_get_magic, drm_irq_busid,<br>
> drm_set_version, drm_modeset_ctl, drm_gem_close, drm_gem_flink,<br>
> drm_gem_open, drm_get_cap, drm_set_client_cap, drm_auth_magic,<br>
> drm_noop, drm_control, drm_ctx, drm_rm_ctx, drm_get_ctx, drm_lock,<br>
> drm_prime_handle_to_fd, drm_prime_fd_to_handle, drm_crtc_get_sequence,<br>
> drm_crtc_queue_sequence, drm_mode_get_resources, drm_mode_crtc,<br>
> drm_mode_print_modeinfo, drm_mode_cursor, drm_mode_get_encoder,<br>
> drm_mode_gamma, drm_mode_get_property, drm_mode_set_property,<br>
> drm_mode_get_prop_blob, drm_mode_get_fb, drm_mode_add_fb,<br>
> drm_mode_rm_fb, drm_mode_page_flip, drm_mode_create_dumb,<br>
> drm_mode_dirty_fb, drm_mode_map_dumb, drm_mode_destroy_dumb,<br>
> drm_mode_getplane, drm_mode_setplane, drm_mode_cursor2,<br>
> drm_mode_atomic, drm_mode_createpropblob, drm_mode_destroypropblob,<br>
> drm_syncobj_create, drm_syncobj_destroy, drm_syncobj_handle_fd,<br>
> drm_syncobj_wait, drm_syncobj_reset_or_signal, drm_mode_create_lease,<br>
> drm_mode_list_lessees, drm_mode_get_lease, drm_mode_revoke_lease,<br>
> drm_syncobj_timeline_wait, drm_syncobj_query_or_timeline_signal,<br>
> drm_syncobj_transfer): New functions.<br>
> (drm_ioctl): Use them and use drm_ioctl_mpers.<br>
<br>
Ditto.<br>
<br>
> --- /dev/null<br>
> +++ b/drm.c<br>
> @@ -0,0 +1,1531 @@<br>
> +/*<br>
> + * Copyright (c) 2019 Patrik Jakobsson <<a href="mailto:pjakobsson@suse.de" target="_blank">pjakobsson@suse.de</a>><br>
The copyright of the original patch dates back to (at least) 2015[1],<br>
and it was claimed by Intel back then.<br>
<br>
Your copyright attribution is also missing (adding generic "The strace<br>
developers" attribution would be sufficient as well).<br>
<br>
[1] <a href="https://lists.freedesktop.org/archives/intel-gfx/2015-August/074252.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/archives/intel-gfx/2015-August/074252.html</a></blockquote><div><br></div><div>Sure. Will do.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + *<br>
> + * SPDX-License-Identifier: LGPL-2.1-or-later<br>
> + */<br>
> +<br>
> +#include "defs.h"<br>
> +#include "print_fields.h"<br>
> +<br>
> +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H)<br>
> +<br>
<br>
> +# ifdef HAVE_DRM_H<br>
> +#  include <drm.h><br>
> +# else<br>
> +#  include <drm/drm.h><br>
> +# endif<br>
I wonder what happens if neither of those is found. </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Note also that this file can be rather old and do not contain all the<br>
ioctl constants, so it is likely also worth having an xlat with all<br>
related DRM_IOC constants. <br></blockquote><div><br></div><div>OK, I will give it a try. And does this also mean those ifdefs around </div><div>ioctl constants which you said are unreliable could be removed </div><div>because all DRM_IOC constants can be found in this case?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +<br>
> +static void<br>
> +print_drm_iowr(const unsigned int nr, const unsigned int size,<br>
> +            const char *str)<br>
> +{<br>
> +     tprintf("DRM_IOWR(%#x, %#x) /* %s */", nr, size, str);<br>
This likely needs xlat style support.<br></blockquote><div><br></div><div> Any example or reference?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +}<br>
> +<br>
> +int<br>
> +drm_decode_number(struct tcb *const tcp, const unsigned int code)<br>
> +{<br>
> +     const unsigned int nr = _IOC_NR(code);<br>
> +<br>
> +     if (_IOC_DIR(code) == (_IOC_READ | _IOC_WRITE)) {<br>
> +             switch (nr) {<br>
<br>
> +                     case 0xa7:<br>
Unnecessary indentation.<br></blockquote><div><br></div><div>Got it.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +                             /* In Linux commit v3.12-rc7~26^2~2 a u32 padding was added */<br>
> +                             /* to struct drm_mode_get_connector so in old kernel headers */<br>
> +                             /* the size of this structure is 0x4c instead of 0x50. */<br>
drm argument sizes are constantly changing (please take a look at Gleb's<br>
ioctl-related commits, they can provide a rough impression; I'm not sure<br>
to which extent it is applicable to core DRM ioctls, however), a more<br>
robust way of decoding drm ioctl number have to be implemented.<br>
<br></blockquote><div>Yes I've seen Gleb updating ioctl entries according to the kernel release.</div><div>The only thing that is changing is the size. Would _IOC_NR() be enough?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Considering the code below, it may be the case for<br>
DRM_IOCTL_MODE_GETPLANERESOURCES, DRM_IOCTL_MODE_OBJ_GETPROPERTIES,<br>
and DRM_IOCTL_MODE_OBJ_SETPROPERTY as well.<br>
<br></blockquote><div> </div><div>It seems the corresponding structure of these three ioctl constants are not changing.</div><div>The sizes of them are different among personalities because of byte-alignment. I will</div><div>check git history to confirm that.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +                             if (_IOC_SIZE(code) == 0x4c) {<br>
> +                                     print_drm_iowr(nr, _IOC_SIZE(code),<br>
> +                                                    "DRM_IOCTL_MODE_GETCONNECTOR");<br>
> +                                     return IOCTL_NUMBER_STOP_LOOKUP;<br>
> +                             } else {<br>
> +                                     return 0;<br>
> +                             }<br>
> +             }<br>
> +     }<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_get_magic(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_auth auth;<br>
> +<br>
> +     if (exiting(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &auth))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +<br>
> +             PRINT_FIELD_U("{", auth, magic);<br>
> +             tprints("}");<br>
> +<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     }<br>
> +<br>
> +     return 0;<br>
<br>
<br>
I'd rewrite exiting-only decoding as<br>
<br>
        if (entering(tcp)) {<br>
                tprints(", ");<br>
                return 0;<br>
        }<br>
<br>
        /* exiting-only code */<br>
<br>
As it allows remove one indentation level. It's slightly less readable,<br>
though, but it is also common enough among strace decoders to sacrifice<br>
a bit of readability.<br>
<br>
The other thing is printing comma on exiting vs. on entering elsewhere,<br>
which is somewhat inconsistent.<br>
<br>
> +}<br>
<br>
> +# include "xlat/drm_modeset_cmd.h"<br>
I'd prefer to have all the includes at the beginning of the file, it<br>
simplifies maintenance of the code in the long run.<br></blockquote><div><br></div><div>OK. I will move them all to the beginning right away. The code below</div><div>is similar so I rather not to reply all the "inline #include" one by one.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +# include "xlat/drm_capability.h"<br>
Ditto.<br>
<br>
> +<br>
> +static int<br>
> +drm_get_cap(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_get_cap cap;<br>
> +<br>
> +     if (entering(tcp)) {<br>
<br>
> +             tprints(", ");<br>
Okay, now I feel that this comma printing can well be re-factored out of<br>
the specific argument printing functions; moreover, that would make<br>
these printing functions structure printing funtions, ergo more<br>
re-usable (in case printing of the same structures will be needed<br>
somewhere else in the code).<br>
<br>
> +             if (umove_or_printaddr(tcp, arg, &cap))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
<br>
> +             tprints("{capability=");<br>
> +             printxval(drm_capability, cap.capability, "DRM_CAP_???");<br>
Why not "PRINT_FIELD_XVAL("{", cap, capability, drm_capability, "DRM_CAP_???")"?<br>
<br></blockquote><div> </div><div>Sorry I didn't notice there is a PRINT_FIELD_* macro for this.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &cap)) {<br>
Adding a separate umove_no_syserror() macro might be a good idea.<br>
<br></blockquote><div> </div><div>You mean umove_nor_syserror() ? Should this macro be added to defs.h</div><div>or just in this file scope? I see some other code uses this, too.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +             PRINT_FIELD_U(", ", cap, value);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
<br>
> +# include "xlat/drm_client_capability.h"<br>
Inline include.<br>
<br>
> +<br>
> +static int<br>
> +drm_set_client_cap(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_set_client_cap cap;<br>
> +<br>
> +     tprints(", ");<br>
<br>
> +     if (!umove_or_printaddr(tcp, arg, &cap)) {<br>
<br>
I'd stick to<br>
<br>
        if (umove_or_printaddr(tcp, arg, &cap))<br>
                return RVAL_IOCTL_DECODED;<br>
<br>
        /* umove success path */<br>
<br>
Again, for the sake of reducing indentation.<br></blockquote><div><br></div><div>Sure, will follow this style in my code.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             tprints("{capability=");<br>
> +             printxval(drm_client_capability, cap.capability, "DRM_CLIENT_CAP_???");<br>
Why not PRINT_FIELD_XVAL?<br>
<br>
> +             PRINT_FIELD_U(", ", cap, value);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_auth_magic(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_auth auth;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &auth)) {<br>
> +             PRINT_FIELD_U("{", auth, magic);<br>
I suspect that "magic" is expected to be hexadecimal.<br></blockquote><div><br></div><div>I see that in the kernel code drivers/gpu/drm/drm_auth.c, there is a</div><div>DRM_DEBUG() invocation which prints magic using "%u" so I simply</div><div>follow that. Should magic numbers always be hexadecimal?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
<br>
> +# include "xlat/drm_control_func.h"<br>
Inline include.<br>
<br>
> +<br>
> +static int<br>
> +drm_control(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_control control;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &control)) {<br>
<br>
> +             tprints("{func=");<br>
> +             printxval(drm_control_func, control.func, "DRM_???");<br>
Why not PRINT_FIELD_XVAL?<br>
<br>
> +             PRINT_FIELD_D(", ", control, irq);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_ctx(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_ctx ctx;<br>
> +<br>
> +     if (exiting(tcp)) {<br>
Exiting-only decoding.<br>
<br>
> +             tprints(", ");<br>
Comma on exiting.<br>
<br>
> +             if (umove_or_printaddr(tcp, arg, &ctx))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", ctx, handle);<br>
Are handles sequential or random in nature? In the latter case, please<br>
consider hexadecimal output format.<br>
<br>
> +             tprints("}");<br>
<br>
This code can be factored out into a separate function that is called by<br>
drm_ctx/drm_rm_ctx (and optionally drm_get_ctx).<br></blockquote><div><br></div><div>Sure, will check it later.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     }<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_rm_ctx(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_ctx ctx;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
<br>
> +             if (umove_or_printaddr(tcp, arg, &ctx))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", ctx, handle);<br>
> +             tprints("}");<br>
Can be factored out.<br></blockquote><div><br></div><div>Sorry for so many redundant codes. This week I will spend part of</div><div>my time re-factoring codes.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +# include "xlat/drm_ctx_flags.h"<br>
> +<br>
> +static int<br>
> +drm_get_ctx(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_ctx ctx;<br>
> +<br>
> +     if (exiting(tcp)) {<br>
Exiting-only decoding.<br>
<br>
> +             tprints(", ");<br>
Comma on exiting.<br>
<br>
> +             if (umove_or_printaddr(tcp, arg, &ctx))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
<br>
> +             tprints("{flags=");<br>
> +             printxval(drm_ctx_flags, ctx.flags, "_DRM_CONTEXT_???");<br>
Why printxval and not printflags?<br>
<br>
Why not PRINT_FIELD_FLAGS?<br></blockquote><div><br></div><div>Well that's what I was confused with. I thought printxval is enough for all</div><div>those printing so I didn't take a look into printflags. Obviously I was wrong</div><div>and I should check all the flags in the code and use PRINT_FIELD_FLAGS</div><div>as you mentioned a lot of them in the comments. I'd not to bother replying</div><div>all of them because they are similar and I've already mark them down for</div><div>revision.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Why handle field is not printed here? I suspect it has to be printed on<br>
entering, hasn't it?<br></blockquote><div><br></div><div>I took reference from the updated kernel code. In</div><div>drivers/gpu/drm/drm_context.c: drm_legacy_getctx(), I couldn't see any use</div><div>of handle field and the flags is also marked to zero. Actually I'm not sure what</div><div>is going on here. Any thoughts?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             tprints("}");<br>
> +<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     }<br>
> +<br>
> +     return 0;<br>
> +}<br>
<br>
> +# include "xlat/drm_lock_flags.h"<br>
Inline include.<br>
<br>
> +static int<br>
> +drm_lock(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_lock lock;<br>
> +<br>
> +     if (exiting(tcp)) {<br>
Exiting-only decoding.<br>
<br>
> +             tprints(", ");<br>
Comma on exiting.<br>
<br>
> +             if (umove_or_printaddr(tcp, arg, &lock))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_D("{", lock, context);<br>
<br>
> +             tprints(", flags=");<br>
> +             printxval(drm_lock_flags, lock.flags, "_DRM_LOCK_???");<br>
Why printxval and not printflags?<br>
<br>
Why not PRINT_FIELD_XVAL?<br>
<br>
> +             tprints("}");<br>
> +<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     }<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_prime_handle_to_fd(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_prime_handle handle;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &handle))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", handle, handle);<br>
<br>
> +             PRINT_FIELD_X(", ", handle, flags);<br>
The flags field can have DRM_CLOEXEC and DRM_RDWR flags.<br></blockquote><div><br></div><div>OK. Seems I've missed some of the flags decoding.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &handle)) {<br>
<br>
> +             PRINT_FIELD_U(", ", handle, fd);<br>
Why not PRINT_FIELD_FD?<br></blockquote><div><br></div><div>Will use PRINT_FIELD_FD.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_prime_fd_to_handle(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_prime_handle handle;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &handle))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
<br>
> +             PRINT_FIELD_U("{", handle, fd);<br>
Why not PRINT_FIELD_FD?<br>
<br>
This also makes order of printing different comparing to order of fields<br>
in the structure.<br></blockquote><div><br></div><div>It seems this one is using the fd the get a handle, shouldn't it be printed in such order?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &handle)) {<br>
<br>
> +             PRINT_FIELD_U(", ", handle, handle);<br>
I feel that drm_handle_t likely needs a separate PRINT_FIELD_* macro.<br></blockquote><div><br></div><div>You mean like others in print_fields.h?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +<br>
> +}<br>
> +<br>
<br>
> +# ifdef DRM_IOCTL_CRTC_GET_SEQUENCE<br>
Ugh, those ifdef's make strace behaviour depend on the version of drm<br>
headers it is built against, which is quite unreliable.<br></blockquote><div><br></div><div>That's true. Will adding xlat for all of the DRM_IOC constants avoid this?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +static int<br>
> +drm_crtc_get_sequence(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_crtc_get_sequence seq;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &seq))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", seq, crtc_id);<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &seq)) {<br>
> +             PRINT_FIELD_U(", ", seq, active);<br>
> +             PRINT_FIELD_U(", ", seq, sequence);<br>
> +             PRINT_FIELD_D(", ", seq, sequence_ns);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_CRTC_QUEUE_SEQUENCE<br>
> +<br>
> +# include "xlat/drm_crtc_sequence_flags.h"<br>
> +<br>
> +static int<br>
> +drm_crtc_queue_sequence(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_crtc_queue_sequence seq;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +<br>
> +     if (umove_or_printaddr(tcp, arg, &seq))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     tprints("{");<br>
It's probably better to put this opening brace printing into respective<br>
PRINT_FIELD_* macros.<br></blockquote><div><br></div><div>Indeed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +     if (entering(tcp)) {<br>
> +             PRINT_FIELD_U("", seq, crtc_id);<br>
<br>
> +             tprints(", flags=");<br>
> +             printxval(drm_crtc_sequence_flags, seq.flags, "DRM_CRTC_SEQUENCE_???");<br>
Why printxval and not printflags?<br>
<br>
Why not PRINT_FIELD_XVAL?<br>
<br>
> +             PRINT_FIELD_U(", ", seq, user_data);<br>
user_data is opaque and likely has to be printed as hexadecimal.<br></blockquote><div><br></div><div>OK.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             PRINT_FIELD_U(", ", seq, sequence);<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
<br>
> +     PRINT_FIELD_U("", seq, sequence);<br>
... or at least put this into "else" block, just to improve readability a bit.<br></blockquote><div><br></div><div>There are some other similar parts in the code. Should I use else block</div><div>for all of them or just because this is only one line?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +static int<br>
> +drm_mode_get_resources(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_card_res res;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &res))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
<br>
> +             PRINT_FIELD_PTR("{", res, fb_id_ptr);<br>
> +             PRINT_FIELD_PTR(", ", res, crtc_id_ptr);<br>
> +             PRINT_FIELD_PTR(", ", res, connector_id_ptr);<br>
> +             PRINT_FIELD_PTR(", ", res, encoder_id_ptr);<br>
Why PRINT_FIELD_PTR if this is a non-mpers ioctl? Should PRINT_FIELD_ADDR<br>
or PRINT_FIELD_ADDR64 be used, for example?<br></blockquote><div><br></div><div>Oh sorry. I didn't notice that there is a mpers_ptr_t type casting in</div><div>PRINT_FIELD_PTR and I just simply use it as its name imply.</div><div>BTW, I think a short comment or description of this PRINT_FIELD_*</div><div>part would make it more clear considering some of them are similar.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Those are arrays of count_* elements (which are R/W, by the way), they likely<br>
has to be treated as such.<br></blockquote><div><br></div><div>Directly print all the array elements, you mean? Any elegant way</div><div>to do this? I mean putting a lot of for loop makes a lot of redundant</div><div>code. Will a macro or function be better?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &res)) {<br>
> +             PRINT_FIELD_U(", ", res, count_fbs);<br>
> +             PRINT_FIELD_U(", ", res, count_crtcs);<br>
> +             PRINT_FIELD_U(", ", res, count_connectors);<br>
> +             PRINT_FIELD_U(", ", res, count_encoders);<br>
> +             PRINT_FIELD_U(", ", res, min_width);<br>
> +             PRINT_FIELD_U(", ", res, max_width);<br>
> +             PRINT_FIELD_U(", ", res, min_height);<br>
> +             PRINT_FIELD_U(", ", res, max_height);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +# include "xlat/drm_mode_type.h"<br>
> +# include "xlat/drm_mode_flags.h"<br>
Inline include.<br>
<br>
> +<br>
> +static void<br>
> +drm_mode_print_modeinfo(struct drm_mode_modeinfo *info)<br>
> +{<br>
Why curly braces are not printed here?<br></blockquote><div><br></div><div>They are printed in the caller of this function but seems printing</div><div>them here is better. Will change this.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +     PRINT_FIELD_U("", *info, clock);<br>
> +     PRINT_FIELD_U(", ", *info, hdisplay);<br>
> +     PRINT_FIELD_U(", ", *info, hsync_start);<br>
> +     PRINT_FIELD_U(", ", *info, hsync_end);<br>
> +     PRINT_FIELD_U(", ", *info, htotal);<br>
> +     PRINT_FIELD_U(", ", *info, hskew);<br>
> +     PRINT_FIELD_U(", ", *info, vdisplay);<br>
> +     PRINT_FIELD_U(", ", *info, vsync_start);<br>
> +     PRINT_FIELD_U(", ", *info, vsync_end);<br>
> +     PRINT_FIELD_U(", ", *info, vtotal);<br>
> +     PRINT_FIELD_U(", ", *info, vscan);<br>
> +     PRINT_FIELD_U(", ", *info, vrefresh);<br>
<br>
> +     tprints(", flags=");<br>
> +     printxval(drm_mode_flags, info->flags, "DRM_MODE_FLAG_PIC_???");<br>
Why printxval and not printflags?<br>
<br>
Why not PRINT_FIELD_FLAGS?<br>
<br>
> +     tprints(", type=");<br>
> +     printxval(drm_mode_type, info->type, "DRM_MODE_TYPE_???");<br>
Why not PRINT_FIELD_XVAL?<br>
<br>
> +     PRINT_FIELD_CSTRING(", ", *info, name);<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_crtc(struct tcb *const tcp, const kernel_ulong_t arg,<br>
> +               bool is_get)<br>
> +{<br>
> +     struct drm_mode_crtc crtc;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &crtc))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", crtc, crtc_id);<br>
> +             if (is_get)<br>
> +                     return 0;<br>
> +             PRINT_FIELD_PTR(", ", crtc, set_connectors_ptr);<br>
Why not PRINT_FIELD_ADDR64?<br>
<br>
It is treated as a pointer to u32 array containing count_connectors<br>
elements in kernel, why not printing it like that?<br>
<br>
> +             PRINT_FIELD_U(", ", crtc, count_connectors);<br>
This one is R/W.<br></blockquote><div><br></div><div>Oh my fault. As I see, most of the count_* fields are R/W and</div><div>that's what I missed. Will double-check them.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             PRINT_FIELD_U(", ", crtc, fb_id);<br>
> +             PRINT_FIELD_U(", ", crtc, x);<br>
> +             PRINT_FIELD_U(", ", crtc, y);<br>
> +             PRINT_FIELD_U(", ", crtc, gamma_size);<br>
> +             PRINT_FIELD_U(", ", crtc, mode_valid);<br>
> +             tprints(", mode={");<br>
> +<br>
> +             drm_mode_print_modeinfo(&crtc.mode);<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &crtc)) {<br>
Why not check for "is_get" first here and avoid nested "if" at all?<br></blockquote><div><br></div><div>OK.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +             if (is_get) {<br>
<br>
> +                     PRINT_FIELD_PTR(", ", crtc, set_connectors_ptr);<br>
Why not PRINT_FIELD_ADDR64?<br>
<br>
It is treated as a pointer to u32 array containing count_connectors<br>
elements in kernel, why not printing it like that?<br>
<br>
> +                     PRINT_FIELD_U(", ", crtc, count_connectors);<br>
> +                     PRINT_FIELD_U(", ", crtc, fb_id);<br>
> +                     PRINT_FIELD_U(", ", crtc, x);<br>
> +                     PRINT_FIELD_U(", ", crtc, y);<br>
> +                     PRINT_FIELD_U(", ", crtc, gamma_size);<br>
> +                     PRINT_FIELD_U(", ", crtc, mode_valid);<br>
> +                     tprints(", mode={");<br>
> +<br>
> +                     drm_mode_print_modeinfo(&crtc.mode);<br>
> +                     tprints("}");<br>
This code looks the same for entering and exiting, it probably can be factored<br>
out in a separate function (print_drm_mode_crtc_tail or something like<br>
that).<br></blockquote><div><br></div><div>OK will try.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             }<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_cursor(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_cursor cursor;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &cursor))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +<br>
> +             PRINT_FIELD_X("{", cursor, flags);<br>
It can have DRM_MODE_CURSOR_BO and DRM_MODE_CURSOR_MOVE. <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +             PRINT_FIELD_U(", ", cursor, crtc_id);<br>
I feel like crtc_id needs a separate PRINT_FIELD_* macro.</blockquote><div><br></div><div>OK.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +             PRINT_FIELD_D(", ", cursor, x);<br>
> +             PRINT_FIELD_D(", ", cursor, y);<br>
> +             PRINT_FIELD_U(", ", cursor, width);<br>
> +             PRINT_FIELD_U(", ", cursor, height);<br>
> +             PRINT_FIELD_U(", ", cursor, handle);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_gamma(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_crtc_lut lut;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &lut))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +<br>
> +             /* We don't print the entire table, just the pointers */<br>
> +             PRINT_FIELD_U("{", lut, crtc_id);<br>
> +             PRINT_FIELD_U(", ", lut, gamma_size);<br>
<br>
> +             PRINT_FIELD_PTR(", ", lut, red);<br>
> +             PRINT_FIELD_PTR(", ", lut, green);<br>
> +             PRINT_FIELD_PTR(", ", lut, blue);<br>
Why not PRINT_FIELD_ADDR/PRINT_FIELD_ADDR64? And why not retrieve and<br>
print these arrays?<br></blockquote><div><br></div><div>OK will do.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +# include "xlat/drm_mode_encoder_type.h"<br>
Inline include.<br>
<br>
> +<br>
> +static int<br>
> +drm_mode_get_encoder(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_get_encoder enc;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +<br>
> +     if (umove_or_printaddr(tcp, arg, &enc))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     PRINT_FIELD_U("{", enc, encoder_id);<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
<br>
> +     tprints(", encoder_type=");<br>
> +     printxval(drm_mode_encoder_type, enc.encoder_type, "DRM_MODE_ENCODER_???");<br>
Why not PRINT_FIELD_XVAL?<br>
<br>
> +     PRINT_FIELD_U(", ", enc, crtc_id);<br>
> +     PRINT_FIELD_X(", ", enc, possible_crtcs);<br>
> +     PRINT_FIELD_X(", ", enc, possible_clones);<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_get_property(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_get_property prop;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &prop))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", prop, prop_id);<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &prop)) {<br>
<br>
> +             PRINT_FIELD_PTR(", ", prop, values_ptr);<br>
> +             PRINT_FIELD_PTR(", ", prop, enum_blob_ptr);<br>
Why not PRINT_FIELD_ADDR/PRINT_FIELD_ADDR64?<br>
<br>
enum_blob_ptr is an array of count_enum_blobs elements of struct<br>
drm_mode_property_enum type, why not print it as such?<br>
<br>
> +             PRINT_FIELD_X(", ", prop, flags);<br>
This can have DRM_MODE_PROP_PENDING, DRM_MODE_PROP_RANGE,<br>
DRM_MODE_PROP_IMMUTABLE, DRM_MODE_PROP_ENUM, DRM_MODE_PROP_BLOB,<br>
DRM_MODE_PROP_BITMASK as flags, and DRM_MODE_PROP_EXTENDED_TYPE in<br>
higher bits.<br>
<br></blockquote><div><br></div><div>Seems I've missed a big part.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +             PRINT_FIELD_CSTRING(", ", prop, name);<br>
<br>
> +             PRINT_FIELD_U(", ", prop, count_values);<br>
> +             PRINT_FIELD_U(", ", prop, count_enum_blobs);<br>
These are R/W.<br>
<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_set_property(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_connector_set_property prop;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &prop)) {<br>
> +             PRINT_FIELD_U("{", prop, value);<br>
> +             PRINT_FIELD_U(", ", prop, prop_id);<br>
> +             PRINT_FIELD_U(", ", prop, connector_id);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_get_prop_blob(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_get_blob blob;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &blob))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", blob, blob_id);<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &blob)) {<br>
<br>
> +             PRINT_FIELD_U(", ", blob, length);<br>
This one is R/W.<br>
<br>
> +             PRINT_FIELD_U(", ", blob, data);<br>
Data is a pointer to blob, it likely has to be printed using printstr<br>
with appropriate flags.<br></blockquote><div><br></div><div>Will try it later.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_get_fb(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_fb_cmd cmd;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &cmd))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", cmd, fb_id);<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &cmd)) {<br>
<br>
> +             PRINT_FIELD_U(", ", cmd, width);<br>
> +             PRINT_FIELD_U(", ", cmd, height);<br>
> +             PRINT_FIELD_U(", ", cmd, pitch);<br>
> +             PRINT_FIELD_U(", ", cmd, bpp);<br>
> +             PRINT_FIELD_U(", ", cmd, depth);<br>
> +             PRINT_FIELD_U(", ", cmd, handle);<br>
This code is duplicated between drm_mode_get_fb and drm_mode_add_fb,<br>
it can be factored out in a separate function.<br></blockquote><div><br></div><div>OK.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_add_fb(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_fb_cmd cmd;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &cmd))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", cmd, width);<br>
> +             PRINT_FIELD_U(", ", cmd, height);<br>
> +             PRINT_FIELD_U(", ", cmd, pitch);<br>
> +             PRINT_FIELD_U(", ", cmd, bpp);<br>
> +             PRINT_FIELD_U(", ", cmd, depth);<br>
> +             PRINT_FIELD_U(", ", cmd, handle);<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove_or_printaddr(tcp, arg, &cmd)) {<br>
> +             PRINT_FIELD_U(", ", cmd, fb_id);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_rm_fb(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     unsigned int handle;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &handle))<br>
> +             tprintf("%u", handle);<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +# include "xlat/drm_mode_page_flip_flags.h"<br>
Inline include.<br>
<br>
> +<br>
> +static int<br>
> +drm_mode_page_flip(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_crtc_page_flip flip;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &flip)) {<br>
> +             PRINT_FIELD_U("{", flip, crtc_id);<br>
> +             PRINT_FIELD_U(", ", flip, fb_id);<br>
<br>
> +             tprints(", flags=");<br>
> +             printxval(drm_mode_page_flip_flags, flip.flags, "DRM_MODE_PAGE_FLIP_???");<br>
Why printxval and not printflags?<br>
<br>
Why not PRINT_FIELD_FLAGS?<br>
<br>
> +             PRINT_FIELD_X(", ", flip, user_data);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_dirty_fb(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_fb_dirty_cmd cmd;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &cmd)) {<br>
> +             PRINT_FIELD_U("{", cmd, fb_id);<br>
<br>
> +             PRINT_FIELD_X(", ", cmd, flags);<br>
This one can have DRM_MODE_FB_DIRTY_ANNOTATE_COPY and<br>
DRM_MODE_FB_DIRTY_ANNOTATE_FILL as flags.<br>
<br>
> +             PRINT_FIELD_X(", ", cmd, color);<br>
> +             PRINT_FIELD_U(", ", cmd, num_clips);<br>
<br>
> +             PRINT_FIELD_PTR(", ", cmd, clips_ptr);<br>
Why not PRINT_FIELD_ADDr/PRINT_FIELD_ADDR64?<br>
<br>
clips_ptr is a pointer to an array of num_clips elements of struct drm_clip_rect<br>
type, why not print it as such?<br>
<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_destroy_dumb(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_destroy_dumb dumb;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &dumb)) {<br>
> +             PRINT_FIELD_U("{", dumb, handle);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_getplane(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_get_plane plane;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +<br>
> +     if (umove_or_printaddr(tcp, arg, &plane))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     tprints("{");<br>
> +     if (entering(tcp)) {<br>
> +             PRINT_FIELD_U("", plane, plane_id);<br>
<br>
> +             PRINT_FIELD_PTR(", ", plane, format_type_ptr);<br>
Why not PRINT_FIELD_ADDR64?<br>
<br>
This is a pointer to array of count_format_types elements of type<br>
uint32_t, and likely has to be treated as such.<br>
<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
<br>
> +     PRINT_FIELD_PTR("", plane, format_type_ptr);<br>
Why not PRINT_FIELD_ADDR64?<br>
<br>
This is a pointer to array of count_format_types elements of type<br>
uint32_t, and likely has to be treated as such.<br>
<br>
> +     PRINT_FIELD_U(", ", plane, crtc_id);<br>
> +     PRINT_FIELD_U(", ", plane, fb_id);<br>
> +     PRINT_FIELD_U(", ", plane, possible_crtcs);<br>
> +     PRINT_FIELD_U(", ", plane, gamma_size);<br>
<br>
> +     PRINT_FIELD_U(", ", plane, count_format_types);<br>
This one is R/W.<br>
<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_setplane(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_set_plane plane;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &plane))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", plane, plane_id);<br>
> +             PRINT_FIELD_U(", ", plane, crtc_id);<br>
> +             PRINT_FIELD_U(", ", plane, fb_id);<br>
<br>
> +             PRINT_FIELD_X(", ", plane, flags);<br>
Seems like it can have DRM_MODE_PRESENT_TOP_FIELD and<br>
DRM_MODE_PRESENT_BOTTOM_FIELD flags.<br>
<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &plane)) {<br>
> +             PRINT_FIELD_D(", ", plane, crtc_x);<br>
> +             PRINT_FIELD_D(", ", plane, crtc_y);<br>
<br>
> +             PRINT_FIELD_D(", ", plane, crtc_w);<br>
> +             PRINT_FIELD_D(", ", plane, crtc_h);<br>
These are unsigned.</blockquote><div><br></div><div>My fault.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +             PRINT_FIELD_D(", ", plane, src_x);<br>
> +             PRINT_FIELD_D(", ", plane, src_y);<br>
> +             PRINT_FIELD_D(", ", plane, src_h);<br>
> +             PRINT_FIELD_D(", ", plane, src_w);<br>
These are unsigned and are in 16.16 fixed point format.</blockquote><div><br></div><div>Well this is new to me. I should search for information.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_cursor2(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_cursor2 cursor;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &cursor))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +<br>
> +             PRINT_FIELD_X("{", cursor, flags);<br>
This one employs the same DRM_MODE_CURSOR_BO and DRM_MODE_CURSOR_MOVE<br>
flags as the flags field in drm_mode_cursor. <br>
<br>
> +             PRINT_FIELD_U(", ", cursor, crtc_id);<br>
> +             PRINT_FIELD_D(", ", cursor, x);<br>
> +             PRINT_FIELD_D(", ", cursor, y);<br>
> +             PRINT_FIELD_U(", ", cursor, width);<br>
> +             PRINT_FIELD_U(", ", cursor, height);<br>
> +             PRINT_FIELD_U(", ", cursor, handle);<br>
> +             PRINT_FIELD_D(", ", cursor, hot_x);<br>
> +             PRINT_FIELD_D(", ", cursor, hot_y);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_atomic(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_atomic atomic;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &atomic)) {<br>
> +             PRINT_FIELD_X("{", atomic, flags);<br>
> +             PRINT_FIELD_U(", ", atomic, count_objs);<br>
<br>
> +             PRINT_FIELD_PTR(", ", atomic, objs_ptr);<br>
> +             PRINT_FIELD_PTR(", ", atomic, count_props_ptr);<br>
> +             PRINT_FIELD_PTR(", ", atomic, props_ptr);<br>
> +             PRINT_FIELD_PTR(", ", atomic, prop_values_ptr);<br>
Why not PRINT_FIELD_ADDR64?<br>
<br>
Those are arrays and likely hast to be treated as such.<br>
<br>
> +             PRINT_FIELD_U(", ", atomic, reserved);<br>
It usually makes sense to print reserved fields only in case they are<br>
non-zero.<br></blockquote><div> </div><div>OK, padding field is similar right? Will check them all.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             PRINT_FIELD_U(", ", atomic, user_data);<br>
user_data is opaque and likely has to be printed as hexadecimal.<br>
<br></blockquote><div> </div><div>OK</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_createpropblob(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_create_blob blob;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &blob))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +<br>
<br>
> +             PRINT_FIELD_PTR("{", blob, data);<br>
Why not PRINT_FIELD_ADDR/PRINT_FIELD_ADDR64?<br>
<br>
Since it is a blob, it makes sense to print it using printstr with<br>
appropriate flags.<br>
<br>
> +             PRINT_FIELD_U(", ", blob, length);<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &blob)) {<br>
> +             PRINT_FIELD_U(", ", blob, blob_id);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_destroypropblob(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_destroy_blob blob;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &blob)) {<br>
> +             PRINT_FIELD_U("{", blob, blob_id);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_CREATE<br>
> +<br>
> +# include "xlat/drm_syncobj_flags.h"<br>
> +<br>
> +static int<br>
> +drm_syncobj_create(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_syncobj_create create;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &create))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +<br>
<br>
> +             tprints("{flags=");<br>
> +             printxval(drm_syncobj_flags, create.flags, "DRM_SYNCOJB_???");<br>
Why printxval and not printflags?<br>
<br>
Why not PRINT_FIELD_FLAGS?<br>
<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &create)) {<br>
> +             PRINT_FIELD_U(", ", create, handle);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_DESTROY<br>
> +static int<br>
> +drm_syncobj_destroy(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_syncobj_destroy destroy;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &destroy)) {<br>
> +             PRINT_FIELD_U("{", destroy, handle);<br>
<br>
> +             PRINT_FIELD_U(", ", destroy, pad);<br>
It likely makes sense to print this one only in case it is non-zero.<br>
<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD<br>
> +static int<br>
> +drm_syncobj_handle_fd(struct tcb *const tcp, const kernel_ulong_t arg,<br>
> +                      bool is_handle_to_fd)<br>
> +{<br>
> +     struct drm_syncobj_handle handle;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &handle))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +<br>
> +             if (is_handle_to_fd)<br>
> +                     PRINT_FIELD_U("{", handle, handle);<br>
> +             else<br>
<br>
> +                     PRINT_FIELD_D("{", handle, fd);<br>
Why not PRINT_FIELD_FD?<br>
<br>
> +             PRINT_FIELD_X(", ", handle, flags);<br>
This one can have DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE or<br>
DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE flag.<br>
<br>
> +             PRINT_FIELD_U(", ", handle, pad);<br>
It likely makes sense to print this one only in case it is non-zero.<br>
<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &handle)) {<br>
> +             if (is_handle_to_fd)<br>
<br>
> +                     PRINT_FIELD_D(", ", handle, fd);<br>
Why not PRINT_FIELD_FD?<br>
<br>
> +             else<br>
> +                     PRINT_FIELD_U(", ", handle, handle);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_WAIT<br>
> +static int<br>
> +drm_syncobj_wait(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_syncobj_wait wait;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &wait))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
<br>
> +             PRINT_FIELD_PTR("{", wait, handles);<br>
Why not PRINT_FIELD_ADDR/PRINT_FIELD_ADDR64?<br>
<br>
This is an array of count_handles elements of uint32_t type, and likely<br>
has to be treated as such.<br>
<br>
> +             PRINT_FIELD_D(", ", wait, timeout_nsec);<br>
If this timeout is absolute, it might make sense to print a time stamp in<br>
ISO 8601 format as well.<br></blockquote><div><br></div><div>This one is also new to me. I will search for info later.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             PRINT_FIELD_U(", ", wait, count_handles);<br>
<br>
> +             PRINT_FIELD_X(", ", wait, flags);<br>
This one can have DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL,<br>
ant DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT flags.<br>
<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &wait)) {<br>
> +             PRINT_FIELD_U(", ", wait, first_signaled);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_RESET<br>
> +static int<br>
> +drm_syncobj_reset_or_signal(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_syncobj_array array;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &array)) {<br>
<br>
> +             PRINT_FIELD_PTR("{", array, handles);<br>
Why not PRINT_FIELD_ADDR/PRINT_FIELD_ADDR64?<br>
<br>
This is an array of count_handles elements of uint32_t type, and likely<br>
has to be treated as such.<br>
<br>
> +             PRINT_FIELD_U(", ", array, count_handles);<br>
<br>
> +             PRINT_FIELD_U(", ", array, pad);<br>
It likely makes sense to print this one only in case it is non-zero.<br>
<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_MODE_CREATE_LEASE<br>
> +static int<br>
> +drm_mode_create_lease(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_create_lease lease;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &lease))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +<br>
<br>
> +             PRINT_FIELD_PTR("{", lease, object_ids);<br>
Why not PRINT_FIELD_ADDR/PRINT_FIELD_ADDR64?<br>
<br>
This is an array of object_count elements of uint32_t type, and likely<br>
has to be treated as such.<br>
<br>
> +             PRINT_FIELD_U(", ", lease, object_count);<br>
<br>
> +             PRINT_FIELD_X(", ", lease, flags);<br>
It can have O_CLOEXEC and O_NONBLOCK, at least.<br>
<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &lease)) {<br>
> +             PRINT_FIELD_U(", ", lease, lessee_id);<br>
<br>
> +             PRINT_FIELD_U(", ", lease, fd);<br>
Why not PRINT_FIELD_FD?<br>
<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_MODE_LIST_LESSEES<br>
> +static int<br>
> +drm_mode_list_lessees(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_list_lessees lessees;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +<br>
> +     if (umove_or_printaddr(tcp, arg, &lessees))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     tprints("{");<br>
> +     if (entering(tcp)) {<br>
<br>
> +             PRINT_FIELD_U("", lessees, pad);<br>
It likely makes sense to print this one only in case it is non-zero.<br>
<br>
> +             PRINT_FIELD_PTR(", ",lessees, lessees_ptr);<br>
Why not PRINT_FIELD_ADDR/PRINT_FIELD_ADDR64?<br>
<br>
This is an array of count_lessees elements of uint32_t type, and likely<br>
has to be treated as such.<br>
<br>
> +             PRINT_FIELD_U(", ", lessees, count_lessees);<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     PRINT_FIELD_U("", lessees, count_lessees);<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_MODE_GET_LEASE<br>
> +static int<br>
> +drm_mode_get_lease(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_get_lease lease;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +<br>
> +     if (umove_or_printaddr(tcp, arg, &lease))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     tprints("{");<br>
> +     if (entering(tcp)) {<br>
<br>
> +             PRINT_FIELD_U("", lease, pad);<br>
It likely makes sense to print this one only in case it is non-zero.<br>
<br>
> +             PRINT_FIELD_PTR(", ", lease, objects_ptr);<br>
Why not PRINT_FIELD_ADDR/PRINT_FIELD_ADDR64?<br>
<br>
This is an array of count_object elements of uint32_t type, and likely<br>
has to be treated as such.<br>
<br>
> +             PRINT_FIELD_U(", ", lease, count_objects);<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     PRINT_FIELD_U("", lease, count_objects);<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_MODE_REVOKE_LEASE<br>
> +static int<br>
> +drm_mode_revoke_lease(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_mode_revoke_lease lease;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &lease)) {<br>
> +             PRINT_FIELD_U("{", lease, lessee_id);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT<br>
> +<br>
> +# include "xlat/drm_syncobj_wait_flags.h"<br>
> +<br>
> +static int<br>
> +drm_syncobj_timeline_wait(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_syncobj_timeline_wait wait;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &wait))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
<br>
> +             PRINT_FIELD_PTR("{", wait, handles);<br>
Why not PRINT_FIELD_ADDR/PRINT_FIELD_ADDR64?<br>
<br>
This is an array of count_handles elements of uint32_t type, and likely<br>
has to be treated as such.<br>
<br>
> +             PRINT_FIELD_D(", ", wait, timeout_nsec);<br>
If this timeout is absolute, it might make sense to print a time stamp in<br>
ISO 8601 format as well.<br>
<br>
> +             PRINT_FIELD_U(", ", wait, count_handles);<br>
<br>
> +             tprints(", flags=");<br>
> +             printxval(drm_syncobj_wait_flags, wait.flags, "DRM_SYNCOBJ_WAIT_FLAGS_???");<br>
Why printxval and not printflags?<br>
<br>
Why not PRINT_FIELD_FLAGS?<br>
<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &wait)) {<br>
> +             PRINT_FIELD_U(", ", wait, first_signaled);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_QUERY<br>
> +static int<br>
> +drm_syncobj_query_or_timeline_signal(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_syncobj_timeline_array array;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &array)) {<br>
<br>
> +             PRINT_FIELD_PTR("{", array, handles);<br>
> +             PRINT_FIELD_PTR(", ", array, points);<br>
Why not PRINT_FIELD_ADDR/PRINT_FIELD_ADDR64?<br>
<br>
"handles" is an array of count_handles elements of uint32_t type, and likely<br>
has to be treated as such.<br>
<br>
"points" is an array of count_handles elements of uint64_t type, and likely<br>
has to be treated as such.<br>
<br>
> +             PRINT_FIELD_U(", ", array, count_handles);<br>
<br>
> +             PRINT_FIELD_U(", ", array, pad);<br>
It likely makes sense to print this one only in case it is non-zero.<br>
<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_TRANSFER<br>
> +static int<br>
> +drm_syncobj_transfer(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct drm_syncobj_transfer transfer;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &transfer)) {<br>
> +             PRINT_FIELD_U("{", transfer, src_handle);<br>
> +             PRINT_FIELD_U(", ", transfer, dst_handle);<br>
> +             PRINT_FIELD_U(", ", transfer, src_point);<br>
> +             PRINT_FIELD_U(", ", transfer, dst_point);<br>
<br>
> +             tprints(", flags=");<br>
> +             printxval(drm_syncobj_wait_flags, transfer.flags, "DRM_SYNCOBJ_WAIT_FLAGS_???");<br>
Why printxval and not printflags?<br>
<br>
Why not PRINT_FIELD_FLAGS?<br>
<br>
> +             PRINT_FIELD_U(", ", transfer, pad);<br>
It likely makes sense to print this one only in case it is non-zero.<br>
<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +# endif<br>
> +<br>
> +int<br>
> +drm_ioctl(struct tcb *const tcp, const unsigned int code,<br>
> +       const kernel_ulong_t arg)<br>
> +{<br>
<br>
> +     switch (code) {<br>
I'd rather perform dispatch against _IOC_NR(code) if there's nothing<br>
preventing that.<br></blockquote><div><br></div><div>Why? I saw that other decoders use switch(code) directly.</div><div>But it makes sense here because the size of arguments are</div><div>always changing as we've discussed above. If I do so, does</div><div>that mean I have to add _IOC_NR() to every case DRM_IOCTL</div><div>constants? Would that look redundant?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +     case DRM_IOCTL_GET_MAGIC: /* R */<br>
> +             return drm_get_magic(tcp, arg);<br>
> +     case DRM_IOCTL_IRQ_BUSID: /* RW */<br>
> +             return drm_irq_busid(tcp, arg);<br>
> +     case DRM_IOCTL_SET_VERSION: /* RW */<br>
> +             return drm_set_version(tcp, arg);<br>
> +     case DRM_IOCTL_MODESET_CTL: /* W */<br>
> +             return drm_modeset_ctl(tcp, arg);<br>
> +     case DRM_IOCTL_GEM_CLOSE: /* W */<br>
> +             return drm_gem_close(tcp, arg);<br>
> +     case DRM_IOCTL_GEM_FLINK: /* RW */<br>
> +             return drm_gem_flink(tcp, arg);<br>
> +     case DRM_IOCTL_GEM_OPEN: /* RW */<br>
> +             return drm_gem_open(tcp, arg);<br>
> +     case DRM_IOCTL_GET_CAP: /* RW */<br>
> +             return drm_get_cap(tcp, arg);<br>
> +     case DRM_IOCTL_SET_CLIENT_CAP: /* W */<br>
> +             return drm_set_client_cap(tcp, arg);<br>
> +     case DRM_IOCTL_AUTH_MAGIC: /* W */<br>
> +             return drm_auth_magic(tcp, arg);<br>
> +     case DRM_IOCTL_BLOCK: /* RW */<br>
> +     case DRM_IOCTL_UNBLOCK: /* RW */<br>
> +     case DRM_IOCTL_MOD_CTX: /* W */<br>
> +     case DRM_IOCTL_ADD_DRAW: /* RW */<br>
> +     case DRM_IOCTL_RM_DRAW: /* RW */<br>
> +     case DRM_IOCTL_FINISH: /* W */<br>
> +     case DRM_IOCTL_MODE_ATTACHMODE: /* RW */<br>
> +     case DRM_IOCTL_MODE_DETACHMODE: /* RW */<br>
> +             return drm_noop(tcp, arg);<br>
> +     case DRM_IOCTL_CONTROL: /* W */<br>
> +             return drm_control(tcp, arg);<br>
> +     case DRM_IOCTL_ADD_CTX: /* RW */<br>
> +     case DRM_IOCTL_SWITCH_CTX: /* W */<br>
> +     case DRM_IOCTL_NEW_CTX: /* W */<br>
> +             return drm_ctx(tcp, arg);<br>
> +     case DRM_IOCTL_RM_CTX: /* RW */<br>
> +             return drm_rm_ctx(tcp, arg);<br>
> +     case DRM_IOCTL_GET_CTX: /* RW */<br>
> +             return drm_get_ctx(tcp, arg);<br>
> +     case DRM_IOCTL_LOCK: /* W */<br>
> +     case DRM_IOCTL_UNLOCK: /* W */<br>
> +             return drm_lock(tcp, arg);<br>
> +     case DRM_IOCTL_PRIME_HANDLE_TO_FD: /* RW */<br>
> +             return drm_prime_handle_to_fd(tcp, arg);<br>
> +     case DRM_IOCTL_PRIME_FD_TO_HANDLE: /* RW */<br>
> +             return drm_prime_fd_to_handle(tcp, arg);<br>
> +# ifdef DRM_IOCTL_CRTC_GET_SEQUENCE<br>
> +     case DRM_IOCTL_CRTC_GET_SEQUENCE: /* RW */<br>
> +             return drm_crtc_get_sequence(tcp, arg);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_CRTC_QUEUE_SEQUENCE<br>
> +     case DRM_IOCTL_CRTC_QUEUE_SEQUENCE: /* RW */<br>
> +             return drm_crtc_queue_sequence(tcp, arg);<br>
> +# endif<br>
> +     case DRM_IOCTL_MODE_GETRESOURCES: /* RW */<br>
> +             return drm_mode_get_resources(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_GETCRTC: /* RW */<br>
> +     case DRM_IOCTL_MODE_SETCRTC: /* RW */<br>
> +             return drm_mode_crtc(tcp, arg, code == DRM_IOCTL_MODE_GETCRTC);<br>
> +     case DRM_IOCTL_MODE_CURSOR: /* RW */<br>
> +             return drm_mode_cursor(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_GETGAMMA: /* RW */<br>
> +     case DRM_IOCTL_MODE_SETGAMMA: /* RW */<br>
> +             return drm_mode_gamma(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_GETENCODER: /* RW */<br>
> +             return drm_mode_get_encoder(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_GETPROPERTY: /* RW */<br>
> +             return drm_mode_get_property(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_SETPROPERTY: /* RW */<br>
> +             return drm_mode_set_property(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_GETPROPBLOB: /* RW */<br>
> +             return drm_mode_get_prop_blob(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_GETFB: /* RW */<br>
> +             return drm_mode_get_fb(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_ADDFB: /* RW */<br>
> +             return drm_mode_add_fb(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_RMFB: /* RW */<br>
> +             return drm_mode_rm_fb(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_PAGE_FLIP: /* RW */<br>
> +             return drm_mode_page_flip(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_DIRTYFB: /* RW */<br>
> +             return drm_mode_dirty_fb(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_CREATE_DUMB: /* RW */<br>
> +             return drm_mode_create_dumb(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_MAP_DUMB: /* RW */<br>
> +             return drm_mode_map_dumb(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_DESTROY_DUMB: /* RW */<br>
> +             return drm_mode_destroy_dumb(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_GETPLANE: /* RW */<br>
> +             return drm_mode_getplane(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_SETPLANE: /* RW */<br>
> +             return drm_mode_setplane(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_CURSOR2: /* RW */<br>
> +             return drm_mode_cursor2(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_ATOMIC: /* RW */<br>
> +             return drm_mode_atomic(tcp, arg);//<br>
> +     case DRM_IOCTL_MODE_CREATEPROPBLOB: /* RW */<br>
> +             return drm_mode_createpropblob(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_DESTROYPROPBLOB: /* RW */<br>
> +             return drm_mode_destroypropblob(tcp, arg);<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_CREATE<br>
> +     case DRM_IOCTL_SYNCOBJ_CREATE: /* RW */<br>
> +             return drm_syncobj_create(tcp, arg);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_DESTROY<br>
> +     case DRM_IOCTL_SYNCOBJ_DESTROY: /* RW */<br>
> +             return drm_syncobj_destroy(tcp, arg);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD<br>
> +     case DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD: /* RW */<br>
> +     case DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE: /* RW */<br>
> +             return drm_syncobj_handle_fd(tcp, arg, code == DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_WAIT<br>
> +     case DRM_IOCTL_SYNCOBJ_WAIT: /* RW */<br>
> +             return drm_syncobj_wait(tcp, arg);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_RESET<br>
> +     case DRM_IOCTL_SYNCOBJ_RESET: /* RW */<br>
> +     case DRM_IOCTL_SYNCOBJ_SIGNAL: /* RW */<br>
> +             return drm_syncobj_reset_or_signal(tcp, arg);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_MODE_CREATE_LEASE<br>
> +     case DRM_IOCTL_MODE_CREATE_LEASE: /* RW */<br>
> +             return drm_mode_create_lease(tcp, arg);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_MODE_LIST_LESSEES<br>
> +     case DRM_IOCTL_MODE_LIST_LESSEES: /* RW */<br>
> +             return drm_mode_list_lessees(tcp, arg);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_MODE_GET_LEASE<br>
> +     case DRM_IOCTL_MODE_GET_LEASE: /* RW */<br>
> +             return drm_mode_get_lease(tcp, arg);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_MODE_REVOKE_LEASE<br>
> +     case DRM_IOCTL_MODE_REVOKE_LEASE: /* RW */<br>
> +             return drm_mode_revoke_lease(tcp, arg);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT<br>
> +     case DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT: /* RW */<br>
> +             return drm_syncobj_timeline_wait(tcp, arg);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_QUERY<br>
> +     case DRM_IOCTL_SYNCOBJ_QUERY: /* RW */<br>
> +     case DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL: /* RW */<br>
> +             return drm_syncobj_query_or_timeline_signal(tcp, arg);<br>
> +# endif<br>
> +# ifdef DRM_IOCTL_SYNCOBJ_TRANSFER<br>
> +     case DRM_IOCTL_SYNCOBJ_TRANSFER: /* RW */<br>
> +             return drm_syncobj_transfer(tcp, arg);<br>
> +# endif<br>
> +     default:<br>
> +             return drm_ioctl_mpers(tcp, code, arg);<br>
> +     }<br>
> +}<br>
> +<br>
> +#endif /* HAVE_DRM_H || HAVE_DRM_DRM_H */<br>
> diff --git a/drm_mpers.c b/drm_mpers.c<br>
> new file mode 100644<br>
> index 00000000..1b685a52<br>
> --- /dev/null<br>
> +++ b/drm_mpers.c<br>
> @@ -0,0 +1,815 @@<br>
> +#include "defs.h"<br>
> +<br>
> +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H)<br>
> +<br>
> +#ifdef HAVE_DRM_H<br>
> +# include <drm.h><br>
> +#else<br>
> +# include <drm/drm.h><br>
> +#endif<br>
> +<br>
> +#include DEF_MPERS_TYPE(struct_drm_version)<br>
> +#include DEF_MPERS_TYPE(struct_drm_unique)<br>
> +#include DEF_MPERS_TYPE(struct_drm_map)<br>
> +#include DEF_MPERS_TYPE(struct_drm_client)<br>
> +#include DEF_MPERS_TYPE(struct_drm_stats)<br>
> +#include DEF_MPERS_TYPE(struct_drm_buf_desc)<br>
> +#include DEF_MPERS_TYPE(struct_drm_buf_info)<br>
> +#include DEF_MPERS_TYPE(struct_drm_buf_map)<br>
> +#include DEF_MPERS_TYPE(struct_drm_buf_free)<br>
> +#include DEF_MPERS_TYPE(struct_drm_ctx_priv_map)<br>
> +#include DEF_MPERS_TYPE(struct_drm_ctx_res)<br>
> +#include DEF_MPERS_TYPE(struct_drm_agp_mode)<br>
> +#include DEF_MPERS_TYPE(struct_drm_agp_info)<br>
> +#include DEF_MPERS_TYPE(struct_drm_agp_buffer)<br>
> +#include DEF_MPERS_TYPE(struct_drm_agp_binding)<br>
> +#include DEF_MPERS_TYPE(struct_drm_scatter_gather)<br>
> +#include DEF_MPERS_TYPE(union_drm_wait_vblank)<br>
> +#include DEF_MPERS_TYPE(struct_drm_mode_get_connector)<br>
> +#include DEF_MPERS_TYPE(struct_drm_mode_fb_cmd2)<br>
> +#include DEF_MPERS_TYPE(struct_drm_mode_get_plane_res)<br>
> +#include DEF_MPERS_TYPE(struct_drm_mode_obj_get_properties)<br>
> +#include DEF_MPERS_TYPE(struct_drm_mode_obj_set_property)<br>
> +<br>
> +typedef struct drm_version struct_drm_version;<br>
> +typedef struct drm_unique struct_drm_unique;<br>
> +typedef struct drm_map struct_drm_map;<br>
> +typedef struct drm_client struct_drm_client;<br>
> +typedef struct drm_stats struct_drm_stats;<br>
> +typedef struct drm_buf_desc struct_drm_buf_desc;<br>
> +typedef struct drm_buf_info struct_drm_buf_info;<br>
> +typedef struct drm_buf_map struct_drm_buf_map;<br>
> +typedef struct drm_buf_free struct_drm_buf_free;<br>
> +typedef struct drm_ctx_priv_map struct_drm_ctx_priv_map;<br>
> +typedef struct drm_ctx_res struct_drm_ctx_res;<br>
> +typedef struct drm_agp_mode struct_drm_agp_mode;<br>
> +typedef struct drm_agp_info struct_drm_agp_info;<br>
> +typedef struct drm_agp_buffer struct_drm_agp_buffer;<br>
> +typedef struct drm_agp_binding struct_drm_agp_binding;<br>
> +typedef struct drm_scatter_gather struct_drm_scatter_gather;<br>
> +typedef union drm_wait_vblank union_drm_wait_vblank;<br>
> +typedef struct drm_mode_get_connector struct_drm_mode_get_connector;<br>
> +typedef struct drm_mode_fb_cmd2 struct_drm_mode_fb_cmd2;<br>
> +typedef struct drm_mode_get_plane_res struct_drm_mode_get_plane_res;<br>
> +typedef struct drm_mode_obj_get_properties struct_drm_mode_obj_get_properties;<br>
> +typedef struct drm_mode_obj_set_property struct_drm_mode_obj_set_property;<br>
> +<br>
> +#endif /* HAVE_DRM_H || HAVE_DRM_DRM_H */<br>
> +<br>
> +#include MPERS_DEFS<br>
> +<br>
> +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H)<br>
> +<br>
> +# include "print_fields.h"<br>
> +<br>
> +static int<br>
> +drm_version(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_version ver;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +     if (umove_or_printaddr(tcp, arg, &ver))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     if (entering(tcp)) {<br>
> +             PRINT_FIELD_U("{", ver, name_len);<br>
> +             PRINT_FIELD_U(", ", ver, date_len);<br>
> +             PRINT_FIELD_U(", ", ver, desc_len);<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     PRINT_FIELD_D("{", ver, version_major);<br>
> +     PRINT_FIELD_D(", ", ver, version_minor);<br>
> +     PRINT_FIELD_D(", ", ver, version_patchlevel);<br>
> +     PRINT_FIELD_U(", ", ver, name_len);<br>
<br>
> +     tprints(", name=");<br>
> +     printstrn(tcp, ptr_to_kulong(<a href="http://ver.name" rel="noreferrer" target="_blank">ver.name</a>), ver.name_len);<br>
Why not PRINT_FIELD_STRN?<br></blockquote><div><br></div><div> In include/uapi/drm/drm.h, the <a href="http://ver.name">ver.name</a> field is a char * type. Seems</div><div>in PRINT_FIELD_STRN macro we have to cast char * to kernel_ulong_t</div><div>which is not achievable in this macro. Am I missing anything here?</div><div>Some other comments below are similar.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +     PRINT_FIELD_U(", ", ver, date_len);<br>
<br>
> +     tprints(", date=");<br>
> +     printstrn(tcp, ptr_to_kulong(ver.date), ver.date_len);<br>
Why not PRINT_FIELD_STRN?<br>
<br>
> +     PRINT_FIELD_U(", ", ver, desc_len);<br>
<br>
> +     tprints(", desc=");<br>
> +     printstrn(tcp, ptr_to_kulong(ver.desc), ver.desc_len);<br>
Why not PRINT_FIELD_STRN?<br>
<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_unique(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_unique unique;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +     if (umove_or_printaddr(tcp, arg, &unique))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     PRINT_FIELD_U("{", unique, unique_len);<br>
> +     if (entering(tcp)) {<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
<br>
> +     tprints(", unique=");<br>
> +     printstrn(tcp, ptr_to_kulong(unique.unique), unique.unique_len);<br>
Why not PRINT_FIELD_STRN?<br>
<br>
I suspect that a cast to uintptr_t is sufficient here.<br></blockquote><div><br></div><div>Yes you're right. BTW what's the difference? I mean when</div><div>to use ptr_to_kulong and when casting to uintptr_t?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
<br>
> +# include "xlat/drm_map_type.h"<br>
> +# include "xlat/drm_map_flags.h"<br>
Inline include.<br>
<br>
> +<br>
> +static int<br>
> +drm_get_map(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_map map;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +<br>
> +     if (umove_or_printaddr(tcp, arg, &map))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     PRINT_FIELD_X("{", map, offset);<br>
> +     if (entering(tcp)) {<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     PRINT_FIELD_U(", ", map, size);<br>
<br>
> +     tprints(", type=");<br>
> +     printxval(drm_map_type, map.type, "_DRM_???");<br>
Why not PRINT_FIELD_XVAL?<br>
<br>
> +     tprints(", flags=");<br>
> +     printxval(drm_map_flags, map.flags, "_DRM_???");<br>
Why printxval and not printflags?<br>
<br>
Why not PRINT_FIELD_FLAGS?<br>
<br>
> +     PRINT_FIELD_PTR(", ", map, handle);<br>
> +     PRINT_FIELD_D(", ", map, mtrr);<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_get_client(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_client client;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &client))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_D("{", client, idx);<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &client)) {<br>
> +             PRINT_FIELD_D(", ", client, auth);<br>
> +             PRINT_FIELD_U(", ", client, pid);<br>
<br>
> +             PRINT_FIELD_U(", ", client, uid);<br>
Why not PRINT_FIELD_UID?<br></blockquote><div><br></div><div>OK.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             PRINT_FIELD_U(", ", client, magic);<br>
> +             PRINT_FIELD_U(", ", client, iocs);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +# include "xlat/drm_stat_type.h"<br>
> +<br>
> +static int<br>
> +drm_get_stats(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_stats stats;<br>
> +<br>
> +     if (exiting(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &stats))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", stats, count);<br>
> +             tprints(", data=[");<br>
> +<br>
<br>
> +             for (unsigned int i = 0; i < 15; i++) {<br>
Why is "15" hard-coded?<br>
<br>
Also, it seems that it should go up to "count".<br></blockquote><div><br></div><div>My fault.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +                     tprintf("%s%lu", i > 0 ? ", {value=" : "{value=", (unsigned long) stats.data[i].value);<br>
Overly long line.<br>
<br>
> +                     tprints(", type=");<br>
<br>
> +                     printxval(drm_stat_type, stats.data[i].type, "_DRM_STAT_???");<br>
Overly long line.<br></blockquote><div><br></div><div>My fault.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Why not PRINT_FIELD_XVAL?<br>
<br>
> +                     tprints("}");<br>
> +             }<br>
<br>
It could be worth considering incorporating something like<br>
print_local_array from "Add support for printing local arrays to<br>
print_array" commit from esyr/evdev_decode_bitset branch instead<br>
of open-coding it.<br>
<br></blockquote><div><br></div><div>Will this work for all those arrays pointed by *_ptr fields</div><div>in some structures above and below?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +             tprints("]}");<br>
> +<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     }<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_add_map(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_map map;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &map)) {<br>
> +             PRINT_FIELD_X("{", map, offset);<br>
> +             PRINT_FIELD_U(", ", map, size);<br>
<br>
> +             tprints(", type=");<br>
> +             printxval(drm_map_type, map.type, "_DRM_???");<br>
Why not PRINT_FIELD_XVAL?<br>
<br>
> +             tprints(", flags");<br>
> +             printxval(drm_map_flags, map.flags, "_DRM_???");<br>
Why printxval and not printflags?<br>
<br>
Why not PRINT_FIELD_FLAGS?<br>
<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +# include "xlat/drm_buf_desc_flags.h"<br>
> +<br>
> +static int<br>
> +drm_add_bufs(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_buf_desc desc;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +     if (umove_or_printaddr(tcp, arg, &desc))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     PRINT_FIELD_D("{", desc, count);<br>
> +     PRINT_FIELD_D(", ", desc, size);<br>
> +     if (entering(tcp)) {<br>
<br>
> +             tprints(", flags=");<br>
> +             printxval(drm_buf_desc_flags, desc.flags, "_DRM_???");<br>
Why printxval and not printflags?<br>
<br>
Why not PRINT_FIELD_FLAGS?<br>
<br>
> +             PRINT_FIELD_X(", ", desc, agp_start);<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mark_bufs(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_buf_desc desc;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &desc)) {<br>
> +             PRINT_FIELD_D("{", desc, size);<br>
> +             PRINT_FIELD_D(", ", desc, low_mark);<br>
> +             PRINT_FIELD_D(", ", desc, high_mark);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_info_bufs(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_buf_info info;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +     if (umove_or_printaddr(tcp, arg, &info))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     tprints("{");<br>
> +     if (entering(tcp)) {<br>
<br>
> +             PRINT_FIELD_PTR("", info, list);<br>
It is an array of count elements of struct drm_buf_desc type, and<br>
likely has to be treated as such.<br>
<br>
> +             PRINT_FIELD_D(", ", info, count);<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     PRINT_FIELD_D("", info, count);<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_map_bufs(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_buf_map map;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &map))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_D("{", map, count);<br>
> +             PRINT_FIELD_PTR(", ", map, virtual);<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &map)) {<br>
<br>
> +             PRINT_FIELD_PTR(", ", map, list);<br>
It is an array of count elements of struct drm_buf_pub type, and<br>
likely has to be treated as such.<br>
<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_free_bufs(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_buf_free free;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &free)) {<br>
> +             PRINT_FIELD_D("{", free, count);<br>
<br>
> +             PRINT_FIELD_PTR(", ", free, list);<br>
It's an array of count elements of int type, and likely has to be<br>
treated as such.<br>
<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_rm_map(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_map map;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &map)) {<br>
> +             PRINT_FIELD_PTR("{", map, handle);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_sarea_ctx(struct tcb *const tcp, const kernel_ulong_t arg,<br>
> +           const bool is_get)<br>
> +{<br>
> +     struct_drm_ctx_priv_map map;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &map))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", map, ctx_id);<br>
> +             if (!is_get)<br>
> +                     PRINT_FIELD_PTR(", ", map, handle);<br>
> +             return 0;<br>
> +     }<br>
> +<br>
<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &map)) {<br>
> +             if (is_get)<br>
"if" nesting can be avoided here.<br></blockquote><div><br></div><div>Sure.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +                     PRINT_FIELD_PTR(", ", map, handle);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_res_ctx(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_ctx_res ctx;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +     if (umove_or_printaddr(tcp, arg, &ctx))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     PRINT_FIELD_D("{", ctx, count);<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
<br>
> +     PRINT_FIELD_PTR(", ", ctx, contexts);<br>
It's an array of count elements of struct drm_ctx type, and likely<br>
has to be treated as such.<br>
<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +<br>
> +static int<br>
> +drm_agp_enable(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_agp_mode mode;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &mode)) {<br>
> +             PRINT_FIELD_U("{", mode, mode);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_agp_info(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_agp_info info;<br>
> +<br>
> +     if (exiting(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &info))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_D("{", info, agp_version_major);<br>
> +             PRINT_FIELD_D(", ", info, agp_version_minor);<br>
> +             PRINT_FIELD_U(", ", info, mode);<br>
<br>
> +             PRINT_FIELD_U(", ", info, aperture_base);<br>
This is likely better to be printed as hexadecimal.<br>
<br></blockquote><div> </div><div>OK.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +             PRINT_FIELD_U(", ", info, aperture_size);<br>
> +             PRINT_FIELD_U(", ", info, memory_allowed);<br>
> +             PRINT_FIELD_U(", ", info, memory_used);<br>
<br>
> +             PRINT_FIELD_U(", ", info, id_vendor);<br>
> +             PRINT_FIELD_U(", ", info, id_device);<br>
These are likely better to be printed as hexadecimal.<br>
<br></blockquote><div> </div><div>OK. </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +             tprints("}");<br>
> +<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     }<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_agp_alloc(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_agp_buffer buffer;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &buffer))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             PRINT_FIELD_U("{", buffer, size);<br>
> +             PRINT_FIELD_U(", ", buffer, type);<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &buffer)) {<br>
> +             PRINT_FIELD_U(", ", buffer, handle);<br>
> +             PRINT_FIELD_U(", ", buffer, physical);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_agp_free(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_agp_buffer buffer;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &buffer)) {<br>
> +             PRINT_FIELD_U("{", buffer, handle);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_agp_bind(struct tcb *const tcp, const kernel_ulong_t arg, bool is_bind)<br>
> +{<br>
> +     struct_drm_agp_binding binding;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &binding)) {<br>
> +             PRINT_FIELD_U("{", binding, handle);<br>
> +             if (is_bind)<br>
> +                     PRINT_FIELD_X(", ", binding, offset);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_scatter_gather(struct tcb *const tcp, const kernel_ulong_t arg,<br>
> +                bool is_alloc)<br>
> +{<br>
> +     struct_drm_scatter_gather sg;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &sg))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +             if (is_alloc)<br>
> +                     PRINT_FIELD_U("{", sg, size);<br>
> +             else<br>
> +                     PRINT_FIELD_U("{", sg, handle);<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &sg)) {<br>
> +             if (is_alloc)<br>
> +                     PRINT_FIELD_U(", ", sg, handle);<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +# include "xlat/drm_vblank_seq_type.h"<br>
> +<br>
> +static int<br>
> +drm_wait_vblank(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     union_drm_wait_vblank vblank;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &vblank))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +<br>
> +             tprints("{request={type=");<br>
> +             printxval(drm_vblank_seq_type, vblank.request.type, "_DRM_VBLANK_???");<br>
Overly long line.<br></blockquote><div><br></div><div>My fault.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Seems like the value consists of three parts (_DRM_VBLANK_TYPES_MASK,<br>
_DRM_VBLANK_HIGH_CRTC_MASK, and _DRM_VBLANK_FLAGS_MASK; the latter omits<br>
_DRM_VBLANK_FLIP for unknown reason), and only bits covered by<br>
_DRM_VBLANK_TYPES_MASK and  _DRM_VBLANK_HIGH_CRTC_MASK has to be decoded as xval,<br>
the rest is flags.</blockquote><div> </div><div>You're right. Somewhere else in the code may has the same problem.</div><div>Will double-check.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             PRINT_FIELD_U(", ", vblank.request, sequence);<br>
> +             PRINT_FIELD_U(", ", vblank.request, signal);<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &vblank)) {<br>
<br>
> +             PRINT_FIELD_U(", reply={", vblank.reply, type);<br>
This field has the same "enum drm_vblank_seq_type" type.<br>
<br>
> +             PRINT_FIELD_U(", ", vblank.reply, sequence);<br>
<br>
> +             PRINT_FIELD_U(", ", vblank.reply, tval_sec);<br>
> +             PRINT_FIELD_U(", ", vblank.reply, tval_usec);<br>
They both are signed.<br>
<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_get_connector(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_mode_get_connector con;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +<br>
> +     if (umove_or_printaddr(tcp, arg, &con))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     PRINT_FIELD_U("{", con, connector_id);<br>
> +     PRINT_FIELD_U(", ", con, count_encoders);<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
<br>
> +     PRINT_FIELD_PTR(", ", con, encoders_ptr);<br>
It's an array of count_encoders elements of uint32_t type, and likely<br>
has to be treated as such.<br>
<br>
> +     PRINT_FIELD_PTR(", ", con, modes_ptr);<br>
It's an array of count_modes elements of struct drm_mode_modeinfo type,<br>
and likely has to be treated as such.<br>
<br>
> +     PRINT_FIELD_PTR(", ", con, props_ptr);<br>
It's an array of count_props elements of struct uint32_t type, and likely<br>
has to be treated as such.<br>
<br>
> +     PRINT_FIELD_PTR(", ", con, prop_values_ptr);<br>
It's an array of count_props elements of struct uint64_t type, and likely<br>
has to be treated as such.<br>
<br>
> +     PRINT_FIELD_U(", ", con, count_modes);<br>
> +     PRINT_FIELD_U(", ", con, count_props);<br>
These seems to be R/W.<br>
<br>
> +     PRINT_FIELD_U(", ", con, encoder_id);<br>
> +     PRINT_FIELD_U(", ", con, connector_type);<br>
> +     PRINT_FIELD_U(", ", con, connector_type_id);<br>
> +     PRINT_FIELD_U(", ", con, connection);<br>
> +     PRINT_FIELD_U(", ", con, mm_width);<br>
> +     PRINT_FIELD_U(", ", con, mm_height);<br>
> +     PRINT_FIELD_U(", ", con, subpixel);<br>
<br>
There's also "pad" field that may be worth printing in case it is<br>
non-zero.<br>
<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_add_fb2(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_mode_fb_cmd2 cmd;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &cmd))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
<br>
> +             tprintf("{width=%u, height=%u, pixel_format=%#x, flags=%u, "<br>
It may be worth considering rewriting this using PRINT_FIELDS_*.<br></blockquote><div><br></div><div>Indeed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
"flags" field can contain DRM_MODE_FB_INTERLACED and DRM_MODE_FB_MODIFIERS flags.<br>
<br>
> +                     "handles=[%u, %u, %u, %u], "<br>
> +                     "pitches=[%u, %u, %u, %u], "<br>
> +                     "offsets=[%u, %u, %u, %u]",<br>
> +                     cmd.width, cmd.height, cmd.pixel_format, cmd.flags,<br>
<br>
> +                     cmd.handles[0], cmd.handles[1], cmd.handles[2],<br>
> +                     cmd.handles[3], cmd.pitches[0], cmd.pitches[1],<br>
> +                     cmd.pitches[2], cmd.pitches[3], cmd.offsets[0],<br>
> +                     cmd.offsets[1], cmd.offsets[2], cmd.offsets[3]);<br>
This part is quite difficult to read.<br>
<br></blockquote><div><br></div><div>I should use for loop for this or perhaps print_local_array?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +#ifdef HAVE_STRUCT_DRM_MODE_FB_CMD2_MODIFIER<br>
> +             tprintf(", modifiers=[%" PRIu64 ", %" PRIu64 ", %" PRIu64 ", %" PRIu64 "]",<br>
> +                     (uint64_t) cmd.modifier[0], (uint64_t) cmd.modifier[1],<br>
> +                     (uint64_t) cmd.modifier[2], (uint64_t) cmd.modifier[3]);<br>
> +#endif<br>
Again, wrapping part of decoding in ifdef's  seems fragile.</blockquote><div><br></div><div>Well, modifier field in struct drm_mode_fb_cmd2 is added in some</div><div>newer version of libdrm. It's not like add xlat for DRM_IOC constants.</div><div>What's your suggestion on handling this?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +<br>
> +                     return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &cmd))<br>
> +             PRINT_FIELD_U(", ", cmd, fb_id);<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_getplaneresources(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_mode_get_plane_res res;<br>
> +<br>
> +     if (entering(tcp))<br>
> +             tprints(", ");<br>
> +     else if (syserror(tcp))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +     else<br>
> +             tprints(" => ");<br>
> +<br>
> +     if (umove_or_printaddr(tcp, arg, &res))<br>
> +             return RVAL_IOCTL_DECODED;<br>
> +<br>
> +     tprints("{");<br>
> +     if (entering(tcp)) {<br>
<br>
> +             PRINT_FIELD_PTR("", res, plane_id_ptr);<br>
Why not PRINT_FIELD_ADDR64?<br>
<br>
"plane_id_ptr" is an array of count_planes elements of uint32_t type,<br>
and likely has to be treated as such.<br>
<br>
> +             PRINT_FIELD_U(", ", res, count_planes);<br>
> +             tprints("}");<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
<br>
> +     PRINT_FIELD_U("", res, count_planes);<br>
This one seems to be R/W.<br>
<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_obj_getproperties(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_mode_obj_get_properties prop;<br>
> +<br>
> +     if (entering(tcp)) {<br>
> +             tprints(", ");<br>
> +             if (umove_or_printaddr(tcp, arg, &prop))<br>
> +                     return RVAL_IOCTL_DECODED;<br>
> +<br>
> +             PRINT_FIELD_PTR("{", prop, props_ptr);<br>
Why not PRINT_FIELD_ADDR64?<br>
<br>
"props_ptr" is an array of count_props elements of uint32_t type,<br>
and likely has to be treated as such.<br>
<br>
> +             PRINT_FIELD_PTR(", ", prop, prop_values_ptr);<br>
Why not PRINT_FIELD_ADDR64?<br>
<br>
"prop_values_ptr" is an array of count_props elements of uint64_t type,<br>
and likely has to be treated as such.<br>
<br>
> +             PRINT_FIELD_U(", ", prop, obj_id);<br>
> +             PRINT_FIELD_U(", ", prop, obj_type);<br>
> +<br>
> +             return 0;<br>
> +     }<br>
> +<br>
> +     if (!syserror(tcp) && !umove(tcp, arg, &prop)) {<br>
<br>
> +             PRINT_FIELD_U(", ", prop, count_props);<br>
This seems to be R/W.<br>
<br>
> +     }<br>
> +<br>
> +     tprints("}");<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +static int<br>
> +drm_mode_obj_setproperty(struct tcb *const tcp, const kernel_ulong_t arg)<br>
> +{<br>
> +     struct_drm_mode_obj_set_property prop;<br>
> +<br>
> +     tprints(", ");<br>
> +     if (!umove_or_printaddr(tcp, arg, &prop)) {<br>
> +             PRINT_FIELD_U("{", prop, value);<br>
> +             PRINT_FIELD_U(", ", prop, prop_id);<br>
> +             PRINT_FIELD_U(", ", prop, obj_id);<br>
> +             PRINT_FIELD_U(", ", prop, obj_type);<br>
> +             tprints("}");<br>
> +     }<br>
> +<br>
> +     return RVAL_IOCTL_DECODED;<br>
> +}<br>
> +<br>
> +MPERS_PRINTER_DECL(int, drm_ioctl_mpers, struct tcb *const tcp,<br>
> +                const unsigned int code, const kernel_ulong_t arg)<br>
> +{<br>
> +     switch (code) {<br>
I'd rather perform dispatch against _IOC_NR(code) if there's nothing<br>
preventing that. </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +     case DRM_IOCTL_VERSION: /* RW */<br>
> +             return drm_version(tcp, arg);<br>
> +     case DRM_IOCTL_GET_UNIQUE: /* RW */<br>
<br>
> +     //case DRM_IOCTL_SET_UNIQUE: /* W */ /* invalid op */<br>
Leftover comment.<br>
<br></blockquote><div> </div><div>Will remove.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +             return drm_unique(tcp, arg);<br>
> +     case DRM_IOCTL_GET_MAP: /* RW */<br>
> +             return drm_get_map(tcp, arg);<br>
> +     case DRM_IOCTL_GET_CLIENT: /* RW */<br>
> +             return drm_get_client(tcp, arg);<br>
> +     case DRM_IOCTL_GET_STATS: /* R */<br>
> +             return drm_get_stats(tcp, arg);<br>
> +     case DRM_IOCTL_ADD_MAP: /* RW */<br>
> +             return drm_add_map(tcp, arg);<br>
> +     case DRM_IOCTL_ADD_BUFS: /* RW */<br>
> +             return drm_add_bufs(tcp, arg);<br>
> +     case DRM_IOCTL_MARK_BUFS: /* W */<br>
> +             return drm_mark_bufs(tcp, arg);<br>
> +     case DRM_IOCTL_INFO_BUFS: /* RW */<br>
> +             return drm_info_bufs(tcp, arg);<br>
> +     case DRM_IOCTL_MAP_BUFS: /* RW */<br>
> +             return drm_map_bufs(tcp, arg);<br>
> +     case DRM_IOCTL_FREE_BUFS: /* W */<br>
> +             return drm_free_bufs(tcp, arg);<br>
> +     case DRM_IOCTL_RM_MAP: /* W */<br>
> +             return drm_rm_map(tcp, arg);<br>
> +     case DRM_IOCTL_SET_SAREA_CTX:<br>
> +     case DRM_IOCTL_GET_SAREA_CTX:<br>
> +             return drm_sarea_ctx(tcp, arg, code == DRM_IOCTL_GET_SAREA_CTX);<br>
> +     case DRM_IOCTL_RES_CTX: /* RW */<br>
> +             return drm_res_ctx(tcp, arg);<br>
> +     case DRM_IOCTL_AGP_ENABLE: /* W */<br>
> +             return drm_agp_enable(tcp, arg);<br>
> +     case DRM_IOCTL_AGP_INFO: /* R */<br>
> +             return drm_agp_info(tcp, arg);<br>
> +     case DRM_IOCTL_AGP_ALLOC: /* RW */<br>
> +             return drm_agp_alloc(tcp, arg);<br>
> +     case DRM_IOCTL_AGP_FREE: /* W */<br>
> +             return drm_agp_free(tcp, arg);<br>
> +     case DRM_IOCTL_AGP_BIND: /* W */<br>
> +     case DRM_IOCTL_AGP_UNBIND: /* W */<br>
> +             return drm_agp_bind(tcp, arg, code == DRM_IOCTL_AGP_BIND);<br>
> +     case DRM_IOCTL_SG_ALLOC: /* RW */<br>
> +     case DRM_IOCTL_SG_FREE: /* W */<br>
> +             return drm_scatter_gather(tcp, arg, code == DRM_IOCTL_SG_ALLOC);<br>
> +     case DRM_IOCTL_WAIT_VBLANK: /* RW */<br>
> +             return drm_wait_vblank(tcp, arg);<br>
> +     case DRM_IOCTL_MODE_ADDFB2: /* RW */<br>
> +             return drm_mode_add_fb2(tcp, arg);<br>
> +     }<br>
> +     /* variable length, so we can't use switch(code) to identify DRM_IOCTL_MODE_GETCONNECTOR */<br>
> +     if (_IOC_NR(code) == _IOC_NR(DRM_IOCTL_MODE_GETCONNECTOR)) {<br>
> +             return drm_mode_get_connector(tcp, arg);<br>
> +     }<br>
> +<br>
> +     if (_IOC_NR(code) == _IOC_NR(DRM_IOCTL_MODE_GETPLANERESOURCES)) {<br>
> +             return drm_mode_getplaneresources(tcp, arg);<br>
> +     }<br>
> +<br>
> +     if (_IOC_NR(code) == _IOC_NR(DRM_IOCTL_MODE_OBJ_GETPROPERTIES)) {<br>
> +             return drm_mode_obj_getproperties(tcp, arg);<br>
> +     }<br>
> +<br>
> +     if (_IOC_NR(code) == _IOC_NR(DRM_IOCTL_MODE_OBJ_SETPROPERTY)) {<br>
> +             return drm_mode_obj_setproperty(tcp, arg);<br>
> +     }<br>
> +<br>
> +     return RVAL_DECODED;<br>
> +}<br>
> +<br>
> +#endif /* HAVE_DRM_H || HAVE_DRM_DRM_H */<br>
> diff --git a/ioctl.c b/ioctl.c<br>
> index b80292cb..b9aa9833 100644<br>
> --- a/ioctl.c<br>
> +++ b/ioctl.c<br>
> @@ -195,6 +195,10 @@ ioctl_decode_command_number(struct tcb *tcp)<br>
>                               return 1;<br>
>                       }<br>
>                       return 0;<br>
> +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H)<br>
> +             case 'd':<br>
> +                     return drm_decode_number(tcp, code);<br>
> +#endif<br>
I'd prefer having this code unconditional here and put a stub drm_decode_number<br>
in drm_ioctl.c, but it's more up to Dmitry.<br>
<br></blockquote><div> </div><div>Can you explain this part a little bit? I'm not sure I'm following. Since the </div><div>whole bulk of code in drm_ioctl.c is ifdefed, would that be a problem if this</div><div>part is used unconditionally?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
>               default:<br>
>                       return 0;<br>
>       }<br>
> @@ -264,6 +268,10 @@ ioctl_decode(struct tcb *tcp)<br>
>               return fs_x_ioctl(tcp, code, arg);<br>
>       case 0x22:<br>
>               return scsi_ioctl(tcp, code, arg);<br>
<br>
> +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H)<br>
> +     case 'd':<br>
> +             return drm_ioctl(tcp, code, arg);<br>
> +#endif<br>
Same here regarding drm_ioctl.<br>
<br>
>       case 'L':<br>
>               return loop_ioctl(tcp, code, arg);<br>
>  #ifdef HAVE_STRUCT_MTD_WRITE_REQ<br>
> diff --git a/xlat/<a href="http://drm_buf_desc_flags.in" rel="noreferrer" target="_blank">drm_buf_desc_flags.in</a> b/xlat/<a href="http://drm_buf_desc_flags.in" rel="noreferrer" target="_blank">drm_buf_desc_flags.in</a><br>
> new file mode 100644<br>
> index 00000000..64c41fa9<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_buf_desc_flags.in" rel="noreferrer" target="_blank">drm_buf_desc_flags.in</a><br>
> @@ -0,0 +1,5 @@<br>
"#sorted" is (probably) missing here.<br></blockquote><div><br></div><div>OK, I didn't notice this before. I will check all the files in xlat/</div><div>to make sure the comments are in place.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +_DRM_PAGE_ALIGN              0x01<br>
> +_DRM_AGP_BUFFER              0x02<br>
> +_DRM_SG_BUFFER               0x04<br>
> +_DRM_FB_BUFFER               0x08<br>
> +_DRM_PCI_BUFFER_RO   0x10<br>
> diff --git a/xlat/<a href="http://drm_capability.in" rel="noreferrer" target="_blank">drm_capability.in</a> b/xlat/<a href="http://drm_capability.in" rel="noreferrer" target="_blank">drm_capability.in</a><br>
> new file mode 100644<br>
> index 00000000..ac30d02f<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_capability.in" rel="noreferrer" target="_blank">drm_capability.in</a><br>
> @@ -0,0 +1,14 @@<br>
"#value_indexed" is missing here.<br>
<br>
> +DRM_CAP_DUMB_BUFFER          0x1<br>
> +DRM_CAP_VBLANK_HIGH_CRTC     0x2<br>
> +DRM_CAP_DUMB_PREFERRED_DEPTH 0x3<br>
> +DRM_CAP_DUMB_PREFER_SHADOW   0x4<br>
> +DRM_CAP_PRIME                        0x5<br>
> +DRM_CAP_TIMESTAMP_MONOTONIC  0x6<br>
> +DRM_CAP_ASYNC_PAGE_FLIP              0x7<br>
> +DRM_CAP_CURSOR_WIDTH         0x8<br>
> +DRM_CAP_CURSOR_HEIGHT                0x9<br>
> +DRM_CAP_ADDFB2_MODIFIERS     0x10<br>
> +DRM_CAP_PAGE_FLIP_TARGET     0x11<br>
> +DRM_CAP_CRTC_IN_VBLANK_EVENT 0x12<br>
> +DRM_CAP_SYNCOBJ                      0x13<br>
> +DRM_CAP_SYNCOBJ_TIMELINE     0x14<br>
> diff --git a/xlat/<a href="http://drm_client_capability.in" rel="noreferrer" target="_blank">drm_client_capability.in</a> b/xlat/<a href="http://drm_client_capability.in" rel="noreferrer" target="_blank">drm_client_capability.in</a><br>
> new file mode 100644<br>
> index 00000000..7c3d867c<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_client_capability.in" rel="noreferrer" target="_blank">drm_client_capability.in</a><br>
> @@ -0,0 +1,5 @@<br>
"#value_indexed" is missing here.<br>
<br>
> +DRM_CLIENT_CAP_STEREO_3D             1<br>
> +DRM_CLIENT_CAP_UNIVERSAL_PLANES              2<br>
> +DRM_CLIENT_CAP_ATOMIC                        3<br>
> +DRM_CLIENT_CAP_ASPECT_RATIO          4<br>
> +DRM_CLIENT_CAP_WRITEBACK_CONNECTORS  5<br>
> diff --git a/xlat/<a href="http://drm_control_func.in" rel="noreferrer" target="_blank">drm_control_func.in</a> b/xlat/<a href="http://drm_control_func.in" rel="noreferrer" target="_blank">drm_control_func.in</a><br>
> new file mode 100644<br>
> index 00000000..c270bc62<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_control_func.in" rel="noreferrer" target="_blank">drm_control_func.in</a><br>
> @@ -0,0 +1,4 @@<br>
"#value_indexed" is missing here.<br>
<br>
> +DRM_ADD_COMMAND              0<br>
> +DRM_RM_COMMAND               1<br>
> +DRM_INST_HANDLER     2<br>
> +DRM_UNINST_HANDLER   3<br>
> diff --git a/xlat/<a href="http://drm_crtc_sequence_flags.in" rel="noreferrer" target="_blank">drm_crtc_sequence_flags.in</a> b/xlat/<a href="http://drm_crtc_sequence_flags.in" rel="noreferrer" target="_blank">drm_crtc_sequence_flags.in</a><br>
> new file mode 100644<br>
> index 00000000..8283287d<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_crtc_sequence_flags.in" rel="noreferrer" target="_blank">drm_crtc_sequence_flags.in</a><br>
> @@ -0,0 +1,2 @@<br>
"#sorted" is (probably) missing here.<br>
<br>
> +DRM_CRTC_SEQUENCE_RELATIVE   0x00000001<br>
> +DRM_CRTC_SEQUENCE_NEXT_ON_MISS       0x00000002<br>
> diff --git a/xlat/<a href="http://drm_ctx_flags.in" rel="noreferrer" target="_blank">drm_ctx_flags.in</a> b/xlat/<a href="http://drm_ctx_flags.in" rel="noreferrer" target="_blank">drm_ctx_flags.in</a><br>
> new file mode 100644<br>
> index 00000000..4225a19b<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_ctx_flags.in" rel="noreferrer" target="_blank">drm_ctx_flags.in</a><br>
> @@ -0,0 +1,2 @@<br>
"#sorted" is (probably) missing here.<br>
<br>
> +_DRM_CONTEXT_PRESERVED       0x01<br>
> +_DRM_CONTEXT_2DONLY  0x02<br>
> diff --git a/xlat/<a href="http://drm_lock_flags.in" rel="noreferrer" target="_blank">drm_lock_flags.in</a> b/xlat/<a href="http://drm_lock_flags.in" rel="noreferrer" target="_blank">drm_lock_flags.in</a><br>
> new file mode 100644<br>
> index 00000000..960d946f<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_lock_flags.in" rel="noreferrer" target="_blank">drm_lock_flags.in</a><br>
> @@ -0,0 +1,6 @@<br>
"#sorted" is (probably) missing here.<br>
<br>
> +_DRM_LOCK_READY              0x01<br>
> +_DRM_LOCK_QUIESCENT  0x02<br>
> +_DRM_LOCK_FLUSH              0x04<br>
> +_DRM_LOCK_FLUSH_ALL  0x08<br>
> +_DRM_HALT_ALL_QUEUES 0x10<br>
> +_DRM_HALT_CUR_QUEUES 0x20<br>
> diff --git a/xlat/<a href="http://drm_map_flags.in" rel="noreferrer" target="_blank">drm_map_flags.in</a> b/xlat/<a href="http://drm_map_flags.in" rel="noreferrer" target="_blank">drm_map_flags.in</a><br>
> new file mode 100644<br>
> index 00000000..9cd46901<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_map_flags.in" rel="noreferrer" target="_blank">drm_map_flags.in</a><br>
"#sorted" is (probably) missing here.<br>
<br>
> @@ -0,0 +1,8 @@<br>
> +_DRM_RESTRICTED              0x01<br>
> +_DRM_READ_ONLY               0x02<br>
> +_DRM_LOCKED          0x04<br>
> +_DRM_KERNEL          0x08<br>
> +_DRM_WRITE_COMBINING 0x10<br>
> +_DRM_CONTAINS_LOCK   0x20<br>
> +_DRM_REMOVABLE               0x40<br>
> +_DRM_DRIVER          0x80<br>
> diff --git a/xlat/<a href="http://drm_map_type.in" rel="noreferrer" target="_blank">drm_map_type.in</a> b/xlat/<a href="http://drm_map_type.in" rel="noreferrer" target="_blank">drm_map_type.in</a><br>
> new file mode 100644<br>
> index 00000000..6a51a66a<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_map_type.in" rel="noreferrer" target="_blank">drm_map_type.in</a><br>
> @@ -0,0 +1,6 @@<br>
"#value_indexed" is missing here.<br>
<br>
> +_DRM_FRAME_BUFFER    0<br>
> +_DRM_REGISTERS               1<br>
> +_DRM_SHM             2<br>
> +_DRM_AGP             3<br>
> +_DRM_SCATTER_GATHER  4<br>
> +_DRM_CONSISTENT              5<br>
> diff --git a/xlat/<a href="http://drm_mode_encoder_type.in" rel="noreferrer" target="_blank">drm_mode_encoder_type.in</a> b/xlat/<a href="http://drm_mode_encoder_type.in" rel="noreferrer" target="_blank">drm_mode_encoder_type.in</a><br>
> new file mode 100644<br>
> index 00000000..cbd67918<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_mode_encoder_type.in" rel="noreferrer" target="_blank">drm_mode_encoder_type.in</a><br>
> @@ -0,0 +1,9 @@<br>
"#value_indexed" is missing here.<br>
<br>
> +DRM_MODE_ENCODER_NONE                0<br>
> +DRM_MODE_ENCODER_DAC         1<br>
> +DRM_MODE_ENCODER_TMDS                2<br>
> +DRM_MODE_ENCODER_LVDS                3<br>
> +DRM_MODE_ENCODER_TVDAC               4<br>
> +DRM_MODE_ENCODER_VIRTUAL     5<br>
> +DRM_MODE_ENCODER_DSI         6<br>
> +DRM_MODE_ENCODER_DPMST               7<br>
> +DRM_MODE_ENCODER_DPI         8<br>
> diff --git a/xlat/<a href="http://drm_mode_flags.in" rel="noreferrer" target="_blank">drm_mode_flags.in</a> b/xlat/<a href="http://drm_mode_flags.in" rel="noreferrer" target="_blank">drm_mode_flags.in</a><br>
> new file mode 100644<br>
> index 00000000..d862689c<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_mode_flags.in" rel="noreferrer" target="_blank">drm_mode_flags.in</a><br>
> @@ -0,0 +1,6 @@<br>
"#sorted" is (probably) missing here.<br>
<br>
> +DRM_MODE_FLAG_PIC_AR_MASK    (0x0F << 19)<br>
I doubt this one should be here.<br>
<br>
> +DRM_MODE_FLAG_PIC_AR_NONE    (0 << 19)<br>
> +DRM_MODE_FLAG_PIC_AR_4_3     (1 << 19)<br>
> +DRM_MODE_FLAG_PIC_AR_16_9    (2 << 19)<br>
> +DRM_MODE_FLAG_PIC_AR_64_27   (3 << 19)<br>
> +DRM_MODE_FLAG_PIC_AR_256_135 (4 << 19)<br>
> diff --git a/xlat/<a href="http://drm_mode_page_flip_flags.in" rel="noreferrer" target="_blank">drm_mode_page_flip_flags.in</a> b/xlat/<a href="http://drm_mode_page_flip_flags.in" rel="noreferrer" target="_blank">drm_mode_page_flip_flags.in</a><br>
> new file mode 100644<br>
> index 00000000..b0006d84<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_mode_page_flip_flags.in" rel="noreferrer" target="_blank">drm_mode_page_flip_flags.in</a><br>
> @@ -0,0 +1,6 @@<br>
<br>
> +DRM_MODE_PAGE_FLIP_EVENT<br>
> +DRM_MODE_PAGE_FLIP_ASYNC<br>
> +DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE<br>
> +DRM_MODE_PAGE_FLIP_TARGET_RELATIVE<br>
Those doesn't feel to be arch-specific, so their values can be provided<br>
here.<br>
<br>
> +DRM_MODE_PAGE_FLIP_TARGET<br>
It should be before DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE and<br>
DRM_MODE_PAGE_FLIP_TARGET_RELATIVE in order to be decoded by printflags.<br>
<br>
> +DRM_MODE_PAGE_FLIP_FLAGS<br>
It should go first in order to be decoded bt printflags. I'm not sure,<br>
however, whether this is a part of kernel API.<br>
<br>
> diff --git a/xlat/<a href="http://drm_mode_type.in" rel="noreferrer" target="_blank">drm_mode_type.in</a> b/xlat/<a href="http://drm_mode_type.in" rel="noreferrer" target="_blank">drm_mode_type.in</a><br>
> new file mode 100644<br>
> index 00000000..3e9a9094<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_mode_type.in" rel="noreferrer" target="_blank">drm_mode_type.in</a><br>
> @@ -0,0 +1,8 @@<br>
"#sorted" is (probably) missing here.<br>
<br>
> +DRM_MODE_TYPE_BUILTIN                (1 << 0)<br>
> +DRM_MODE_TYPE_CLOCK_C                (1 << 1 | DRM_MODE_TYPE_BUILTIN)<br>
> +DRM_MODE_TYPE_CRTC_C         (1 << 2 | DRM_MODE_TYPE_BUILTIN)<br>
> +DRM_MODE_TYPE_PREFERRED              (1 << 3)<br>
> +DRM_MODE_TYPE_DEFAULT                (1 << 4)<br>
> +DRM_MODE_TYPE_USERDEF                (1 << 5)<br>
> +DRM_MODE_TYPE_DRIVER         (1 << 6)<br>
> +DRM_MODE_TYPE_ALL            (DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_USERDEF | DRM_MODE_TYPE_DRIVER)<br>
> diff --git a/xlat/<a href="http://drm_modeset_cmd.in" rel="noreferrer" target="_blank">drm_modeset_cmd.in</a> b/xlat/<a href="http://drm_modeset_cmd.in" rel="noreferrer" target="_blank">drm_modeset_cmd.in</a><br>
> new file mode 100644<br>
> index 00000000..a58a9076<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_modeset_cmd.in" rel="noreferrer" target="_blank">drm_modeset_cmd.in</a><br>
> @@ -0,0 +1,2 @@<br>
"#value_indexed" is missing here.<br>
<br>
> +_DRM_PRE_MODESET     1<br>
> +_DRM_POST_MODESET    2<br>
<br>
> diff --git a/xlat/<a href="http://drm_stat_type.in" rel="noreferrer" target="_blank">drm_stat_type.in</a> b/xlat/<a href="http://drm_stat_type.in" rel="noreferrer" target="_blank">drm_stat_type.in</a><br>
> new file mode 100644<br>
> index 00000000..b15f18d5<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_stat_type.in" rel="noreferrer" target="_blank">drm_stat_type.in</a><br>
> @@ -0,0 +1,15 @@<br>
"#value_indexed" is missing here.<br>
<br>
> +_DRM_STAT_LOCK               0<br>
> +_DRM_STAT_OPENS              1<br>
<br>
> diff --git a/xlat/<a href="http://drm_syncobj_flags.in" rel="noreferrer" target="_blank">drm_syncobj_flags.in</a> b/xlat/<a href="http://drm_syncobj_flags.in" rel="noreferrer" target="_blank">drm_syncobj_flags.in</a><br>
> new file mode 100644<br>
> index 00000000..b047f1c0<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_syncobj_flags.in" rel="noreferrer" target="_blank">drm_syncobj_flags.in</a><br>
> @@ -0,0 +1 @@<br>
"#sorted" is missing here.<br>
<br>
> +DRM_SYNCOBJ_CREATE_SIGNALED  (1 << 0)<br>
> diff --git a/xlat/<a href="http://drm_syncobj_wait_flags.in" rel="noreferrer" target="_blank">drm_syncobj_wait_flags.in</a> b/xlat/<a href="http://drm_syncobj_wait_flags.in" rel="noreferrer" target="_blank">drm_syncobj_wait_flags.in</a><br>
> new file mode 100644<br>
> index 00000000..185f94b1<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_syncobj_wait_flags.in" rel="noreferrer" target="_blank">drm_syncobj_wait_flags.in</a><br>
> @@ -0,0 +1,3 @@<br>
"#sorted" is missing here.<br>
<br>
> +DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL              (1 << 0)<br>
> +DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT       (1 << 1)<br>
> +DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE        (1 << 2)<br>
<br>
<br>
> diff --git a/xlat/<a href="http://drm_vblank_seq_type.in" rel="noreferrer" target="_blank">drm_vblank_seq_type.in</a> b/xlat/<a href="http://drm_vblank_seq_type.in" rel="noreferrer" target="_blank">drm_vblank_seq_type.in</a><br>
> new file mode 100644<br>
> index 00000000..6460cf55<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://drm_vblank_seq_type.in" rel="noreferrer" target="_blank">drm_vblank_seq_type.in</a><br>
> @@ -0,0 +1,8 @@<br>
"#sorted" is missing here.<br>
<br>
> +_DRM_VBLANK_ABSOLUTE         0x0<br>
> +_DRM_VBLANK_RELATIVE         0x1<br>
<br>
> +_DRM_VBLANK_HIGH_CRTC_MASK   0x0000003e<br>
I'm not sure about leading zeroes here, it complicates reading.<br>
<br>
> +_DRM_VBLANK_EVENT            0x4000000<br>
> +_DRM_VBLANK_FLIP             0x8000000<br>
> +_DRM_VBLANK_NEXTONMISS               0x10000000<br>
> +_DRM_VBLANK_SECONDARY                0x20000000<br>
> +_DRM_VBLANK_SIGNAL           0x40000000<br>
<br>
<br>
It's also likely worth considering abbreviating output for some<br>
informational ioctls.<br></blockquote><div><br></div><div>You mean something like -v option? I've been thinking about this and</div><div>planing to add support for this in the next patchset.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
-- <br>
Strace-devel mailing list<br>
<a href="mailto:Strace-devel@lists.strace.io" target="_blank">Strace-devel@lists.strace.io</a><br>
<a href="https://lists.strace.io/mailman/listinfo/strace-devel" rel="noreferrer" target="_blank">https://lists.strace.io/mailman/listinfo/strace-devel</a><br>
</blockquote></div></div>