Patchwork [3/4] Switch from array based resource allocation to list

login
register
mail settings
Submitter Alexey Korolev
Date April 19, 2012, 7 a.m.
Message ID <4F8FB819.6050709@endace.com>
Download mbox | patch
Permalink /patch/153659/
State New
Headers show

Comments

Alexey Korolev - April 19, 2012, 7 a.m.
On Thu, Apr 12, 2012 at 03:28:02PM +1200, Alexey Korolev wrote:
>> On 12/04/12 15:15, Kevin O'Connor wrote:
>>> This was also me playing with one of Gerd's patches.  It just makes
>>> the bar read/write code 64bit aware.  It doesn't actually program
>>> them.  The logic to do real 64bit allocations would require list
>>> merging.  Is this something you have looked at?
>> Right. I see what you mean here. Shall I play around with 64bit
>> support on top of these patches?
> If that makes sense, then sure.
>
> -Kevin
Hi Kevin,

Here is the whole series of patches including 64bit support.
I don't send this in a different thread as this continues the previous discussion.

To save your time on parsing I've added little descriptions of changes comparing to patches you have sent.

[Patches 1-4]
The same as previous patches just comment about bar=-1 is added

[Patch 5]
Track-alignment-explicitly
Almost the same as the previous, just changed priority from r->align to r->sum when setting start address of root regions.

I guess there are more chances to fit memory regions if we try place regions with higher r->sum like it was before.
Consider default config
#define BUILD_PCIMEM_START       0xe0000000
#define BUILD_PCIMEM_END          0xfec00000 

Image we have 1 pref. mem. region of 128MB. And many small memory regions which take rest of available 492MB - 128MB
If we have alignment priority.
PCI pref memory region will start from F000 0000
and
PCI memoryregion will start from 0xe0000000
and do not fit.

If we choose size based priority it will fit.

[Patch 6]
pciinit-bridges-can-have-two-regions-too
Changed as we discussed. Now taking into account ROM regions too.

[Patch 7]
Switch-to-64bit-variable-types.patch
Same as prvious

[Patch 8 ]
New: The pci_region structure is added.
 Move setting of bus base address to pci_region_map_entries.

[Patch 9 ]
New: Add discovery if bridge region is 64bit is capable.

[Patch 10]
New:  Migrate 64bit entries to 64bit pci regions
 if they do not fit in 32bit range. Pci region stats
 are now calculated. Added protection when total size of
 PCI resources is over 4GB.

[Patch 11]
New: This patch solves issues on Windows guests, when 64bit
 BAR's are present. It is also helpful on Linux guests
 when use_crs kernel boot option is set.
Kevin O'Connor - April 20, 2012, 3:36 a.m.
On Thu, Apr 19, 2012 at 07:00:41PM +1200, Alexey Korolev wrote:
> Here is the whole series of patches including 64bit support.

Thanks.

[...]
> [Patch 5]
> Track-alignment-explicitly
> Almost the same as the previous, just changed priority from r->align to r->sum when setting start address of root regions.
> 
> I guess there are more chances to fit memory regions if we try place regions with higher r->sum like it was before.
> Consider default config
> #define BUILD_PCIMEM_START       0xe0000000
> #define BUILD_PCIMEM_END          0xfec00000 
> 
> Image we have 1 pref. mem. region of 128MB. And many small memory regions which take rest of available 492MB - 128MB
> If we have alignment priority.
> PCI pref memory region will start from F000 0000
> and
> PCI memoryregion will start from 0xe0000000
> and do not fit.

The section with the lowest alignment would get allocated first and be
at a higher address.  So, in your example, the regular memory region
would be assigned 0xe8000000 and then pref mem would be assigned
0xe0000000.

Your example is why alignment should be used instead of size.  If size
is used, then what you cite above occurs (pref mem has the lower size
and is thus allocated first at the higher address).

> [Patch 10]
> New:  Migrate 64bit entries to 64bit pci regions
>  if they do not fit in 32bit range. Pci region stats
>  are now calculated. Added protection when total size of
>  PCI resources is over 4GB.

Patches 1-9 and 11 look okay to me.

On patch 10, it would be preferable to separate the dynamic
calculation of sum/alignment changes from the 64bit support.
Otherwise, the core algorithm of patch 10 looks okay.  Though it seems
like the code is recalculating sum/alignment more than needed, and I
think the list manipulation in pci_region_migrate_64bit_entries could
be streamlined.

Thanks again,
-Kevin
Alexey Korolev - April 20, 2012, 8:54 a.m.
[...]
>> [Patch 5]
>> Track-alignment-explicitly
>> Almost the same as the previous, just changed priority from r->align to r->sum when setting start address of root regions.
>>
>> I guess there are more chances to fit memory regions if we try place regions with higher r->sum like it was before.
>> Consider default config
>> #define BUILD_PCIMEM_START       0xe0000000
>> #define BUILD_PCIMEM_END          0xfec00000
>>
>> Image we have 1 pref. mem. region of 128MB. And many small memory regions which take rest of available 492MB - 128MB
>> If we have alignment priority.
>> PCI pref memory region will start from F000 0000
>> and
>> PCI memoryregion will start from 0xe0000000
>> and do not fit.
>
>The section with the lowest alignment would get allocated first and be
>at a higher address.  So, in your example, the regular memory region
>would be assigned 0xe8000000 and then pref mem would be assigned
>0xe0000000.
>
>Your example is why alignment should be used instead of size.  If size
>is used, then what you cite above occurs (pref mem has the lower size
>and is thus allocated first at the higher address).

Ahh the lowest alignment! In other words if we go from PCIMEM_START to PCIMEM_END we first
assign address range for the region with the largest PCI resource size (highest alignment). 
I see. I will put it back then.

....
>On patch 10, it would be preferable to separate the dynamic
>calculation of sum/alignment changes from the 64bit support.
>Otherwise, the core algorithm of patch 10 looks okay.  Though it seems
>like the code is recalculating sum/alignment more than needed, and I
>think the list manipulation in pci_region_migrate_64bit_entries could
>be streamlined.
Right, sum/alignment recalculation is important for migration. Will split it and submit new series.

Thanks,
Alexey

Patch

From b17c5396d60b546f263b6e22baaab9e1b9c835c3 Mon Sep 17 00:00:00 2001
From: Alexey Korolev <alexey.korolev@endace.com>
Date: Thu, 19 Apr 2012 17:52:41 +1200
Subject: [PATCH 11/11] Fix 64bit PCI issues on Windows

 This patch solves issues on Windows guests, when 64bit
 BAR's are present. It is also helpful on Linux guests
 when use_crs kernel boot option is set.

Signed-off-by: Alexey Korolev <alexey.korolev@endace.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 src/acpi-dsdt.dsl |    7 +++++
 src/acpi-dsdt.hex |   64 +++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 4bdc268..4a18617 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -175,6 +175,13 @@  DefinitionBlock (
                     0x00000000,         // Address Translation Offset
                     0x1EC00000,         // Address Length
                     ,, , AddressRangeMemory, TypeStatic)
+                QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
+                    0x00000000,          // Address Space Granularity
+                    0x8000000000,        // Address Range Minimum
+                    0xFFFFFFFFFF,        // Address Range Maximum
+                    0x00000000,          // Address Translation Offset
+                    0x8000000000,        // Address Length
+                    ,, , AddressRangeMemory, TypeStatic)
             })
         }
     }
diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex
index a4af597..07f0e18 100644
--- a/src/acpi-dsdt.hex
+++ b/src/acpi-dsdt.hex
@@ -3,12 +3,12 @@  static unsigned char AmlCode[] = {
 0x53,
 0x44,
 0x54,
-0x21,
+0x4f,
 0x11,
 0x0,
 0x0,
 0x1,
-0xe8,
+0xca,
 0x42,
 0x58,
 0x50,
@@ -110,16 +110,16 @@  static unsigned char AmlCode[] = {
 0x47,
 0x42,
 0x10,
-0x44,
-0x81,
+0x42,
+0x84,
 0x5f,
 0x53,
 0x42,
 0x5f,
 0x5b,
 0x82,
-0x4c,
-0x80,
+0x4a,
+0x83,
 0x50,
 0x43,
 0x49,
@@ -2064,10 +2064,10 @@  static unsigned char AmlCode[] = {
 0x52,
 0x53,
 0x11,
-0x42,
-0x7,
+0x40,
 0xa,
-0x6e,
+0xa,
+0x9c,
 0x88,
 0xd,
 0x0,
@@ -2176,6 +2176,52 @@  static unsigned char AmlCode[] = {
 0x0,
 0xc0,
 0x1e,
+0x8a,
+0x2b,
+0x0,
+0x0,
+0xc,
+0x3,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x80,
+0x0,
+0x0,
+0x0,
+0xff,
+0xff,
+0xff,
+0xff,
+0xff,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x80,
+0x0,
+0x0,
+0x0,
 0x79,
 0x0,
 0x10,
-- 
1.7.5.4