Re: [PART1 RFC v4 09/11] svm: Do not expose x2APIC when enable AVIC

From	Paolo Bonzini <pbonzini@redhat.com>
Newsgroups	linux.kernel
Subject	Re: [PART1 RFC v4 09/11] svm: Do not expose x2APIC when enable AVIC
Date	2016-04-13 00:10 +0200
Message-ID	<rneFs-6vF-9@gated-at.bofh.it> (permalink)
References	<rldu9-10J-3@gated-at.bofh.it> <rldua-10J-7@gated-at.bofh.it> <rmR6a-3xa-3@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw


On 11/04/2016 22:54, Radim Krčmář wrote:
>> >  
>> >  static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
>> >  {
>> >  	switch (func) {
>> > +	case 0x00000001:
> ("case 1:" or "case 0x1:" would be easier to read.)
> 
>> > +		if (avic)
>> > +			entry->ecx &= ~bit(X86_FEATURE_X2APIC);
>> > +		break;
> 
> ---
> A rant for the unlikely case I get back to fix the broader situation:
> Only one of these two additions is needed.  If we do the second one,
> then userspace should not set X2APIC, therefore the first one is
> useless.
> 
> Omitting the second one allows userspace to clear apicv_active and set
> X86_FEATURE_X2APIC, but it needs a non-intuitive order of ioctls, so I
> think we should have the second one.
> 
> The problem is that KVM doesn't seems to check whether userspace sets
> cpuid that is a subset of supported ones, so omitting the first one
> needlessly expands the space for potential failures.

Yes, we need both.

Paolo

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PART1 RFC v4 09/11] svm: Do not expose x2APIC when enable AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-04-07 10:30 +0200
  Re: [PART1 RFC v4 09/11] svm: Do not expose x2APIC when enable AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-04-11 23:00 +0200
    Re: [PART1 RFC v4 09/11] svm: Do not expose x2APIC when enable AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-04-13 00:10 +0200

csiph-web