On Tue, Oct 09, 2012 at 08:48:38AM +0200, Gerd Hoffmann wrote:
On 10/09/12 05:35, Jason Baron wrote:
From: Jason Baron jbaron@redhat.com
Set up the UC area of mtrr dynamically based on mtrr_base. This allows the bios to work for other chipsets that might want to set the mtrr.
Nice catch, didn't notice the mtrr thing when making the pci window dynamic.
+u64 mtrr_base = BUILD_PCIMEM_START;
u64 pcimem_start = BUILD_PCIMEM_START;
Any reason you add a new variable instead of just using pcimem_start directly?
For the piix case. pcimem_start can be set to RamSize at the beginning of pci_bios_map_devices(). And the mtrr uc region has always been at 0xe000000, so I needed two variables.
That raises a question to me as to whether the uc region is covering the pci mem space if RamSize != 0xe000000?
I guess when pcimem_start is updated mtrr_base should be updated too. And we probably should make sure pcimem_start has a value mtrr can handle. Which pretty much implies pcimem_start can be one of 0x80000000, 0xc0000000 or 0xe000000 depending on the amout of memory the guest has if we want stick to a single mtrr register.
That's why I set mtrr_base on q35 to 0xc0000000 (in order to have a single mtrr uc region). That said, it does not cover the mmconfig space currently, which runs from 0xb0000000-0xc0000000. So, I'm not sure if mmconfig should be covered by the UC region? If so, I can shift the beginning of the mmconfig to 0xc0000000 (making the pci i/o space start at 0xd0000000), or we can introduce a second mtrr region.
Thanks,
-Jason