Re: [PATCH 2/2] acpi based pci gap caluculation v2

From: Alok Kataria
Date: Wed Jun 25 2008 - 21:18:45 EST


On Wed, 2008-06-25 at 18:00 -0700, Zhao Yakui wrote:
> On Wed, 2008-06-25 at 09:57 -0700, Alok Kataria wrote:
> > On Wed, 2008-06-25 at 01:38 -0700, Zhao Yakui wrote:
> > > On Tue, 2008-06-24 at 23:04 -0700, Alok kataria wrote:

> > > The pci_mem_start is still gotten by parsing the E820 table.But the
> > > input parameter start_addr will be used in the function of
> > > e820_search_gap.
> > > If the following is the resource start address returned by the PCI0
> > > _CRS object , maybe the different pci_mem_start will be gotten.
> > > 0xE0000000
> > > 0xE4000000
> > >
> > > At the same time if several start address is returned by the _CRS
> > > object, the e820 table will be parsed several times.
> >
> > Yes we will parse e820 several times, but we don't initialize
> > pci_mem_start in every pass.
> > It will be initialized only twice once via the e820_setup_gap code path
> > and once via pci_setup_gap.
> > And i think you agree that both of these are required ?
> Agree with what you said and IMO it is OK.
> >
> > During the gap calculation the previous code or the code now in
> > e820_setup_gap does this...
> > calculates a gap in e820_map from 0 to 2^32.
> > Initializes pci_mem_start.
> >
> > And now with this patch, the code in pci_setup_gap
> > does this...
> > for each _CRS under PCI0
> > search gap from start_addr of _CRS to 2^32 *[1].
> > Initialize pci_mem_start with the biggest gap that we could
> > find.
> Yes. But maybe the pci_mem_start obtained in pci_setup_gap will be
> different with that obtained in e820_setup_gap on some bogus BIOS.
> If the pci_mem_start obtained in pci_setup_gap can meet the requirement,
> it is also reasonable.

Ok, so the whole problem crops up from what if the BIOS itself is bogus.
Ok in that case too we take proper care that pci_mem_start is within the
expected limits, i.e. we make sure that
1. pci_mem_start is not initialized to an already allocated resource
address and..
2. pci_mem_start is within the lower 32bit of address space.

I assume you also agree that all the checks are in place.
Also it would be great if you could ACK both the patches as well.

Thanks for the review.
Alok

> > Essentially, what we are doing is just limiting the gap calculation to a
> > smaller address space depending on the ACPI information we get.
> >
> > Now then, what problem do you see with this approach ?
> > *[1]
> > While writing this mail i figured out that, instead of searching from
> > start_addr of _CRS to 2^32 we should just search till *end_addr* of _CRS resource.
> > I will send a patch to that effect.
> If this, IMO it will be OK.
> > Thanks,
> > Alok
> >
> >
> > >
> > > Thanks.
> > > Yakui
> > >
> > > > Thanks,
> > > > Alok
> > >
> >
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/