[PATCHv3 4/6] efi_selftest_memory: check for duplicates first

[Ilias Apalodimas]

On Fri, 17 Apr 2026 at 19:51, Randolph Sapp <rs at ti.com> wrote:
>
> On Fri Apr 17, 2026 at 3:17 AM CDT, Ilias Apalodimas wrote:
> > On Thu, 16 Apr 2026 at 23:26, Randolph Sapp <rs at ti.com> wrote:
> >>
> >> On Thu Apr 16, 2026 at 3:55 AM CDT, Ilias Apalodimas wrote:
> >> > On Mon, 13 Apr 2026 at 23:36, <rs at ti.com> wrote:
> >> >>
> >> >> From: Randolph Sapp <rs at ti.com>
> >> >>
> >> >> Check for duplicate memory mappings before reporting any incorrect
> >> >> attributes. Could be that second allocation has the correct type while
> >> >> the first doesn't. Knowing there is a duplicate in this scenario is
> >> >> more helpful than just reporting the first mismatch.
> >> >>
> >> >> Signed-off-by: Randolph Sapp <rs at ti.com>
> >> >
> >> > [...]
> >> >
> >> >>                         }
> >> >> -                       found = true;
> >> >> -                       if (memory_type != entry->type) {
> >> >> -                               efi_st_error
> >> >> -                                       ("Wrong memory type %d, expected %d\n",
> >> >> -                                        entry->type, memory_type);
> >> >> -                               return EFI_ST_FAILURE;
> >> >> -                       }
> >> >> +                       match = entry;
> >> >>                 }
> >> >>         }
> >> >> -       if (!found) {
> >> >> +       if (!match) {
> >> >>                 efi_st_error("Missing memory map entry\n");
> >> >>                 return EFI_ST_FAILURE;
> >> >>         }
> >> >> +       if (memory_type != match->type) {
> >> >> +               efi_st_error("Wrong memory type %d, expected %d\n", match->type,
> >> >> +                            memory_type);
> >> >> +               return EFI_ST_FAILURE;
> >> >
> >> > This check is now outside the loop and only checks for the last entry.
> >> > If you wan't to split the fucntionality, don't we need a loop over all
> >> > memory areas and the type?
> >> >
> >> > Thanks
> >> > /Ilias
> >>
> >> Not necessarily. At the end of the day we can only really raise one exception
> >> anyway. I just think informing the user about a duplicate should take priority
> >> above mismatched attributes. It hints at a bigger issue.
> >
> > I dont mind about the priority. We can swap that over. But with this
> > patch we will check for less problems than we currently do.
> >
> > Thanks
> > /Ilias
>
> In what regards? We're still checking for mismatched attributes, we just check
> for duplicates first. If there's a dupe it doesn't really matter what the
> attributes are on either allocation, good or bad something horrible has
> occurred.
>
> The prior version of this would never report if there was a duplicate unless the
> first allocation it saw had the correct attributes.

Ah I misread the loop. I thought it was trying to do that for multiple
addresses, but we only have a single address in 'addr'.

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>

>
> >>
> >> >> +       }
> >> >>         return EFI_ST_SUCCESS;
> >> >>  }
> >> >>
> >> >> --
> >> >> 2.53.0
> >> >>
> >>
>