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;
Date:   Tue, 20 Sep 2022 18:46:28 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Oliver Upton <oliver.upton@linux.dev>
Cc:     Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Andrew Jones <andrew.jones@linux.dev>,
        Anup Patel <anup@brainfault.org>,
        Atish Patra <atishp@atishpatra.org>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Janosch Frank <frankja@linux.ibm.com>,
        Claudio Imbrenda <imbrenda@linux.ibm.com>
Subject: Re: [PATCH 4/5] KVM: selftests: Explicitly verify KVM doesn't patch
 hypercall if quirk==off
Message-ID: <YyoKhKPf6Bv4X56B@google.com>
References: <20220908233134.3523339-1-seanjc@google.com>
 <20220908233134.3523339-5-seanjc@google.com>
 <Yyjd7pcBw0NkYVQE@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yyjd7pcBw0NkYVQE@google.com>
Precedence: bulk

On Mon, Sep 19, 2022, Oliver Upton wrote:
> On Thu, Sep 08, 2022 at 11:31:33PM +0000, Sean Christopherson wrote:
> > @@ -75,12 +76,28 @@ static void guest_main(void)
> >  	}
> >  
> >  	/*
> > -	 * The hypercall didn't #UD (guest_ud_handler() signals "done" if a #UD
> > -	 * occurs).  Verify that a #UD is NOT expected and that KVM patched in
> > -	 * the native hypercall.
> > +	 * If the quirk is disabled, verify that guest_ud_handler() "returned"
> > +	 * -EFAULT and that KVM did NOT patch the hypercall.  If the quirk is
> > +	 * enabled, verify that the hypercall succeeded and that KVM patched in
> > +	 * the "right" hypercall.
> >  	 */
> > -	GUEST_ASSERT(!ud_expected);
> > -	GUEST_ASSERT(!memcmp(native_hypercall_insn, hypercall_insn, HYPERCALL_INSN_SIZE));
> > +	if (ud_expected) {
> > +		GUEST_ASSERT(ret == (uint64_t)-EFAULT);
> > +
> > +		/*
> > +		 * Divergence should occur only on the last byte, as the VMCALL
> > +		 * (0F 01 C1) and VMMCALL (0F 01 D9) share the first two bytes.
> > +		 */
> > +		GUEST_ASSERT(!memcmp(native_hypercall_insn, hypercall_insn,
> > +				     HYPERCALL_INSN_SIZE - 1));
> > +		GUEST_ASSERT(memcmp(native_hypercall_insn, hypercall_insn,
> > +				    HYPERCALL_INSN_SIZE));
> 
> Should we just keep the assertions consistent for both cases (patched
> and unpatched)?

Not sure I follow what you're suggesting.  By "consistent" do you mean doing
something like snapshotting hypercall_insn and verifying that it's not changed?