Re: [Qemu-devel] [PATCH 6/7] target/i386: add VMX features

[Eduardo Habkost]

On Sat, Jul 06, 2019 at 12:42:22AM +0200, Paolo Bonzini wrote:
> On 06/07/19 00:33, Eduardo Habkost wrote:
> > Oh, that's the info I was missing.  I always expected
> > kvm_arch_get_supported_*() to be subject to change (depending on
> > KVM and hardware capabilities), and not be part of guest ABI.
> 
> For most bits that's true.  Just not for these ones, because they are
> integer values rather than bit flags.
> 
> The reason for the complex rules is that you need to know what is a
> flag, what is a fixed value that the guest uses, and what is a maximum
> supported value.  Simpler userspace than QEMU can just use the defaults
> since they don't care about maintaining the guest ABI.
> 
> > Now, if KVM is going to to implement the guest ABI guarantee at
> > KVM_GET_MSRS, that's OK.  Is this going to be obvious to people
> > touching KVM_GET_MSRS in the future?
> > 
> > What if we do want the guest ABI to change in the future?  How do
> > you expect QEMU to ask KVM to enable the new guest ABI?  How do
> > you expect the user to ask QEMU to enable the new guest ABI?
> 
> That would be with ioctl(KVM_ENABLE_CAP) for KVM, and with -cpu for QEMU.

Makes sense to me.

> 
> >> - KVM could change bits 16-24, but it always allows writing a value that
> >> is _smaller_ than the one you read.  So I'm zeroing those, ensuring no
> >> future ABI changes.
> >>
> >> - KVM could in theory change bits 25-27: here it also allows writing a
> >> value that is smaller than the one you read, so guest ABI is preserved.
> >>  Such a change is very unlikely, all Intel silicon has always had 0
> >> here.  But I can change the code to zero these three bits just like bits
> >> 16-24.
> > 
> > The complex rules above make me a bit nervous.  Can we at least
> > make QEMU validate the values returned by
> > kvm_arch_get_supported_msr_feature() to catch ABI-breaking
> > mistakes in the future?
> 
> I don't know... I'm a bit wary of adding hard-coded values in QEMU,
> userspace simply should not care.  But I can add comments to KVM to
> remind people of values that should not be changed.

Sounds good to me.  If we're worried about breaking guest ABI by
accident, we can include the MSRs in the guest ABI validation
test cases I'm working on.

-- 
Eduardo