On 24/11/2023 22:00, Carsten Haitzler wrote:
- Maybe the most important issue is the discussion at [1]: AFAICT all
these uapi struct changes will cause the DRM_IOCTL_* command values to change on PCuABI kernels, which will break existing arm64 binaries. The approach we went for in the MMC driver is to hardcode the struct size in the command definition, so that it remains unchanged despite the native struct growing in PCuABI. It's not great, honestly, but the only plausible alternative seems to consider different command values in compat64 (i.e. a separate top-level compat handler that considers the appropriate command value). Certainly not unprecedented, but quite a bit of extra work.
interestingly... existing arm64 binaries haven't broken. i literally am using the debian arm64 drm package binaries... they work on my kernel. tbh i actually haven't dug into why. but i can run all the arm64 compiled userspace tests with an arm64 libdrm right from upstream debian pkgs and ... they work. :) my newly built purecap ones also work...
Well.. a bit of digging... it's because the drm_icotl dispatch func strips off the parts of the ioctl # that encode size etc. ... it just strips down to ioctl "number" that drm defines.
unsigned int nr = DRM_IOCTL_NR(cmd);
that means that for:
#define DRM_IOCTL_MODE_GETRESOURCES DRM_IOWR(0xA0, struct drm_mode_card_res)
I end up with 0xA0 ... that's all drm cares about. it then dispatches in its local table from here (with range checks etc. done before) so... it seems it works by either deliberate design or by luck of the design... but it will consistently work unless drm core code stops stripping this out.
so ... i'm safe. :) it ends up with 2 different ioctl values coming into the kernel but it ends up stripping out the size bits along the way and calling the right func which means the code can continue as it does now. i'm not going to suddenly see it break - i wasn't getting magically lucky.
Very interesting, in fact it's even better than that: drm_ioctl() also looks at the size (_IOC_SIZE(cmd)) and uses that to decide how much to copy in/out! It's as if drm_ioctl() was already meant to handle differences in size between compat and native struct... Very nice, for once that's something forward-thinking in an ioctl handler :D In fact that makes me feel like we could use the same approach in other situations where the command value changes, including the MMC one we've already encountered. Need to think about it some more.
Kevin