linux-kernel - Re: [PATCH 1/2] MIPS: SiByte: Set 32-bit bus mask for BCM1250 PCI

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.21.1811071100030.20378@eddie.linux-mips.org>
Date:   Wed, 7 Nov 2018 11:08:08 +0000 (GMT)
From:   "Maciej W. Rozycki" <macro@...ux-mips.org>
To:     Christoph Hellwig <hch@....de>
cc:     Ralf Baechle <ralf@...ux-mips.org>,
        Paul Burton <paul.burton@...s.com>, linux-mips@...ux-mips.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] MIPS: SiByte: Set 32-bit bus mask for BCM1250 PCI

On Wed, 7 Nov 2018, Christoph Hellwig wrote:

> > +static int sb1250_bus_dma_mask(struct pci_dev *dev, void *data)
> > +{
> > +	struct sb1250_bus_dma_mask_exclude *exclude = data;
> > +
> > +	if (!exclude->set && (dev->vendor == PCI_VENDOR_ID_SIBYTE &&
> > +			      dev->device == PCI_DEVICE_ID_BCM1250_HT)) {
> > +		exclude->start = dev->subordinate->number;
> > +		exclude->end = pci_bus_max_busnr(dev->subordinate);
> > +		exclude->set = true;
> > +		dev_dbg(&dev->dev, "not disabling DAC for [bus %02x-%02x]",
> > +			exclude->start, exclude->end);
> > +	} else if (!exclude->set ||
> > +		   (exclude->set && (dev->bus->number < exclude->start ||
> > +				     dev->bus->number > exclude->end))) {
> > +		dev_dbg(&dev->dev, "disabling DAC for device");
> > +		dev->dev.bus_dma_mask = DMA_BIT_MASK(32);
> > +	} else {
> > +		dev_dbg(&dev->dev, "not disabling DAC for device");
> > +	}
> > +	return 0;
> 
> Hmm, these conditions look very hard to read to me.  Wouldn't this
> have the same effect?
> 
> 	if (exclude->set)
> 		return;

 Nope, `exclude->set' only means we already know what range to exclude 
(and that gets set mid-way through scanning as the HT bridge is 
encountered).  Then if it's unset, we know we are (still) outside that 
range.

 Maybe I can flatten the conditions at the small cost of executing some 
code unnecessarily.  But that won't be a big deal as this stuff is only 
executed once at boot and isn't performance critical.

 It'll have to wait until next week though as I'll be travelling 
throughout the rest of this and won't be able to test anything.

  Maciej