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?
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.
cheers, Gerd
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
On 10/09/12 16:55, Jason Baron wrote:
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?
It doesn't which is a bug (this is what I've tried to say with the "nice catch" above).
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.
[ note: the above refers to piix ]
That's why I set mtrr_base on q35 to 0xc0000000 (in order to have a single mtrr uc region).
Yep. You could likewise pick 0x80000000 or 0xc0000000 depending on installed guest memory. Would mean to make mmconfig location dynamic too (at 0x70000000 / 0xb0000000), not sure how easy that is wrt initialization ordering. But it isn't that important long-term, with the increating memory sizes for physical & virtual machines it will be more and more rare that we can use 0x8000000+ for the pci window.
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?
Dunno. On my real laptop it seems to be not covered. But at the end of the day this is virtual only so it is more or less a cosmetic issue.
cheers, Gerd
-
Gerd Hoffmann -
Jason Baron