From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53790) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fVt6M-00041z-F9 for qemu-devel@nongnu.org; Thu, 21 Jun 2018 02:29:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fVt6J-0002da-AP for qemu-devel@nongnu.org; Thu, 21 Jun 2018 02:29:50 -0400 Received: from 2.mo177.mail-out.ovh.net ([178.33.109.80]:55738) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fVt6J-0002cs-0S for qemu-devel@nongnu.org; Thu, 21 Jun 2018 02:29:47 -0400 Received: from player789.ha.ovh.net (unknown [10.109.108.22]) by mo177.mail-out.ovh.net (Postfix) with ESMTP id 78FAAB5D12 for ; Thu, 21 Jun 2018 08:29:43 +0200 (CEST) References: <20180618063606.2513-1-david@gibson.dropbear.id.au> <20180618063606.2513-7-david@gibson.dropbear.id.au> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: Date: Thu, 21 Jun 2018 08:29:36 +0200 MIME-Version: 1.0 In-Reply-To: <20180618063606.2513-7-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 6/9] spapr: Use maximum page size capability to simplify memory backend checking List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson , groug@kaod.org, abologna@redhat.com Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, aik@ozlabs.ru On 06/18/2018 08:36 AM, David Gibson wrote: > The way we used to handle KVM allowable guest pagesizes for PAPR guests > required some convoluted checking of memory attached to the guest. >=20 > The allowable pagesizes advertised to the guest cpus depended on the me= mory > which was attached at boot, but then we needed to ensure that any memor= y > later hotplugged didn't change which pagesizes were allowed. >=20 > Now that we have an explicit machine option to control the allowable > maximum pagesize we can simplify this. We just check all memory backen= ds > against that declared pagesize. We check base and cold-plugged memory = at > reset time, and hotplugged memory at pre_plug() time. >=20 > Signed-off-by: David Gibson One minor question below. Nevertheless, Reviewed-by: C=C3=A9dric Le Goater Thanks, C. > --- > hw/ppc/spapr.c | 17 +++++++---------- > hw/ppc/spapr_caps.c | 20 ++++++++++++++++++++ > include/hw/ppc/spapr.h | 3 +++ > target/ppc/kvm.c | 14 -------------- > target/ppc/kvm_ppc.h | 6 ------ > 5 files changed, 30 insertions(+), 30 deletions(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 74a76e7e09..efd36e92e2 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -3192,11 +3192,13 @@ static void spapr_memory_pre_plug(HotplugHandle= r *hotplug_dev, DeviceState *dev, > Error **errp) > { > const sPAPRMachineClass *smc =3D SPAPR_MACHINE_GET_CLASS(hotplug_d= ev); > + sPAPRMachineState *spapr =3D SPAPR_MACHINE(hotplug_dev); > PCDIMMDevice *dimm =3D PC_DIMM(dev); > PCDIMMDeviceClass *ddc =3D PC_DIMM_GET_CLASS(dimm); > MemoryRegion *mr; > uint64_t size; > - char *mem_dev; > + Object *memdev; > + hwaddr pagesize; > =20 > if (!smc->dr_lmb_enabled) { > error_setg(errp, "Memory hotplug not supported for this machin= e"); > @@ -3215,15 +3217,10 @@ static void spapr_memory_pre_plug(HotplugHandle= r *hotplug_dev, DeviceState *dev, > return; > } > =20 > - mem_dev =3D object_property_get_str(OBJECT(dimm), PC_DIMM_MEMDEV_P= ROP, NULL); > - if (mem_dev && !kvmppc_is_mem_backend_page_size_ok(mem_dev)) { > - error_setg(errp, "Memory backend has bad page size. " > - "Use 'memory-backend-file' with correct mem-path.")= ; > - goto out; > - } > - > -out: > - g_free(mem_dev); > + memdev =3D object_property_get_link(OBJECT(dimm), PC_DIMM_MEMDEV_P= ROP, > + &error_abort); > + pagesize =3D host_memory_backend_pagesize(MEMORY_BACKEND(memdev)); > + spapr_check_pagesize(spapr, pagesize, errp); > } > =20 > struct sPAPRDIMMState { > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > index 6cdc0c94e7..9fc739b3f5 100644 > --- a/hw/ppc/spapr_caps.c > +++ b/hw/ppc/spapr_caps.c > @@ -26,6 +26,7 @@ > #include "qapi/error.h" > #include "qapi/visitor.h" > #include "sysemu/hw_accel.h" > +#include "exec/ram_addr.h" > #include "target/ppc/cpu.h" > #include "target/ppc/mmu-hash64.h" > #include "cpu-models.h" > @@ -304,6 +305,23 @@ static void cap_safe_indirect_branch_apply(sPAPRMa= chineState *spapr, > =20 > #define VALUE_DESC_TRISTATE " (broken, workaround, fixed)" > =20 > +void spapr_check_pagesize(sPAPRMachineState *spapr, hwaddr pagesize, > + Error **errp) > +{ > + hwaddr maxpagesize =3D (1ULL << spapr->eff.caps[SPAPR_CAP_HPT_MPS]= ); I suppose this is SPAPR_CAP_HPT_MAXPAGESIZE now ?=20 > + > + 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 implement= ation", > + 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(sPAPRMachineS= tate *spapr, > } else if (val < 16) { > warn_report("Many guests require at least 64kiB hpt-max-page-s= ize"); > } > + > + spapr_check_pagesize(spapr, qemu_getrampagesize(), errp); > } > =20 > sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] =3D { > 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); > =20 > +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 &=3D ~PPC_HASH64_1TSEG; > } > } > - > -bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) > -{ > - Object *mem_obj =3D object_resolve_path(obj_path, NULL); > - long pagesize =3D host_memory_backend_pagesize(MEMORY_BACKEND(mem_= obj)); > - > - return pagesize >=3D max_cpu_page_size; > -} > - > #else /* defined (TARGET_PPC64) */ > =20 > static inline void kvm_fixup_page_sizes(PowerPCCPU *cpu) > { > } > =20 > -bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) > -{ > - return true; > -} > - > #endif /* !defined (TARGET_PPC64) */ > =20 > 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); > =20 > bool kvmppc_hpt_needs_host_contiguous_pages(void); > -bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path); > =20 > #else > =20 > @@ -228,11 +227,6 @@ static inline bool kvmppc_hpt_needs_host_contiguou= s_pages(void) > return false; > } > =20 > -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; >=20