lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 28 Feb 2020 10:05:03 -0800
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Peter Xu <peterx@...hat.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>, jianjay.zhou@...wei.com
Subject: Re: [PATCH] KVM: Remove unecessary asm/kvm_host.h includes

s/unecessary/unnecessary

On Wed, Feb 26, 2020 at 10:55:58AM -0500, Peter Xu wrote:
> linux/kvm_host.h and asm/kvm_host.h have a dependency in that the asm
> header should be included first, then we can define arch-specific
> macros in asm/ header and use "#ifndef" in linux/ header to define the
> generic value of the macro.  One example is KVM_MAX_VCPU_ID.
> 
> Now in many C files we've got both the headers included, and
> linux/kvm_host.h is included even earlier.  It's working only because
> in linux/kvm_host.h we also included asm/kvm_host.h anyway so the
> explicit inclusion of asm/kvm_host.h in the C files are meaningless.

I'd prefer to word this much more strongly, i.e. there is no "should"
about it, including asm/kvm_host.h in linux/kvm_host.h is deliberate, 
it's not serendipitous.

```
Remove includes of asm/kvm_host.h from files that already include
linux/kvm_host.h to make it more obvious that there is no ordering issue
between the two headers.  linux/kvm_host.h includes asm/kvm_host.h to
pick up architecture specific settings, and this will never change, i.e.
including asm/kvm_host.h after linux/kvm_host.h may seem problematic,
but in practice is simply redundant.
```

As for the change itself, I'm indifferent.

> To avoid the confusion, remove the asm/ header if the linux/ header is
> included.
> 
> Signed-off-by: Peter Xu <peterx@...hat.com>
> ---
>  arch/arm/kvm/coproc.c                | 1 -
>  arch/arm64/kvm/fpsimd.c              | 1 -
>  arch/arm64/kvm/guest.c               | 1 -
>  arch/arm64/kvm/hyp/switch.c          | 1 -
>  arch/arm64/kvm/sys_regs.c            | 1 -
>  arch/arm64/kvm/sys_regs_generic_v8.c | 1 -
>  arch/powerpc/kvm/book3s_64_vio.c     | 1 -
>  arch/powerpc/kvm/book3s_64_vio_hv.c  | 1 -
>  arch/powerpc/kvm/book3s_hv.c         | 1 -
>  arch/powerpc/kvm/mpic.c              | 1 -
>  arch/powerpc/kvm/powerpc.c           | 1 -
>  arch/powerpc/kvm/timing.h            | 1 -
>  arch/s390/kvm/intercept.c            | 1 -
>  arch/x86/kvm/mmu/page_track.c        | 1 -
>  virt/kvm/arm/psci.c                  | 1 -
>  15 files changed, 15 deletions(-)
> 
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 07745ee022a1..f0c09049ee99 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -10,7 +10,6 @@
>  #include <linux/kvm_host.h>
>  #include <linux/uaccess.h>
>  #include <asm/kvm_arm.h>
> -#include <asm/kvm_host.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_coproc.h>
>  #include <asm/kvm_mmu.h>
> diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> index 525010504f9d..e329a36b2bee 100644
> --- a/arch/arm64/kvm/fpsimd.c
> +++ b/arch/arm64/kvm/fpsimd.c
> @@ -11,7 +11,6 @@
>  #include <linux/kvm_host.h>
>  #include <asm/fpsimd.h>
>  #include <asm/kvm_asm.h>
> -#include <asm/kvm_host.h>
>  #include <asm/kvm_mmu.h>
>  #include <asm/sysreg.h>
>  
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 2bd92301d32f..23ebe51410f0 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -25,7 +25,6 @@
>  #include <asm/kvm.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_coproc.h>
> -#include <asm/kvm_host.h>
>  #include <asm/sigcontext.h>
>  
>  #include "trace.h"
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index dfe8dd172512..f3e0ab961565 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -17,7 +17,6 @@
>  #include <asm/kprobes.h>
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_emulate.h>
> -#include <asm/kvm_host.h>
>  #include <asm/kvm_hyp.h>
>  #include <asm/kvm_mmu.h>
>  #include <asm/fpsimd.h>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 3e909b117f0c..b95f7b7743c8 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -22,7 +22,6 @@
>  #include <asm/kvm_arm.h>
>  #include <asm/kvm_coproc.h>
>  #include <asm/kvm_emulate.h>
> -#include <asm/kvm_host.h>
>  #include <asm/kvm_hyp.h>
>  #include <asm/kvm_mmu.h>
>  #include <asm/perf_event.h>
> diff --git a/arch/arm64/kvm/sys_regs_generic_v8.c b/arch/arm64/kvm/sys_regs_generic_v8.c
> index 2b4a3e2d1b89..9cb6b4c8355a 100644
> --- a/arch/arm64/kvm/sys_regs_generic_v8.c
> +++ b/arch/arm64/kvm/sys_regs_generic_v8.c
> @@ -12,7 +12,6 @@
>  #include <asm/cputype.h>
>  #include <asm/kvm_arm.h>
>  #include <asm/kvm_asm.h>
> -#include <asm/kvm_host.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_coproc.h>
>  #include <asm/sysreg.h>
> diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
> index ee6c103bb7d5..50555ad1db93 100644
> --- a/arch/powerpc/kvm/book3s_64_vio.c
> +++ b/arch/powerpc/kvm/book3s_64_vio.c
> @@ -27,7 +27,6 @@
>  #include <asm/hvcall.h>
>  #include <asm/synch.h>
>  #include <asm/ppc-opcode.h>
> -#include <asm/kvm_host.h>
>  #include <asm/udbg.h>
>  #include <asm/iommu.h>
>  #include <asm/tce.h>
> diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
> index ab6eeb8e753e..6fcaf1fa8e02 100644
> --- a/arch/powerpc/kvm/book3s_64_vio_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
> @@ -24,7 +24,6 @@
>  #include <asm/hvcall.h>
>  #include <asm/synch.h>
>  #include <asm/ppc-opcode.h>
> -#include <asm/kvm_host.h>
>  #include <asm/udbg.h>
>  #include <asm/iommu.h>
>  #include <asm/tce.h>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 2cefd071b848..f065d6956342 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -72,7 +72,6 @@
>  #include <asm/xics.h>
>  #include <asm/xive.h>
>  #include <asm/hw_breakpoint.h>
> -#include <asm/kvm_host.h>
>  #include <asm/kvm_book3s_uvmem.h>
>  #include <asm/ultravisor.h>
>  
> diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c
> index fe312c160d97..23e9c2bd9f27 100644
> --- a/arch/powerpc/kvm/mpic.c
> +++ b/arch/powerpc/kvm/mpic.c
> @@ -32,7 +32,6 @@
>  #include <linux/uaccess.h>
>  #include <asm/mpic.h>
>  #include <asm/kvm_para.h>
> -#include <asm/kvm_host.h>
>  #include <asm/kvm_ppc.h>
>  #include <kvm/iodev.h>
>  
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 1af96fb5dc6f..c1f23cb4206c 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -32,7 +32,6 @@
>  #include <asm/plpar_wrappers.h>
>  #endif
>  #include <asm/ultravisor.h>
> -#include <asm/kvm_host.h>
>  
>  #include "timing.h"
>  #include "irq.h"
> diff --git a/arch/powerpc/kvm/timing.h b/arch/powerpc/kvm/timing.h
> index ace65f9fed30..feef7885ba82 100644
> --- a/arch/powerpc/kvm/timing.h
> +++ b/arch/powerpc/kvm/timing.h
> @@ -10,7 +10,6 @@
>  #define __POWERPC_KVM_EXITTIMING_H__
>  
>  #include <linux/kvm_host.h>
> -#include <asm/kvm_host.h>
>  
>  #ifdef CONFIG_KVM_EXIT_TIMING
>  void kvmppc_init_timing_stats(struct kvm_vcpu *vcpu);
> diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
> index a389fa85cca2..3655196f1c03 100644
> --- a/arch/s390/kvm/intercept.c
> +++ b/arch/s390/kvm/intercept.c
> @@ -12,7 +12,6 @@
>  #include <linux/errno.h>
>  #include <linux/pagemap.h>
>  
> -#include <asm/kvm_host.h>
>  #include <asm/asm-offsets.h>
>  #include <asm/irq.h>
>  #include <asm/sysinfo.h>
> diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
> index 3521e2d176f2..0713778b8e12 100644
> --- a/arch/x86/kvm/mmu/page_track.c
> +++ b/arch/x86/kvm/mmu/page_track.c
> @@ -14,7 +14,6 @@
>  #include <linux/kvm_host.h>
>  #include <linux/rculist.h>
>  
> -#include <asm/kvm_host.h>
>  #include <asm/kvm_page_track.h>
>  
>  #include "mmu.h"
> diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
> index 17e2bdd4b76f..14a162e295a9 100644
> --- a/virt/kvm/arm/psci.c
> +++ b/virt/kvm/arm/psci.c
> @@ -12,7 +12,6 @@
>  
>  #include <asm/cputype.h>
>  #include <asm/kvm_emulate.h>
> -#include <asm/kvm_host.h>
>  
>  #include <kvm/arm_psci.h>
>  #include <kvm/arm_hypercalls.h>
> -- 
> 2.24.1
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ