[{"id":1770852,"web_url":"http://patchwork.ozlabs.org/comment/1770852/","msgid":"<20170918212329.GD27153@umbus>","list_archive_url":null,"date":"2017-09-18T21:23:29","subject":"Re: [Qemu-devel] [PATCH 4/4] ppc/kvm: generalize the use of\n\tkvmppc_get_htab_fd()","submitter":{"id":47,"url":"http://patchwork.ozlabs.org/api/people/47/","name":"David Gibson","email":"david@gibson.dropbear.id.au"},"content":"On Fri, Sep 15, 2017 at 03:16:20PM +0200, Greg Kurz wrote:\n> The use of KVM_PPC_GET_HTAB_FD is open-coded in kvmppc_read_hptes()\n> and kvmppc_write_hpte().\n> \n> This patch modifies kvmppc_get_htab_fd() so that it can be used\n> everywhere we need to access the in-kernel htab:\n> - add an index argument\n> => only kvmppc_read_hptes() passes an actual index, all other users\n> pass 0\n> - add an errp argument to propagate error messages to the caller.\n> => spapr migration code prints the error\n> => hpte helpers pass &error_abort to keep the current behavior\n> of hw_error()\n> \n> While here, this also fixes a bug in kvmppc_write_hpte() so that it\n> opens the htab fd for writing instead of reading as it currently does.\n> This never broke anything because we currently never call this code,\n> as explained in the changelog of commit c1385933804bb:\n> \n> \"This support updating htab managed by the hypervisor. Currently\n> we don't have any user for this feature. This actually bring the\n> store_hpte interface in-line with the load_hpte one. We may want\n> to use this when we want to emulate henter hcall in qemu for HV\n> kvm.\"\n> \n> The above is still true today.\n> \n> Signed-off-by: Greg Kurz \n\nApplied to ppc-for-2.11.\n\n> ---\n> hw/ppc/spapr.c | 15 +++++++--------\n> target/ppc/kvm.c | 27 +++++++++------------------\n> target/ppc/kvm_ppc.h | 4 ++--\n> 3 files changed, 18 insertions(+), 28 deletions(-)\n> \n> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c\n> index 1ae79326d1ac..eeef549fbc15 100644\n> --- a/hw/ppc/spapr.c\n> +++ b/hw/ppc/spapr.c\n> @@ -1211,14 +1211,15 @@ static uint64_t spapr_get_patbe(PPCVirtualHypervisor *vhyp)\n> */\n> static int get_htab_fd(sPAPRMachineState *spapr)\n> {\n> + Error *local_err = NULL;\n> +\n> if (spapr->htab_fd >= 0) {\n> return spapr->htab_fd;\n> }\n> \n> - spapr->htab_fd = kvmppc_get_htab_fd(false);\n> + spapr->htab_fd = kvmppc_get_htab_fd(false, 0, &local_err);\n> if (spapr->htab_fd < 0) {\n> - error_report(\"Unable to open fd for reading hash table from KVM: %s\",\n> - strerror(spapr->htab_fd));\n> + error_report_err(local_err);\n> }\n> \n> return spapr->htab_fd;\n> @@ -1931,6 +1932,7 @@ static int htab_load(QEMUFile *f, void *opaque, int version_id)\n> sPAPRMachineState *spapr = opaque;\n> uint32_t section_hdr;\n> int fd = -1;\n> + Error *local_err = NULL;\n> \n> if (version_id < 1 || version_id > 1) {\n> error_report(\"htab_load() bad version\");\n> @@ -1945,8 +1947,6 @@ static int htab_load(QEMUFile *f, void *opaque, int version_id)\n> }\n> \n> if (section_hdr) {\n> - Error *local_err = NULL;\n> -\n> /* First section gives the htab size */\n> spapr_reallocate_hpt(spapr, section_hdr, &local_err);\n> if (local_err) {\n> @@ -1959,10 +1959,9 @@ static int htab_load(QEMUFile *f, void *opaque, int version_id)\n> if (!spapr->htab) {\n> assert(kvm_enabled());\n> \n> - fd = kvmppc_get_htab_fd(true);\n> + fd = kvmppc_get_htab_fd(true, 0, &local_err);\n> if (fd < 0) {\n> - error_report(\"Unable to open fd to restore KVM hash table: %s\",\n> - strerror(fd));\n> + error_report_err(local_err);\n> return fd;\n> }\n> }\n> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c\n> index 09d7dea79e2d..c37d74941085 100644\n> --- a/target/ppc/kvm.c\n> +++ b/target/ppc/kvm.c\n> @@ -2550,21 +2550,25 @@ int kvmppc_define_rtas_kernel_token(uint32_t token, const char *function)\n> return kvm_vm_ioctl(kvm_state, KVM_PPC_RTAS_DEFINE_TOKEN, &args);\n> }\n> \n> -int kvmppc_get_htab_fd(bool write)\n> +int kvmppc_get_htab_fd(bool write, uint64_t index, Error **errp)\n> {\n> struct kvm_get_htab_fd s = {\n> .flags = write ? KVM_GET_HTAB_WRITE : 0,\n> - .start_index = 0,\n> + .start_index = index,\n> };\n> int ret;\n> \n> if (!cap_htab_fd) {\n> - fprintf(stderr, \"KVM version doesn't support saving the hash table\\n\");\n> + error_setg(errp, \"KVM version doesn't support %s the HPT\",\n> + write ? \"writing\" : \"reading\");\n> return -ENOTSUP;\n> }\n> \n> ret = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &s);\n> if (ret < 0) {\n> + error_setg(errp, \"Unable to open fd for %s HPT %s KVM: %s\",\n> + write ? \"writing\" : \"reading\", write ? \"to\" : \"from\",\n> + strerror(errno));\n> return -errno;\n> }\n> \n> @@ -2648,17 +2652,10 @@ void kvm_arch_init_irq_routing(KVMState *s)\n> \n> void kvmppc_read_hptes(ppc_hash_pte64_t *hptes, hwaddr ptex, int n)\n> {\n> - struct kvm_get_htab_fd ghf = {\n> - .flags = 0,\n> - .start_index = ptex,\n> - };\n> int fd, rc;\n> int i;\n> \n> - fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);\n> - if (fd < 0) {\n> - hw_error(\"kvmppc_read_hptes: Unable to open HPT fd\");\n> - }\n> + fd = kvmppc_get_htab_fd(false, ptex, &error_abort);\n> \n> i = 0;\n> while (i < n) {\n> @@ -2700,19 +2697,13 @@ void kvmppc_read_hptes(ppc_hash_pte64_t *hptes, hwaddr ptex, int n)\n> void kvmppc_write_hpte(hwaddr ptex, uint64_t pte0, uint64_t pte1)\n> {\n> int fd, rc;\n> - struct kvm_get_htab_fd ghf;\n> struct {\n> struct kvm_get_htab_header hdr;\n> uint64_t pte0;\n> uint64_t pte1;\n> } buf;\n> \n> - ghf.flags = 0;\n> - ghf.start_index = 0; /* Ignored */\n> - fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);\n> - if (fd < 0) {\n> - hw_error(\"kvmppc_write_hpte: Unable to open HPT fd\");\n> - }\n> + fd = kvmppc_get_htab_fd(true, 0 /* Ignored */, &error_abort);\n> \n> buf.hdr.n_valid = 1;\n> buf.hdr.n_invalid = 0;\n> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h\n> index 08aab46c5a56..349f892631bf 100644\n> --- a/target/ppc/kvm_ppc.h\n> +++ b/target/ppc/kvm_ppc.h\n> @@ -51,7 +51,7 @@ uint64_t kvmppc_rma_size(uint64_t current_size, unsigned int hash_shift);\n> #endif /* !CONFIG_USER_ONLY */\n> bool kvmppc_has_cap_epr(void);\n> int kvmppc_define_rtas_kernel_token(uint32_t token, const char *function);\n> -int kvmppc_get_htab_fd(bool write);\n> +int kvmppc_get_htab_fd(bool write, uint64_t index, Error **errp);\n> int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns);\n> int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,\n> uint16_t n_valid, uint16_t n_invalid);\n> @@ -245,7 +245,7 @@ static inline int kvmppc_define_rtas_kernel_token(uint32_t token,\n> return -1;\n> }\n> \n> -static inline int kvmppc_get_htab_fd(bool write)\n> +static inline int kvmppc_get_htab_fd(bool write, uint64_t index, Error **errp)\n> {\n> return -1;\n> }\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=gibson.dropbear.id.au\n\theader.i=@gibson.dropbear.id.au header.b=\"j076JcvJ\"; \n\tdkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxKNg2PD3z9sBZ\n\tfor ;\n\tTue, 19 Sep 2017 20:48:33 +1000 (AEST)","from localhost ([::1]:41412 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1duG4q-0002Hr-Bk\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 06:48:28 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:51087)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1duFtT-0001D1-K0\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 06:36:49 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1duFtQ-0002no-UB\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 06:36:43 -0400","from ozlabs.org ([2401:3900:2:1::2]:35151)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1duFtQ-0002ib-8O; Tue, 19 Sep 2017 06:36:40 -0400","by ozlabs.org (Postfix, from userid 1007)\n\tid 3xxK6p0C7dz9t33; Tue, 19 Sep 2017 20:36:33 +1000 (AEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=gibson.dropbear.id.au; s=201602; t=1505817394;\n\tbh=SHnySxhGggYkv1tMV+zGOHntviqRssu19lxEFwUYGFY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=j076JcvJ61RnaGV1Kih3x6SSWz4VMlUoxlEuIeWxWkYdS87ppdfE3XXd14A6+xWuH\n\tMHX9bIlIwm3H/CnWGgtwYZWY+/GTdZPNLc+7ecpl6KlQEdXqh8s4eaw9jMnwTY9RTm\n\tH2+tZsBdB5Il2Naa95UsAjzvaQ0DkKrHnzYojNhg=","Date":"Tue, 19 Sep 2017 07:23:29 +1000","From":"David Gibson ","To":"Greg Kurz ","Message-ID":"<20170918212329.GD27153@umbus>","References":"<150548133297.5945.7401220081077343726.stgit@bahia.lan>\n\t<150548138021.5945.4408791416827487125.stgit@bahia.lan>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"+KJYzRxRHjYqLGl5\"","Content-Disposition":"inline","In-Reply-To":"<150548138021.5945.4408791416827487125.stgit@bahia.lan>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2401:3900:2:1::2","Subject":"Re: [Qemu-devel] [PATCH 4/4] ppc/kvm: generalize the use of\n\tkvmppc_get_htab_fd()","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Alexey Kardashevskiy , Thomas Huth ,\n\tqemu-ppc@nongnu.org, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]