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. [...] >> + 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 Christian