[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine
From: |
David Hildenbrand |
Subject: |
Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state |
Date: |
Thu, 31 Aug 2017 15:11:28 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
>> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>> +{
>> + S390CcwMachineState *ms = S390_CCW_MACHINE(qdev_get_machine());
>> +
>> + if (cpu_addr >= max_cpus) {
>> + return NULL;
>> + }
>> +
>> + /* Fast lookup via CPU ID */
>> + return ms->cpus[cpu_addr];
>> +}
>
> I wonder whether that function should rather go into a file in
> target/s390x/ instead, since it is also used there and its prototype is
> in cpu.h ?
I thought about the same thing, but as it works directly on the machine,
like ri_allowed() and friends. So I decided to keep it here for now.
I'll think about moving the definition also into
include/hw/s390x/s390-virtio-ccw.h
>
> [...]
>> diff --git a/include/hw/s390x/s390-virtio-ccw.h
>> b/include/hw/s390x/s390-virtio-ccw.h
>> index 41a9d2862b..4bef28ec39 100644
>> --- a/include/hw/s390x/s390-virtio-ccw.h
>> +++ b/include/hw/s390x/s390-virtio-ccw.h
>> @@ -21,11 +21,14 @@
>> #define S390_MACHINE_CLASS(klass) \
>> OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)
>>
>> +struct S390CPU;
>
> You define a "struct S390CPU" here ...
>
>> typedef struct S390CcwMachineState {
>> /*< private >*/
>> MachineState parent_obj;
>>
>> /*< public >*/
>> + S390CPU **cpus;
I'll simply convert this to struct S390CPU, so this header stays
independent from cpu headers.
>
> ... but use the typedef'ed S390CPU here ... looks somewhat suspicious, I
> wonder whether the typedef is really in the right place?
>
>> bool aes_key_wrap;
>> bool dea_key_wrap;
>> uint8_t loadparm[8];
>
> Anyway, that were just nits, I'm also fine with the patch as it is, so:
>
> Reviewed-by: Thomas Huth <address@hidden>
>
Thanks!
--
Thanks,
David
- [Qemu-devel] [PATCH v1 00/11] next round of s390x cleanups, David Hildenbrand, 2017/08/30
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, David Hildenbrand, 2017/08/31
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, Thomas Huth, 2017/08/31
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, David Hildenbrand, 2017/08/31
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, Thomas Huth, 2017/08/31
[Qemu-devel] [PATCH v1 04/11] s390x: get rid of s390-virtio.c, David Hildenbrand, 2017/08/30