Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 11.1 \(3445.4.7\))
Subject: Re: [RESEND RFC 0/2] Paravirtualized Control Register pinning
From:   Liran Alon <liran.alon@oracle.com>
In-Reply-To: <f4d3d392-8edf-54b2-1b90-417447240e22@redhat.com>
Date:   Tue, 24 Dec 2019 00:49:22 +0200
Cc:     John Andersen <john.s.andersen@intel.com>, tglx@linutronix.de,
        mingo@redhat.com, bp@alien8.de, x86@kernel.org, hpa@zytor.com,
        sean.j.christopherson@intel.com, vkuznets@redhat.com,
        wanpengli@tencent.com, jmattson@google.com, joro@8bytes.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <7E2D4616-83B6-41E8-8904-407F7DB33EB4@oracle.com>
References: <20191220192701.23415-1-john.s.andersen@intel.com>
 <1EBCD42E-9109-47A1-B959-6363A509D48D@oracle.com>
 <15b57d6b-0f46-01f5-1f75-b9b55db0611a@redhat.com>
 <03F5FE31-E769-4497-922B-C8613F0951FA@oracle.com>
 <f4d3d392-8edf-54b2-1b90-417447240e22@redhat.com>
To:     Paolo Bonzini <pbonzini@redhat.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On 23 Dec 2019, at 19:46, Paolo Bonzini <pbonzini@redhat.com> wrote:
>=20
> On 23/12/19 18:28, Liran Alon wrote:
>>>> Why reset CR pinned MSRs by userspace instead of KVM INIT
>>>> handling?
>>> Most MSRs are not reset by INIT, are they?
>>>=20
>>> Paolo
>>>=20
>> MSR_KVM_SYSTEM_TIME saved in vcpu->arch.time is reset at
>> kvmclock_reset() which is called by kvm_vcpu_reset() (KVM INIT
>> handler). In addition, vmx_vcpu_reset(), called from
>> kvm_vcpu_reset(), also resets multiple MSRs such as:
>> MSR_IA32_SPEC_CTRL (vmx->spec_ctrl) and MSR_IA32_UMWAIT_CONTROL
>> (msr_ia32_umwait_control).
>=20
> These probably can be removed, since they are zero at startup and at
> least SPEC_CTRL is documented[1] to be unaffected by INIT.  However, I
> couldn't find information about UMWAIT_CONTROL.
>=20
>> Having said that, I see indeed that most of MSRs are being set by
>> QEMU in kvm_put_msrs() when level >=3D KVM_PUT_RESET_STATE. When is
>> triggered by qemu_system_reset() -> cpu_synchronize_all_post_reset ->
>> cpu_synchronize_post_reset() -> kvm_cpu_synchronize_post_reset().
>>=20
>> So given current design, OK I agree with you that CR pinned MSRs
>> should be zeroed by userspace VMM.
>>=20
>> It does though seems kinda weird to me that part of CPU state is
>> initialised on KVM INIT handler and part of it in userspace VMM. It
>> could lead to inconsistent (i.e. diverging from spec) CPU behaviour.
>=20
> The reason for that is the even on real hardware INIT does not touch
> most MSRs:
>=20
>  9.1 Initialization overview
>=20
>  Asserting the INIT# pin on the processor invokes a similar response =
to
>  a hardware reset. The major difference is that during an INIT, the
>  internal caches, MSRs, MTRRs, and x87 FPU state are left unchanged
>  (although, the TLBs and BTB are invalidated as with a hardware =
reset).
>  An INIT provides a method for switching from protected to =
real-address
>  mode while maintaining the contents of the internal caches.
>=20
> Paolo
>=20
> [1]
> =
https://urldefense.proofpoint.com/v2/url?u=3Dhttps-3A__software.intel.com_=
security-2Dsoftware-2Dguidance_api-2Dapp_sites_default_files_336996-2DSpec=
ulative-2DExecution-2DSide-2DChannel-2DMitigations.pdf&d=3DDwICaQ&c=3DRoP1=
YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=3DJk6Q8nNzkQ6LJ6g42qARkg6ryIDGQr=
-yKXPNGZbpTx0&m=3DMRhvCoZcvqLvHMhI3y3iSkpiAfitrNoCgPtWaTXhzNQ&s=3D66DAue_O=
jp-a_msxZ2W9bLtQWJB4W1p9F9GrBBQ8dnw&e=3D=20
>=20

Oh right. That=E2=80=99s true.
There is a difference between power-up and asserting RESET# pin to =
asserting INIT# pin or receiving INIT IPI.
The first does reset all the processor state while the latter reset only =
part of it as indeed Intel SDM section 9.1 describes.

So basically because KVM is only aware of INIT IPI but not on power-up =
and asserting RESET# pin events,
then the job of emulating the latter events is currently implemented in =
userspace VMM.

Having said that, one could argue that because KVM implements the vCPU, =
it should just expose a relevant ioctl
for userspace VMM to trigger =E2=80=9Cfull vCPU state reset=E2=80=9D on =
power-up or asserting RESET# pin events.
i.e. Making userspace VMM only the one which emulates the peripheral =
hardware that triggers the event,
but not implementing the vCPU emulation that responds to this event. I =
think this would have resulted in a cleaner design.

But we are diverting from original patch-series discussion so if we want =
to discuss this further, let=E2=80=99s open a separate email thread on =
that. :)

-Liran