diff mbox series

KVM: MIPS: Change the definition of kvm type

Message ID 1599734031-28746-1-git-send-email-chenhc@lemote.com (mailing list archive)
State Accepted
Commit 15e9e35cd1dec2bc138464de6bf8ef828df19235
Headers show
Series KVM: MIPS: Change the definition of kvm type | expand

Commit Message

Huacai Chen Sept. 10, 2020, 10:33 a.m. UTC 
MIPS defines two kvm types:

 #define KVM_VM_MIPS_TE          0
 #define KVM_VM_MIPS_VZ          1

In Documentation/virt/kvm/api.rst it is said that "You probably want to
use 0 as machine type", which implies that type 0 be the "automatic" or
"default" type. And, in user-space libvirt use the null-machine (with
type 0) to detect the kvm capability, which returns "KVM not supported"
on a VZ platform.

I try to fix it in QEMU but it is ugly:
https://lists.nongnu.org/archive/html/qemu-devel/2020-08/msg05629.html

And Thomas Huth suggests me to change the definition of kvm type:
https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg03281.html

So I define like this:

 #define KVM_VM_MIPS_AUTO        0
 #define KVM_VM_MIPS_VZ          1
 #define KVM_VM_MIPS_TE          2

Since VZ and TE cannot co-exists, using type 0 on a TE platform will
still return success (so old user-space tools have no problems on new
kernels); the advantage is that using type 0 on a VZ platform will not
return failure. So, the only problem is "new user-space tools use type
2 on old kernels", but if we treat this as a kernel bug, we can backport
this patch to old stable kernels.

Cc: stable@vger.kernel.org
Signed-off-by: Huacai Chen <chenhc@lemote.com>
---
 arch/mips/kvm/mips.c     | 2 ++
 include/uapi/linux/kvm.h | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

Thomas Huth Sept. 10, 2020, 10:57 a.m. UTC | #1 
On 10/09/2020 12.33, Huacai Chen wrote:
> MIPS defines two kvm types:
> 
>  #define KVM_VM_MIPS_TE          0
>  #define KVM_VM_MIPS_VZ          1
> 
> In Documentation/virt/kvm/api.rst it is said that "You probably want to
> use 0 as machine type", which implies that type 0 be the "automatic" or
> "default" type. And, in user-space libvirt use the null-machine (with
> type 0) to detect the kvm capability, which returns "KVM not supported"
> on a VZ platform.
> 
> I try to fix it in QEMU but it is ugly:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-08/msg05629.html
> 
> And Thomas Huth suggests me to change the definition of kvm type:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg03281.html
> 
> So I define like this:
> 
>  #define KVM_VM_MIPS_AUTO        0
>  #define KVM_VM_MIPS_VZ          1
>  #define KVM_VM_MIPS_TE          2
> 
> Since VZ and TE cannot co-exists, using type 0 on a TE platform will
> still return success (so old user-space tools have no problems on new
> kernels); the advantage is that using type 0 on a VZ platform will not
> return failure. So, the only problem is "new user-space tools use type
> 2 on old kernels", but if we treat this as a kernel bug, we can backport
> this patch to old stable kernels.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> ---
>  arch/mips/kvm/mips.c     | 2 ++
>  include/uapi/linux/kvm.h | 5 +++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index d7ba3f9..9efeb67 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -138,6 +138,8 @@ extern void kvm_init_loongson_ipi(struct kvm *kvm);
>  int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  {
>  	switch (type) {
> +	case KVM_VM_MIPS_AUTO:
> +		break;
>  #ifdef CONFIG_KVM_MIPS_VZ
>  	case KVM_VM_MIPS_VZ:
>  #else
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 29ba8e8..cfc1ae2 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -790,9 +790,10 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_VM_PPC_HV 1
>  #define KVM_VM_PPC_PR 2
>  
> -/* on MIPS, 0 forces trap & emulate, 1 forces VZ ASE */
> -#define KVM_VM_MIPS_TE		0
> +/* on MIPS, 0 indicates auto, 1 forces VZ ASE, 2 forces trap & emulate */
> +#define KVM_VM_MIPS_AUTO	0
>  #define KVM_VM_MIPS_VZ		1
> +#define KVM_VM_MIPS_TE		2
>  
>  #define KVM_S390_SIE_PAGE_OFFSET 1

Thanks, I think that's the right way to go if we don't want mips to
behave completely different compared to the other architectures (e.g.
powerpc is also using 0 as "automatic" type in arch/powerpc/kvm/powerpc.c).

Reviewed-by: Thomas Huth <thuth@redhat.com>
Sasha Levin Sept. 10, 2020, 4:34 p.m. UTC | #2 
Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.8.7, v5.4.63, v4.19.143, v4.14.196, v4.9.235, v4.4.235.

v5.8.7: Build OK!
v5.4.63: Build OK!
v4.19.143: Build OK!
v4.14.196: Build OK!
v4.9.235: Failed to apply! Possible dependencies:
    06c158c96ed8 ("KVM: MIPS/MMU: Convert guest physical map to page table")
    1534b3964901 ("KVM: MIPS/MMU: Simplify ASID restoration")
    1581ff3dbf69 ("KVM: MIPS/MMU: Move preempt/ASID handling to implementation")
    91cdee5710d5 ("KVM: MIPS/T&E: Restore host asid on return to host")
    a2c046e40ff1 ("KVM: MIPS: Add vcpu_run() & vcpu_reenter() callbacks")
    a31b50d741bd ("KVM: MIPS/MMU: Invalidate GVA PTs on ASID changes")
    a60b8438bdba ("KVM: MIPS: Convert get/set_regs -> vcpu_load/put")
    a7ebb2e410f8 ("KVM: MIPS/T&E: active_mm = init_mm in guest context")
    a8a3c426772e ("KVM: MIPS: Add VZ & TE capabilities")
    c550d53934d8 ("KVM: MIPS: Remove duplicated ASIDs from vcpu")
    c92701322711 ("KVM: PPC: Book3S HV: Add userspace interfaces for POWER9 MMU")

v4.4.235: Failed to apply! Possible dependencies:
    107d44a2c5bf ("KVM: document KVM_REINJECT_CONTROL ioctl")
    366baf28ee3f ("KVM: PPC: Use RCU for arch.spapr_tce_tables")
    462ee11e58c9 ("KVM: PPC: Replace SPAPR_TCE_SHIFT with IOMMU_PAGE_SHIFT_4K")
    58ded4201ff0 ("KVM: PPC: Add support for 64bit TCE windows")
    5ee7af18642c ("KVM: PPC: Move reusable bits of H_PUT_TCE handler to helpers")
    a8a3c426772e ("KVM: MIPS: Add VZ & TE capabilities")
    c92701322711 ("KVM: PPC: Book3S HV: Add userspace interfaces for POWER9 MMU")
    d3695aa4f452 ("KVM: PPC: Add support for multiple-TCE hcalls")
    f8626985c7c2 ("KVM: PPC: Account TCE-containing pages in locked_vm")
    fcbb2ce67284 ("KVM: PPC: Rework H_PUT_TCE/H_GET_TCE handlers")
    fe26e52712cc ("KVM: PPC: Add @page_shift to kvmppc_spapr_tce_table")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?
Philippe Mathieu-Daudé Sept. 10, 2020, 4:52 p.m. UTC | #3 
On Thu, Sep 10, 2020 at 12:34 PM Huacai Chen <chenhc@lemote.com> wrote:
>
> MIPS defines two kvm types:
>
>  #define KVM_VM_MIPS_TE          0
>  #define KVM_VM_MIPS_VZ          1
>
> In Documentation/virt/kvm/api.rst it is said that "You probably want to
> use 0 as machine type", which implies that type 0 be the "automatic" or
> "default" type. And, in user-space libvirt use the null-machine (with
> type 0) to detect the kvm capability, which returns "KVM not supported"
> on a VZ platform.
>
> I try to fix it in QEMU but it is ugly:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-08/msg05629.html

I'm not sure this is helpful information to keep in the commit message.

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>
> And Thomas Huth suggests me to change the definition of kvm type:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg03281.html

Suggested-by: Thomas Huth <thuth@redhat.com>

>
> So I define like this:
>
>  #define KVM_VM_MIPS_AUTO        0
>  #define KVM_VM_MIPS_VZ          1
>  #define KVM_VM_MIPS_TE          2
>
> Since VZ and TE cannot co-exists, using type 0 on a TE platform will
> still return success (so old user-space tools have no problems on new
> kernels); the advantage is that using type 0 on a VZ platform will not
> return failure. So, the only problem is "new user-space tools use type
> 2 on old kernels", but if we treat this as a kernel bug, we can backport
> this patch to old stable kernels.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> ---
>  arch/mips/kvm/mips.c     | 2 ++
>  include/uapi/linux/kvm.h | 5 +++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index d7ba3f9..9efeb67 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -138,6 +138,8 @@ extern void kvm_init_loongson_ipi(struct kvm *kvm);
>  int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  {
>         switch (type) {
> +       case KVM_VM_MIPS_AUTO:
> +               break;
>  #ifdef CONFIG_KVM_MIPS_VZ
>         case KVM_VM_MIPS_VZ:
>  #else
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 29ba8e8..cfc1ae2 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -790,9 +790,10 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_VM_PPC_HV 1
>  #define KVM_VM_PPC_PR 2
>
> -/* on MIPS, 0 forces trap & emulate, 1 forces VZ ASE */
> -#define KVM_VM_MIPS_TE         0
> +/* on MIPS, 0 indicates auto, 1 forces VZ ASE, 2 forces trap & emulate */
> +#define KVM_VM_MIPS_AUTO       0
>  #define KVM_VM_MIPS_VZ         1
> +#define KVM_VM_MIPS_TE         2
>
>  #define KVM_S390_SIE_PAGE_OFFSET 1
>
> --
> 2.7.0
>
Huacai Chen Sept. 11, 2020, 12:10 a.m. UTC | #4 
Hi, Sasha,

On Fri, Sep 11, 2020 at 1:18 AM Sasha Levin <sashal@kernel.org> wrote:
>
> Hi
>
> [This is an automated email]
>
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
>
> The bot has tested the following trees: v5.8.7, v5.4.63, v4.19.143, v4.14.196, v4.9.235, v4.4.235.
>
> v5.8.7: Build OK!
> v5.4.63: Build OK!
> v4.19.143: Build OK!
> v4.14.196: Build OK!
> v4.9.235: Failed to apply! Possible dependencies:
>     06c158c96ed8 ("KVM: MIPS/MMU: Convert guest physical map to page table")
>     1534b3964901 ("KVM: MIPS/MMU: Simplify ASID restoration")
>     1581ff3dbf69 ("KVM: MIPS/MMU: Move preempt/ASID handling to implementation")
>     91cdee5710d5 ("KVM: MIPS/T&E: Restore host asid on return to host")
>     a2c046e40ff1 ("KVM: MIPS: Add vcpu_run() & vcpu_reenter() callbacks")
>     a31b50d741bd ("KVM: MIPS/MMU: Invalidate GVA PTs on ASID changes")
>     a60b8438bdba ("KVM: MIPS: Convert get/set_regs -> vcpu_load/put")
>     a7ebb2e410f8 ("KVM: MIPS/T&E: active_mm = init_mm in guest context")
>     a8a3c426772e ("KVM: MIPS: Add VZ & TE capabilities")
>     c550d53934d8 ("KVM: MIPS: Remove duplicated ASIDs from vcpu")
>     c92701322711 ("KVM: PPC: Book3S HV: Add userspace interfaces for POWER9 MMU")
>
> v4.4.235: Failed to apply! Possible dependencies:
>     107d44a2c5bf ("KVM: document KVM_REINJECT_CONTROL ioctl")
>     366baf28ee3f ("KVM: PPC: Use RCU for arch.spapr_tce_tables")
>     462ee11e58c9 ("KVM: PPC: Replace SPAPR_TCE_SHIFT with IOMMU_PAGE_SHIFT_4K")
>     58ded4201ff0 ("KVM: PPC: Add support for 64bit TCE windows")
>     5ee7af18642c ("KVM: PPC: Move reusable bits of H_PUT_TCE handler to helpers")
>     a8a3c426772e ("KVM: MIPS: Add VZ & TE capabilities")
>     c92701322711 ("KVM: PPC: Book3S HV: Add userspace interfaces for POWER9 MMU")
>     d3695aa4f452 ("KVM: PPC: Add support for multiple-TCE hcalls")
>     f8626985c7c2 ("KVM: PPC: Account TCE-containing pages in locked_vm")
>     fcbb2ce67284 ("KVM: PPC: Rework H_PUT_TCE/H_GET_TCE handlers")
>     fe26e52712cc ("KVM: PPC: Add @page_shift to kvmppc_spapr_tce_table")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?
Backport to 4.14+ is enough.

Huacai
>
> --
> Thanks
> Sasha
Paolo Bonzini Sept. 11, 2020, 5:22 p.m. UTC | #5 
On 10/09/20 12:33, Huacai Chen wrote:
> MIPS defines two kvm types:
> 
>  #define KVM_VM_MIPS_TE          0
>  #define KVM_VM_MIPS_VZ          1
> 
> In Documentation/virt/kvm/api.rst it is said that "You probably want to
> use 0 as machine type", which implies that type 0 be the "automatic" or
> "default" type. And, in user-space libvirt use the null-machine (with
> type 0) to detect the kvm capability, which returns "KVM not supported"
> on a VZ platform.
> 
> I try to fix it in QEMU but it is ugly:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-08/msg05629.html
> 
> And Thomas Huth suggests me to change the definition of kvm type:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg03281.html
> 
> So I define like this:
> 
>  #define KVM_VM_MIPS_AUTO        0
>  #define KVM_VM_MIPS_VZ          1
>  #define KVM_VM_MIPS_TE          2
> 
> Since VZ and TE cannot co-exists, using type 0 on a TE platform will
> still return success (so old user-space tools have no problems on new
> kernels); the advantage is that using type 0 on a VZ platform will not
> return failure. So, the only problem is "new user-space tools use type
> 2 on old kernels", but if we treat this as a kernel bug, we can backport
> this patch to old stable kernels.

I'm a bit wary to do that.  However it's not an issue for QEMU since it
generally updates the kernel headers.

Paolo

> Cc: stable@vger.kernel.org
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> ---
>  arch/mips/kvm/mips.c     | 2 ++
>  include/uapi/linux/kvm.h | 5 +++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index d7ba3f9..9efeb67 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -138,6 +138,8 @@ extern void kvm_init_loongson_ipi(struct kvm *kvm);
>  int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  {
>  	switch (type) {
> +	case KVM_VM_MIPS_AUTO:
> +		break;
>  #ifdef CONFIG_KVM_MIPS_VZ
>  	case KVM_VM_MIPS_VZ:
>  #else
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 29ba8e8..cfc1ae2 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -790,9 +790,10 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_VM_PPC_HV 1
>  #define KVM_VM_PPC_PR 2
>  
> -/* on MIPS, 0 forces trap & emulate, 1 forces VZ ASE */
> -#define KVM_VM_MIPS_TE		0
> +/* on MIPS, 0 indicates auto, 1 forces VZ ASE, 2 forces trap & emulate */
> +#define KVM_VM_MIPS_AUTO	0
>  #define KVM_VM_MIPS_VZ		1
> +#define KVM_VM_MIPS_TE		2
>  
>  #define KVM_S390_SIE_PAGE_OFFSET 1
>  
>
Thomas Huth Sept. 19, 2020, 9:18 a.m. UTC | #6 
On 11/09/2020 19.22, Paolo Bonzini wrote:
> On 10/09/20 12:33, Huacai Chen wrote:
>> MIPS defines two kvm types:
>>
>>  #define KVM_VM_MIPS_TE          0
>>  #define KVM_VM_MIPS_VZ          1
>>
>> In Documentation/virt/kvm/api.rst it is said that "You probably want to
>> use 0 as machine type", which implies that type 0 be the "automatic" or
>> "default" type. And, in user-space libvirt use the null-machine (with
>> type 0) to detect the kvm capability, which returns "KVM not supported"
>> on a VZ platform.
>>
>> I try to fix it in QEMU but it is ugly:
>> https://lists.nongnu.org/archive/html/qemu-devel/2020-08/msg05629.html
>>
>> And Thomas Huth suggests me to change the definition of kvm type:
>> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg03281.html
>>
>> So I define like this:
>>
>>  #define KVM_VM_MIPS_AUTO        0
>>  #define KVM_VM_MIPS_VZ          1
>>  #define KVM_VM_MIPS_TE          2
>>
>> Since VZ and TE cannot co-exists, using type 0 on a TE platform will
>> still return success (so old user-space tools have no problems on new
>> kernels); the advantage is that using type 0 on a VZ platform will not
>> return failure. So, the only problem is "new user-space tools use type
>> 2 on old kernels", but if we treat this as a kernel bug, we can backport
>> this patch to old stable kernels.
> 
> I'm a bit wary to do that.  However it's not an issue for QEMU since it
> generally updates the kernel headers.

Are there any other userspace programs beside QEMU that use KVM on MIPS?
If there aren't any other serious userspace programs, I think we should
go ahead with this patch here. Otherwise, what are the other programs
that could be affected?

 Thomas
Paolo Bonzini Sept. 19, 2020, 3:36 p.m. UTC | #7 
On 19/09/20 11:18, Thomas Huth wrote:
>>>
>>> So I define like this:
>>>
>>>  #define KVM_VM_MIPS_AUTO        0
>>>  #define KVM_VM_MIPS_VZ          1
>>>  #define KVM_VM_MIPS_TE          2
>>>
>>> Since VZ and TE cannot co-exists, using type 0 on a TE platform will
>>> still return success (so old user-space tools have no problems on new
>>> kernels); the advantage is that using type 0 on a VZ platform will not
>>> return failure. So, the only problem is "new user-space tools use type
>>> 2 on old kernels", but if we treat this as a kernel bug, we can backport
>>> this patch to old stable kernels.
>> I'm a bit wary to do that.  However it's not an issue for QEMU since it
>> generally updates the kernel headers.
> Are there any other userspace programs beside QEMU that use KVM on MIPS?
> If there aren't any other serious userspace programs, I think we should
> go ahead with this patch here. Otherwise, what are the other programs
> that could be affected?

kvmtool (aka lkvm) I guess.  I don't know if it is affected but I
wouldn't be worried.

Paolo
diff mbox series

Patch

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index d7ba3f9..9efeb67 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -138,6 +138,8 @@  extern void kvm_init_loongson_ipi(struct kvm *kvm);
 int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 {
 	switch (type) {
+	case KVM_VM_MIPS_AUTO:
+		break;
 #ifdef CONFIG_KVM_MIPS_VZ
 	case KVM_VM_MIPS_VZ:
 #else
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 29ba8e8..cfc1ae2 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -790,9 +790,10 @@  struct kvm_ppc_resize_hpt {
 #define KVM_VM_PPC_HV 1
 #define KVM_VM_PPC_PR 2
 
-/* on MIPS, 0 forces trap & emulate, 1 forces VZ ASE */
-#define KVM_VM_MIPS_TE		0
+/* on MIPS, 0 indicates auto, 1 forces VZ ASE, 2 forces trap & emulate */
+#define KVM_VM_MIPS_AUTO	0
 #define KVM_VM_MIPS_VZ		1
+#define KVM_VM_MIPS_TE		2
 
 #define KVM_S390_SIE_PAGE_OFFSET 1