* [PATCH v2 1/2] KVM: Refactor kvm_arch_vcpu_fault() to return a struct page pointer
[not found] <YRQcZqCWwVH8bCGc@google.com>
@ 2021-08-12 4:02 ` Hou Wenlong
2021-08-12 9:04 ` David Hildenbrand
0 siblings, 1 reply; 4+ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 4+ 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 9:04 ` David Hildenbrand
2021-08-12 15:41 ` Paolo Bonzini
2021-08-23 14:12 ` Christian Borntraeger
0 siblings, 2 replies; 4+ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ 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 ` David Hildenbrand
@ 2021-08-12 15:41 ` Paolo Bonzini
2021-08-23 14:12 ` Christian Borntraeger
1 sibling, 0 replies; 4+ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ 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 ` David Hildenbrand
2021-08-12 15:41 ` Paolo Bonzini
@ 2021-08-23 14:12 ` Christian Borntraeger
1 sibling, 0 replies; 4+ 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.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-23 14:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <YRQcZqCWwVH8bCGc@google.com>
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 9:04 ` 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).