lkml.org 
[lkml]   [2014]   [Apr]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 2/5] x86/PCI: Support additional MMIO range capabilities
From
On Sun, Apr 20, 2014 at 1:59 AM, Borislav Petkov <bp@suse.de> wrote:
> 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?

It does look as if there is some duplication with respect to setting
MSR_AMD64_NB_CFG's (which is aliased at D18F3x8c [1])
ENABLE_CF8_EXT_CFG enable bit but there are at least a couple of
differences.

enable_pci_io_ecs() only sets the bit on one NB whereas
pci_enable_pci_io_ecs iterates over all the NBs (as you mentioned
above). The other difference has something to do with Xen; see the
origin of pci_enable_pci_io_ecs - commit 24d9b70b8.

>
> Yes, no?

Suravee, Kim - either of you want to chime in here?


[1] Read-write; Per-node. MSRC001_001F[31:0] is an alias of D18F3x88.
MSRC001_001F[63:32] is an alias of
D18F3x8C.

Myron
>
> --
> Regards/Gruss,
> Boris.
>
> Sent from a fat crate under my desk. Formatting is fine.
> --
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html


\
 
 \ /
  Last update: 2014-04-26 00:41    [W:0.062 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site