[RFC,10/11] target/ppc: created tcg-stub.c file

Created a file with stubs needed to compile disabling TCG.

We're not sure about keeping the softmmu stubs in this file. if there is
a better place to put them, please let us know.

The other 3 functions have been stubbed because we didn't know what to
do with them. Making the file compile in the !TCG case would create an
ifdef hell, but extracting the functions meant moving many others as
well, and there weren't any good places to put them.

Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
---
 target/ppc/tcg-stub.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 target/ppc/tcg-stub.c

On 5/12/21 9:08 AM, Bruno Larsen (billionai) wrote:
> +++ b/target/ppc/tcg-stub.c
> @@ -0,0 +1,33 @@
> +
> +#include "qemu/osdep.h"

All files get copyright boilerplate.

> +#include "exec/hwaddr.h"
> +#include "cpu.h"
> +#include "hw/ppc/spapr.h"
> +
> +hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> +{
> +    return 0;
> +}

This is used by gdbstub.

If there's a way for kvm to convert a virtual address to a physical address 
using the hardware, then use that.  I suspect there is not.

Otherwise, you have to keep all of the mmu page table walking stuff for kvm as 
well as tcg.  Which probably means that all of the other stuff that you're 
stubbing out is used or usable as well.


r~

On 12/05/2021 15:39, Richard Henderson wrote:
> On 5/12/21 9:08 AM, Bruno Larsen (billionai) wrote:
>> +++ b/target/ppc/tcg-stub.c
>> @@ -0,0 +1,33 @@
>> +
>> +#include "qemu/osdep.h"
>
> All files get copyright boilerplate.
yeah, I didn't expect this file to stick around, though, as the last 
time we made a stub file, it ended up not being used, so I decided to go 
the quick route
>
>> +#include "exec/hwaddr.h"
>> +#include "cpu.h"
>> +#include "hw/ppc/spapr.h"
>> +
>> +hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
>> +{
>> +    return 0;
>> +}
>
> This is used by gdbstub.
>
> If there's a way for kvm to convert a virtual address to a physical 
> address using the hardware, then use that.  I suspect there is not.
>
> Otherwise, you have to keep all of the mmu page table walking stuff 
> for kvm as well as tcg.  Which probably means that all of the other 
> stuff that you're stubbing out is used or usable as well.
ah, this probably means we'll need to compile mmu_helper.c too... that 
was something we were hoping to avoid, because of the sheer size.
>
>
> r~

On Wed, May 12, 2021 at 11:08:12AM -0300, Bruno Larsen (billionai) wrote:
> Created a file with stubs needed to compile disabling TCG.
> 
> We're not sure about keeping the softmmu stubs in this file. if there is
> a better place to put them, please let us know.
> 
> The other 3 functions have been stubbed because we didn't know what to
> do with them. Making the file compile in the !TCG case would create an
> ifdef hell, but extracting the functions meant moving many others as
> well, and there weren't any good places to put them.
> 
> Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> ---
>  target/ppc/tcg-stub.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 target/ppc/tcg-stub.c
> 
> diff --git a/target/ppc/tcg-stub.c b/target/ppc/tcg-stub.c
> new file mode 100644
> index 0000000000..67099e2676
> --- /dev/null
> +++ b/target/ppc/tcg-stub.c
> @@ -0,0 +1,33 @@
> +
> +#include "qemu/osdep.h"
> +#include "exec/hwaddr.h"
> +#include "cpu.h"
> +#include "hw/ppc/spapr.h"
> +
> +hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> +{
> +    return 0;
> +}
> +
> +void dump_mmu(CPUPPCState *env)
> +{
> +}
> +
> +void ppc_tlb_invalidate_all(CPUPPCState *env)
> +{
> +}
> +
> +target_ulong softmmu_resize_hpt_prepare(PowerPCCPU *cpu,
> +                                        SpaprMachineState *spapr,
> +                                        target_ulong shift)
> +{
> +    g_assert_not_reached();
> +}
> +
> +target_ulong softmmu_resize_hpt_commit(PowerPCCPU* cpu,
> +                                       SpaprMachineState *spapr,
> +                                       target_ulong flags,
> +                                       target_ulong shift)
> +{
> +    g_assert_not_reached();
> +}

I think these last two stubs should be obsoleted by the patch from
Lucas I already merged "hw/ppc: moved hcalls that depend on softmmu".

On 13/05/2021 00:59, David Gibson wrote:
> On Wed, May 12, 2021 at 11:08:12AM -0300, Bruno Larsen (billionai) wrote:
>> Created a file with stubs needed to compile disabling TCG.
>>
>> We're not sure about keeping the softmmu stubs in this file. if there is
>> a better place to put them, please let us know.
>>
>> The other 3 functions have been stubbed because we didn't know what to
>> do with them. Making the file compile in the !TCG case would create an
>> ifdef hell, but extracting the functions meant moving many others as
>> well, and there weren't any good places to put them.
>>
>> Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
>> ---
>>   target/ppc/tcg-stub.c | 33 +++++++++++++++++++++++++++++++++
>>   1 file changed, 33 insertions(+)
>>   create mode 100644 target/ppc/tcg-stub.c
>>
>> diff --git a/target/ppc/tcg-stub.c b/target/ppc/tcg-stub.c
>> new file mode 100644
>> index 0000000000..67099e2676
>> --- /dev/null
>> +++ b/target/ppc/tcg-stub.c
>> @@ -0,0 +1,33 @@
>> +
>> +#include "qemu/osdep.h"
>> +#include "exec/hwaddr.h"
>> +#include "cpu.h"
>> +#include "hw/ppc/spapr.h"
>> +
>> +hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
>> +{
>> +    return 0;
>> +}
>> +
>> +void dump_mmu(CPUPPCState *env)
>> +{
>> +}
>> +
>> +void ppc_tlb_invalidate_all(CPUPPCState *env)
>> +{
>> +}
>> +
>> +target_ulong softmmu_resize_hpt_prepare(PowerPCCPU *cpu,
>> +                                        SpaprMachineState *spapr,
>> +                                        target_ulong shift)
>> +{
>> +    g_assert_not_reached();
>> +}
>> +
>> +target_ulong softmmu_resize_hpt_commit(PowerPCCPU* cpu,
>> +                                       SpaprMachineState *spapr,
>> +                                       target_ulong flags,
>> +                                       target_ulong shift)
>> +{
>> +    g_assert_not_reached();
>> +}
> I think these last two stubs should be obsoleted by the patch from
> Lucas I already merged "hw/ppc: moved hcalls that depend on softmmu".

They aren't, when talking to him he said he wanted to use as few ifdefs 
as possible. Which do you think is better, to go back and ifdef away 
those calls, or keep the stubs? And if we keep the stubs, do we keep 
them here or in hw/ppc/spapr_hcall.c, along with other stubs?

On 12/05/2021 15:39, Richard Henderson wrote:
> On 5/12/21 9:08 AM, Bruno Larsen (billionai) wrote:
>> +++ b/target/ppc/tcg-stub.c
>> @@ -0,0 +1,33 @@
>> +
>> +#include "qemu/osdep.h"
>
> All files get copyright boilerplate.
>
>> +#include "exec/hwaddr.h"
>> +#include "cpu.h"
>> +#include "hw/ppc/spapr.h"
>> +
>> +hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
>> +{
>> +    return 0;
>> +}
>
> This is used by gdbstub.
>
> If there's a way for kvm to convert a virtual address to a physical 
> address using the hardware, then use that.  I suspect there is not.
>
> Otherwise, you have to keep all of the mmu page table walking stuff 
> for kvm as well as tcg.  Which probably means that all of the other 
> stuff that you're stubbing out is used or usable as well.

 From what I can tell, KVM can't do it, so we'll have to extract the 
function. Looking at it, the main problem is that it might call 
get_physical_address and use struct mmu_ctx_t, if the mmu_model isn't 
one of: POWERPC_MMU_64B, POWERPC_MMU_2_03, POWERPC_MMU_2_06, 
POWERPC_MMU_2_07, POWERPC_MMU_3_00, POWERPC_MMU_32B, POWERPC_MMU_601.

Is it possible that a machine with an mmu not listed in here could build 
a !TCG version of qemu? if it's not possible, we can separate that part 
into a separate function left in mmu_helper.c and move the rest to 
somewhere else.

Looking at dump_mmu and ppc_tlb_invalidate_all, looks like we need to 
move enough code that it make sense to create an mmu_common.c for common 
code. Otherwise, it's probably easier to compile all of mmu_helper.c 
instead of picking those functions out.

On Thu, May 13, 2021 at 09:56:27AM -0300, Bruno Piazera Larsen wrote:
> 
> On 13/05/2021 00:59, David Gibson wrote:
> > On Wed, May 12, 2021 at 11:08:12AM -0300, Bruno Larsen (billionai) wrote:
> > > Created a file with stubs needed to compile disabling TCG.
> > > 
> > > We're not sure about keeping the softmmu stubs in this file. if there is
> > > a better place to put them, please let us know.
> > > 
> > > The other 3 functions have been stubbed because we didn't know what to
> > > do with them. Making the file compile in the !TCG case would create an
> > > ifdef hell, but extracting the functions meant moving many others as
> > > well, and there weren't any good places to put them.
> > > 
> > > Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> > > ---
> > >   target/ppc/tcg-stub.c | 33 +++++++++++++++++++++++++++++++++
> > >   1 file changed, 33 insertions(+)
> > >   create mode 100644 target/ppc/tcg-stub.c
> > > 
> > > diff --git a/target/ppc/tcg-stub.c b/target/ppc/tcg-stub.c
> > > new file mode 100644
> > > index 0000000000..67099e2676
> > > --- /dev/null
> > > +++ b/target/ppc/tcg-stub.c
> > > @@ -0,0 +1,33 @@
> > > +
> > > +#include "qemu/osdep.h"
> > > +#include "exec/hwaddr.h"
> > > +#include "cpu.h"
> > > +#include "hw/ppc/spapr.h"
> > > +
> > > +hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> > > +{
> > > +    return 0;
> > > +}
> > > +
> > > +void dump_mmu(CPUPPCState *env)
> > > +{
> > > +}
> > > +
> > > +void ppc_tlb_invalidate_all(CPUPPCState *env)
> > > +{
> > > +}
> > > +
> > > +target_ulong softmmu_resize_hpt_prepare(PowerPCCPU *cpu,
> > > +                                        SpaprMachineState *spapr,
> > > +                                        target_ulong shift)
> > > +{
> > > +    g_assert_not_reached();
> > > +}
> > > +
> > > +target_ulong softmmu_resize_hpt_commit(PowerPCCPU* cpu,
> > > +                                       SpaprMachineState *spapr,
> > > +                                       target_ulong flags,
> > > +                                       target_ulong shift)
> > > +{
> > > +    g_assert_not_reached();
> > > +}
> > I think these last two stubs should be obsoleted by the patch from
> > Lucas I already merged "hw/ppc: moved hcalls that depend on softmmu".
> 
> They aren't,

Ah, sorry.  I forgot (again) that the resize_hpt stuff is a bit
different from the other kvm-implemented mmu hypercalls.

> when talking to him he said he wanted to use as few ifdefs as
> possible. Which do you think is better, to go back and ifdef away those
> calls, or keep the stubs? And if we keep the stubs, do we keep them here or
> in hw/ppc/spapr_hcall.c, along with other stubs?

Hmm.. I don't think you should need to do either.  IIUC, when in a
!TCG build, kvm_enabled() should evaluate to true at compile time.  In
which case as long as the calls to these functions are protected by an
if (!kvm_enabled()) the compiler should be able to just figure it out
without stubs.