[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] s390: sclp base support
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] s390: sclp base support |
Date: |
Wed, 26 Sep 2012 21:01:07 +0200 |
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?
Alex