From nobody Mon Nov 25 03:05:51 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass(p=reject dis=none) header.from=citrix.com ARC-Seal: i=1; a=rsa-sha256; t=1718297913; cv=none; d=zohomail.com; s=zohoarc; b=nI0JydVh7C7PII0BlLaHS7OmOd9B6yXz9v3Hn7J4DrrN64nw1px7VJ0NlW7NM3susReSnFsnlQjAfKPbVF1a+bmg6XSOC9SD4whSyeG6SUPK5eGDQEfT8M3NeZbZbT0dB1qdmZ/wrOunwHSri4+BD4e2U2ha3dl2+uyd5Hj0ONw= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1718297913; h=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:Subject:To:To:Message-Id:Reply-To; bh=OY8rwLUUYs4x7J36qwiR2SVYEkIjFJPbuqF3KJ+AInE=; b=P4aiM6e3E8GdqmP1++WMuidriWXnKVV96stp13ViKVVr2U22f6Uiz8+6xZXNOewZr4WlopXY7Zt/1UfhGLNV8xe0wi9o5n9Iy08IRtayQOJF+WZ3RspHttsUNUznxOvpgdxDj5uMv6sI5+romeXbAC5TheEGB3P9nhK9QIwLls8= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass header.from= (p=reject dis=none) Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1718297913546949.1356322696994; Thu, 13 Jun 2024 09:58:33 -0700 (PDT) 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 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-Outflank-Mailman: Message body and most headers restored to incoming version 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 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @citrix.com) X-ZM-MESSAGEID: 1718297914977100001 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} s= et 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 v= alid 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=C3=A9 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, c= onst cpumask_t *mask) } =20 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 an= d the + * current destination is to be offlined. This is only possib= le if + * the CPUs in old_cpu_mask intersect with the affinity mask p= assed + * in the 'mask' parameter. + */ + desc->arch.vector =3D 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] =3D irq; + + /* Cancel the pending move and release the current vector. */ + desc->arch.old_vector =3D IRQ_VECTOR_UNASSIGNED; + cpumask_clear(desc->arch.old_cpu_mask); + desc->arch.move_in_progress =3D 0; + desc->arch.move_cleanup_count =3D 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. + */ + } =20 err =3D -ENOSPC; =20 @@ -600,7 +651,24 @@ next: current_vector =3D vector; current_offset =3D offset; =20 - 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_ma= p)); + /* + * Special case when evacuating an interrupt from a CPU to be + * offlined and the interrupt was already in the process of be= ing + * 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 rele= ase + * 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; } =20 - /* - * 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 he= re. - * - * 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] =3D ~irq; - spin_unlock(&vector_lock); - - release_old_vec(desc); - desc->arch.move_in_progress =3D 0; - } - if ( !cpumask_intersects(mask, desc->affinity) ) { break_affinity =3D true; --=20 2.45.2