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

[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