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

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

[Emily Deng]

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@amd.com>
---
 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Deng, Emily]

Ping ......

>-----Original Message-----
>From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Emily
>Deng
>Sent: Thursday, February 14, 2019 3:54 PM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily <Emily.Deng@amd.com>
>Subject: [PATCH libdrm] libdrm: Fix issue about differrent domainID but same
>BDF
>
>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@amd.com>
>---
> 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@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Deng, Emily]

Ping ......

Best wishes
Emily Deng

>-----Original Message-----
>From: Deng, Emily <Emily.Deng@amd.com>
>Sent: Friday, February 15, 2019 11:51 AM
>To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@lists.freedesktop.org
>Subject: RE: [PATCH libdrm] libdrm: Fix issue about differrent domainID but same
>BDF
>
>Ping ......
>
>>-----Original Message-----
>>From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>Emily Deng
>>Sent: Thursday, February 14, 2019 3:54 PM
>>To: amd-gfx@lists.freedesktop.org
>>Cc: Deng, Emily <Emily.Deng@amd.com>
>>Subject: [PATCH libdrm] libdrm: Fix issue about differrent domainID but
>>same BDF
>>
>>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@amd.com>
>>---
>> 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@lists.freedesktop.org
>>https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Emil Velikov via amd-gfx]

Hi Emily,

Please note that code outside of amdgpu/ is used by all open source drivers.
Thus patches should have dri-deve@ in to/cc as mentioned in CONTRIBUTING

On Thu, 14 Feb 2019 at 07:53, Emily Deng <Emily.Deng@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.
>
First and foremost, I wish we can stop using using these legacy APIs.
They're fairly fragile and as you can see the are strange things
happening.
We could instead use drmGetDevices2() to gather a list of devices and
pick the one we're interested.

That aside, I think we can do a slightly better fix. Have you tried:
 - resetting the pci_domain_ok=1 on each iteration, and
 - continuing to the next device when the second
drmSetInterfaceVersion() call fails

AFAICT it should produce the same result, while being shorter and faster.

Thanks
-Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Alex Deucher via amd-gfx]

Adding dri-devel.

On Thu, Feb 14, 2019 at 2:53 AM Emily Deng <Emily.Deng@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@amd.com>
> ---
>  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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Deng, Emily]

Hi Emil,
     Understand, thanks.

Best wishes
Emily Deng

>-----Original Message-----
>From: Emil Velikov <emil.l.velikov@gmail.com>
>Sent: Friday, February 15, 2019 11:02 PM
>To: Deng, Emily <Emily.Deng@amd.com>
>Cc: amd-gfx mailing list <amd-gfx@lists.freedesktop.org>
>Subject: Re: [PATCH libdrm] libdrm: Fix issue about differrent domainID but same
>BDF
>
>Hi Emily,
>
>Please note that code outside of amdgpu/ is used by all open source drivers.
>Thus patches should have dri-deve@ in to/cc as mentioned in CONTRIBUTING
>
>On Thu, 14 Feb 2019 at 07:53, Emily Deng <Emily.Deng@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.
>>
>First and foremost, I wish we can stop using using these legacy APIs.
>They're fairly fragile and as you can see the are strange things happening.
>We could instead use drmGetDevices2() to gather a list of devices and pick the
>one we're interested.
>
>That aside, I think we can do a slightly better fix. Have you tried:
> - resetting the pci_domain_ok=1 on each iteration, and
> - continuing to the next device when the second
>drmSetInterfaceVersion() call fails
>
>AFAICT it should produce the same result, while being shorter and faster.
>
>Thanks
>-Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Deng, Emily]

Thanks Alex to help to add the dri-devel. 

Best wishes
Emily Deng


>-----Original Message-----
>From: Alex Deucher <alexdeucher@gmail.com>
>Sent: Friday, February 15, 2019 11:14 PM
>To: Deng, Emily <Emily.Deng@amd.com>; Maling list - DRI developers <dri-
>devel@lists.freedesktop.org>
>Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
>Subject: Re: [PATCH libdrm] libdrm: Fix issue about differrent domainID but same
>BDF
>
>Adding dri-devel.
>
>On Thu, Feb 14, 2019 at 2:53 AM Emily Deng <Emily.Deng@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@amd.com>
>> ---
>>  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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Deng, Emily]

Ping ......

>-----Original Message-----
>From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Deng,
>Emily
>Sent: Monday, February 18, 2019 10:17 AM
>To: Alex Deucher <alexdeucher@gmail.com>; Maling list - DRI developers <dri-
>devel@lists.freedesktop.org>
>Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
>Subject: RE: [PATCH libdrm] libdrm: Fix issue about differrent domainID but same
>BDF
>
>Thanks Alex to help to add the dri-devel.
>
>Best wishes
>Emily Deng
>
>
>>-----Original Message-----
>>From: Alex Deucher <alexdeucher@gmail.com>
>>Sent: Friday, February 15, 2019 11:14 PM
>>To: Deng, Emily <Emily.Deng@amd.com>; Maling list - DRI developers
>><dri- devel@lists.freedesktop.org>
>>Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
>>Subject: Re: [PATCH libdrm] libdrm: Fix issue about differrent domainID
>>but same BDF
>>
>>Adding dri-devel.
>>
>>On Thu, Feb 14, 2019 at 2:53 AM Emily Deng <Emily.Deng@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@amd.com>
>>> ---
>>>  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@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>_______________________________________________
>amd-gfx mailing list
>amd-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Alex Deucher]

On Thu, Feb 14, 2019 at 2:53 AM Emily Deng <Emily.Deng@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@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Deng, Emily]

Hi Alex,
    Please help, thanks.

Best wishes
Emily Deng



>-----Original Message-----
>From: Alex Deucher <alexdeucher@gmail.com>
>Sent: Friday, February 22, 2019 12:13 AM
>To: Deng, Emily <Emily.Deng@amd.com>; Maling list - DRI developers <dri-
>devel@lists.freedesktop.org>
>Cc: amd-gfx list <amd-gfx@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@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@amd.com>
>
>Reviewed-by: Alex Deucher <alexander.deucher@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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Alex Deucher]

Pushed.  Thanks!

Alex

On Thu, Feb 21, 2019 at 9:36 PM Deng, Emily <Emily.Deng@amd.com> wrote:
>
> Hi Alex,
>     Please help, thanks.
>
> Best wishes
> Emily Deng
>
>
>
> >-----Original Message-----
> >From: Alex Deucher <alexdeucher@gmail.com>
> >Sent: Friday, February 22, 2019 12:13 AM
> >To: Deng, Emily <Emily.Deng@amd.com>; Maling list - DRI developers <dri-
> >devel@lists.freedesktop.org>
> >Cc: amd-gfx list <amd-gfx@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@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@amd.com>
> >
> >Reviewed-by: Alex Deucher <alexander.deucher@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@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Deng, Emily]

Thank you very much.

Best wishes
Emily Deng



>-----Original Message-----
>From: Alex Deucher <alexdeucher@gmail.com>
>Sent: Saturday, February 23, 2019 5:05 AM
>To: Deng, Emily <Emily.Deng@amd.com>
>Cc: Maling list - DRI developers <dri-devel@lists.freedesktop.org>; amd-gfx list
><amd-gfx@lists.freedesktop.org>
>Subject: Re: [PATCH libdrm] libdrm: Fix issue about differrent domainID but same
>BDF
>
>Pushed.  Thanks!
>
>Alex
>
>On Thu, Feb 21, 2019 at 9:36 PM Deng, Emily <Emily.Deng@amd.com> wrote:
>>
>> Hi Alex,
>>     Please help, thanks.
>>
>> Best wishes
>> Emily Deng
>>
>>
>>
>> >-----Original Message-----
>> >From: Alex Deucher <alexdeucher@gmail.com>
>> >Sent: Friday, February 22, 2019 12:13 AM
>> >To: Deng, Emily <Emily.Deng@amd.com>; Maling list - DRI developers
>> ><dri- devel@lists.freedesktop.org>
>> >Cc: amd-gfx list <amd-gfx@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@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@amd.com>
>> >
>> >Reviewed-by: Alex Deucher <alexander.deucher@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@lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Emil Velikov]

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?

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@gmail.com> wrote:
>
> Pushed.  Thanks!
>
> Alex
>
> On Thu, Feb 21, 2019 at 9:36 PM Deng, Emily <Emily.Deng@amd.com> wrote:
> >
> > Hi Alex,
> >     Please help, thanks.
> >
> > Best wishes
> > Emily Deng
> >
> >
> >
> > >-----Original Message-----
> > >From: Alex Deucher <alexdeucher@gmail.com>
> > >Sent: Friday, February 22, 2019 12:13 AM
> > >To: Deng, Emily <Emily.Deng@amd.com>; Maling list - DRI developers <dri-
> > >devel@lists.freedesktop.org>
> > >Cc: amd-gfx list <amd-gfx@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@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@amd.com>
> > >
> > >Reviewed-by: Alex Deucher <alexander.deucher@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@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Emil
> Velikov
> Sent: Monday, February 25, 2019 8:09 AM
> To: Alex Deucher <alexdeucher@gmail.com>
> Cc: Deng, Emily <Emily.Deng@amd.com>; Maling list - DRI developers <dri-
> devel@lists.freedesktop.org>; amd-gfx list <amd-gfx@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@gmail.com> wrote:
> >
> > Pushed.  Thanks!
> >
> > Alex
> >
> > On Thu, Feb 21, 2019 at 9:36 PM Deng, Emily <Emily.Deng@amd.com>
> wrote:
> > >
> > > Hi Alex,
> > >     Please help, thanks.
> > >
> > > Best wishes
> > > Emily Deng
> > >
> > >
> > >
> > > >-----Original Message-----
> > > >From: Alex Deucher <alexdeucher@gmail.com>
> > > >Sent: Friday, February 22, 2019 12:13 AM
> > > >To: Deng, Emily <Emily.Deng@amd.com>; Maling list - DRI developers
> > > ><dri- devel@lists.freedesktop.org>
> > > >Cc: amd-gfx list <amd-gfx@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@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@amd.com>
> > > >
> > > >Reviewed-by: Alex Deucher <alexander.deucher@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@lists.freedesktop.org
> > > >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Emil Velikov]

On Mon, 25 Feb 2019 at 19:53, Deucher, Alexander
<Alexander.Deucher@amd.com> wrote:
>
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Emil
> > Velikov
> > Sent: Monday, February 25, 2019 8:09 AM
> > To: Alex Deucher <alexdeucher@gmail.com>
> > Cc: Deng, Emily <Emily.Deng@amd.com>; Maling list - DRI developers <dri-
> > devel@lists.freedesktop.org>; amd-gfx list <amd-gfx@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?
>

Doesn't seem like there's any follow-up work on this, so I've reverted
this for now.
I'm a sad panda for doing it - hopefully v2 will come shortly.


-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Deng, Emily]

Hi Emil and Alex,
    Sorry for miss your emails. I will update a new patch as Emil's suggestion.

Best wishes
Emily Deng



>-----Original Message-----
>From: Emil Velikov <emil.l.velikov@gmail.com>
>Sent: Thursday, April 18, 2019 2:26 AM
>To: Deucher, Alexander <Alexander.Deucher@amd.com>
>Cc: Alex Deucher <alexdeucher@gmail.com>; Deng, Emily
><Emily.Deng@amd.com>; Maling list - DRI developers <dri-
>devel@lists.freedesktop.org>; amd-gfx list <amd-gfx@lists.freedesktop.org>
>Subject: Re: [PATCH libdrm] libdrm: Fix issue about differrent domainID but
>same BDF
>
>On Mon, 25 Feb 2019 at 19:53, Deucher, Alexander
><Alexander.Deucher@amd.com> wrote:
>>
>> > -----Original Message-----
>> > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> > Emil Velikov
>> > Sent: Monday, February 25, 2019 8:09 AM
>> > To: Alex Deucher <alexdeucher@gmail.com>
>> > Cc: Deng, Emily <Emily.Deng@amd.com>; Maling list - DRI developers
>> > <dri- devel@lists.freedesktop.org>; amd-gfx list
>> > <amd-gfx@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?
>>
>
>Doesn't seem like there's any follow-up work on this, so I've reverted this for
>now.
>I'm a sad panda for doing it - hopefully v2 will come shortly.
>
>
>-Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Deng, Emily]

Hi Emil,
    I don't understand your idea clear about follow, what about the case that only has 1 GPU, and don't support pci_domain? For this case, it still need to fallback to pci_domain_ok=0.
>That aside, I think we can do a slightly better fix. Have you tried:
> - resetting the pci_domain_ok=1 on each iteration, and
> - continuing to the next device when the second
>drmSetInterfaceVersion() call fails

Best wishes
Emily Deng


>-----Original Message-----
>From: Emil Velikov <emil.l.velikov@gmail.com>
>Sent: Friday, February 15, 2019 11:02 PM
>To: Deng, Emily <Emily.Deng@amd.com>
>Cc: amd-gfx mailing list <amd-gfx@lists.freedesktop.org>
>Subject: Re: [PATCH libdrm] libdrm: Fix issue about differrent domainID but
>same BDF
>
>Hi Emily,
>
>Please note that code outside of amdgpu/ is used by all open source drivers.
>Thus patches should have dri-deve@ in to/cc as mentioned in CONTRIBUTING
>
>On Thu, 14 Feb 2019 at 07:53, Emily Deng <Emily.Deng@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.
>>
>First and foremost, I wish we can stop using using these legacy APIs.
>They're fairly fragile and as you can see the are strange things happening.
>We could instead use drmGetDevices2() to gather a list of devices and pick the
>one we're interested.
>
>That aside, I think we can do a slightly better fix. Have you tried:
> - resetting the pci_domain_ok=1 on each iteration, and
> - continuing to the next device when the second
>drmSetInterfaceVersion() call fails
>
>AFAICT it should produce the same result, while being shorter and faster.
>
>Thanks
>-Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Emil Velikov]

Hi Emily,

On Wed, 24 Apr 2019 at 10:19, Deng, Emily <Emily.Deng@amd.com> wrote:
>
> Hi Emil,
>     I don't understand your idea clear about follow, what about the case that only has 1 GPU, and don't support pci_domain? For this case, it still need to fallback to pci_domain_ok=0.

I'm struggling to understand - seems like some of the grammar is off.
Can you elaborate?

Aside: interleaved posting style is used for open-source development
(kernel&libdrm at least). Please use tweak your apps to produce it.

Thanks
Emil

[1] https://www.extendoffice.com/documents/outlook/4006-outlook-reply-quote.html
See "Reply With Quoting Original Message In Outlook" although try to
use interleaved [2]/bottom posting style.
[2] https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel