>
> On 16-Jul-19 2:46 PM, jer...@marvell.com wrote:
> > From: David Marchand <david.march...@redhat.com>
> >
> > The incriminated commit broke the use of RTE_PCI_DRV_IOVA_AS_VA
> which
> > was intended to mean "driver only supports VA" but had been understood
> > as "driver supports both PA and VA" by most net drivers and used to
> > let dpdk processes to run as non root (which do not have access to
> > physical addresses on recent kernels).
> >
> > The check on physical addresses actually closed the gap for those
> > drivers. We don't need to mark them with RTE_PCI_DRV_IOVA_AS_VA and
> > this flag can retain its intended meaning.
> > Document explicitly its meaning.
> >
> > We can check that a driver requirement wrt to IOVA mode is fulfilled
> > before trying to probe a device.
> >
> > Finally, document the heuristic used to select the IOVA mode and hope
> > that we won't break it again.
> >
> > Fixes: 703458e19c16 ("bus/pci: consider only usable devices for IOVA
> > mode")
> >
> > Signed-off-by: David Marchand <david.march...@redhat.com>
> > Reviewed-by: Jerin Jacob <jer...@marvell.com>
> > Tested-by: Jerin Jacob <jer...@marvell.com>
> > ---
>
> <snip>
>
> > @@ -629,12 +643,16 @@ rte_pci_get_iommu_class(void)
> > devices_want_va = true;
> > }
> > }
> > - if (devices_want_pa) {
> > - iova_mode = RTE_IOVA_PA;
> > - if (devices_want_va)
> > - RTE_LOG(WARNING, EAL, "Some devices want 'VA'
> but forcing 'PA' because other devices want it\n");
> > - } else if (devices_want_va) {
> > + if (devices_want_va && !devices_want_pa) {
> > iova_mode = RTE_IOVA_VA;
> > + } else if (devices_want_pa && !devices_want_va) {
> > + iova_mode = RTE_IOVA_PA;
> > + } else {
> > + iova_mode = RTE_IOVA_DC;
> > + if (devices_want_va) {
> > + RTE_LOG(WARNING, EAL, "Some devices want 'VA'
> but forcing 'DC' because other devices want 'PA'.\n");
> > + RTE_LOG(WARNING, EAL, "Depending on the final
> decision by the EAL,
> > +part of your devices won't initialise.\n");
>
> Tiny nitpick - i generally don't like personal appeals in log messages, so
> perhaps drop the "your"? I.e. "Depending on final decision by EAL, not all
> devices may be able to initialize."? Same applies to the other instance of
> this
> error message.
I will fix it in v3.
>
> Otherwise, LGTM
>
> Acked-by: Anatoly Burakov <anatoly.bura...@intel.com>
>
> --
> Thanks,
> Anatoly
