| Message ID | 20200407192015.19887-7-akrowiak@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390/vfio-ap: dynamic configuration support | expand |
On Tue, 7 Apr 2020 15:20:06 -0400 Tony Krowiak <akrowiak@linux.ibm.com> wrote: > The matrix of adapters and domains configured in a guest's CRYCB may > differ from the matrix of adapters and domains assigned to the matrix mdev, > so this patch introduces a sysfs attribute to display the CRYCB of a guest > using the matrix mdev. For a matrix mdev denoted by $uuid, the crycb for a > guest using the matrix mdev can be displayed as follows: > > cat /sys/devices/vfio_ap/matrix/$uuid/guest_matrix > > If a guest is not using the matrix mdev at the time the crycb is displayed, > an error (ENODEV) will be returned. > > Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> > --- > drivers/s390/crypto/vfio_ap_ops.c | 58 +++++++++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > +static DEVICE_ATTR_RO(guest_matrix); Hm... should information like the guest configuration be readable by everyone? Or should it be restricted a bit more? > + > static struct attribute *vfio_ap_mdev_attrs[] = { > &dev_attr_assign_adapter.attr, > &dev_attr_unassign_adapter.attr, > @@ -1050,6 +1107,7 @@ static struct attribute *vfio_ap_mdev_attrs[] = { > &dev_attr_unassign_control_domain.attr, > &dev_attr_control_domains.attr, > &dev_attr_matrix.attr, > + &dev_attr_guest_matrix.attr, > NULL, > }; >
On 4/8/20 6:33 AM, Cornelia Huck wrote: > On Tue, 7 Apr 2020 15:20:06 -0400 > Tony Krowiak <akrowiak@linux.ibm.com> wrote: > >> The matrix of adapters and domains configured in a guest's CRYCB may >> differ from the matrix of adapters and domains assigned to the matrix mdev, >> so this patch introduces a sysfs attribute to display the CRYCB of a guest >> using the matrix mdev. For a matrix mdev denoted by $uuid, the crycb for a >> guest using the matrix mdev can be displayed as follows: >> >> cat /sys/devices/vfio_ap/matrix/$uuid/guest_matrix >> >> If a guest is not using the matrix mdev at the time the crycb is displayed, >> an error (ENODEV) will be returned. >> >> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> >> --- >> drivers/s390/crypto/vfio_ap_ops.c | 58 +++++++++++++++++++++++++++++++ >> 1 file changed, 58 insertions(+) >> +static DEVICE_ATTR_RO(guest_matrix); > Hm... should information like the guest configuration be readable by > everyone? Or should it be restricted a bit more? Why? The matrix attribute already displays the APQNs of the queues assigned to the matrix mdev. The guest_matrix attribute merely displays a subset of the matrix (i.e., the APQNs assigned to the mdev that reference queue devices bound to the vfio_ap device driver). How can this be restricted? > >> + >> static struct attribute *vfio_ap_mdev_attrs[] = { >> &dev_attr_assign_adapter.attr, >> &dev_attr_unassign_adapter.attr, >> @@ -1050,6 +1107,7 @@ static struct attribute *vfio_ap_mdev_attrs[] = { >> &dev_attr_unassign_control_domain.attr, >> &dev_attr_control_domains.attr, >> &dev_attr_matrix.attr, >> + &dev_attr_guest_matrix.attr, >> NULL, >> }; >>
On Wed, 8 Apr 2020 12:38:49 -0400 Tony Krowiak <akrowiak@linux.ibm.com> wrote: > On 4/8/20 6:33 AM, Cornelia Huck wrote: > > On Tue, 7 Apr 2020 15:20:06 -0400 > > Tony Krowiak <akrowiak@linux.ibm.com> wrote: > > > >> The matrix of adapters and domains configured in a guest's CRYCB may > >> differ from the matrix of adapters and domains assigned to the matrix mdev, > >> so this patch introduces a sysfs attribute to display the CRYCB of a guest > >> using the matrix mdev. For a matrix mdev denoted by $uuid, the crycb for a > >> guest using the matrix mdev can be displayed as follows: > >> > >> cat /sys/devices/vfio_ap/matrix/$uuid/guest_matrix > >> > >> If a guest is not using the matrix mdev at the time the crycb is displayed, > >> an error (ENODEV) will be returned. > >> > >> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> > >> --- > >> drivers/s390/crypto/vfio_ap_ops.c | 58 +++++++++++++++++++++++++++++++ > >> 1 file changed, 58 insertions(+) > >> +static DEVICE_ATTR_RO(guest_matrix); > > Hm... should information like the guest configuration be readable by > > everyone? Or should it be restricted a bit more? > > Why? The matrix attribute already displays the APQNs of the queues > assigned to the matrix mdev. The guest_matrix attribute merely displays > a subset of the matrix (i.e., the APQNs assigned to the mdev that reference > queue devices bound to the vfio_ap device driver). > > How can this be restricted? I was thinking of using e.g. 400 instead of 444 for the permissions. I'm not sure if the info about what subset of the queues the guest actually uses is all that interesting (except for managing guests); but if I see guest information being exposed, I get a little wary, so I just stumbled over this. Maybe I'll come back to that once I have looked at the series in more detail, but this might not be a problem at all. > > > > >> + > >> static struct attribute *vfio_ap_mdev_attrs[] = { > >> &dev_attr_assign_adapter.attr, > >> &dev_attr_unassign_adapter.attr, > >> @@ -1050,6 +1107,7 @@ static struct attribute *vfio_ap_mdev_attrs[] = { > >> &dev_attr_unassign_control_domain.attr, > >> &dev_attr_control_domains.attr, > >> &dev_attr_matrix.attr, > >> + &dev_attr_guest_matrix.attr, > >> NULL, > >> }; > >> >
On 4/8/20 12:46 PM, Cornelia Huck wrote: > On Wed, 8 Apr 2020 12:38:49 -0400 > Tony Krowiak <akrowiak@linux.ibm.com> wrote: > >> On 4/8/20 6:33 AM, Cornelia Huck wrote: >>> On Tue, 7 Apr 2020 15:20:06 -0400 >>> Tony Krowiak <akrowiak@linux.ibm.com> wrote: >>> >>>> The matrix of adapters and domains configured in a guest's CRYCB may >>>> differ from the matrix of adapters and domains assigned to the matrix mdev, >>>> so this patch introduces a sysfs attribute to display the CRYCB of a guest >>>> using the matrix mdev. For a matrix mdev denoted by $uuid, the crycb for a >>>> guest using the matrix mdev can be displayed as follows: >>>> >>>> cat /sys/devices/vfio_ap/matrix/$uuid/guest_matrix >>>> >>>> If a guest is not using the matrix mdev at the time the crycb is displayed, >>>> an error (ENODEV) will be returned. >>>> >>>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> >>>> --- >>>> drivers/s390/crypto/vfio_ap_ops.c | 58 +++++++++++++++++++++++++++++++ >>>> 1 file changed, 58 insertions(+) >>>> +static DEVICE_ATTR_RO(guest_matrix); >>> Hm... should information like the guest configuration be readable by >>> everyone? Or should it be restricted a bit more? >> Why? The matrix attribute already displays the APQNs of the queues >> assigned to the matrix mdev. The guest_matrix attribute merely displays >> a subset of the matrix (i.e., the APQNs assigned to the mdev that reference >> queue devices bound to the vfio_ap device driver). >> >> How can this be restricted? > I was thinking of using e.g. 400 instead of 444 for the permissions. > > I'm not sure if the info about what subset of the queues the guest > actually uses is all that interesting (except for managing guests); but > if I see guest information being exposed, I get a little wary, so I > just stumbled over this. > > Maybe I'll come back to that once I have looked at the series in more > detail, but this might not be a problem at all. I created this patch primarily because I found it very helpful when testing. It saved me from logging on to the guest and executing lszcrypt to verify the AP configuration for the guest. I thought this might also add value for a system administrator responsible for KVM guests. It would not be a problem to remove this patch or change the permissions for the attribute. Let me know when you complete the series review. Thanks. > >>> >>>> + >>>> static struct attribute *vfio_ap_mdev_attrs[] = { >>>> &dev_attr_assign_adapter.attr, >>>> &dev_attr_unassign_adapter.attr, >>>> @@ -1050,6 +1107,7 @@ static struct attribute *vfio_ap_mdev_attrs[] = { >>>> &dev_attr_unassign_control_domain.attr, >>>> &dev_attr_control_domains.attr, >>>> &dev_attr_matrix.attr, >>>> + &dev_attr_guest_matrix.attr, >>>> NULL, >>>> }; >>>>
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index b8b678032ab7..beb2e68a2b1c 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1041,6 +1041,63 @@ static ssize_t matrix_show(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RO(matrix); +static ssize_t guest_matrix_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct mdev_device *mdev = mdev_from_dev(dev); + struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev); + char *bufpos = buf; + unsigned long apid; + unsigned long apqi; + unsigned long apid1; + unsigned long apqi1; + unsigned long napm_bits = matrix_mdev->shadow_crycb.apm_max + 1; + unsigned long naqm_bits = matrix_mdev->shadow_crycb.aqm_max + 1; + int nchars = 0; + int n; + + if (!vfio_ap_mdev_has_crycb(matrix_mdev)) + return -ENODEV; + + apid1 = find_first_bit_inv(matrix_mdev->shadow_crycb.apm, napm_bits); + apqi1 = find_first_bit_inv(matrix_mdev->shadow_crycb.aqm, naqm_bits); + + mutex_lock(&matrix_dev->lock); + + if ((apid1 < napm_bits) && (apqi1 < naqm_bits)) { + for_each_set_bit_inv(apid, matrix_mdev->shadow_crycb.apm, + napm_bits) { + for_each_set_bit_inv(apqi, + matrix_mdev->shadow_crycb.aqm, + naqm_bits) { + n = sprintf(bufpos, "%02lx.%04lx\n", apid, + apqi); + bufpos += n; + nchars += n; + } + } + } else if (apid1 < napm_bits) { + for_each_set_bit_inv(apid, matrix_mdev->shadow_crycb.apm, + napm_bits) { + n = sprintf(bufpos, "%02lx.\n", apid); + bufpos += n; + nchars += n; + } + } else if (apqi1 < naqm_bits) { + for_each_set_bit_inv(apqi, matrix_mdev->shadow_crycb.aqm, + naqm_bits) { + n = sprintf(bufpos, ".%04lx\n", apqi); + bufpos += n; + nchars += n; + } + } + + mutex_unlock(&matrix_dev->lock); + + return nchars; +} +static DEVICE_ATTR_RO(guest_matrix); + static struct attribute *vfio_ap_mdev_attrs[] = { &dev_attr_assign_adapter.attr, &dev_attr_unassign_adapter.attr, @@ -1050,6 +1107,7 @@ static struct attribute *vfio_ap_mdev_attrs[] = { &dev_attr_unassign_control_domain.attr, &dev_attr_control_domains.attr, &dev_attr_matrix.attr, + &dev_attr_guest_matrix.attr, NULL, };
The matrix of adapters and domains configured in a guest's CRYCB may differ from the matrix of adapters and domains assigned to the matrix mdev, so this patch introduces a sysfs attribute to display the CRYCB of a guest using the matrix mdev. For a matrix mdev denoted by $uuid, the crycb for a guest using the matrix mdev can be displayed as follows: cat /sys/devices/vfio_ap/matrix/$uuid/guest_matrix If a guest is not using the matrix mdev at the time the crycb is displayed, an error (ENODEV) will be returned. Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> --- drivers/s390/crypto/vfio_ap_ops.c | 58 +++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)