headers
Kevin O'Connor | 1 Oct 2011 21:01

[PATCH 0/6] PCI init code refactoring

Some cleanups to the pciinit.c code.

These are some things I noticed while tracking down the recent bug
reports caused by calling ALIGN_DOWN with a zero alignment.  The fix
to that bug was small enough to commit immediately.  However, I think
it would be worthwhile to also commit the cleanups I found.

Given this is just a cleanup, I don't intend to commit until after
v1.6.3 is tagged.

-Kevin

Kevin O'Connor (6):
  Use standard formatting for PCI info during PCI init pass.
  Separate pciinit.c into clearly delineated sections.
  Simplify pci_bios_init_root_regions().
  Use pci->header_type in pci_bar() to avoid unnecessary
    pci_config_readb.
  Introduce PCI child device iterators and use in pciinit.c.
  Simplify pci_slot_get_irq().

 src/pci.c     |    2 +
 src/pci.h     |   12 ++
 src/pciinit.c |  337 +++++++++++++++++++++++++++------------------------------
 3 files changed, 173 insertions(+), 178 deletions(-)

--

-- 
1.7.6.2
Permalink | Reply | 7 comments |
headers
Kevin O'Connor | 1 Oct 2011 21:02

[PATCH 1/6] Use standard formatting for PCI info during PCI init pass.

Format BDF (bus, device, fn) and vendor:device debug output in a more
user-readable format.

Signed-off-by: Kevin O'Connor <kevin <at> koconnor.net>
---
 src/pciinit.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index 0d8758e..ebd8152 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
 <at>  <at>  -235,8 +235,8  <at>  <at>  static void pci_bios_init_device(struct pci_device *pci)
     u16 bdf = pci->bdf;
     int pin, pic_irq;

-    dprintf(1, "PCI: bus=%d devfn=0x%02x: vendor_id=0x%04x device_id=0x%04x\n"
-            , pci_bdf_to_bus(bdf), pci_bdf_to_devfn(bdf)
+    dprintf(1, "PCI: init bdf=%02x:%02x.%x id=%04x:%04x\n"
+            , pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf)
             , pci->vendor, pci->device);
     pci_init_device(pci_class_tbl, pci, NULL);

 <at>  <at>  -507,9 +507,11  <at>  <at>  static void pci_bios_map_device_in_bus(int bus)
     struct pci_device *pci;

     foreachpci(pci) {
-        if (pci_bdf_to_bus(pci->bdf) != bus)
+        u16 bdf = pci->bdf;
+        if (pci_bdf_to_bus(bdf) != bus)
(Continue reading)

Permalink | Reply | 0 comments |
headers
Kevin O'Connor | 1 Oct 2011 21:02

[PATCH 2/6] Separate pciinit.c into clearly delineated sections.

There are four separate phases of the current PCI initialization code:
bus initialization, bus sizing, bar allocation, and misc device init.
Move the code exclusively called in each phase next to each other, and
clearly mark each section.

Signed-off-by: Kevin O'Connor <kevin <at> koconnor.net>
---
 src/pciinit.c |  221 +++++++++++++++++++++++++++++++--------------------------
 1 files changed, 121 insertions(+), 100 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index ebd8152..62583a3 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
 <at>  <at>  -45,11 +45,6  <at>  <at>  static struct pci_bus {
 } *busses;
 static int busses_count;

-static void pci_bios_init_device_in_bus(int bus);
-static void pci_bios_check_device_in_bus(int bus);
-static void pci_bios_init_bus_bases(struct pci_bus *bus);
-static void pci_bios_map_device_in_bus(int bus);
-
 static int pci_size_to_index(u32 size, enum pci_region_type type)
 {
     int index = __fls(size);
 <at>  <at>  -79,17 +74,6  <at>  <at>  static enum pci_region_type pci_addr_to_type(u32 addr)
     return PCI_REGION_TYPE_MEM;
 }
(Continue reading)

Permalink | Reply | 0 comments |
headers
Kevin O'Connor | 1 Oct 2011 21:03

[PATCH 3/6] Simplify pci_bios_init_root_regions().

Add some comments and refactor out some duplicated code.

Signed-off-by: Kevin O'Connor <kevin <at> koconnor.net>
---
 src/pciinit.c |   38 +++++++++++++-------------------------
 1 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index 62583a3..ccf71d1 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
 <at>  <at>  -439,38 +439,26  <at>  <at>  static void pci_bios_check_device_in_bus(int bus)

 #define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)

+// Setup region bases (given the regions' size and alignment)
 static int pci_bios_init_root_regions(u32 start, u32 end)
 {
     struct pci_bus *bus = &busses[0];

     bus->r[PCI_REGION_TYPE_IO].base = 0xc000;

-    if (bus->r[PCI_REGION_TYPE_MEM].sum < bus->r[PCI_REGION_TYPE_PREFMEM].sum) {
-        bus->r[PCI_REGION_TYPE_MEM].base =
-            ROOT_BASE(end,
-                      bus->r[PCI_REGION_TYPE_MEM].sum,
-                      bus->r[PCI_REGION_TYPE_MEM].max);
-        bus->r[PCI_REGION_TYPE_PREFMEM].base =
-            ROOT_BASE(bus->r[PCI_REGION_TYPE_MEM].base,
-                      bus->r[PCI_REGION_TYPE_PREFMEM].sum,
(Continue reading)

Permalink | Reply | 0 comments |
headers
Kevin O'Connor | 1 Oct 2011 21:03

[PATCH 4/6] Use pci->header_type in pci_bar() to avoid unnecessary pci_config_readb.

Pass a 'struct pci_device' into pci_bar and update all callers.

Signed-off-by: Kevin O'Connor <kevin <at> koconnor.net>
---
 src/pciinit.c |   52 +++++++++++++++++++++++-----------------------------
 1 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index ccf71d1..e384a68 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
 <at>  <at>  -74,25 +74,21  <at>  <at>  static enum pci_region_type pci_addr_to_type(u32 addr)
     return PCI_REGION_TYPE_MEM;
 }

-static u32 pci_bar(u16 bdf, int region_num)
+static u32 pci_bar(struct pci_device *pci, int region_num)
 {
     if (region_num != PCI_ROM_SLOT) {
         return PCI_BASE_ADDRESS_0 + region_num * 4;
     }

 #define PCI_HEADER_TYPE_MULTI_FUNCTION 0x80
-    u8 type = pci_config_readb(bdf, PCI_HEADER_TYPE);
-    type &= ~PCI_HEADER_TYPE_MULTI_FUNCTION;
+    u8 type = pci->header_type & ~PCI_HEADER_TYPE_MULTI_FUNCTION;
     return type == PCI_HEADER_TYPE_BRIDGE ? PCI_ROM_ADDRESS1 : PCI_ROM_ADDRESS;
 }

-static void pci_set_io_region_addr(u16 bdf, int region_num, u32 addr)
(Continue reading)

Permalink | Reply | 0 comments |
headers
Kevin O'Connor | 1 Oct 2011 21:05

[PATCH 5/6] Introduce PCI child device iterators and use in pciinit.c.

Replace O(n^2) bus iterators with new a host iterator (for bus 0) and
a new pci bridge child bus iterator.

Signed-off-by: Kevin O'Connor <kevin <at> koconnor.net>
---
 src/pci.c     |    2 +
 src/pci.h     |   12 +++++++
 src/pciinit.c |   94 +++++++++++++++++++++++----------------------------------
 3 files changed, 52 insertions(+), 56 deletions(-)

diff --git a/src/pci.c b/src/pci.c
index 6031c9f..7697583 100644
--- a/src/pci.c
+++ b/src/pci.c
 <at>  <at>  -137,6 +137,8  <at>  <at>  pci_probe_devices(void)
                     MaxPCIBus = bus;
             } else {
                 rootbus = parent->rootbus;
+                if (!parent->firstchild)
+                    parent->firstchild = dev;
             }

             // Populate pci_device info.
diff --git a/src/pci.h b/src/pci.h
index a2a5a4c..b19b2e9 100644
--- a/src/pci.h
+++ b/src/pci.h
 <at>  <at>  -44,6 +44,7  <at>  <at>  struct pci_device {
     u8 rootbus;
     struct pci_device *next;
(Continue reading)

Permalink | Reply | 0 comments |
headers
Kevin O'Connor | 1 Oct 2011 21:05

[PATCH 6/6] Simplify pci_slot_get_irq().

Signed-off-by: Kevin O'Connor <kevin <at> koconnor.net>
---
 src/pciinit.c |   20 +++++++-------------
 1 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index b87825d..e2c5f3c 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
 <at>  <at>  -101,13 +101,11  <at>  <at>  const u8 pci_irqs[4] = {
     10, 10, 11, 11
 };

-/* return the global irq number corresponding to a given device irq
-   pin. We could also use the bus number to have a more precise
-   mapping. */
-static int pci_slot_get_pirq(u16 bdf, int irq_num)
+// Return the global irq number corresponding to a host bus device irq pin.
+static int pci_slot_get_irq(u16 bdf, int pin)
 {
     int slot_addend = pci_bdf_to_dev(bdf) - 1;
-    return (irq_num + slot_addend) & 3;
+    return pci_irqs[(pin - 1 + slot_addend) & 3];
 }

 /* PIIX3/PIIX4 PCI to ISA bridge */
 <at>  <at>  -215,23 +213,19  <at>  <at>  static const struct pci_device_id pci_device_tbl[] = {
 static void pci_bios_init_device(struct pci_device *pci)
 {
     u16 bdf = pci->bdf;
(Continue reading)

Permalink | Reply | 0 comments |
headers
Kevin O'Connor | 1 Oct 2011 22:01

[PATCH] Locally allocate busses[] and always make it maximum sized.

One more patch.

-Kevin

From c0068c484162337afda1ede0322a4cfcd37ded23 Mon Sep 17 00:00:00 2001
From: Kevin O'Connor <kevin <at> koconnor.net>
Date: Sat, 1 Oct 2011 15:50:55 -0400
Subject: [PATCH] Locally allocate busses[] and always make it maximum sized.
To: seabios <at> seabios.org

Don't bother tracking busses_count - there is plenty of dynamic ram -
just always allocate the maximum size it can be (256).

Since only a few functions need to access busses[] pass it by
reference instead of using a global.

Signed-off-by: Kevin O'Connor <kevin <at> koconnor.net>
---
 src/pciinit.c |   42 +++++++++++++++++-------------------------
 1 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index e2c5f3c..c618a8f 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
 <at>  <at>  -1,6 +1,6  <at>  <at> 
 // Initialize PCI devices (on emulators)
 //
-// Copyright (C) 2008  Kevin O'Connor <kevin <at> koconnor.net>
+// Copyright (C) 2008-2011  Kevin O'Connor <kevin <at> koconnor.net>
(Continue reading)

Permalink | Reply | 0 comments |
headers
Gleb Natapov | 2 Oct 2011 10:31
Picon
Favicon

Re: [PATCH] acpi: add Local APIC NMI Structure to MADT.

On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
> Add Local APIC NMI Structure to ACPI MADT.
> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji <at> jp.fujitsu.com>
> ---
>  src/acpi.c |   24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> Index: seabios/src/acpi.c
> ===================================================================
> --- seabios.orig/src/acpi.c
> +++ seabios/src/acpi.c
>  <at>  <at>  -134,6 +134,14  <at>  <at>  struct madt_intsrcovr {
>      u16 flags;
>  } PACKED;
>  
> +struct madt_local_nmi {
> +    ACPI_SUB_HEADER_DEF
> +    u8  processor_id;           /* ACPI processor id */
> +    u16 flags;                  /* MPS INTI flags */
> +    u8  lint;                   /* Local APIC LINT# */
> +} PACKED;
> +
> +
>  /*
>   * ACPI 2.0 Generic Address Space definition.
>   */
>  <at>  <at>  -288,7 +296,9  <at>  <at>  build_madt(void)
>      int madt_size = (sizeof(struct multiple_apic_table)
>                       + sizeof(struct madt_processor_apic) * MaxCountCPUs
(Continue reading)

Permalink | Reply | 3 comments |
headers
Gleb Natapov | 2 Oct 2011 10:50
Picon
Favicon

Re: [PATCH] acpi: add Local APIC NMI Structure to MADT.

On Mon, Sep 26, 2011 at 07:58:56PM +0900, Kenji Kaneshige wrote:
> (2011/09/23 13:31), Kevin O'Connor wrote:
> >On Thu, Sep 22, 2011 at 09:58:58PM +0900, Kenji Kaneshige wrote:
> >>Add Local APIC NMI Structure to ACPI MADT.
> >
> >Thanks.  Can you give a brief description of what this is needed by?
> >
> >Also, is this for kvm, qemu, or some other emulator?
> 
> This describes LINT pin (LINT0 or LINT1) to which NMI is connected,
> and this information is needed by OS to initialize local APIC.
> Current seabios provides this information through MP configuration
> table but doesn't provide it through ACPI MADT. I found this when
> I reviewed "inject NMI" feature in qemu/kvm.
> 
> By the way, I think usually NMI is connected to all processors'
> LINT1 as MultiProcessor spec indicates, and my patch assumes this.
> But, I noticed that seabios's MP configuration table provides only
> a NMI information connected to BSP. Is there any special reason
> about this?
> 
Should be changed to 0xff too.

--
			Gleb.
Permalink | Reply | 1 comment |

Gmane