{"id":803007,"url":"http://patchwork.ozlabs.org/api/patches/803007/?format=json","web_url":"http://patchwork.ozlabs.org/project/kvm-ppc/patch/1503022258.4493.133.camel@kernel.crashing.org/","project":{"id":23,"url":"http://patchwork.ozlabs.org/api/projects/23/?format=json","name":"KVM PowerPC development","link_name":"kvm-ppc","list_id":"kvm-ppc.vger.kernel.org","list_email":"kvm-ppc@vger.kernel.org","web_url":null,"scm_url":null,"webscm_url":null,"list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<1503022258.4493.133.camel@kernel.crashing.org>","list_archive_url":null,"date":"2017-08-18T02:10:58","name":"[2/2] kvm/xive: Add missing barriers and document them","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"a2bba3887f9c587c3ddf88c44a84da781b9055cd","submitter":{"id":38,"url":"http://patchwork.ozlabs.org/api/people/38/?format=json","name":"Benjamin Herrenschmidt","email":"benh@kernel.crashing.org"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/kvm-ppc/patch/1503022258.4493.133.camel@kernel.crashing.org/mbox/","series":[],"comments":"http://patchwork.ozlabs.org/api/patches/803007/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/803007/checks/","tags":{},"related":[],"headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xYRQv2T1nz9t48\n\tfor ;\n\tFri, 18 Aug 2017 12:11:35 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753858AbdHRCLe (ORCPT );\n\tThu, 17 Aug 2017 22:11:34 -0400","from gate.crashing.org ([63.228.1.57]:33073 \"EHLO\n\tgate.crashing.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1753857AbdHRCLd (ORCPT );\n\tThu, 17 Aug 2017 22:11:33 -0400","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby gate.crashing.org (8.14.1/8.13.8) with ESMTP id v7I2AxQW004393;\n\tThu, 17 Aug 2017 21:11:00 -0500"],"Message-ID":"<1503022258.4493.133.camel@kernel.crashing.org>","Subject":"[PATCH 2/2] kvm/xive: Add missing barriers and document them","From":"Benjamin Herrenschmidt ","To":"kvm-ppc@vger.kernel.org","Cc":"linuxppc dev list ,\n\t\"kvm@vger.kernel.org\" , paulus@samba.org","Date":"Fri, 18 Aug 2017 12:10:58 +1000","Content-Type":"text/plain; charset=\"UTF-8\"","X-Mailer":"Evolution 3.24.4 (3.24.4-1.fc26) ","Mime-Version":"1.0","Content-Transfer-Encoding":"7bit","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"kvm-ppc@vger.kernel.org"},"content":"This adds missing memory barriers to order updates/tests of\nthe virtual CPPR and MFRR, thus fixing a lost IPI problem.\n\nWhile at it also document all barriers in this file\n\nThis fixes a bug causing guest IPIs to occasionally get lost.\n\nSigned-off-by: Benjamin Herrenschmidt \n---\n arch/powerpc/kvm/book3s_xive_template.c | 57 +++++++++++++++++++++++++++++++--\n 1 file changed, 55 insertions(+), 2 deletions(-)\n\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe kvm-ppc\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","diff":"diff --git a/arch/powerpc/kvm/book3s_xive_template.c b/arch/powerpc/kvm/book3s_xive_template.c\nindex 150be86b1018..d1ed2c41b5d2 100644\n--- a/arch/powerpc/kvm/book3s_xive_template.c\n+++ b/arch/powerpc/kvm/book3s_xive_template.c\n@@ -17,6 +17,12 @@ static void GLUE(X_PFX,ack_pending)(struct kvmppc_xive_vcpu *xc)\n \tu16 ack;\n \n \t/*\n+\t * Ensure any previous store to CPPR is ordered vs.\n+\t * the subsequent loads from PIPR or ACK.\n+\t */\n+\teieio();\n+\n+\t/*\n \t * DD1 bug workaround: If PIPR is less favored than CPPR\n \t * ignore the interrupt or we might incorrectly lose an IPB\n \t * bit.\n@@ -244,6 +250,11 @@ static u32 GLUE(X_PFX,scan_interrupts)(struct kvmppc_xive_vcpu *xc,\n \t/*\n \t * If we found an interrupt, adjust what the guest CPPR should\n \t * be as if we had just fetched that interrupt from HW.\n+\t *\n+\t * Note: This can only make xc->cppr smaller as the previous\n+\t * loop will only exit with hirq != 0 if prio is lower than\n+\t * the current xc->cppr. Thus we don't need to re-check xc->mfrr\n+\t * for pending IPIs.\n \t */\n \tif (hirq)\n \t\txc->cppr = prio;\n@@ -390,6 +401,12 @@ X_STATIC int GLUE(X_PFX,h_cppr)(struct kvm_vcpu *vcpu, unsigned long cppr)\n \txc->cppr = cppr;\n \n \t/*\n+\t * Order the above update of xc->cppr with the subsequent\n+\t * read of xc->mfrr inside push_pending_to_hw()\n+\t */\n+\tsmp_mb();\n+\n+\t/*\n \t * We are masking less, we need to look for pending things\n \t * to deliver and set VP pending bits accordingly to trigger\n \t * a new interrupt otherwise we might miss MFRR changes for\n@@ -429,21 +446,37 @@ X_STATIC int GLUE(X_PFX,h_eoi)(struct kvm_vcpu *vcpu, unsigned long xirr)\n \t * used to signal MFRR changes is EOId when fetched from\n \t * the queue.\n \t */\n-\tif (irq == XICS_IPI || irq == 0)\n+\tif (irq == XICS_IPI || irq == 0) {\n+\t\t/*\n+\t\t * This barrier orders the setting of xc->cppr vs.\n+\t\t * subsquent test of xc->mfrr done inside\n+\t\t * scan_interrupts and push_pending_to_hw\n+\t\t */\n+\t\tsmp_mb();\n \t\tgoto bail;\n+\t}\n \n \t/* Find interrupt source */\n \tsb = kvmppc_xive_find_source(xive, irq, &src);\n \tif (!sb) {\n \t\tpr_devel(\" source not found !\\n\");\n \t\trc = H_PARAMETER;\n+\t\t/* Same as above */\n+\t\tsmp_mb();\n \t\tgoto bail;\n \t}\n \tstate = &sb->irq_state[src];\n \tkvmppc_xive_select_irq(state, &hw_num, &xd);\n \n \tstate->in_eoi = true;\n-\tmb();\n+\n+\t/*\n+\t * This barrier orders both setting of in_eoi above vs,\n+\t * subsequent test of guest_priority, and the setting\n+\t * of xc->cppr vs. subsquent test of xc->mfrr done inside\n+\t * scan_interrupts and push_pending_to_hw\n+\t */\n+\tsmp_mb();\n \n again:\n \tif (state->guest_priority == MASKED) {\n@@ -470,6 +503,14 @@ X_STATIC int GLUE(X_PFX,h_eoi)(struct kvm_vcpu *vcpu, unsigned long xirr)\n \n \t}\n \n+\t/*\n+\t * This barrier orders the above guest_priority check\n+\t * and spin_lock/unlock with clearing in_eoi below.\n+\t *\n+\t * It also has to be a full mb() as it must ensure\n+\t * the MMIOs done in source_eoi() are completed before\n+\t * state->in_eoi is visible.\n+\t */\n \tmb();\n \tstate->in_eoi = false;\n bail:\n@@ -504,6 +545,18 @@ X_STATIC int GLUE(X_PFX,h_ipi)(struct kvm_vcpu *vcpu, unsigned long server,\n \t/* Locklessly write over MFRR */\n \txc->mfrr = mfrr;\n \n+\t/*\n+\t * The load of xc->cppr below and the subsequent MMIO store\n+\t * to the IPI must happen after the above mfrr update is\n+\t * globally visible so that:\n+\t *\n+\t * - Synchronize with another CPU doing an H_EOI or a H_CPPR\n+\t * updating xc->cppr then reading xc->mfrr.\n+\t *\n+\t * - The target of the IPI sees the xc->mfrr update\n+\t */\n+\tmb();\n+\n \t/* Shoot the IPI if most favored than target cppr */\n \tif (mfrr < xc->cppr)\n \t\t__x_writeq(0, __x_trig_page(&xc->vp_ipi_data));\n","prefixes":["2/2"]}