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

David Gibson posted 7 patches 7 years, 9 months ago
[Qemu-devel] [RFC for-2.13 3/7] target/ppc: Add ppc_hash64_filter_pagesizes()
Posted by David Gibson 7 years, 9 months ago
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 <david@gibson.dropbear.id.au>
---
 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;
+            }
+        }
+
+        /* Clear rest of the row */
+        for (j = m; j < ARRAY_SIZE(sps->enc); j++) {
+            memset(&sps->enc[j], 0, sizeof(sps->enc[j]));
+        }
+
+        if (m) {
+            n++;
+        }
+    }
+
+    /* Clear the rest of the table */
+    for (i = n; i < ARRAY_SIZE(opts->sps); i++) {
+        memset(&opts->sps[i], 0, sizeof(opts->sps[i]));
+    }
+
+    if (!ci_largepage) {
+        opts->flags &= ~PPC_HASH64_CI_LARGEPAGE;
+    }
+}
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index f23b78d787..1aa2453497 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -20,6 +20,9 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
 void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val);
 void ppc_hash64_init(PowerPCCPU *cpu);
 void ppc_hash64_finalize(PowerPCCPU *cpu);
+void ppc_hash64_filter_pagesizes(PowerPCCPU *cpu,
+                                 bool (*cb)(void *, uint32_t, uint32_t),
+                                 void *opaque);
 #endif
 
 /*
-- 
2.14.3


Re: [Qemu-devel] [RFC for-2.13 3/7] target/ppc: Add ppc_hash64_filter_pagesizes()
Posted by Murilo Opsfelder Araujo 7 years, 9 months ago
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 <david@gibson.dropbear.id.au>
> ---
>  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.

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

Cheers
Murilo

> +        }
> +
> +        /* Clear rest of the row */
> +        for (j = m; j < ARRAY_SIZE(sps->enc); j++) {
> +            memset(&sps->enc[j], 0, sizeof(sps->enc[j]));
> +        }
> +
> +        if (m) {
> +            n++;
> +        }
> +    }
> +
> +    /* Clear the rest of the table */
> +    for (i = n; i < ARRAY_SIZE(opts->sps); i++) {
> +        memset(&opts->sps[i], 0, sizeof(opts->sps[i]));
> +    }
> +
> +    if (!ci_largepage) {
> +        opts->flags &= ~PPC_HASH64_CI_LARGEPAGE;
> +    }
> +}
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index f23b78d787..1aa2453497 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -20,6 +20,9 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
>  void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val);
>  void ppc_hash64_init(PowerPCCPU *cpu);
>  void ppc_hash64_finalize(PowerPCCPU *cpu);
> +void ppc_hash64_filter_pagesizes(PowerPCCPU *cpu,
> +                                 bool (*cb)(void *, uint32_t, uint32_t),
> +                                 void *opaque);
>  #endif
>  
>  /*
> -- 
> 2.14.3
> 
> 


Re: [Qemu-devel] [RFC for-2.13 3/7] target/ppc: Add ppc_hash64_filter_pagesizes()
Posted by David Gibson 7 years, 9 months ago
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 <david@gibson.dropbear.id.au>
> > ---
> >  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