[PATCH 0/5] KVM: PPC: Book3S: Modules cleanup and unification

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/5] KVM: PPC: Book3S: Modules cleanup and unification
@ 2021-09-01 17:33 ` Fabiano Rosas
  0 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

This series merges our three kvm modules kvm.ko, kvm-hv.ko and
kvm-pr.ko into one kvm.ko module.

The main reason for this is to deal with the issue that kvm.ko can be
loaded on its own without any of the other modules present. This can
happen if one or both of the modules fail to init or if the user loads
kvm.ko only.

With only kvm.ko loaded, the userspace can call any of the KVM ioctls
which will fail more or less gracefully depending on what kind of
verification we do in powerpc.c.

Instead of adding a check to every entry point or finding a hack to
link the modules so that when one fails (hv/pr), the other (kvm)
exits, I think it is cleaner to just make them all a single module.

The two KVM implementations are already selected by Kconfig options,
so the only thing that changes is that they are now in the same
module. I also kept kvm-hv and kvm-pr as aliases to kvm, so that
people don't get too surprised with the change.

There is a possible issue with the larger module size for kernel
builds that should support both HV-only and PR-only environments, but
PR is usually not used in production so I'm not sure if that is a real
issue.

Patches 1,2,3 are standalone cleanups.
Patches 4,5 are the unification work.

Fabiano Rosas (5):
  KVM: PPC: Book3S HV: Check return value of kvmppc_radix_init
  KVM: PPC: Book3S HV: Delay setting of kvm ops
  KVM: PPC: Book3S HV: Free allocated memory if module init fails
  KVM: PPC: Book3S: Unify kvm-hv and kvm-pr modules
  KVM: PPC: Book3S: Stop exporting non-builtin symbols

 arch/powerpc/configs/powernv_defconfig |  2 +-
 arch/powerpc/configs/ppc64_defconfig   |  2 +-
 arch/powerpc/configs/pseries_defconfig |  2 +-
 arch/powerpc/kvm/Kconfig               | 72 ++++++++++++--------------
 arch/powerpc/kvm/Makefile              | 11 ++--
 arch/powerpc/kvm/book3s.c              | 61 ++++++++++++++--------
 arch/powerpc/kvm/book3s.h              | 19 +++++++
 arch/powerpc/kvm/book3s_64_mmu_radix.c |  3 --
 arch/powerpc/kvm/book3s_64_vio.c       |  3 --
 arch/powerpc/kvm/book3s_hv.c           | 38 ++++++++------
 arch/powerpc/kvm/book3s_pr.c           | 13 -----
 arch/powerpc/kvm/book3s_rtas.c         |  1 -
 arch/powerpc/kvm/book3s_xics.c         |  4 --
 arch/powerpc/kvm/book3s_xive.c         |  6 ---
 arch/powerpc/kvm/emulate.c             |  1 -
 arch/powerpc/kvm/powerpc.c             | 14 -----
 kernel/irq/irqdesc.c                   |  2 +-
 17 files changed, 125 insertions(+), 129 deletions(-)

-- 
2.29.2

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 0/5] KVM: PPC: Book3S: Modules cleanup and unification
@ 2021-09-01 17:33 ` Fabiano Rosas
  0 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

This series merges our three kvm modules kvm.ko, kvm-hv.ko and
kvm-pr.ko into one kvm.ko module.

The main reason for this is to deal with the issue that kvm.ko can be
loaded on its own without any of the other modules present. This can
happen if one or both of the modules fail to init or if the user loads
kvm.ko only.

With only kvm.ko loaded, the userspace can call any of the KVM ioctls
which will fail more or less gracefully depending on what kind of
verification we do in powerpc.c.

Instead of adding a check to every entry point or finding a hack to
link the modules so that when one fails (hv/pr), the other (kvm)
exits, I think it is cleaner to just make them all a single module.

The two KVM implementations are already selected by Kconfig options,
so the only thing that changes is that they are now in the same
module. I also kept kvm-hv and kvm-pr as aliases to kvm, so that
people don't get too surprised with the change.

There is a possible issue with the larger module size for kernel
builds that should support both HV-only and PR-only environments, but
PR is usually not used in production so I'm not sure if that is a real
issue.

Patches 1,2,3 are standalone cleanups.
Patches 4,5 are the unification work.

Fabiano Rosas (5):
  KVM: PPC: Book3S HV: Check return value of kvmppc_radix_init
  KVM: PPC: Book3S HV: Delay setting of kvm ops
  KVM: PPC: Book3S HV: Free allocated memory if module init fails
  KVM: PPC: Book3S: Unify kvm-hv and kvm-pr modules
  KVM: PPC: Book3S: Stop exporting non-builtin symbols

 arch/powerpc/configs/powernv_defconfig |  2 +-
 arch/powerpc/configs/ppc64_defconfig   |  2 +-
 arch/powerpc/configs/pseries_defconfig |  2 +-
 arch/powerpc/kvm/Kconfig               | 72 ++++++++++++--------------
 arch/powerpc/kvm/Makefile              | 11 ++--
 arch/powerpc/kvm/book3s.c              | 61 ++++++++++++++--------
 arch/powerpc/kvm/book3s.h              | 19 +++++++
 arch/powerpc/kvm/book3s_64_mmu_radix.c |  3 --
 arch/powerpc/kvm/book3s_64_vio.c       |  3 --
 arch/powerpc/kvm/book3s_hv.c           | 38 ++++++++------
 arch/powerpc/kvm/book3s_pr.c           | 13 -----
 arch/powerpc/kvm/book3s_rtas.c         |  1 -
 arch/powerpc/kvm/book3s_xics.c         |  4 --
 arch/powerpc/kvm/book3s_xive.c         |  6 ---
 arch/powerpc/kvm/emulate.c             |  1 -
 arch/powerpc/kvm/powerpc.c             | 14 -----
 kernel/irq/irqdesc.c                   |  2 +-
 17 files changed, 125 insertions(+), 129 deletions(-)

-- 
2.29.2

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/5] KVM: PPC: Book3S HV: Check return value of kvmppc_radix_init
  2021-09-01 17:33 ` Fabiano Rosas
@ 2021-09-01 17:33   ` Fabiano Rosas
  -1 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

The return of the function is being shadowed by the call to
kvmppc_uvmem_init.

Fixes: ca9f4942670c ("KVM: PPC: Book3S HV: Support for running secure guests")
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 085fb8ecbf68..4cdf2e6e1cf5 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5996,8 +5996,11 @@ static int kvmppc_book3s_init_hv(void)
 	if (r)
 		return r;
 
-	if (kvmppc_radix_possible())
+	if (kvmppc_radix_possible()) {
 		r = kvmppc_radix_init();
+		if (r)
+			return r;
+	}
 
 	r = kvmppc_uvmem_init();
 	if (r < 0)
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 1/5] KVM: PPC: Book3S HV: Check return value of kvmppc_radix_init
@ 2021-09-01 17:33   ` Fabiano Rosas
  0 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

The return of the function is being shadowed by the call to
kvmppc_uvmem_init.

Fixes: ca9f4942670c ("KVM: PPC: Book3S HV: Support for running secure guests")
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 085fb8ecbf68..4cdf2e6e1cf5 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5996,8 +5996,11 @@ static int kvmppc_book3s_init_hv(void)
 	if (r)
 		return r;
 
-	if (kvmppc_radix_possible())
+	if (kvmppc_radix_possible()) {
 		r = kvmppc_radix_init();
+		if (r)
+			return r;
+	}
 
 	r = kvmppc_uvmem_init();
 	if (r < 0)
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/5] KVM: PPC: Book3S HV: Delay setting of kvm ops
  2021-09-01 17:33 ` Fabiano Rosas
@ 2021-09-01 17:33   ` Fabiano Rosas
  -1 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

Delay the setting of kvm_hv_ops until after all init code has
completed. This avoids leaving the ops still accessible if the init
fails.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 4cdf2e6e1cf5..fe20074faa61 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5985,9 +5985,6 @@ static int kvmppc_book3s_init_hv(void)
 	}
 #endif
 
-	kvm_ops_hv.owner = THIS_MODULE;
-	kvmppc_hv_ops = &kvm_ops_hv;
-
 	init_default_hcalls();
 
 	init_vcore_lists();
@@ -6003,10 +6000,15 @@ static int kvmppc_book3s_init_hv(void)
 	}
 
 	r = kvmppc_uvmem_init();
-	if (r < 0)
+	if (r < 0) {
 		pr_err("KVM-HV: kvmppc_uvmem_init failed %d\n", r);
+		return r;
+	}
 
-	return r;
+	kvm_ops_hv.owner = THIS_MODULE;
+	kvmppc_hv_ops = &kvm_ops_hv;
+
+	return 0;
 }
 
 static void kvmppc_book3s_exit_hv(void)
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/5] KVM: PPC: Book3S HV: Delay setting of kvm ops
@ 2021-09-01 17:33   ` Fabiano Rosas
  0 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

Delay the setting of kvm_hv_ops until after all init code has
completed. This avoids leaving the ops still accessible if the init
fails.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 4cdf2e6e1cf5..fe20074faa61 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5985,9 +5985,6 @@ static int kvmppc_book3s_init_hv(void)
 	}
 #endif
 
-	kvm_ops_hv.owner = THIS_MODULE;
-	kvmppc_hv_ops = &kvm_ops_hv;
-
 	init_default_hcalls();
 
 	init_vcore_lists();
@@ -6003,10 +6000,15 @@ static int kvmppc_book3s_init_hv(void)
 	}
 
 	r = kvmppc_uvmem_init();
-	if (r < 0)
+	if (r < 0) {
 		pr_err("KVM-HV: kvmppc_uvmem_init failed %d\n", r);
+		return r;
+	}
 
-	return r;
+	kvm_ops_hv.owner = THIS_MODULE;
+	kvmppc_hv_ops = &kvm_ops_hv;
+
+	return 0;
 }
 
 static void kvmppc_book3s_exit_hv(void)
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/5] KVM: PPC: Book3S HV: Free allocated memory if module init fails
  2021-09-01 17:33 ` Fabiano Rosas
@ 2021-09-01 17:33   ` Fabiano Rosas
  -1 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

The module's exit function is not called when the init fails, we need
to do cleanup before returning.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index fe20074faa61..a7b82eb438f5 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5963,7 +5963,7 @@ static int kvmppc_book3s_init_hv(void)
 
 	r = kvm_init_subcore_bitmap();
 	if (r)
-		return r;
+		goto err;
 
 	/*
 	 * We need a way of accessing the XICS interrupt controller,
@@ -5978,7 +5978,8 @@ static int kvmppc_book3s_init_hv(void)
 		np = of_find_compatible_node(NULL, NULL, "ibm,opal-intc");
 		if (!np) {
 			pr_err("KVM-HV: Cannot determine method for accessing XICS\n");
-			return -ENODEV;
+			r = -ENODEV;
+			goto err;
 		}
 		/* presence of intc confirmed - node can be dropped again */
 		of_node_put(np);
@@ -5991,12 +5992,12 @@ static int kvmppc_book3s_init_hv(void)
 
 	r = kvmppc_mmu_hv_init();
 	if (r)
-		return r;
+		goto err;
 
 	if (kvmppc_radix_possible()) {
 		r = kvmppc_radix_init();
 		if (r)
-			return r;
+			goto err;
 	}
 
 	r = kvmppc_uvmem_init();
@@ -6009,6 +6010,12 @@ static int kvmppc_book3s_init_hv(void)
 	kvmppc_hv_ops = &kvm_ops_hv;
 
 	return 0;
+
+err:
+	kvmhv_nested_exit();
+	kvmppc_radix_exit();
+
+	return r;
 }
 
 static void kvmppc_book3s_exit_hv(void)
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/5] KVM: PPC: Book3S HV: Free allocated memory if module init fails
@ 2021-09-01 17:33   ` Fabiano Rosas
  0 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

The module's exit function is not called when the init fails, we need
to do cleanup before returning.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index fe20074faa61..a7b82eb438f5 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5963,7 +5963,7 @@ static int kvmppc_book3s_init_hv(void)
 
 	r = kvm_init_subcore_bitmap();
 	if (r)
-		return r;
+		goto err;
 
 	/*
 	 * We need a way of accessing the XICS interrupt controller,
@@ -5978,7 +5978,8 @@ static int kvmppc_book3s_init_hv(void)
 		np = of_find_compatible_node(NULL, NULL, "ibm,opal-intc");
 		if (!np) {
 			pr_err("KVM-HV: Cannot determine method for accessing XICS\n");
-			return -ENODEV;
+			r = -ENODEV;
+			goto err;
 		}
 		/* presence of intc confirmed - node can be dropped again */
 		of_node_put(np);
@@ -5991,12 +5992,12 @@ static int kvmppc_book3s_init_hv(void)
 
 	r = kvmppc_mmu_hv_init();
 	if (r)
-		return r;
+		goto err;
 
 	if (kvmppc_radix_possible()) {
 		r = kvmppc_radix_init();
 		if (r)
-			return r;
+			goto err;
 	}
 
 	r = kvmppc_uvmem_init();
@@ -6009,6 +6010,12 @@ static int kvmppc_book3s_init_hv(void)
 	kvmppc_hv_ops = &kvm_ops_hv;
 
 	return 0;
+
+err:
+	kvmhv_nested_exit();
+	kvmppc_radix_exit();
+
+	return r;
 }
 
 static void kvmppc_book3s_exit_hv(void)
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/5] KVM: PPC: Book3S: Unify kvm-hv and kvm-pr modules
  2021-09-01 17:33 ` Fabiano Rosas
@ 2021-09-01 17:33   ` Fabiano Rosas
  -1 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

Our three virtualization modules (kvm, kvm-hv, kvm-pr) can be
loaded/unloaded in such a way that could leave kvm.ko loaded without
kvm-hv.ko or kvm-pr.ko. That means the userspace could continue to
issue ioctls to KVM while there is no code on our side to service
them.

We currently select at config time which module will be built, either
kvm-hv, kvm-pr or both, with the kvm module being always present. For
32 bits the kvm module is the only one present and contains the code
from kvm-pr in it. We can do the same for 64 bit and keep hv and pr
inside kvm.ko.

With this, we do not lose the ability of running two guests at the
same time, each using a different implementation (although only POWER
bare-metal with Hash MMU supports HV and PR at the same time).

We lose the ability of loading and unloading the code, which means any
Linux installation that wants to support *both* KVM-HV guests on POWER
bare-metal and KVM-PR nested guests on top of PowerVM would have a
binary with a larger memory footprint (assuming PR is only used when
HV is not present).

This patch alters the build to output only one module at all times and
relies on the Kconfig to select which implementation will be present.

The module init code was rearranged to initialize and cleanup both
implementations or only one of them.

The Kconfig was altered to provide one boolean for each implementation
(KVM_BOOK3S_PR_POSSIBLE, KVM_BOOK3S_HV_POSSIBLE), while keeping the
old tristate for the kvm.ko module (KVM_BOOK3S_64). The old
KVM_BOOK3S_32 already selects KVM_BOOK3S_PR_POSSIBLE, so nothing
changes there.

Two module aliases were created to facilitate the introduction of the
new scheme so `modprobe kvm-hv`and `modprobe kvm-pr` are the same as
`modprobe kvm`.

The license macro was removed because it is already included by
virt/kvm/kvm_main.c.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

---
Here's a summary of the environments that can run KVM:

* 64 bit powernv w/Radix              KVM-HV
* 64 bit powernv w/Hash               KVM-HV        KVM-PR
  32 bit powernv w/Hash                             KVM-PR
  32 bit powernv w/Radix              N/A
* 64 bit pseries w/Radix on powernv   KVM-HV nested
* 64 bit pseries w/Hash  on powernv                 KVM-PR nested
  32 bit pseries w/Hash  on powernv                 KVM-PR nested
* 64 bit pseries w/Radix on PowerVM   N/A
* 64 bit pseries w/Hash  on PowerVM                 KVM-PR nested

Lines with an asterisk were tested with all combinations for the
module and the HV/PR configs.
---
 arch/powerpc/configs/powernv_defconfig |  2 +-
 arch/powerpc/configs/ppc64_defconfig   |  2 +-
 arch/powerpc/configs/pseries_defconfig |  2 +-
 arch/powerpc/kvm/Kconfig               | 72 ++++++++++++--------------
 arch/powerpc/kvm/Makefile              | 11 ++--
 arch/powerpc/kvm/book3s.c              | 46 +++++++++++++---
 arch/powerpc/kvm/book3s.h              | 19 +++++++
 arch/powerpc/kvm/book3s_hv.c           | 10 +---
 arch/powerpc/kvm/book3s_pr.c           | 13 -----
 kernel/irq/irqdesc.c                   |  2 +-
 10 files changed, 105 insertions(+), 74 deletions(-)

diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
index 8bfeea6c7de7..853b95685d9f 100644
--- a/arch/powerpc/configs/powernv_defconfig
+++ b/arch/powerpc/configs/powernv_defconfig
@@ -341,7 +341,7 @@ CONFIG_CRYPTO_DEV_NX=y
 CONFIG_CRYPTO_DEV_VMX=y
 CONFIG_VIRTUALIZATION=y
 CONFIG_KVM_BOOK3S_64=m
-CONFIG_KVM_BOOK3S_64_HV=m
+CONFIG_KVM_BOOK3S_HV_POSSIBLE=y
 CONFIG_VHOST_NET=m
 CONFIG_PRINTK_TIME=y
 CONFIG_PRINTK_CALLER=y
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index 0ad2291337a7..46ace457def6 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -61,7 +61,7 @@ CONFIG_PCCARD=y
 CONFIG_ELECTRA_CF=y
 CONFIG_VIRTUALIZATION=y
 CONFIG_KVM_BOOK3S_64=m
-CONFIG_KVM_BOOK3S_64_HV=m
+CONFIG_KVM_BOOK3S_HV_POSSIBLE=y
 CONFIG_VHOST_NET=m
 CONFIG_KPROBES=y
 CONFIG_JUMP_LABEL=y
diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
index b183629f1bcf..6804b1a6bef1 100644
--- a/arch/powerpc/configs/pseries_defconfig
+++ b/arch/powerpc/configs/pseries_defconfig
@@ -321,7 +321,7 @@ CONFIG_CRYPTO_DEV_NX_ENCRYPT=m
 CONFIG_CRYPTO_DEV_VMX=y
 CONFIG_VIRTUALIZATION=y
 CONFIG_KVM_BOOK3S_64=m
-CONFIG_KVM_BOOK3S_64_HV=m
+CONFIG_KVM_BOOK3S_HV_POSSIBLE=y
 CONFIG_VHOST_NET=m
 CONFIG_PRINTK_TIME=y
 CONFIG_PRINTK_CALLER=y
diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
index e45644657d49..2d080f57ce3b 100644
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -41,12 +41,43 @@ config KVM_BOOK3S_64_HANDLER
 	select PPC_DAWR_FORCE_ENABLE
 
 config KVM_BOOK3S_PR_POSSIBLE
-	bool
+	bool "KVM support without using hypervisor mode in host"
+	depends on KVM_BOOK3S_64 || KVM_BOOK3S_32
 	select KVM_MMIO
 	select MMU_NOTIFIER
+	help
+	  Support running guest kernels in virtual machines on processors
+	  without using hypervisor mode in the host, by running the
+	  guest in user mode (problem state) and emulating all
+	  privileged instructions and registers.
+
+	  This is not as fast as using hypervisor mode, but works on
+	  where hypervisor mode is not available or not usable,
+	  and can emulate processors that are different from the host
+	  processor, including emulating 32-bit processors on a 64-bit
+	  host.
+
 
 config KVM_BOOK3S_HV_POSSIBLE
-	bool
+	bool "KVM for POWER7 and later using hypervisor mode in host"
+	depends on KVM_BOOK3S_64
+	select MMU_NOTIFIER
+	select CMA
+	help
+	  Support running unmodified book3s_64 guest kernels in
+	  virtual machines on POWER7 and newer processors that have
+	  hypervisor mode available to the host.
+
+	  If you say Y here, KVM will use the hardware virtualization
+	  facilities of POWER7 (and later) processors, meaning that
+	  guest operating systems will run at full hardware speed
+	  using supervisor and user modes.  However, this also means
+	  that KVM is not usable under PowerVM (pHyp), is only usable
+	  on POWER7 or later processors, and cannot emulate a
+	  different processor from the host processor.
+
+	  If unsure, say N.
+
 
 config KVM_BOOK3S_32
 	tristate "KVM support for PowerPC book3s_32 processors"
@@ -80,43 +111,6 @@ config KVM_BOOK3S_64
 
 	  If unsure, say N.
 
-config KVM_BOOK3S_64_HV
-	tristate "KVM for POWER7 and later using hypervisor mode in host"
-	depends on KVM_BOOK3S_64 && PPC_POWERNV
-	select KVM_BOOK3S_HV_POSSIBLE
-	select MMU_NOTIFIER
-	select CMA
-	help
-	  Support running unmodified book3s_64 guest kernels in
-	  virtual machines on POWER7 and newer processors that have
-	  hypervisor mode available to the host.
-
-	  If you say Y here, KVM will use the hardware virtualization
-	  facilities of POWER7 (and later) processors, meaning that
-	  guest operating systems will run at full hardware speed
-	  using supervisor and user modes.  However, this also means
-	  that KVM is not usable under PowerVM (pHyp), is only usable
-	  on POWER7 or later processors, and cannot emulate a
-	  different processor from the host processor.
-
-	  If unsure, say N.
-
-config KVM_BOOK3S_64_PR
-	tristate "KVM support without using hypervisor mode in host"
-	depends on KVM_BOOK3S_64
-	select KVM_BOOK3S_PR_POSSIBLE
-	help
-	  Support running guest kernels in virtual machines on processors
-	  without using hypervisor mode in the host, by running the
-	  guest in user mode (problem state) and emulating all
-	  privileged instructions and registers.
-
-	  This is not as fast as using hypervisor mode, but works on
-	  machines where hypervisor mode is not available or not usable,
-	  and can emulate processors that are different from the host
-	  processor, including emulating 32-bit processors on a 64-bit
-	  host.
-
 config KVM_BOOK3S_HV_EXIT_TIMING
 	bool "Detailed timing for hypervisor real-mode code"
 	depends on KVM_BOOK3S_HV_POSSIBLE && DEBUG_FS
diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
index 583c14ef596e..704380065df3 100644
--- a/arch/powerpc/kvm/Makefile
+++ b/arch/powerpc/kvm/Makefile
@@ -108,6 +108,14 @@ kvm-book3s_64-module-objs := \
 	book3s_rtas.o \
 	$(kvm-book3s_64-objs-y)
 
+ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+kvm-book3s_64-module-objs += $(kvm-hv-y)
+endif
+
+ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
+kvm-book3s_64-module-objs += $(kvm-pr-y)
+endif
+
 kvm-objs-$(CONFIG_KVM_BOOK3S_64) := $(kvm-book3s_64-module-objs)
 
 kvm-book3s_32-objs := \
@@ -134,7 +142,4 @@ obj-$(CONFIG_KVM_E500MC) += kvm.o
 obj-$(CONFIG_KVM_BOOK3S_64) += kvm.o
 obj-$(CONFIG_KVM_BOOK3S_32) += kvm.o
 
-obj-$(CONFIG_KVM_BOOK3S_64_PR) += kvm-pr.o
-obj-$(CONFIG_KVM_BOOK3S_64_HV) += kvm-hv.o
-
 obj-y += $(kvm-book3s_64-builtin-objs-y)
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 79833f78d1da..c381bb17b0f9 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -1065,6 +1065,24 @@ int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned irqchip, unsigned pin)
 
 #endif /* CONFIG_KVM_XICS */
 
+static int kvmppc_init(void)
+{
+	int r;
+
+	r = kvmppc_book3s_init_hv();
+	if (r)
+		pr_err("KVM-HV: could not load (%d)\n", r);
+
+	r = kvmppc_book3s_init_pr();
+	if (r)
+		pr_err("KVM-PR: could not load (%d)\n", r);
+
+	if (!kvmppc_hv_ops && !kvmppc_pr_ops)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int kvmppc_book3s_init(void)
 {
 	int r;
@@ -1072,9 +1090,10 @@ static int kvmppc_book3s_init(void)
 	r = kvm_init(NULL, sizeof(struct kvm_vcpu), 0, THIS_MODULE);
 	if (r)
 		return r;
-#ifdef CONFIG_KVM_BOOK3S_32_HANDLER
-	r = kvmppc_book3s_init_pr();
-#endif
+
+	r = kvmppc_init();
+	if (r)
+		goto exit;
 
 #ifdef CONFIG_KVM_XICS
 #ifdef CONFIG_KVM_XIVE
@@ -1087,22 +1106,35 @@ static int kvmppc_book3s_init(void)
 #endif
 		kvm_register_device_ops(&kvm_xics_ops, KVM_DEV_TYPE_XICS);
 #endif
+	return 0;
+
+exit:
+	kvm_exit();
 	return r;
 }
 
 static void kvmppc_book3s_exit(void)
 {
-#ifdef CONFIG_KVM_BOOK3S_32_HANDLER
 	kvmppc_book3s_exit_pr();
-#endif
+	kvmppc_book3s_exit_hv();
 	kvm_exit();
 }
 
 module_init(kvmppc_book3s_init);
 module_exit(kvmppc_book3s_exit);
 
-/* On 32bit this is our one and only kernel module */
-#ifdef CONFIG_KVM_BOOK3S_32_HANDLER
 MODULE_ALIAS_MISCDEV(KVM_MINOR);
 MODULE_ALIAS("devname:kvm");
+
+/*
+ * Whether we use KVM-HV or KVM-PR is dependent exclusively on the
+ * config options. These aliases are here only to ease the transition
+ * to the one module model we have now.
+ */
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+MODULE_ALIAS("kvm-hv");
+#endif
+
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
+MODULE_ALIAS("kvm-pr");
 #endif
diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h
index 740e51def5a5..d21772904971 100644
--- a/arch/powerpc/kvm/book3s.h
+++ b/arch/powerpc/kvm/book3s.h
@@ -22,8 +22,27 @@ extern int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu,
 					int sprn, ulong spr_val);
 extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu,
 					int sprn, ulong *spr_val);
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
 extern int kvmppc_book3s_init_pr(void);
 extern void kvmppc_book3s_exit_pr(void);
+#else
+static inline int kvmppc_book3s_init_pr(void)
+{
+	return 0;
+}
+static inline void kvmppc_book3s_exit_pr(void) {}
+#endif
+
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+extern int kvmppc_book3s_init_hv(void);
+extern void kvmppc_book3s_exit_hv(void);
+#else
+static inline int kvmppc_book3s_init_hv(void)
+{
+	return 0;
+}
+static inline void kvmppc_book3s_exit_hv(void) {}
+#endif
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 extern void kvmppc_emulate_tabort(struct kvm_vcpu *vcpu, int ra_val);
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index a7b82eb438f5..6ba9545ef58e 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5941,7 +5941,7 @@ static int kvmppc_radix_possible(void)
 	return cpu_has_feature(CPU_FTR_ARCH_300) && radix_enabled();
 }
 
-static int kvmppc_book3s_init_hv(void)
+int kvmppc_book3s_init_hv(void)
 {
 	int r;
 
@@ -6018,7 +6018,7 @@ static int kvmppc_book3s_init_hv(void)
 	return r;
 }
 
-static void kvmppc_book3s_exit_hv(void)
+void kvmppc_book3s_exit_hv(void)
 {
 	kvmppc_uvmem_free();
 	kvmppc_free_host_rm_ops();
@@ -6027,9 +6027,3 @@ static void kvmppc_book3s_exit_hv(void)
 	kvmppc_hv_ops = NULL;
 	kvmhv_nested_exit();
 }
-
-module_init(kvmppc_book3s_init_hv);
-module_exit(kvmppc_book3s_exit_hv);
-MODULE_LICENSE("GPL");
-MODULE_ALIAS_MISCDEV(KVM_MINOR);
-MODULE_ALIAS("devname:kvm");
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 6bc9425acb32..1d017c4b3eb4 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -2100,16 +2100,3 @@ void kvmppc_book3s_exit_pr(void)
 	kvmppc_pr_ops = NULL;
 	kvmppc_mmu_hpte_sysexit();
 }
-
-/*
- * We only support separate modules for book3s 64
- */
-#ifdef CONFIG_PPC_BOOK3S_64
-
-module_init(kvmppc_book3s_init_pr);
-module_exit(kvmppc_book3s_exit_pr);
-
-MODULE_LICENSE("GPL");
-MODULE_ALIAS_MISCDEV(KVM_MINOR);
-MODULE_ALIAS("devname:kvm");
-#endif
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 4e3c29bb603c..7c9c9e794c01 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -352,7 +352,7 @@ struct irq_desc *irq_to_desc(unsigned int irq)
 {
 	return radix_tree_lookup(&irq_desc_tree, irq);
 }
-#ifdef CONFIG_KVM_BOOK3S_64_HV_MODULE
+#ifdef CONFIG_KVM_BOOK3S_64_MODULE
 EXPORT_SYMBOL_GPL(irq_to_desc);
 #endif
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/5] KVM: PPC: Book3S: Unify kvm-hv and kvm-pr modules
@ 2021-09-01 17:33   ` Fabiano Rosas
  0 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

Our three virtualization modules (kvm, kvm-hv, kvm-pr) can be
loaded/unloaded in such a way that could leave kvm.ko loaded without
kvm-hv.ko or kvm-pr.ko. That means the userspace could continue to
issue ioctls to KVM while there is no code on our side to service
them.

We currently select at config time which module will be built, either
kvm-hv, kvm-pr or both, with the kvm module being always present. For
32 bits the kvm module is the only one present and contains the code
from kvm-pr in it. We can do the same for 64 bit and keep hv and pr
inside kvm.ko.

With this, we do not lose the ability of running two guests at the
same time, each using a different implementation (although only POWER
bare-metal with Hash MMU supports HV and PR at the same time).

We lose the ability of loading and unloading the code, which means any
Linux installation that wants to support *both* KVM-HV guests on POWER
bare-metal and KVM-PR nested guests on top of PowerVM would have a
binary with a larger memory footprint (assuming PR is only used when
HV is not present).

This patch alters the build to output only one module at all times and
relies on the Kconfig to select which implementation will be present.

The module init code was rearranged to initialize and cleanup both
implementations or only one of them.

The Kconfig was altered to provide one boolean for each implementation
(KVM_BOOK3S_PR_POSSIBLE, KVM_BOOK3S_HV_POSSIBLE), while keeping the
old tristate for the kvm.ko module (KVM_BOOK3S_64). The old
KVM_BOOK3S_32 already selects KVM_BOOK3S_PR_POSSIBLE, so nothing
changes there.

Two module aliases were created to facilitate the introduction of the
new scheme so `modprobe kvm-hv`and `modprobe kvm-pr` are the same as
`modprobe kvm`.

The license macro was removed because it is already included by
virt/kvm/kvm_main.c.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

---
Here's a summary of the environments that can run KVM:

* 64 bit powernv w/Radix              KVM-HV
* 64 bit powernv w/Hash               KVM-HV        KVM-PR
  32 bit powernv w/Hash                             KVM-PR
  32 bit powernv w/Radix              N/A
* 64 bit pseries w/Radix on powernv   KVM-HV nested
* 64 bit pseries w/Hash  on powernv                 KVM-PR nested
  32 bit pseries w/Hash  on powernv                 KVM-PR nested
* 64 bit pseries w/Radix on PowerVM   N/A
* 64 bit pseries w/Hash  on PowerVM                 KVM-PR nested

Lines with an asterisk were tested with all combinations for the
module and the HV/PR configs.
---
 arch/powerpc/configs/powernv_defconfig |  2 +-
 arch/powerpc/configs/ppc64_defconfig   |  2 +-
 arch/powerpc/configs/pseries_defconfig |  2 +-
 arch/powerpc/kvm/Kconfig               | 72 ++++++++++++--------------
 arch/powerpc/kvm/Makefile              | 11 ++--
 arch/powerpc/kvm/book3s.c              | 46 +++++++++++++---
 arch/powerpc/kvm/book3s.h              | 19 +++++++
 arch/powerpc/kvm/book3s_hv.c           | 10 +---
 arch/powerpc/kvm/book3s_pr.c           | 13 -----
 kernel/irq/irqdesc.c                   |  2 +-
 10 files changed, 105 insertions(+), 74 deletions(-)

diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
index 8bfeea6c7de7..853b95685d9f 100644
--- a/arch/powerpc/configs/powernv_defconfig
+++ b/arch/powerpc/configs/powernv_defconfig
@@ -341,7 +341,7 @@ CONFIG_CRYPTO_DEV_NX=y
 CONFIG_CRYPTO_DEV_VMX=y
 CONFIG_VIRTUALIZATION=y
 CONFIG_KVM_BOOK3S_64=m
-CONFIG_KVM_BOOK3S_64_HV=m
+CONFIG_KVM_BOOK3S_HV_POSSIBLE=y
 CONFIG_VHOST_NET=m
 CONFIG_PRINTK_TIME=y
 CONFIG_PRINTK_CALLER=y
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index 0ad2291337a7..46ace457def6 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -61,7 +61,7 @@ CONFIG_PCCARD=y
 CONFIG_ELECTRA_CF=y
 CONFIG_VIRTUALIZATION=y
 CONFIG_KVM_BOOK3S_64=m
-CONFIG_KVM_BOOK3S_64_HV=m
+CONFIG_KVM_BOOK3S_HV_POSSIBLE=y
 CONFIG_VHOST_NET=m
 CONFIG_KPROBES=y
 CONFIG_JUMP_LABEL=y
diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
index b183629f1bcf..6804b1a6bef1 100644
--- a/arch/powerpc/configs/pseries_defconfig
+++ b/arch/powerpc/configs/pseries_defconfig
@@ -321,7 +321,7 @@ CONFIG_CRYPTO_DEV_NX_ENCRYPT=m
 CONFIG_CRYPTO_DEV_VMX=y
 CONFIG_VIRTUALIZATION=y
 CONFIG_KVM_BOOK3S_64=m
-CONFIG_KVM_BOOK3S_64_HV=m
+CONFIG_KVM_BOOK3S_HV_POSSIBLE=y
 CONFIG_VHOST_NET=m
 CONFIG_PRINTK_TIME=y
 CONFIG_PRINTK_CALLER=y
diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
index e45644657d49..2d080f57ce3b 100644
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -41,12 +41,43 @@ config KVM_BOOK3S_64_HANDLER
 	select PPC_DAWR_FORCE_ENABLE
 
 config KVM_BOOK3S_PR_POSSIBLE
-	bool
+	bool "KVM support without using hypervisor mode in host"
+	depends on KVM_BOOK3S_64 || KVM_BOOK3S_32
 	select KVM_MMIO
 	select MMU_NOTIFIER
+	help
+	  Support running guest kernels in virtual machines on processors
+	  without using hypervisor mode in the host, by running the
+	  guest in user mode (problem state) and emulating all
+	  privileged instructions and registers.
+
+	  This is not as fast as using hypervisor mode, but works on
+	  where hypervisor mode is not available or not usable,
+	  and can emulate processors that are different from the host
+	  processor, including emulating 32-bit processors on a 64-bit
+	  host.
+
 
 config KVM_BOOK3S_HV_POSSIBLE
-	bool
+	bool "KVM for POWER7 and later using hypervisor mode in host"
+	depends on KVM_BOOK3S_64
+	select MMU_NOTIFIER
+	select CMA
+	help
+	  Support running unmodified book3s_64 guest kernels in
+	  virtual machines on POWER7 and newer processors that have
+	  hypervisor mode available to the host.
+
+	  If you say Y here, KVM will use the hardware virtualization
+	  facilities of POWER7 (and later) processors, meaning that
+	  guest operating systems will run at full hardware speed
+	  using supervisor and user modes.  However, this also means
+	  that KVM is not usable under PowerVM (pHyp), is only usable
+	  on POWER7 or later processors, and cannot emulate a
+	  different processor from the host processor.
+
+	  If unsure, say N.
+
 
 config KVM_BOOK3S_32
 	tristate "KVM support for PowerPC book3s_32 processors"
@@ -80,43 +111,6 @@ config KVM_BOOK3S_64
 
 	  If unsure, say N.
 
-config KVM_BOOK3S_64_HV
-	tristate "KVM for POWER7 and later using hypervisor mode in host"
-	depends on KVM_BOOK3S_64 && PPC_POWERNV
-	select KVM_BOOK3S_HV_POSSIBLE
-	select MMU_NOTIFIER
-	select CMA
-	help
-	  Support running unmodified book3s_64 guest kernels in
-	  virtual machines on POWER7 and newer processors that have
-	  hypervisor mode available to the host.
-
-	  If you say Y here, KVM will use the hardware virtualization
-	  facilities of POWER7 (and later) processors, meaning that
-	  guest operating systems will run at full hardware speed
-	  using supervisor and user modes.  However, this also means
-	  that KVM is not usable under PowerVM (pHyp), is only usable
-	  on POWER7 or later processors, and cannot emulate a
-	  different processor from the host processor.
-
-	  If unsure, say N.
-
-config KVM_BOOK3S_64_PR
-	tristate "KVM support without using hypervisor mode in host"
-	depends on KVM_BOOK3S_64
-	select KVM_BOOK3S_PR_POSSIBLE
-	help
-	  Support running guest kernels in virtual machines on processors
-	  without using hypervisor mode in the host, by running the
-	  guest in user mode (problem state) and emulating all
-	  privileged instructions and registers.
-
-	  This is not as fast as using hypervisor mode, but works on
-	  machines where hypervisor mode is not available or not usable,
-	  and can emulate processors that are different from the host
-	  processor, including emulating 32-bit processors on a 64-bit
-	  host.
-
 config KVM_BOOK3S_HV_EXIT_TIMING
 	bool "Detailed timing for hypervisor real-mode code"
 	depends on KVM_BOOK3S_HV_POSSIBLE && DEBUG_FS
diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
index 583c14ef596e..704380065df3 100644
--- a/arch/powerpc/kvm/Makefile
+++ b/arch/powerpc/kvm/Makefile
@@ -108,6 +108,14 @@ kvm-book3s_64-module-objs := \
 	book3s_rtas.o \
 	$(kvm-book3s_64-objs-y)
 
+ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+kvm-book3s_64-module-objs += $(kvm-hv-y)
+endif
+
+ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
+kvm-book3s_64-module-objs += $(kvm-pr-y)
+endif
+
 kvm-objs-$(CONFIG_KVM_BOOK3S_64) := $(kvm-book3s_64-module-objs)
 
 kvm-book3s_32-objs := \
@@ -134,7 +142,4 @@ obj-$(CONFIG_KVM_E500MC) += kvm.o
 obj-$(CONFIG_KVM_BOOK3S_64) += kvm.o
 obj-$(CONFIG_KVM_BOOK3S_32) += kvm.o
 
-obj-$(CONFIG_KVM_BOOK3S_64_PR) += kvm-pr.o
-obj-$(CONFIG_KVM_BOOK3S_64_HV) += kvm-hv.o
-
 obj-y += $(kvm-book3s_64-builtin-objs-y)
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 79833f78d1da..c381bb17b0f9 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -1065,6 +1065,24 @@ int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned irqchip, unsigned pin)
 
 #endif /* CONFIG_KVM_XICS */
 
+static int kvmppc_init(void)
+{
+	int r;
+
+	r = kvmppc_book3s_init_hv();
+	if (r)
+		pr_err("KVM-HV: could not load (%d)\n", r);
+
+	r = kvmppc_book3s_init_pr();
+	if (r)
+		pr_err("KVM-PR: could not load (%d)\n", r);
+
+	if (!kvmppc_hv_ops && !kvmppc_pr_ops)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int kvmppc_book3s_init(void)
 {
 	int r;
@@ -1072,9 +1090,10 @@ static int kvmppc_book3s_init(void)
 	r = kvm_init(NULL, sizeof(struct kvm_vcpu), 0, THIS_MODULE);
 	if (r)
 		return r;
-#ifdef CONFIG_KVM_BOOK3S_32_HANDLER
-	r = kvmppc_book3s_init_pr();
-#endif
+
+	r = kvmppc_init();
+	if (r)
+		goto exit;
 
 #ifdef CONFIG_KVM_XICS
 #ifdef CONFIG_KVM_XIVE
@@ -1087,22 +1106,35 @@ static int kvmppc_book3s_init(void)
 #endif
 		kvm_register_device_ops(&kvm_xics_ops, KVM_DEV_TYPE_XICS);
 #endif
+	return 0;
+
+exit:
+	kvm_exit();
 	return r;
 }
 
 static void kvmppc_book3s_exit(void)
 {
-#ifdef CONFIG_KVM_BOOK3S_32_HANDLER
 	kvmppc_book3s_exit_pr();
-#endif
+	kvmppc_book3s_exit_hv();
 	kvm_exit();
 }
 
 module_init(kvmppc_book3s_init);
 module_exit(kvmppc_book3s_exit);
 
-/* On 32bit this is our one and only kernel module */
-#ifdef CONFIG_KVM_BOOK3S_32_HANDLER
 MODULE_ALIAS_MISCDEV(KVM_MINOR);
 MODULE_ALIAS("devname:kvm");
+
+/*
+ * Whether we use KVM-HV or KVM-PR is dependent exclusively on the
+ * config options. These aliases are here only to ease the transition
+ * to the one module model we have now.
+ */
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+MODULE_ALIAS("kvm-hv");
+#endif
+
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
+MODULE_ALIAS("kvm-pr");
 #endif
diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h
index 740e51def5a5..d21772904971 100644
--- a/arch/powerpc/kvm/book3s.h
+++ b/arch/powerpc/kvm/book3s.h
@@ -22,8 +22,27 @@ extern int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu,
 					int sprn, ulong spr_val);
 extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu,
 					int sprn, ulong *spr_val);
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
 extern int kvmppc_book3s_init_pr(void);
 extern void kvmppc_book3s_exit_pr(void);
+#else
+static inline int kvmppc_book3s_init_pr(void)
+{
+	return 0;
+}
+static inline void kvmppc_book3s_exit_pr(void) {}
+#endif
+
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+extern int kvmppc_book3s_init_hv(void);
+extern void kvmppc_book3s_exit_hv(void);
+#else
+static inline int kvmppc_book3s_init_hv(void)
+{
+	return 0;
+}
+static inline void kvmppc_book3s_exit_hv(void) {}
+#endif
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 extern void kvmppc_emulate_tabort(struct kvm_vcpu *vcpu, int ra_val);
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index a7b82eb438f5..6ba9545ef58e 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5941,7 +5941,7 @@ static int kvmppc_radix_possible(void)
 	return cpu_has_feature(CPU_FTR_ARCH_300) && radix_enabled();
 }
 
-static int kvmppc_book3s_init_hv(void)
+int kvmppc_book3s_init_hv(void)
 {
 	int r;
 
@@ -6018,7 +6018,7 @@ static int kvmppc_book3s_init_hv(void)
 	return r;
 }
 
-static void kvmppc_book3s_exit_hv(void)
+void kvmppc_book3s_exit_hv(void)
 {
 	kvmppc_uvmem_free();
 	kvmppc_free_host_rm_ops();
@@ -6027,9 +6027,3 @@ static void kvmppc_book3s_exit_hv(void)
 	kvmppc_hv_ops = NULL;
 	kvmhv_nested_exit();
 }
-
-module_init(kvmppc_book3s_init_hv);
-module_exit(kvmppc_book3s_exit_hv);
-MODULE_LICENSE("GPL");
-MODULE_ALIAS_MISCDEV(KVM_MINOR);
-MODULE_ALIAS("devname:kvm");
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 6bc9425acb32..1d017c4b3eb4 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -2100,16 +2100,3 @@ void kvmppc_book3s_exit_pr(void)
 	kvmppc_pr_ops = NULL;
 	kvmppc_mmu_hpte_sysexit();
 }
-
-/*
- * We only support separate modules for book3s 64
- */
-#ifdef CONFIG_PPC_BOOK3S_64
-
-module_init(kvmppc_book3s_init_pr);
-module_exit(kvmppc_book3s_exit_pr);
-
-MODULE_LICENSE("GPL");
-MODULE_ALIAS_MISCDEV(KVM_MINOR);
-MODULE_ALIAS("devname:kvm");
-#endif
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 4e3c29bb603c..7c9c9e794c01 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -352,7 +352,7 @@ struct irq_desc *irq_to_desc(unsigned int irq)
 {
 	return radix_tree_lookup(&irq_desc_tree, irq);
 }
-#ifdef CONFIG_KVM_BOOK3S_64_HV_MODULE
+#ifdef CONFIG_KVM_BOOK3S_64_MODULE
 EXPORT_SYMBOL_GPL(irq_to_desc);
 #endif
 
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/5] KVM: PPC: Book3S: Stop exporting non-builtin symbols
  2021-09-01 17:33 ` Fabiano Rosas
@ 2021-09-01 17:33   ` Fabiano Rosas
  -1 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

Now that we have only one kvm module, we can stop exporting some
symbols.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 arch/powerpc/kvm/book3s.c              | 15 ---------------
 arch/powerpc/kvm/book3s_64_mmu_radix.c |  3 ---
 arch/powerpc/kvm/book3s_64_vio.c       |  3 ---
 arch/powerpc/kvm/book3s_rtas.c         |  1 -
 arch/powerpc/kvm/book3s_xics.c         |  4 ----
 arch/powerpc/kvm/book3s_xive.c         |  6 ------
 arch/powerpc/kvm/emulate.c             |  1 -
 arch/powerpc/kvm/powerpc.c             | 14 --------------
 8 files changed, 47 deletions(-)

diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index c381bb17b0f9..120a68c76501 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -191,27 +191,23 @@ void kvmppc_book3s_queue_irqprio(struct kvm_vcpu *vcpu, unsigned int vec)
 	printk(KERN_INFO "Queueing interrupt %x\n", vec);
 #endif
 }
-EXPORT_SYMBOL_GPL(kvmppc_book3s_queue_irqprio);
 
 void kvmppc_core_queue_machine_check(struct kvm_vcpu *vcpu, ulong flags)
 {
 	/* might as well deliver this straight away */
 	kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_MACHINE_CHECK, flags);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_queue_machine_check);
 
 void kvmppc_core_queue_syscall(struct kvm_vcpu *vcpu)
 {
 	kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_SYSCALL, 0);
 }
-EXPORT_SYMBOL(kvmppc_core_queue_syscall);
 
 void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
 {
 	/* might as well deliver this straight away */
 	kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_PROGRAM, flags);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_queue_program);
 
 void kvmppc_core_queue_fpunavail(struct kvm_vcpu *vcpu)
 {
@@ -235,19 +231,16 @@ void kvmppc_core_queue_dec(struct kvm_vcpu *vcpu)
 {
 	kvmppc_book3s_queue_irqprio(vcpu, BOOK3S_INTERRUPT_DECREMENTER);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_queue_dec);
 
 int kvmppc_core_pending_dec(struct kvm_vcpu *vcpu)
 {
 	return test_bit(BOOK3S_IRQPRIO_DECREMENTER, &vcpu->arch.pending_exceptions);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_pending_dec);
 
 void kvmppc_core_dequeue_dec(struct kvm_vcpu *vcpu)
 {
 	kvmppc_book3s_dequeue_irqprio(vcpu, BOOK3S_INTERRUPT_DECREMENTER);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_dequeue_dec);
 
 void kvmppc_core_queue_external(struct kvm_vcpu *vcpu,
                                 struct kvm_interrupt *irq)
@@ -290,13 +283,11 @@ void kvmppc_core_queue_data_storage(struct kvm_vcpu *vcpu, ulong dar,
 	kvmppc_set_dsisr(vcpu, flags);
 	kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_DATA_STORAGE, 0);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_queue_data_storage);
 
 void kvmppc_core_queue_inst_storage(struct kvm_vcpu *vcpu, ulong flags)
 {
 	kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_INST_STORAGE, flags);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_queue_inst_storage);
 
 static int kvmppc_book3s_irqprio_deliver(struct kvm_vcpu *vcpu,
 					 unsigned int priority)
@@ -428,7 +419,6 @@ int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_prepare_to_enter);
 
 kvm_pfn_t kvmppc_gpa_to_pfn(struct kvm_vcpu *vcpu, gpa_t gpa, bool writing,
 			bool *writable)
@@ -454,7 +444,6 @@ kvm_pfn_t kvmppc_gpa_to_pfn(struct kvm_vcpu *vcpu, gpa_t gpa, bool writing,
 
 	return gfn_to_pfn_prot(vcpu->kvm, gfn, writing, writable);
 }
-EXPORT_SYMBOL_GPL(kvmppc_gpa_to_pfn);
 
 int kvmppc_xlate(struct kvm_vcpu *vcpu, ulong eaddr, enum xlate_instdata xlid,
 		 enum xlate_readwrite xlrw, struct kvmppc_pte *pte)
@@ -501,7 +490,6 @@ int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
 	else
 		return EMULATE_AGAIN;
 }
-EXPORT_SYMBOL_GPL(kvmppc_load_last_inst);
 
 int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu)
 {
@@ -788,7 +776,6 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u64 msr)
 {
 	vcpu->kvm->arch.kvm_ops->set_msr(vcpu, msr);
 }
-EXPORT_SYMBOL_GPL(kvmppc_set_msr);
 
 int kvmppc_vcpu_run(struct kvm_vcpu *vcpu)
 {
@@ -964,7 +951,6 @@ int kvmppc_h_logical_ci_load(struct kvm_vcpu *vcpu)
 
 	return H_SUCCESS;
 }
-EXPORT_SYMBOL_GPL(kvmppc_h_logical_ci_load);
 
 int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu)
 {
@@ -1004,7 +990,6 @@ int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu)
 
 	return H_SUCCESS;
 }
-EXPORT_SYMBOL_GPL(kvmppc_h_logical_ci_store);
 
 int kvmppc_core_check_processor_compat(void)
 {
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index b5905ae4377c..f354ccfed56d 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -81,7 +81,6 @@ unsigned long __kvmhv_copy_tofrom_guest_radix(int lpid, int pid,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(__kvmhv_copy_tofrom_guest_radix);
 
 static long kvmhv_copy_tofrom_guest_radix(struct kvm_vcpu *vcpu, gva_t eaddr,
 					  void *to, void *from, unsigned long n)
@@ -117,14 +116,12 @@ long kvmhv_copy_from_guest_radix(struct kvm_vcpu *vcpu, gva_t eaddr, void *to,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(kvmhv_copy_from_guest_radix);
 
 long kvmhv_copy_to_guest_radix(struct kvm_vcpu *vcpu, gva_t eaddr, void *from,
 			       unsigned long n)
 {
 	return kvmhv_copy_tofrom_guest_radix(vcpu, eaddr, NULL, from, n);
 }
-EXPORT_SYMBOL_GPL(kvmhv_copy_to_guest_radix);
 
 int kvmppc_mmu_walk_radix_tree(struct kvm_vcpu *vcpu, gva_t eaddr,
 			       struct kvmppc_pte *gpte, u64 root,
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 8da93fdfa59e..5e717512d9c4 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -606,7 +606,6 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(kvmppc_h_put_tce);
 
 long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
 		unsigned long liobn, unsigned long ioba,
@@ -703,7 +702,6 @@ long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(kvmppc_h_put_tce_indirect);
 
 long kvmppc_h_stuff_tce(struct kvm_vcpu *vcpu,
 		unsigned long liobn, unsigned long ioba,
@@ -752,4 +750,3 @@ long kvmppc_h_stuff_tce(struct kvm_vcpu *vcpu,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(kvmppc_h_stuff_tce);
diff --git a/arch/powerpc/kvm/book3s_rtas.c b/arch/powerpc/kvm/book3s_rtas.c
index 0f847f1e5ddd..91094dfb9bc4 100644
--- a/arch/powerpc/kvm/book3s_rtas.c
+++ b/arch/powerpc/kvm/book3s_rtas.c
@@ -294,7 +294,6 @@ int kvmppc_rtas_hcall(struct kvm_vcpu *vcpu)
 	 */
 	return rc;
 }
-EXPORT_SYMBOL_GPL(kvmppc_rtas_hcall);
 
 void kvmppc_rtas_tokens_free(struct kvm *kvm)
 {
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index 303e3cb096db..234a80c958f1 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -870,7 +870,6 @@ int kvmppc_xics_rm_complete(struct kvm_vcpu *vcpu, u32 hcall)
 
 	return H_SUCCESS;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xics_rm_complete);
 
 int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
 {
@@ -917,7 +916,6 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
 
 	return rc;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xics_hcall);
 
 
 /* -- Initialisation code etc. -- */
@@ -1498,7 +1496,6 @@ void kvmppc_xics_set_mapped(struct kvm *kvm, unsigned long irq,
 	ics->irq_state[idx].host_irq = host_irq;
 	ics->irq_state[idx].intr_cpu = -1;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xics_set_mapped);
 
 void kvmppc_xics_clr_mapped(struct kvm *kvm, unsigned long irq,
 			    unsigned long host_irq)
@@ -1513,4 +1510,3 @@ void kvmppc_xics_clr_mapped(struct kvm *kvm, unsigned long irq,
 
 	ics->irq_state[idx].host_irq = 0;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xics_clr_mapped);
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 8cfab3547494..77b350805649 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -126,7 +126,6 @@ void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu)
 			vcpu->arch.xive_esc_on = 0;
 	}
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_push_vcpu);
 
 /*
  * Pull a vcpu's context from the XIVE on guest exit.
@@ -157,7 +156,6 @@ void kvmppc_xive_pull_vcpu(struct kvm_vcpu *vcpu)
 	vcpu->arch.xive_pushed = 0;
 	eieio();
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_pull_vcpu);
 
 void kvmppc_xive_rearm_escalation(struct kvm_vcpu *vcpu)
 {
@@ -191,7 +189,6 @@ void kvmppc_xive_rearm_escalation(struct kvm_vcpu *vcpu)
 	}
 	mb();
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_rearm_escalation);
 
 /*
  * This is a simple trigger for a generic XIVE IRQ. This must
@@ -1016,7 +1013,6 @@ int kvmppc_xive_set_mapped(struct kvm *kvm, unsigned long guest_irq,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_set_mapped);
 
 int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,
 			   struct irq_desc *host_desc)
@@ -1097,7 +1093,6 @@ int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_clr_mapped);
 
 void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu)
 {
@@ -2169,7 +2164,6 @@ int kvmppc_xive_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
 
 	return H_UNSUPPORTED;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_xics_hcall);
 
 int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu)
 {
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index ee1147c98cd8..468efa502d56 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -304,4 +304,3 @@ int kvmppc_emulate_instruction(struct kvm_vcpu *vcpu)
 
 	return emulated;
 }
-EXPORT_SYMBOL_GPL(kvmppc_emulate_instruction);
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index b4e6f70b97b9..b3a8853bf6ba 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -41,9 +41,7 @@
 #include "trace.h"
 
 struct kvmppc_ops *kvmppc_hv_ops;
-EXPORT_SYMBOL_GPL(kvmppc_hv_ops);
 struct kvmppc_ops *kvmppc_pr_ops;
-EXPORT_SYMBOL_GPL(kvmppc_pr_ops);
 
 
 int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
@@ -135,7 +133,6 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
 	local_irq_enable();
 	return r;
 }
-EXPORT_SYMBOL_GPL(kvmppc_prepare_to_enter);
 
 #if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_KVM_BOOK3S_PR_POSSIBLE)
 static void kvmppc_swab_shared(struct kvm_vcpu *vcpu)
@@ -248,7 +245,6 @@ int kvmppc_kvm_pv(struct kvm_vcpu *vcpu)
 
 	return r;
 }
-EXPORT_SYMBOL_GPL(kvmppc_kvm_pv);
 
 int kvmppc_sanity_check(struct kvm_vcpu *vcpu)
 {
@@ -277,7 +273,6 @@ int kvmppc_sanity_check(struct kvm_vcpu *vcpu)
 	vcpu->arch.sane = r;
 	return r ? 0 : -EINVAL;
 }
-EXPORT_SYMBOL_GPL(kvmppc_sanity_check);
 
 int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
 {
@@ -319,7 +314,6 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
 
 	return r;
 }
-EXPORT_SYMBOL_GPL(kvmppc_emulate_mmio);
 
 int kvmppc_st(struct kvm_vcpu *vcpu, ulong *eaddr, int size, void *ptr,
 	      bool data)
@@ -362,7 +356,6 @@ int kvmppc_st(struct kvm_vcpu *vcpu, ulong *eaddr, int size, void *ptr,
 
 	return EMULATE_DONE;
 }
-EXPORT_SYMBOL_GPL(kvmppc_st);
 
 int kvmppc_ld(struct kvm_vcpu *vcpu, ulong *eaddr, int size, void *ptr,
 		      bool data)
@@ -411,7 +404,6 @@ int kvmppc_ld(struct kvm_vcpu *vcpu, ulong *eaddr, int size, void *ptr,
 
 	return EMULATE_DONE;
 }
-EXPORT_SYMBOL_GPL(kvmppc_ld);
 
 int kvm_arch_hardware_enable(void)
 {
@@ -1281,7 +1273,6 @@ int kvmppc_handle_load(struct kvm_vcpu *vcpu,
 {
 	return __kvmppc_handle_load(vcpu, rt, bytes, is_default_endian, 0);
 }
-EXPORT_SYMBOL_GPL(kvmppc_handle_load);
 
 /* Same as above, but sign extends */
 int kvmppc_handle_loads(struct kvm_vcpu *vcpu,
@@ -1378,7 +1369,6 @@ int kvmppc_handle_store(struct kvm_vcpu *vcpu,
 
 	return EMULATE_DO_MMIO;
 }
-EXPORT_SYMBOL_GPL(kvmppc_handle_store);
 
 #ifdef CONFIG_VSX
 static inline int kvmppc_get_vsr_data(struct kvm_vcpu *vcpu, int rs, u64 *val)
@@ -2478,26 +2468,22 @@ long kvmppc_alloc_lpid(void)
 
 	return lpid;
 }
-EXPORT_SYMBOL_GPL(kvmppc_alloc_lpid);
 
 void kvmppc_claim_lpid(long lpid)
 {
 	set_bit(lpid, lpid_inuse);
 }
-EXPORT_SYMBOL_GPL(kvmppc_claim_lpid);
 
 void kvmppc_free_lpid(long lpid)
 {
 	clear_bit(lpid, lpid_inuse);
 }
-EXPORT_SYMBOL_GPL(kvmppc_free_lpid);
 
 void kvmppc_init_lpid(unsigned long nr_lpids_param)
 {
 	nr_lpids = min_t(unsigned long, KVMPPC_NR_LPIDS, nr_lpids_param);
 	memset(lpid_inuse, 0, sizeof(lpid_inuse));
 }
-EXPORT_SYMBOL_GPL(kvmppc_init_lpid);
 
 int kvm_arch_init(void *opaque)
 {
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/5] KVM: PPC: Book3S: Stop exporting non-builtin symbols
@ 2021-09-01 17:33   ` Fabiano Rosas
  0 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-01 17:33 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, david

Now that we have only one kvm module, we can stop exporting some
symbols.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 arch/powerpc/kvm/book3s.c              | 15 ---------------
 arch/powerpc/kvm/book3s_64_mmu_radix.c |  3 ---
 arch/powerpc/kvm/book3s_64_vio.c       |  3 ---
 arch/powerpc/kvm/book3s_rtas.c         |  1 -
 arch/powerpc/kvm/book3s_xics.c         |  4 ----
 arch/powerpc/kvm/book3s_xive.c         |  6 ------
 arch/powerpc/kvm/emulate.c             |  1 -
 arch/powerpc/kvm/powerpc.c             | 14 --------------
 8 files changed, 47 deletions(-)

diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index c381bb17b0f9..120a68c76501 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -191,27 +191,23 @@ void kvmppc_book3s_queue_irqprio(struct kvm_vcpu *vcpu, unsigned int vec)
 	printk(KERN_INFO "Queueing interrupt %x\n", vec);
 #endif
 }
-EXPORT_SYMBOL_GPL(kvmppc_book3s_queue_irqprio);
 
 void kvmppc_core_queue_machine_check(struct kvm_vcpu *vcpu, ulong flags)
 {
 	/* might as well deliver this straight away */
 	kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_MACHINE_CHECK, flags);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_queue_machine_check);
 
 void kvmppc_core_queue_syscall(struct kvm_vcpu *vcpu)
 {
 	kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_SYSCALL, 0);
 }
-EXPORT_SYMBOL(kvmppc_core_queue_syscall);
 
 void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
 {
 	/* might as well deliver this straight away */
 	kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_PROGRAM, flags);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_queue_program);
 
 void kvmppc_core_queue_fpunavail(struct kvm_vcpu *vcpu)
 {
@@ -235,19 +231,16 @@ void kvmppc_core_queue_dec(struct kvm_vcpu *vcpu)
 {
 	kvmppc_book3s_queue_irqprio(vcpu, BOOK3S_INTERRUPT_DECREMENTER);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_queue_dec);
 
 int kvmppc_core_pending_dec(struct kvm_vcpu *vcpu)
 {
 	return test_bit(BOOK3S_IRQPRIO_DECREMENTER, &vcpu->arch.pending_exceptions);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_pending_dec);
 
 void kvmppc_core_dequeue_dec(struct kvm_vcpu *vcpu)
 {
 	kvmppc_book3s_dequeue_irqprio(vcpu, BOOK3S_INTERRUPT_DECREMENTER);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_dequeue_dec);
 
 void kvmppc_core_queue_external(struct kvm_vcpu *vcpu,
                                 struct kvm_interrupt *irq)
@@ -290,13 +283,11 @@ void kvmppc_core_queue_data_storage(struct kvm_vcpu *vcpu, ulong dar,
 	kvmppc_set_dsisr(vcpu, flags);
 	kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_DATA_STORAGE, 0);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_queue_data_storage);
 
 void kvmppc_core_queue_inst_storage(struct kvm_vcpu *vcpu, ulong flags)
 {
 	kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_INST_STORAGE, flags);
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_queue_inst_storage);
 
 static int kvmppc_book3s_irqprio_deliver(struct kvm_vcpu *vcpu,
 					 unsigned int priority)
@@ -428,7 +419,6 @@ int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(kvmppc_core_prepare_to_enter);
 
 kvm_pfn_t kvmppc_gpa_to_pfn(struct kvm_vcpu *vcpu, gpa_t gpa, bool writing,
 			bool *writable)
@@ -454,7 +444,6 @@ kvm_pfn_t kvmppc_gpa_to_pfn(struct kvm_vcpu *vcpu, gpa_t gpa, bool writing,
 
 	return gfn_to_pfn_prot(vcpu->kvm, gfn, writing, writable);
 }
-EXPORT_SYMBOL_GPL(kvmppc_gpa_to_pfn);
 
 int kvmppc_xlate(struct kvm_vcpu *vcpu, ulong eaddr, enum xlate_instdata xlid,
 		 enum xlate_readwrite xlrw, struct kvmppc_pte *pte)
@@ -501,7 +490,6 @@ int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
 	else
 		return EMULATE_AGAIN;
 }
-EXPORT_SYMBOL_GPL(kvmppc_load_last_inst);
 
 int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu)
 {
@@ -788,7 +776,6 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u64 msr)
 {
 	vcpu->kvm->arch.kvm_ops->set_msr(vcpu, msr);
 }
-EXPORT_SYMBOL_GPL(kvmppc_set_msr);
 
 int kvmppc_vcpu_run(struct kvm_vcpu *vcpu)
 {
@@ -964,7 +951,6 @@ int kvmppc_h_logical_ci_load(struct kvm_vcpu *vcpu)
 
 	return H_SUCCESS;
 }
-EXPORT_SYMBOL_GPL(kvmppc_h_logical_ci_load);
 
 int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu)
 {
@@ -1004,7 +990,6 @@ int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu)
 
 	return H_SUCCESS;
 }
-EXPORT_SYMBOL_GPL(kvmppc_h_logical_ci_store);
 
 int kvmppc_core_check_processor_compat(void)
 {
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index b5905ae4377c..f354ccfed56d 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -81,7 +81,6 @@ unsigned long __kvmhv_copy_tofrom_guest_radix(int lpid, int pid,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(__kvmhv_copy_tofrom_guest_radix);
 
 static long kvmhv_copy_tofrom_guest_radix(struct kvm_vcpu *vcpu, gva_t eaddr,
 					  void *to, void *from, unsigned long n)
@@ -117,14 +116,12 @@ long kvmhv_copy_from_guest_radix(struct kvm_vcpu *vcpu, gva_t eaddr, void *to,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(kvmhv_copy_from_guest_radix);
 
 long kvmhv_copy_to_guest_radix(struct kvm_vcpu *vcpu, gva_t eaddr, void *from,
 			       unsigned long n)
 {
 	return kvmhv_copy_tofrom_guest_radix(vcpu, eaddr, NULL, from, n);
 }
-EXPORT_SYMBOL_GPL(kvmhv_copy_to_guest_radix);
 
 int kvmppc_mmu_walk_radix_tree(struct kvm_vcpu *vcpu, gva_t eaddr,
 			       struct kvmppc_pte *gpte, u64 root,
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 8da93fdfa59e..5e717512d9c4 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -606,7 +606,6 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(kvmppc_h_put_tce);
 
 long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
 		unsigned long liobn, unsigned long ioba,
@@ -703,7 +702,6 @@ long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(kvmppc_h_put_tce_indirect);
 
 long kvmppc_h_stuff_tce(struct kvm_vcpu *vcpu,
 		unsigned long liobn, unsigned long ioba,
@@ -752,4 +750,3 @@ long kvmppc_h_stuff_tce(struct kvm_vcpu *vcpu,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(kvmppc_h_stuff_tce);
diff --git a/arch/powerpc/kvm/book3s_rtas.c b/arch/powerpc/kvm/book3s_rtas.c
index 0f847f1e5ddd..91094dfb9bc4 100644
--- a/arch/powerpc/kvm/book3s_rtas.c
+++ b/arch/powerpc/kvm/book3s_rtas.c
@@ -294,7 +294,6 @@ int kvmppc_rtas_hcall(struct kvm_vcpu *vcpu)
 	 */
 	return rc;
 }
-EXPORT_SYMBOL_GPL(kvmppc_rtas_hcall);
 
 void kvmppc_rtas_tokens_free(struct kvm *kvm)
 {
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index 303e3cb096db..234a80c958f1 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -870,7 +870,6 @@ int kvmppc_xics_rm_complete(struct kvm_vcpu *vcpu, u32 hcall)
 
 	return H_SUCCESS;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xics_rm_complete);
 
 int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
 {
@@ -917,7 +916,6 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
 
 	return rc;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xics_hcall);
 
 
 /* -- Initialisation code etc. -- */
@@ -1498,7 +1496,6 @@ void kvmppc_xics_set_mapped(struct kvm *kvm, unsigned long irq,
 	ics->irq_state[idx].host_irq = host_irq;
 	ics->irq_state[idx].intr_cpu = -1;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xics_set_mapped);
 
 void kvmppc_xics_clr_mapped(struct kvm *kvm, unsigned long irq,
 			    unsigned long host_irq)
@@ -1513,4 +1510,3 @@ void kvmppc_xics_clr_mapped(struct kvm *kvm, unsigned long irq,
 
 	ics->irq_state[idx].host_irq = 0;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xics_clr_mapped);
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 8cfab3547494..77b350805649 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -126,7 +126,6 @@ void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu)
 			vcpu->arch.xive_esc_on = 0;
 	}
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_push_vcpu);
 
 /*
  * Pull a vcpu's context from the XIVE on guest exit.
@@ -157,7 +156,6 @@ void kvmppc_xive_pull_vcpu(struct kvm_vcpu *vcpu)
 	vcpu->arch.xive_pushed = 0;
 	eieio();
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_pull_vcpu);
 
 void kvmppc_xive_rearm_escalation(struct kvm_vcpu *vcpu)
 {
@@ -191,7 +189,6 @@ void kvmppc_xive_rearm_escalation(struct kvm_vcpu *vcpu)
 	}
 	mb();
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_rearm_escalation);
 
 /*
  * This is a simple trigger for a generic XIVE IRQ. This must
@@ -1016,7 +1013,6 @@ int kvmppc_xive_set_mapped(struct kvm *kvm, unsigned long guest_irq,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_set_mapped);
 
 int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,
 			   struct irq_desc *host_desc)
@@ -1097,7 +1093,6 @@ int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_clr_mapped);
 
 void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu)
 {
@@ -2169,7 +2164,6 @@ int kvmppc_xive_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
 
 	return H_UNSUPPORTED;
 }
-EXPORT_SYMBOL_GPL(kvmppc_xive_xics_hcall);
 
 int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu)
 {
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index ee1147c98cd8..468efa502d56 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -304,4 +304,3 @@ int kvmppc_emulate_instruction(struct kvm_vcpu *vcpu)
 
 	return emulated;
 }
-EXPORT_SYMBOL_GPL(kvmppc_emulate_instruction);
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index b4e6f70b97b9..b3a8853bf6ba 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -41,9 +41,7 @@
 #include "trace.h"
 
 struct kvmppc_ops *kvmppc_hv_ops;
-EXPORT_SYMBOL_GPL(kvmppc_hv_ops);
 struct kvmppc_ops *kvmppc_pr_ops;
-EXPORT_SYMBOL_GPL(kvmppc_pr_ops);
 
 
 int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
@@ -135,7 +133,6 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
 	local_irq_enable();
 	return r;
 }
-EXPORT_SYMBOL_GPL(kvmppc_prepare_to_enter);
 
 #if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_KVM_BOOK3S_PR_POSSIBLE)
 static void kvmppc_swab_shared(struct kvm_vcpu *vcpu)
@@ -248,7 +245,6 @@ int kvmppc_kvm_pv(struct kvm_vcpu *vcpu)
 
 	return r;
 }
-EXPORT_SYMBOL_GPL(kvmppc_kvm_pv);
 
 int kvmppc_sanity_check(struct kvm_vcpu *vcpu)
 {
@@ -277,7 +273,6 @@ int kvmppc_sanity_check(struct kvm_vcpu *vcpu)
 	vcpu->arch.sane = r;
 	return r ? 0 : -EINVAL;
 }
-EXPORT_SYMBOL_GPL(kvmppc_sanity_check);
 
 int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
 {
@@ -319,7 +314,6 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
 
 	return r;
 }
-EXPORT_SYMBOL_GPL(kvmppc_emulate_mmio);
 
 int kvmppc_st(struct kvm_vcpu *vcpu, ulong *eaddr, int size, void *ptr,
 	      bool data)
@@ -362,7 +356,6 @@ int kvmppc_st(struct kvm_vcpu *vcpu, ulong *eaddr, int size, void *ptr,
 
 	return EMULATE_DONE;
 }
-EXPORT_SYMBOL_GPL(kvmppc_st);
 
 int kvmppc_ld(struct kvm_vcpu *vcpu, ulong *eaddr, int size, void *ptr,
 		      bool data)
@@ -411,7 +404,6 @@ int kvmppc_ld(struct kvm_vcpu *vcpu, ulong *eaddr, int size, void *ptr,
 
 	return EMULATE_DONE;
 }
-EXPORT_SYMBOL_GPL(kvmppc_ld);
 
 int kvm_arch_hardware_enable(void)
 {
@@ -1281,7 +1273,6 @@ int kvmppc_handle_load(struct kvm_vcpu *vcpu,
 {
 	return __kvmppc_handle_load(vcpu, rt, bytes, is_default_endian, 0);
 }
-EXPORT_SYMBOL_GPL(kvmppc_handle_load);
 
 /* Same as above, but sign extends */
 int kvmppc_handle_loads(struct kvm_vcpu *vcpu,
@@ -1378,7 +1369,6 @@ int kvmppc_handle_store(struct kvm_vcpu *vcpu,
 
 	return EMULATE_DO_MMIO;
 }
-EXPORT_SYMBOL_GPL(kvmppc_handle_store);
 
 #ifdef CONFIG_VSX
 static inline int kvmppc_get_vsr_data(struct kvm_vcpu *vcpu, int rs, u64 *val)
@@ -2478,26 +2468,22 @@ long kvmppc_alloc_lpid(void)
 
 	return lpid;
 }
-EXPORT_SYMBOL_GPL(kvmppc_alloc_lpid);
 
 void kvmppc_claim_lpid(long lpid)
 {
 	set_bit(lpid, lpid_inuse);
 }
-EXPORT_SYMBOL_GPL(kvmppc_claim_lpid);
 
 void kvmppc_free_lpid(long lpid)
 {
 	clear_bit(lpid, lpid_inuse);
 }
-EXPORT_SYMBOL_GPL(kvmppc_free_lpid);
 
 void kvmppc_init_lpid(unsigned long nr_lpids_param)
 {
 	nr_lpids = min_t(unsigned long, KVMPPC_NR_LPIDS, nr_lpids_param);
 	memset(lpid_inuse, 0, sizeof(lpid_inuse));
 }
-EXPORT_SYMBOL_GPL(kvmppc_init_lpid);
 
 int kvm_arch_init(void *opaque)
 {
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] KVM: PPC: Book3S: Modules cleanup and unification
  2021-09-01 17:33 ` Fabiano Rosas
@ 2021-09-02  1:28   ` David Gibson
  -1 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2021-09-02  1:28 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: linuxppc-dev, npiggin, kvm-ppc

[-- Attachment #1: Type: text/plain, Size: 3261 bytes --]

On Wed, Sep 01, 2021 at 02:33:52PM -0300, Fabiano Rosas wrote:
> This series merges our three kvm modules kvm.ko, kvm-hv.ko and
> kvm-pr.ko into one kvm.ko module.

That doesn't sound like a good idea to me.  People who aren't on BookS
servers don't want - and can't use - kvm-hv.  Almost nobody wants
kvm-pr.  It's also kind of inconsistent with x86, which has the
separate AMD and Intel modules.

> The main reason for this is to deal with the issue that kvm.ko can be
> loaded on its own without any of the other modules present. This can
> happen if one or both of the modules fail to init or if the user loads
> kvm.ko only.
> 
> With only kvm.ko loaded, the userspace can call any of the KVM ioctls
> which will fail more or less gracefully depending on what kind of
> verification we do in powerpc.c.

I see that that's awkward, but I'm not sure it justifies compromising
the actual natural structure of the dependencies.

> Instead of adding a check to every entry point or finding a hack to
> link the modules so that when one fails (hv/pr), the other (kvm)
> exits, I think it is cleaner to just make them all a single module.
> 
> The two KVM implementations are already selected by Kconfig options,
> so the only thing that changes is that they are now in the same
> module. I also kept kvm-hv and kvm-pr as aliases to kvm, so that
> people don't get too surprised with the change.
> 
> There is a possible issue with the larger module size for kernel
> builds that should support both HV-only and PR-only environments, but
> PR is usually not used in production so I'm not sure if that is a real
> issue.
> 
> Patches 1,2,3 are standalone cleanups.
> Patches 4,5 are the unification work.
> 
> Fabiano Rosas (5):
>   KVM: PPC: Book3S HV: Check return value of kvmppc_radix_init
>   KVM: PPC: Book3S HV: Delay setting of kvm ops
>   KVM: PPC: Book3S HV: Free allocated memory if module init fails
>   KVM: PPC: Book3S: Unify kvm-hv and kvm-pr modules
>   KVM: PPC: Book3S: Stop exporting non-builtin symbols
> 
>  arch/powerpc/configs/powernv_defconfig |  2 +-
>  arch/powerpc/configs/ppc64_defconfig   |  2 +-
>  arch/powerpc/configs/pseries_defconfig |  2 +-
>  arch/powerpc/kvm/Kconfig               | 72 ++++++++++++--------------
>  arch/powerpc/kvm/Makefile              | 11 ++--
>  arch/powerpc/kvm/book3s.c              | 61 ++++++++++++++--------
>  arch/powerpc/kvm/book3s.h              | 19 +++++++
>  arch/powerpc/kvm/book3s_64_mmu_radix.c |  3 --
>  arch/powerpc/kvm/book3s_64_vio.c       |  3 --
>  arch/powerpc/kvm/book3s_hv.c           | 38 ++++++++------
>  arch/powerpc/kvm/book3s_pr.c           | 13 -----
>  arch/powerpc/kvm/book3s_rtas.c         |  1 -
>  arch/powerpc/kvm/book3s_xics.c         |  4 --
>  arch/powerpc/kvm/book3s_xive.c         |  6 ---
>  arch/powerpc/kvm/emulate.c             |  1 -
>  arch/powerpc/kvm/powerpc.c             | 14 -----
>  kernel/irq/irqdesc.c                   |  2 +-
>  17 files changed, 125 insertions(+), 129 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] KVM: PPC: Book3S: Modules cleanup and unification
@ 2021-09-02  1:28   ` David Gibson
  0 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2021-09-02  1:28 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: linuxppc-dev, npiggin, kvm-ppc

[-- Attachment #1: Type: text/plain, Size: 3261 bytes --]

On Wed, Sep 01, 2021 at 02:33:52PM -0300, Fabiano Rosas wrote:
> This series merges our three kvm modules kvm.ko, kvm-hv.ko and
> kvm-pr.ko into one kvm.ko module.

That doesn't sound like a good idea to me.  People who aren't on BookS
servers don't want - and can't use - kvm-hv.  Almost nobody wants
kvm-pr.  It's also kind of inconsistent with x86, which has the
separate AMD and Intel modules.

> The main reason for this is to deal with the issue that kvm.ko can be
> loaded on its own without any of the other modules present. This can
> happen if one or both of the modules fail to init or if the user loads
> kvm.ko only.
> 
> With only kvm.ko loaded, the userspace can call any of the KVM ioctls
> which will fail more or less gracefully depending on what kind of
> verification we do in powerpc.c.

I see that that's awkward, but I'm not sure it justifies compromising
the actual natural structure of the dependencies.

> Instead of adding a check to every entry point or finding a hack to
> link the modules so that when one fails (hv/pr), the other (kvm)
> exits, I think it is cleaner to just make them all a single module.
> 
> The two KVM implementations are already selected by Kconfig options,
> so the only thing that changes is that they are now in the same
> module. I also kept kvm-hv and kvm-pr as aliases to kvm, so that
> people don't get too surprised with the change.
> 
> There is a possible issue with the larger module size for kernel
> builds that should support both HV-only and PR-only environments, but
> PR is usually not used in production so I'm not sure if that is a real
> issue.
> 
> Patches 1,2,3 are standalone cleanups.
> Patches 4,5 are the unification work.
> 
> Fabiano Rosas (5):
>   KVM: PPC: Book3S HV: Check return value of kvmppc_radix_init
>   KVM: PPC: Book3S HV: Delay setting of kvm ops
>   KVM: PPC: Book3S HV: Free allocated memory if module init fails
>   KVM: PPC: Book3S: Unify kvm-hv and kvm-pr modules
>   KVM: PPC: Book3S: Stop exporting non-builtin symbols
> 
>  arch/powerpc/configs/powernv_defconfig |  2 +-
>  arch/powerpc/configs/ppc64_defconfig   |  2 +-
>  arch/powerpc/configs/pseries_defconfig |  2 +-
>  arch/powerpc/kvm/Kconfig               | 72 ++++++++++++--------------
>  arch/powerpc/kvm/Makefile              | 11 ++--
>  arch/powerpc/kvm/book3s.c              | 61 ++++++++++++++--------
>  arch/powerpc/kvm/book3s.h              | 19 +++++++
>  arch/powerpc/kvm/book3s_64_mmu_radix.c |  3 --
>  arch/powerpc/kvm/book3s_64_vio.c       |  3 --
>  arch/powerpc/kvm/book3s_hv.c           | 38 ++++++++------
>  arch/powerpc/kvm/book3s_pr.c           | 13 -----
>  arch/powerpc/kvm/book3s_rtas.c         |  1 -
>  arch/powerpc/kvm/book3s_xics.c         |  4 --
>  arch/powerpc/kvm/book3s_xive.c         |  6 ---
>  arch/powerpc/kvm/emulate.c             |  1 -
>  arch/powerpc/kvm/powerpc.c             | 14 -----
>  kernel/irq/irqdesc.c                   |  2 +-
>  17 files changed, 125 insertions(+), 129 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] KVM: PPC: Book3S: Modules cleanup and unification
  2021-09-02  1:28   ` David Gibson
@ 2021-09-02 14:32     ` Fabiano Rosas
  -1 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-02 14:32 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, npiggin, kvm-ppc

David Gibson <david@gibson.dropbear.id.au> writes:

> On Wed, Sep 01, 2021 at 02:33:52PM -0300, Fabiano Rosas wrote:
>> This series merges our three kvm modules kvm.ko, kvm-hv.ko and
>> kvm-pr.ko into one kvm.ko module.
>
> That doesn't sound like a good idea to me.  People who aren't on BookS
> servers don't want - and can't use - kvm-hv.  Almost nobody wants
> kvm-pr.  It's also kind of inconsistent with x86, which has the
> separate AMD and Intel modules.

But this is not altering the ability of having only kvm-hv or only
kvm-pr. I'm taking the Kconfig options that used to produce separate
modules and using them to select which code gets built into the one
kvm.ko module.

Currently:

CONFIG_KVM_BOOK3S_64=m     <-- produces kvm.ko
CONFIG_KVM_BOOK3S_64_HV=m  <-- produces kvm-hv.ko
CONFIG_KVM_BOOK3S_64_PR=m  <-- produces kvm-pr.ko

I'm making it so we now have one kvm.ko everywhere, but there is still:

CONFIG_KVM_BOOK3S_64=m           <-- produces kvm.ko
CONFIG_KVM_BOOK3S_HV_POSSIBLE=y  <-- includes HV in kvm.ko
CONFIG_KVM_BOOK3S_PR_POSSIBLE=y  <-- includes PR in kvm.ko

In other words, if you are going to have at least two modules loaded at
all times (kvm + kvm-hv or kvm + kvm-pr), why not put all that into one
module? No one needs to build code they are not going to use, this is
not changing.

About consistency with x86, this situation is not analogous because we
need to be able to load both modules at the same time, which means
kvm.ko needs to stick around when one module goes away in case we want
to load the other module. The KVM common code states that it expects to
have at most one implementation:

        /*
         * kvm_arch_init makes sure there's at most one caller
         * for architectures that support multiple implementations,
         * like intel and amd on x86.
         (...)

which is not true in our case due to this requirement of having two
separate modules loading independently.

(tangent) We are already quite different from other architectures since
we're not making use of kvm_arch_init and some other KVM hooks, such as
kvm_arch_check_processor_compat. So while other archs have their init
dispatched by kvm common code, our init and cleanup happens
independently in the ppc-specific modules, which obviously works but is
needlessly different and has subtleties in the ordering of operations
wrt. the kvm common code. (tangent)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] KVM: PPC: Book3S: Modules cleanup and unification
@ 2021-09-02 14:32     ` Fabiano Rosas
  0 siblings, 0 replies; 18+ messages in thread
From: Fabiano Rosas @ 2021-09-02 14:32 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, npiggin, kvm-ppc

David Gibson <david@gibson.dropbear.id.au> writes:

> On Wed, Sep 01, 2021 at 02:33:52PM -0300, Fabiano Rosas wrote:
>> This series merges our three kvm modules kvm.ko, kvm-hv.ko and
>> kvm-pr.ko into one kvm.ko module.
>
> That doesn't sound like a good idea to me.  People who aren't on BookS
> servers don't want - and can't use - kvm-hv.  Almost nobody wants
> kvm-pr.  It's also kind of inconsistent with x86, which has the
> separate AMD and Intel modules.

But this is not altering the ability of having only kvm-hv or only
kvm-pr. I'm taking the Kconfig options that used to produce separate
modules and using them to select which code gets built into the one
kvm.ko module.

Currently:

CONFIG_KVM_BOOK3S_64=m     <-- produces kvm.ko
CONFIG_KVM_BOOK3S_64_HV=m  <-- produces kvm-hv.ko
CONFIG_KVM_BOOK3S_64_PR=m  <-- produces kvm-pr.ko

I'm making it so we now have one kvm.ko everywhere, but there is still:

CONFIG_KVM_BOOK3S_64=m           <-- produces kvm.ko
CONFIG_KVM_BOOK3S_HV_POSSIBLE=y  <-- includes HV in kvm.ko
CONFIG_KVM_BOOK3S_PR_POSSIBLE=y  <-- includes PR in kvm.ko

In other words, if you are going to have at least two modules loaded at
all times (kvm + kvm-hv or kvm + kvm-pr), why not put all that into one
module? No one needs to build code they are not going to use, this is
not changing.

About consistency with x86, this situation is not analogous because we
need to be able to load both modules at the same time, which means
kvm.ko needs to stick around when one module goes away in case we want
to load the other module. The KVM common code states that it expects to
have at most one implementation:

        /*
         * kvm_arch_init makes sure there's at most one caller
         * for architectures that support multiple implementations,
         * like intel and amd on x86.
         (...)

which is not true in our case due to this requirement of having two
separate modules loading independently.

(tangent) We are already quite different from other architectures since
we're not making use of kvm_arch_init and some other KVM hooks, such as
kvm_arch_check_processor_compat. So while other archs have their init
dispatched by kvm common code, our init and cleanup happens
independently in the ppc-specific modules, which obviously works but is
needlessly different and has subtleties in the ordering of operations
wrt. the kvm common code. (tangent)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] KVM: PPC: Book3S: Modules cleanup and unification
  2021-09-02 14:32     ` Fabiano Rosas
@ 2021-09-03  5:13       ` David Gibson
  -1 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2021-09-03  5:13 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: linuxppc-dev, npiggin, kvm-ppc

[-- Attachment #1: Type: text/plain, Size: 3208 bytes --]

On Thu, Sep 02, 2021 at 11:32:41AM -0300, Fabiano Rosas wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > On Wed, Sep 01, 2021 at 02:33:52PM -0300, Fabiano Rosas wrote:
> >> This series merges our three kvm modules kvm.ko, kvm-hv.ko and
> >> kvm-pr.ko into one kvm.ko module.
> >
> > That doesn't sound like a good idea to me.  People who aren't on BookS
> > servers don't want - and can't use - kvm-hv.  Almost nobody wants
> > kvm-pr.  It's also kind of inconsistent with x86, which has the
> > separate AMD and Intel modules.
> 
> But this is not altering the ability of having only kvm-hv or only
> kvm-pr. I'm taking the Kconfig options that used to produce separate
> modules and using them to select which code gets built into the one
> kvm.ko module.

> 
> Currently:
> 
> CONFIG_KVM_BOOK3S_64=m     <-- produces kvm.ko
> CONFIG_KVM_BOOK3S_64_HV=m  <-- produces kvm-hv.ko
> CONFIG_KVM_BOOK3S_64_PR=m  <-- produces kvm-pr.ko
> 
> I'm making it so we now have one kvm.ko everywhere, but there is still:
> 
> CONFIG_KVM_BOOK3S_64=m           <-- produces kvm.ko
> CONFIG_KVM_BOOK3S_HV_POSSIBLE=y  <-- includes HV in kvm.ko
> CONFIG_KVM_BOOK3S_PR_POSSIBLE=y  <-- includes PR in kvm.ko
> 
> In other words, if you are going to have at least two modules loaded at
> all times (kvm + kvm-hv or kvm + kvm-pr), why not put all that into one
> module? No one needs to build code they are not going to use, this is
> not changing.

Ah.. I see, you're removing the runtime switch from one to the other
at the same time as having just a single one loaded, but leaving the
ability to compile time switch.  And compile time is arguably good
enough for the cases I've described.

Ok, I see your point.

I still think it's conceptually not ideal, but the practical benefit
is more important.  Objection withdrawn.


> About consistency with x86, this situation is not analogous because we
> need to be able to load both modules at the same time, which means
> kvm.ko needs to stick around when one module goes away in case we want
> to load the other module. The KVM common code states that it expects to
> have at most one implementation:
> 
>         /*
>          * kvm_arch_init makes sure there's at most one caller
>          * for architectures that support multiple implementations,
>          * like intel and amd on x86.
>          (...)
> 
> which is not true in our case due to this requirement of having two
> separate modules loading independently.
> 
> (tangent) We are already quite different from other architectures since
> we're not making use of kvm_arch_init and some other KVM hooks, such as
> kvm_arch_check_processor_compat. So while other archs have their init
> dispatched by kvm common code, our init and cleanup happens
> independently in the ppc-specific modules, which obviously works but is
> needlessly different and has subtleties in the ordering of operations
> wrt. the kvm common code. (tangent)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] KVM: PPC: Book3S: Modules cleanup and unification
@ 2021-09-03  5:13       ` David Gibson
  0 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2021-09-03  5:13 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: linuxppc-dev, npiggin, kvm-ppc

[-- Attachment #1: Type: text/plain, Size: 3208 bytes --]

On Thu, Sep 02, 2021 at 11:32:41AM -0300, Fabiano Rosas wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > On Wed, Sep 01, 2021 at 02:33:52PM -0300, Fabiano Rosas wrote:
> >> This series merges our three kvm modules kvm.ko, kvm-hv.ko and
> >> kvm-pr.ko into one kvm.ko module.
> >
> > That doesn't sound like a good idea to me.  People who aren't on BookS
> > servers don't want - and can't use - kvm-hv.  Almost nobody wants
> > kvm-pr.  It's also kind of inconsistent with x86, which has the
> > separate AMD and Intel modules.
> 
> But this is not altering the ability of having only kvm-hv or only
> kvm-pr. I'm taking the Kconfig options that used to produce separate
> modules and using them to select which code gets built into the one
> kvm.ko module.

> 
> Currently:
> 
> CONFIG_KVM_BOOK3S_64=m     <-- produces kvm.ko
> CONFIG_KVM_BOOK3S_64_HV=m  <-- produces kvm-hv.ko
> CONFIG_KVM_BOOK3S_64_PR=m  <-- produces kvm-pr.ko
> 
> I'm making it so we now have one kvm.ko everywhere, but there is still:
> 
> CONFIG_KVM_BOOK3S_64=m           <-- produces kvm.ko
> CONFIG_KVM_BOOK3S_HV_POSSIBLE=y  <-- includes HV in kvm.ko
> CONFIG_KVM_BOOK3S_PR_POSSIBLE=y  <-- includes PR in kvm.ko
> 
> In other words, if you are going to have at least two modules loaded at
> all times (kvm + kvm-hv or kvm + kvm-pr), why not put all that into one
> module? No one needs to build code they are not going to use, this is
> not changing.

Ah.. I see, you're removing the runtime switch from one to the other
at the same time as having just a single one loaded, but leaving the
ability to compile time switch.  And compile time is arguably good
enough for the cases I've described.

Ok, I see your point.

I still think it's conceptually not ideal, but the practical benefit
is more important.  Objection withdrawn.


> About consistency with x86, this situation is not analogous because we
> need to be able to load both modules at the same time, which means
> kvm.ko needs to stick around when one module goes away in case we want
> to load the other module. The KVM common code states that it expects to
> have at most one implementation:
> 
>         /*
>          * kvm_arch_init makes sure there's at most one caller
>          * for architectures that support multiple implementations,
>          * like intel and amd on x86.
>          (...)
> 
> which is not true in our case due to this requirement of having two
> separate modules loading independently.
> 
> (tangent) We are already quite different from other architectures since
> we're not making use of kvm_arch_init and some other KVM hooks, such as
> kvm_arch_check_processor_compat. So while other archs have their init
> dispatched by kvm common code, our init and cleanup happens
> independently in the ppc-specific modules, which obviously works but is
> needlessly different and has subtleties in the ordering of operations
> wrt. the kvm common code. (tangent)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-09-03  5:14 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-01 17:33 [PATCH 0/5] KVM: PPC: Book3S: Modules cleanup and unification Fabiano Rosas
2021-09-01 17:33 ` Fabiano Rosas
2021-09-01 17:33 ` [PATCH 1/5] KVM: PPC: Book3S HV: Check return value of kvmppc_radix_init Fabiano Rosas
2021-09-01 17:33   ` Fabiano Rosas
2021-09-01 17:33 ` [PATCH 2/5] KVM: PPC: Book3S HV: Delay setting of kvm ops Fabiano Rosas
2021-09-01 17:33   ` Fabiano Rosas
2021-09-01 17:33 ` [PATCH 3/5] KVM: PPC: Book3S HV: Free allocated memory if module init fails Fabiano Rosas
2021-09-01 17:33   ` Fabiano Rosas
2021-09-01 17:33 ` [PATCH 4/5] KVM: PPC: Book3S: Unify kvm-hv and kvm-pr modules Fabiano Rosas
2021-09-01 17:33   ` Fabiano Rosas
2021-09-01 17:33 ` [PATCH 5/5] KVM: PPC: Book3S: Stop exporting non-builtin symbols Fabiano Rosas
2021-09-01 17:33   ` Fabiano Rosas
2021-09-02  1:28 ` [PATCH 0/5] KVM: PPC: Book3S: Modules cleanup and unification David Gibson
2021-09-02  1:28   ` David Gibson
2021-09-02 14:32   ` Fabiano Rosas
2021-09-02 14:32     ` Fabiano Rosas
2021-09-03  5:13     ` David Gibson
2021-09-03  5:13       ` David Gibson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.