* [PATCH] kvm: x86: move architecture-specific code into kvm_arch_vcpu_fault @ 2021-08-11 3:17 Hou Wenlong 2021-08-11 18:52 ` Sean Christopherson 0 siblings, 1 reply; 7+ messages in thread From: Hou Wenlong @ 2021-08-11 3:17 UTC (permalink / raw) To: kvm Cc: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-kernel The function kvm_arch_vcpu_fault can handle architecture-specific case, so move pio-data fault case into it for x86. Signed-off-by: Hou Wenlong <houwenlong93@linux.alibaba.com> --- arch/x86/kvm/x86.c | 8 ++++++++ virt/kvm/kvm_main.c | 4 ---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index e5d5c5ed7dd4..30b0706eced8 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5348,6 +5348,14 @@ long kvm_arch_vcpu_ioctl(struct file *filp, vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { + if (vmf->pgoff == KVM_PIO_PAGE_OFFSET) { + struct page *page = virt_to_page(vcpu->arch.pio_data); + + get_page(page); + vmf->page = page; + return 0; + } + return VM_FAULT_SIGBUS; } diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index b50dbe269f4b..084fba09eca0 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3370,10 +3370,6 @@ static vm_fault_t kvm_vcpu_fault(struct vm_fault *vmf) if (vmf->pgoff == 0) page = virt_to_page(vcpu->run); -#ifdef CONFIG_X86 - else if (vmf->pgoff == KVM_PIO_PAGE_OFFSET) - page = virt_to_page(vcpu->arch.pio_data); -#endif #ifdef CONFIG_KVM_MMIO else if (vmf->pgoff == KVM_COALESCED_MMIO_PAGE_OFFSET) page = virt_to_page(vcpu->kvm->coalesced_mmio_ring); -- 2.31.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: x86: move architecture-specific code into kvm_arch_vcpu_fault 2021-08-11 3:17 [PATCH] kvm: x86: move architecture-specific code into kvm_arch_vcpu_fault Hou Wenlong @ 2021-08-11 18:52 ` Sean Christopherson 2021-08-12 4:02 ` [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer Hou Wenlong 0 siblings, 1 reply; 7+ messages in thread From: Sean Christopherson @ 2021-08-11 18:52 UTC (permalink / raw) To: Hou Wenlong Cc: kvm, Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-kernel On Wed, Aug 11, 2021, Hou Wenlong wrote: > The function kvm_arch_vcpu_fault can handle architecture-specific > case, so move pio-data fault case into it for x86. > > Signed-off-by: Hou Wenlong <houwenlong93@linux.alibaba.com> > --- > arch/x86/kvm/x86.c | 8 ++++++++ > virt/kvm/kvm_main.c | 4 ---- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index e5d5c5ed7dd4..30b0706eced8 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5348,6 +5348,14 @@ long kvm_arch_vcpu_ioctl(struct file *filp, > > vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > { > + if (vmf->pgoff == KVM_PIO_PAGE_OFFSET) { > + struct page *page = virt_to_page(vcpu->arch.pio_data); > + > + get_page(page); > + vmf->page = page; > + return 0; > + } > + > return VM_FAULT_SIGBUS; What about a prep patch (below) to refactor kvm_arch_vcpu_fault() to return a struct page pointer instead of vm_fault_t? That would simplify this patch to: diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 8c1871f0211c..1c5d68ced3be 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5348,6 +5348,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp, struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { + if (vmf->pgoff == KVM_PIO_PAGE_OFFSET) + virt_to_page(vcpu->arch.pio_data); return NULL; } diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index a75848799712..c3b1e8f55251 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3486,10 +3486,6 @@ static vm_fault_t kvm_vcpu_fault(struct vm_fault *vmf) if (vmf->pgoff == 0) page = virt_to_page(vcpu->run); -#ifdef CONFIG_X86 - else if (vmf->pgoff == KVM_PIO_PAGE_OFFSET) - page = virt_to_page(vcpu->arch.pio_data); -#endif #ifdef CONFIG_KVM_MMIO else if (vmf->pgoff == KVM_COALESCED_MMIO_PAGE_OFFSET) page = virt_to_page(vcpu->kvm->coalesced_mmio_ring); From 94ce06a001af4bd79635f1c8d681f560bca13cba Mon Sep 17 00:00:00 2001 From: Sean Christopherson <seanjc@google.com> Date: Wed, 11 Aug 2021 11:28:03 -0700 Subject: [PATCH] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer Refactor kvm_arch_vcpu_fault() to return 'struct page *' instead of 'vm_fault_t' to simplify architecture specific implementations that do more than return SIGBUS. Currently this only applies to s390, but a future patch will move x86's pio_data handling into x86 where it belongs. No functional changed intended. Cc: Hou Wenlong <houwenlong93@linux.alibaba.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/arm64/kvm/arm.c | 4 ++-- arch/mips/kvm/mips.c | 4 ++-- arch/powerpc/kvm/powerpc.c | 4 ++-- arch/s390/kvm/kvm-s390.c | 12 ++++-------- arch/x86/kvm/x86.c | 4 ++-- include/linux/kvm_host.h | 2 +- virt/kvm/kvm_main.c | 5 ++++- 7 files changed, 17 insertions(+), 18 deletions(-) diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index e9a2b8f27792..83f4ffe3e4f2 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -161,9 +161,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) return ret; } -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { - return VM_FAULT_SIGBUS; + return NULL; } diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c index 75c6f264c626..ecfbcfe00b78 100644 --- a/arch/mips/kvm/mips.c +++ b/arch/mips/kvm/mips.c @@ -1049,9 +1049,9 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) return -ENOIOCTLCMD; } -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { - return VM_FAULT_SIGBUS; + return NULL; } int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index be33b5321a76..b9c21f9ab784 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -2090,9 +2090,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp, return r; } -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { - return VM_FAULT_SIGBUS; + return NULL; } static int kvm_vm_ioctl_get_pvinfo(struct kvm_ppc_pvinfo *pvinfo) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 4dc7e966a720..eab9c4820185 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -4975,17 +4975,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp, return r; } -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { #ifdef CONFIG_KVM_S390_UCONTROL - if ((vmf->pgoff == KVM_S390_SIE_PAGE_OFFSET) - && (kvm_is_ucontrol(vcpu->kvm))) { - vmf->page = virt_to_page(vcpu->arch.sie_block); - get_page(vmf->page); - return 0; - } + if (vmf->pgoff == KVM_S390_SIE_PAGE_OFFSET && kvm_is_ucontrol(vcpu->kvm)) + return virt_to_page(vcpu->arch.sie_block); #endif - return VM_FAULT_SIGBUS; + return NULL; } /* Section: memory related */ diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index fdc0c18339fb..8c1871f0211c 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5346,9 +5346,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp, return r; } -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { - return VM_FAULT_SIGBUS; + return NULL; } static int kvm_vm_ioctl_set_tss_addr(struct kvm *kvm, unsigned long addr) diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 03f37e4def6f..7df730cdc935 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -1000,7 +1000,7 @@ long kvm_arch_dev_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg); long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg); -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf); +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf); int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 3e67c93ca403..a75848799712 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3499,7 +3499,10 @@ static vm_fault_t kvm_vcpu_fault(struct vm_fault *vmf) &vcpu->dirty_ring, vmf->pgoff - KVM_DIRTY_LOG_PAGE_OFFSET); else - return kvm_arch_vcpu_fault(vcpu, vmf); + page = kvm_arch_vcpu_fault(vcpu, vmf); + if (!page) + return VM_FAULT_SIGBUS; + get_page(page); vmf->page = page; return 0; -- 2.33.0.rc1.237.g0d66db33f3-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer 2021-08-11 18:52 ` Sean Christopherson @ 2021-08-12 4:02 ` Hou Wenlong 2021-08-12 4:02 ` [PATCH v2 2/2] kvm: x86: move architecture specific code into kvm_arch_vcpu_fault Hou Wenlong 2021-08-12 9:04 ` [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer David Hildenbrand 0 siblings, 2 replies; 7+ messages in thread From: Hou Wenlong @ 2021-08-12 4:02 UTC (permalink / raw) To: kvm Cc: Sean Christopherson, Marc Zyngier, James Morse, Alexandru Elisei, Suzuki K Poulose, Catalin Marinas, Will Deacon, Huacai Chen, Aleksandar Markovic, Thomas Bogendoerfer, Paul Mackerras, Michael Ellerman, Benjamin Herrenschmidt, Christian Borntraeger, Janosch Frank, David Hildenbrand, Cornelia Huck, Claudio Imbrenda, Heiko Carstens, Vasily Gorbik, Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-arm-kernel, kvmarm, linux-kernel, linux-mips, kvm-ppc, linuxppc-dev, linux-s390 From: Sean Christopherson <seanjc@google.com> Refactor kvm_arch_vcpu_fault() to return 'struct page *' instead of 'vm_fault_t' to simplify architecture specific implementations that do more than return SIGBUS. Currently this only applies to s390, but a future patch will move x86's pio_data handling into x86 where it belongs. No functional changed intended. Cc: Hou Wenlong <houwenlong93@linux.alibaba.com> Signed-off-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Hou Wenlong <houwenlong93@linux.alibaba.com> --- arch/arm64/kvm/arm.c | 4 ++-- arch/mips/kvm/mips.c | 4 ++-- arch/powerpc/kvm/powerpc.c | 4 ++-- arch/s390/kvm/kvm-s390.c | 12 ++++-------- arch/x86/kvm/x86.c | 4 ++-- include/linux/kvm_host.h | 2 +- virt/kvm/kvm_main.c | 5 ++++- 7 files changed, 17 insertions(+), 18 deletions(-) diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index e9a2b8f27792..83f4ffe3e4f2 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -161,9 +161,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) return ret; } -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { - return VM_FAULT_SIGBUS; + return NULL; } diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c index af9dd029a4e1..ae79874e6fd2 100644 --- a/arch/mips/kvm/mips.c +++ b/arch/mips/kvm/mips.c @@ -1053,9 +1053,9 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) return -ENOIOCTLCMD; } -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { - return VM_FAULT_SIGBUS; + return NULL; } int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index be33b5321a76..b9c21f9ab784 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -2090,9 +2090,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp, return r; } -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { - return VM_FAULT_SIGBUS; + return NULL; } static int kvm_vm_ioctl_get_pvinfo(struct kvm_ppc_pvinfo *pvinfo) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 02574d7b3612..e1b69833e228 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -4979,17 +4979,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp, return r; } -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { #ifdef CONFIG_KVM_S390_UCONTROL - if ((vmf->pgoff == KVM_S390_SIE_PAGE_OFFSET) - && (kvm_is_ucontrol(vcpu->kvm))) { - vmf->page = virt_to_page(vcpu->arch.sie_block); - get_page(vmf->page); - return 0; - } + if (vmf->pgoff == KVM_S390_SIE_PAGE_OFFSET && kvm_is_ucontrol(vcpu->kvm)) + return virt_to_page(vcpu->arch.sie_block); #endif - return VM_FAULT_SIGBUS; + return NULL; } /* Section: memory related */ diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 3cedc7cc132a..1e3bbe5cd33a 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5347,9 +5347,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp, return r; } -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { - return VM_FAULT_SIGBUS; + return NULL; } static int kvm_vm_ioctl_set_tss_addr(struct kvm *kvm, unsigned long addr) diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 492d183dd7d0..a949de534722 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -995,7 +995,7 @@ long kvm_arch_dev_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg); long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg); -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf); +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf); int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 30d322519253..f7d21418971b 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3448,7 +3448,10 @@ static vm_fault_t kvm_vcpu_fault(struct vm_fault *vmf) &vcpu->dirty_ring, vmf->pgoff - KVM_DIRTY_LOG_PAGE_OFFSET); else - return kvm_arch_vcpu_fault(vcpu, vmf); + page = kvm_arch_vcpu_fault(vcpu, vmf); + if (!page) + return VM_FAULT_SIGBUS; + get_page(page); vmf->page = page; return 0; -- 2.31.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] kvm: x86: move architecture specific code into kvm_arch_vcpu_fault 2021-08-12 4:02 ` [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer Hou Wenlong @ 2021-08-12 4:02 ` Hou Wenlong 2021-08-12 9:04 ` [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer David Hildenbrand 1 sibling, 0 replies; 7+ messages in thread From: Hou Wenlong @ 2021-08-12 4:02 UTC (permalink / raw) To: kvm Cc: Sean Christopherson, Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-kernel The function kvm_arch_vcpu_fault can handle architecture specific case, so move x86's pio_data hanlding into it. Suggested-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Hou Wenlong <houwenlong93@linux.alibaba.com> --- arch/x86/kvm/x86.c | 2 ++ virt/kvm/kvm_main.c | 4 ---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 1e3bbe5cd33a..25c0752d6cd9 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5349,6 +5349,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp, struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { + if (vmf->pgoff == KVM_PIO_PAGE_OFFSET) + return virt_to_page(vcpu->arch.pio_data); return NULL; } diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index f7d21418971b..43064df5ad87 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3435,10 +3435,6 @@ static vm_fault_t kvm_vcpu_fault(struct vm_fault *vmf) if (vmf->pgoff == 0) page = virt_to_page(vcpu->run); -#ifdef CONFIG_X86 - else if (vmf->pgoff == KVM_PIO_PAGE_OFFSET) - page = virt_to_page(vcpu->arch.pio_data); -#endif #ifdef CONFIG_KVM_MMIO else if (vmf->pgoff == KVM_COALESCED_MMIO_PAGE_OFFSET) page = virt_to_page(vcpu->kvm->coalesced_mmio_ring); -- 2.31.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer 2021-08-12 4:02 ` [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer Hou Wenlong 2021-08-12 4:02 ` [PATCH v2 2/2] kvm: x86: move architecture specific code into kvm_arch_vcpu_fault Hou Wenlong @ 2021-08-12 9:04 ` David Hildenbrand 2021-08-12 15:41 ` Paolo Bonzini 2021-08-23 14:12 ` Christian Borntraeger 1 sibling, 2 replies; 7+ messages in thread From: David Hildenbrand @ 2021-08-12 9:04 UTC (permalink / raw) To: Hou Wenlong, kvm Cc: Sean Christopherson, Marc Zyngier, James Morse, Alexandru Elisei, Suzuki K Poulose, Catalin Marinas, Will Deacon, Huacai Chen, Aleksandar Markovic, Thomas Bogendoerfer, Paul Mackerras, Michael Ellerman, Benjamin Herrenschmidt, Christian Borntraeger, Janosch Frank, Cornelia Huck, Claudio Imbrenda, Heiko Carstens, Vasily Gorbik, Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-arm-kernel, kvmarm, linux-kernel, linux-mips, kvm-ppc, linuxppc-dev, linux-s390 On 12.08.21 06:02, Hou Wenlong wrote: > From: Sean Christopherson <seanjc@google.com> > > Refactor kvm_arch_vcpu_fault() to return 'struct page *' instead of > 'vm_fault_t' to simplify architecture specific implementations that do > more than return SIGBUS. Currently this only applies to s390, but a > future patch will move x86's pio_data handling into x86 where it belongs. > > No functional changed intended. > > Cc: Hou Wenlong <houwenlong93@linux.alibaba.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Hou Wenlong <houwenlong93@linux.alibaba.com> > --- > arch/arm64/kvm/arm.c | 4 ++-- > arch/mips/kvm/mips.c | 4 ++-- > arch/powerpc/kvm/powerpc.c | 4 ++-- > arch/s390/kvm/kvm-s390.c | 12 ++++-------- > arch/x86/kvm/x86.c | 4 ++-- > include/linux/kvm_host.h | 2 +- > virt/kvm/kvm_main.c | 5 ++++- > 7 files changed, 17 insertions(+), 18 deletions(-) > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index e9a2b8f27792..83f4ffe3e4f2 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -161,9 +161,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) > return ret; > } > > -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > { > - return VM_FAULT_SIGBUS; > + return NULL; > } > > > diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c > index af9dd029a4e1..ae79874e6fd2 100644 > --- a/arch/mips/kvm/mips.c > +++ b/arch/mips/kvm/mips.c > @@ -1053,9 +1053,9 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) > return -ENOIOCTLCMD; > } > > -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > { > - return VM_FAULT_SIGBUS; > + return NULL; > } > > int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index be33b5321a76..b9c21f9ab784 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -2090,9 +2090,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp, > return r; > } > > -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > { > - return VM_FAULT_SIGBUS; > + return NULL; > } > > static int kvm_vm_ioctl_get_pvinfo(struct kvm_ppc_pvinfo *pvinfo) > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 02574d7b3612..e1b69833e228 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -4979,17 +4979,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp, > return r; > } > > -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > { > #ifdef CONFIG_KVM_S390_UCONTROL > - if ((vmf->pgoff == KVM_S390_SIE_PAGE_OFFSET) > - && (kvm_is_ucontrol(vcpu->kvm))) { > - vmf->page = virt_to_page(vcpu->arch.sie_block); > - get_page(vmf->page); > - return 0; > - } > + if (vmf->pgoff == KVM_S390_SIE_PAGE_OFFSET && kvm_is_ucontrol(vcpu->kvm)) > + return virt_to_page(vcpu->arch.sie_block); > #endif > - return VM_FAULT_SIGBUS; > + return NULL; > } > > /* Section: memory related */ > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 3cedc7cc132a..1e3bbe5cd33a 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5347,9 +5347,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp, > return r; > } > > -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > { > - return VM_FAULT_SIGBUS; > + return NULL; > } > > static int kvm_vm_ioctl_set_tss_addr(struct kvm *kvm, unsigned long addr) > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 492d183dd7d0..a949de534722 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -995,7 +995,7 @@ long kvm_arch_dev_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg); > long kvm_arch_vcpu_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg); > -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf); > +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf); > > int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext); > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 30d322519253..f7d21418971b 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -3448,7 +3448,10 @@ static vm_fault_t kvm_vcpu_fault(struct vm_fault *vmf) > &vcpu->dirty_ring, > vmf->pgoff - KVM_DIRTY_LOG_PAGE_OFFSET); > else > - return kvm_arch_vcpu_fault(vcpu, vmf); > + page = kvm_arch_vcpu_fault(vcpu, vmf); > + if (!page) > + return VM_FAULT_SIGBUS; > + > get_page(page); > vmf->page = page; > return 0; > Reviewed-by: David Hildenbrand <david@redhat.com> But at the same time I wonder if we should just get rid of CONFIG_KVM_S390_UCONTROL and consequently kvm_arch_vcpu_fault(). In practice CONFIG_KVM_S390_UCONTROL, is never enabled in any reasonable kernel build and consequently it's never tested; further, exposing the sie_block to user space allows user space to generate random SIE validity intercepts. CONFIG_KVM_S390_UCONTROL feels like something that should just be maintained out of tree by someone who really needs to hack deep into hw virtualization for testing purposes etc. -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer 2021-08-12 9:04 ` [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer David Hildenbrand @ 2021-08-12 15:41 ` Paolo Bonzini 2021-08-23 14:12 ` Christian Borntraeger 1 sibling, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2021-08-12 15:41 UTC (permalink / raw) To: David Hildenbrand, Hou Wenlong, kvm Cc: Sean Christopherson, Marc Zyngier, James Morse, Alexandru Elisei, Suzuki K Poulose, Catalin Marinas, Will Deacon, Huacai Chen, Aleksandar Markovic, Thomas Bogendoerfer, Paul Mackerras, Michael Ellerman, Benjamin Herrenschmidt, Christian Borntraeger, Janosch Frank, Cornelia Huck, Claudio Imbrenda, Heiko Carstens, Vasily Gorbik, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-arm-kernel, kvmarm, linux-kernel, linux-mips, kvm-ppc, linuxppc-dev, linux-s390 On 12/08/21 11:04, David Hildenbrand wrote: > > Reviewed-by: David Hildenbrand <david@redhat.com> > > But at the same time I wonder if we should just get rid of > CONFIG_KVM_S390_UCONTROL and consequently kvm_arch_vcpu_fault(). > > In practice CONFIG_KVM_S390_UCONTROL, is never enabled in any reasonable > kernel build and consequently it's never tested; further, exposing the > sie_block to user space allows user space to generate random SIE > validity intercepts. > > CONFIG_KVM_S390_UCONTROL feels like something that should just be > maintained out of tree by someone who really needs to hack deep into hw > virtualization for testing purposes etc. I have no preference either way. It should definitely have selftests, but in x86 land there are some features that are not covered by QEMU and were nevertheless accepted upstream with selftests. Paolo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer 2021-08-12 9:04 ` [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer David Hildenbrand 2021-08-12 15:41 ` Paolo Bonzini @ 2021-08-23 14:12 ` Christian Borntraeger 1 sibling, 0 replies; 7+ messages in thread From: Christian Borntraeger @ 2021-08-23 14:12 UTC (permalink / raw) To: David Hildenbrand, Hou Wenlong, kvm Cc: Sean Christopherson, Marc Zyngier, James Morse, Alexandru Elisei, Suzuki K Poulose, Catalin Marinas, Will Deacon, Huacai Chen, Aleksandar Markovic, Thomas Bogendoerfer, Paul Mackerras, Michael Ellerman, Benjamin Herrenschmidt, Janosch Frank, Cornelia Huck, Claudio Imbrenda, Heiko Carstens, Vasily Gorbik, Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-arm-kernel, kvmarm, linux-kernel, linux-mips, kvm-ppc, linuxppc-dev, linux-s390 On 12.08.21 11:04, David Hildenbrand wrote: > On 12.08.21 06:02, Hou Wenlong wrote: >> From: Sean Christopherson <seanjc@google.com> >> >> Refactor kvm_arch_vcpu_fault() to return 'struct page *' instead of >> 'vm_fault_t' to simplify architecture specific implementations that do >> more than return SIGBUS. Currently this only applies to s390, but a >> future patch will move x86's pio_data handling into x86 where it belongs. >> >> No functional changed intended. >> >> Cc: Hou Wenlong <houwenlong93@linux.alibaba.com> >> Signed-off-by: Sean Christopherson <seanjc@google.com> >> Signed-off-by: Hou Wenlong <houwenlong93@linux.alibaba.com> >> --- >> arch/arm64/kvm/arm.c | 4 ++-- >> arch/mips/kvm/mips.c | 4 ++-- >> arch/powerpc/kvm/powerpc.c | 4 ++-- >> arch/s390/kvm/kvm-s390.c | 12 ++++-------- >> arch/x86/kvm/x86.c | 4 ++-- >> include/linux/kvm_host.h | 2 +- >> virt/kvm/kvm_main.c | 5 ++++- >> 7 files changed, 17 insertions(+), 18 deletions(-) >> >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c >> index e9a2b8f27792..83f4ffe3e4f2 100644 >> --- a/arch/arm64/kvm/arm.c >> +++ b/arch/arm64/kvm/arm.c >> @@ -161,9 +161,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) >> return ret; >> } >> -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) >> +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) >> { >> - return VM_FAULT_SIGBUS; >> + return NULL; >> } >> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c >> index af9dd029a4e1..ae79874e6fd2 100644 >> --- a/arch/mips/kvm/mips.c >> +++ b/arch/mips/kvm/mips.c >> @@ -1053,9 +1053,9 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) >> return -ENOIOCTLCMD; >> } >> -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) >> +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) >> { >> - return VM_FAULT_SIGBUS; >> + return NULL; >> } >> int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) >> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c >> index be33b5321a76..b9c21f9ab784 100644 >> --- a/arch/powerpc/kvm/powerpc.c >> +++ b/arch/powerpc/kvm/powerpc.c >> @@ -2090,9 +2090,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp, >> return r; >> } >> -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) >> +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) >> { >> - return VM_FAULT_SIGBUS; >> + return NULL; >> } >> static int kvm_vm_ioctl_get_pvinfo(struct kvm_ppc_pvinfo *pvinfo) >> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >> index 02574d7b3612..e1b69833e228 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -4979,17 +4979,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp, >> return r; >> } >> -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) >> +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) >> { >> #ifdef CONFIG_KVM_S390_UCONTROL >> - if ((vmf->pgoff == KVM_S390_SIE_PAGE_OFFSET) >> - && (kvm_is_ucontrol(vcpu->kvm))) { >> - vmf->page = virt_to_page(vcpu->arch.sie_block); >> - get_page(vmf->page); >> - return 0; >> - } >> + if (vmf->pgoff == KVM_S390_SIE_PAGE_OFFSET && kvm_is_ucontrol(vcpu->kvm)) >> + return virt_to_page(vcpu->arch.sie_block); >> #endif >> - return VM_FAULT_SIGBUS; >> + return NULL; >> } >> /* Section: memory related */ >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 3cedc7cc132a..1e3bbe5cd33a 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -5347,9 +5347,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp, >> return r; >> } >> -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) >> +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) >> { >> - return VM_FAULT_SIGBUS; >> + return NULL; >> } >> static int kvm_vm_ioctl_set_tss_addr(struct kvm *kvm, unsigned long addr) >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index 492d183dd7d0..a949de534722 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -995,7 +995,7 @@ long kvm_arch_dev_ioctl(struct file *filp, >> unsigned int ioctl, unsigned long arg); >> long kvm_arch_vcpu_ioctl(struct file *filp, >> unsigned int ioctl, unsigned long arg); >> -vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf); >> +struct page *kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf); >> int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext); >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >> index 30d322519253..f7d21418971b 100644 >> --- a/virt/kvm/kvm_main.c >> +++ b/virt/kvm/kvm_main.c >> @@ -3448,7 +3448,10 @@ static vm_fault_t kvm_vcpu_fault(struct vm_fault *vmf) >> &vcpu->dirty_ring, >> vmf->pgoff - KVM_DIRTY_LOG_PAGE_OFFSET); >> else >> - return kvm_arch_vcpu_fault(vcpu, vmf); >> + page = kvm_arch_vcpu_fault(vcpu, vmf); >> + if (!page) >> + return VM_FAULT_SIGBUS; >> + >> get_page(page); >> vmf->page = page; >> return 0; >> > > Reviewed-by: David Hildenbrand <david@redhat.com> > > But at the same time I wonder if we should just get rid of CONFIG_KVM_S390_UCONTROL and consequently kvm_arch_vcpu_fault(). > > > In practice CONFIG_KVM_S390_UCONTROL, is never enabled in any reasonable kernel build and consequently it's never tested; further, exposing the sie_block to user space allows user space to generate random SIE validity intercepts. > > CONFIG_KVM_S390_UCONTROL feels like something that should just be maintained out of tree by someone who really needs to hack deep into hw virtualization for testing purposes etc. I recently talked to the ucontrol users and they will look into selftests. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-23 14:14 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-11 3:17 [PATCH] kvm: x86: move architecture-specific code into kvm_arch_vcpu_fault Hou Wenlong 2021-08-11 18:52 ` Sean Christopherson 2021-08-12 4:02 ` [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer Hou Wenlong 2021-08-12 4:02 ` [PATCH v2 2/2] kvm: x86: move architecture specific code into kvm_arch_vcpu_fault Hou Wenlong 2021-08-12 9:04 ` [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer David Hildenbrand 2021-08-12 15:41 ` Paolo Bonzini 2021-08-23 14:12 ` Christian Borntraeger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).