Re: [PATCH 4/9] target/arm: Support migration when FPSR/FPCR won't fit in the FPSCR

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 28 Jun 2024 at 17:01, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 6/28/24 07:23, Peter Maydell wrote:
> > To support FPSR and FPCR bits that don't exist in the AArch32 FPSCR
> > view of floating point control and status (such as the FEAT_AFP ones),
> > we need to make sure those bits can be migrated. This commit allows
> > that, whilst maintaining backwards and forwards migration compatibility
> > for CPUs where there are no such bits:
> >
> > On sending:
> >   * If either the FPCR or the FPSR include set bits that are not
> >     visible in the AArch32 FPSCR view of floating point control/status
> >     then we send the FPCR and FPSR as two separate fields in a new
> >     cpu/vfp/fpcr_fpsr subsection, and we send a 0 for the old
> >     FPSCR field in cpu/vfp
> >   * Otherwise, we don't send the fpcr_fpsr subsection, and we send
> >     an FPSCR-format value in cpu/vfp as we did previously
> >
> > On receiving:
> >   * if we see a non-zero FPSCR field, that is the right information
> >   * if we see a fpcr_fpsr subsection then that has the information
> >   * if we see neither, then FPSCR/FPCR/FPSR are all zero on the source;
> >     cpu_pre_load() ensures the CPU state defaults to that
> >   * if we see both, then the migration source is buggy or malicious;
> >     either the fpcr_fpsr or the FPSCR will "win" depending which
> >     is first in the migration stream; we don't care which that is
> >
> > We make the new FPCR and FPSR on-the-wire data be 64 bits, because
> > architecturally these registers are that wide, and this avoids the
> > need to engage in further migration-compatibility contortions in
> > future if some new architecture revision defines bits in the high
> > half of either register.
> >
> > (We won't ever send the new migration subsection until we add support
> > for a CPU feature which enables setting overlapping FPCR bits, like
> > FEAT_AFP.)
> >
> > Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> > ---
> >   target/arm/machine.c | 134 ++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 132 insertions(+), 2 deletions(-)
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> Not ideal, as vfp_get_{fpcr,fpsr} are called 3 or 4 times during migration.  But unless we
> have separate 'fp*r_migrate' fields in cpu state, initialized in pre_save, there's no
> getting around it.  And I suppose migration isn't exactly performance critical.

Yeah, we could have done it that way, but I am assuming that
the time taken for this is pretty miniscule in the general
scheme of how long migration takes, so I preferred the
way that doesn't clutter up the CPU state struct with
migration-only fields.

If somebody cares about migration downtime performance (which
does actually matter for some workload/use cases AIUI) they
can do some benchmarking and tell us what the actually
slow parts are :-)

thanks
-- PMM