| Message ID | 1375366359-11553-5-git-send-email-jjherne@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
Am 01.08.2013 16:12, schrieb Jason J. Herne: > From: "Jason J. Herne" <jjherne@us.ibm.com> > > Introduces global access to storage key data so we can set it for each cpu in > the S390 cpu initialization routine. > > Signed-off-by: Jason J. Herne <jjherne@us.ibm.com> > --- > hw/s390x/s390-virtio-ccw.c | 5 ++--- > hw/s390x/s390-virtio.c | 21 ++++++++++++++++----- > hw/s390x/s390-virtio.h | 2 +- > target-s390x/cpu.h | 4 ++++ > 4 files changed, 23 insertions(+), 9 deletions(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index aebbbf1..b469960 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -65,7 +65,6 @@ static void ccw_init(QEMUMachineInitArgs *args) > MemoryRegion *sysmem = get_system_memory(); > MemoryRegion *ram = g_new(MemoryRegion, 1); > int shift = 0; > - uint8_t *storage_keys; > int ret; > VirtualCssBus *css_bus; > > @@ -94,10 +93,10 @@ static void ccw_init(QEMUMachineInitArgs *args) > memory_region_add_subregion(sysmem, 0, ram); > > /* allocate storage keys */ > - storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE); > + s390_alloc_storage_keys(my_ram_size); > > /* init CPUs */ > - s390_init_cpus(args->cpu_model, storage_keys); > + s390_init_cpus(args->cpu_model); > > if (kvm_enabled()) { > kvm_s390_enable_css_support(s390_cpu_addr2state(0)); > diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c > index 439d732..21e9124 100644 > --- a/hw/s390x/s390-virtio.c > +++ b/hw/s390x/s390-virtio.c > @@ -123,6 +123,18 @@ static void s390_virtio_register_hcalls(void) > s390_virtio_hcall_set_status); > } > > +static uint8_t *storage_keys; > + > +uint8_t *s390_get_storage_keys(void) > +{ > + return storage_keys; > +} > + > +void s390_alloc_storage_keys(ram_addr_t ram_size) > +{ > + storage_keys = g_malloc0(ram_size / TARGET_PAGE_SIZE); > +} > + > /* > * The number of running CPUs. On s390 a shutdown is the state of all CPUs > * being either stopped or disabled (for interrupts) waiting. We have to > @@ -176,7 +188,7 @@ void s390_init_ipl_dev(const char *kernel_filename, > qdev_init_nofail(dev); > } > > -void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) > +void s390_init_cpus(const char *cpu_model) > { > int i; > > @@ -196,7 +208,7 @@ void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) > ipi_states[i] = cpu; > cs->halted = 1; > cpu->env.exception_index = EXCP_HLT; > - cpu->env.storage_keys = storage_keys; > + cpu->env.storage_keys = s390_get_storage_keys(); Why not go this from the CPU initfn? Is there any ccw- vs. non-ccw difference? Thinking about -device s390-cpu here. I believe it's safe to assume that machine init and thus allocation has run before the CPU is instantiated - possibly assert that. Andreas > } > } > > @@ -231,7 +243,6 @@ static void s390_init(QEMUMachineInitArgs *args) > MemoryRegion *sysmem = get_system_memory(); > MemoryRegion *ram = g_new(MemoryRegion, 1); > int shift = 0; > - uint8_t *storage_keys; > void *virtio_region; > hwaddr virtio_region_len; > hwaddr virtio_region_start; > @@ -270,10 +281,10 @@ static void s390_init(QEMUMachineInitArgs *args) > virtio_region_len); > > /* allocate storage keys */ > - storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE); > + s390_alloc_storage_keys(my_ram_size); > > /* init CPUs */ > - s390_init_cpus(args->cpu_model, storage_keys); > + s390_init_cpus(args->cpu_model); > > /* Create VirtIO network adapters */ > s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390"); > diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h > index 5c405e7..c1cb042 100644 > --- a/hw/s390x/s390-virtio.h > +++ b/hw/s390x/s390-virtio.h > @@ -20,7 +20,7 @@ > typedef int (*s390_virtio_fn)(const uint64_t *args); > void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn); > > -void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys); > +void s390_init_cpus(const char *cpu_model); > void s390_init_ipl_dev(const char *kernel_filename, > const char *kernel_cmdline, > const char *initrd_filename, > diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h > index 65bef86..877eac7 100644 > --- a/target-s390x/cpu.h > +++ b/target-s390x/cpu.h > @@ -374,6 +374,10 @@ static inline void kvm_s390_interrupt_internal(S390CPU *cpu, int type, > { > } > #endif > + > +uint8_t *s390_get_storage_keys(void); > +void s390_alloc_storage_keys(ram_addr_t ram_size); > + > S390CPU *s390_cpu_addr2state(uint16_t cpu_addr); > void s390_add_running_cpu(S390CPU *cpu); > unsigned s390_del_running_cpu(S390CPU *cpu); >
On 01.08.2013, at 16:12, Jason J. Herne wrote: > From: "Jason J. Herne" <jjherne@us.ibm.com> > > Introduces global access to storage key data so we can set it for each cpu in > the S390 cpu initialization routine. > > Signed-off-by: Jason J. Herne <jjherne@us.ibm.com> > --- > hw/s390x/s390-virtio-ccw.c | 5 ++--- > hw/s390x/s390-virtio.c | 21 ++++++++++++++++----- > hw/s390x/s390-virtio.h | 2 +- > target-s390x/cpu.h | 4 ++++ > 4 files changed, 23 insertions(+), 9 deletions(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index aebbbf1..b469960 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -65,7 +65,6 @@ static void ccw_init(QEMUMachineInitArgs *args) > MemoryRegion *sysmem = get_system_memory(); > MemoryRegion *ram = g_new(MemoryRegion, 1); > int shift = 0; > - uint8_t *storage_keys; > int ret; > VirtualCssBus *css_bus; > > @@ -94,10 +93,10 @@ static void ccw_init(QEMUMachineInitArgs *args) > memory_region_add_subregion(sysmem, 0, ram); > > /* allocate storage keys */ > - storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE); > + s390_alloc_storage_keys(my_ram_size); > > /* init CPUs */ > - s390_init_cpus(args->cpu_model, storage_keys); > + s390_init_cpus(args->cpu_model); > > if (kvm_enabled()) { > kvm_s390_enable_css_support(s390_cpu_addr2state(0)); > diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c > index 439d732..21e9124 100644 > --- a/hw/s390x/s390-virtio.c > +++ b/hw/s390x/s390-virtio.c > @@ -123,6 +123,18 @@ static void s390_virtio_register_hcalls(void) > s390_virtio_hcall_set_status); > } > > +static uint8_t *storage_keys; > + > +uint8_t *s390_get_storage_keys(void) > +{ > + return storage_keys; This is basically the same as having a global in my point of view. I would personally just make it a global and call it a day. However, it might make sense to make storage keys be a device. That way you get direct access for free (you can ask for the device by-path) and you have somewhere to hook migration off of. Alex > +} > + > +void s390_alloc_storage_keys(ram_addr_t ram_size) > +{ > + storage_keys = g_malloc0(ram_size / TARGET_PAGE_SIZE); > +} > + > /* > * The number of running CPUs. On s390 a shutdown is the state of all CPUs > * being either stopped or disabled (for interrupts) waiting. We have to > @@ -176,7 +188,7 @@ void s390_init_ipl_dev(const char *kernel_filename, > qdev_init_nofail(dev); > } > > -void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) > +void s390_init_cpus(const char *cpu_model) > { > int i; > > @@ -196,7 +208,7 @@ void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) > ipi_states[i] = cpu; > cs->halted = 1; > cpu->env.exception_index = EXCP_HLT; > - cpu->env.storage_keys = storage_keys; > + cpu->env.storage_keys = s390_get_storage_keys(); > } > } > > @@ -231,7 +243,6 @@ static void s390_init(QEMUMachineInitArgs *args) > MemoryRegion *sysmem = get_system_memory(); > MemoryRegion *ram = g_new(MemoryRegion, 1); > int shift = 0; > - uint8_t *storage_keys; > void *virtio_region; > hwaddr virtio_region_len; > hwaddr virtio_region_start; > @@ -270,10 +281,10 @@ static void s390_init(QEMUMachineInitArgs *args) > virtio_region_len); > > /* allocate storage keys */ > - storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE); > + s390_alloc_storage_keys(my_ram_size); > > /* init CPUs */ > - s390_init_cpus(args->cpu_model, storage_keys); > + s390_init_cpus(args->cpu_model); > > /* Create VirtIO network adapters */ > s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390"); > diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h > index 5c405e7..c1cb042 100644 > --- a/hw/s390x/s390-virtio.h > +++ b/hw/s390x/s390-virtio.h > @@ -20,7 +20,7 @@ > typedef int (*s390_virtio_fn)(const uint64_t *args); > void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn); > > -void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys); > +void s390_init_cpus(const char *cpu_model); > void s390_init_ipl_dev(const char *kernel_filename, > const char *kernel_cmdline, > const char *initrd_filename, > diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h > index 65bef86..877eac7 100644 > --- a/target-s390x/cpu.h > +++ b/target-s390x/cpu.h > @@ -374,6 +374,10 @@ static inline void kvm_s390_interrupt_internal(S390CPU *cpu, int type, > { > } > #endif > + > +uint8_t *s390_get_storage_keys(void); > +void s390_alloc_storage_keys(ram_addr_t ram_size); > + > S390CPU *s390_cpu_addr2state(uint16_t cpu_addr); > void s390_add_running_cpu(S390CPU *cpu); > unsigned s390_del_running_cpu(S390CPU *cpu); > -- > 1.7.10.4 >
On 09/05/2013 07:46 AM, Andreas Färber wrote: > Am 01.08.2013 16:12, schrieb Jason J. Herne: >> From: "Jason J. Herne" <jjherne@us.ibm.com> >> >> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c >> index 439d732..21e9124 100644 >> --- a/hw/s390x/s390-virtio.c >> +++ b/hw/s390x/s390-virtio.c >> @@ -123,6 +123,18 @@ static void s390_virtio_register_hcalls(void) >> s390_virtio_hcall_set_status); >> } >> >> +static uint8_t *storage_keys; >> + >> +uint8_t *s390_get_storage_keys(void) >> +{ >> + return storage_keys; >> +} >> + >> +void s390_alloc_storage_keys(ram_addr_t ram_size) >> +{ >> + storage_keys = g_malloc0(ram_size / TARGET_PAGE_SIZE); >> +} >> + >> /* >> * The number of running CPUs. On s390 a shutdown is the state of all CPUs >> * being either stopped or disabled (for interrupts) waiting. We have to >> @@ -176,7 +188,7 @@ void s390_init_ipl_dev(const char *kernel_filename, >> qdev_init_nofail(dev); >> } >> >> -void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) >> +void s390_init_cpus(const char *cpu_model) >> { >> int i; >> >> @@ -196,7 +208,7 @@ void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) >> ipi_states[i] = cpu; >> cs->halted = 1; >> cpu->env.exception_index = EXCP_HLT; >> - cpu->env.storage_keys = storage_keys; >> + cpu->env.storage_keys = s390_get_storage_keys(); > > Why not go this from the CPU initfn? Is there any ccw- vs. non-ccw > difference? Thinking about -device s390-cpu here. I believe it's safe to > assume that machine init and thus allocation has run before the CPU is > instantiated - possibly assert that. > > Patch 6/8 does exactly that.
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index aebbbf1..b469960 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -65,7 +65,6 @@ static void ccw_init(QEMUMachineInitArgs *args) MemoryRegion *sysmem = get_system_memory(); MemoryRegion *ram = g_new(MemoryRegion, 1); int shift = 0; - uint8_t *storage_keys; int ret; VirtualCssBus *css_bus; @@ -94,10 +93,10 @@ static void ccw_init(QEMUMachineInitArgs *args) memory_region_add_subregion(sysmem, 0, ram); /* allocate storage keys */ - storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE); + s390_alloc_storage_keys(my_ram_size); /* init CPUs */ - s390_init_cpus(args->cpu_model, storage_keys); + s390_init_cpus(args->cpu_model); if (kvm_enabled()) { kvm_s390_enable_css_support(s390_cpu_addr2state(0)); diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c index 439d732..21e9124 100644 --- a/hw/s390x/s390-virtio.c +++ b/hw/s390x/s390-virtio.c @@ -123,6 +123,18 @@ static void s390_virtio_register_hcalls(void) s390_virtio_hcall_set_status); } +static uint8_t *storage_keys; + +uint8_t *s390_get_storage_keys(void) +{ + return storage_keys; +} + +void s390_alloc_storage_keys(ram_addr_t ram_size) +{ + storage_keys = g_malloc0(ram_size / TARGET_PAGE_SIZE); +} + /* * The number of running CPUs. On s390 a shutdown is the state of all CPUs * being either stopped or disabled (for interrupts) waiting. We have to @@ -176,7 +188,7 @@ void s390_init_ipl_dev(const char *kernel_filename, qdev_init_nofail(dev); } -void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) +void s390_init_cpus(const char *cpu_model) { int i; @@ -196,7 +208,7 @@ void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) ipi_states[i] = cpu; cs->halted = 1; cpu->env.exception_index = EXCP_HLT; - cpu->env.storage_keys = storage_keys; + cpu->env.storage_keys = s390_get_storage_keys(); } } @@ -231,7 +243,6 @@ static void s390_init(QEMUMachineInitArgs *args) MemoryRegion *sysmem = get_system_memory(); MemoryRegion *ram = g_new(MemoryRegion, 1); int shift = 0; - uint8_t *storage_keys; void *virtio_region; hwaddr virtio_region_len; hwaddr virtio_region_start; @@ -270,10 +281,10 @@ static void s390_init(QEMUMachineInitArgs *args) virtio_region_len); /* allocate storage keys */ - storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE); + s390_alloc_storage_keys(my_ram_size); /* init CPUs */ - s390_init_cpus(args->cpu_model, storage_keys); + s390_init_cpus(args->cpu_model); /* Create VirtIO network adapters */ s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390"); diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h index 5c405e7..c1cb042 100644 --- a/hw/s390x/s390-virtio.h +++ b/hw/s390x/s390-virtio.h @@ -20,7 +20,7 @@ typedef int (*s390_virtio_fn)(const uint64_t *args); void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn); -void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys); +void s390_init_cpus(const char *cpu_model); void s390_init_ipl_dev(const char *kernel_filename, const char *kernel_cmdline, const char *initrd_filename, diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h index 65bef86..877eac7 100644 --- a/target-s390x/cpu.h +++ b/target-s390x/cpu.h @@ -374,6 +374,10 @@ static inline void kvm_s390_interrupt_internal(S390CPU *cpu, int type, { } #endif + +uint8_t *s390_get_storage_keys(void); +void s390_alloc_storage_keys(ram_addr_t ram_size); + S390CPU *s390_cpu_addr2state(uint16_t cpu_addr); void s390_add_running_cpu(S390CPU *cpu); unsigned s390_del_running_cpu(S390CPU *cpu);