From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33918) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VT8MF-0007Ge-Ax for qemu-devel@nongnu.org; Mon, 07 Oct 2013 06:48:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VT8M6-0008E9-24 for qemu-devel@nongnu.org; Mon, 07 Oct 2013 06:48:11 -0400 Received: from e06smtp12.uk.ibm.com ([195.75.94.108]:40371) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VT8M5-0008Dd-PL for qemu-devel@nongnu.org; Mon, 07 Oct 2013 06:48:01 -0400 Received: from /spool/local by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 7 Oct 2013 11:47:59 +0100 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id B44F617D8069 for ; Mon, 7 Oct 2013 11:48:15 +0100 (BST) Received: from d06av06.portsmouth.uk.ibm.com (d06av06.portsmouth.uk.ibm.com [9.149.37.217]) by b06cxnps3075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r97AlgJ865536100 for ; Mon, 7 Oct 2013 10:47:43 GMT Received: from d06av06.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av06.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r97AltEV019847 for ; Mon, 7 Oct 2013 04:47:55 -0600 Date: Mon, 7 Oct 2013 12:47:53 +0200 From: Michael Mueller Message-ID: <20131007124753.76c5ec84@bee> In-Reply-To: <524D84CE.1000601@twiddle.net> References: <1380713622-22325-1-git-send-email-mimu@linux.vnet.ibm.com> <1380713622-22325-5-git-send-email-mimu@linux.vnet.ibm.com> <524D84CE.1000601@twiddle.net> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC 04/11] s390/qemu: cpu model cpu facilitiy support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: qemu-devel@nongnu.org On Thu, 03 Oct 2013 07:53:02 -0700 Richard Henderson wrote: > On 10/02/2013 04:33 AM, Michael Mueller wrote: > > +/* set a specific bit in facility set */ > > +static void set_facility(unsigned int nr, void *facilities) > > +{ > > + unsigned char *ptr; > > + > > + if (nr >= MAX_S390_FACILITY_BIT) { > > + return; > > + } > > + ptr = (unsigned char *) facilities + (nr >> 3); > > + *ptr |= (0x80 >> (nr & 7)); > > +} > > I'd like to see this done in a host endian independent way. valid point, I will address that. > > See my recent patch set to add facility support to the tcg side > of target-s390, with which this patch set is going to conflict. I saw it, that's why I've pushed this patch set out for RFC. > > Is there a good reason not to compute these facility masks at > compile-time? See > > http://patchwork.ozlabs.org/patch/279534/ > > where I have pre-computed (possibly incomplete) facilities lists > for the major cpu revisions. Your facilities lists have been derived from constants introduced in head.S. They represent model specific "required" facilities. That does not necessarily mean for all of them that they have been introduced with the respective model. Some have been introduced already with model: N-1, GA>1 > > It just seems like your facility_availability array is the wrong > way to go about things, taking up more memory and startup time > than necessary. > The reason why I represent them in the data segment is that they are are not considered as constants in the s390 system perspective. I plan to be able to simulate firmware migration that introduce new facility bits without the need of restarting the guest OS. A second reason for using 2k of memory here is to fully represent the facilities as defined in the s390x architecture. The SIE state needs it and I want to represent it identically in user space and KVM. Otherwise I would need a specific interface just for the facilities. I will consider to alternatively use your way of FAC definition, but still that would include a copy. In regard to the startup time, I will figure out what the overhead is. Thanks a lot! Michael > > r~ >