Re: Discussion: Patch series that adds disable-tcg option for ppc targets

[David Gibson]

On Tue, Apr 06, 2021 at 06:38:21PM +0000, Bruno Piazera Larsen wrote:
> > t's usually best to focus on logical changes, rather than
> > file-by-file.  That said, I'd probably suggest changing the .c files
> > first, then changing the meson file.
> 
> OK, will do!
> 
> > I'd lean towards building a whole series, since I think the individual
> > file changes won't make a lot of sense on their own.  That said, it's
> > ok and encouraged to post a relatively early draft of the series as an
> > RFC, so that the overall idea can be reviewed, even if it has obvious
> > omissions (just mention them in the cover letter).
> 
> Good to know. I'm guessing that the RFC should at least compile,
> though, right? That's mostly what is holding me from sharing what we
> have so far.

Yes, compiling would definitely be preferable.  If you're able to
reduce scope of your initial draft to accomplish that, then I'd
recommend it, so that you can get something out for initial review
sooner.

> 
> > I'd expect  mmu-*.c to be excluded.  Those are softmmu implementations
> > which shouldn't be used with KVM.  It's possible there are a few
> > helpers that will need to be extracted, though.
> 
> weirdly enough, removing the mmu-* don't give compilation errors,
> but linker errors.

That's not surprising - the prototypes will be in a header somewhere,
so the compiler will be happy.  It's only at link time it will
discover it doesn't have the helpers that come from those files.

> There are a few helpers that we'll have to see
> how to deal with, but something that I found odd is that cpu_list
> and all it's related functions are defined in translate_init.c.inc,
> instead of cpu.c like most other targets. I'm thinking about moving
> those to the cpu.c and follow the general standard of the rest

That sounds like a good idea (note I haven't looked in any depth to
see if there are some gotchas with that).

> > You'll probably also need changes in hw/ppc/spapr_hcall.c and maybe
> > some other parts of the spapr code: there are a number of hypercalls
> > that we implement in qemu for TCG, but which are (and must be)
> > implemented in KVM when KVM is in use.  So, I expect you'll need to
> > suppress compilation of h_enter, h_remove, h_protect, h_read and
> > h_bulk_remove at least in the !TCG case.
> 
> Yeah. there are 6 files dealing with spapr that are having linking
> problems, though some might be because of the cpu_list
> problem... But I'm a bit confused on how to interface with KVM, do I
> use an ioctl explicitly, or can it handle it implicitly? wenever I
> google or ask on IRC for KVM stuff the answer is usually "KVM
> handles it" so I never know if/when to call it

I'm afraid this will mostly need to be figured out on a case by case
basis, with some knowledge of the PAPR interfaces.

One general guideline I can give you: look for the (mostly indirect)
calls to kvmppc_enable_hcall().  That call instructs KVM to handle the
designated hypercall, so it's a hint that you may not need the qemu
implementation of it in the KVM case.  However, there are still some
gotchas - in some cases the hypercall must be implemented by KVM for a
KVM guest or it won't work (e.g. H_ENTER), in other cases the KVM
implementation is just an optimization or fast path and the qemu
implementation is needed as well (e.g. H_PUT_TCE).

The other thing to do is to actually look at the KVM code which
dispatches KVM handled hypercalls, specifically
kvmppc_pseries_do_hcall() in arch/powerpc/kvm/book3s_hv.c and
hcall_real_table in arch/powerpc/kvm/book3s_hv_rmhandlers.S.

At the moment, the TCG-only and always-needed hcall implementations in
qemu aren't split from each other.  Doing so would probably be a good
idea, and I'd be happy to add a new TCG-only .c file for it.

Feel free to ask about specific hypercalls, and I'll try to help out.

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