On Thu, Jun 21, 2018 at 12:29:14PM +0200, Greg Kurz wrote:
> On Mon, 18 Jun 2018 16:36:03 +1000
> David Gibson <da...@gibson.dropbear.id.au> wrote:
[snip]
> > @@ -304,6 +305,23 @@ static void
> > cap_safe_indirect_branch_apply(sPAPRMachineState *spapr,
> >
> > #define VALUE_DESC_TRISTATE " (broken, workaround, fixed)"
> >
> > +void spapr_check_pagesize(sPAPRMachineState *spapr, hwaddr pagesize,
> > + Error **errp)
> > +{
> > + hwaddr maxpagesize = (1ULL << spapr->eff.caps[SPAPR_CAP_HPT_MPS]);
>
> s/SPAPR_CAP_HPT_MPS/SPAPR_CAP_HPT_MAXPAGESIZE
Fixed.
> > +
> > + if (!kvmppc_hpt_needs_host_contiguous_pages()) {
> > + return;
> > + }
> > +
> > + if (maxpagesize > pagesize) {
> > + error_setg(errp,
> > + "Can't support %"HWADDR_PRIu" kiB guest pages with %"
> > + HWADDR_PRIu" kiB host pages with this KVM
> > implementation",
> > + maxpagesize >> 10, pagesize >> 10);
> > + }
> > +}
> > +
> > static void cap_hpt_maxpagesize_apply(sPAPRMachineState *spapr,
> > uint8_t val, Error **errp)
> > {
> > @@ -312,6 +330,8 @@ static void cap_hpt_maxpagesize_apply(sPAPRMachineState
> > *spapr,
> > } else if (val < 16) {
> > warn_report("Many guests require at least 64kiB
> > hpt-max-page-size");
> > }
> > +
> > + spapr_check_pagesize(spapr, qemu_getrampagesize(), errp);
>
> Even if in this precise case QEMU will always exit gracefully since
> errp == &error_fatal, passing errp several times is a fragile pattern.
> It may cause a crash if *errp was already allocated.
>
> Maybe use a local_err variable and error_propagate() or at least return
> in the (val < 12) block above.
Actually, just a return; after the first error should be sufficient.
I've put that in.
>
> Rest looks good. With the two issues addressed:
>
> Reviewed-by: Greg Kurz <gr...@kaod.org>
>
> > }
> >
> > sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index c97593d032..75e2cf2687 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -806,4 +806,7 @@ void spapr_caps_cpu_apply(sPAPRMachineState *spapr,
> > PowerPCCPU *cpu);
> > void spapr_caps_add_properties(sPAPRMachineClass *smc, Error **errp);
> > int spapr_caps_post_migration(sPAPRMachineState *spapr);
> >
> > +void spapr_check_pagesize(sPAPRMachineState *spapr, hwaddr pagesize,
> > + Error **errp);
> > +
> > #endif /* HW_SPAPR_H */
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index 50b5d01432..9cfbd388ad 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -500,26 +500,12 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
> > cpu->hash64_opts->flags &= ~PPC_HASH64_1TSEG;
> > }
> > }
> > -
> > -bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
> > -{
> > - Object *mem_obj = object_resolve_path(obj_path, NULL);
> > - long pagesize = host_memory_backend_pagesize(MEMORY_BACKEND(mem_obj));
> > -
> > - return pagesize >= max_cpu_page_size;
> > -}
> > -
> > #else /* defined (TARGET_PPC64) */
> >
> > static inline void kvm_fixup_page_sizes(PowerPCCPU *cpu)
> > {
> > }
> >
> > -bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
> > -{
> > - return true;
> > -}
> > -
> > #endif /* !defined (TARGET_PPC64) */
> >
> > unsigned long kvm_arch_vcpu_id(CPUState *cpu)
> > diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> > index a7ddb8a5d6..443fca0a4e 100644
> > --- a/target/ppc/kvm_ppc.h
> > +++ b/target/ppc/kvm_ppc.h
> > @@ -71,7 +71,6 @@ int kvmppc_resize_hpt_commit(PowerPCCPU *cpu,
> > target_ulong flags, int shift);
> > bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
> >
> > bool kvmppc_hpt_needs_host_contiguous_pages(void);
> > -bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path);
> >
> > #else
> >
> > @@ -228,11 +227,6 @@ static inline bool
> > kvmppc_hpt_needs_host_contiguous_pages(void)
> > return false;
> > }
> >
> > -static inline bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
> > -{
> > - return true;
> > -}
> > -
> > static inline bool kvmppc_has_cap_spapr_vfio(void)
> > {
> > return false;
>
--
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
