Re: [PATCH v11 06/13] tests/qtest: Adjust and document query-cpu-model-expansion test for arm

[Peter Maydell]

On Fri, 28 Apr 2023 at 08:45, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/27/23 14:16, Fabiano Rosas wrote:
> > Richard Henderson <richard.henderson@linaro.org> writes:
> >
> >> On 4/26/23 19:00, Fabiano Rosas wrote:
> >>> We're about to move the 32-bit CPUs under CONFIG_TCG, so adjust the
> >>> query-cpu-model-expansion test to check against the cortex-a7, which
> >>> is already under CONFIG_TCG. That allows the next patch to contain
> >>> only code movement.
> >>>
> >>> While here add comments clarifying what we're testing.
> >>>
> >>> Signed-off-by: Fabiano Rosas<farosas@suse.de>
> >>> Suggested-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> >>> ---
> >>>    tests/qtest/arm-cpu-features.c | 20 +++++++++++++++++---
> >>>    1 file changed, 17 insertions(+), 3 deletions(-)
> >>
> >> I don't see why you're changing the cpu model here.
> >> Neither cpu will work, of course, but why change?
> >>
> >
> > Because there's already a patch in master that puts the cortex-a7 under
> > CONFIG_TCG, so I can have the whole if/else in this patch.
> >
> > If I keep the cortex-a15, this change needs to go into the next patch
> > ("move cpu_tcg to tcg/cpu32.c") which moves the rest of the 32bit cpus,
> > which was supposed to be only code movement.
>
> Well, I still think the change to a7 is wrong.
> If the two patches need to be merged to break bisection,
> then so be it -- just mention that fact in the commit message.
>
> Peter, do you agree?

I see your point, but on the other hand this is only test
code, and the situation it's checking is a rather
uninteresting one anyway. So given that this series is
already on v11 and it's an awkward one to have to keep
rebasing, I'd rather take it as-is than ask for a reroll.

thanks
-- PMM