Bjorn Helgaas wrote:
> On Wednesday 11 November 2009 12:48:08 pm Yinghai Lu wrote:
>> Bjorn Helgaas wrote:
>>> This patch encapsulate pci_mmcfg_config[] updates. All alloc/free is now
>>> done in pci_mmconfig_add() and free_all_mcfg(), so all updates to
>>> pci_mmcfg_config[] and pci_mmcfg_config_num are in those two functions.
>>>
>>> This replaces the previous sequence of extend_mmcfg(), fill_one_mmcfg()
>>> with the single pci_mmconfig_add() interface. This interface is currently
>>> static but will eventually be used in the host bridge hot-add path.
>>>
>>> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@xxxxxx>
>>> ---
>>> arch/x86/pci/mmconfig-shared.c | 85 ++++++++++++++++++----------------------
>>> 1 files changed, 39 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
>>> index 7a7b6ba..62a8ecd 100644
>>> --- a/arch/x86/pci/mmconfig-shared.c
>>> +++ b/arch/x86/pci/mmconfig-shared.c
>>> @@ -26,14 +26,24 @@
>>> /* Indicate if the mmcfg resources have been placed into the resource table. */
>>> static int __initdata pci_mmcfg_resources_inserted;
>>>
>>> -static __init int extend_mmcfg(int num)
>>> +static __init void free_all_mmcfg(void)
>>> +{
>>> + pci_mmcfg_arch_free();
>>> + pci_mmcfg_config_num = 0;
>>> + kfree(pci_mmcfg_config);
>>> + pci_mmcfg_config = NULL;
>>> +}
>>> +
>>> +static __init struct acpi_mcfg_allocation *pci_mmconfig_add(int segment,
>>> + int start, int end, u64 addr)
>>> {
>>> struct acpi_mcfg_allocation *new;
>>> - int new_num = pci_mmcfg_config_num + num;
>>> + int new_num = pci_mmcfg_config_num + 1;
>>> + int i = pci_mmcfg_config_num;
>>>
>>> new = kzalloc(sizeof(pci_mmcfg_config[0]) * new_num, GFP_KERNEL);
>>> if (!new)
>>> - return -1;
>>> + return NULL;
>>>
>>> if (pci_mmcfg_config) {
>>> memcpy(new, pci_mmcfg_config,
>>> @@ -42,18 +52,13 @@ static __init int extend_mmcfg(int num)
>>> }
>>> pci_mmcfg_config = new;
>>>
>>> - return 0;
>>> -}
>>> -
>>> -static __init void fill_one_mmcfg(u64 addr, int segment, int start, int end)
>>> -{
>>> - int i = pci_mmcfg_config_num;
>>> -
>>> pci_mmcfg_config_num++;
>>> pci_mmcfg_config[i].address = addr;
>>> pci_mmcfg_config[i].pci_segment = segment;
>>> pci_mmcfg_config[i].start_bus_number = start;
>>> pci_mmcfg_config[i].end_bus_number = end;
>>> +
>>> + return &pci_mmcfg_config[i];
>>> }
>>>
>>> static const char __init *pci_mmcfg_e7520(void)
>>> @@ -65,11 +70,9 @@ static const char __init *pci_mmcfg_e7520(void)
>>> if (win == 0x0000 || win == 0xf000)
>>> return NULL;
>>>
>>> - if (extend_mmcfg(1) == -1)
>>> + if (pci_mmconfig_add(0, 0, 255, win << 16) == NULL)
>>> return NULL;
>>>
>>> - fill_one_mmcfg(win << 16, 0, 0, 255);
>>> -
>>> return "Intel Corporation E7520 Memory Controller Hub";
>>> }
>>>
>>> @@ -111,11 +114,9 @@ static const char __init *pci_mmcfg_intel_945(void)
>>> if ((pciexbar & mask) >= 0xf0000000U)
>>> return NULL;
>>>
>>> - if (extend_mmcfg(1) == -1)
>>> + if (pci_mmconfig_add(0, 0, (len >> 20) - 1, pciexbar & mask) == NULL)
>>> return NULL;
>>>
>>> - fill_one_mmcfg(pciexbar & mask, 0, 0, (len >> 20) - 1);
>>> -
>>> return "Intel Corporation 945G/GZ/P/PL Express Memory Controller Hub";
>>> }
>>>
>>> @@ -124,7 +125,7 @@ static const char __init *pci_mmcfg_amd_fam10h(void)
>>> u32 low, high, address;
>>> u64 base, msr;
>>> int i;
>>> - unsigned segnbits = 0, busnbits;
>>> + unsigned segnbits = 0, busnbits, end_bus;
>>>
>>> if (!(pci_probe & PCI_CHECK_ENABLE_AMD_MMCONF))
>>> return NULL;
>>> @@ -158,11 +159,13 @@ static const char __init *pci_mmcfg_amd_fam10h(void)
>>> busnbits = 8;
>>> }
>>>
>>> - if (extend_mmcfg(1 << segnbits) == -1)
>>> - return NULL;
>>> -
>>> + end_bus = (1 << busnbits) - 1;
>>> for (i = 0; i < (1 << segnbits); i++)
>>> - fill_one_mmcfg(base + (1<<28) * i, i, 0, (1 << busnbits) - 1);
>>> + if (pci_mmconfig_add(i, 0, end_bus,
>>> + base + (1<<28) * i) == NULL) {
>>> + free_all_mmcfg();
>>> + return NULL;
>>> + }
>> here it is not right. will have extra alloc/and free.
>>
>> you could use extend/fill to do pci_mmconfig_add
>
> I don't see the change in behavior. Can you clarify and give an
> example, please?
>
> Previously, we would extend the table by N entries, then fill them
> all. If the table couldn't be extended, we wouldn't add any entries
> at all. The fill never fails. We return with either all the entries
> added, or none of them.
if you got 16 entries:
pci_mmconfig_add every time will allocate and copy the old.
you will allocate 16 times and copy 15 times.
orginal is extend 16, and fill one by one.
your following patches change that to list. so it is ok at last
YH
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
