Re: [PATCH] always report x2apic as supported feature

[Gleb Natapov <gleb@redhat.com>]

On Thu, Jul 16, 2009 at 02:09:09PM +0800, Sheng Yang wrote:
> On Thursday 16 July 2009 14:00:15 Gleb Natapov wrote:
> > On Thu, Jul 16, 2009 at 09:46:21AM +0800, Sheng Yang wrote:
> > > On Thursday 16 July 2009 07:01:30 Marcelo Tosatti wrote:
> > > > On Sun, Jul 12, 2009 at 04:10:55PM +0300, Gleb Natapov wrote:
> > > > > We emulate x2apic in software, so host support is not required.
> > > > >
> > > > > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > > index 00844eb..c256da7 100644
> > > > > --- a/arch/x86/kvm/x86.c
> > > > > +++ b/arch/x86/kvm/x86.c
> > > > > @@ -1497,6 +1497,9 @@ static void do_cpuid_ent(struct
> > > > > kvm_cpuid_entry2 *entry, u32 function, case 1:
> > > > >  		entry->edx &= kvm_supported_word0_x86_features;
> > > > >  		entry->ecx &= kvm_supported_word4_x86_features;
> > > > > +		/* we support x2apic emulation even if host does not support
> > > > > +		   it since we emulate x2apic in software */
> > > > > +		entry->ecx |= F(X2APIC);
> > > > >  		break;
> > > > >  	/* function 2 entries are STATEFUL. That is, repeated cpuid
> > > > > commands * may return different values. This forces us to get_cpu()
> > > > > before --
> > > > > 			Gleb.
> > > >
> > > > What if you have an older host that does not support emulate x2apic?
> > >
> > > Due to interrupt remapping can't be enabled with KVM now, I think older
> > > host would just ignore this info... (The new one can work without
> > > interrupt remapping with KVM).
> > >
> > > By the way, I saw X2APIC in host supported CPUID feature list(1.ecx),
> > > which I
> >
> > Where have you seen it? If you mean kvm_supported_word4_x86_features
> > then it is not what is supported by the host, but what is supported by
> > KVM. Host unsupported bits are dropped from there before reporting to
> > userspace. That is why this patch what necessary.
> 
> Yes, that's what I mean. x2apic feature needn't judged by host feature, we can 
> always set the bit to support it, no need for a filter. I think put it in the 
> kvm_supported_word4_x86_features is a little misleading means that KVM support 
> it through host feature rather than emulation.
> 
Yeah, I put it there initially since I misunderstood how things work and
thought that it will be reported to userspace (and usercpace had a bug
that prevented me from discovering the problem).

--
			Gleb.