[PATCH v2 27/32] test: cedit: use allocated address for reading file

[Simon Glass]

Hi Sughosh,

On Fri, 16 Aug 2024 at 11:53, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> On Fri, 16 Aug 2024 at 22:39, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > On Fri, 16 Aug 2024 at 20:17, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Fri, 16 Aug 2024 at 04:34, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > >
> > > > On Fri, 16 Aug 2024 at 02:02, Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Wed, 14 Aug 2024 at 05:02, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > > >
> > > > > > Instead of a randomly selected address, use an LMB allocated one for
> > > > > > reading the file into memory. With the LMB map now being persistent
> > > > > > and global, the address used for reading the file might be already
> > > > > > allocated as non-overwritable, resulting in a failure. Get a valid
> > > > > > address from LMB and then read the file to that address.
> > > > > >
> > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > > > > ---
> > > > > > Changes since V1:
> > > > > > * Don't use the API version with flags parameter.
> > > > > >
> > > > > >  test/boot/cedit.c | 6 +++++-
> > > > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > > >
> > > > >
> > > > > No, this address needs to work fine without using lmb. Same with any
> > > > > other tests. Tests make use of the sandbox memory space memory
> > > > > addresses and it makes things easier to code and debug.
> > > >
> > > > Like I had explained earlier [1], not using the LMB API for allocating
> > > > the address results in issues, since the load command internally
> > > > checks if the address can be used for reading the dtb. Without this
> > > > patch, the cmd_ut test fails. I am not sure why you do not like this
> > > > solution. But in any case, can you propose some other solution? I
> > > > believe I can tweak the address to some other value, but that would
> > > > not be a proper solution, but simply kicking the can down the road.
> > > > Thanks.
> > >
> > > But this is why I suggested having lmb_push() and lmb_pop(), so you
> > > should be able to use those in the few tests which have this problem.
> >
> > Ah yes, I forgot about that change. Let me try running the test again
> > and see what I get. Thanks.
>
> Sorry, I think I was hasty in my reply. This would not work. We have
> the lmb_push()/lmb_get() functions only for the lmb specific tests. In
> this scenario, we have the load command which checks the lmb address
> against reservations, which are the "normal operation" memory map. So,
> if we have to have this to work, this might need a way of using a new
> lmb structure instance for every test. So that every test that gets
> invoked, does so like the way the lmb tests are.

Why does the 'load' operation fail? When I revert this patch the tests
still seem to pass.

Regards,
Simon


>
> -sughosh
>
> >
> > -sughosh
> >
> > >
> > > If it becomes more widespread and affects a lot of tests, we could do
> > > thing automatically in the test framework, but for now, that doesn't
> > > seem necessary.
> > >
> > > >
> > > > -sughosh
> > > >
> > > > [1] - https://lists.denx.de/pipermail/u-boot/2024-July/560569.html
> > >
> > > Regards,
> > > Simon