| Message ID | 20220506092403.47406-4-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: KVM: CPU Topology | expand |
On 06.05.22 11:24, Pierre Morel wrote: > During a subsystem reset the Topology-Change-Report is cleared. > Let's give userland the possibility to clear the MTCR in the case > of a subsystem reset. > > To migrate the MTCR, let's give userland the possibility to > query the MTCR state. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > arch/s390/include/uapi/asm/kvm.h | 5 ++ > arch/s390/kvm/kvm-s390.c | 79 ++++++++++++++++++++++++++++++++ > 2 files changed, 84 insertions(+) > > diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h > index 7a6b14874d65..abdcf4069343 100644 > --- a/arch/s390/include/uapi/asm/kvm.h > +++ b/arch/s390/include/uapi/asm/kvm.h > @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req { > #define KVM_S390_VM_CRYPTO 2 > #define KVM_S390_VM_CPU_MODEL 3 > #define KVM_S390_VM_MIGRATION 4 > +#define KVM_S390_VM_CPU_TOPOLOGY 5 > > /* kvm attributes for mem_ctrl */ > #define KVM_S390_VM_MEM_ENABLE_CMMA 0 > @@ -171,6 +172,10 @@ struct kvm_s390_vm_cpu_subfunc { > #define KVM_S390_VM_MIGRATION_START 1 > #define KVM_S390_VM_MIGRATION_STATUS 2 > > +/* kvm attributes for cpu topology */ > +#define KVM_S390_VM_CPU_TOPO_MTR_CLEAR 0 > +#define KVM_S390_VM_CPU_TOPO_MTR_SET 1 > + > /* 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 c8bdce31464f..80a1244f0ead 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -1731,6 +1731,76 @@ static void kvm_s390_sca_set_mtcr(struct kvm *kvm) > ipte_unlock(kvm); > } > > +/** > + * kvm_s390_sca_clear_mtcr > + * @kvm: guest KVM description > + * > + * Is only relevant if the topology facility is present, > + * the caller should check KVM facility 11 > + * > + * Updates the Multiprocessor Topology-Change-Report to signal > + * the guest with a topology change. > + */ > +static void kvm_s390_sca_clear_mtcr(struct kvm *kvm) > +{ > + struct bsca_block *sca = kvm->arch.sca; /* SCA version doesn't matter */ > + > + ipte_lock(kvm); > + sca->utility &= ~SCA_UTILITY_MTCR; One space too much. sca->utility &= ~SCA_UTILITY_MTCR; > + ipte_unlock(kvm); > +} > + > +static int kvm_s390_set_topology(struct kvm *kvm, struct kvm_device_attr *attr) > +{ > + if (!test_kvm_facility(kvm, 11)) > + return -ENXIO; > + > + switch (attr->attr) { > + case KVM_S390_VM_CPU_TOPO_MTR_SET: > + kvm_s390_sca_set_mtcr(kvm); > + break; > + case KVM_S390_VM_CPU_TOPO_MTR_CLEAR: > + kvm_s390_sca_clear_mtcr(kvm); > + break; > + } > + return 0; > +} > + > +/** > + * kvm_s390_sca_get_mtcr > + * @kvm: guest KVM description > + * > + * Is only relevant if the topology facility is present, > + * the caller should check KVM facility 11 > + * > + * reports to QEMU the Multiprocessor Topology-Change-Report. > + */ > +static int kvm_s390_sca_get_mtcr(struct kvm *kvm) > +{ > + struct bsca_block *sca = kvm->arch.sca; /* SCA version doesn't matter */ > + int val; > + > + ipte_lock(kvm); > + val = !!(sca->utility & SCA_UTILITY_MTCR); > + ipte_unlock(kvm); > + > + return val; > +} > + > +static int kvm_s390_get_topology(struct kvm *kvm, struct kvm_device_attr *attr) > +{ > + int mtcr; I think we prefer something like u16 when copying to user space. > + > + if (!test_kvm_facility(kvm, 11)) > + return -ENXIO; > + > + mtcr = kvm_s390_sca_get_mtcr(kvm); > + if (copy_to_user((void __user *)attr->addr, &mtcr, sizeof(mtcr))) > + return -EFAULT; > + > + return 0; > +} You should probably add documentation, and document that only the last bit (0x1) has a meaning. Apart from that LGTM.
On Thu, 12 May 2022 11:31:18 +0200 David Hildenbrand <david@redhat.com> wrote: > On 06.05.22 11:24, Pierre Morel wrote: > > During a subsystem reset the Topology-Change-Report is cleared. > > Let's give userland the possibility to clear the MTCR in the case > > of a subsystem reset. > > > > To migrate the MTCR, let's give userland the possibility to > > query the MTCR state. > > > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > --- > > arch/s390/include/uapi/asm/kvm.h | 5 ++ > > arch/s390/kvm/kvm-s390.c | 79 ++++++++++++++++++++++++++++++++ > > 2 files changed, 84 insertions(+) > > > > diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h > > index 7a6b14874d65..abdcf4069343 100644 > > --- a/arch/s390/include/uapi/asm/kvm.h > > +++ b/arch/s390/include/uapi/asm/kvm.h > > @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req { > > #define KVM_S390_VM_CRYPTO 2 > > #define KVM_S390_VM_CPU_MODEL 3 > > #define KVM_S390_VM_MIGRATION 4 > > +#define KVM_S390_VM_CPU_TOPOLOGY 5 > > > > /* kvm attributes for mem_ctrl */ > > #define KVM_S390_VM_MEM_ENABLE_CMMA 0 > > @@ -171,6 +172,10 @@ struct kvm_s390_vm_cpu_subfunc { > > #define KVM_S390_VM_MIGRATION_START 1 > > #define KVM_S390_VM_MIGRATION_STATUS 2 > > > > +/* kvm attributes for cpu topology */ > > +#define KVM_S390_VM_CPU_TOPO_MTR_CLEAR 0 > > +#define KVM_S390_VM_CPU_TOPO_MTR_SET 1 > > + > > /* 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 c8bdce31464f..80a1244f0ead 100644 > > --- a/arch/s390/kvm/kvm-s390.c > > +++ b/arch/s390/kvm/kvm-s390.c > > @@ -1731,6 +1731,76 @@ static void kvm_s390_sca_set_mtcr(struct kvm *kvm) > > ipte_unlock(kvm); > > } > > > > +/** > > + * kvm_s390_sca_clear_mtcr > > + * @kvm: guest KVM description > > + * > > + * Is only relevant if the topology facility is present, > > + * the caller should check KVM facility 11 > > + * > > + * Updates the Multiprocessor Topology-Change-Report to signal > > + * the guest with a topology change. > > + */ > > +static void kvm_s390_sca_clear_mtcr(struct kvm *kvm) > > +{ > > + struct bsca_block *sca = kvm->arch.sca; /* SCA version doesn't matter */ > > + > > + ipte_lock(kvm); > > + sca->utility &= ~SCA_UTILITY_MTCR; > > > One space too much. > > sca->utility &= ~SCA_UTILITY_MTCR; > > > + ipte_unlock(kvm); > > +} > > + > > +static int kvm_s390_set_topology(struct kvm *kvm, struct kvm_device_attr *attr) > > +{ > > + if (!test_kvm_facility(kvm, 11)) > > + return -ENXIO; > > + > > + switch (attr->attr) { > > + case KVM_S390_VM_CPU_TOPO_MTR_SET: > > + kvm_s390_sca_set_mtcr(kvm); > > + break; > > + case KVM_S390_VM_CPU_TOPO_MTR_CLEAR: > > + kvm_s390_sca_clear_mtcr(kvm); > > + break; > > + } > > + return 0; > > +} > > + > > +/** > > + * kvm_s390_sca_get_mtcr > > + * @kvm: guest KVM description > > + * > > + * Is only relevant if the topology facility is present, > > + * the caller should check KVM facility 11 > > + * > > + * reports to QEMU the Multiprocessor Topology-Change-Report. > > + */ > > +static int kvm_s390_sca_get_mtcr(struct kvm *kvm) > > +{ > > + struct bsca_block *sca = kvm->arch.sca; /* SCA version doesn't matter */ > > + int val; > > + > > + ipte_lock(kvm); > > + val = !!(sca->utility & SCA_UTILITY_MTCR); > > + ipte_unlock(kvm); > > + > > + return val; > > +} > > + > > +static int kvm_s390_get_topology(struct kvm *kvm, struct kvm_device_attr *attr) > > +{ > > + int mtcr; > > I think we prefer something like u16 when copying to user space. but then userspace also has to expect a u16, right? > > > + > > + if (!test_kvm_facility(kvm, 11)) > > + return -ENXIO; > > + > > + mtcr = kvm_s390_sca_get_mtcr(kvm); > > + if (copy_to_user((void __user *)attr->addr, &mtcr, sizeof(mtcr))) > > + return -EFAULT; > > + > > + return 0; > > +} > > You should probably add documentation, and document that only the last > bit (0x1) has a meaning. > > Apart from that LGTM. >
>> >> I think we prefer something like u16 when copying to user space. > > but then userspace also has to expect a u16, right? Yep.
On 5/12/22 11:31, David Hildenbrand wrote: > On 06.05.22 11:24, Pierre Morel wrote: >> During a subsystem reset the Topology-Change-Report is cleared. >> Let's give userland the possibility to clear the MTCR in the case >> of a subsystem reset. >> >> To migrate the MTCR, let's give userland the possibility to >> query the MTCR state. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> arch/s390/include/uapi/asm/kvm.h | 5 ++ >> arch/s390/kvm/kvm-s390.c | 79 ++++++++++++++++++++++++++++++++ >> 2 files changed, 84 insertions(+) >> >> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h >> index 7a6b14874d65..abdcf4069343 100644 >> --- a/arch/s390/include/uapi/asm/kvm.h >> +++ b/arch/s390/include/uapi/asm/kvm.h >> @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req { >> #define KVM_S390_VM_CRYPTO 2 >> #define KVM_S390_VM_CPU_MODEL 3 >> #define KVM_S390_VM_MIGRATION 4 >> +#define KVM_S390_VM_CPU_TOPOLOGY 5 >> >> /* kvm attributes for mem_ctrl */ >> #define KVM_S390_VM_MEM_ENABLE_CMMA 0 >> @@ -171,6 +172,10 @@ struct kvm_s390_vm_cpu_subfunc { >> #define KVM_S390_VM_MIGRATION_START 1 >> #define KVM_S390_VM_MIGRATION_STATUS 2 >> >> +/* kvm attributes for cpu topology */ >> +#define KVM_S390_VM_CPU_TOPO_MTR_CLEAR 0 >> +#define KVM_S390_VM_CPU_TOPO_MTR_SET 1 >> + >> /* 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 c8bdce31464f..80a1244f0ead 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -1731,6 +1731,76 @@ static void kvm_s390_sca_set_mtcr(struct kvm *kvm) >> ipte_unlock(kvm); >> } >> >> +/** >> + * kvm_s390_sca_clear_mtcr >> + * @kvm: guest KVM description >> + * >> + * Is only relevant if the topology facility is present, >> + * the caller should check KVM facility 11 >> + * >> + * Updates the Multiprocessor Topology-Change-Report to signal >> + * the guest with a topology change. >> + */ >> +static void kvm_s390_sca_clear_mtcr(struct kvm *kvm) >> +{ >> + struct bsca_block *sca = kvm->arch.sca; /* SCA version doesn't matter */ >> + >> + ipte_lock(kvm); >> + sca->utility &= ~SCA_UTILITY_MTCR; > > > One space too much. > > sca->utility &= ~SCA_UTILITY_MTCR; > >> + ipte_unlock(kvm); >> +} >> + >> +static int kvm_s390_set_topology(struct kvm *kvm, struct kvm_device_attr *attr) >> +{ >> + if (!test_kvm_facility(kvm, 11)) >> + return -ENXIO; >> + >> + switch (attr->attr) { >> + case KVM_S390_VM_CPU_TOPO_MTR_SET: >> + kvm_s390_sca_set_mtcr(kvm); >> + break; >> + case KVM_S390_VM_CPU_TOPO_MTR_CLEAR: >> + kvm_s390_sca_clear_mtcr(kvm); >> + break; >> + } >> + return 0; >> +} >> + >> +/** >> + * kvm_s390_sca_get_mtcr >> + * @kvm: guest KVM description >> + * >> + * Is only relevant if the topology facility is present, >> + * the caller should check KVM facility 11 >> + * >> + * reports to QEMU the Multiprocessor Topology-Change-Report. >> + */ >> +static int kvm_s390_sca_get_mtcr(struct kvm *kvm) >> +{ >> + struct bsca_block *sca = kvm->arch.sca; /* SCA version doesn't matter */ >> + int val; >> + >> + ipte_lock(kvm); >> + val = !!(sca->utility & SCA_UTILITY_MTCR); >> + ipte_unlock(kvm); >> + >> + return val; >> +} >> + >> +static int kvm_s390_get_topology(struct kvm *kvm, struct kvm_device_attr *attr) >> +{ >> + int mtcr; > > I think we prefer something like u16 when copying to user space. If you prefer. The original idea was to have something like a bool but then I should have change get_mtcr to has_mtcr. > >> + >> + if (!test_kvm_facility(kvm, 11)) >> + return -ENXIO; >> + >> + mtcr = kvm_s390_sca_get_mtcr(kvm); >> + if (copy_to_user((void __user *)attr->addr, &mtcr, sizeof(mtcr))) >> + return -EFAULT; >> + >> + return 0; >> +} > > You should probably add documentation, and document that only the last > bit (0x1) has a meaning. > > Apart from that LGTM. >
On 5/12/22 12:01, David Hildenbrand wrote: >>> >>> I think we prefer something like u16 when copying to user space. >> >> but then userspace also has to expect a u16, right? > > Yep. > Yes but in fact, inspired by previous discussion I had on the VFIO interface, that is the reason why I did prefer an int. It is much simpler than a u16 and the definition of a bit. Despite a bit in a u16 is what the s3990 achitecture proposes I thought we could make it easier on the KVM/QEMU interface. But if the discussion stops here, I will do as you both propose change to u16 in KVM and userland and add the documentation for the interface. Regards, Pierre
On 5/12/22 11:31, David Hildenbrand wrote: > On 06.05.22 11:24, Pierre Morel wrote: >> During a subsystem reset the Topology-Change-Report is cleared. >> Let's give userland the possibility to clear the MTCR in the case >> of a subsystem reset. >> >> To migrate the MTCR, let's give userland the possibility to >> query the MTCR state. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> arch/s390/include/uapi/asm/kvm.h | 5 ++ >> arch/s390/kvm/kvm-s390.c | 79 ++++++++++++++++++++++++++++++++ >> 2 files changed, 84 insertions(+) >> >> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h >> index 7a6b14874d65..abdcf4069343 100644 >> --- a/arch/s390/include/uapi/asm/kvm.h >> +++ b/arch/s390/include/uapi/asm/kvm.h >> @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req { >> #define KVM_S390_VM_CRYPTO 2 >> #define KVM_S390_VM_CPU_MODEL 3 >> #define KVM_S390_VM_MIGRATION 4 >> +#define KVM_S390_VM_CPU_TOPOLOGY 5 >> >> /* kvm attributes for mem_ctrl */ >> #define KVM_S390_VM_MEM_ENABLE_CMMA 0 >> @@ -171,6 +172,10 @@ struct kvm_s390_vm_cpu_subfunc { >> #define KVM_S390_VM_MIGRATION_START 1 >> #define KVM_S390_VM_MIGRATION_STATUS 2 >> >> +/* kvm attributes for cpu topology */ >> +#define KVM_S390_VM_CPU_TOPO_MTR_CLEAR 0 >> +#define KVM_S390_VM_CPU_TOPO_MTR_SET 1 >> + >> /* 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 c8bdce31464f..80a1244f0ead 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -1731,6 +1731,76 @@ static void kvm_s390_sca_set_mtcr(struct kvm *kvm) >> ipte_unlock(kvm); >> } >> >> +/** >> + * kvm_s390_sca_clear_mtcr >> + * @kvm: guest KVM description >> + * >> + * Is only relevant if the topology facility is present, >> + * the caller should check KVM facility 11 >> + * >> + * Updates the Multiprocessor Topology-Change-Report to signal >> + * the guest with a topology change. >> + */ >> +static void kvm_s390_sca_clear_mtcr(struct kvm *kvm) >> +{ >> + struct bsca_block *sca = kvm->arch.sca; /* SCA version doesn't matter */ >> + >> + ipte_lock(kvm); >> + sca->utility &= ~SCA_UTILITY_MTCR; > > > One space too much. > > sca->utility &= ~SCA_UTILITY_MTCR; > >> + ipte_unlock(kvm); >> +} >> + >> +static int kvm_s390_set_topology(struct kvm *kvm, struct kvm_device_attr *attr) >> +{ >> + if (!test_kvm_facility(kvm, 11)) >> + return -ENXIO; >> + >> + switch (attr->attr) { >> + case KVM_S390_VM_CPU_TOPO_MTR_SET: >> + kvm_s390_sca_set_mtcr(kvm); >> + break; >> + case KVM_S390_VM_CPU_TOPO_MTR_CLEAR: >> + kvm_s390_sca_clear_mtcr(kvm); >> + break; >> + } >> + return 0; >> +} >> + >> +/** >> + * kvm_s390_sca_get_mtcr >> + * @kvm: guest KVM description >> + * >> + * Is only relevant if the topology facility is present, >> + * the caller should check KVM facility 11 >> + * >> + * reports to QEMU the Multiprocessor Topology-Change-Report. >> + */ >> +static int kvm_s390_sca_get_mtcr(struct kvm *kvm) >> +{ >> + struct bsca_block *sca = kvm->arch.sca; /* SCA version doesn't matter */ >> + int val; >> + >> + ipte_lock(kvm); >> + val = !!(sca->utility & SCA_UTILITY_MTCR); >> + ipte_unlock(kvm); >> + >> + return val; >> +} >> + >> +static int kvm_s390_get_topology(struct kvm *kvm, struct kvm_device_attr *attr) >> +{ >> + int mtcr; > > I think we prefer something like u16 when copying to user space. I come back here. I think I prefer to keep the int. the u16 is more than the MTCR but the entire utility field, so what should I do: rename the function to kvm_s390_get_sca_utility() ? and then should I modify the KVM_S390_VM_CPU_TOPOLOGY to KVM_S390_VM_SCA_UTILITY ? I do not like that, I do not think we should report/handle more information than expected/needed. I can mask the MTCR bit and return a u16 with bit 0 (0x8000) set but I find this a little weird I admit an int is may be not optimal. logically I should report a bool but I do not like to report a bool through the UAPI. The more I think about it the more I think an int is OK. Or in the case we want to spare memory space I can create a flag in a u16 but it should theoretically be different than the firmware MTCR bit. Could be 0x0001. But still, it is only to leave during the copy_to_user where the copy of an int may be as good or better than the copy of a u16. So any more opinion on this? Regards, Pierre > >> + >> + if (!test_kvm_facility(kvm, 11)) >> + return -ENXIO; >> + >> + mtcr = kvm_s390_sca_get_mtcr(kvm); >> + if (copy_to_user((void __user *)attr->addr, &mtcr, sizeof(mtcr))) >> + return -EFAULT; >> + >> + return 0; >> +} > > You should probably add documentation, and document that only the last > bit (0x1) has a meaning. > > Apart from that LGTM. >
On 16.05.22 16:21, Pierre Morel wrote: > > > On 5/12/22 12:01, David Hildenbrand wrote: >>>> >>>> I think we prefer something like u16 when copying to user space. >>> >>> but then userspace also has to expect a u16, right? >> >> Yep. >> > > Yes but in fact, inspired by previous discussion I had on the VFIO > interface, that is the reason why I did prefer an int. > It is much simpler than a u16 and the definition of a bit. > > Despite a bit in a u16 is what the s3990 achitecture proposes I thought > we could make it easier on the KVM/QEMU interface. > > But if the discussion stops here, I will do as you both propose change > to u16 in KVM and userland and add the documentation for the interface. In general, we pass via the ABI fixed-sized values -- u8, u16, u32, u64 ... instead of int. Simply because sizeof(int) is in theory variable (e.g., 32bit vs 64bit). Take a look at arch/s390/include/uapi/asm/kvm.h and you won't find any usage of int or bool. Having that said, I'll let the maintainers decide. Using e.g., u8 is just the natural thing to do on a Linux ABI, but we don't really support 32 bit ... maybe we'll support 128bit at one point? ;)
On 5/18/22 16:33, David Hildenbrand wrote: > On 16.05.22 16:21, Pierre Morel wrote: >> >> >> On 5/12/22 12:01, David Hildenbrand wrote: >>>>> >>>>> I think we prefer something like u16 when copying to user space. >>>> >>>> but then userspace also has to expect a u16, right? >>> >>> Yep. >>> >> >> Yes but in fact, inspired by previous discussion I had on the VFIO >> interface, that is the reason why I did prefer an int. >> It is much simpler than a u16 and the definition of a bit. >> >> Despite a bit in a u16 is what the s3990 achitecture proposes I thought >> we could make it easier on the KVM/QEMU interface. >> >> But if the discussion stops here, I will do as you both propose change >> to u16 in KVM and userland and add the documentation for the interface. > > In general, we pass via the ABI fixed-sized values -- u8, u16, u32, u64 > ... instead of int. Simply because sizeof(int) is in theory variable > (e.g., 32bit vs 64bit). > > Take a look at arch/s390/include/uapi/asm/kvm.h and you won't find any > usage of int or bool. > > Having that said, I'll let the maintainers decide. Using e.g., u8 is > just the natural thing to do on a Linux ABI, but we don't really support > 32 bit ... maybe we'll support 128bit at one point? ;) > OK then I use u16 with a flag in case we get something in the utilities which is related to the topology in the future. Thanks, Pierre
diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h index 7a6b14874d65..abdcf4069343 100644 --- a/arch/s390/include/uapi/asm/kvm.h +++ b/arch/s390/include/uapi/asm/kvm.h @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req { #define KVM_S390_VM_CRYPTO 2 #define KVM_S390_VM_CPU_MODEL 3 #define KVM_S390_VM_MIGRATION 4 +#define KVM_S390_VM_CPU_TOPOLOGY 5 /* kvm attributes for mem_ctrl */ #define KVM_S390_VM_MEM_ENABLE_CMMA 0 @@ -171,6 +172,10 @@ struct kvm_s390_vm_cpu_subfunc { #define KVM_S390_VM_MIGRATION_START 1 #define KVM_S390_VM_MIGRATION_STATUS 2 +/* kvm attributes for cpu topology */ +#define KVM_S390_VM_CPU_TOPO_MTR_CLEAR 0 +#define KVM_S390_VM_CPU_TOPO_MTR_SET 1 + /* 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 c8bdce31464f..80a1244f0ead 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -1731,6 +1731,76 @@ static void kvm_s390_sca_set_mtcr(struct kvm *kvm) ipte_unlock(kvm); } +/** + * kvm_s390_sca_clear_mtcr + * @kvm: guest KVM description + * + * Is only relevant if the topology facility is present, + * the caller should check KVM facility 11 + * + * Updates the Multiprocessor Topology-Change-Report to signal + * the guest with a topology change. + */ +static void kvm_s390_sca_clear_mtcr(struct kvm *kvm) +{ + struct bsca_block *sca = kvm->arch.sca; /* SCA version doesn't matter */ + + ipte_lock(kvm); + sca->utility &= ~SCA_UTILITY_MTCR; + ipte_unlock(kvm); +} + +static int kvm_s390_set_topology(struct kvm *kvm, struct kvm_device_attr *attr) +{ + if (!test_kvm_facility(kvm, 11)) + return -ENXIO; + + switch (attr->attr) { + case KVM_S390_VM_CPU_TOPO_MTR_SET: + kvm_s390_sca_set_mtcr(kvm); + break; + case KVM_S390_VM_CPU_TOPO_MTR_CLEAR: + kvm_s390_sca_clear_mtcr(kvm); + break; + } + return 0; +} + +/** + * kvm_s390_sca_get_mtcr + * @kvm: guest KVM description + * + * Is only relevant if the topology facility is present, + * the caller should check KVM facility 11 + * + * reports to QEMU the Multiprocessor Topology-Change-Report. + */ +static int kvm_s390_sca_get_mtcr(struct kvm *kvm) +{ + struct bsca_block *sca = kvm->arch.sca; /* SCA version doesn't matter */ + int val; + + ipte_lock(kvm); + val = !!(sca->utility & SCA_UTILITY_MTCR); + ipte_unlock(kvm); + + return val; +} + +static int kvm_s390_get_topology(struct kvm *kvm, struct kvm_device_attr *attr) +{ + int mtcr; + + if (!test_kvm_facility(kvm, 11)) + return -ENXIO; + + mtcr = kvm_s390_sca_get_mtcr(kvm); + if (copy_to_user((void __user *)attr->addr, &mtcr, sizeof(mtcr))) + return -EFAULT; + + return 0; +} + static int kvm_s390_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr) { int ret; @@ -1751,6 +1821,9 @@ static int kvm_s390_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr) case KVM_S390_VM_MIGRATION: ret = kvm_s390_vm_set_migration(kvm, attr); break; + case KVM_S390_VM_CPU_TOPOLOGY: + ret = kvm_s390_set_topology(kvm, attr); + break; default: ret = -ENXIO; break; @@ -1776,6 +1849,9 @@ static int kvm_s390_vm_get_attr(struct kvm *kvm, struct kvm_device_attr *attr) case KVM_S390_VM_MIGRATION: ret = kvm_s390_vm_get_migration(kvm, attr); break; + case KVM_S390_VM_CPU_TOPOLOGY: + ret = kvm_s390_get_topology(kvm, attr); + break; default: ret = -ENXIO; break; @@ -1849,6 +1925,9 @@ static int kvm_s390_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr) case KVM_S390_VM_MIGRATION: ret = 0; break; + case KVM_S390_VM_CPU_TOPOLOGY: + ret = test_kvm_facility(kvm, 11) ? 0 : -ENXIO; + break; default: ret = -ENXIO; break;
During a subsystem reset the Topology-Change-Report is cleared. Let's give userland the possibility to clear the MTCR in the case of a subsystem reset. To migrate the MTCR, let's give userland the possibility to query the MTCR state. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- arch/s390/include/uapi/asm/kvm.h | 5 ++ arch/s390/kvm/kvm-s390.c | 79 ++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+)