Re: [PATCH] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION

[Laurent Vivier]

Le 26/02/2020 à 12:38, address@hidden a écrit :
> From: Chen Gang <address@hidden>
> 
> The other DRM_IOCTL_* commands will be done later.
> 
> Signed-off-by: Chen Gang <address@hidden>
> ---
>  linux-user/ioctls.h        |   3 +
>  linux-user/syscall.c       | 134 +++++++++++++++++++++++++++++++++++++
>  linux-user/syscall_defs.h  |  16 +++++
>  linux-user/syscall_types.h |  12 ++++
>  4 files changed, 165 insertions(+)
> 
> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> index 0defa1d8c1..c2294b48a0 100644
> --- a/linux-user/ioctls.h
> +++ b/linux-user/ioctls.h
> @@ -574,6 +574,9 @@
>    IOCTL_SPECIAL(SIOCDELRT, IOC_W, do_ioctl_rt,
>                  MK_PTR(MK_STRUCT(STRUCT_rtentry)))
>  
> +  IOCTL_SPECIAL(DRM_IOCTL_VERSION, IOC_RW, do_ioctl_drm,
> +                MK_PTR(MK_STRUCT(STRUCT_drm_version)))
> +

Rather than adding a specific function to process the structure, perhaps
we can add this in a generic way?

The problem with drm_version structure is the pointers to the strings.

Did you try to add a TYPE_STRING in
thunk_type_size()/thunk_type_align()/think_convert()/do_ioctl() to do that?

...

> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 152ec637cb..5e455a32af 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -1167,6 +1167,9 @@ struct target_rtc_pll_info {
>  #define TARGET_DM_TARGET_MSG          TARGET_IOWRU(0xfd, 0x0e)
>  #define TARGET_DM_DEV_SET_GEOMETRY    TARGET_IOWRU(0xfd, 0x0f)
>  
> +/* drm ioctls */
> +#define TARGET_DRM_IOCTL_VERSION      TARGET_IOWRU('d', 0x00)

Why do you use the TARGET_IOWRU variant?

Can't you use TARGET_IOWR('d', 0x00, struct target_drm_version)?

> +
>  /* from asm/termbits.h */
>  
>  #define TARGET_NCC 8
> @@ -2598,6 +2602,18 @@ struct target_mq_attr {
>      abi_long mq_curmsgs;
>  };
>  
> +struct target_drm_version {
> +    int version_major;
> +    int version_minor;
> +    int version_patchlevel;
> +    abi_ulong name_len;
> +    abi_ulong name;
> +    abi_ulong date_len;
> +    abi_ulong date;
> +    abi_ulong desc_len;
> +    abi_ulong desc;
> +};
> +
>  #include "socket.h"
>  
>  #include "errno_defs.h"
> diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
> index 4e12c1661e..52a031ad35 100644
> --- a/linux-user/syscall_types.h
> +++ b/linux-user/syscall_types.h
> @@ -292,6 +292,18 @@ STRUCT(dm_target_versions,
>  STRUCT(dm_target_msg,
>         TYPE_ULONGLONG) /* sector */
>  
> +/* TODO: use #ifdef 32 bit #else 64 bit, next */
> +STRUCT(drm_version,
> +       TYPE_INT, /* version_major */
> +       TYPE_INT, /* version_minor */
> +       TYPE_INT, /* version_patchlevel */
> +       TYPE_ULONG, /* name_len */
> +       TYPE_PTRVOID, /* name */
> +       TYPE_ULONG, /* date_len */
> +       TYPE_PTRVOID, /* date */
> +       TYPE_ULONG, /* desc_len */
> +       TYPE_PTRVOID) /* desc */
> +

After defining a TYPE_STRING, you could use MKPTR(TYPE_STRING) for name,
date and desc.

Thanks,
Laurent