On 07/31/2018 10:38 AM, Cornelia Huck wrote:
On Thu, 26 Jul 2018 21:54:21 +0200 Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote: Regardless whether the activation approach is a good idea...+static int vfio_ap_verify_queues_reserved(struct ap_matrix_mdev *matrix_mdev) +{ + unsigned long apid, apqi; + int ret = 0; + + for_each_set_bit_inv(apid, matrix_mdev->matrix.apm, + matrix_mdev->matrix.apm_max + 1) { + for_each_set_bit_inv(apqi, matrix_mdev->matrix.aqm, + matrix_mdev->matrix.aqm_max + 1) { + if (!ap_owned_by_def_drv((int)apid, (int)apqi)) + continue; + + /* + * We want to log every APQN that is not reserved by + * the driver, so record the return code, log a message + * and allow the loop to continue + */ + ret = -EPERM; + pr_warn("%s: activate for %s failed: queue %02lx.%04lx owned by default driver\n", + VFIO_AP_MODULE_NAME, matrix_mdev->name, apid, + apqi);...I do not think that the syslog is a good place to log those errors. AFAICS these are setup/administration errors, and the admin may want to inspect the data to find out what went wrong. Maybe better to log attempts to assign APQNs reserved to the default drivers into a dedicated dbf or somesuch, and log it in a format that can also be digested by scripts?
I agree, the error reporting is suboptimal. My preferred solution is an API with proper error reporting though. Even with dbf the problem, that one has to read some log and figure out what error corresponds to what request persist. Optimally the user should get feedback via the same interface an interactive action was triggered. In fact, I had a proof of concept implementation but it would need some rebasing. Another IMHO desirable property of this interface is that it changes (or rejects change to) the matrix (masks) in one atomic operation. So there is no 'dirty' state and no transaction involved in specifying or changing the matrix. Regards, Halil
+ } + } + + return ret; +} + +static void vfio_ap_mdev_log_sharing_err(struct ap_matrix_mdev *matrix_mdev, + unsigned long apid, unsigned long apqi) +{ + pr_warn("%s: AP queue %02lx.%04lx is assigned to %s device\n", __func__, + apid, apqi, matrix_mdev->name); +}Same applies to cross-vm misconfigurations.
