Re: [PATCH v3 05/27] KVM: SVM: Add helpers to set/clear ASID flush in VMCB

[Yosry Ahmed]

> > > > +static inline void vmcb_set_flush_asid(struct vmcb *vmcb)
> > > > +{
> > > > +       vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ASID;
> > > > +}
> > > > +
> > > > +static inline void vmcb_clr_flush_asid(struct vmcb *vmcb)
> > >
> > > The name of this function makes it sound like the inverse of
> > > vmcb_set_flush_asid(), which it isn't. Maybe vmcb_clr_tlb_ctl()?
> >
> > Why not? vmcb_set_flush_asid() sets FLUSH_ASID and
> > vmcb_clr_flush_asid() clears it, right?
>
> Not exactly. The tlb_ctl field is not a boolean.

Right, but the current implementation treats it as such (with the
exception of falling back to FLUSH_ALL_ASID, but that becomes
transparent to the callers).

>
> vmcb_set_flush_asid() sets tlb_ctl to 3 (flush current ASID).
> vmcb_clr_flush_asid() sets tlb_ctl to 0 (do not flush).
>
> Other legal values are 1 (flush entire TLB) and 7 (flush non-global
> entries of current ASID).
>
> And, in fact, these values were chosen so that older hardware would
> see both 3 and 7 as "flush entire TLB."
>
> So, it's a little more nuanced than set and clear "flush by ASID."

I agree, I chose to abstract it as a boolean, but I understand it can
be confusing.

>
> Maybe vmcb_set_flush_asid() and vmcb_set_flush_nothing()?

I would prefer vmcb_set_flush_asid() and vmcb_set_no_flush().