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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/25/2014 5:24 PM, Myron Stowe wrote:
On Sun, Apr 20, 2014 at 1:59 AM, Borislav Petkov <bp@xxxxxxx> 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?

I believe the notifier is mainly for the cores which are initially offline, and then brought up online afterward.

Suravee

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux