Patchwork AMD MMCONF Support

Arne Georg Gleditsch <arne.gleditsch@numascale.com> writes:
> With this change, I can re-insert the RES_PCI_IO wihtout ill effects.

Move initialization of MMCONF BAR to cache_as_ram setup phase, in order
to make sure MMCONF is set up before use.  Otherwise, PCI config
accesses run before init_cpus() will be lost if MMCONF is enabled
(unless explicitly done as port-based accesses).

This obsoletes removal of RES_PCI_IO, PCI_ADDR(0, 1, 0, 0x78) in
mcp55_early_setup, so reinsert.

Signed-off-by: Arne Georg Gleditsch <arne.gleditsch@numascale.com>

> Arne Georg Gleditsch <arne.gleditsch@numascale.com> writes:
> > With this change, I can re-insert the RES_PCI_IO wihtout ill effects.
> 
> Move initialization of MMCONF BAR to cache_as_ram setup phase, in order
> to make sure MMCONF is set up before use.  Otherwise, PCI config
> accesses run before init_cpus() will be lost if MMCONF is enabled
> (unless explicitly done as port-based accesses).
> 
> This obsoletes removal of RES_PCI_IO, PCI_ADDR(0, 1, 0, 0x78) in
> mcp55_early_setup, so reinsert.
> 
> Signed-off-by: Arne Georg Gleditsch <arne.gleditsch@numascale.com>

Acked-by: Myles Watson <mylesgw@gmail.com>

Thanks for tracking it down!

Myles

On Mon, Sep 13, 2010 at 6:52 AM, Myles Watson <mylesgw@gmail.com> wrote:
>
>> Arne Georg Gleditsch <arne.gleditsch@numascale.com> writes:
>> > With this change, I can re-insert the RES_PCI_IO wihtout ill effects.
>>
>> Move initialization of MMCONF BAR to cache_as_ram setup phase, in order
>> to make sure MMCONF is set up before use.  Otherwise, PCI config
>> accesses run before init_cpus() will be lost if MMCONF is enabled
>> (unless explicitly done as port-based accesses).
>>
>> This obsoletes removal of RES_PCI_IO, PCI_ADDR(0, 1, 0, 0x78) in
>> mcp55_early_setup, so reinsert.
>>
>> Signed-off-by: Arne Georg Gleditsch <arne.gleditsch@numascale.com>
>
> Acked-by: Myles Watson <mylesgw@gmail.com>
Rev 5810.

Arne:
Could you help me understand MMCONF_SUPPORT and
MMCONF_SUPPORT_DEFAULT?  It looks like the area gets reserved for
MMCONF_SUPPORT, even if MMCONF_SUPPORT_DEFAULT isn't selected.

Thanks,
Myles

Thanks,
Myles

Myles Watson <mylesgw@gmail.com> writes:
> Rev 5810.

Thanks!

> Arne:
> Could you help me understand MMCONF_SUPPORT and
> MMCONF_SUPPORT_DEFAULT?  It looks like the area gets reserved for
> MMCONF_SUPPORT, even if MMCONF_SUPPORT_DEFAULT isn't selected.

My understanding of the code as it was before I started messing with it,
is that MMCONF_SUPPORT is intended to indicate whether the facility is
available at all, and MMCONF_SUPPORT_DEFAULT toggles the actual use of
it (by Coreboot).  Both src/northbridge/amd/amdfam10/northbridge.c and
src/northbridge/intel/i945/northbridge.c contains code like this:

  #if CONFIG_MMCONF_SUPPORT_DEFAULT
          .ops_pci_bus      = &pci_ops_mmconf,
  #else
          .ops_pci_bus      = &pci_cf8_conf1,
  #endif

which, alongside romcc_io.h's 

  #if CONFIG_MMCONF_SUPPORT_DEFAULT
          return pci_mmio_read_config32(dev, where);
  #else
          return pci_io_read_config32(dev, where);
  #endif

is more or less the only use for MMCONF_SUPPORT_DEFAULT as far as I can
see.  I'll leave it to others to determine if this actually makes sense.

(Having SUPPORT without SUPPORT_DEFAULT gives you an MCFG area that you
can communicate to the OS (at least potentially), as well as the option
of explicitly doing individual config accesses via mmio instead of
ports.  I don't think anyone is doing either of these today.)

>> Arne:
>> Could you help me understand MMCONF_SUPPORT and
>> MMCONF_SUPPORT_DEFAULT?  It looks like the area gets reserved for
>> MMCONF_SUPPORT, even if MMCONF_SUPPORT_DEFAULT isn't selected.
>
> My understanding of the code as it was before I started messing with it,
> is that MMCONF_SUPPORT is intended to indicate whether the facility is
> available at all, and MMCONF_SUPPORT_DEFAULT toggles the actual use of
> it (by Coreboot).  Both src/northbridge/amd/amdfam10/northbridge.c and
> src/northbridge/intel/i945/northbridge.c contains code like this:
>
>  #if CONFIG_MMCONF_SUPPORT_DEFAULT
>          .ops_pci_bus      = &pci_ops_mmconf,
>  #else
>          .ops_pci_bus      = &pci_cf8_conf1,
>  #endif
>
> which, alongside romcc_io.h's
>
>  #if CONFIG_MMCONF_SUPPORT_DEFAULT
>          return pci_mmio_read_config32(dev, where);
>  #else
>          return pci_io_read_config32(dev, where);
>  #endif
>
> is more or less the only use for MMCONF_SUPPORT_DEFAULT as far as I can
> see.  I'll leave it to others to determine if this actually makes sense.
>
> (Having SUPPORT without SUPPORT_DEFAULT gives you an MCFG area that you
> can communicate to the OS (at least potentially), as well as the option
> of explicitly doing individual config accesses via mmio instead of
> ports.  I don't think anyone is doing either of these today.)

Thanks for the explanation.  It looks like the trouble with reserving
a region for MMCONF is related to broken UMA code.

Thanks,
Myles