On 30.08.2017 19:05, David Hildenbrand wrote:
> Let's avoid global variables. While at it, move both functions using it,
> so we won't have to temporarily add includes (we'll be getting rid of
> s390-virtio.c soon).
>
> Signed-off-by: David Hildenbrand <[email protected]>
> ---
> hw/s390x/s390-virtio-ccw.c | 39
> ++++++++++++++++++++++++++++++++++++++
> hw/s390x/s390-virtio.c | 38 -------------------------------------
> hw/s390x/s390-virtio.h | 1 -
> include/hw/s390x/s390-virtio-ccw.h | 3 +++
> 4 files changed, 42 insertions(+), 39 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index dd504dd5ae..ffd56af834 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -32,6 +32,45 @@
> #include "migration/register.h"
> #include "cpu_models.h"
>
> +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 ?
[...]
> 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;
... 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 <[email protected]>
