Re: [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg

2021-05-04 Thread David Gibson
On Tue, May 04, 2021 at 03:14:17PM -0300, Lucas Mateus Martins Araujo e Castro 
wrote:
> 
> On 03/05/2021 01:34, David Gibson wrote:
> > On Fri, Apr 30, 2021 at 03:40:47PM -0300, Lucas Mateus Castro (alqotel) 
> > wrote:
> > > Moved h_enter, remove_hpte, h_remove, h_bulk_remove,h_protect and
> > > h_read to spapr_hcall_tcg.c, added h_tcg_only to be used in a !TCG
> > > environment in spapr_hcall.c and changed build options to only build
> > > spapr_hcall_tcg.c when CONFIG_TCG is enabled.
> > This looks good.  I'd suggest the name 'spapr_softmmu.c' instead,
> > which more specifically describes what's special about these
> > functions.
> > 
> > h_resize_hpt_prepare(), h_resize_hpt_commit() and the functions they
> > depend on belong in the softmmu set as well.
> 
> Moved these hcalls to spapr_softmmu.c as well, along with the most
> functions they depend on, but 1 function (push_sregs_to_kvm_pr) is
> also used by hcalls in spapr_hcall.c, so for now I'm just leaving it in
> spapr_hcall.c and exporting it to be used in spapr_softmmu.c.

Right.  That one's an ugly workaround for some weird KVM PR behaviour,
and we will need it in the common code.

> On a related note, from what I've seen h_resize_hpt_prepare and
> h_resize_hpt_commit are not implementede in KVM, so they're only
> called when there's softmmu so that's why they can be moved to
> spapr_softmmu.c?

Ah, sorry, I forgot how these worked.  The bulk of the logic to
implement these *is* in KVM, but KVM doesn't directly intercept them
as hcalls.  Instead the hcall is caught by qemu, but it just forwards
them back to KVM by calling an ioctl().  See the calls to
kvmppc_resize_hpt_prepare() and kvmppc_resize_hpt_commit() in
spapr_hcall.c.

[Aside: the reason for this is that they're not latency critical, and
 it's useful for qemu to be aware of and possibly filter HPT resize
 requests, but because KVM manages the HPT, qemu can't implement these
 directly for the KVM case]

So what we'll need to do for those is keep the first part if
h_resize_hpt_{prepare,commit}() in common code - that's basically just
parameter validation - up to the call to
kvmppc_resize_hpt_{prepare,commit}().

if the kvmppc_() call fails with -ENOSYS then we need to do
if (kvm_enabled()) {
return H_HARDWARE;
}
softmmu_resize_hpt_{prepare,commit}()

Where that softmmu_..() function is the remainder of the logic that's
in the current implementation.  That bit can move to the softmmu file
and can be stubbed with an assert-not-reached for the !TCG case.

In practice we should never actually hit that H_HARDWARE - we should
have failed at machine setup if we tried to enable the resize HPT
feature on a KVM that doesn't implement it.

-- 
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


signature.asc
Description: PGP signature


Re: [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg

2021-05-04 Thread Lucas Mateus Martins Araujo e Castro


On 03/05/2021 01:34, David Gibson wrote:

On Fri, Apr 30, 2021 at 03:40:47PM -0300, Lucas Mateus Castro (alqotel) wrote:

Moved h_enter, remove_hpte, h_remove, h_bulk_remove,h_protect and
h_read to spapr_hcall_tcg.c, added h_tcg_only to be used in a !TCG
environment in spapr_hcall.c and changed build options to only build
spapr_hcall_tcg.c when CONFIG_TCG is enabled.

This looks good.  I'd suggest the name 'spapr_softmmu.c' instead,
which more specifically describes what's special about these
functions.

h_resize_hpt_prepare(), h_resize_hpt_commit() and the functions they
depend on belong in the softmmu set as well.


Moved these hcalls to spapr_softmmu.c as well, along with the most
functions they depend on, but 1 function (push_sregs_to_kvm_pr) is
also used by hcalls in spapr_hcall.c, so for now I'm just leaving it in
spapr_hcall.c and exporting it to be used in spapr_softmmu.c.

On a related note, from what I've seen h_resize_hpt_prepare and
h_resize_hpt_commit are not implementede in KVM, so they're only
called when there's softmmu so that's why they can be moved to
spapr_softmmu.c?

Added the function h_tcg_only to be used for hypercalls that should be
called only in TCG environment but have been called in a TCG-less
one.

Again, 'h_softmmu' would be a better name.

Ok, I will use that name.

Right now, a #ifndef is used to know if there is a need of a h_tcg_only
function to be implemented and used as hypercalls, I initially thought
of always having that option turned on and having spapr_hcall_tcg.c
overwrite those hypercalls when TCG is enabled, but
spapr_register_hypercalls checks if a hypercall already exists for that
opcode so that wouldn't work, so if anyone has any suggestions I'm
interested.

The ifdef is fine.  We don't want to litter the code with them, but a
few is fine.  Especially in this context where it's pretty clearly
just excluding some things from a simple list of calls.


Also spapr_hcall_tcg.c only has 2 duplicated functions (valid_ptex and
is_ram_address), what is the advised way to deal with these
duplications?

valid_ptex() is only used by softmmu functions that are moving, so it
should travel with them, no need for duplication.  is_ram_address() is
also used by h_page_init() which is also needed in !TCG code.  So,
leave it in spapr_hcall.c and just export it for use in the TCG only
code.


On both is_ram_address and push_sregs_to_kvm_pr I exported them

by adding their prototypes to spapr.h and made them non-static.


Signed-off-by: Lucas Mateus Castro (alqotel)
---
  hw/ppc/meson.build   |   3 +
  hw/ppc/spapr_hcall.c | 300 ++
  hw/ppc/spapr_hcall_tcg.c | 343 +++
  3 files changed, 363 insertions(+), 283 deletions(-)
  create mode 100644 hw/ppc/spapr_hcall_tcg.c

diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index 218631c883..3c7f2f08b7 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -29,6 +29,9 @@ ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files(
'spapr_numa.c',
'pef.c',
  ))
+ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], if_true: files(
+  'spapr_hcall_tcg.c'
+))
  ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
  ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
'spapr_pci_vfio.c',
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 4b0ba69841..b37fbdc32e 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -22,6 +22,15 @@
  #include "mmu-book3s-v3.h"
  #include "hw/mem/memory-device.h"
  
+#ifndef CONFIG_TCG

+static target_ulong h_tcg_only(PowerPCCPU *cpu, SpaprMachineState *spapr,
+target_ulong opcode, target_ulong *args)
+{
+g_assert_not_reached();
+return 0;
+}
+#endif /* !CONFIG_TCG */
+
  static bool has_spr(PowerPCCPU *cpu, int spr)
  {
  /* We can test whether the SPR is defined by checking for a valid name */
@@ -55,284 +64,6 @@ static bool is_ram_address(SpaprMachineState *spapr, hwaddr 
addr)
  return false;
  }
  
-static target_ulong h_enter(PowerPCCPU *cpu, SpaprMachineState *spapr,

-target_ulong opcode, target_ulong *args)
-{
-target_ulong flags = args[0];
-target_ulong ptex = args[1];
-target_ulong pteh = args[2];
-target_ulong ptel = args[3];
-unsigned apshift;
-target_ulong raddr;
-target_ulong slot;
-const ppc_hash_pte64_t *hptes;
-
-apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
-if (!apshift) {
-/* Bad page size encoding */
-return H_PARAMETER;
-}
-
-raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
-
-if (is_ram_address(spapr, raddr)) {
-/* Regular RAM - should have WIMG=0010 */
-if ((ptel & HPTE64_R_WIMG) != HPTE64_R_M) {
-return H_PARAMETER;
-}
-} else {
-target_ulong wimg_flags;
-/* Looks like an IO address */
-/* FIXME: What WIMG combinations could be 

Re: [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg

2021-05-02 Thread David Gibson
On Fri, Apr 30, 2021 at 03:40:47PM -0300, Lucas Mateus Castro (alqotel) wrote:
> Moved h_enter, remove_hpte, h_remove, h_bulk_remove,h_protect and
> h_read to spapr_hcall_tcg.c, added h_tcg_only to be used in a !TCG
> environment in spapr_hcall.c and changed build options to only build
> spapr_hcall_tcg.c when CONFIG_TCG is enabled.

This looks good.  I'd suggest the name 'spapr_softmmu.c' instead,
which more specifically describes what's special about these
functions.

h_resize_hpt_prepare(), h_resize_hpt_commit() and the functions they
depend on belong in the softmmu set as well.

> Added the function h_tcg_only to be used for hypercalls that should be
> called only in TCG environment but have been called in a TCG-less
> one.

Again, 'h_softmmu' would be a better name.

> 
> Right now, a #ifndef is used to know if there is a need of a h_tcg_only
> function to be implemented and used as hypercalls, I initially thought
> of always having that option turned on and having spapr_hcall_tcg.c
> overwrite those hypercalls when TCG is enabled, but
> spapr_register_hypercalls checks if a hypercall already exists for that
> opcode so that wouldn't work, so if anyone has any suggestions I'm
> interested.

The ifdef is fine.  We don't want to litter the code with them, but a
few is fine.  Especially in this context where it's pretty clearly
just excluding some things from a simple list of calls.

> 
> Also spapr_hcall_tcg.c only has 2 duplicated functions (valid_ptex and
> is_ram_address), what is the advised way to deal with these
> duplications?

valid_ptex() is only used by softmmu functions that are moving, so it
should travel with them, no need for duplication.  is_ram_address() is
also used by h_page_init() which is also needed in !TCG code.  So,
leave it in spapr_hcall.c and just export it for use in the TCG only
code.

> 
> Signed-off-by: Lucas Mateus Castro (alqotel) 
> ---
>  hw/ppc/meson.build   |   3 +
>  hw/ppc/spapr_hcall.c | 300 ++
>  hw/ppc/spapr_hcall_tcg.c | 343 +++
>  3 files changed, 363 insertions(+), 283 deletions(-)
>  create mode 100644 hw/ppc/spapr_hcall_tcg.c
> 
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index 218631c883..3c7f2f08b7 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -29,6 +29,9 @@ ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files(
>'spapr_numa.c',
>'pef.c',
>  ))
> +ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], if_true: files(
> +  'spapr_hcall_tcg.c'
> +))
>  ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
>  ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
>'spapr_pci_vfio.c',
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 4b0ba69841..b37fbdc32e 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -22,6 +22,15 @@
>  #include "mmu-book3s-v3.h"
>  #include "hw/mem/memory-device.h"
>  
> +#ifndef CONFIG_TCG
> +static target_ulong h_tcg_only(PowerPCCPU *cpu, SpaprMachineState *spapr,
> +target_ulong opcode, target_ulong *args)
> +{
> +g_assert_not_reached();
> +return 0;
> +}
> +#endif /* !CONFIG_TCG */
> +
>  static bool has_spr(PowerPCCPU *cpu, int spr)
>  {
>  /* We can test whether the SPR is defined by checking for a valid name */
> @@ -55,284 +64,6 @@ static bool is_ram_address(SpaprMachineState *spapr, 
> hwaddr addr)
>  return false;
>  }
>  
> -static target_ulong h_enter(PowerPCCPU *cpu, SpaprMachineState *spapr,
> -target_ulong opcode, target_ulong *args)
> -{
> -target_ulong flags = args[0];
> -target_ulong ptex = args[1];
> -target_ulong pteh = args[2];
> -target_ulong ptel = args[3];
> -unsigned apshift;
> -target_ulong raddr;
> -target_ulong slot;
> -const ppc_hash_pte64_t *hptes;
> -
> -apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
> -if (!apshift) {
> -/* Bad page size encoding */
> -return H_PARAMETER;
> -}
> -
> -raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
> -
> -if (is_ram_address(spapr, raddr)) {
> -/* Regular RAM - should have WIMG=0010 */
> -if ((ptel & HPTE64_R_WIMG) != HPTE64_R_M) {
> -return H_PARAMETER;
> -}
> -} else {
> -target_ulong wimg_flags;
> -/* Looks like an IO address */
> -/* FIXME: What WIMG combinations could be sensible for IO?
> - * For now we allow WIMG=010x, but are there others? */
> -/* FIXME: Should we check against registered IO addresses? */
> -wimg_flags = (ptel & (HPTE64_R_W | HPTE64_R_I | HPTE64_R_M));
> -
> -if (wimg_flags != HPTE64_R_I &&
> -wimg_flags != (HPTE64_R_I | HPTE64_R_M)) {
> -return H_PARAMETER;
> -}
> -}
> -
> -pteh &= ~0x60ULL;
> -
> -if (!valid_ptex(cpu, ptex)) {
> -return H_PARAMETER;
> -}
> -
> -  

Re: [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg

2021-05-02 Thread David Gibson
On Fri, Apr 30, 2021 at 08:38:05PM -0300, Fabiano Rosas wrote:
> "Lucas Mateus Castro (alqotel)"  writes:
> 
> > Also spapr_hcall_tcg.c only has 2 duplicated functions (valid_ptex and
> > is_ram_address), what is the advised way to deal with these
> > duplications?
> 
> valid_ptex is only needed by the TCG hcalls isn't it?
> 
> is_ram_address could in theory stay where it is but be exposed via
> hw/ppc/spapr.h since spapr_hcall.c will always be present.
> 
> > Signed-off-by: Lucas Mateus Castro (alqotel) 
> > ---
> >  hw/ppc/meson.build   |   3 +
> >  hw/ppc/spapr_hcall.c | 300 ++
> >  hw/ppc/spapr_hcall_tcg.c | 343 +++
> >  3 files changed, 363 insertions(+), 283 deletions(-)
> >  create mode 100644 hw/ppc/spapr_hcall_tcg.c
> 
> 
> 
> > @@ -2021,14 +1752,17 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, 
> > target_ulong opcode,
> >  
> >  static void hypercall_register_types(void)
> >  {
> > +
> > +#ifndef CONFIG_TCG
> >  /* hcall-pft */
> > -spapr_register_hypercall(H_ENTER, h_enter);
> > -spapr_register_hypercall(H_REMOVE, h_remove);
> > -spapr_register_hypercall(H_PROTECT, h_protect);
> > -spapr_register_hypercall(H_READ, h_read);
> > +spapr_register_hypercall(H_ENTER, h_tcg_only);
> > +spapr_register_hypercall(H_REMOVE, h_tcg_only);
> > +spapr_register_hypercall(H_PROTECT, h_tcg_only);
> > +spapr_register_hypercall(H_READ, h_tcg_only);
> >  
> >  /* hcall-bulk */
> > -spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
> > +spapr_register_hypercall(H_BULK_REMOVE, h_tcg_only);
> > +#endif /* !CONFIG_TCG */
> 
> My suggestion for this was:
> 
> #ifndef CONFIG_TCG
> static target_ulong h_tcg_only(PowerPCCPU *cpu, SpaprMachineState *spapr,
>target_ulong opcode, target_ulong *args)
> {
> g_assert_not_reached();
> }
> 
> static void hypercall_register_tcg(void)
> {
> spapr_register_hypercall(H_ENTER, h_tcg_only);
> spapr_register_hypercall(H_REMOVE, h_tcg_only);
> spapr_register_hypercall(H_PROTECT, h_tcg_only);
> spapr_register_hypercall(H_READ, h_tcg_only);
> (...)
> }
> #endif
> 
> static void hypercall_register_types(void)
> {
> hypercall_register_tcg();
> 
> 
> }
> type_init(hypercall_register_types);

Eh, swings and roundabouts.  Either of these approaches is fine.

> > +static void hypercall_register_types(void)
> > +{
> > +/* hcall-pft */
> > +spapr_register_hypercall(H_ENTER, h_enter);
> > +spapr_register_hypercall(H_REMOVE, h_remove);
> > +spapr_register_hypercall(H_PROTECT, h_protect);
> > +spapr_register_hypercall(H_READ, h_read);
> > +
> > +/* hcall-bulk */
> > +spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
> > +}
> > +
> > +type_init(hypercall_register_types)
> 
> And here:
> 
> void hypercall_register_tcg(void)
> {
> /* hcall-pft */
> spapr_register_hypercall(H_ENTER, h_enter);
> spapr_register_hypercall(H_REMOVE, h_remove);
> spapr_register_hypercall(H_PROTECT, h_protect);
> spapr_register_hypercall(H_READ, h_read);
> (...)
> }
> 
> Because the TCG and KVM builds are not mutually exlusive, so you would
> end up calling type_init twice (which I don't know much about but I
> assume is not allowed).
> 

-- 
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


signature.asc
Description: PGP signature


Re: [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg

2021-04-30 Thread Fabiano Rosas
"Lucas Mateus Castro (alqotel)"  writes:

> Also spapr_hcall_tcg.c only has 2 duplicated functions (valid_ptex and
> is_ram_address), what is the advised way to deal with these
> duplications?

valid_ptex is only needed by the TCG hcalls isn't it?

is_ram_address could in theory stay where it is but be exposed via
hw/ppc/spapr.h since spapr_hcall.c will always be present.

> Signed-off-by: Lucas Mateus Castro (alqotel) 
> ---
>  hw/ppc/meson.build   |   3 +
>  hw/ppc/spapr_hcall.c | 300 ++
>  hw/ppc/spapr_hcall_tcg.c | 343 +++
>  3 files changed, 363 insertions(+), 283 deletions(-)
>  create mode 100644 hw/ppc/spapr_hcall_tcg.c



> @@ -2021,14 +1752,17 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, 
> target_ulong opcode,
>  
>  static void hypercall_register_types(void)
>  {
> +
> +#ifndef CONFIG_TCG
>  /* hcall-pft */
> -spapr_register_hypercall(H_ENTER, h_enter);
> -spapr_register_hypercall(H_REMOVE, h_remove);
> -spapr_register_hypercall(H_PROTECT, h_protect);
> -spapr_register_hypercall(H_READ, h_read);
> +spapr_register_hypercall(H_ENTER, h_tcg_only);
> +spapr_register_hypercall(H_REMOVE, h_tcg_only);
> +spapr_register_hypercall(H_PROTECT, h_tcg_only);
> +spapr_register_hypercall(H_READ, h_tcg_only);
>  
>  /* hcall-bulk */
> -spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
> +spapr_register_hypercall(H_BULK_REMOVE, h_tcg_only);
> +#endif /* !CONFIG_TCG */

My suggestion for this was:

#ifndef CONFIG_TCG
static target_ulong h_tcg_only(PowerPCCPU *cpu, SpaprMachineState *spapr,
   target_ulong opcode, target_ulong *args)
{
g_assert_not_reached();
}

static void hypercall_register_tcg(void)
{
spapr_register_hypercall(H_ENTER, h_tcg_only);
spapr_register_hypercall(H_REMOVE, h_tcg_only);
spapr_register_hypercall(H_PROTECT, h_tcg_only);
spapr_register_hypercall(H_READ, h_tcg_only);
(...)
}
#endif

static void hypercall_register_types(void)
{
hypercall_register_tcg();


}
type_init(hypercall_register_types);

> +static void hypercall_register_types(void)
> +{
> +/* hcall-pft */
> +spapr_register_hypercall(H_ENTER, h_enter);
> +spapr_register_hypercall(H_REMOVE, h_remove);
> +spapr_register_hypercall(H_PROTECT, h_protect);
> +spapr_register_hypercall(H_READ, h_read);
> +
> +/* hcall-bulk */
> +spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
> +}
> +
> +type_init(hypercall_register_types)

And here:

void hypercall_register_tcg(void)
{
/* hcall-pft */
spapr_register_hypercall(H_ENTER, h_enter);
spapr_register_hypercall(H_REMOVE, h_remove);
spapr_register_hypercall(H_PROTECT, h_protect);
spapr_register_hypercall(H_READ, h_read);
(...)
}

Because the TCG and KVM builds are not mutually exlusive, so you would
end up calling type_init twice (which I don't know much about but I
assume is not allowed).




[RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg

2021-04-30 Thread Lucas Mateus Castro (alqotel)
Moved h_enter, remove_hpte, h_remove, h_bulk_remove,h_protect and
h_read to spapr_hcall_tcg.c, added h_tcg_only to be used in a !TCG
environment in spapr_hcall.c and changed build options to only build
spapr_hcall_tcg.c when CONFIG_TCG is enabled.

Added the function h_tcg_only to be used for hypercalls that should be
called only in TCG environment but have been called in a TCG-less one.

Right now, a #ifndef is used to know if there is a need of a h_tcg_only
function to be implemented and used as hypercalls, I initially thought
of always having that option turned on and having spapr_hcall_tcg.c
overwrite those hypercalls when TCG is enabled, but
spapr_register_hypercalls checks if a hypercall already exists for that
opcode so that wouldn't work, so if anyone has any suggestions I'm
interested.

Also spapr_hcall_tcg.c only has 2 duplicated functions (valid_ptex and
is_ram_address), what is the advised way to deal with these
duplications?

Signed-off-by: Lucas Mateus Castro (alqotel) 
---
 hw/ppc/meson.build   |   3 +
 hw/ppc/spapr_hcall.c | 300 ++
 hw/ppc/spapr_hcall_tcg.c | 343 +++
 3 files changed, 363 insertions(+), 283 deletions(-)
 create mode 100644 hw/ppc/spapr_hcall_tcg.c

diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index 218631c883..3c7f2f08b7 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -29,6 +29,9 @@ ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files(
   'spapr_numa.c',
   'pef.c',
 ))
+ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], if_true: files(
+  'spapr_hcall_tcg.c'
+))
 ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
 ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
   'spapr_pci_vfio.c',
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 4b0ba69841..b37fbdc32e 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -22,6 +22,15 @@
 #include "mmu-book3s-v3.h"
 #include "hw/mem/memory-device.h"
 
+#ifndef CONFIG_TCG
+static target_ulong h_tcg_only(PowerPCCPU *cpu, SpaprMachineState *spapr,
+target_ulong opcode, target_ulong *args)
+{
+g_assert_not_reached();
+return 0;
+}
+#endif /* !CONFIG_TCG */
+
 static bool has_spr(PowerPCCPU *cpu, int spr)
 {
 /* We can test whether the SPR is defined by checking for a valid name */
@@ -55,284 +64,6 @@ static bool is_ram_address(SpaprMachineState *spapr, hwaddr 
addr)
 return false;
 }
 
-static target_ulong h_enter(PowerPCCPU *cpu, SpaprMachineState *spapr,
-target_ulong opcode, target_ulong *args)
-{
-target_ulong flags = args[0];
-target_ulong ptex = args[1];
-target_ulong pteh = args[2];
-target_ulong ptel = args[3];
-unsigned apshift;
-target_ulong raddr;
-target_ulong slot;
-const ppc_hash_pte64_t *hptes;
-
-apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
-if (!apshift) {
-/* Bad page size encoding */
-return H_PARAMETER;
-}
-
-raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
-
-if (is_ram_address(spapr, raddr)) {
-/* Regular RAM - should have WIMG=0010 */
-if ((ptel & HPTE64_R_WIMG) != HPTE64_R_M) {
-return H_PARAMETER;
-}
-} else {
-target_ulong wimg_flags;
-/* Looks like an IO address */
-/* FIXME: What WIMG combinations could be sensible for IO?
- * For now we allow WIMG=010x, but are there others? */
-/* FIXME: Should we check against registered IO addresses? */
-wimg_flags = (ptel & (HPTE64_R_W | HPTE64_R_I | HPTE64_R_M));
-
-if (wimg_flags != HPTE64_R_I &&
-wimg_flags != (HPTE64_R_I | HPTE64_R_M)) {
-return H_PARAMETER;
-}
-}
-
-pteh &= ~0x60ULL;
-
-if (!valid_ptex(cpu, ptex)) {
-return H_PARAMETER;
-}
-
-slot = ptex & 7ULL;
-ptex = ptex & ~7ULL;
-
-if (likely((flags & H_EXACT) == 0)) {
-hptes = ppc_hash64_map_hptes(cpu, ptex, HPTES_PER_GROUP);
-for (slot = 0; slot < 8; slot++) {
-if (!(ppc_hash64_hpte0(cpu, hptes, slot) & HPTE64_V_VALID)) {
-break;
-}
-}
-ppc_hash64_unmap_hptes(cpu, hptes, ptex, HPTES_PER_GROUP);
-if (slot == 8) {
-return H_PTEG_FULL;
-}
-} else {
-hptes = ppc_hash64_map_hptes(cpu, ptex + slot, 1);
-if (ppc_hash64_hpte0(cpu, hptes, 0) & HPTE64_V_VALID) {
-ppc_hash64_unmap_hptes(cpu, hptes, ptex + slot, 1);
-return H_PTEG_FULL;
-}
-ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
-}
-
-spapr_store_hpte(cpu, ptex + slot, pteh | HPTE64_V_HPTE_DIRTY, ptel);
-
-args[0] = ptex + slot;
-return H_SUCCESS;
-}
-
-typedef enum {
-REMOVE_SUCCESS = 0,
-REMOVE_NOT_FOUND = 1,
-REMOVE_PARM = 2,
-REMOVE_HW = 3,
-} RemoveResult;
-
-static RemoveResult remove_hpte(PowerPCCPU