linux-kernel - Re: [PATCH v7 3/4] s390: ap: implement PAPQ AQIC interception in kernel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 30 Apr 2019 15:26:05 +0200
From:   Halil Pasic <pasic@...ux.ibm.com>
To:     Pierre Morel <pmorel@...ux.ibm.com>
Cc:     borntraeger@...ibm.com, alex.williamson@...hat.com,
        cohuck@...hat.com, linux-kernel@...r.kernel.org,
        linux-s390@...r.kernel.org, kvm@...r.kernel.org,
        frankja@...ux.ibm.com, akrowiak@...ux.ibm.com, david@...hat.com,
        schwidefsky@...ibm.com, heiko.carstens@...ibm.com,
        freude@...ux.ibm.com, mimu@...ux.ibm.com
Subject: Re: [PATCH v7 3/4] s390: ap: implement PAPQ AQIC interception in
 kernel

On Fri, 26 Apr 2019 15:01:27 +0200
Pierre Morel <pmorel@...ux.ibm.com> wrote:

> +/**
> + * vfio_ap_clrirq: Disable Interruption for a APQN
> + *
> + * @dev: the device associated with the ap_queue
> + * @q:   the vfio_ap_queue holding AQIC parameters
> + *
> + * Issue the host side PQAP/AQIC
> + * On success: unpin the NIB saved in *q and unregister from GIB
> + * interface
> + *
> + * Return the ap_queue_status returned by the ap_aqic()
> + */
> +static struct ap_queue_status vfio_ap_clrirq(struct vfio_ap_queue *q)
> +{
> +	struct ap_qirq_ctrl aqic_gisa = {};
> +	struct ap_queue_status status;
> +	int checks = 10;
> +
> +	status = ap_aqic(q->apqn, aqic_gisa, NULL);
> +	if (!status.response_code) {
> +		while (status.irq_enabled && checks--) {
> +			msleep(20);

Hm, that seems like a lot of time to me. And I suppose we are holding the
kvm lock: e.g. no other instruction can be interpreted by kvm in the
meantime.

> +			status = ap_tapq(q->apqn, NULL);
> +		}
> +		if (checks >= 0)
> +			vfio_ap_free_irq_data(q);

Actually we don't have to wait for the async part to do it's magic
(indicated by the status.irq_enabled --> !status.irq_enabled transition)
in the instruction handler. We have to wait so we can unpin the NIB but
that could be done async (e.g. workqueue).

BTW do you have any measurements here? How many msleep(20) do we
experience for one clear on average?

If linux is not using clear (you told so offline, and I also remember
something similar), we can probably get away with something like this,
and do it properly (from performance standpoint) later.

Regards,
Halil

> +		else
> +			WARN_ONCE("%s: failed disabling IRQ", __func__);
> +	}
> +
> +	return status;
> +}