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
