On Thu, Jun 21, 2018 at 09:01:27AM +0200, Cédric Le Goater wrote: > On 06/18/2018 08:36 AM, David Gibson wrote: > > KVM HV has some limitations (deriving from the hardware) that mean not all > > host-cpu supported pagesizes may be usable in the guest. At present this > > means that KVM guests and TCG guests may see different available page sizes > > even if they notionally have the same vcpu model. This is confusing and > > also prevents migration between TCG and KVM. > > > > This patch makes the environment consistent by always allowing the same set > > of pagesizes. Since we can't remove the KVM limitations, we do this by > > always applying the same limitations it has, even to TCG guests. > > > > Signed-off-by: David Gibson <[email protected]> > > > > --- > > hw/ppc/spapr_caps.c | 33 +++++++++++++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > > > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > > index 9fc739b3f5..0584c7c6ab 100644 > > --- a/hw/ppc/spapr_caps.c > > +++ b/hw/ppc/spapr_caps.c > > @@ -334,6 +334,38 @@ static void > > cap_hpt_maxpagesize_apply(sPAPRMachineState *spapr, > > spapr_check_pagesize(spapr, qemu_getrampagesize(), errp); > > } > > > > +static bool spapr_pagesize_cb(void *opaque, uint32_t seg_pshift, uint32_t > > pshift) > > +{ > > + unsigned maxshift = *((unsigned *)opaque); > > + > > + assert(pshift >= seg_pshift); > > you could check that elsewhere.
Um.. I'm not sure what you're getting at.
> > + /* Don't allow the guest to use pages bigger than the configured
> > + * maximum size */
> > + if (pshift > maxshift) {
> > + return false;
> > + }
> > +
> > + /* For whatever reason, KVM doesn't allow multiple pagesizes
> > + * within a segment, *except* for the case of 16M pages in a 4k or
> > + * 64k segment. Always exclude other cases, so that TCG and KVM
> > + * guests see a consistent environment */
> > + if ((pshift != seg_pshift) && (pshift != 24)) {
> > + return false;
> > + }
Note the stanza above, I'll refer to it below.
> > +
> > + return true;
> > +}
>
> So, do we really need ppc_hash64_filter_pagesizes() to have a callback ?
I agree that it seems overly involved, but it was the best way I could
see to logically separate the TCG / softmmu specific logic from the
spapr specific logic.
> It seems that we only use the routine once in the patchset and that the
> only thing we need to check is 'maxshift'.
Not quite. An earlier draft had this routine just take a max page
size and clamp accordingly. But that failed when I wrote the code to
check against the KVM capabilities, because KVM also excludes some
other pagesize combinations. That's what the stanza I point out above
is about
> Do you envision other usage of the routine ?
Not really, no.
>
> Thanks,
>
> C.
>
> > +static void cap_hpt_maxpagesize_cpu_apply(sPAPRMachineState *spapr,
> > + PowerPCCPU *cpu,
> > + uint8_t val, Error **errp)
> > +{
> > + unsigned maxshift = val;
> > +
> > + ppc_hash64_filter_pagesizes(cpu, spapr_pagesize_cb, &maxshift);
> > +}
> > +
> > sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> > [SPAPR_CAP_HTM] = {
> > .name = "htm",
> > @@ -401,6 +433,7 @@ sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> > .set = spapr_cap_set_pagesize,
> > .type = "int",
> > .apply = cap_hpt_maxpagesize_apply,
> > + .cpu_apply = cap_hpt_maxpagesize_cpu_apply,
> > },
> > };
> >
> >
>
--
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
