Re: [PATCH 2/5] spapr: Fix vpa dispatch count for record-replay

[Nicholas Piggin]

On Fri Dec 20, 2024 at 7:14 PM AEST, Harsh Prateek Bora wrote:
> Hi Nick,
>
> On 12/19/24 09:10, Nicholas Piggin wrote:
> > The dispatch count is a field in guest memory that the hypervisor
> > increments when preempting and dispatching the guest. This was not
> > being done deterministically with respect to icount, because tcg
> > exec exit is not deterministic (e.g., an async event could cause it).
> > 
> > Change vpa dispatch count increment to keep track of whether the
> > vCPU is considered dispatched or not, and only consider it preempted
> > when calling cede / confer / join / stop-self / etc.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   include/hw/ppc/spapr_cpu_core.h |  3 +++
> >   hw/ppc/spapr.c                  | 36 ++-------------------------------
> >   hw/ppc/spapr_hcall.c            | 33 ++++++++++++++++++++++++++++++
> >   hw/ppc/spapr_rtas.c             |  1 +
> >   4 files changed, 39 insertions(+), 34 deletions(-)
> > 
>
> <snipped>
>
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 5e1d020e3df..907e09c2c36 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -487,6 +487,36 @@ static target_ulong h_register_vpa(PowerPCCPU *cpu, 
> > SpaprMachineState *spapr,
> >       return ret;
> >   }
> >   
> > +void vpa_dispatch(CPUState *cs, SpaprCpuState *spapr_cpu, bool dispatch)
> > +{
> > +    uint32_t counter;
> > +
> > +    if (!dispatch) {
> > +        assert(spapr_cpu->dispatched);
> > +    } else {
> > +        assert(!spapr_cpu->dispatched);
> > +    }
>
> Would it look better as:
>         assert(!(dispatch == spapr_cpu->dispatched));

I think I wanted to know which way it was failing, but I guess
assert_cmpint() will tell us? I'll change.

>
> > +    spapr_cpu->dispatched = dispatch;
> > +
> > +    return;
>
> Returning here unconditionally makes below code unreachable.

Good catch. That was a testing hack. Will respin and retest.

Thanks,
Nick