<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    Good point. Thanks.<br>
    <br>
    Eric<br>
    <br>
    <div class="moz-cite-prefix">On 2021-05-04 10:30 a.m., Lazar, Lijo
      wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:MN2PR12MB45495C464A09AB31CE635986975A9@MN2PR12MB4549.namprd12.prod.outlook.com">
      
      <p style="font-family:Arial;font-size:11pt;color:#0078D7;margin:5pt;" align="Left">
        [AMD Official Use Only - Internal Distribution Only]<br>
      </p>
      <br>
      <div>
        <div style="color: rgb(33, 33, 33); background-color: rgb(255,
          255, 255);" dir="auto">
          Converting using pxm_to_node and then comparing against pxm
          value looks a bit odd. Shouldn't the comparsion be between
          equals - node to node or pxm to pxm?</div>
        <div id="ms-outlook-mobile-signature">
          <div><br>
          </div>
          Thanks,<br>
          Lijo</div>
        <hr style="display:inline-block;width:98%" tabindex="-1">
        <div id="divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>From:</b>
            Huang, JinHuiEric <a class="moz-txt-link-rfc2396E" href="mailto:JinHuiEric.Huang@amd.com"><JinHuiEric.Huang@amd.com></a><br>
            <b>Sent:</b> Tuesday, May 4, 2021 7:30:44 PM<br>
            <b>To:</b> Lazar, Lijo <a class="moz-txt-link-rfc2396E" href="mailto:Lijo.Lazar@amd.com"><Lijo.Lazar@amd.com></a>;
            <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>
            <a class="moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org"><amd-gfx@lists.freedesktop.org></a><br>
            <b>Subject:</b> Re: [PATCH] drm/amdkfd: add ACPI SRAT
            parsing for topology</font>
          <div> </div>
        </div>
        <div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
              <div class="PlainText">Like I answer Oak's question,<br>
                "For GCD parsing, the relation of GCD to CCD is defined
                by AMD, generic <br>
                parsing in srat.c is considering a GCD as a new numa
                node which is not <br>
                suitable for our need."<br>
                <br>
                GCD's pxm domain will get a wrong numa node which may be
                bigger than CCD <br>
                domains, so I have to do a sanity check to correct it.<br>
                <br>
                Regards,<br>
                Eric<br>
                <br>
                On 2021-05-04 3:46 a.m., Lazar, Lijo wrote:<br>
                > [AMD Public Use]<br>
                ><br>
                >> *numa_node > max_pxm<br>
                > Why numa node number is compared to a proximity
                domain? Since you are already using pxm_to_node() API,
                assume that should take care.<br>
                ><br>
                > That also will avoid parsing
                ACPI_SRAT_TYPE_CPU_AFFINITY structs.<br>
                ><br>
                > Thanks,<br>
                > Lijo<br>
                ><br>
                ><br>
                > -----Original Message-----<br>
                > From: amd-gfx
                <a class="moz-txt-link-rfc2396E" href="mailto:amd-gfx-bounces@lists.freedesktop.org"><amd-gfx-bounces@lists.freedesktop.org></a> On Behalf
                Of Eric Huang<br>
                > Sent: Wednesday, April 28, 2021 8:42 PM<br>
                > To: <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><br>
                > Cc: Huang, JinHuiEric
                <a class="moz-txt-link-rfc2396E" href="mailto:JinHuiEric.Huang@amd.com"><JinHuiEric.Huang@amd.com></a><br>
                > Subject: [PATCH] drm/amdkfd: add ACPI SRAT parsing
                for topology<br>
                ><br>
                > In NPS4 BIOS we need to find the closest numa node
                when creating topology io link between cpu and gpu, if
                PCI driver doesn't set it.<br>
                ><br>
                > Signed-off-by: Eric Huang
                <a class="moz-txt-link-rfc2396E" href="mailto:jinhuieric.huang@amd.com"><jinhuieric.huang@amd.com></a><br>
                > ---<br>
                >   drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 94
                ++++++++++++++++++++++++++-<br>
                >   1 file changed, 91 insertions(+), 3 deletions(-)<br>
                ><br>
                > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
                b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c<br>
                > index 38d45711675f..57518136c7d7 100644<br>
                > --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c<br>
                > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c<br>
                > @@ -1759,6 +1759,87 @@ static int
                kfd_fill_gpu_memory_affinity(int *avail_size,<br>
                >        return 0;<br>
                >   }<br>
                >   <br>
                > +#ifdef CONFIG_ACPI<br>
                > +static void kfd_find_numa_node_in_srat(struct
                kfd_dev *kdev,<br>
                > +             int *numa_node)<br>
                > +{<br>
                > +     struct acpi_table_header *table_header =
                NULL;<br>
                > +     struct acpi_subtable_header *sub_header =
                NULL;<br>
                > +     unsigned long table_end, subtable_len;<br>
                > +     u32 pci_id =
                pci_domain_nr(kdev->pdev->bus) << 16 |<br>
                > +                     pci_dev_id(kdev->pdev);<br>
                > +     u32 bdf;<br>
                > +     acpi_status status;<br>
                > +     struct acpi_srat_cpu_affinity *cpu;<br>
                > +     struct acpi_srat_generic_affinity *gpu;<br>
                > +     int pxm = 0, max_pxm = 0;<br>
                > +     bool found = false;<br>
                > +<br>
                > +     /* Fetch the SRAT table from ACPI */<br>
                > +     status = acpi_get_table(ACPI_SIG_SRAT, 0,
                &table_header);<br>
                > +     if (status == AE_NOT_FOUND) {<br>
                > +             pr_warn("SRAT table not found\n");<br>
                > +             return;<br>
                > +     } else if (ACPI_FAILURE(status)) {<br>
                > +             const char *err =
                acpi_format_exception(status);<br>
                > +             pr_err("SRAT table error: %s\n",
                err);<br>
                > +             return;<br>
                > +     }<br>
                > +<br>
                > +     table_end = (unsigned long)table_header +
                table_header->length;<br>
                > +<br>
                > +     /* Parse all entries looking for a match. */<br>
                > +<br>
                > +     sub_header = (struct acpi_subtable_header *)<br>
                > +                     ((unsigned long)table_header
                +<br>
                > +                     sizeof(struct
                acpi_table_srat));<br>
                > +     subtable_len = sub_header->length;<br>
                > +<br>
                > +     while (((unsigned long)sub_header) +
                subtable_len  < table_end) {<br>
                > +             /*<br>
                > +              * If length is 0, break from this
                loop to avoid<br>
                > +              * infinite loop.<br>
                > +              */<br>
                > +             if (subtable_len == 0) {<br>
                > +                     pr_err("SRAT invalid zero
                length\n");<br>
                > +                     break;<br>
                > +             }<br>
                > +<br>
                > +             switch (sub_header->type) {<br>
                > +             case ACPI_SRAT_TYPE_CPU_AFFINITY:<br>
                > +                     cpu = (struct
                acpi_srat_cpu_affinity *)sub_header;<br>
                > +                     pxm = *((u32
                *)cpu->proximity_domain_hi) << 8 |<br>
                > +                                    
                cpu->proximity_domain_lo;<br>
                > +                     if (pxm > max_pxm)<br>
                > +                             max_pxm = pxm;<br>
                > +                     break;<br>
                > +             case ACPI_SRAT_TYPE_GENERIC_AFFINITY:<br>
                > +                     gpu = (struct
                acpi_srat_generic_affinity *)sub_header;<br>
                > +                     bdf = *((u16
                *)(&gpu->device_handle[0])) << 16 |<br>
                > +                                     *((u16
                *)(&gpu->device_handle[2]));<br>
                > +                     if (bdf == pci_id) {<br>
                > +                             found = true;<br>
                > +                             *numa_node =
                pxm_to_node(gpu->proximity_domain);<br>
                > +                     }<br>
                > +                     break;<br>
                > +             default:<br>
                > +                     break;<br>
                > +             }<br>
                > +<br>
                > +             if (found)<br>
                > +                     break;<br>
                > +<br>
                > +             sub_header = (struct
                acpi_subtable_header *)<br>
                > +                             ((unsigned
                long)sub_header + subtable_len);<br>
                > +             subtable_len = sub_header->length;<br>
                > +     }<br>
                > +<br>
                > +     /* workaround bad cpu-gpu binding case */<br>
                > +     if (found && (*numa_node < 0 ||
                *numa_node > max_pxm))<br>
                > +             *numa_node = 0;<br>
                > +}<br>
                > +#endif<br>
                > +<br>
                >   /* kfd_fill_gpu_direct_io_link - Fill in direct
                io link from GPU<br>
                >    * to its NUMA node<br>
                >    *  @avail_size: Available size in the memory<br>
                > @@ -1774,6 +1855,9 @@ static int
                kfd_fill_gpu_direct_io_link_to_cpu(int *avail_size,<br>
                >                        uint32_t proximity_domain)<br>
                >   {<br>
                >        struct amdgpu_device *adev = (struct
                amdgpu_device *)kdev->kgd;<br>
                > +#ifdef CONFIG_NUMA<br>
                > +     int numa_node = 0;<br>
                > +#endif<br>
                >   <br>
                >        *avail_size -= sizeof(struct
                crat_subtype_iolink);<br>
                >        if (*avail_size < 0)<br>
                > @@ -1805,9 +1889,13 @@ static int
                kfd_fill_gpu_direct_io_link_to_cpu(int *avail_size,<br>
                >   <br>
                >        sub_type_hdr->proximity_domain_from =
                proximity_domain;  #ifdef CONFIG_NUMA<br>
                > -     if (kdev->pdev->dev.numa_node ==
                NUMA_NO_NODE)<br>
                > -             sub_type_hdr->proximity_domain_to
                = 0;<br>
                > -     else<br>
                > +     if (kdev->pdev->dev.numa_node ==
                NUMA_NO_NODE) { #ifdef CONFIG_ACPI<br>
                > +             kfd_find_numa_node_in_srat(kdev,
                &numa_node); #endif<br>
                > +             sub_type_hdr->proximity_domain_to
                = numa_node;<br>
                > +            
                set_dev_node(&kdev->pdev->dev, numa_node);<br>
                > +     } else<br>
                >                sub_type_hdr->proximity_domain_to
                = kdev->pdev->dev.numa_node;  #else<br>
                >        sub_type_hdr->proximity_domain_to = 0;<br>
                > --<br>
                > 2.17.1<br>
                ><br>
                > _______________________________________________<br>
                > amd-gfx mailing list<br>
                > <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><br>
                > <a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Clijo.lazar%40amd.com%7C96808a6aab7b40861eeb08d90a580524%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552195438132467%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ipBmGTX%2Fokto1zRuQ8jlDA8p%2B8BOjHZa5WGGKNJszEY%3D&amp;reserved=0" moz-do-not-send="true">
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Clijo.lazar%40amd.com%7C96808a6aab7b40861eeb08d90a580524%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552195438132467%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ipBmGTX%2Fokto1zRuQ8jlDA8p%2B8BOjHZa5WGGKNJszEY%3D&amp;reserved=0</a><br>
                <br>
              </div>
            </span></font></div>
      </div>
    </blockquote>
    <br>
  </body>
</html>