This patch makes the memory window for PCI memory bars configurable via Kconfig.
[ v2: handle old pcimem assignment method ]
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/Kconfig | 10 ++++++++++ src/config.h | 9 ++++----- 2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/Kconfig b/src/Kconfig index 6d55b23..91a89e3 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -187,6 +187,16 @@ menu "Hardware support" depends on EXTRA_PCI_ROOTS hex "Extra secondary PCI root bus number" default 0x00 + config PCIMEM_ENABLE_PREFETCH + bool "Use separate window for prefetchable PCI memory" + default y + config PCIMEM_START + hex "PCI memory window start address" + default 0xf0000000 + config PCIMEM_SIZE + depends on PCIMEM_ENABLE_PREFETCH + hex "PCI memory window size for non-prefetchable memory" + default 0x08000000
config USE_SMM depends on !COREBOOT diff --git a/src/config.h b/src/config.h index 5b40488..2aa14c5 100644 --- a/src/config.h +++ b/src/config.h @@ -47,18 +47,17 @@ #define BUILD_BIOS_TMP_ADDR 0x30000 #define BUILD_MAX_HIGHMEM 0xe0000000
+#ifndef CONFIG_PCIMEM_ENABLE_PREFETCH // Support old pci mem assignment behaviour -//#define CONFIG_OLD_PCIMEM_ASSIGNMENT 1 -#if CONFIG_OLD_PCIMEM_ASSIGNMENT -#define BUILD_PCIMEM_START 0xf0000000 +#define BUILD_PCIMEM_START CONFIG_PCIMEM_START #define BUILD_PCIMEM_SIZE (BUILD_PCIMEM_END - BUILD_PCIMEM_START) #define BUILD_PCIMEM_END 0xfec00000 /* IOAPIC is mapped at */ #define BUILD_PCIPREFMEM_START 0 #define BUILD_PCIPREFMEM_SIZE 0 #define BUILD_PCIPREFMEM_END 0 #else -#define BUILD_PCIMEM_START 0xf0000000 -#define BUILD_PCIMEM_SIZE 0x08000000 /* half- of pci window */ +#define BUILD_PCIMEM_START CONFIG_PCIMEM_START +#define BUILD_PCIMEM_SIZE CONFIG_PCIMEM_SIZE #define BUILD_PCIMEM_END (BUILD_PCIMEM_START + BUILD_PCIMEM_SIZE) #define BUILD_PCIPREFMEM_START BUILD_PCIMEM_END #define BUILD_PCIPREFMEM_SIZE (BUILD_PCIPREFMEM_END - BUILD_PCIPREFMEM_START)
On Thu, Apr 28, 2011 at 04:49:52PM +0200, Gerd Hoffmann wrote:
This patch makes the memory window for PCI memory bars configurable via Kconfig.
These BUILD_PCI* variables are only used by four lines of code in pciinit.c. The code only uses BUILD_PCIMEM_START, BUILD_PCIMEM_END, BUILD_PCIPREFMEM_START, BUILD_PCIPREFMEM_END.
I think it would be better to define the kconfig variables needed (eg, PCIMEM_START, PCIMEM_SIZE, PCIMEM_PREFMEM_SIZE) and then change the pciinit.c code to use the CONFIG_* variables directly.
-Kevin
On Thu, Apr 28, 2011 at 08:28:40PM -0400, Kevin O'Connor wrote: [...]
I think it would be better to define the kconfig variables needed (eg, PCIMEM_START, PCIMEM_SIZE, PCIMEM_PREFMEM_SIZE) [...]
Hrmm - on second thought, we know the PCI range should end at 0xfec00000, so I think kconfig only really needs to ask for PCIMEM_SIZE and PCIMEM_PREFMEM_SIZE.
-Kevin
On 04/29/11 02:41, Kevin O'Connor wrote:
On Thu, Apr 28, 2011 at 08:28:40PM -0400, Kevin O'Connor wrote: [...]
I think it would be better to define the kconfig variables needed (eg, PCIMEM_START, PCIMEM_SIZE, PCIMEM_PREFMEM_SIZE) [...]
Hrmm - on second thought, we know the PCI range should end at 0xfec00000, so I think kconfig only really needs to ask for PCIMEM_SIZE and PCIMEM_PREFMEM_SIZE.
I think it is better to ask for the start address, then calculate the memory window size like the patch does as the size is a odd value (0x0ec00000) and I guess for most people it isn't obvious why ...
In case we have a separate window prefetchable memory we'll need one of the sizes too (then calculate the other).
Is there any reason why there is a fixed split, other than making the code simpler (i.e. need only one instead of two passes over all pci devices)?
cheers, Gerd
On Fri, Apr 29, 2011 at 09:37:05AM +0200, Gerd Hoffmann wrote:
On 04/29/11 02:41, Kevin O'Connor wrote:
On Thu, Apr 28, 2011 at 08:28:40PM -0400, Kevin O'Connor wrote: [...]
I think it would be better to define the kconfig variables needed (eg, PCIMEM_START, PCIMEM_SIZE, PCIMEM_PREFMEM_SIZE) [...]
Hrmm - on second thought, we know the PCI range should end at 0xfec00000, so I think kconfig only really needs to ask for PCIMEM_SIZE and PCIMEM_PREFMEM_SIZE.
I think it is better to ask for the start address, then calculate the memory window size like the patch does as the size is a odd value (0x0ec00000) and I guess for most people it isn't obvious why ...
In case we have a separate window prefetchable memory we'll need one of the sizes too (then calculate the other).
Is there any reason why there is a fixed split, other than making the code simpler (i.e. need only one instead of two passes over all pci devices)?
Because pci-to-pci bridge handles memory access and preferchable memory access differently. They are filtered by memory base/limit register and preferchable memory base/limit register. So in order to support multi pci buses, memory and preferchable memory needs to be handled separately.
I guess you're familiar with Linux kernel. Linux kernel also does so. linux/drivers/pci/setup-bus.c has pci_setup_bridge_mmio() and pci_setup_bridge_mmio_pref()
thanks,
Hi,
Is there any reason why there is a fixed split, other than making the code simpler (i.e. need only one instead of two passes over all pci devices)?
Because pci-to-pci bridge handles memory access and preferchable memory access differently.
Sure. The reason for the two regions is clear. But we could still calculate the size at runtime I think. i.e. do one pass, figure how much bars with which sizes we have for each type, calculate the window sizes we'll need, then do a second pass and map the pci regions. That would also allow to pack the regions better.
cheers, Gerd
On Fri, Apr 29, 2011 at 11:25:26AM +0200, Gerd Hoffmann wrote:
Hi,
Is there any reason why there is a fixed split, other than making the code simpler (i.e. need only one instead of two passes over all pci devices)?
Because pci-to-pci bridge handles memory access and preferchable memory access differently.
Sure. The reason for the two regions is clear. But we could still calculate the size at runtime I think. i.e. do one pass, figure how much bars with which sizes we have for each type, calculate the window sizes we'll need, then do a second pass and map the pci regions. That would also allow to pack the regions better.
Oh, I see. It's just for code simplicity. When I discussed with Keven about the two(or more) path way, Kevin didn't require it. So I only added overlap check and didn't implemented smarter way with multi path.
Please refer to http://www.seabios.org/pipermail/seabios/2010-June/000694.html http://www.seabios.org/pipermail/seabios/2010-June/000697.html
If we go for multi path way, we can check the top of the low memory calculate BUILD_PCIMEM_START dynamically so that larger area can be used.
One another trick to pack area is to sort BARs in descending order by its size for alignment requirement. This is well-known trick.
thanks,
On Fri, Apr 29, 2011 at 09:37:05AM +0200, Gerd Hoffmann wrote:
On 04/29/11 02:41, Kevin O'Connor wrote:
On Thu, Apr 28, 2011 at 08:28:40PM -0400, Kevin O'Connor wrote: [...]
I think it would be better to define the kconfig variables needed (eg, PCIMEM_START, PCIMEM_SIZE, PCIMEM_PREFMEM_SIZE) [...]
Hrmm - on second thought, we know the PCI range should end at 0xfec00000, so I think kconfig only really needs to ask for PCIMEM_SIZE and PCIMEM_PREFMEM_SIZE.
I think it is better to ask for the start address, then calculate the memory window size like the patch does as the size is a odd value (0x0ec00000) and I guess for most people it isn't obvious why ...
In case we have a separate window prefetchable memory we'll need one of the sizes too (then calculate the other).
That's fine with me.
Is there any reason why there is a fixed split, other than making the code simpler (i.e. need only one instead of two passes over all pci devices)?
I think the only reason is that a two pass PCI scan is more work than anyone wanted to do.
-Kevin
Hi,
Is there any reason why there is a fixed split, other than making the code simpler (i.e. need only one instead of two passes over all pci devices)?
I think the only reason is that a two pass PCI scan is more work than anyone wanted to do.
Are there any alignment requirements for the two pci memory windows, other than the ones dictated by pci bars in there? To create mtrr entries for the prefetchable memory maybe?
Is there a way to figure the usable address space at runtime instead of depending on BUILD_PCIMEM_START? Can I just check RamSize?
cheers, Gerd
On Tue, May 03, 2011 at 12:34:42PM +0200, Gerd Hoffmann wrote:
Is there any reason why there is a fixed split, other than making the code simpler (i.e. need only one instead of two passes over all pci devices)?
I think the only reason is that a two pass PCI scan is more work than anyone wanted to do.
Are there any alignment requirements for the two pci memory windows, other than the ones dictated by pci bars in there? To create mtrr entries for the prefetchable memory maybe?
I don't know of any alignment restrictions other than that specified in the BAR. SeaBIOS doesn't setup MTRRs right now for pref mem.
Looks like each bus uses 1meg alignment for all devices on that bus.
Is there a way to figure the usable address space at runtime instead of depending on BUILD_PCIMEM_START? Can I just check RamSize?
As far as I'm aware, it would be possible to use RamSize (or better yet, the e820 map).
I'm not really an expert here though - Isaku may know more.
-Kevin
On Tue, May 03, 2011 at 08:47:04PM -0400, Kevin O'Connor wrote:
Is there a way to figure the usable address space at runtime instead of depending on BUILD_PCIMEM_START? Can I just check RamSize?
As far as I'm aware, it would be possible to use RamSize (or better yet, the e820 map).
I suggest e820. Not only ram, but also other chipset functions may use memory area like PCI MMCONFIG which is reserved by e820.
On 05/04/11 04:26, Isaku Yamahata wrote:
On Tue, May 03, 2011 at 08:47:04PM -0400, Kevin O'Connor wrote:
Is there a way to figure the usable address space at runtime instead of depending on BUILD_PCIMEM_START? Can I just check RamSize?
As far as I'm aware, it would be possible to use RamSize (or better yet, the e820 map).
I suggest e820. Not only ram, but also other chipset functions may use memory area like PCI MMCONFIG which is reserved by e820.
Do you have a qemu q35 branch in git somewhere which I could use to test the bridging stuff?
thanks, Gerd
Hi,
I don't know of any alignment restrictions other than that specified in the BAR. SeaBIOS doesn't setup MTRRs right now for pref mem.
Ok.
Looks like each bus uses 1meg alignment for all devices on that bus.
That looks like a random pick to me. Isaku? Looking at all devices behind the bridge, then pick something where all bars fit in and are properly aligned should work, right?
cheers, Gerd
On Wed, May 04, 2011 at 10:06:27AM +0200, Gerd Hoffmann wrote:
On 05/04/11 04:26, Isaku Yamahata wrote:
On Tue, May 03, 2011 at 08:47:04PM -0400, Kevin O'Connor wrote:
Is there a way to figure the usable address space at runtime instead of depending on BUILD_PCIMEM_START? Can I just check RamSize?
As far as I'm aware, it would be possible to use RamSize (or better yet, the e820 map).
I suggest e820. Not only ram, but also other chipset functions may use memory area like PCI MMCONFIG which is reserved by e820.
Do you have a qemu q35 branch in git somewhere which I could use to test the bridging stuff?
You can get it from the followings. The modified seabios and acpi dsdt are also needed to boot it.
git clone http://people.valinux.co.jp/~yamahata/qemu/q35/20110316/qemu git clone http://people.valinux.co.jp/~yamahata/qemu/q35/20110316/seabios
Example: qemu-system-x86_64 ... -M pc_q35 -acpitable 'load_header,data=roms/seabios/src/q35-acpi-dsdt.aml
On Wed, May 04, 2011 at 10:10:10AM +0200, Gerd Hoffmann wrote:
Looks like each bus uses 1meg alignment for all devices on that bus.
That looks like a random pick to me. Isaku?
I think so. At least chipset in qemu/qemu-kvm (PIIX4, q35) doesn't have such restriction.
Looking at all devices behind the bridge, then pick something where all bars fit in and are properly aligned should work, right?
Yes, right. You also want to take hot plug slots into range calculation. hot plug slots needs some ranges for devices that will be inserted in future.
-
Gerd Hoffmann -
Isaku Yamahata -
Kevin O'Connor