Messages in this thread | | | Subject | Re: [PATCH v11 07/14] s390/vfio-ap: sysfs attribute to display the guest's matrix | From | Tony Krowiak <> | Date | Thu, 19 Nov 2020 13:15:25 -0500 |
| |
On 11/13/20 6:12 PM, Halil Pasic wrote: > On Fri, 13 Nov 2020 12:27:32 -0500 > Tony Krowiak <akrowiak@linux.ibm.com> wrote: > >> >> On 10/28/20 4:17 AM, Halil Pasic wrote: >>> On Thu, 22 Oct 2020 13:12:02 -0400 >>> Tony Krowiak <akrowiak@linux.ibm.com> wrote: >>> >>>> +static ssize_t guest_matrix_show(struct device *dev, >>>> + struct device_attribute *attr, char *buf) >>>> +{ >>>> + ssize_t nchars; >>>> + struct mdev_device *mdev = mdev_from_dev(dev); >>>> + struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev); >>>> + >>>> + if (!vfio_ap_mdev_has_crycb(matrix_mdev)) >>>> + return -ENODEV; >>> I'm wondering, would it make sense to have guest_matrix display the would >>> be guest matrix when we don't have a KVM? With the filtering in >>> place, the question in what guest_matrix would my (assign) matrix result >>> right now if I were to hook up my vfio_ap_mdev to a guest seems a >>> legitimate one. >> A couple of thoughts here: >> * The ENODEV informs the user that there is no guest running >> which makes sense to me given this interface displays the >> guest matrix. The alternative, which I considered, was to >> display an empty matrix (i.e., nothing). >> * This would be a pretty drastic change to the design because >> the shadow_apcb - which is what is displayed via this interface - is >> only updated when the guest is started and while it is running (i.e., >> hot plug of new adapters/domains). Making this change would >> require changing that entire design concept which I am reluctant >> to do at this point in the game. >> >> > No problem. My thinking was, that, because we can do the > assign/unassing ops also for the running guest, that we also have > the code to do the maintenance on the shadow_apcb. In this > series this code is conditional with respect to vfio_ap_mdev_has_crycb(). > E.g. > > static ssize_t assign_adapter_store(struct device *dev, > struct device_attribute *attr, > const char *buf, size_t count) > { > [..] > if (vfio_ap_mdev_has_crycb(matrix_mdev)) > if (vfio_ap_mdev_filter_guest_matrix(matrix_mdev, true)) > vfio_ap_mdev_commit_shadow_apcb(matrix_mdev); > > If one were to move the > vfio_ap_mdev_has_crycb() check into vfio_ap_mdev_commit_shadow_apcb() > then we would have an always up to date shatdow_apcb, we could display. > > I don't feel strongly about this. Was just an idea, because if the result > of the filtering is surprising, currently the only to see, without > knowing the algorithm, and possibly the state, and the history of the > system, is to actually start a guest.
Okay, I can buy this and will make the change.
> > Regards, > Halil >
| |