Re: [PATCH v2 1/6] s390/uvdevice: Add info IOCTL

[Janosch Frank <frankja@linux.ibm.com>]

On 5/19/23 11:37, Steffen Eiden wrote:
> Add an IOCTL that allows userspace to find out which IOCTLs the uvdevice
> supports without trial and error.
> 
> Explicitly expose the IOCTL nr for the request types.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
[...]
>   #endif /* __S390_ASM_UVDEVICE_H */
> diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
> index 1d40457c7b10..61c7f284c5c5 100644
> --- a/drivers/s390/char/uvdevice.c
> +++ b/drivers/s390/char/uvdevice.c
> @@ -20,6 +20,7 @@
>    *  channel for userspace to the Ultravisor.
>    */
>   
> +#include "asm-generic/ioctl.h"
>   #include <linux/module.h>
>   #include <linux/kernel.h>
>   #include <linux/miscdevice.h>
> @@ -32,6 +33,51 @@
>   #include <asm/uvdevice.h>
>   #include <asm/uv.h>
>   
> +/* Mapping from IOCTL-nr to UVC-bit */
> +static const u64 ioctl_nr_to_uvc_bit[] __initconst = {

This can be u32 since cmds are u16.

> +	[UVIO_IOCTL_UVDEV_INFO_NR] = -1UL,

#define BIT_UVIO_INTERNAL U32_MAX

> +	[UVIO_IOCTL_ATT_NR] = BIT_UVC_CMD_RETR_ATTEST,
> +};
> +
> +static_assert(ARRAY_SIZE(ioctl_nr_to_uvc_bit) == UVIO_IOCTL_NUM_IOCTLS);
> +
> +static struct uvio_uvdev_info uvdev_info = {
> +	.supp_uvio_cmds = GENMASK_ULL(UVIO_IOCTL_NUM_IOCTLS - 1, 0),
> +};
> +
> +static void __init set_supp_uv_cmds(struct uvio_uvdev_info *info)
> +{
> +	int i;
> +
> +	for (i = 0; i < UVIO_IOCTL_NUM_IOCTLS; i++) {
> +		if (ioctl_nr_to_uvc_bit[i] == -1UL)

BIT_UVIO_INTERNAL

> +			continue;
> +		if (!test_bit_inv(ioctl_nr_to_uvc_bit[i], uv_info.inst_calls_list))
> +			continue;
> +		__set_bit(i, (unsigned long *)&info->supp_uv_cmds);

Move the cast to the caller and make this function take a "unsigned long 
*)" argument.

> +	}
> +}
> +
> +/**
> + * uvio_uvdev_info() - get information about the uvdevice
> + *
> + * @uv_ioctl: ioctl control block
> + *
> + * Lists all supported IOCTLs by this uvdevice

Lists all IOCTLs that are supported by this uvdevice

[...]

Rest seems fine to me.