From patchwork Thu Jun 13 16:56:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Roger Pau Monne X-Patchwork-Id: 13697141 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 70D7FC27C4F for ; Thu, 13 Jun 2024 16:58:25 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.740136.1147158 (Exim 4.92) (envelope-from ) id 1sHnm5-00014G-MT; Thu, 13 Jun 2024 16:58:09 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 740136.1147158; Thu, 13 Jun 2024 16:58:09 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sHnm5-000149-Jo; Thu, 13 Jun 2024 16:58:09 +0000 Received: by outflank-mailman (input) for mailman id 740136; Thu, 13 Jun 2024 16:58:08 +0000 Received: from se1-gles-sth1-in.inumbo.com ([159.253.27.254] helo=se1-gles-sth1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sHnm4-00013v-Ei for xen-devel@lists.xenproject.org; Thu, 13 Jun 2024 16:58:08 +0000 Received: from mail-qv1-xf31.google.com (mail-qv1-xf31.google.com [2607:f8b0:4864:20::f31]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id 1a4f6571-29a6-11ef-90a3-e314d9c70b13; Thu, 13 Jun 2024 18:58:07 +0200 (CEST) Received: by mail-qv1-xf31.google.com with SMTP id 6a1803df08f44-6b072522bd5so6230486d6.2 for ; Thu, 13 Jun 2024 09:58:07 -0700 (PDT) Received: from localhost ([213.195.124.163]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6b2a5efb07csm8501036d6.134.2024.06.13.09.58.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Jun 2024 09:58:04 -0700 (PDT) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 1a4f6571-29a6-11ef-90a3-e314d9c70b13 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=citrix.com; s=google; t=1718297884; x=1718902684; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=PC76GIkUGU8UlLIXtJNN8BuZtt7P1TDX2gYZkFadrsY=; b=Q3HxLKZGA8/LB9HR7L5aozT4/ibGj8IBRCVM4dXwQS3Xx6wocgDkCDXCcvknPtJGnk TA4Y1kCc/Oq+YwgA0NaMHqK5PAgTQBlvQyIXm+HDEHOA5F/PxJivpu83CN7tJOuSVQpp yhyufF23TYpXHE5TuQgflSa4X01/v6VlQyv3U= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718297884; x=1718902684; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PC76GIkUGU8UlLIXtJNN8BuZtt7P1TDX2gYZkFadrsY=; b=URHiF41Hvs6fpDzEMuxk+qKi2eAurp+KzSsgfpYbRCzfhdi3F6DM9Ay//p7byVxn4P Ol0ep40URTC/iMTGTHKNt7qwLgIS+3hdMdvjNNj9JpzW2iG9USkMCgqvPDUHP6P1K8Qm Mv6JGfHAvRe8liVCejzeigvUeJk5sHiYc9uZafSvkUM/0B+rb4f4FEnjN8vCeDvZh9Y8 ntmLMGFXmfymt4iA/qsq3UjJZQ/zHfIDRjRxzeawCcSifeacdH4y3G6NrsMM247LNBtQ DJ8T6WT6481KulXAe2ivhId1kwV2mhh5oHT9YMp6hP3LqvNmfRQ0TRiOo53EfSod+plI G79A== X-Gm-Message-State: AOJu0Yw6kg5BShd5ECmrgmEJFoSFm+ihdVsCq3axhv2dFcWI/2sNGrsA tWk90kb8ovdcNuO7A6teJP7xoPm5XiC7eGaH1H/gcnhsWmX1qkxVeAQPHzJ6jFw+uEz1ZHqXlkd d X-Google-Smtp-Source: AGHT+IGul1+xQlyP5wng89N5Wm066PxOGU65bM5Nel+PzvhHTqNKFIMShWyiLZjCbJahOiflP3+Pzg== X-Received: by 2002:a05:6214:943:b0:6b0:91d4:5825 with SMTP id 6a1803df08f44-6b2afd958d0mr1142786d6.56.1718297884533; Thu, 13 Jun 2024 09:58:04 -0700 (PDT) From: Roger Pau Monne To: xen-devel@lists.xenproject.org Cc: Roger Pau Monne , Jan Beulich , Andrew Cooper Subject: [PATCH v3 1/3] x86/irq: deal with old_cpu_mask for interrupts in movement in fixup_irqs() Date: Thu, 13 Jun 2024 18:56:15 +0200 Message-ID: <20240613165617.42538-2-roger.pau@citrix.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240613165617.42538-1-roger.pau@citrix.com> References: <20240613165617.42538-1-roger.pau@citrix.com> MIME-Version: 1.0 Given the current logic it's possible for ->arch.old_cpu_mask to get out of sync: if a CPU set in old_cpu_mask is offlined and then onlined again without old_cpu_mask having been updated the data in the mask will no longer be accurate, as when brought back online the CPU will no longer have old_vector configured to handle the old interrupt source. If there's an interrupt movement in progress, and the to be offlined CPU (which is the call context) is in the old_cpu_mask clear it and update the mask, so it doesn't contain stale data. Note that when the system is going down fixup_irqs() will be called by smp_send_stop() from CPU 0 with a mask with only CPU 0 on it, effectively asking to move all interrupts to the current caller (CPU 0) which is the only CPU to remain online. In that case we don't care to migrate interrupts that are in the process of being moved, as it's likely we won't be able to move all interrupts to CPU 0 due to vector shortage anyway. Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich --- Changes since v2: - Adjust commit message. - Add comment about excluding smp_send_stop() case. - Use cpu_online(). --- xen/arch/x86/irq.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c index 263e502bc0f6..d305aed317f2 100644 --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -2526,7 +2526,7 @@ void fixup_irqs(const cpumask_t *mask, bool verbose) for ( irq = 0; irq < nr_irqs; irq++ ) { bool break_affinity = false, set_affinity = true; - unsigned int vector; + unsigned int vector, cpu = smp_processor_id(); cpumask_t *affinity = this_cpu(scratch_cpumask); if ( irq == 2 ) @@ -2569,6 +2569,33 @@ void fixup_irqs(const cpumask_t *mask, bool verbose) affinity); } + if ( desc->arch.move_in_progress && + /* + * Only attempt to adjust the mask if the current CPU is going + * offline, otherwise the whole system is going down and leaving + * stale data in the masks is fine. + */ + !cpu_online(cpu) && + cpumask_test_cpu(cpu, desc->arch.old_cpu_mask) ) + { + /* + * This CPU is going offline, remove it from ->arch.old_cpu_mask + * and possibly release the old vector if the old mask becomes + * empty. + * + * Note cleaning ->arch.old_cpu_mask is required if the CPU is + * brought offline and then online again, as when re-onlined the + * per-cpu vector table will no longer have ->arch.old_vector + * setup, and hence ->arch.old_cpu_mask would be stale. + */ + cpumask_clear_cpu(cpu, desc->arch.old_cpu_mask); + if ( cpumask_empty(desc->arch.old_cpu_mask) ) + { + desc->arch.move_in_progress = 0; + release_old_vec(desc); + } + } + /* * Avoid shuffling the interrupt around as long as current target CPUs * are a subset of the input mask. What fixup_irqs() cares about is From patchwork Thu Jun 13 16:56:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Roger Pau Monne X-Patchwork-Id: 13697142 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 838E4C27C78 for ; Thu, 13 Jun 2024 16:58:27 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.740138.1147173 (Exim 4.92) (envelope-from ) id 1sHnm9-0001Mh-7d; Thu, 13 Jun 2024 16:58:13 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 740138.1147173; Thu, 13 Jun 2024 16:58:13 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sHnm9-0001Lv-2Z; Thu, 13 Jun 2024 16:58:13 +0000 Received: by outflank-mailman (input) for mailman id 740138; Thu, 13 Jun 2024 16:58:11 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sHnm7-0000pc-Jx for xen-devel@lists.xenproject.org; Thu, 13 Jun 2024 16:58:11 +0000 Received: from mail-qk1-x729.google.com (mail-qk1-x729.google.com [2607:f8b0:4864:20::729]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 1c37be52-29a6-11ef-b4bb-af5377834399; Thu, 13 Jun 2024 18:58:10 +0200 (CEST) Received: by mail-qk1-x729.google.com with SMTP id af79cd13be357-795482e114cso71841885a.1 for ; Thu, 13 Jun 2024 09:58:09 -0700 (PDT) Received: from localhost ([213.195.124.163]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-441f2ff7fe5sm7757661cf.79.2024.06.13.09.58.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Jun 2024 09:58:06 -0700 (PDT) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 1c37be52-29a6-11ef-b4bb-af5377834399 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=citrix.com; s=google; t=1718297887; x=1718902687; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=OY8rwLUUYs4x7J36qwiR2SVYEkIjFJPbuqF3KJ+AInE=; b=RDU9WeiA5fuNlpa4SqrO8yc3nDLNMKUodv0znPy0ThqyG1booQET3h4YqCSuz3q5u0 TO/1VRCS8cxC9C/hilVoG2IJpZfZf8UZcMBIZwPgc4eQ1npCUFuJ1+f6rxG1BE3TyWsD idksdIKneg2znpGWgu44GzqODO2w5ndoOe3CI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718297887; x=1718902687; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=OY8rwLUUYs4x7J36qwiR2SVYEkIjFJPbuqF3KJ+AInE=; b=EZxb5G/3u6iLuLizPWeLelRn3llcvYMJHQOZtMJtx4zXREv10NW7q39g5BgzM+G0/D s+VOWtoUOwS/KVkh7KoyY3lllom8YW8qC4/9gDUaWB2KA9IddAbFKzmN1lX9ne3OLr+N x/4OAPVXgQWzLKD6iNmo7gMHY1s2sqZ9//VymKESUzeEHkIylY4pqmE+JfiVpqyIb855 Y8IFMmaJxuheQ+0F9qPTVju9QdAWRHQo0nd11+cO4bWgYPkLUY4qaqWV6th+dE01vAAm l5nP3s/XN0zY9gZC9ZxgqeNOtzJsFkXYaeYNLzAr9av/kZh3d5oQsixKkTwjcjwveYjZ 00rQ== X-Gm-Message-State: AOJu0Yxec2bo/d3ErVBPCIjfJcs/8zsEfqqY9GLml7pGCx7WbVIKpevg ykzmQNeZtxspPKX6NdxOQ/qwVqRxq2pK148gwRSTRM+KNNts9InPIvkEK9lab9yu+Wd3E8yft52 e X-Google-Smtp-Source: AGHT+IGZ1Mvm9118ZVFAEgnpnOMJEyewmbOC5FpeLdNjo2w51nZeduRXnb3h8W3COXmtzSe3y0e2eQ== X-Received: by 2002:ad4:4b6d:0:b0:6b0:7b39:3c6d with SMTP id 6a1803df08f44-6b2afd8150bmr1035656d6.52.1718297886771; Thu, 13 Jun 2024 09:58:06 -0700 (PDT) From: Roger Pau Monne To: xen-devel@lists.xenproject.org Cc: Roger Pau Monne , Jan Beulich , Andrew Cooper Subject: [PATCH v3 2/3] x86/irq: handle moving interrupts in _assign_irq_vector() Date: Thu, 13 Jun 2024 18:56:16 +0200 Message-ID: <20240613165617.42538-3-roger.pau@citrix.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240613165617.42538-1-roger.pau@citrix.com> References: <20240613165617.42538-1-roger.pau@citrix.com> MIME-Version: 1.0 Currently there's logic in fixup_irqs() that attempts to prevent _assign_irq_vector() from failing, as fixup_irqs() is required to evacuate all interrupts from the CPUs not present in the input mask. The current logic in fixup_irqs() is incomplete, as it doesn't deal with interrupts that have move_cleanup_count > 0 and a non-empty ->arch.old_cpu_mask field. Instead of attempting to fixup the interrupt descriptor in fixup_irqs() so that _assign_irq_vector() cannot fail, introduce logic in _assign_irq_vector() to deal with interrupts that have either move_{in_progress,cleanup_count} set and no remaining online CPUs in ->arch.cpu_mask. If _assign_irq_vector() is requested to move an interrupt in the state described above, first attempt to see if ->arch.old_cpu_mask contains any valid CPUs that could be used as fallback, and if that's the case do move the interrupt back to the previous destination. Note this is easier because the vector hasn't been released yet, so there's no need to allocate and setup a new vector on the destination. Due to the logic in fixup_irqs() that clears offline CPUs from ->arch.old_cpu_mask (and releases the old vector if the mask becomes empty) it shouldn't be possible to get into _assign_irq_vector() with ->arch.move_{in_progress,cleanup_count} set but no online CPUs in ->arch.old_cpu_mask. However if ->arch.move_{in_progress,cleanup_count} is set and the interrupt has also changed affinity, it's possible the members of ->arch.old_cpu_mask are no longer part of the affinity set, move the interrupt to a different CPU part of the provided mask and keep the current ->arch.old_{cpu_mask,vector} for the pending interrupt movement to be completed. Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich --- Changes since v2: - Adjust comments. - Clean old vector from used_vectors mask. Changes since v1: - Further refine the logic in _assign_irq_vector(). --- xen/arch/x86/irq.c | 99 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 70 insertions(+), 29 deletions(-) diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c index d305aed317f2..f36962fc1dc3 100644 --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -544,7 +544,58 @@ static int _assign_irq_vector(struct irq_desc *desc, const cpumask_t *mask) } if ( desc->arch.move_in_progress || desc->arch.move_cleanup_count ) - return -EAGAIN; + { + /* + * If the current destination is online refuse to shuffle. Retry after + * the in-progress movement has finished. + */ + if ( cpumask_intersects(desc->arch.cpu_mask, &cpu_online_map) ) + return -EAGAIN; + + /* + * Due to the logic in fixup_irqs() that clears offlined CPUs from + * ->arch.old_cpu_mask it shouldn't be possible to get here with + * ->arch.move_{in_progress,cleanup_count} set and no online CPUs in + * ->arch.old_cpu_mask. + */ + ASSERT(valid_irq_vector(desc->arch.old_vector)); + ASSERT(cpumask_intersects(desc->arch.old_cpu_mask, &cpu_online_map)); + + if ( cpumask_intersects(desc->arch.old_cpu_mask, mask) ) + { + /* + * Fallback to the old destination if moving is in progress and the + * current destination is to be offlined. This is only possible if + * the CPUs in old_cpu_mask intersect with the affinity mask passed + * in the 'mask' parameter. + */ + desc->arch.vector = desc->arch.old_vector; + cpumask_and(desc->arch.cpu_mask, desc->arch.old_cpu_mask, mask); + + /* Undo any possibly done cleanup. */ + for_each_cpu(cpu, desc->arch.cpu_mask) + per_cpu(vector_irq, cpu)[desc->arch.vector] = irq; + + /* Cancel the pending move and release the current vector. */ + desc->arch.old_vector = IRQ_VECTOR_UNASSIGNED; + cpumask_clear(desc->arch.old_cpu_mask); + desc->arch.move_in_progress = 0; + desc->arch.move_cleanup_count = 0; + if ( desc->arch.used_vectors ) + { + ASSERT(test_bit(old_vector, desc->arch.used_vectors)); + clear_bit(old_vector, desc->arch.used_vectors); + } + + return 0; + } + + /* + * There's an interrupt movement in progress but the destination(s) in + * ->arch.old_cpu_mask are not suitable given the 'mask' parameter, go + * through the full logic to find a new vector in a suitable CPU. + */ + } err = -ENOSPC; @@ -600,7 +651,24 @@ next: current_vector = vector; current_offset = offset; - if ( valid_irq_vector(old_vector) ) + if ( desc->arch.move_in_progress || desc->arch.move_cleanup_count ) + { + ASSERT(!cpumask_intersects(desc->arch.cpu_mask, &cpu_online_map)); + /* + * Special case when evacuating an interrupt from a CPU to be + * offlined and the interrupt was already in the process of being + * moved. Leave ->arch.old_{vector,cpu_mask} as-is and just + * replace ->arch.{cpu_mask,vector} with the new destination. + * Cleanup will be done normally for the old fields, just release + * the current vector here. + */ + if ( desc->arch.used_vectors ) + { + ASSERT(test_bit(old_vector, desc->arch.used_vectors)); + clear_bit(old_vector, desc->arch.used_vectors); + } + } + else if ( valid_irq_vector(old_vector) ) { cpumask_and(desc->arch.old_cpu_mask, desc->arch.cpu_mask, &cpu_online_map); @@ -2607,33 +2675,6 @@ void fixup_irqs(const cpumask_t *mask, bool verbose) continue; } - /* - * In order for the affinity adjustment below to be successful, we - * need _assign_irq_vector() to succeed. This in particular means - * clearing desc->arch.move_in_progress if this would otherwise - * prevent the function from succeeding. Since there's no way for the - * flag to get cleared anymore when there's no possible destination - * left (the only possibility then would be the IRQs enabled window - * after this loop), there's then also no race with us doing it here. - * - * Therefore the logic here and there need to remain in sync. - */ - if ( desc->arch.move_in_progress && - !cpumask_intersects(mask, desc->arch.cpu_mask) ) - { - unsigned int cpu; - - cpumask_and(affinity, desc->arch.old_cpu_mask, &cpu_online_map); - - spin_lock(&vector_lock); - for_each_cpu(cpu, affinity) - per_cpu(vector_irq, cpu)[desc->arch.old_vector] = ~irq; - spin_unlock(&vector_lock); - - release_old_vec(desc); - desc->arch.move_in_progress = 0; - } - if ( !cpumask_intersects(mask, desc->affinity) ) { break_affinity = true; From patchwork Thu Jun 13 16:56:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Roger Pau Monne X-Patchwork-Id: 13697144 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A96DEC27C6E for ; Thu, 13 Jun 2024 16:58:26 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.740137.1147168 (Exim 4.92) (envelope-from ) id 1sHnm8-0001K1-UM; Thu, 13 Jun 2024 16:58:12 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 740137.1147168; Thu, 13 Jun 2024 16:58:12 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sHnm8-0001Jo-R9; Thu, 13 Jun 2024 16:58:12 +0000 Received: by outflank-mailman (input) for mailman id 740137; Thu, 13 Jun 2024 16:58:11 +0000 Received: from se1-gles-sth1-in.inumbo.com ([159.253.27.254] helo=se1-gles-sth1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sHnm7-00013v-BK for xen-devel@lists.xenproject.org; Thu, 13 Jun 2024 16:58:11 +0000 Received: from mail-ot1-x32f.google.com (mail-ot1-x32f.google.com [2607:f8b0:4864:20::32f]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id 1cff2ad8-29a6-11ef-90a3-e314d9c70b13; Thu, 13 Jun 2024 18:58:10 +0200 (CEST) Received: by mail-ot1-x32f.google.com with SMTP id 46e09a7af769-6f97a4c4588so698040a34.2 for ; Thu, 13 Jun 2024 09:58:10 -0700 (PDT) Received: from localhost ([213.195.124.163]) by smtp.gmail.com with ESMTPSA id af79cd13be357-798abc037desm66960085a.97.2024.06.13.09.58.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Jun 2024 09:58:08 -0700 (PDT) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 1cff2ad8-29a6-11ef-90a3-e314d9c70b13 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=citrix.com; s=google; t=1718297889; x=1718902689; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=2OqTsWR9SuIo4uzvIZP/P5NOenHh2rQIF4jI5UgbX64=; b=DNVthAALlq9MSTc7XvwOPslvo1Y+ykqZ3BCSR8Wsb5lLhoP0qsVyCKl+oXwf3u6uhR MR1K6R/QO4a7rr7mB0K4jGeSKHUPxPzpBMyWml+2pAJTIPig8sQwP40mc3F+FYKPc6/R UwORBkvFyuyx91WQoviuKI1wCb5H+Ta2oZrtk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718297889; x=1718902689; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=2OqTsWR9SuIo4uzvIZP/P5NOenHh2rQIF4jI5UgbX64=; b=lNYsWJTrEJ3elRLB9bza0EmkN5lHX35KpAT/uLX/dR7p/I/YWbvqNWmUrB5psaa1m2 dS3BYnV7BAHGuGfs30wCRV7EoOIRi8PyAJ5zwsQa5p6Twvox74ysIfL8GTSaPL7aqoJM bL+VV366RAHIczHDHL803yn4FzgQ/IMH81yrbzbHAIXkdNh3pIxToRnmi7gPvd2XZjSi eJ8I6OAyKa7wcWrgJzzx0evhRSoRetlqw5dgnO5Ur1+D+JU4DyrGOMUUc91O/QKD/G0N kkyuV5MQ+kpKQFRRRyyh1l1xARTb5tB1ln9Didp4ve/PLyHA2WNPDVmQUDlNqnzS4yAm VL7w== X-Gm-Message-State: AOJu0YxfYnf3N2MKBhLlI4JlwhmPAV2VsKwshIfqE4Y4jeFTbpnxLXIl cm+/wkHruX/GFPO9nOINu5rg9bkXnEUOiafvOkc8uuqplxbGhjjvi9nsGWBpZWUs4lLgXGDQyQJ w X-Google-Smtp-Source: AGHT+IH2m9tIxZZ8OzShQsKDG0eFpEjdz4EKdEJZcMSsChf60s3+TVzqkR3vmgdqdLizA1ele7eBXw== X-Received: by 2002:a05:6830:4429:b0:6f9:74db:5dc4 with SMTP id 46e09a7af769-6fb9376dcdamr409423a34.14.1718297888900; Thu, 13 Jun 2024 09:58:08 -0700 (PDT) From: Roger Pau Monne To: xen-devel@lists.xenproject.org Cc: Roger Pau Monne , Jan Beulich , Andrew Cooper Subject: [PATCH v3 3/3] x86/irq: forward pending interrupts to new destination in fixup_irqs() Date: Thu, 13 Jun 2024 18:56:17 +0200 Message-ID: <20240613165617.42538-4-roger.pau@citrix.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240613165617.42538-1-roger.pau@citrix.com> References: <20240613165617.42538-1-roger.pau@citrix.com> MIME-Version: 1.0 fixup_irqs() is used to evacuate interrupts from to be offlined CPUs. Given the CPU is to become offline, the normal migration logic used by Xen where the vector in the previous target(s) is left configured until the interrupt is received on the new destination is not suitable. Instead attempt to do as much as possible in order to prevent loosing interrupts. If fixup_irqs() is called from the CPU to be offlined (as is currently the case) attempt to forward pending vectors when interrupts that target the current CPU are migrated to a different destination. Additionally, for interrupts that have already been moved from the current CPU prior to the call to fixup_irqs() but that haven't been delivered to the new destination (iow: interrupts with move_in_progress set and the current CPU set in ->arch.old_cpu_mask) also check whether the previous vector is pending and forward it to the new destination. This allows us to remove the window with interrupts enabled at the bottom of fixup_irqs(). Such window wasn't safe anyway: references to the CPU to become offline are removed from interrupts masks, but the per-CPU vector_irq[] array is not updated to reflect those changes (as the CPU is going offline anyway). Signed-off-by: Roger Pau Monné --- Changes since v2: - Remove interrupt enabled window from fixup_irqs(). - Adjust comments and commit message. Changes since v1: - Rename to apic_irr_read(). --- xen/arch/x86/include/asm/apic.h | 5 ++++ xen/arch/x86/irq.c | 42 ++++++++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/xen/arch/x86/include/asm/apic.h b/xen/arch/x86/include/asm/apic.h index d1cb001fb4ab..7bd66dc6e151 100644 --- a/xen/arch/x86/include/asm/apic.h +++ b/xen/arch/x86/include/asm/apic.h @@ -132,6 +132,11 @@ static inline bool apic_isr_read(uint8_t vector) (vector & 0x1f)) & 1; } +static inline bool apic_irr_read(unsigned int vector) +{ + return apic_read(APIC_IRR + (vector / 32 * 0x10)) & (1U << (vector % 32)); +} + static inline u32 get_apic_id(void) { u32 id = apic_read(APIC_ID); diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c index f36962fc1dc3..a2b04c793292 100644 --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -2593,7 +2593,7 @@ void fixup_irqs(const cpumask_t *mask, bool verbose) for ( irq = 0; irq < nr_irqs; irq++ ) { - bool break_affinity = false, set_affinity = true; + bool break_affinity = false, set_affinity = true, check_irr = false; unsigned int vector, cpu = smp_processor_id(); cpumask_t *affinity = this_cpu(scratch_cpumask); @@ -2646,6 +2646,25 @@ void fixup_irqs(const cpumask_t *mask, bool verbose) !cpu_online(cpu) && cpumask_test_cpu(cpu, desc->arch.old_cpu_mask) ) { + /* + * This to be offlined CPU was the target of an interrupt that's + * been moved, and the new destination target hasn't yet + * acknowledged any interrupt from it. + * + * We know the interrupt is configured to target the new CPU at + * this point, so we can check IRR for any pending vectors and + * forward them to the new destination. + * + * Note that for the other case of an interrupt movement being in + * progress (move_cleanup_count being non-zero) we know the new + * destination has already acked at least one interrupt from this + * source, and hence there's no need to forward any stale + * interrupts. + */ + if ( apic_irr_read(desc->arch.old_vector) ) + send_IPI_mask(cpumask_of(cpumask_any(desc->arch.cpu_mask)), + desc->arch.vector); + /* * This CPU is going offline, remove it from ->arch.old_cpu_mask * and possibly release the old vector if the old mask becomes @@ -2686,11 +2705,27 @@ void fixup_irqs(const cpumask_t *mask, bool verbose) if ( desc->handler->disable ) desc->handler->disable(desc); + /* + * If the current CPU is going offline and is (one of) the target(s) of + * the interrupt, signal to check whether there are any pending vectors + * to be handled in the local APIC after the interrupt has been moved. + */ + if ( !cpu_online(cpu) && cpumask_test_cpu(cpu, desc->arch.cpu_mask) ) + check_irr = true; + if ( desc->handler->set_affinity ) desc->handler->set_affinity(desc, affinity); else if ( !(warned++) ) set_affinity = false; + if ( check_irr && apic_irr_read(vector) ) + /* + * Forward pending interrupt to the new destination, this CPU is + * going offline and otherwise the interrupt would be lost. + */ + send_IPI_mask(cpumask_of(cpumask_any(desc->arch.cpu_mask)), + desc->arch.vector); + if ( desc->handler->enable ) desc->handler->enable(desc); @@ -2707,11 +2742,6 @@ void fixup_irqs(const cpumask_t *mask, bool verbose) printk("Broke affinity for IRQ%u, new: {%*pbl}\n", irq, CPUMASK_PR(affinity)); } - - /* That doesn't seem sufficient. Give it 1ms. */ - local_irq_enable(); - mdelay(1); - local_irq_disable(); } void fixup_eoi(void)