On Wed, Jul 20, 2022, Maciej S. Szmigiero wrote:
> On 20.07.2022 10:43, Maxim Levitsky wrote:
> > On Mon, 2022-07-18 at 17:47 +0200, Maciej S. Szmigiero wrote:
> > > Fixes: 6ef88d6e36c2 ("KVM: SVM: Re-inject INT3/INTO instead of retrying the instruction")
> > > Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@xxxxxxxxxx>
> > > ---
> > > arch/x86/kvm/svm/nested.c | 9 +++++----
> > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > > index adf4120b05d90..23252ab821941 100644
> > > --- a/arch/x86/kvm/svm/nested.c
> > > +++ b/arch/x86/kvm/svm/nested.c
> > > @@ -639,7 +639,8 @@ static bool is_evtinj_nmi(u32 evtinj)
> > > }
> > > static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
> > > - unsigned long vmcb12_rip)
> > > + unsigned long vmcb12_rip,
> > > + unsigned long vmcb12_csbase)
> >
> > Honestly I don't like that nested_vmcb02_prepare_control starts to grow its parameter list,
> > because it kind of defeats the purpose of vmcb12 cache we added back then.
> >
> > I think that it is better to add csbase/rip to vmcb_save_area_cached,
> > but I am not 100% sure. What do you think?
>
> This function has only 3 parameters now, so they fit well into registers
> without taking any extra memory (even assuming it won't get inlined).
>
> If in the future more parameters need to be added to this function
> (which may or may not happen) then they all can be moved to, for example,
> vmcb_ctrl_area_cached.
I don't think Maxim is concerned about the size, rather that we have a dedicated
struct for snapshotting select save state and aren't using it.
IIRC, I deliberately avoided using the "cache" because the main/original purpose
of the cache is to avoid TOCTOU issues. And because RIP and CS.base aren't checked,
there's no need to throw them in the cache.
