linux-kernel - Re: [PATCH v2 2/5] x86/PCI: Support additional MMIO range capabilities

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 for Android: free password hash cracker in your pocket

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

Date:	Sun, 20 Apr 2014 09:59:37 +0200
From:	Borislav Petkov <bp@...e.de>
To:	Myron Stowe <myron.stowe@...hat.com>
Cc:	bhelgaas@...gle.com, linux-pci@...r.kernel.org,
	suravee.suthikulpanit@....com, aravind.gopalakrishnan@....com,
	kim.naru@....com, daniel@...ascale.com, tglx@...utronix.de,
	mingo@...hat.com, hpa@...or.com, x86@...nel.org, bp@...e.de,
	sp@...ascale.com, linux-acpi@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/5] x86/PCI: Support additional MMIO range
 capabilities

Drop Andreas' old email address from CC as it keeps bouncing.

On Sat, Apr 19, 2014 at 03:52:20PM +0200, Borislav Petkov wrote:
> > -static void __init pci_enable_pci_io_ecs(void)
> > +static void __init pci_enable_pci_io_ecs(u8 bus, u8 slot)
> >  {
> >  #ifdef CONFIG_AMD_NB
> >  	unsigned int i, n;
> > +	u8 limit;
> >  
> >  	for (n = i = 0; !n && amd_nb_bus_dev_ranges[i].dev_limit; ++i) {
> > -		u8 bus = amd_nb_bus_dev_ranges[i].bus;
> > -		u8 slot = amd_nb_bus_dev_ranges[i].dev_base;
> > -		u8 limit = amd_nb_bus_dev_ranges[i].dev_limit;
> > +		/* Try matching for the bus range */
> > +		if ((bus != amd_nb_bus_dev_ranges[i].bus) ||
> > +		    (slot != amd_nb_bus_dev_ranges[i].dev_base))
> > +			continue;
> > +
> > +		limit = amd_nb_bus_dev_ranges[i].dev_limit;
> >  
> > +		/* Setup all northbridges within the range */
> >  		for (; slot < limit; ++slot) {
> >  			u32 val = read_pci_config(bus, slot, 3, 0);
> > -
> > -			if (!early_is_amd_nb(val))
> > +			if (!val)
> >  				continue;
> >  
> >  			val = read_pci_config(bus, slot, 3, 0x8c);
> > @@ -375,13 +457,14 @@ static void __init pci_enable_pci_io_ecs(void)
> >  				val |= ENABLE_CF8_EXT_CFG >> 32;
> 
> What a fun shifting!
> 
> Maybe you should do
> 
> #define ENABLE_CF8_EXT_CFG	BIT(46 - 32)
> 
> to show exactly what you mean and how the bit is defined in MSR NB_CFG1
> and also show how the high 32-bits are mapped into F3x8c, while at it.
> 
> And then you can drop the shifting at the call site.

Ok, I see another fun with this ECS enabling:

There's a enable_pci_io_ecs() which enables ECS through the NB_CFG MSR
which is called as part of the notifier *and* there's a PCI write to
that same bit in pci_enable_pci_io_ecs() which iterates over all NBs.

So, AFAICT, we do it twice and the second time is not needed. Which
means, you probably can drop pci_enable_pci_io_ecs() completely and use
solely the notifier?

Yes, no?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/