On 06/26/2014 09:14 AM, Christian Borntraeger wrote:
> On 25/06/14 16:27, Matthew Rosato wrote:
>> Add memory information to read SCP info and add handlers for
>> Read Storage Element Information, Attach Storage Element,
>> Assign Storage and Unassign Storage.
>>
>> Signed-off-by: Matthew Rosato <mjros...@linux.vnet.ibm.com>
>
> In general this looks fine. I gave it some testing and most of it seems to
> work.
> With lots of standy chunks (-m 10240,maxmem=204800M,slots=16) I get an abort,
> though:
>
> #0 0x000003fffb9cffd8 in raise () from /lib64/libc.so.6
> #1 0x000003fffb9d1b30 in abort () from /lib64/libc.so.6
> #2 0x000003fffb9c7376 in __assert_fail_base () from /lib64/libc.so.6
> #3 0x000003fffb9c7418 in __assert_fail () from /lib64/libc.so.6
> #4 0x000000008001db28 in find_ram_offset (size=<optimized out>) at
> /home/cborntra/REPOS/qemu/exec.c:1109
> #5 ram_block_add (new_block=0x3fd70001ab0) at
> /home/cborntra/REPOS/qemu/exec.c:1236
> #6 0x000000008005ced0 in memory_region_init_ram (mr=mr@entry=0x3fd700019e0,
> owner=owner@entry=0x0, name=name@entry=0x3fd759fda98 "standby.ram1", size=0)
> at /home/cborntra/REPOS/qemu/memory.c:1033
>
> Why do we pass size==0?
>
> #7 0x0000000080080cdc in assign_storage (sccb=0x3fd759fca98) at
> /home/cborntra/REPOS/qemu/hw/s390x/sclp.c:242
> #8 sclp_execute (code=<optimized out>, sccb=0x3fd759fca98) at
> /home/cborntra/REPOS/qemu/hw/s390x/sclp.c:348
> #9 sclp_service_call (env=env@entry=0x808dcf98, sccb=2144296960,
> code=<optimized out>) at /home/cborntra/REPOS/qemu/hw/s390x/sclp.c:395
> #10 0x00000000800b127c in kvm_sclp_service_call (run=<optimized out>,
> ipbh0=35, cpu=0x808d4d00) at /home/cborntra/REPOS/qemu/target-s390x/kvm.c:705
> #11 handle_b2 (run=<optimized out>, ipa1=<optimized out>, cpu=0x808d4d00) at
> /home/cborntra/REPOS/qemu/target-s390x/kvm.c:777
> #12 handle_instruction (run=<optimized out>, cpu=0x808d4d00) at
> /home/cborntra/REPOS/qemu/target-s390x/kvm.c:1001
> #13 handle_intercept (cpu=0x808d4d00) at
> /home/cborntra/REPOS/qemu/target-s390x/kvm.c:1060
> #14 kvm_arch_handle_exit (cs=cs@entry=0x808d4d00,
> run=run@entry=0x3fffd081000) at
> /home/cborntra/REPOS/qemu/target-s390x/kvm.c:1177
> #15 0x000000008005a696 in kvm_cpu_exec (cpu=cpu@entry=0x808d4d00) at
> /home/cborntra/REPOS/qemu/kvm-all.c:1784
> #16 0x000000008004668a in qemu_kvm_cpu_thread_fn (arg=0x808d4d00) at
> /home/cborntra/REPOS/qemu/cpus.c:874
> #17 0x000003fffce18412 in start_thread () from /lib64/libpthread.so.0
> #18 0x000003fffba9e0ae in thread_start () from /lib64/libc.so.6'
Looks like this_subregion_size is overflowing...
[..snip..]
>> +static void assign_storage(SCCB *sccb)
>> +{
>> + MemoryRegion *mr = NULL;
>> + int this_subregion_size;
This shouldn't be an int, everything else is 64-bit unsigned terms, so
it's causing truncation with large subregion sizes. Should be uint64_t
this_subregion_size, which is what memory_region_init_ram expects.
This fixes the problem for me. I'll do some more testing with that and
include it in the next version -- Thanks!
>> + AssignStorage *assign_info = (AssignStorage *) sccb;
>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>> + assert(mhd);
>> + ram_addr_t assign_addr = (assign_info->rn - 1) * mhd->rzm;
>> + MemoryRegion *sysmem = get_system_memory();
>> +
>> + if ((assign_addr % MEM_SECTION_SIZE == 0) &&
>> + (assign_addr >= mhd->padded_ram_size)) {
>> + /* Re-use existing memory region if found */
>> + mr = memory_region_find(sysmem, assign_addr, 1).mr;
>> + if (!mr) {
>> +
>> + MemoryRegion *standby_ram = g_new(MemoryRegion, 1);
>> +
>> + /* offset to align to standby_subregion_size for allocation */
>> + ram_addr_t offset = assign_addr -
>> + (assign_addr - mhd->padded_ram_size)
>> + % mhd->standby_subregion_size;
>> +
>> + /* strlen("standby.ram") + 4 (Max of KVM_MEMORY_SLOTS) + NULL
>> */
>> + char id[16];
>> + snprintf(id, 16, "standby.ram%d",
>> + (int)((offset - mhd->padded_ram_size) /
>> + mhd->standby_subregion_size) + 1);
>> +
>> + /* Allocate a subregion of the calculated
>> standby_subregion_size */
>> + if (offset + mhd->standby_subregion_size >
>> + mhd->padded_ram_size + mhd->standby_mem_size) {
>> + this_subregion_size = mhd->padded_ram_size +
>> + mhd->standby_mem_size - offset;
>> + } else {
>> + this_subregion_size = mhd->standby_subregion_size;
>> + }
>> +
>> + memory_region_init_ram(standby_ram, NULL, id,
>> this_subregion_size);
>> + vmstate_register_ram_global(standby_ram);
>> + memory_region_add_subregion(sysmem, offset, standby_ram);
>> + }
>> + /* The specified subregion is no longer in standby */
>> + mhd->standby_state_map[(assign_addr - mhd->padded_ram_size)
>> + / MEM_SECTION_SIZE] = 1;
>> + }
>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
>> +}
>> +
>> +static void unassign_storage(SCCB *sccb)
>> +{
>> + MemoryRegion *mr = NULL;
>> + AssignStorage *assign_info = (AssignStorage *) sccb;
>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>> + assert(mhd);
>> + ram_addr_t unassign_addr = (assign_info->rn - 1) * mhd->rzm;
>> + MemoryRegion *sysmem = get_system_memory();
>> +
>> + /* if the addr is a multiple of 256 MB */
>> + if ((unassign_addr % MEM_SECTION_SIZE == 0) &&
>> + (unassign_addr >= mhd->padded_ram_size)) {
>> + mhd->standby_state_map[(unassign_addr -
>> + mhd->padded_ram_size) / MEM_SECTION_SIZE] = 0;
>> +
>> + /* find the specified memory region and destroy it */
>> + mr = memory_region_find(sysmem, unassign_addr, 1).mr;
>> + if (mr) {
>> + int i;
>> + int is_removable = 1;
>> + ram_addr_t map_offset = (unassign_addr - mhd->padded_ram_size -
>> + (unassign_addr - mhd->padded_ram_size)
>> + % mhd->standby_subregion_size);
>> + /* Mark all affected subregions as 'standby' once again */
>> + for (i = 0;
>> + i < (mhd->standby_subregion_size / MEM_SECTION_SIZE);
>> + i++) {
>> +
>> + if (mhd->standby_state_map[i + map_offset /
>> MEM_SECTION_SIZE]) {
>> + is_removable = 0;
>> + break;
>> + }
>> + }
>> + if (is_removable) {
>> + memory_region_del_subregion(sysmem, mr);
>> + memory_region_destroy(mr);
>> + g_free(mr);
>> + }
>> + }
>> + }
>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
>> +}
>> +
>> /* Provide information about the CPU */
>> static void sclp_read_cpu_info(SCCB *sccb)
>> {
>> @@ -103,6 +334,22 @@ static void sclp_execute(SCCB *sccb, uint32_t code)
>> case SCLP_CMDW_READ_CPU_INFO:
>> sclp_read_cpu_info(sccb);
>> break;
>> + case SCLP_READ_STORAGE_ELEMENT_INFO:
>> + if (code & 0xff00) {
>> + read_storage_element1_info(sccb);
>> + } else {
>> + read_storage_element0_info(sccb);
>> + }
>> + break;
>> + case SCLP_ATTACH_STORAGE_ELEMENT:
>> + attach_storage_element(sccb, (code & 0xff00) >> 8);
>> + break;
>> + case SCLP_ASSIGN_STORAGE:
>> + assign_storage(sccb);
>> + break;
>> + case SCLP_UNASSIGN_STORAGE:
>> + unassign_storage(sccb);
>> + break;
>> default:
>> efc->command_handler(ef, sccb, code);
>> break;
>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>> index ba0e4b4..1c1681c 100644
>> --- a/target-s390x/cpu.h
>> +++ b/target-s390x/cpu.h
>> @@ -1047,6 +1047,7 @@ static inline void cpu_inject_crw_mchk(S390CPU *cpu)
>>
>> /* from s390-virtio-ccw */
>> #define MEM_SECTION_SIZE 0x10000000UL
>> +#define MAX_AVAIL_SLOTS 32
>>
>> /* fpu_helper.c */
>> uint32_t set_cc_nz_f32(float32 v);
>> @@ -1070,6 +1071,7 @@ void kvm_s390_enable_css_support(S390CPU *cpu);
>> int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>> int vq, bool assign);
>> int kvm_s390_cpu_restart(S390CPU *cpu);
>> +int kvm_s390_get_memslot_count(KVMState *s);
>> void kvm_s390_clear_cmma_callback(void *opaque);
>> #else
>> static inline void kvm_s390_io_interrupt(uint16_t subchannel_id,
>> @@ -1097,6 +1099,10 @@ static inline int kvm_s390_cpu_restart(S390CPU *cpu)
>> static inline void kvm_s390_clear_cmma_callback(void *opaque)
>> {
>> }
>> +static inline int kvm_s390_get_memslot_count(KVMState *s)
>> +{
>> + return MAX_AVAIL_SLOTS;
>> +}
>> #endif
>>
>> static inline void cmma_reset(S390CPU *cpu)
>> @@ -1115,6 +1121,15 @@ static inline int s390_cpu_restart(S390CPU *cpu)
>> return -ENOSYS;
>> }
>>
>> +static inline int s390_get_memslot_count(KVMState *s)
>> +{
>> + if (kvm_enabled()) {
>> + return kvm_s390_get_memslot_count(s);
>> + } else {
>> + return MAX_AVAIL_SLOTS;
>> + }
>> +}
>> +
>> void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
>> uint32_t io_int_parm, uint32_t io_int_word);
>> void s390_crw_mchk(void);
>> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
>> index a6e587b..4b05d48 100644
>> --- a/target-s390x/kvm.c
>> +++ b/target-s390x/kvm.c
>> @@ -1283,3 +1283,8 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
>> *notifier, uint32_t sch,
>> }
>> return kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD, &kick);
>> }
>> +
>> +int kvm_s390_get_memslot_count(KVMState *s)
>> +{
>> + return kvm_check_extension(s, KVM_CAP_NR_MEMSLOTS);
>> +}
>>
>
>
>
>
