Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 19 Dec 2019 13:57:13 +0100
From:   Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= 
        <marmarek@invisiblethingslab.com>
To:     Jan Beulich <jbeulich@suse.com>
Cc:     xen-devel@lists.xenproject.org,
        Roger Pau =?utf-8?B?TW9ubsOp?= <roger.pau@citrix.com>,
        YueHaibing <yuehaibing@huawei.com>,
        Simon Gaiser <simon@invisiblethingslab.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        Juergen Gross <jgross@suse.com>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] xen-pciback: optionally allow interrupt enable flag
 writes
Message-ID: <20191219125713.GN25235@mail-itl>
References: <20191219034941.19141-1-marmarek@invisiblethingslab.com>
 <c0e27fbe-e2f7-22ca-c3f4-bafb252c7bcc@suse.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="D9sZ58tf58331Q5M"
Content-Disposition: inline
In-Reply-To: <c0e27fbe-e2f7-22ca-c3f4-bafb252c7bcc@suse.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--D9sZ58tf58331Q5M
Content-Type: text/plain; protected-headers=v1; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PATCH v2] xen-pciback: optionally allow interrupt enable flag
 writes

On Thu, Dec 19, 2019 at 12:20:24PM +0100, Jan Beulich wrote:
> On 19.12.2019 04:49, Marek Marczykowski-G=C3=B3recki  wrote:
> > +enum interrupt_type xen_pcibk_get_interrupt_type(struct pci_dev *dev)
> > +{
> > +	int err;
> > +	u16 val;
> > +
> > +	err =3D pci_read_config_word(dev, PCI_COMMAND, &val);
> > +	if (err)
> > +		return INTERRUPT_TYPE_ERR;
> > +	if (!(val & PCI_COMMAND_INTX_DISABLE))
> > +		return INTERRUPT_TYPE_INTX;
> > +
> > +	/* Do not trust dev->msi(x)_enabled here, as enabling could be done
> > +	 * bypassing the pci_*msi* functions, by the qemu.
> > +	 */
>=20
> Judging from this comment, how can you assume only one of the
> three variants is actually enabled? It's against the spec, yes,
> but it's not at all impossible afaict. I think you want the
> return value here to be
> - negative errno values (no need to discard the actual error
>   codes) or
> - a non-negative bitmap indicating which of the interrupt types
>   is/are currently enabled.

Good idea, I'll change that.

> That way ...
>=20
> > +static int msi_msix_flags_write(struct pci_dev *dev, int offset, u16 n=
ew_value,
> > +				void *data)
> > +{
> > +	int err;
> > +	u16 old_value;
> > +	const struct msi_msix_field_config *field_config =3D data;
> > +	const struct xen_pcibk_dev_data *dev_data =3D pci_get_drvdata(dev);
> > +
> > +	if (xen_pcibk_permissive || dev_data->permissive)
> > +		goto write;
> > +
> > +	err =3D pci_read_config_word(dev, offset, &old_value);
> > +	if (err)
> > +		return err;
> > +
> > +	if (new_value =3D=3D old_value)
> > +		return 0;
> > +
> > +	if (!dev_data->allow_interrupt_control ||
> > +	    (new_value ^ old_value) & ~field_config->enable_bit)
> > +		return PCIBIOS_SET_FAILED;
> > +
> > +	if (new_value & field_config->enable_bit) {
> > +		/* don't allow enabling together with other interrupt types */
> > +		const enum interrupt_type int_type =3D xen_pcibk_get_interrupt_type(=
dev);
> > +		if (int_type =3D=3D INTERRUPT_TYPE_NONE ||
> > +		    int_type =3D=3D field_config->int_type)
>=20
> ... equality comparisons like this one will actually become safe.
>=20
> Jan

--=20
Best Regards,
Marek Marczykowski-G=C3=B3recki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

--D9sZ58tf58331Q5M
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAl37c6oACgkQ24/THMrX
1yx2tQf/XZes/aHUFlMPeDhciYxLI4xPYvF0dQuJgQsY4ffyPZUERevQ8NaEJmDd
8rMXFRVKzhmjjr+zeRtaiH+qdUKttLuSgPE6ezsiP2kNSQ2iATacV61mPT4q1REr
YzqFUQOVUQpl9StWWwbyTpsOA2yiwhkDQKpBmVSRHmhR7YZ+R2LX4+yvGAAvP3sd
svvdTkOs8WJCDJig4gCv6vqZy5bBZfDd7Azn/IXc8rvOU2HKSTMsvqz+fHKL57YG
bGnGb9vTIeShWk5dHmn5JhkEXGNHqKuhuYrGI2EBEpyXepgA2K1YZn0A8iY5hX5j
3A89KK+cJrDjUY/uZvjG8H0lBcRVcA==
=SMFN
-----END PGP SIGNATURE-----

--D9sZ58tf58331Q5M--