| Message ID | 1380713622-22325-5-git-send-email-mimu@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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. 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. 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. 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. r~
On Thu, 03 Oct 2013 07:53:02 -0700 Richard Henderson <rth@twiddle.net> 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~ >
On Mon, 7 Oct 2013 12:47:53 +0200 Michael Mueller <mimu@linux.vnet.ibm.com> wrote: > On Thu, 03 Oct 2013 07:53:02 -0700 > Richard Henderson <rth@twiddle.net> 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. A measurement on a z12EC shows the overhead time to be between 500-800 ns per model. Currently 28 models are defined that means the whole calculation time takes well below 30 us. > > Thanks a lot! > Michael > > > > r~ > > > >
On 10/07/2013 03:47 AM, Michael Mueller wrote: > 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. If KVM needs the data in the format that you've got it in now, that's fair enough. Otherwise, 128 bits per s390 system seems nicer than 32 bits per facility. r~
On Tue, 15 Oct 2013 15:46:57 -0700 Richard Henderson <rth@twiddle.net> wrote: > On 10/07/2013 03:47 AM, Michael Mueller wrote: > > 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. > > If KVM needs the data in the format that you've got it in now, that's fair enough. > > Otherwise, 128 bits per s390 system seems nicer than 32 bits per facility. > I wrote it down with defines but it is really a mess, I need to double/tripple-check it: #define FAC0_CPU_S390_2064_GA1 \ ( FAC_BIT(0, FAC_N3) \ | FAC_BIT(0, FAC_ZARCH) \ | FAC_BIT(0, FAC_ZARCH_ACTIVE)) #define FAC1_CPU_S390_2064_GA1 0ul #define FAC0_CPU_S390_2064_GA2 \ ( FAC0_CPU_S390_2064_GA1 \ | FAC_BIT(0, FAC_EXTENDED_TRANSLATION_2)) #define FAC1_CPU_S390_2064_GA2 FAC1_CPU_S390_2064_GA1 #define FAC0_CPU_S390_2064_GA3 FAC0_CPU_S390_2064_GA2 #define FAC1_CPU_S390_2064_GA3 FAC1_CPU_S390_2064_GA2 #define FAC0_CPU_S390_2066_GA1 FAC0_CPU_S390_2064_GA3 #define FAC1_CPU_S390_2066_GA1 FAC0_CPU_S390_2064_GA3 #define FAC0_CPU_S390_2084_GA1 \ ( FAC0_CPU_S390_2064_GA3 \ | FAC_BIT(0, FAC_DAT_ENH) \ | FAC_BIT(0, FAC_MESSAGE_SECURITY_ASSIST) \ | FAC_BIT(0, FAC_LONG_DISPLACEMENT) \ | FAC_BIT(0, FAC_LONG_DISPLACEMENT_FAST) \ | FAC_BIT(0, FAC_HFP_MADDSUB)) #define FAC1_CPU_S390_2084_GA1 FAC1_CPU_S390_2064_GA3 #define FAC0_CPU_S390_2084_GA2 \ ( FAC0_CPU_S390_2084_GA1 \ | FAC_BIT(0, 4)) #define FAC1_CPU_S390_2084_GA2 FAC1_CPU_S390_2084_GA1 #define FAC0_CPU_S390_2084_GA3 \ ( FAC0_CPU_S390_2084_GA2 \ | FAC_BIT(0, FAC_ASN_LX_REUSE) \ | FAC_BIT(0, FAC_EXTENDED_TRANSLATION_3)) #define FAC1_CPU_S390_2084_GA3 FAC1_CPU_S390_2084_GA2 #define FAC0_CPU_S390_2084_GA4 FAC0_CPU_S390_2084_GA3 #define FAC1_CPU_S390_2084_GA4 FAC1_CPU_S390_2084_GA3 #define FAC0_CPU_S390_2084_GA5 \ ( FAC0_CPU_S390_2084_GA4 \ | FAC_BIT(0, FAC_TOD_CLOCK_STEERING)) #define FAC1_CPU_S390_2084_GA5 FAC1_CPU_S390_2084_GA4 #define FAC0_CPU_S390_2086_GA1 FAC0_CPU_S390_2084_GA3 #define FAC1_CPU_S390_2086_GA1 FAC1_CPU_S390_2084_GA3 #define FAC0_CPU_S390_2086_GA2 FAC0_CPU_S390_2084_GA4 #define FAC1_CPU_S390_2086_GA2 FAC1_CPU_S390_2084_GA4 #define FAC0_CPU_S390_2086_GA3 FAC0_CPU_S390_2084_GA5 #define FAC1_CPU_S390_2086_GA3 FAC1_CPU_S390_2084_GA5 #define FAC0_CPU_S390_2094_GA1 \ ( FAC0_CPU_S390_2084_GA5 \ | FAC_BIT(0, FAC_STFLE) \ | FAC_BIT(0, FAC_EXTENDED_IMMEDIATE) \ | FAC_BIT(0, FAC_HFP_UNNORMALIZED_EXT) \ | FAC_BIT(0, FAC_ETF2_ENH) \ | FAC_BIT(0, FAC_STORE_CLOCK_FAST) \ | FAC_BIT(0, FAC_ETF3_ENH) \ | FAC_BIT(0, FAC_EXTRACT_CPU_TIME)) #define FAC1_CPU_S390_2094_GA1 FAC1_CPU_S390_2084_GA5 #define FAC0_CPU_S390_2094_GA2 \ ( FAC0_CPU_S390_2094_GA1 \ | FAC_BIT(0, FAC_SENSE_RUNNING_STATUS) \ | FAC_BIT(0, FAC_MOVE_WITH_OPTIONAL_SPEC) \ | FAC_BIT(0, FAC_COMPARE_AND_SWAP_AND_STORE) \ | FAC_BIT(0, FAC_FLOATING_POINT_SUPPPORT_ENH) \ | FAC_BIT(0, FAC_DFP)) #define FAC1_CPU_S390_2094_GA2 FAC1_CPU_S390_2094_GA1 #define FAC0_CPU_S390_2094_GA3 \ ( FAC0_CPU_S390_2094_GA2 \ | FAC_BIT(0, FAC_PFPO)) #define FAC1_CPU_S390_2094_GA3 FAC1_CPU_S390_2094_GA2 #define FAC0_CPU_S390_2096_GA1 FAC0_CPU_S390_2094_GA3 #define FAC1_CPU_S390_2096_GA1 FAC1_CPU_S390_2094_GA3 #define FAC0_CPU_S390_2096_GA2 FAC0_CPU_S390_2096_GA1 #define FAC1_CPU_S390_2096_GA2 FAC1_CPU_S390_2096_GA1 #define FAC0_CPU_S390_2097_GA1 \ ( FAC0_CPU_S390_2094_GA3 \ | FAC_BIT(0, FAC_ENHANCED_DAT_1) \ | FAC_BIT(0, FAC_CONDITIONAL_SSKE) \ | FAC_BIT(0, FAC_CONFIGURATION_TOPOLOGY) \ | FAC_BIT(0, FAC_PARSING_ENH) \ | FAC_BIT(0, FAC_COMPARE_AND_SWAP_AND_STORE_2) \ | FAC_BIT(0, FAC_GENERAL_INSTRUCTIONS_EXT) \ | FAC_BIT(0, FAC_EXECUTE_EXT) \ | FAC_BIT(0, FAC_DFP_FAST)) #define FAC1_CPU_S390_2097_GA1 FAC1_CPU_S390_2094_GA3 #define FAC0_CPU_S390_2097_GA2 FAC0_CPU_S390_2097_GA1 #define FAC1_CPU_S390_2097_GA2 \ ( FAC1_CPU_S390_2097_GA1 \ | FAC_BIT(1, 65) \ | FAC_BIT(1, FAC_CPU_MEASUREMENT_COUNTER) \ | FAC_BIT(1, FAC_CPU_MEASUREMENT_SAMPLING)) #define FAC0_CPU_S390_2097_GA3 \ ( FAC0_CPU_S390_2097_GA2 \ | FAC_BIT(0, FAC_SET_PROGRAM_PARAMETERS)) #define FAC1_CPU_S390_2097_GA3 FAC1_CPU_S390_2097_GA2 #define FAC0_CPU_S390_2098_GA1 FAC0_CPU_S390_2097_GA2 #define FAC1_CPU_S390_2098_GA1 FAC1_CPU_S390_2097_GA2 #define FAC0_CPU_S390_2098_GA2 FAC0_CPU_S390_2097_GA3 #define FAC1_CPU_S390_2098_GA2 FAC1_CPU_S390_2097_GA3 #define FAC0_CPU_S390_2817_GA1 \ ( FAC0_CPU_S390_2097_GA3 \ | FAC_BIT(0, FAC_ENHANCED_MONITOR) \ | FAC_BIT(0, FAC_FLOATING_POINT_EXT) \ | FAC_BIT(0, FAC_MULTI_45) \ | FAC_BIT(0, 46)) #define FAC1_CPU_S390_2817_GA1 \ ((FAC1_CPU_S390_2097_GA3 \ & ~FAC_BIT(1, 65)) \ | FAC_BIT(1, FAC_ACCESS_EXCEPTION_FS_INDICATION)) #define FAC0_CPU_S390_2817_GA2 \ ( FAC0_CPU_S390_2817_GA1 \ | FAC_BIT(0, FAC_IPTE_RANGE) \ | FAC_BIT(0, FAC_NONQ_KEY_SETTING) \ | FAC_BIT(0, FAC_CMPSC_ENH)) #define FAC1_CPU_S390_2817_GA2 \ ( FAC1_CPU_S390_2817_GA1 \ | FAC_BIT(1, FAC_RESET_REFERENCE_BITS_MULTIPLE) \ | FAC_BIT(1, FAC_MESSAGE_SECURITY_ASSIST_3) \ | FAC_BIT(1, FAC_MESSAGE_SECURITY_ASSIST_4)) #define FAC0_CPU_S390_2818_GA1 FAC0_CPU_S390_2817_GA2 #define FAC1_CPU_S390_2818_GA1 FAC1_CPU_S390_2817_GA2 #define FAC0_CPU_S390_2827_GA1 \ ( FAC0_CPU_S390_2817_GA2 \ | FAC_BIT(0, FAC_DFP_ZONED_CONVERSION) \ | FAC_BIT(0, FAC_MULTI_49) \ | FAC_BIT(0, FAC_CONSTRAINT_TRANSACTIONAL_EXE) \ | FAC_BIT(0, FAC_LOCAL_TLB_CLEARING) \ | FAC_BIT(0, FAC_INTERLOCKED_ACCESS_2)) #define FAC1_CPU_S390_2827_GA1 \ ( FAC1_CPU_S390_2817_GA2 \ | FAC_BIT(1, FAC_TRANSACTIONAL_EXE) \ | FAC_BIT(1, FAC_ENHANCED_DAT_2)) #define FAC0_CPU_S390_2827_GA2 FAC0_CPU_S390_2827_GA1 #define FAC1_CPU_S390_2827_GA2 FAC1_CPU_S390_2827_GA1 #define FAC0_CPU_S390_2828_GA1 FAC0_CPU_S390_2827_GA2 #define FAC1_CPU_S390_2828_GA1 FAC1_CPU_S390_2827_GA2
diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c index e2e8019..b813f89 100644 --- a/target-s390x/cpu-models.c +++ b/target-s390x/cpu-models.c @@ -27,7 +27,7 @@ cc->class = cpu_class(_cpu_id); \ cc->ga = cpu_ga(_cpu_id); \ cc->generation = cpu_generation(_cpu_id); \ - cc->facilities = NULL; \ + cc->facilities = facilities(_cpu_id); \ cc->kvm_facilities = NULL; \ cc->is_active = true; \ cc->is_host = false; \ @@ -47,6 +47,130 @@ } \ type_init(glue(_cpu_id, _cpu_register_types)) +static unsigned int facility_availability[MAX_S390_FACILITY_BIT] = { + [0] = CPU_S390_2064_GA1, + [1] = CPU_S390_2064_GA1, + [2] = CPU_S390_2064_GA1, + [3] = CPU_S390_2084_GA1, + [4] = CPU_S390_2084_GA2, + [6] = CPU_S390_2084_GA3, + [7] = CPU_S390_2094_GA1, + [8] = CPU_S390_2097_GA1, + [9] = CPU_S390_2094_GA2, + [10] = CPU_S390_2097_GA1, + [11] = CPU_S390_2097_GA1, + [13] = CPU_S390_2817_GA2, + [14] = CPU_S390_2817_GA2, + [16] = CPU_S390_2064_GA2, /* GA1+ */ + [17] = CPU_S390_2084_GA1, + [18] = CPU_S390_2084_GA1, + [19] = CPU_S390_2084_GA1, + [20] = CPU_S390_2084_GA1, + [21] = CPU_S390_2094_GA1, + [22] = CPU_S390_2084_GA3, + [23] = CPU_S390_2094_GA1, + [24] = CPU_S390_2094_GA1, + [25] = CPU_S390_2094_GA1, + [26] = CPU_S390_2097_GA1, + [27] = CPU_S390_2094_GA2, + [28] = CPU_S390_2084_GA5, + [30] = CPU_S390_2094_GA1, + [31] = CPU_S390_2094_GA1, + [32] = CPU_S390_2094_GA2, + [33] = CPU_S390_2097_GA1, + [34] = CPU_S390_2097_GA1, + [35] = CPU_S390_2097_GA1, + [36] = CPU_S390_2817_GA1, + [37] = CPU_S390_2817_GA1, + [40] = CPU_S390_2097_GA3, + [41] = CPU_S390_2094_GA2, + [42] = CPU_S390_2094_GA2, + [43] = CPU_S390_2097_GA1, + [44] = CPU_S390_2094_GA3, + [45] = CPU_S390_2817_GA1, + [46] = CPU_S390_2817_GA1, + [47] = CPU_S390_2817_GA2, + [48] = CPU_S390_2827_GA1, + [49] = CPU_S390_2827_GA1, + [50] = CPU_S390_2827_GA1, + [51] = CPU_S390_2827_GA1, + [52] = CPU_S390_2827_GA1, + [65] = CPU_S390_2097_GA2, + [66] = CPU_S390_2817_GA2, + [67] = CPU_S390_2097_GA2, + [68] = CPU_S390_2097_GA2, + [73] = CPU_S390_2827_GA1, + [75] = CPU_S390_2817_GA1, + [76] = CPU_S390_2817_GA2, + [77] = CPU_S390_2817_GA2, + [78] = CPU_S390_2827_GA1, +}; + +static unsigned int facility_revocation[MAX_S390_FACILITY_BIT] = { + [65] = CPU_S390_2817_GA1, +}; + +/* 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)); +} + +/* clear a specific bit in facility set */ +static void clear_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)); +} + +/* test a specific bit in facility set to be set */ +static inline int test_facility(unsigned long nr, void *facilities) +{ + unsigned char *ptr; + + if (nr >= MAX_S390_FACILITY_BIT) + return 0; + ptr = (unsigned char *) facilities + (nr >> 3); + return (*ptr & (0x80 >> (nr & 7))) != 0; +} + +/* calculate the facility set for a specific cpu type */ +static unsigned long *facilities(unsigned int cpu_id) +{ + unsigned long *fac; + unsigned int i; + + fac = g_malloc0(MAX_S390_FACILITY_BYTE); + if (fac) { + for (i = 0; i < ARRAY_SIZE(facility_availability); i++) { + if (facility_availability[i]) { + if (cpu_order(facility_availability[i]) <= cpu_order(cpu_id)) { + set_facility(i, fac); + } + } + } + for (i = 0; i < ARRAY_SIZE(facility_revocation); i++) { + if (facility_revocation[i]) { + if (cpu_order(facility_revocation[i]) <= cpu_order(cpu_id)) { + clear_facility(i, fac); + } + } + } + } + return fac; +} + /* define S390 CPU model classes */ S390_PROC_DEF("2064-ga1", CPU_S390_2064_GA1, "IBM zSeries 900 GA1") S390_PROC_DEF("2064-ga2", CPU_S390_2064_GA2, "IBM zSeries 900 GA2")
This patch defines s390 cpu facilities and their appearance/disappearance. The implemented functions allow to calculate cpu model specific facility sets. These sets are assiciated to the defiend cpu classes used to calculate the list of supported cpu models. Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> --- target-s390x/cpu-models.c | 126 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 125 insertions(+), 1 deletion(-)