From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [RFC PATCH] PCI: Introduce INTx check & mask API Date: Thu, 24 May 2012 09:02:17 -0300 Message-ID: <4FBE2349.6040800@siemens.com> References: <4FBDE6D6.80700@ozlabs.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Alex Williamson , Benjamin Herrenschmidt , David Gibson , Alex Graf , kvm@vger.kernel.org, qemu-devel@nongnu.org To: Alexey Kardashevskiy Return-path: Received: from david.siemens.de ([192.35.17.14]:26904 "EHLO david.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756436Ab2EXMCv (ORCPT ); Thu, 24 May 2012 08:02:51 -0400 In-Reply-To: <4FBDE6D6.80700@ozlabs.ru> Sender: kvm-owner@vger.kernel.org List-ID: On 2012-05-24 04:44, Alexey Kardashevskiy wrote: > [Found while debugging VFIO on POWER but it is platform independent] >=20 > There is a feature in PCI (>=3D2.3?) to mask/unmask INTx via PCI_COMM= AND and > PCI_STATUS registers. Yes, 2.3 introduced this. Masking is done via command register, checkin= g if the source was the PCI in question via the status register. The latter is important for supporting IRQ sharing - and that's why we introduced this masking API to the PCI layer. >=20 > And there is some API to support that (commit a2e27787f893621c5a6b865= acf6b7766f8671328). >=20 > I have a network adapter: > 0001:00:01.0 Ethernet controller: Chelsio Communications Inc T310 10G= bE Single Port Adapter > Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ = Stepping- SERR+ FastB2B- DisINTx- > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=3Dfast >TAbort- SERR- =20 > pci_intx_mask_supported() reports that the feature is supported for t= his adapter > BUT the adapter does not set PCI_STATUS_INTERRUPT so pci_check_and_se= t_intx_mask() > never changes PCI_COMMAND and INTx does not work on it when we use it= as VFIO-PCI device. >=20 > If I remove the check of this bit, it works fine as it is called from= an interrupt handler and > Status bit check is redundant. >=20 > Opened a spec: > PCI LOCAL BUS SPECIFICATION, REV. 3.0, Table 6-2: Status Register Bit= s > =3D=3D=3D > 3 This read-only bit reflects the state of the interrupt in the > device/function. Only when the Interrupt Disable bit in the command > register is a 0 and this Interrupt Status bit is a 1, will the > device=E2=80=99s/function=E2=80=99s INTx# signal be asserted. Setting= the Interrupt > Disable bit to a 1 has no effect on the state of this bit. > =3D=3D=3D > With this adapter, INTx# is asserted but Status bit is still 0. >=20 > Is it mandatory for a device to set Status bit if it supports INTx ma= sking? >=20 > 2 Alex: if it is mandatory, then we need to be able to disable pci_2_= 3 in VFIO-PCI > somehow. Since PCI 2.3, this bit is mandatory, and it should be independent of the masking bit. The question is, if your device is supposed to support 2.3, thus is just buggy, or if our detection algorithm is unreliable. I= t basically builds on the assumption that, if we can flip the mask bit, the feature should be present. I guess that is the best we can do. Mayb= e we can augment this with a blacklist of devices that "support" flipping without actually providing the feature. Jan --=20 Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux