Messages in this thread | | | Date | Sun, 05 Jul 2009 08:27:22 +0300 | From | Avi Kivity <> | Subject | Re: [PATCH v5] enable x2APIC without interrupt remapping under KVM |
| |
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.
| |