[igt-dev] [PATCH v1] tools/intel_gtt: stop confusing GTT and BAR sizes

[Daniel Tatianin]

On Wed, Nov 23, 2022 at 2:46 PM Kamil Konieczny <
kamil.konieczny at linux.intel.com> wrote:

> Hi Daniil,
>
> On 2022-11-18 at 01:59:07 +0300, Daniil Tatianin wrote:
> > The GTT iteration loop would erroneously check "< gtt_size" as an
> > indicator for when it should stop, which would make it exit the loop
> > right after dumping 2-8 mib of address space depending on the
> > GPU generation.
> >
> > Calculate the actual size of GTT in 'gtt_max_addr', and rewrite the
> > iteration loop to use it. While at it, also make virtual addresses
> > 64-bit so that they don't wrap around at the 4GiB mark.
> >
> > Signed-off-by: Daniil Tatianin <99danilt at gmail.com>
>
> There is only one concern - this programm assumes 4KB page size
>
Right, I think this entire utility assumes 4KB page size. Is that not
always the case?
I think that it should be a separate patch if we're going to get rid of
that assumption, as
it basically has that constant all over the place.


> and uses unsigned instead of unsigned int (or uint64_t).
>
Oops, my bad


> Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>
> > ---
> >  tools/intel_gtt.c | 32 ++++++++++++++++++--------------
> >  1 file changed, 18 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/intel_gtt.c b/tools/intel_gtt.c
> > index 311694ba..c0f3d16a 100644
> > --- a/tools/intel_gtt.c
> > +++ b/tools/intel_gtt.c
> > @@ -96,6 +96,11 @@ static uint64_t get_phys(uint32_t pt_offset)
> >       return (phys | pae) & ~0xfff;
> >  }
> >
> > +static int get_pte_size(void)
> > +{
> > +     return intel_gen(devid) < 8 ? 4 : 8;
> > +}
> > +
> >  static void pte_dump(int size, uint32_t offset) {
> >       int pte_size;
> >       int entries;
> > @@ -105,11 +110,7 @@ static void pte_dump(int size, uint32_t offset) {
> >       if (size % 16)
> >               size = (size + 16) & ~0xffff;
> >
> > -     if (intel_gen(devid) < 8)
> > -             pte_size = 4;
> > -     else
> > -             pte_size = 8;
> > -
> > +     pte_size = get_pte_size();
> >       entries = size / pte_size;
> >
> >       printf("GTT offset   |                 %d PTEs (%d MB)\n", entries,
> > @@ -139,7 +140,8 @@ static void pte_dump(int size, uint32_t offset) {
> >  int main(int argc, char **argv)
> >  {
> >       struct pci_device *pci_dev;
> > -     unsigned int start, gtt_size;
> > +     unsigned gtt_size;
> > +     uint64_t start, gtt_max_addr;
> >       int flag[] = {
> >               PCI_DEV_MAP_FLAG_WRITE_COMBINE,
> >               PCI_DEV_MAP_FLAG_WRITABLE,
> > @@ -190,14 +192,15 @@ int main(int argc, char **argv)
> >               return 0;
> >       }
> >
> > -     for (start = 0; start < gtt_size; start += KB(4)) {
> > -             uint64_t start_phys = get_phys(start);
> > -             uint32_t end;
> > +     gtt_max_addr = (gtt_size / get_pte_size()) * KB(4ull);
> > +
> > +     for (start = 0; start < gtt_max_addr; start += KB(4)) {
> > +             uint64_t end, start_phys = get_phys(start);
> >               int constant_length = 0;
> >               int linear_length = 0;
> >
> >               /* Check if it's a linear sequence */
> > -             for (end = start + KB(4); end < gtt_size; end += KB(4)) {
> > +             for (end = start + KB(4); end < gtt_max_addr; end +=
> KB(4)) {
> >                       uint64_t end_phys = get_phys(end);
> >                       if (end_phys == start_phys + (end - start))
> >                               linear_length++;
> > @@ -205,7 +208,7 @@ int main(int argc, char **argv)
> >                               break;
> >               }
> >               if (linear_length > 0) {
> > -                     printf("0x%08x - 0x%08x: linear from "
> > +                     printf("0x%08" PRIx64 "- 0x%08" PRIx64 ": linear
> from "
> >                              "0x%" PRIx64 " to 0x%" PRIx64 "\n",
> >                              start, end - KB(4),
> >                              start_phys, start_phys + (end - start) -
> KB(4));
> > @@ -214,7 +217,7 @@ int main(int argc, char **argv)
> >               }
> >
> >               /* Check if it's a constant sequence */
> > -             for (end = start + KB(4); end < gtt_size; end += KB(4)) {
> > +             for (end = start + KB(4); end < gtt_max_addr; end +=
> KB(4)) {
> >                       uint64_t end_phys = get_phys(end);
> >                       if (end_phys == start_phys)
> >                               constant_length++;
> > @@ -222,13 +225,14 @@ int main(int argc, char **argv)
> >                               break;
> >               }
> >               if (constant_length > 0) {
> > -                     printf("0x%08x - 0x%08x: constant 0x%" PRIx64 "\n",
> > +                     printf("0x%08" PRIx64 " - 0x%08" PRIx64
> > +                             ": constant 0x%" PRIx64 "\n",
> >                              start, end - KB(4), start_phys);
> >                       start = end - KB(4);
> >                       continue;
> >               }
> >
> > -             printf("0x%08x: 0x%" PRIx64 "\n", start, start_phys);
> > +             printf("0x%08" PRIx64 ": 0x%" PRIx64 "\n", start,
> start_phys);
> >       }
> >
> >       return 0;
> > --
> > 2.36.1.windows.1
> >

Thanks!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20221124/cedbfc4f/attachment-0001.htm>