lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 3 May 2020 17:16:18 -0700 From: "Dey, Megha" <megha.dey@...ux.intel.com> To: Thomas Gleixner <tglx@...utronix.de>, Dave Jiang <dave.jiang@...el.com>, vkoul@...nel.org, maz@...nel.org, bhelgaas@...gle.com, rafael@...nel.org, gregkh@...uxfoundation.org, hpa@...or.com, alex.williamson@...hat.com, jacob.jun.pan@...el.com, ashok.raj@...el.com, jgg@...lanox.com, yi.l.liu@...el.com, baolu.lu@...el.com, kevin.tian@...el.com, sanjay.k.kumar@...el.com, tony.luck@...el.com, jing.lin@...el.com, dan.j.williams@...el.com, kwankhede@...dia.com, eric.auger@...hat.com, parav@...lanox.com Cc: dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org, x86@...nel.org, linux-pci@...r.kernel.org, kvm@...r.kernel.org Subject: Re: [PATCH RFC 05/15] ims-msi: Add mask/unmask routines Hi Thomas, On 4/25/2020 2:49 PM, Thomas Gleixner wrote: > Dave Jiang <dave.jiang@...el.com> writes: >> >> +static u32 __dev_ims_desc_mask_irq(struct msi_desc *desc, u32 flag) > > ...mask_irq()? This is doing both mask and unmask depending on the > availability of the ops callbacks. yes, should have called it __dev_ims_desc_mask_unmask_irq perhaps. > >> +{ >> + u32 mask_bits = desc->platform.masked; >> + const struct platform_msi_ops *ops; >> + >> + ops = desc->platform.msi_priv_data->ops; >> + if (!ops) >> + return 0; >> + >> + if (flag) { > > flag? Darn, this has a clear boolean meaning of mask or unmask and 'u32 > flag' is the most natural and obvious self explaining expression for > this, right? will change it a more meaningful name next time around .. > >> + if (ops->irq_mask) >> + mask_bits = ops->irq_mask(desc); >> + } else { >> + if (ops->irq_unmask) >> + mask_bits = ops->irq_unmask(desc); >> + } >> + >> + return mask_bits; > > What's mask_bits? This is about _ONE_ IMS interrupt. Can it have > multiple mask bits and if so then the explanation which I decoded by > crystal ball probably looks like this: > > Bit 0: Don't know whether it's masked > Bit 1: Perhaps it's masked > Bit 2: Probably it's masked > Bit 3: Mostly masked > ... > Bit 31: Fully masked > > Or something like that. Makes a lot of sense in a XKCD cartoon at least. > After a close look, we can simply do away with this mask_bits. Looks like a crystal ball will not be required next time around after all. >> +} >> + >> +/** >> + * dev_ims_mask_irq - Generic irq chip callback to mask IMS interrupts >> + * @data: pointer to irqdata associated to that interrupt >> + */ >> +static void dev_ims_mask_irq(struct irq_data *data) >> +{ >> + struct msi_desc *desc = irq_data_get_msi_desc(data); >> + >> + desc->platform.masked = __dev_ims_desc_mask_irq(desc, 1); > > The purpose of this masked information is? serves no purpose, borrowed this concept from the PCI-msi code but is just junk here. Will be removed next time around. > > Thanks, > > tglx >
Powered by blists - more mailing lists