On Tue, Oct 8, 2019 at 11:32 PM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>
> On 09/10/19 02:41, Aaron Lewis wrote:
> > * Set value of MSR for VCPU.
> > */
> > -void vcpu_set_msr(struct kvm_vm *vm, uint32_t vcpuid, uint64_t msr_index,
> > - uint64_t msr_value)
> > +void vcpu_set_msr_expect_result(struct kvm_vm *vm, uint32_t vcpuid,
> > + uint64_t msr_index, uint64_t msr_value,
> > + int result)
> > {
> > struct vcpu *vcpu = vcpu_find(vm, vcpuid);
> > struct {
> > @@ -899,10 +901,30 @@ void vcpu_set_msr(struct kvm_vm *vm, uint32_t vcpuid, uint64_t msr_index,
> > buffer.entry.index = msr_index;
> > buffer.entry.data = msr_value;
> > r = ioctl(vcpu->fd, KVM_SET_MSRS, &buffer.header);
> > - TEST_ASSERT(r == 1, "KVM_SET_MSRS IOCTL failed,\n"
> > + TEST_ASSERT(r == result, "KVM_SET_MSRS IOCTL failed,\n"
> > " rc: %i errno: %i", r, errno);
> > }
>
> This is a library, so the functions to some extent should make sense
> even outside tests. Please make a function _vcpu_set_msr that returns
> the result of the ioctl; it can still be used in vcpu_set_msr, and the
> tests can TEST_ASSERT what they want.
>
> > +uint32_t kvm_get_cpuid_max_basic(void)
> > +{
> > + return kvm_get_supported_cpuid_entry(0)->eax;
> > +}
> > +
> > +uint32_t kvm_get_cpuid_max_extended(void)
>
> I would leave the existing function aside, and call this one
> kvm_get_cpuid_max_amd() since CPUID leaves at 0x80000000 are allocated
> by AMD.
The existing function *is* the one that gives the largest
AMD-allocated leaf. Note that Intel documents CPUID.80000000:EAX as
"Maximum Input Value for Extended Function CPUID Information," and AMD
documents this as "Largest extended function."
> Otherwise looks good.
>
> Paolo
>
