Re: [SeaBIOS] [PATCH v2 1/2] pci: introduce pci_region to manage pci io/memory/prefmemory regions.

[Michael S. Tsirkin]

On Thu, Oct 28, 2010 at 02:04:05PM +0900, Isaku Yamahata wrote:
> This patch adds helper functions to manage pci area.
>
> Signed-off-by: Isaku Yamahata
>
> Changes v1 -> v2
> add comments
>
> Makefile         |    3 +-
>  src/pci_region.c |   77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util.h       |   28 +++++++++++++++++++
>  3 files changed, 107 insertions(+), 1 deletions(-)
>  create mode 100644 src/pci_region.c
>
> diff --git a/Makefile b/Makefile
> index 9d412f1..1663a5d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -19,7 +19,8 @@ SRCBOTH=misc.c pmm.c stacks.c output.c util.c block.c floppy.c ata.c mouse.c \
>  SRC16=$(SRCBOTH) system.c disk.c font.c
>  SRC32FLAT=$(SRCBOTH) post.c shadow.c memmap.c coreboot.c boot.c \
>        acpi.c smm.c mptable.c smbios.c pciinit.c optionroms.c mtrr.c \
>  lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c dev-i440fx.c
>  lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c dev-i440fx.c \
>  pci_region.c
>
> SRC32SEG=util.c output.c pci.c pcibios.c apm.c stacks.c
>
> cc-option = $(shell if test -z "`$(1) $(2) -S -o /dev/null -xc \
> diff --git a/src/pci_region.c b/src/pci_region.c
> new file mode 100644
> index 0000000..c8656cb
> --- /dev/null
> +++ b/src/pci_region.c
> @@ -0,0 +1,77 @@
> +// helper functions to manage pci io/memory/prefetch memory region
> +//
> +// Copyright (C) 2009 Isaku Yamahata <yamahata at valinux co jp>
> +//
> +// This file may be distributed under the terms of the GNU LGPLv3 license.
> +//
> +//
>
> +#include "util.h"
>
> +#define PCI_REGION_DISABLED     (-1)
>
> +void pci_region_init(struct pci_region *r, u32 start, u32 end)
> +{
> r->start = start;
> r->end = end;
> r->cur_end = start;
>
> +}
>
> +// PCI_REGION_DISABLED represents that the region is in special state.
> +// its value is chosen such that cur_end can't be PCI_REGION_DISABLED normally.
> +u32 pci_region_disable(struct pci_region *r)
> +{
> return r->cur_end = PCI_REGION_DISABLED;
>
> +}
>
> +static int pci_region_disabled(const struct pci_region *r)
> +{
> return r->cur_end == PCI_REGION_DISABLED;
>
> +}
>
> +static u32 pci_region_alloc_align(struct pci_region *r, u32 size, u32 align)
> +{
> if (pci_region_disabled(r)) {
>    return 0;
> }
> u32 s = ALIGN(r->cur_end, align);
> if (s > r->end || s < r->cur_end) {
>    return 0;
> }
> u32 e = s + size;
> if (e > r->end || e < s) {
>    return 0;
> }
> r->cur_end = e;
> return s;
>
> +}
>
> +u32 pci_region_alloc(struct pci_region *r, u32 size)
> +{
> return pci_region_alloc_align(r, size, size);
>
> +}
>
> +u32 pci_region_align(struct pci_region *r, u32 align)
> +{
> return pci_region_alloc_align(r, 0, align);
>
> +}
>
> +void pci_region_revert(struct pci_region *r, u32 addr)
> +{
> r->cur_end = addr;
>
> +}
>
> +u32 pci_region_addr(const struct pci_region *r)
> +{
> if (pci_region_disabled(r)){
>    return r->end;
> }
> return r->cur_end;
>
> +}
>
> +u32 pci_region_size(const struct pci_region *r)
> +{
> return r->end - r->start;
>
> +}
> diff --git a/src/util.h b/src/util.h
> index 5cc9f17..17eedd0 100644
> --- a/src/util.h
> +++ b/src/util.h
> @@ -344,6 +344,34 @@ void qemu_prep_reset(void);
>  void smm_save_and_copy(void);
>  void smm_relocate_and_restore(void);
>
> +// pci_region.c
> +// region allocator. pci region allocates the requested region
> +// sequentially with overflow check.
> +struct pci_region {
> // The region is [start, region).

[start end)?
> u32 start;
> u32 end;
> // The next allocation is start from this.
> // i.e. [start, cur_end) is allocated.
> // Right after initialization cur_end == start.
> u32 cur_end;

What is the value for a region at 0xfffff000 and size 0x1000?

Maybe first, last is better to detect this?
> +};
> +// initialize the pci_region of [start, end)
> +void pci_region_init(struct pci_region *r, u32 start, u32 end);
> +// allocate the region of size
> +u32 pci_region_alloc(struct pci_region *r, u32 size);
> +// make the next allocation aligned to align
> +u32 pci_region_align(struct pci_region *r, u32 align);
> +// revert the allocation to addr.
> +void pci_region_revert(struct pci_region *r, u32 addr);
> +// make the allocation fail.
> +u32 pci_region_disable(struct pci_region *r);
> +// returns the current allocation point.
> +u32 pci_region_addr(const struct pci_region *r);
> +// returns the region size.
> +u32 pci_region_size(const struct pci_region *r);
>
> // pciinit.c
>  extern const u8 pci_irqs[4];
>  void pci_bios_allocate_regions(u16 bdf, void *arg);
> -- 
> 1.7.1.1

[PATCH v2 1/2] pci: introduce pci_region to manage pci io/memory/prefmemory regions.

[Isaku Yamahata]

On Thu, Oct 28, 2010 at 07:56:41AM +0200, Michael S. Tsirkin wrote:
> > diff --git a/src/util.h b/src/util.h
> > index 5cc9f17..17eedd0 100644
> > --- a/src/util.h
> > +++ b/src/util.h
> > @@ -344,6 +344,34 @@ void qemu_prep_reset(void);
> >  void smm_save_and_copy(void);
> >  void smm_relocate_and_restore(void);
> >
> > +// pci_region.c
> > +// region allocator. pci region allocates the requested region
> > +// sequentially with overflow check.
> > +struct pci_region {
> > // The region is [start, region).
>
> [start end)?

Yes. Will fix.
> > u32 start;
> > u32 end;
> > // The next allocation is start from this.
> > // i.e. [start, cur_end) is allocated.
> > // Right after initialization cur_end == start.
> > u32 cur_end;
>
> What is the value for a region at 0xfffff000 and size 0x1000?
>
> Maybe first, last is better to detect this?

You mean [first, last] instead of [start, end).
Maybe Makes sense.
Anyway the area right below 4G can't be used for PCI
because it's used for LAPIC and so on.
> > +};
> > +// initialize the pci_region of [start, end)
> > +void pci_region_init(struct pci_region *r, u32 start, u32 end);
> > +// allocate the region of size
> > +u32 pci_region_alloc(struct pci_region *r, u32 size);
> > +// make the next allocation aligned to align
> > +u32 pci_region_align(struct pci_region *r, u32 align);
> > +// revert the allocation to addr.
> > +void pci_region_revert(struct pci_region *r, u32 addr);
> > +// make the allocation fail.
> > +u32 pci_region_disable(struct pci_region *r);
> > +// returns the current allocation point.
> > +u32 pci_region_addr(const struct pci_region *r);
> > +// returns the region size.
> > +u32 pci_region_size(const struct pci_region *r);
> >
> > // pciinit.c
> >  extern const u8 pci_irqs[4];
> >  void pci_bios_allocate_regions(u16 bdf, void *arg);
> > -- 
> > 1.7.1.1