From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=BAYES_00,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1BA85C433E3 for ; Fri, 26 Mar 2021 10:33:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D966561A4A for ; Fri, 26 Mar 2021 10:33:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230007AbhCZKcx (ORCPT ); Fri, 26 Mar 2021 06:32:53 -0400 Received: from mail.kernel.org ([198.145.29.99]:53522 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229474AbhCZKcu (ORCPT ); Fri, 26 Mar 2021 06:32:50 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DF24E61A48; Fri, 26 Mar 2021 10:32:49 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94) (envelope-from ) id 1lPjln-003xXC-LI; Fri, 26 Mar 2021 10:32:47 +0000 Date: Fri, 26 Mar 2021 10:32:45 +0000 Message-ID: <87v99efbn6.wl-maz@kernel.org> From: Marc Zyngier To: Thomas Gleixner Cc: Megha Dey , linux-kernel@vger.kernel.org, dave.jiang@intel.com, ashok.raj@intel.com, kevin.tian@intel.com, dwmw@amazon.co.uk, x86@kernel.org, tony.luck@intel.com, dan.j.williams@intel.com, jgg@mellanox.com, kvm@vger.kernel.org, iommu@lists.linux-foundation.org, alex.williamson@redhat.com, bhelgaas@google.com, linux-pci@vger.kernel.org, baolu.lu@linux.intel.com, ravi.v.shankar@intel.com Subject: Re: [Patch V2 08/13] genirq: Set auxiliary data for an interrupt In-Reply-To: <87im5fvz2z.fsf@nanos.tec.linutronix.de> References: <1614370277-23235-1-git-send-email-megha.dey@intel.com> <1614370277-23235-9-git-send-email-megha.dey@intel.com> <871rc3rvuc.wl-maz@kernel.org> <87im5fvz2z.fsf@nanos.tec.linutronix.de> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: tglx@linutronix.de, megha.dey@intel.com, linux-kernel@vger.kernel.org, dave.jiang@intel.com, ashok.raj@intel.com, kevin.tian@intel.com, dwmw@amazon.co.uk, x86@kernel.org, tony.luck@intel.com, dan.j.williams@intel.com, jgg@mellanox.com, kvm@vger.kernel.org, iommu@lists.linux-foundation.org, alex.williamson@redhat.com, bhelgaas@google.com, linux-pci@vger.kernel.org, baolu.lu@linux.intel.com, ravi.v.shankar@intel.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 25 Mar 2021 18:59:48 +0000, Thomas Gleixner wrote: > > On Thu, Mar 25 2021 at 17:23, Marc Zyngier wrote: > >> +{ > >> + struct irq_desc *desc; > >> + struct irq_data *data; > >> + unsigned long flags; > >> + int res = -ENODEV; > >> + > >> + desc = irq_get_desc_buslock(irq, &flags, 0); > >> + if (!desc) > >> + return -EINVAL; > >> + > >> + for (data = &desc->irq_data; data; data = irqd_get_parent_data(data)) { > >> + if (data->chip->irq_set_auxdata) { > >> + res = data->chip->irq_set_auxdata(data, which, val); > > > > And this is where things can break: because you don't define what > > 'which' is, you can end-up with two stacked layers clashing in their > > interpretation of 'which', potentially doing the wrong thing. > > > > Short of having a global, cross architecture definition of all the > > possible states, this is frankly dodgy. > > My bad, I suggested this in the first place. > > So what you suggest is to make 'which' an enum and have that in > include/linux/whatever.h so we end up with unique identifiers accross > architectures, irqdomains and whatever, right? Exactly. As long as 'which' is unique and unambiguous, we can solve the stacking issue (which is oddly starting to smell like the ghost of the SVR3 STREAMS... /me runs ;-). Once we have that, I can start killing the irq_set_vcpu_affinity() abuse I have been guilty of over the past years. Even more, we could kill irq_set_vcpu_affinity() altogether, because we have a generic way of passing side-band information from a driver down to the IRQ stack. > That makes a lot of sense. > > Though that leaves the question of the data type for 'val'. While u64 is > probably good enough for most stuff, anything which needs more than that > is left out (again). union as arguments are horrible especially if you > need the counterpart irq_get_auxdata() for which you need a pointer and > then you can't do a forward declaration. Something like this might work > though and avoid to make the pointer business unconditional: > > struct irq_auxdata { > union { > u64 val; > struct foo *foo; > }; > }; I guess that at some point, irq_get_auxdata() will become a requirement so we might as well bite the bullet now, and the above looks like a good start to me. Thanks, M. -- Without deviation from the norm, progress is not possible.