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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4A5039BA.7030100@redhat.com>
Date:	Sun, 05 Jul 2009 08:27:22 +0300
From:	Avi Kivity <avi@...hat.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
CC:	Gleb Natapov <gleb@...hat.com>, Ingo Molnar <mingo@...e.hu>,
	Suresh Siddha <suresh.b.siddha@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Sheng Yang <sheng@...ux.intel.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>
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@...hat.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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/