Re: [PATCH v5] enable x2APIC without interrupt remapping under KVM

[Avi Kivity <avi@redhat.com>]

On 07/05/2009 03:22 AM, Eric W. Biederman wrote:
> Gleb Natapov<gleb@redhat.com>  writes:
>
>    
>>> Therefore I don't see the point of supporting one without the other.
>>>        
>> x2apic provide us with other benefits as commit message explains, and
>> doesn't add any problems that we don't have now already.
>>      
>
> If this code has a legitimate place on real hardware I am all for it.
>    

As I understood it, x2apic without interrupt remapping will work but is 
not a validated configuration.  Interrupt remapping is only necessary if 
you have > 255 hardware threads + ioapics.  The features are logically 
separate and are only tied together by the vendor's validation practices.


> If this is just a hack to make virtualization faster I don't like the
> extra code paths in the middle core architecture code.  That will
> be a support burden for the foreseeable future.  More code to
> test etc.
>    

There aren't any extra code paths.  The patch separates a long function 
into two smaller ones that each do one thing, and adds a check for kvm.

Maybe it should be split into two to makes that clear.  The first patch 
simplifies the code, the second adds a kvm check.

> Quickly skimming the patch it just appears to stir a mess.
> Plus it adds weird paravirtualization checks, ???
>    

It adds exactly one "weird paravirtualization check ???", then one 
described in the patch description.

> If we are going to have a special code path for virtual hardware
> can we do it right and have something nice to use that makes life
> simpler?

You mean, instead of adding one check in an initialization code path, 
create a new irqchip, a way of describing the topology to the guest, 
support code in kvm (as host)?

> For what we want to do with ioapics they suck and are
> really not suitable.  The only thing that recommends them is that
> they are standard.  But you are deviating from the standard so
> what is the point.
>    

All of the code continues to work.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.