On Mon, Nov 08, 2021, Sean Christopherson wrote:
> On Wed, Aug 11, 2021, Chenyi Qiang wrote:
> > + vcpu->arch.pkrs = vmcs_read64(GUEST_IA32_PKRS);
>
> Hrm. I agree that it's extremely unlikely that IA32_PKRS will ever allow software
> to set bits 63:32, but at the same time there's no real advantage to KVM it as a u32,
> e.g. the extra 4 bytes per vCPU is a non-issue, and could be avoided by shuffling
> kvm_vcpu_arch to get a more efficient layout. On the flip side, using a 32 means
> code like this _looks_ buggy because it's silently dropping bits 63:32, and if the
> architecture ever does get updated, we'll have to modify a bunch of KVM code.
>
> TL;DR: I vote to track PRKS as a u64 even though the kernel tracks it as a u32.
Rats, I forgot that the MMU code for PKRU is going to be reused for PKRS. I withdraw
my vote :-)
Maybe have this code WARN on bits 63:32 being set in the VMCS field? E.g. to
detect if hardware ever changes and KVM fails to update this path, and to document
that KVM intentionally drops those bits.
case VCPU_EXREG_PKRS: {
u64 ia32_pkrs;
ia32_pkrs = vmcs_read64(GUEST_IA32_PKRS);
WARN_ON_ONCE(ia32_pkrs >> 32);
vcpu->arch.pkrs = (u32)ia32_pkrs;
}
