On Fri, Apr 23, 2021 at 07:06:15PM -0300, Fabiano Rosas wrote: > "Lucas Mateus Castro (alqotel)" <lucas.ara...@eldorado.org.br> writes: > > > This file should be used instead of spapr_hcall.c when compiling > > without tcg (--disable-tcg) as it does not call tcg-only functions and > > trips fatal error when invalid functions are called > > Not calling any TCG-specific function is not an indication of the code > being "kvm only" in this case. So I think this patch is backwards, we > should instead aim to remove tcg-only code from spapr_hcall.c.
Right.
> > As of right now some functions are repeated here and in spapr_hcall.c,
> > as they are static, is some other method to deal with this
> > recommended?
>
> Yeah, you should not be repeating the functions. From previous
> discussions on this topic I understood that we'd have another
> hypercall_register_types for TCG. So we could have a spapr_hcall_tcg.c
> that contains tcg-only functions. And they would only be used in that
> file so they would continue being static.
> > Also some functions should only cause a fatal error as KVM should
> > intercept and handle their call, but as I'm not sure which ones I just
> > did this to functions that called tcg-only code.
> >
> > Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.ara...@eldorado.org.br>
> > ---
> > hw/ppc/spapr_hcall_tcg_stub.c | 1824 +++++++++++++++++++++++++++++++++
> > 1 file changed, 1824 insertions(+)
> > create mode 100644 hw/ppc/spapr_hcall_tcg_stub.c
> >
> > diff --git a/hw/ppc/spapr_hcall_tcg_stub.c
> > b/hw/ppc/spapr_hcall_tcg_stub.c
>
> Your usage of stub here is a bit confusing. Take a look at
> target/ppc/kvm-stub.c and accel/stubs/kvm-stub.c. These are files that
> are only included in the build to satisfy any references to the symbols
> they contain. The implementation is just an empty body or an error
> return. So if the feature is included, the actual foo.c will be present
> with the proper implementation; if not, we get the empty stub.
>
> Also, look at target/ppc/kvm.h under #ifndef CONFIG_USER_ONLY. There's
> some similar ideas there that could be of help.
>
> So my suggestion for this patch is take a step back and move first all
> of the TCG-only functions that are certainly not needed. We can then
> figure out what patterns we are going to use to stub them in the
> KVM-only build. After that we take a look at what's left and go from
> there.
Right. You should be able to stub these much more simply than this.
Just a single say "h_tcg_only()" stub, then in the !TCG case you
register that instead of the real implementation with
spapr_register_hypercall().
--
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
