Messages in this thread | | | Subject | Re: [PATCH v10 11/16] s390/vfio-ap: allow hot plug/unplug of AP resources using mdev device | From | Tony Krowiak <> | Date | Mon, 5 Oct 2020 19:05:08 -0400 |
| |
I proposed two algorithms in my last response. The following summarizes the results from executing your scenario:
bound queues: 0.0 0.1
1.0
2.0 2.1
algorithm: use filtering on assign/unassign scenario: echo 0 > assign_domain echo 1 > assign_domain echo 1 > assign_adapter
matrix: 1.0 1.1 guest_matrix: 1.0
echo 0 > assign_adapter
matrix: 0.0 0.1 1.0 1.1 guest_matrix: 0.0 0.1
echo 1 > unassign_adapter 0.0 0.1 guest_matrix: 0.0 0.1
echo 2 > assign_adapter
matrix: 0.0 0.1 2.0 2.1 guest_matrix: 0.0 0.1 2.0 2.1
echo 1 > assign_adapter
matrix: 0.0 0.1 1.0 1.1 2.0 2.1 guest_matrix: 0.0 0.1 2.0 2.1
algorithm: do not plug adapter if all assigned APQNs are not bound scenario: echo 0 > assign_domain echo 1 > assign_domain echo 1 > assign_adapter
matrix: 1.0 1.1 guest_matrix: no bits set
echo 0 > assign_adapter
matrix: 0.0 0.1 1.0 1.1 guest_matrix: 0.0 0.1
echo 1 > unassign_adapter 0.0 0.1 guest_matrix: 0.0 0.1
echo 2 > assign_adapter
matrix: 0.0 0.1 2.0 2.1 guest_matrix: 0.0 0.1 2.0 2.1
echo 1 > assign_adapter
matrix: 0.0 0.1 1.0 1.1 2.0 2.1 guest_matrix: 0.0 0.1 2.0 2.1
On 10/5/20 2:30 PM, Halil Pasic wrote: > On Mon, 5 Oct 2020 12:24:39 -0400 > Tony Krowiak <akrowiak@linux.ibm.com> wrote: > >> >> On 9/27/20 9:01 PM, Halil Pasic wrote: >>> On Fri, 21 Aug 2020 15:56:11 -0400 >>> Tony Krowiak<akrowiak@linux.ibm.com> wrote: >>> >>>> Let's hot plug/unplug adapters, domains and control domains assigned to or >>>> unassigned from an AP matrix mdev device while it is in use by a guest per >>>> the following: >>>> >>>> * When the APID of an adapter is assigned to a matrix mdev in use by a KVM >>>> guest, the adapter will be hot plugged into the KVM guest as long as each >>>> APQN derived from the Cartesian product of the APID being assigned and >>>> the APQIs already assigned to the guest's CRYCB references a queue device >>>> bound to the vfio_ap device driver. >>>> >>>> * When the APID of an adapter is unassigned from a matrix mdev in use by a >>>> KVM guest, the adapter will be hot unplugged from the KVM guest. >>>> >>>> * When the APQI of a domain is assigned to a matrix mdev in use by a KVM >>>> guest, the domain will be hot plugged into the KVM guest as long as each >>>> APQN derived from the Cartesian product of the APQI being assigned and >>>> the APIDs already assigned to the guest's CRYCB references a queue device >>>> bound to the vfio_ap device driver. >>>> >>>> * When the APQI of a domain is unassigned from a matrix mdev in use by a >>>> KVM guest, the domain will be hot unplugged from the KVM guest >>> Hm, I suppose this means that what your guest effectively gets may depend >>> on whether assign_domain or assign_adapter is done first. >>> >>> Suppose we have the queues >>> 0.0 0.1 >>> 1.0 >>> bound to vfio_ap, i.e. 1.1 is missing for a reason different than >>> belonging to the default drivers (for what exact reason no idea). >> I'm not quite sure what you mean be "we have queue". I will >> assume you mean those queues are bound to the vfio_ap >> device driver. > Yes, this is exactly what I've meant. > > >> The only way this could happen is if somebody >> manually unbinds queue 1.1. >> > Assuming that: > 1) every time we observe ap_perm the ap subsystem in in a settled state > (i.e. not in a middle of pushing things left and right > because of an ap_perm change, > 2) the only non-default driver is vfio_ap, and that > 3) queues handle non-operational states by other means than dissapearing > (should be the case with the latest reworks) > I agree what is left is manual unbind, which I lean towards considering > an edge case. > > If this is indeed just about that edge case, maybe we can live with a > simpler algorithm than this one. > > >>> Let's suppose we started with the matix containing only adapter >>> 0 (0.) and domain 0 (.0). >>> >>> After echo 1 > assign_adapter && echo 1 > assign_domain we end up with >>> matrix: >>> 0.0 0.1 >>> 1.0 1.1 >>> guest_matrix: >>> 0.0 0.1 >>> while after echo 1 > assign_domain && echo 1 > assign_adapter we end up >>> with: >>> matrix: >>> 0.0 0.1 >>> 1.0 1.1 >>> guest_matrix: >>> 0.0 >>> 0.1 >>> >>> That means, the set of bound queues and the set of assigned resources do >>> not fully determine the set of resources passed through to the guest. >>> >>> Is that a deliberate design choice? >> Yes, it is a deliberate choice to only allow guest access to queues >> represented by queue devices bound to the vfio_ap device driver. >> The idea here is to adhere to the linux device model. >> > This is not what I've asked. My question was about he fact that > reordering assignments gives different results. Well this was kind > of the case before as well, with the notable difference, that in a > past we always had an error. So if a full sequence of assignments could > be performed without an error, than any permutation would be performed > with the exact same result. > > I'm all for only allowing guest access to queues represented by queue > devices bound to the vfio_ap device driver. I'm concerned with the > permutation (and calculus). > >>>> * When the domain number of a control domain is assigned to a matrix mdev >>>> in use by a KVM guest, the control domain will be hot plugged into the >>>> KVM guest. >>>> >>>> * When the domain number of a control domain is unassigned from a matrix >>>> mdev in use by a KVM guest, the control domain will be hot unplugged >>>> from the KVM guest. >>>> >>>> Signed-off-by: Tony Krowiak<akrowiak@linux.ibm.com> >>>> --- > [..] > >>>> +static bool vfio_ap_mdev_assign_guest_apid(struct ap_matrix_mdev *matrix_mdev, >>>> + unsigned long apid) >>>> +{ >>>> + unsigned long apqi, apqn; >>>> + >>>> + if (!vfio_ap_mdev_has_crycb(matrix_mdev) || >>>> + !test_bit_inv(apid, (unsigned long *)matrix_dev->info.apm)) >>>> + return false; >>>> + >>>> + if (bitmap_empty(matrix_mdev->shadow_apcb.aqm, AP_DOMAINS)) >>>> + return vfio_ap_mdev_assign_apqis_4_apid(matrix_mdev, apid); >>> Hm. Let's say we have the same situation regarding the bound queues as >>> above but we start with the empty matrix, and do all the assignments >>> while the guest is running. >>> >>> Consider the following sequence of actions. >>> >>> 1) echo 0 > assign_domain >> matrix: .0 >> guest_matrix: no APQNs >> >>> 2) echo 1 > assign_domain >> matrix: .0, .1 >> guest_matrix: no APQNs >> >>> 3) echo 1 > assign_adapter >> matrix: 1.0, 1.1 >> guest_matrix: 1.0 >> >>> 4) echo 0 > assign_adapter >> matrix: 0.0, 0.1, 1.0, 1.1 >> guest_matrix: 0.0, 1.0 >>> 5) echo 1 > unassign_adapter >> matrix: 0.0, 0.1 >> guest_matrix: 0.0 >> >>> I understand that at 3), because >>> bitmap_empty(matrix_mdev->shadow_apcb.aqm)we would end up with a shadow >>> aqm containing just domain 0, as queue 1.1 ain't bound to us. >> True >> >>> Thus at the end we would have >>> matrix: >>> 0.0 0.1 >>> guest_matrix: >>> 0.0 >> At the end I had: >> matrix: 0.0, 0.1 >> guest_matrix: 0.0 >> >>> And if we add in an adapter 2. into the mix with the queues 2.0 and 2.1 >>> then after >>> 6) echo 2 > assign_adapter >>> we get >>> Thus at the end we would have >>> matrix: >>> 0.0 0.1 >>> 2.0 2.1 >>> guest_matrix: >>> 0.0 >>> 2.0 >>> >>> This looks very quirky to me. Did I read the code wrong? Opinions? >> You read the code correctly and I agree, this is a bit quirky. I would say >> that after adding adapter 2, we should end up with guest matrix: >> 0.0, 0.1 >> 2.0, 2.1 >> >> If you agree, I'll make the adjustment. >> > I do agree, but maybe we should discuss what adjustments do you have in > mind. > > [..] > >>>> +static bool vfio_ap_mdev_unassign_guest_apid(struct ap_matrix_mdev *matrix_mdev, >>>> + unsigned long apid) >>>> +{ >>>> + if (vfio_ap_mdev_has_crycb(matrix_mdev)) { >>>> + if (test_bit_inv(apid, matrix_mdev->shadow_apcb.apm)) { >>>> + clear_bit_inv(apid, matrix_mdev->shadow_apcb.apm); >>>> + >>>> + /* >>>> + * If there are no APIDs assigned to the guest, then >>>> + * the guest will not have access to any queues, so >>>> + * let's also go ahead and unassign the APQIs. Keeping >>>> + * them around may yield unpredictable results during >>>> + * a probe that is not related to a host AP >>>> + * configuration change (i.e., an AP adapter is >>>> + * configured online). >>>> + */ >>> I don't quite understand this comment. Clearing out the other mask when >>> the other one becomes empty, does allow us to recover the full possible guest >>> matrix in the scenario described above. I don't see any shadow >>> manipulation in the probe handler at this stage. Are we maybe >>> talking about the same effect as I described for assign? >> Patch 15/16 is for the probe. >> > I still don't understand the logic, but I guess we want to make > adjustments anyways, so maybe I don't have to. > > Regards, > Halil
| |