Re: [PATCH v11 07/14] s390/vfio-ap: sysfs attribute to display the guest's matrix

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/13/20 6:12 PM, Halil Pasic wrote:
On Fri, 13 Nov 2020 12:27:32 -0500
Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:


On 10/28/20 4:17 AM, Halil Pasic wrote:
On Thu, 22 Oct 2020 13:12:02 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxx> 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





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux