Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755153AbbBTS7j (ORCPT ); Fri, 20 Feb 2015 13:59:39 -0500 Received: from e06smtp11.uk.ibm.com ([195.75.94.107]:53192 "EHLO e06smtp11.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752384AbbBTS7h (ORCPT ); Fri, 20 Feb 2015 13:59:37 -0500 Date: Fri, 20 Feb 2015 19:59:28 +0100 From: Michael Mueller To: Richard Henderson Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, Gleb Natapov , Alexander Graf , Christian Borntraeger , "Jason J. Herne" , Cornelia Huck , Paolo Bonzini , Andreas Faerber Subject: Re: [Qemu-devel] [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines Message-ID: <20150220195928.6388e017@bee> In-Reply-To: <54E778EB.5050301@twiddle.net> References: <1424183053-4310-1-git-send-email-mimu@linux.vnet.ibm.com> <1424183053-4310-11-git-send-email-mimu@linux.vnet.ibm.com> <54E778EB.5050301@twiddle.net> Organization: IBM X-Mailer: Claws Mail 3.9.3 (GTK+ 2.24.23; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="MP_/T2oA.eLpgZMKZBi.RtF7L8k" X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15022018-0041-0000-0000-00000373CE1C Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4123 Lines: 106 --MP_/T2oA.eLpgZMKZBi.RtF7L8k Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Content-Disposition: inline On Fri, 20 Feb 2015 10:11:55 -0800 Richard Henderson wrote: > > +static inline uint64_t big_endian_bit(unsigned long nr) > > +{ > > + return 1ul << (BITS_PER_LONG - (nr % BITS_PER_LONG)); > > +}; > > This is buggy. NR=0 should map to 63, not 64. I'm sure I was asked to replace my constant 64 and 63 with that defines and at the end I messed it up... :-( > > > + return !!(*ptr & big_endian_bit(nr)); > > Personally I dislike !! as an idiom. Given that big_endian_bit isn't used > anywhere else, can we integrate it and change this to > > static inline int test_facility(unsigned long nr, uint64_t *fac_list) > { > unsigned long word = nr / BITS_PER_LONG; > unsigned long be_bit = 63 - (nr % BITS_PER_LONG); > return (fac_list[word] >> be_bit) & 1; > } Yes, I just use it in this context. I will integrate your version. BTW I changed the whole facility defining code to be generated by an external helper at compile time. That is more simple and safe to change. I will send it with v3. See attachment for an example of the generated header file. Thanks, Michael --MP_/T2oA.eLpgZMKZBi.RtF7L8k Content-Type: text/x-chdr Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=gen-facilities.h /* * AUTOMATICALLY GENERATED, DO NOT MODIFY HERE, EDIT * SOURCE FILE "target-s390x/tools/gen-facilities.c" INSTEAD. * * Copyright 2014, 2015 IBM Corp. * * This work is licensed under the terms of the GNU GPL, version 2 or (at * your option) any later version. See the COPYING file in the top-level * directory. */ #ifndef TARGET_S390X_GEN_FACILITIES_H #define TARGET_S390X_GEN_FACILITIES_H /* S390 CPU facility defines per CPU model */ #define FAC_LIST_CPU_S390_2064_GA1 0xe000000000000000 #define FAC_LIST_CPU_S390_2064_GA2 0xe000800000000000 #define FAC_LIST_CPU_S390_2064_GA3 0xe000800000000000 #define FAC_LIST_CPU_S390_2066_GA1 0xe000800000000000 #define FAC_LIST_CPU_S390_2084_GA1 0xf000f80000000000 #define FAC_LIST_CPU_S390_2084_GA2 0xf800f80000000000 #define FAC_LIST_CPU_S390_2084_GA3 0xfa00fa0000000000 #define FAC_LIST_CPU_S390_2086_GA1 0xfa00fa0000000000 #define FAC_LIST_CPU_S390_2084_GA4 0xfa00fa0000000000 #define FAC_LIST_CPU_S390_2086_GA2 0xfa00fa0000000000 #define FAC_LIST_CPU_S390_2084_GA5 0xfa00fa0800000000 #define FAC_LIST_CPU_S390_2086_GA3 0xfa00fa0800000000 #define FAC_LIST_CPU_S390_2094_GA1 0xfb00ffcb00000000 #define FAC_LIST_CPU_S390_2094_GA2 0xfb40ffdb80600000 #define FAC_LIST_CPU_S390_2094_GA3 0xfb40ffdb80680000 #define FAC_LIST_CPU_S390_2096_GA1 0xfb40ffdb80680000 #define FAC_LIST_CPU_S390_2096_GA2 0xfb40ffdb80680000 #define FAC_LIST_CPU_S390_2097_GA1 0xfbf0fffbf0780000 #define FAC_LIST_CPU_S390_2097_GA2 0xfbf0fffbf0780000,0x5800000000000000 #define FAC_LIST_CPU_S390_2098_GA1 0xfbf0fffbf0780000,0x5800000000000000 #define FAC_LIST_CPU_S390_2097_GA3 0xfbf0fffbf0f80000,0x5800000000000000 #define FAC_LIST_CPU_S390_2098_GA2 0xfbf0fffbf0f80000,0x5800000000000000 #define FAC_LIST_CPU_S390_2817_GA1 0xfbf0fffbfcfe0000,0x1810000000000000 #define FAC_LIST_CPU_S390_2817_GA2 0xfbf6fffbfcff0000,0x381c000000000000 #define FAC_LIST_CPU_S390_2818_GA1 0xfbf6fffbfcff0000,0x381c000000000000 #define FAC_LIST_CPU_S390_2827_GA1 0xfbf6fffbfcfff800,0x3dde000000000000 #define FAC_LIST_CPU_S390_2827_GA2 0xfbf6fffbfcfff800,0x3dde000000000000 #define FAC_LIST_CPU_S390_2828_GA1 0xfbf6fffbfcfff800,0x3dde000000000000 #define FAC_LIST_CPU_S390_2964_GA1 0xfbf6fffbfcfff800,0x3dde000000000000,0x4000000000000000 /* QEMU facility mask defines */ #define FAC_LIST_CPU_S390_MASK_QEMU 0x0000000000000000,0x0580000000000000 /* Maximum size of generated facility list defines */ #define FAC_LIST_CPU_S390_SIZE_UINT64 3 #endif --MP_/T2oA.eLpgZMKZBi.RtF7L8k-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/