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 <npig...@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
