On Thu, Feb 04, 2021, Edgecombe, Rick P wrote: > On Thu, 2021-02-04 at 11:34 +0100, Paolo Bonzini wrote: > > On 04/02/21 03:19, Sean Christopherson wrote: > > > Ah, took me a few minutes, but I see what you're saying. LAM will > > > introduce > > > bits that are repurposed for CR3, but not generic GPAs. And, the > > > behavior is > > > based on CPU support, so it'd make sense to have a mask cached in > > > vcpu->arch > > > as opposed to constantly generating it on the fly. > > > > > > Definitely agree that having a separate cr3_lm_rsvd_bits or > > > whatever is the > > > right way to go when LAM comes along. Not sure it's worth keeping > > > a duplicate > > > field in the meantime, though it would avoid a small amount of > > > thrash. > > > > We don't even know if the cr3_lm_rsvd_bits would be a field in > > vcpu->arch, or rather computed on the fly. So renaming the field in > > vcpu->arch seems like the simplest thing to do now. > > Fair enough. But just to clarify, I meant that I thought the code would > be more confusing to use illegal gpa bit checks for checking cr3. It > seems they are only incidentally the same value. Hmm, yeah, bits 63:52 are incidental. Bits 52:M are not, though. If/when we need to special case CR3, I would like to take a similar approach to __reset_rsvds_bits_mask(), where the high reserved bits start from reserved_gpa_bits and mask off the bits that can't be encoded into a PxE. > Alternatively there could be something like a is_rsvd_cr3_bits() helper that > just uses reserved_gpa_bits for now. Probably put the comment in the wrong > place. It's a minor point in any case. That thought crossed my mind, too. Maybe kvm_vcpu_is_illegal_cr3() to match the gpa helpers?
