Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i1568416f:Fastmail
Date:   Thu, 17 Nov 2022 15:32:00 +0100
From:   Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= 
        <marmarek@invisiblethingslab.com>
To:     Jan Beulich <jbeulich@suse.com>
Cc:     linux-kernel@vger.kernel.org, Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
        "moderated list:XEN HYPERVISOR INTERFACE" 
        <xen-devel@lists.xenproject.org>, David Vrabel <dvrabel@cantab.net>
Subject: Re: [PATCH] xen-pciback: Consider MSI-X enabled only when MASKALL
 bit is cleared
Message-ID: <Y3ZF4EcD/c7Q5yHb@mail-itl>
References: <20221117114122.1588338-1-marmarek@invisiblethingslab.com>
 <0afe3f35-1b25-d1c6-89bb-8dae7a4070e9@cantab.net>
 <Y3YzY0aUc42cQGk/@mail-itl>
 <a84234d8-9f48-4b8f-e171-599c34840581@suse.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="hsWQDkJqLvEsIMcB"
Content-Disposition: inline
In-Reply-To: <a84234d8-9f48-4b8f-e171-599c34840581@suse.com>
Precedence: bulk


--hsWQDkJqLvEsIMcB
Content-Type: text/plain; protected-headers=v1; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Date: Thu, 17 Nov 2022 15:32:00 +0100
From: Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= <marmarek@invisiblethingslab.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: linux-kernel@vger.kernel.org, Juergen Gross <jgross@suse.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
	"moderated list:XEN HYPERVISOR INTERFACE" <xen-devel@lists.xenproject.org>,
	David Vrabel <dvrabel@cantab.net>
Subject: Re: [PATCH] xen-pciback: Consider MSI-X enabled only when MASKALL
 bit is cleared

On Thu, Nov 17, 2022 at 02:33:16PM +0100, Jan Beulich wrote:
> On 17.11.2022 14:13, Marek Marczykowski-G=C3=B3recki wrote:
> > On Thu, Nov 17, 2022 at 12:54:51PM +0000, David Vrabel wrote:
> >> On 17/11/2022 11:41, Marek Marczykowski-G=C3=B3recki wrote:
> >>> Linux enables MSI-X before disabling INTx, but keeps MSI-X masked unt=
il
> >>> the table is filled. Then it disables INTx just before clearing MASKA=
LL
> >>> bit. Currently this approach is rejected by xen-pciback.
> >>> Allow setting PCI_MSIX_FLAGS_ENABLE while INTx is still enabled as lo=
ng
> >>> as PCI_MSIX_FLAGS_MASKALL is set too.
> >>
> >> The use of MSI-X interrupts is conditional on only the MSI-X Enable bi=
t.
> >> Setting MSI-X Enable effectively overrides the Interrupt Disable bit i=
n the
> >> Command register.
> >=20
> > That means the second chunk of the patch may even drop the '(new_value &
> > PCI_MSIX_FLAGS_MASKALL)' part, right?=20
> >=20
> >> PCIe 6.0.1 section 7.7.2.2. "MSI-X Enable ... is prohibited from using=
 INTx
> >> interrupts (if implemented)." And there is similar wording for MSI Ena=
ble.
> >=20
> > And this would mean the 'field_config->int_type =3D=3D INTERRUPT_TYPE_M=
SIX'
> > part isn't necessary either.
> >=20
> > Jan in another thread pointed out that disabling INTx explicitly is
> > still a useful workaround for a flawed hardware. But if that isn't
> > mandated by the spec, maybe it doesn't need to be enforced by pciback
> > either?
>=20
> Well, allowing a device to go into a mode exhibiting undefined behavior
> is what we ought to prevent when it comes to a DomU doing so vs overall
> host safety.

If the spec prohibits using INTx if MSI/MSI-X is enabled (regardless of
PCI_COMMAND_INTX_DISABLE bit), then well-behaving device should be fine
(we aren't hitting undefined behavior). As for buggy device, it wouldn't
be much different from a device ignoring PCI_COMMAND_INTX_DISABLE
completely, no (besides the latter being probably much less probable
bug)?
If the above is assumption is correct, it seems such device may not
function correctly without extra workarounds (which are in the driver
interest to apply), but should not affect overall host safety (as in:
beyond the guest having that device assigned). I think pciback should
only enforce what's necessary to prevent one guest hurting others (or
the hypervisor), but it doesn't need to prevent guest hurting itself.

--=20
Best Regards,
Marek Marczykowski-G=C3=B3recki
Invisible Things Lab

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

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

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAmN2ReEACgkQ24/THMrX
1yzzUQf/fRtFToxRDSo67Ei2SnzYd3QNLi4bA/SAL/6aFptPOlDlc9x5/3vcxPdP
9XXxKSV8XCpz1Ujxbq8jc2F04SBoQOblw8gOvxXzxLx8OF7r0wslheBBKCnd9p4L
K75QUIBMp7FPdG5zZNlE0uEeDZXfZQEQWUe3ZkI3DwQEA0pNge2TQcFboJYCgo7u
jFKA2HUoGKdIHXyOrq32dQA3MIQHGbKuOlh7amutgfjSGVHhf7mKWlicmrac1Xvg
J51tmZU6hTUm/XXN9/NHqVCcPpC+umlxfAWi6wDcsUrS4upTT36fh3XWD3PO6twk
XYA18LE0FTW2NIsUbImeFTak0mp2tw==
=ahAp
-----END PGP SIGNATURE-----

--hsWQDkJqLvEsIMcB--