Messages in this thread | | | Date | Mon, 19 May 2014 13:48:08 +0200 | From | Alexander Graf <> | Subject | Re: [PATCH v1 RFC 6/6] KVM: s390: add cpu model support |
| |
On 19.05.14 12:53, Michael Mueller wrote: > On Fri, 16 May 2014 22:31:12 +0200 > Alexander Graf <agraf@suse.de> wrote: > >> On 16.05.14 17:39, Michael Mueller wrote: >>> On Fri, 16 May 2014 14:08:24 +0200 >>> Alexander Graf <agraf@suse.de> wrote: >>> >>>> On 13.05.14 16:58, Michael Mueller wrote: >>>>> This patch enables cpu model support in kvm/s390 via the vm attribute >>>>> interface. >>>>> >>>>> During KVM initialization, the host properties cpuid, IBC value and the >>>>> facility list are stored in the architecture specific cpu model structure. >>>>> >>>>> During vcpu setup, these properties are taken to initialize the related SIE >>>>> state. This mechanism allows to adjust the properties from user space and thus >>>>> to implement different selectable cpu models. >>>>> >>>>> This patch uses the IBC functionality to block instructions that have not >>>>> been implemented at the requested CPU type and GA level compared to the >>>>> full host capability. >>>>> >>>>> Userspace has to initialize the cpu model before vcpu creation. A cpu model >>>>> change of running vcpus is currently not possible. >>>> Why is this VM global? It usually fits a lot better modeling wise when >>>> CPU types are vcpu properties. >>> It simplifies the code substantially because it inherently guarantees the vcpus being >>> configured identical. In addition, there is no S390 hardware implementation containing >>> inhomogeneous processor types. Thus I consider the properties as machine specific. >>> >>>>> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> >>>>> --- >>>>> arch/s390/include/asm/kvm_host.h | 4 +- >>>>> arch/s390/include/uapi/asm/kvm.h | 23 ++++++ >>>>> arch/s390/kvm/kvm-s390.c | 146 ++++++++++++++++++++++++++++++++++++++- >>>>> arch/s390/kvm/kvm-s390.h | 1 + >>>>> 4 files changed, 172 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h >>>>> index b4751ba..6b826cb 100644 >>>>> --- a/arch/s390/include/asm/kvm_host.h >>>>> +++ b/arch/s390/include/asm/kvm_host.h >>>>> @@ -84,7 +84,8 @@ struct kvm_s390_sie_block { >>>>> atomic_t cpuflags; /* 0x0000 */ >>>>> __u32 : 1; /* 0x0004 */ >>>>> __u32 prefix : 18; >>>>> - __u32 : 13; >>>>> + __u32 : 1; >>>>> + __u32 ibc : 12; >>>>> __u8 reserved08[4]; /* 0x0008 */ >>>>> #define PROG_IN_SIE (1<<0) >>>>> __u32 prog0c; /* 0x000c */ >>>>> @@ -418,6 +419,7 @@ struct kvm_s390_cpu_model { >>>>> unsigned long *sie_fac; >>>>> struct cpuid cpu_id; >>>>> unsigned long *fac_list; >>>>> + unsigned short ibc; >>>>> }; >>>>> >>>>> struct kvm_arch{ >>>>> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h >>>>> index 313100a..82ef1b5 100644 >>>>> --- a/arch/s390/include/uapi/asm/kvm.h >>>>> +++ b/arch/s390/include/uapi/asm/kvm.h >>>>> @@ -58,12 +58,35 @@ struct kvm_s390_io_adapter_req { >>>>> >>>>> /* kvm attr_group on vm fd */ >>>>> #define KVM_S390_VM_MEM_CTRL 0 >>>>> +#define KVM_S390_VM_CPU_MODEL 1 >>>>> >>>>> /* kvm attributes for mem_ctrl */ >>>>> #define KVM_S390_VM_MEM_ENABLE_CMMA 0 >>>>> #define KVM_S390_VM_MEM_CLR_CMMA 1 >>>>> #define KVM_S390_VM_MEM_CLR_PAGES 2 >>>>> >>>>> +/* kvm attributes for cpu_model */ >>>>> + >>>>> +/* the s390 processor related attributes are r/w */ >>>>> +#define KVM_S390_VM_CPU_PROCESSOR 0 >>>>> +struct kvm_s390_vm_cpu_processor { >>>>> + __u64 cpuid; >>>>> + __u16 ibc; >>>>> + __u8 pad[6]; >>>>> + __u64 fac_list[256]; >>>>> +}; >>>>> + >>>>> +/* the machine related attributes are read only */ >>>>> +#define KVM_S390_VM_CPU_MACHINE 1 >>>>> +struct kvm_s390_vm_cpu_machine { >>>>> + __u64 cpuid; >>>>> + __u32 ibc_range; >>>>> + __u8 pad[4]; >>>>> + __u64 fac_mask[256]; >>>>> + __u64 hard_fac_list[256]; >>>>> + __u64 soft_fac_list[256]; >>>>> +}; >>>>> + >>>>> /* for KVM_GET_REGS and KVM_SET_REGS */ >>>>> struct kvm_regs { >>>>> /* general purpose regs for s390 */ >>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >>>>> index a53652f..9965d8b 100644 >>>>> --- a/arch/s390/kvm/kvm-s390.c >>>>> +++ b/arch/s390/kvm/kvm-s390.c >>>>> @@ -369,6 +369,110 @@ static int kvm_s390_mem_control(struct kvm *kvm, struct >>>>> kvm_device_attr *attr) return ret; >>>>> } >>>>> >>>>> +static int kvm_s390_set_processor(struct kvm *kvm, struct kvm_device_attr *attr) >>>>> +{ >>>>> + struct kvm_s390_vm_cpu_processor *proc; >>>>> + >>>>> + if (atomic_read(&kvm->online_vcpus)) >>>>> + return -EBUSY; >>>>> + >>>>> + proc = kzalloc(sizeof(*proc), GFP_KERNEL); >>>>> + if (!proc) >>>>> + return -ENOMEM; >>>>> + >>>>> + if (copy_from_user(proc, (void __user *)attr->addr, >>>>> + sizeof(*proc))) { >>>>> + kfree(proc); >>>>> + return -EFAULT; >>>>> + } >>>>> + >>>>> + mutex_lock(&kvm->lock); >>>>> + memcpy(&kvm->arch.model.cpu_id, &proc->cpuid, >>>>> + sizeof(struct cpuid)); >>>>> + kvm->arch.model.ibc = proc->ibc; >>>>> + kvm_s390_apply_fac_list_mask((long unsigned *)proc->fac_list); >>>>> + memcpy(kvm->arch.model.fac_list, proc->fac_list, >>>>> + S390_ARCH_FAC_LIST_SIZE_BYTE); >>>>> + mutex_unlock(&kvm->lock); >>>>> + kfree(proc); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int kvm_s390_set_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr) >>>>> +{ >>>>> + int ret = -ENXIO; >>>>> + >>>>> + switch (attr->attr) { >>>>> + case KVM_S390_VM_CPU_PROCESSOR: >>>>> + ret = kvm_s390_set_processor(kvm, attr); >>>>> + break; >>>>> + } >>>>> + return ret; >>>>> +} >>>>> + >>>>> +static int kvm_s390_get_processor(struct kvm *kvm, struct kvm_device_attr *attr) >>>>> +{ >>>>> + struct kvm_s390_vm_cpu_processor *proc; >>>>> + int rc = 0; >>>>> + >>>>> + proc = kzalloc(sizeof(*proc), GFP_KERNEL); >>>>> + if (!proc) { >>>>> + rc = -ENOMEM; >>>>> + goto out; >>>>> + } >>>>> + memcpy(&proc->cpuid, &kvm->arch.model.cpu_id, sizeof(struct cpuid)); >>>>> + proc->ibc = kvm->arch.model.ibc; >>>>> + memcpy(&proc->fac_list, kvm->arch.model.fac_list, >>>>> + S390_ARCH_FAC_LIST_SIZE_BYTE); >>>>> + if (copy_to_user((void __user *)attr->addr, proc, sizeof(*proc))) >>>>> + rc = -EFAULT; >>>>> + kfree(proc); >>>>> +out: >>>>> + return rc; >>>>> +} >>>>> + >>>>> +static int kvm_s390_get_machine(struct kvm *kvm, struct kvm_device_attr *attr) >>>>> +{ >>>>> + struct kvm_s390_vm_cpu_machine *mach; >>>>> + int rc = 0; >>>>> + >>>>> + mach = kzalloc(sizeof(*mach), GFP_KERNEL); >>>>> + if (!mach) { >>>>> + rc = -ENOMEM; >>>>> + goto out; >>>>> + } >>>>> + get_cpu_id((struct cpuid *) &mach->cpuid); >>>>> + mach->ibc_range = kvm_s390_lowest_ibc() << 16; >>>>> + mach->ibc_range |= kvm_s390_latest_ibc(); >>>>> + memcpy(&mach->fac_mask, kvm_s390_fac_list_mask, >>>>> + kvm_s390_fac_list_mask_size() * sizeof(u64)); >>>>> + kvm_s390_get_hard_fac_list((long unsigned int *) &mach->hard_fac_list, >>>>> + S390_ARCH_FAC_LIST_SIZE_U64); >>>>> + kvm_s390_get_soft_fac_list((long unsigned int *) &mach->soft_fac_list, >>>>> + S390_ARCH_FAC_LIST_SIZE_U64); >>>> I really have a hard time grasping what hard and soft means. >>> Hard facilities are those that are implemented by the CPU itself, either through processor >>> logic or be means of firmware micro code. That's the list returned by the STFL/STFLE >>> instruction. In addition to that, one can imagine that in future some of that features are >>> emulated on KVM side. These will be placed in the soft facility list and are optionally to >>> request by user space. >> I don't see why we would have to differentiate between the two. User >> space wants features enabled. Whether they are done in hardware or in >> software doesn't matter. > I've tried to make my point on that in last answer of patch 3/6. It's a mistake > to think that user space just wants to have features, they come with different > qualities!
So? If I want to run a z9 compatible guest, I do -cpu z9. I can either
a) run it with emulation of a facility or b) not run it
which one would the user choose?
> >> So all we need is a list of "features the guest sees available" which is >> the same as "features user space wants the guest to see" which then gets >> masked through "features the host can do in hardware". >> >> For emulation we can just check on the global feature availability on >> whether we should emulate them or not. >> >>>> Also, if user space wants to make sure that its feature list is actually >>>> workable on the host kernel, it needs to set and get the features again >>>> and then compare that with the ones it set? That's different from x86's >>>> cpuid implementation but probably workable. >>> User space will probe what facilities are available and match them with the predefined cpu >>> model set. Only those models which use a partial or full subset of the hard/host facility >>> list are selectable. >> Why? > If a host does not offer the features required for a model it is not able to > run efficiently. > >> Please take a look at how x86 does cpuid masking :). >> >> In fact, I'm not 100% convinced that it's a good idea to link cpuid / >> feature list exposure to the guest and actual feature implementation >> inside the guest together. On POWER there is a patch set pending that >> implements these two things separately - admittedly mostly because >> hardware sucks and we can't change the PVR. > That is maybe the big difference with s390. The cpuid in the S390 case is not > directly comparable with the processor version register of POWER. > > In the S390 world we have a well defined CPU model room spanned by the machine > type and its GA count. Thus we can define a bijective mapping between > (type, ga) <-> (cpuid, ibc, facility set). From type and ga we form the model > name which BTW is meaningful also for a human user.
Same thing as POWER.
> > By means of this name, a management interface (libvirt) will draw decisions if > migration to a remote hypervisor is a good idea or not. For that it just needs > to compare if the current model of the guest on the source hypervisor > ("query-cpu-model"), is contained in the supported model list of the target > hypervisor ("query-cpu-definitions").
I don't think this works, since QEMU should always return all the cpu definitions it's aware of on query-cpu-definitions, not just the ones that it thinks may be compatible with the host at a random point in time.
Please check with the x86 people to find out how they do this.
> >>>> I also don't quite grasp what the story behind IBC is. Do you actually >>>> block instructions? Where do you match instructions that have to get >>>> blocked with instructions that user space wants to see exposed? >>>> >>> Instruction Blocking Control is a feature that was first introduced with the 2097 (IBM System >>> z10.) The IBC value is part of the SIE state. Just consider it as a kind of parameter, that >>> allows only instructions that have been implemented up to a certain cpu type and GA level to >>> become executed, all other op codes will end in an illegal opcode abort. E.g. take the >>> "Transactional Memory" instructions, they are implemented since type 2827, GA1 >>> (IBM zEnterprise EC12.). The IBC value has 12 bits 8 for the type and 4 for the GA level. >>> 0x001 means its a z10, GA1. The value 0x021 means it's a 2827 (CMOS generation 12 is 0x02) and >>> GA1 and so forth. A guest running with IBC value 0x012 (z196 GA2) will not be able to use >>> TE instructions in contrast to a guest running with IBC value 0x022 given the host supports >>> it. >> That sounds very similar to the "compat" cpu property that Alexey is >> introducing for POWER. Maybe we can model it identically? > I think it is something different. With "compat" one might be able the express some kind > of compatibility between two processors of the some different generations, upon which > the management interface can draw conclusions if migration makes sense or not. > > The IBC works totally different. It enforces that the instruction set defined for TYPE-GA.
Yes, which is the same as the PCR register on POWER which the "compat" option controls. I think we can simplify s390x because it's not as broken as POWER in what we can fake to the guest, but I think you should at least be aware of the concepts that are around.
Alex
| |