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=us-ascii
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: <15b57d6b-0f46-01f5-1f75-b9b55db0611a@redhat.com>
Date:   Mon, 23 Dec 2019 19:28:13 +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: <03F5FE31-E769-4497-922B-C8613F0951FA@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>
To:     Paolo Bonzini <pbonzini@redhat.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On 23 Dec 2019, at 19:09, Paolo Bonzini <pbonzini@redhat.com> wrote:
>=20
> On 23/12/19 15:48, Liran Alon wrote:
>>> Should userspace expose the CR pining CPUID feature bit, it must =
zero CR
>>> pinned MSRs on reboot. If it does not, it runs the risk of having =
the
>>> guest enable pinning and subsequently cause general protection =
faults on
>>> next boot due to early boot code setting control registers to values
>>> which do not contain the pinned bits.
>>=20
>> Why reset CR pinned MSRs by userspace instead of KVM INIT handling?
>=20
> 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).

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().

So given current design, OK I agree with you that CR pinned MSRs should =
be zeroed by userspace VMM.

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.

-Liran=