On Wed, Nov 01, 2023 at 12:12:02AM +0530, Jerin Jacob wrote: > On Tue, Oct 31, 2023 at 10:45 PM Bruce Richardson > <[email protected]> wrote: > > > > On Tue, Oct 31, 2023 at 10:36:04PM +0530, Jerin Jacob wrote: > > > On Tue, Oct 31, 2023 at 8:43 PM Sevincer, Abdullah > > > <[email protected]> wrote: > > > > > > > > > > > > > +This patch can be splited as two, +1) Generic PCIe function to > > > > > enable/disable PASID +2) Call generic function to disable PASID > > > > > in drivers/event/dlb2/. Also mention which Linux kernel commit is > > > > > introducing this issue in the git commit log. > > > > > > > > Hi Jerrin, I think I need to provide more information here, then we > > > > can decide which way we will go would be good for now. I agree to > > > > having 2 functions in pci common code to enable/disable PASID, but > > > > we need to have hardcoded PASID cap offset inside these functions > > > > as well since PASID capability is not exposed to user. Hence, to be > > > > more specific main reason to have hardcoded PASID is, > > > > rte_pci_find_ext_capability() function to retrieve the offset > > > > returns '0' since PASID is not exposed to user yet. > > > > > > > > We can see this is vfio_pci_config.c in kernel code where PASID is > > > > not exposed to user. [PCI_EXT_CAP_ID_PASID] = 0, /* > > > > not yet */ > > > > > > > > So if it is okay to go with hardcoded offset now in these functions > > > > I will move the implementation to pci_common file. > > > > > > I would suggest, add argument option to API whether to probe the > > > capability or not? - 0 means probe and- non zero means specific PASID > > > cap offset till Linux VFIO is exposing it. > > > > That doesn't seem particularly useful to me. The calling-API in the > > DPDK PMD (assuming it's PMD who use this), is no more likely to know > > whether probing will work. Therefore, I think we just hard-code the > > offset for now. > > I think, there are three things here 1) Whether to have common API for > dealing with generic function like enabling PASID or not? - I think, we > are in agreement to have common public function(Implementation could be > hard-coded or probe) 2) Since it is public PCIe API, I thought of adding > probing in API as it is just LINUX limitation now. No strong opinion on > inclusion of probe in on this as Linux is main EAL which supports PCIe > now. 3) Since it is PCIe capability, In my understanding the offset will > change based on the number of capabilities available in PCIe config space > for a given device. _if so_, an additional argument for the offset needs > to be passed from PMD to common PCI API(I.e it can not be hard-coded in > common PCI code) >
Given these constraints and how late we are in the release cycle, I therefore suggest we take the driver-specific bug fix for now. DLB seems the only driver affected right now (and I can confirm the issue having encountered it myself when running DPDK on Ubuntu 23.04). I think a general function is a good thing to have, but it seems that such a general function should wait until we really can make it generic in the future. Just my 2c. at this point. /Bruce
