From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36701) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SekqL-0005FE-Ju for qemu-devel@nongnu.org; Wed, 13 Jun 2012 06:30:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SekqC-0002Lq-34 for qemu-devel@nongnu.org; Wed, 13 Jun 2012 06:30:29 -0400 Received: from thoth.sbs.de ([192.35.17.2]:24261) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SekqB-0002L4-PM for qemu-devel@nongnu.org; Wed, 13 Jun 2012 06:30:20 -0400 Message-ID: <4FD86BB1.9060900@siemens.com> Date: Wed, 13 Jun 2012 12:30:09 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1338984323-21914-1-git-send-email-jfrei@de.ibm.com> <1338984323-21914-3-git-send-email-jfrei@de.ibm.com> <4FD70CC0.7000901@suse.de> <4FD725EB.7050501@de.ibm.com> <4FD72EC2.2010105@suse.de> <4FD72FD4.6020008@de.ibm.com> <4FD73220.5000408@suse.de> In-Reply-To: <4FD73220.5000408@suse.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/8] s390: autodetect map private List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Jens Freimann , Heinz Graalfs , qemu-devel , Christian Borntraeger , Jens Freimann , Cornelia Huck On 2012-06-12 14:12, Alexander Graf wrote: > On 06/12/2012 02:02 PM, Christian Borntraeger wrote: >> On 12/06/12 13:57, Alexander Graf wrote: >>> Since it lives in an s390 specific branch, the function name should probably be called s390 specific. If we ever need another architecture to have a kvm specific ram allocator, we can make it generic when that time comes. Until then, let's treat s390 as the oddball it is :). >>> >>> Apart from that, this approach looks a lot nicer, yes. >> But then I have to have a *s390* function declared in kvm.h and your other comment >> hits me. You got me in a trap here, heh? ;-) > > Ah, I see what you mean. I was thinking of having a > target-s390x/kvm_s390x.h or so. Then we could add the function > definition there and have everything nicely contained within > target-s390x only. > > Jan, which approach would you think is cleaner? Make this a generic > kvm_arch callback or introduce a special kvm_s390x.h header which would > then have to be explicitly included in exec.c? Maybe somethings like #ifdef __s390__ else if (kvm_enabled()) new_block->host = kvm_arch_vmalloc(size) #endif ? But I have no definitive opinion yet. I think that - the changes to generic code should make clear that it's an s390+kvm specialty - actual work should be done in target-s390/kvm.c (e.g. avoid legacy_s390_alloc) Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux