* Re: [PATCH v7 10/22] s390: vfio-ap: sysfs interfaces to configure adapters
2018-07-31 12:00 ` [PATCH v7 10/22] s390: vfio-ap: sysfs interfaces to configure adapters Christian Borntraeger
@ 2018-08-01 21:43 ` Tony Krowiak
0 siblings, 0 replies; 2+ messages in thread
From: Tony Krowiak @ 2018-08-01 21:43 UTC (permalink / raw)
To: linux-s390, kvm
On 07/27/2018 11:04 AM, Thomas Huth wrote:
> On 07/26/2018 09:54 PM, Christian Borntraeger wrote:
> [...]
>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
>> index e4e8d738a043..07911fb9cdce 100644
>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> @@ -15,7 +15,6 @@
>> #define VFOP_AP_MDEV_TYPE_HWVIRT "passthrough"
>> #define VFIO_AP_MDEV_NAME_HWVIRT "VFIO AP Passthrough Device"
>>
>> -
> Unnecessary white space change.
It is indeed, to be fixed.
>
>> static void vfio_ap_matrix_init(struct ap_config_info *info,
>> struct ap_matrix *matrix)
>> {
>> @@ -103,9 +102,127 @@ static struct attribute_group *vfio_ap_mdev_type_groups[] = {
>> NULL,
>> };
>>
>> +/**
>> + * assign_adapter_store
>> + *
>> + * @dev: the matrix device
>> + * @attr: a mediated matrix device attribute
>> + * @buf: a buffer containing the adapter ID (APID) to be assigned
>> + * @count: the number of bytes in @buf
>> + *
>> + * Parses the APID from @buf and assigns it to the mediated matrix device.
>> + *
>> + * Returns the number of bytes processed if the APID is valid; otherwise returns
>> + * an error.
>> + */
>> +static ssize_t assign_adapter_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + int ret;
>> + unsigned long apid;
>> + struct mdev_device *mdev = mdev_from_dev(dev);
>> + struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
>> + unsigned long max_apid = matrix_mdev->matrix.apm_max;
>> +
>> + ret = kstrtoul(buf, 0, &apid);
>> + if (ret || (apid > max_apid)) {
> You can remove the parentheses around "apid > max_apid".
My personal opinion is that the parenthesis make it easier to
understand .... there is no need to scour the mind to recall
operator precedence rules.
>
>> + pr_warn("%s: %s: adapter id '%s' not a value from 0 to %02lu(%#04lx)\n",
>> + VFIO_AP_MODULE_NAME, __func__, buf, max_apid, max_apid);
>> +
>> + if (!ret)
>> + ret = -EINVAL;
>> +
>> + return ret;
>> + }
>> +
>> + /* Set the bit in the AP mask (APM) corresponding to the AP adapter
>> + * number (APID). The bits in the mask, from most significant to least
>> + * significant bit, correspond to APIDs 0-255.
>> + */
>> + mutex_lock(&matrix_dev.lock);
>> + set_bit_inv(apid, matrix_mdev->matrix.apm);
>> + ret = count;
>> +
>> + mutex_unlock(&matrix_dev.lock);
>> +
>> + return ret;
>> +}
>> +static DEVICE_ATTR_WO(assign_adapter);
>> +
>> +/**
>> + * unassign_adapter_store
>> + *
>> + * @dev: the matrix device
>> + * @attr: a mediated matrix device attribute
>> + * @buf: a buffer containing the adapter ID (APID) to be assigned
>> + * @count: the number of bytes in @buf
>> + *
>> + * Parses the APID from @buf and unassigns it from the mediated matrix device.
>> + * The APID must be a valid value
>> + *
>> + * Returns the number of bytes processed if the APID is valid; otherwise returns
>> + * an error.
>> + */
>> +static ssize_t unassign_adapter_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + int ret;
>> + unsigned long apid;
>> + struct mdev_device *mdev = mdev_from_dev(dev);
>> + struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
>> +
>> + unsigned long max_apid = matrix_mdev->matrix.apm_max;
>> +
>> + ret = kstrtoul(buf, 0, &apid);
>> + if (ret || (apid > max_apid)) {
> dito
ditto
>
>> + pr_warn("%s: %s: adapter id '%s' must be a value from 0 to %02lu(%#04lx)\n",
>> + VFIO_AP_MODULE_NAME, __func__, buf, max_apid, max_apid);
>> +
>> + if (!ret)
>> + ret = -EINVAL;
>> +
>> + return ret;
>> + }
>> +
>> + mutex_lock(&matrix_dev.lock);
>> + if (!test_bit_inv(apid, matrix_mdev->matrix.apm)) {
>> + pr_warn("%s: %s: adapter id %02lu(%#04lx) not assigned\n",
>> + VFIO_AP_MODULE_NAME, __func__, apid, apid);
>> + ret = -ENODEV;
>> + goto done;
>> + }
>> +
>> + clear_bit_inv((unsigned long)apid, matrix_mdev->matrix.apm);
>> + ret = count;
>> +
>> +done:
>> + mutex_unlock(&matrix_dev.lock);
>> +
>> + return ret;
>> +}
>> +DEVICE_ATTR_WO(unassign_adapter);
>> +
>> +static struct attribute *vfio_ap_mdev_attrs[] = {
>> + &dev_attr_assign_adapter.attr,
>> + &dev_attr_unassign_adapter.attr,
>> + NULL
>> +};
>> +
>> +static struct attribute_group vfio_ap_mdev_attr_group = {
>> + .attrs = vfio_ap_mdev_attrs
>> +};
>> +
>> +static const struct attribute_group *vfio_ap_mdev_attr_groups[] = {
>> + &vfio_ap_mdev_attr_group,
>> + NULL
>> +};
>> +
>> static const struct mdev_parent_ops vfio_ap_matrix_ops = {
>> .owner = THIS_MODULE,
>> .supported_type_groups = vfio_ap_mdev_type_groups,
>> + .mdev_attr_groups = vfio_ap_mdev_attr_groups,
>> .create = vfio_ap_mdev_create,
>> .remove = vfio_ap_mdev_remove,
>> };
>>
> Thomas
>
^ permalink raw reply [flat|nested] 2+ messages in thread