From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35497) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGxEm-0008Bf-KR for qemu-devel@nongnu.org; Wed, 26 Sep 2012 15:25:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TGxEj-0002Aw-Lp for qemu-devel@nongnu.org; Wed, 26 Sep 2012 15:25:36 -0400 Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:45492) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGxEj-00026j-DA for qemu-devel@nongnu.org; Wed, 26 Sep 2012 15:25:33 -0400 Received: from /spool/local by e06smtp17.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 26 Sep 2012 20:25:29 +0100 Received: from d06av07.portsmouth.uk.ibm.com (d06av07.portsmouth.uk.ibm.com [9.149.37.248]) by b06cxnps3075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q8QJPJke53674022 for ; Wed, 26 Sep 2012 19:25:19 GMT Received: from d06av07.portsmouth.uk.ibm.com (d06av07.portsmouth.uk.ibm.com [127.0.0.1]) by d06av07.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q8QIppu9013734 for ; Wed, 26 Sep 2012 14:51:51 -0400 Message-ID: <506356A5.7020301@de.ibm.com> Date: Wed, 26 Sep 2012 21:25:25 +0200 From: Christian Borntraeger MIME-Version: 1.0 References: <1345472926-21528-1-git-send-email-jfrei@linux.vnet.ibm.com> <1345472926-21528-2-git-send-email-jfrei@linux.vnet.ibm.com> <8040B9CD-C9FA-4FE6-B9A6-7B076B9D80CA@suse.de> <50632809.7030607@de.ibm.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/4] s390: sclp base support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Heinz Graalfs , qemu-devel , Einar Lueck , Jens Freimann , Cornelia Huck , Andreas Faerber On 26/09/12 21:01, Alexander Graf wrote: > > On 26.09.2012, at 18:06, Christian Borntraeger wrote: > >> On 26/09/12 17:00, Alexander Graf wrote: >> >>>> +/* Provide information about the configuration, CPUs and storage */ >>>> +static void read_SCP_info(SCCB *sccb) >>>> +{ >>>> + ReadInfo *read_info = (ReadInfo *) sccb; >>>> + int shift = 0; >>>> + >>>> + while ((ram_size >> (20 + shift)) > 65535) { >>>> + shift++; >>>> + } >>>> + read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift)); >>>> + read_info->rnsize = 1 << shift; >>> >>> Any reason we can't just always expose rnmax2 and rnsize2 instead? This way we're quite limited on the amount of RAM we can support, right? >> >> Well, we have 65535 * 256 * 1MB == 16TB which is ok for the next 2 or 3 years I guess. >> There are actually some rules that decide about rnmax vs rnmax2 etc, and here >> the non-2 are appropriate. This might change for systems > 16TB or systems with memory hotplug, >> but I would prefer to use those for now. We will add the full logic in case we add memory >> hotplug. > > Fair enough :). > >> >> >> [...] >> >>>> + if (be16_to_cpu(work_sccb.h.length) < 8 || >>> >>> sizeof(SCCBHeader) >> >> ok >> >> >>> >>>> + be16_to_cpu(work_sccb.h.length) > 4096) { >>> >>> SCCB_SIZE >> >> ok >> >> >>>> */ >>>> -int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code) >>>> +int sclp_service_call(uint32_t sccb, uint64_t code) >>> >>> Why not move the whole thing into sclp.c? The only thing remaining here are a few sanity checks that would just as well work in generic sclp handling code, right? >> >> The idea was two-fold: >> - to have one single place were we cross between target-s390x and hw (review feedback from the first series, originally we had all everything in sclp.c) >> - to have the checks that the CPU can do over there and the complex things that look into the sccb in sclp.c >> >> But we could certainly move that, your take > > I would still see both fulfilled by having the 2 condition checks in sclp.c. Why don't we need them for kvm? Does kvm check them in the kernel? KVM needs the checks as well. Thats why kvm calls into sclp_service_call (like the tcg) and then evaluates the return value (like tcg). sclp_service_call is the only place that calls into hw/* code. If we move that code into sclp we have two places that call from target to hw/: kvm.c and op_helper.c (now misc_helper.c). But it really doesnt matter, so lets just move that code. Christian