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

[Suravee Suthikulanit]

On 4/28/2014 4:19 PM, Myron Stowe wrote:
> On Sat, Apr 19, 2014 at 7:52 AM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> > On Fri, Apr 18, 2014 at 08:53:23PM -0600, Myron Stowe wrote:
> > > From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>

.......

> > > @@ -118,6 +154,12 @@ static int __init early_fill_mp_bus_info(void)
> > >
> > >        pr_debug("Found AMD hostbridge at %x:%x.0\n", bus, slot);
> > >
> > > +     /* We enable ECS mode prior to probing MMIO as MMIO related registers
> > > +      * are in the ECS area.
> > > +      */
> > > +     pci_io_ecs_init(bus, slot);
> >
> > Well, if enabling the ECS failed somehow, you probably want to save
> > yourself the _amd_read_pci_config() calls further down.
> >
> > Which means:
> >
> > * you should propagate an error code from that pci_io_ecs_init() function
> >
> > * you can carve out the huge loop iterating over AMD_NB_F1_MMIO_RANGES
> > along with the pci ECS enablement call to pci_io_ecs_init into a
> > separate function and thus slim down that huge early_fill_mp_bus_info()
> > monster a bit.
>
> Yes but this was not trivial since most, if not all, usages ignore any
> return value.
>
> I'm wanting to focus on just fixing the issue for the most part and
> making as few changes as possible.  This is primarly due to:
>    o  The lack of engagement by the AMD engineers (Suravee posted these
> changes but has since been unresponsive),
>    o  As Bjorn just mentioned, we would like to get rid of this entire
> file (as we did in a similar manner with intel_bus) as it is just an
> "endless treadmill" that needs attention with every new HW release,
>    o  I want my name to be minimally related to this  ;)
[Suravee] Sorry for late reply and did not follow up on this patch in a 
timely manner.  Also, thank you Myron for helping to follow up with the 
patch set.

Regarding the error code from pci_io_ecs_init(), it is currently always 
return 0 (success).  I am not sure what error code should I 
check/propagate.  Let me know if I am missing something here.

......

> > > -static int __init pci_io_ecs_init(void)
> > > +static int __init pci_io_ecs_init(u8 bus, u8 slot)
> > >   {
> > >        int cpu;
> > >
> > > @@ -389,9 +472,7 @@ static int __init pci_io_ecs_init(void)
> > >           if (boot_cpu_data.x86 < 0x10)
> > >                return 0;
> > >
> > > -     /* Try the PCI method first. */
> > > -     if (early_pci_allowed())
> > > -             pci_enable_pci_io_ecs();
> >
> > Where did the if-check go?
>
> Good catch.  I assume Suravee didn't drop this on purpose?

[SURAVEE] I dropped the "early_pci_allowed()" here since I have moved 
the calling of "pci_io_ecs_init()" into the "early_fill_mp_bus_info()" 
since I would like to enable PCI ecs access at earlier stage. The 
"early_fill_mp_bus_info() has already check for "early_pci_allowed()" 
already at the beginning of the function. So, this should not be needed 
here.

Suravee


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