On Wed, Jul 03, 2019 at 10:26:39AM +0200, David Marchand wrote: > On Wed, Jul 3, 2019 at 10:17 AM Tiwei Bie <tiwei....@intel.com> wrote: > > On Wed, Jul 03, 2019 at 10:01:44AM +0200, David Marchand wrote: > > On Wed, Jul 3, 2019 at 9:58 AM Tiwei Bie <tiwei....@intel.com> wrote: > > > > On Wed, Jul 03, 2019 at 09:36:26AM +0200, David Marchand wrote: > > > On Wed, Jul 3, 2019 at 9:35 AM Tiwei Bie <tiwei....@intel.com> > wrote: > > > > > > Hi David, > > > > > > On Wed, Jul 03, 2019 at 09:02:59AM +0200, David Marchand > wrote: > > > > Hello, > > > > > > > > On Wed, Jul 3, 2019 at 7:47 AM Tiwei Bie > <tiwei....@intel.com > > > > wrote: > > > > > > > > The value 40 used in VFIO_GET_REGION_ADDR() is a private > value > > > > (VFIO_PCI_OFFSET_SHIFT) defined in Linux kernel source > [1]. It > > > > is not part of VFIO API, and we should not depend on it. > > > > > > > > [1] https://github.com/torvalds/linux/blob/6fbc7275c7a9/ > drivers > > /vfio/ > > > pci/ > > > > vfio_pci_private.h#L19 > > > > > > > > > > > > > > > > I did not follow linux kernel changes, is there something > that > > would > > > change > > > > this offset? > > > > It looks like a cleanup (did not look into the details yet), > do we > > need > > > this > > > > now? > > > > > > In VFIO/mdev [1], the offset can be something different. It > depends > > > on the parent device. It's not just a cleanup. It's a > preparation > > > for the mdev support in DPDK. > > > > > > [1] > https://github.com/torvalds/linux/blob/master/Documentation > / > > > vfio-mediated-device.txt > > > > > > > > > > > > Ok, thanks. > > > So we can wait for mdev to be ready before working on this. > > > > What do you mean by "mdev to be ready"? RFC ready? I don't see > > anything blocking the discussion on this now. > > > > PS. I already sent a RFC series of the mdev support in DPDK > > to the mailing list 3 month ago. > > > > > > If you need it and the mdev support has been posted already, why not > send > a n+1 > > patchset with this patch in it? > > > > This patch alone looked odd to me. > > That series was using the old API which assumes the shift > is 40 which may not work in some cases. And this patch is > to fix the API. I think this patch is actually trying to > fix a relatively independent issue -- i.e. switching to using > the proper VFIO API to get the region offsets instead of > depending on kernel code's internal value. > > > > Fix, then there is something broken ?
I should use "fix" (with "") actually :) > You said this is for mdev support which is not currently part of the features > supported by DPDK. > > > This patch breaks the ABI by extending rte_pci_device. > You must rework it to avoid this break. I didn't expect it to be merged in this release. I just want to draw other's attention on this and kick off the discussion (it would be great if you would like to share your thoughts on this). If there is a way to avoid extending rte_pci_device, it would be definitely great. But if we have to break it, then we would want to send out the announce as early as possible. Thanks, Tiwei > > > -- > David Marchand
