Re: [Qemu-devel] [RFC for-2.13 3/7] target/ppc: Add ppc_hash64_filter_pagesizes()

[David Gibson]

On Thu, May 03, 2018 at 12:57:09PM -0300, Murilo Opsfelder Araujo wrote:
> On Thu, Apr 19, 2018 at 04:29:13PM +1000, David Gibson wrote:
> > The paravirtualized PAPR platform sometimes needs to restrict the guest to
> > using only some of the page sizes actually supported by the host's MMU.
> > At the moment this is handled in KVM specific code, but for consistency we
> > want to apply the same limitations to all accelerators.
> > 
> > This makes a start on this by providing a helper function in the cpu code
> > to allow platform code to remove some of the cpu's page size definitions
> > via a caller supplied callback.
> > 
> > Signed-off-by: David Gibson <address@hidden>
> > ---
> >  target/ppc/mmu-hash64.c | 59 
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  target/ppc/mmu-hash64.h |  3 +++
> >  2 files changed, 62 insertions(+)
> > 
> > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > index a1db20e3a8..b6e62864fd 100644
> > --- a/target/ppc/mmu-hash64.c
> > +++ b/target/ppc/mmu-hash64.c
> > @@ -1165,3 +1165,62 @@ const PPCHash64Options ppc_hash64_opts_POWER7 = {
> >          },
> >      }
> >  };
> > +
> > +void ppc_hash64_filter_pagesizes(PowerPCCPU *cpu,
> > +                                 bool (*cb)(void *, uint32_t, uint32_t),
> > +                                 void *opaque)
> > +{
> > +    PPCHash64Options *opts = cpu->hash64_opts;
> > +    int i;
> > +    int n = 0;
> > +    bool ci_largepage = false;
> > +
> > +    assert(opts);
> > +
> > +    n = 0;
> > +    for (i = 0; i < ARRAY_SIZE(opts->sps); i++) {
> > +        PPCHash64SegmentPageSizes *sps = &opts->sps[i];
> > +        int j;
> > +        int m = 0;
> > +
> > +        assert(n <= i);
> > +
> > +        if (!sps->page_shift) {
> > +            break;
> > +        }
> > +
> > +        for (j = 0; j < ARRAY_SIZE(sps->enc); j++) {
> > +            PPCHash64PageSize *ps = &sps->enc[j];
> > +
> > +            assert(m <= j);
> > +            if (!ps->page_shift) {
> > +                break;
> > +            }
> > +
> > +            if (cb(opaque, sps->page_shift, ps->page_shift)) {
> > +                if (ps->page_shift == 16) {
> > +                    ci_largepage = true;
> > +                }
> > +                sps->enc[m++] = *ps;
> > +            }
> 
> Hi, David.
> 
> Is it possible that both sps->page_shift and ps->page_shift value 24?
> This seems to be a true case for spapr_pagesize_cb(), if I'm reading
> correctly.

Yes, that's possible; in fact, typical.

> Shouldn't ci_largepage also be set to true when ps->page_shift ==
> 24?

Ah.. yeah.. in order to match the existing logic this should be >=
16.  I've changed it in my draft.

Of course, I doubt that any guest would attempt to use 16MiB IO pages,
so I'm not sure it really matters, but.

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