Re: [PATCH v2 5/5] s390x: topology: implementating Store Topology System Information

[Pierre Morel <pmorel@linux.ibm.com>]

On 9/7/21 10:00 AM, Thomas Huth wrote:
> On 22/07/2021 19.42, Pierre Morel wrote:
>> The handling of STSI is enhenced with the interception of the
> 
> s/enhenced/enhanced/

yes, thanks

> 
>> function code 15 for storing CPU topology.

...

>> +static void insert_stsi_15_1_x(S390CPU *cpu, int sel2, __u64 addr, 
>> uint8_t ar)
>> +{
>> +    const MachineState *machine = MACHINE(qdev_get_machine());
>> +    void *p;
>> +    int ret, cc;
>> +
>> +    /*
>> +     * Until the SCLP STSI Facility reporting the MNEST value is used,
>> +     * a sel2 value of 2 is the only value allowed in STSI 15.1.x.
>> +     */
>> +    if (sel2 != 2) {
>> +        setcc(cpu, 3);
>> +        return;
>> +    }
>> +
>> +    p = g_malloc0(4096);
> 
> Use TARGET_PAGE_SIZE instead of magic value 4096?
yes
> 
>> +   insert_stsi_15_1_2(machine, p);
> 
> Wrong indentation (3 instead of 4 spaces).

oh, yes, thanks

> 
>> +    if (s390_is_pv()) {
>> +        ret = s390_cpu_pv_mem_write(cpu, 0, p, 4096);
>> +    } else {
>> +        ret = s390_cpu_virt_mem_write(cpu, addr, ar, p, 4096);
> 
> TARGET_PAGE_SIZE?

yes, of course.

> 
>> +    }
>> +    cc = ret ? 3 : 0;
>> +    setcc(cpu, cc);
>> +    g_free(p);
>> +}
>> +
>>   static int handle_stsi(S390CPU *cpu)
>>   {
>>       CPUState *cs = CPU(cpu);
>> @@ -1920,6 +2017,10 @@ static int handle_stsi(S390CPU *cpu)
>>           /* Only sysib 3.2.2 needs post-handling for now. */
>>           insert_stsi_3_2_2(cpu, run->s390_stsi.addr, run->s390_stsi.ar);
>>           return 0;
>> +    case 15:
>> +        insert_stsi_15_1_x(cpu, run->s390_stsi.sel2, 
>> run->s390_stsi.addr,
>> +                           run->s390_stsi.ar);
>> +        return 0;
>>       default:
>>           return 0;
>>       }
>>
> 
>   Thomas
> 

Thanks for reviewing,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen