Re: [PATCH 06/18] KVM: arm64: Add three sets of flags to the vcpu state

[Marc Zyngier <maz@kernel.org>]

On Thu, 09 Jun 2022 07:10:14 +0100,
Reiji Watanabe <reijiw@google.com> wrote:
> 
> Hi Marc,
> 
> On Sat, May 28, 2022 at 4:38 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > It so appears that each of the vcpu flags is really belonging to
> > one of three categories:
> >
> > - a configuration flag, set once and for all
> > - an input flag generated by the kernel for the hypervisor to use
> > - a state flag that is only for the kernel's own bookkeeping
> >
> > As we are going to split all the existing flags into these three
> > sets, introduce all three in one go.
> >
> > No functional change other than a bit of bloat...
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/include/asm/kvm_host.h | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 5eb6791df608..c9dd0d4e22f2 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -338,6 +338,15 @@ struct kvm_vcpu_arch {
> >         /* Miscellaneous vcpu state flags */
> >         u64 flags;
> >
> > +       /* Configuration flags */
> > +       u64 cflags;
> > +
> > +       /* Input flags to the hypervisor code */
> > +       u64 iflags;
> > +
> > +       /* State flags, unused by the hypervisor code */
> > +       u64 sflags;
> 
> Although I think VCPU_SVE_FINALIZED could be considered "state" rather
> than "configuration", I assume the reason why it is handled by cflags
> in the following patches is because VCPU_SVE_FINALIZED is set once
> for all. If my assumption is correct, it would be clearer to add
> "set once and for all" in the comment for cflags.

Yes, that's indeed the reason for this categorisation. In general,
these flags are, as you put it, set once and for all extremely early
(before the vcpu can run), and are never cleared. I'll update the
comment accordingly.

> Also, if we end up using VCPU_SVE_FINALIZED in hypervisor code later,
> then should it be handled by iflags instead of cflags ?

That'd be my expectation if they ended up changing state at some
point. My view is that the cflags are immutable once the vcpu has
run, and flags that can change state over the life if the vcpu
shouldn't be in that category.

> 
> My understanding of how those flags should be used is as follows.
> Is my understanding correct ?
> 
>  iflags: flags that are used by hypervisor code

Yes. Crucially, they are used as an input to the hypervisor code: it
either consumes these flags (INCREMENT_PC, PENDING_EXCEPTION), or
consult them to decide what to do.

>  cflags: flags that are set once for all and unused by hypervisor code

Yes.

>  sflags: flags that could be set/cleared more than once and unused
>          by hypervisor code

Yes. They are really bookkeeping flags for the kernel code.

I'll try to incorporate some of that in the comments before reposting
the series.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel