Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <53b6874517a76d8d046e665c1f2f378769b721a0.camel@redhat.com>
Subject: Re: [PATCH v2 00/21] KVM: x86: Event/exception fixes and cleanups
From:   Maxim Levitsky <mlevitsk@redhat.com>
To:     Jim Mattson <jmattson@google.com>
Cc:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Joerg Roedel <joro@8bytes.org>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, Oliver Upton <oupton@google.com>,
        Peter Shier <pshier@google.com>
Date:   Fri, 01 Jul 2022 10:37:34 +0300
In-Reply-To: <CALMp9eSWkjHyer9CZL7UN4s8Ashc1svZsAnEgJDd2Q9voSz7HQ@mail.gmail.com>
References: <20220614204730.3359543-1-seanjc@google.com>
         <7e05e0befa13af05f1e5f0fd8658bc4e7bdf764f.camel@redhat.com>
         <CALMp9eSkdj=kwh=4WHPsWZ1mKr9+0VSB527D5CMEx+wpgEGjGw@mail.gmail.com>
         <f55889a50ba404381e3edc1a192770f2779d40f1.camel@redhat.com>
         <CALMp9eSWkjHyer9CZL7UN4s8Ashc1svZsAnEgJDd2Q9voSz7HQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5 (3.36.5-2.fc32) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 2022-06-30 at 09:28 -0700, Jim Mattson wrote:
> On Thu, Jun 30, 2022 at 1:22 AM Maxim Levitsky <mlevitsk@redhat.com> wrote:
> > On Wed, 2022-06-29 at 06:42 -0700, Jim Mattson wrote:
> > > Unlike the AMD "INTn intercept," these trap intercepts *do not* happen
> > > at the start of the instruction.
> > 
> > Are you sure about that?
> 
> I had been sure when I wrote that, but now that I see your response, I
> have to question my memory. The SDM is definitely more authoritative
> than I am.

x86 is like a fractal, the more I know it more I realize I don't.

> 
> > > When you say "ignores," do you mean that AMD ignores a data breakpoint
> > > or single-step trap generated by MOV-SS, or it ignores the fact that
> > > delivering such a #DB trap between the MOV-SS and the subsequent
> > > MOV-ESP will create a stack frame in the wrong place?
> > 
> > Two things which can be infered from the SVM spec.
> >         - AMD doesn't distinguish between MOV SS and STI int shadow.
> >         - AMD has no 'pending debug exception field' in the vmcb.
> > 
> > I don't know what AMD does for #DB that happens on MOV SS, nor if it
> > does distinguish these internally,
> > probably just drops the #DB or something.
> 
> Without carrying pending debug exceptions, it seems that the only two
> choices are to deliver the #DB, with the exception frame in an
> unintended location or to drop the #DB. The latter seems preferable,
> but neither one seems good. What I don't understand is why you claim
> that AMD does this "rightfully." Are you saying that anyone with the
> audacity to run a debugger on legacy code deserves to be thrown in
> front of a moving train?

I understand what you mean, its a tradeof of 100% compliant implementation
vs complexity the corner cases introduce. #DB can already be missed in some
cases I think, especially from my experience from debuggers, and even more especially
when debugging an OS.

It is a pain, as the OS naturally tries to switch tasks and process
interrupts all the time, I even added that _BLOCKIRQ flag to KVM to make it a bit better.

But still I understand what you mean, so maybe indeed VMX did it better.

> 
> > > Hence, the facility for injecting a "pending MTF"--so that it won't be "lost."
> > Yes, though that is would be mostly useful for nesting.
> > 
> > For not nesting hypervisor, if the hypervisor figured out that a higher priority event overrode
> > the MTF, it can just process the MTF - why to re-inject it?
> 
> You're right. The facility is probably just there to make MTF
> virtualizable. Intel was paying much closer attention to
> virtualizability by the time MTF came along.

That makes sense.


> 
> > > These are single-step, I/O and data breakpoint traps.
> > 
> > I am not sure what you mean. single-step, IO, data breakpoints are indeed the trap #DB,
> > while "general detect", code breakpoint are fault #DB, and we also have the task switch #DB, but since the hardware doesn't
> > emulate the task switches, this has to be injected.
> 
> Just enumerating. No more, no less.
> 

All right, thank you very much for the help, especialy for the tables you provided,
all of this should be enough now for me to review the patch series.

Thanks,
Best regards,
	Maxim Levitsky