[PATCH libdrm] libdrm: Fix issue about differrent domainID but same BDF

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Emil
> Velikov
> Sent: Monday, February 25, 2019 8:09 AM
> To: Alex Deucher <alexdeucher at gmail.com>
> Cc: Deng, Emily <Emily.Deng at amd.com>; Maling list - DRI developers <dri-
> devel at lists.freedesktop.org>; amd-gfx list <amd-gfx at lists.freedesktop.org>
> Subject: Re: [PATCH libdrm] libdrm: Fix issue about differrent domainID but
> same BDF
> 
> Hi all,
> 
> This patch causes unnecessary round trip by openning the nodes. As
> mentioned previously this could be trivially fixed [1].
> 
> Even Emily acknowledged that [1], yet the sub-par fix was merged. Can we
> revert+fixup this properly?
> 

Sorry, I totally missed your reply.  I'm having Internet issues at the moment so if you want to revert for now, I'll work with Emily to address your suggestions later in the week or next.  Emily, can you take a look at addressing Emil's concerns with an updated patch?

Alex

> Thanks
> Emil
> 
> [1] https://lists.freedesktop.org/archives/amd-gfx/2019-
> February/031573.html
> 
> On Fri, 22 Feb 2019 at 21:05, Alex Deucher <alexdeucher at gmail.com> wrote:
> >
> > Pushed.  Thanks!
> >
> > Alex
> >
> > On Thu, Feb 21, 2019 at 9:36 PM Deng, Emily <Emily.Deng at amd.com>
> wrote:
> > >
> > > Hi Alex,
> > >     Please help, thanks.
> > >
> > > Best wishes
> > > Emily Deng
> > >
> > >
> > >
> > > >-----Original Message-----
> > > >From: Alex Deucher <alexdeucher at gmail.com>
> > > >Sent: Friday, February 22, 2019 12:13 AM
> > > >To: Deng, Emily <Emily.Deng at amd.com>; Maling list - DRI developers
> > > ><dri- devel at lists.freedesktop.org>
> > > >Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>
> > > >Subject: Re: [PATCH libdrm] libdrm: Fix issue about differrent
> > > >domainID but same BDF
> > > >
> > > >On Thu, Feb 14, 2019 at 2:53 AM Emily Deng <Emily.Deng at amd.com>
> wrote:
> > > >>
> > > >> For multiple GPUs which has the same BDF, but has different
> > > >> domain ID, the drmOpenByBusid will return the wrong fd when startx.
> > > >>
> > > >> The reproduce sequence as below:
> > > >> 1. Call drmOpenByBusid to open Card0, then will return the right
> > > >> fd0, and the
> > > >> fd0 is master privilege;
> > > >> 2. Call drmOpenByBusid to open Card1. In function drmOpenByBusid,
> > > >> it will open Card0 first, this time, the fd1 for opening Card0 is
> > > >> not master privilege, and will call drmSetInterfaceVersion to
> > > >> identify the domain ID feature, as the fd1 is not master
> > > >> privilege, then drmSetInterfaceVersion will fail, and then won't
> > > >> compare domain ID, then
> > > >return the wrong fd for Card1.
> > > >>
> > > >> Solution:
> > > >> First loop search the best match fd about drm 1.4.
> > > >>
> > > >> Signed-off-by: Emily Deng <Emily.Deng at amd.com>
> > > >
> > > >Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> > > >
> > > >Do you need someone to commit this for you?
> > > >
> > > >Alex
> > > >
> > > >> ---
> > > >>  xf86drm.c | 23 +++++++++++++++++++++++
> > > >>  1 file changed, 23 insertions(+)
> > > >>
> > > >> diff --git a/xf86drm.c b/xf86drm.c index 336d64d..b60e029 100644
> > > >> --- a/xf86drm.c
> > > >> +++ b/xf86drm.c
> > > >> @@ -584,11 +584,34 @@ static int drmOpenByBusid(const char
> > > >> *busid, int
> > > >type)
> > > >>      if (base < 0)
> > > >>          return -1;
> > > >>
> > > >> +    /* We need to try for 1.4 first for proper PCI domain
> > > >> + support */
> > > >>      drmMsg("drmOpenByBusid: Searching for BusID %s\n", busid);
> > > >>      for (i = base; i < base + DRM_MAX_MINOR; i++) {
> > > >>          fd = drmOpenMinor(i, 1, type);
> > > >>          drmMsg("drmOpenByBusid: drmOpenMinor returns %d\n", fd);
> > > >>          if (fd >= 0) {
> > > >> +            sv.drm_di_major = 1;
> > > >> +            sv.drm_di_minor = 4;
> > > >> +            sv.drm_dd_major = -1;        /* Don't care */
> > > >> +            sv.drm_dd_minor = -1;        /* Don't care */
> > > >> +            if (!drmSetInterfaceVersion(fd, &sv)) {
> > > >> +                buf = drmGetBusid(fd);
> > > >> +                drmMsg("drmOpenByBusid: drmGetBusid reports %s\n",
> buf);
> > > >> +                if (buf && drmMatchBusID(buf, busid, 1)) {
> > > >> +                    drmFreeBusid(buf);
> > > >> +                    return fd;
> > > >> +                }
> > > >> +                if (buf)
> > > >> +                    drmFreeBusid(buf);
> > > >> +            }
> > > >> +            close(fd);
> > > >> +        }
> > > >> +    }
> > > >> +
> > > >> +   for (i = base; i < base + DRM_MAX_MINOR; i++) {
> > > >> +        fd = drmOpenMinor(i, 1, type);
> > > >> +        drmMsg("drmOpenByBusid: drmOpenMinor returns %d\n", fd);
> > > >> +        if (fd >= 0) {
> > > >>              /* We need to try for 1.4 first for proper PCI domain support
> > > >>               * and if that fails, we know the kernel is busted
> > > >>               */
> > > >> --
> > > >> 2.7.4
> > > >>
> > > >> _______________________________________________
> > > >> amd-gfx mailing list
> > > >> amd-gfx at lists.freedesktop.org
> > > >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx