linux-kernel - Re: [PATCH] kvm: remove in_range from kvm_io

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: <4A439B90.1030109@novell.com>
Date:	Thu, 25 Jun 2009 11:45:20 -0400
From:	Gregory Haskins <ghaskins@...ell.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
CC:	avi@...hat.com, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
	mtosatti@...hat.com, paulmck@...ux.vnet.ibm.com, markmc@...hat.com
Subject: Re: [PATCH] kvm: remove in_range from kvm_io_device

Michael S. Tsirkin wrote:
> On Wed, Jun 24, 2009 at 11:49:01AM +0300, Michael S. Tsirkin wrote:
>   
>> On Tue, Jun 23, 2009 at 09:43:31PM -0400, Gregory Haskins wrote:
>>     
>>> Michael S. Tsirkin wrote:
>>>       
>>>> Remove in_range from kvm_io_device and ask read/write callbacks, if
>>>> supplied, to perform range checks internally. This allows aliasing
>>>> (mostly for in-kernel virtio), as well as better error handling by
>>>> making it possible to pass errors up to userspace. And it's enough to
>>>> look at the diffstat to see that it's a better API anyway.
>>>>
>>>> While we are at it, document locking rules for kvm_io_device.
>>>>   
>>>>         
>>> Hi Michael,
>>>
>>> I just tried to apply this to kvm.git/master, and it blew up really
>>> badly.  What tree should I be using?
>>>       
>> Ugh, this is against 2.6.30. I'll post kvm.git version soon.
>>
>>     
>
> I went ahead and tried to rebase it, to find that it conflicts with
> recent patch 35b3038961f94e83557944ae0d30c8fa0b5012cf merged in kvm.git:
> 'KVM: switch irq injection/acking data structures to irq_lock'
> which now does this:
> 	lock
> 	find
> 	unlock
> 	write
>
> I thought for a while that it might make sense to start small and just
> add in_range parameter for starters ...
> However, I just realised that this only works because devices are not
> added or removed dynamically.
>
> The long term fix is to switch to SRCU for bus management.  But if we
> need to do this for iosignalfd anyway, in_range removal becomes possible
> again.
>
> Short term it might be also possible to go back to keeping kvm lock
> across both find and read - since the lock is taken, we don't
> really win anything currently if we drop the lock earlier.
>
> Comments?
>   

I just took another look.  Things are quite messy and I don't know what
the optimal order of merging is.  Go ahead with your remove-inrange and
srcu plan and I will just defer submitting the iosignalfd related series
till you are happy with the new layout.

Thanks Michael,
-Greg



Download attachment "signature.asc" of type "application/pgp-signature" (267 bytes)