Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 12 May 2023 14:51:35 +0200
From:   Heiko Carstens <hca@linux.ibm.com>
To:     Steffen Eiden <seiden@linux.ibm.com>
Cc:     kvm@vger.kernel.org, linux-s390@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Viktor Mihajlovski <mihajlov@linux.ibm.com>,
        Janosch Frank <frankja@linux.ibm.com>,
        Claudio Imbrenda <imbrenda@linux.ibm.com>,
        Nico Boehr <nrb@linux.ibm.com>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Hendrik Brueckner <brueckner@linux.ibm.com>
Subject: Re: [PATCH 1/5] s390/uvdevice: Add info IOCTL
Message-ID: <ZF42V9fJFtRLR1Ua@osiris>
References: <20230512093153.206378-1-seiden@linux.ibm.com>
 <20230512093153.206378-2-seiden@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230512093153.206378-2-seiden@linux.ibm.com>
Precedence: bulk

On Fri, May 12, 2023 at 11:31:49AM +0200, 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>
> ---
>  arch/s390/include/uapi/asm/uvdevice.h | 45 +++++++++++++++-
>  drivers/s390/char/uvdevice.c          | 77 ++++++++++++++++++++++++---
>  2 files changed, 114 insertions(+), 8 deletions(-)
...
> +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)
> +			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);

Just a minor thing: please use

		__set_bit(i, (unsigned long *)&info->supp_uv_cmds);

set_bit() is the atomic variant which may even generate a compare and swap
loop (dependent on config options) to set bits, which is not what is needed
here. Didn't see that in previous version.
So if atomicity is not needed, please use __set_bit() instead.