Patchwork [V6,29/32] pci: cosmetic on pci_upadte_mappings()

login
register
mail settings
Submitter Isaku Yamahata
Date Oct. 30, 2009, 12:21 p.m.
Message ID <1256905286-25435-30-git-send-email-yamahata@valinux.co.jp>
Download mbox | patch
Permalink /patch/37291/
State New
Headers show

Comments

Isaku Yamahata - Oct. 30, 2009, 12:21 p.m.
Remove one indentation of pci_update_mappings.
Just for cosmetics, no logic change.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pci.c |  146 ++++++++++++++++++++++++++++++++------------------------------
 1 files changed, 76 insertions(+), 70 deletions(-)
Michael S. Tsirkin - Nov. 3, 2009, 2:17 p.m.
On Fri, Oct 30, 2009 at 09:21:23PM +0900, Isaku Yamahata wrote:
> Remove one indentation of pci_update_mappings.
> Just for cosmetics, no logic change.

Good stuff. But since you are not afraid of churn for
cosmetics, let's fix a couple more nits?

> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
>  hw/pci.c |  146 ++++++++++++++++++++++++++++++++------------------------------
>  1 files changed, 76 insertions(+), 70 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index beefae3..b43f6c6 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -637,86 +637,92 @@ static void pci_update_mappings(PCIDevice *d)
>      cmd = pci_get_word(d->config + PCI_COMMAND);
>      for(i = 0; i < PCI_NUM_REGIONS; i++) {
>          r = &d->io_regions[i];
> -        if (r->size != 0) {
> -            if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> -                if (cmd & PCI_COMMAND_IO) {
> -                    new_addr = pci_get_long(d->config + pci_bar(d, i));
> -                    new_addr = new_addr & ~(r->size - 1);
> -                    last_addr = new_addr + r->size - 1;
> -                    /* NOTE: we have only 64K ioports on PC */
> -                    if (last_addr <= new_addr || new_addr == 0 ||
> -                        last_addr >= 0x10000) {
> -                        new_addr = PCI_BAR_UNMAPPED;
> -                    }
> -                } else {
> +
> +        /* this region isn't registered */
> +        if (r->size == 0)

Let's replace by !r->size?

> +            continue;
> +
> +        if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> +            if (cmd & PCI_COMMAND_IO) {
> +                new_addr = pci_get_long(d->config + pci_bar(d, i));
> +                new_addr = new_addr & ~(r->size - 1);
> +                last_addr = new_addr + r->size - 1;
> +                /* NOTE: we have only 64K ioports on PC */
> +                if (last_addr <= new_addr || new_addr == 0 ||
> +                    last_addr >= 0x10000) {
>                      new_addr = PCI_BAR_UNMAPPED;
>                  }
>              } else {
> -                if (cmd & PCI_COMMAND_MEMORY) {
> -                    if (r->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> -                        new_addr = pci_get_quad(d->config + pci_bar(d, i));
> -                    } else {
> -                        new_addr = pci_get_long(d->config + pci_bar(d, i));
> -                    }
> -                    /* the ROM slot has a specific enable bit */
> -                    if (i == PCI_ROM_SLOT && !(new_addr & PCI_ROM_ADDRESS_ENABLE))
> -                        goto no_mem_map;
> -                    new_addr = new_addr & ~(r->size - 1);
> -                    last_addr = new_addr + r->size - 1;
> -                    /* NOTE: we do not support wrapping */
> -                    /* XXX: as we cannot support really dynamic
> -                       mappings, we handle specific values as invalid
> -                       mappings. */
> -                    if (last_addr <= new_addr || new_addr == 0 ||
> -                        last_addr == PCI_BAR_UNMAPPED ||
> -
> -                        /* Now pcibus_t is 64bit.
> -                         * Check if 32 bit BAR wrap around explicitly.
> -                         * Without this, PC ide doesn't work well.
> -                         * TODO: remove this work around.
> -                         */
> -                        (!(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64) &&
> -                         last_addr >= UINT32_MAX) ||
> -
> -                        /*
> -                         * OS is allowed to set BAR beyond its addressable
> -                         * bits. For example, 32 bit OS can set 64bit bar
> -                         * to >4G. Check it.
> -                         */
> -                        last_addr >= TARGET_PHYS_ADDR_MAX) {
> -                        new_addr = PCI_BAR_UNMAPPED;
> -                    }
> +                new_addr = PCI_BAR_UNMAPPED;
> +            }
> +        } else {
> +            if (cmd & PCI_COMMAND_MEMORY) {
> +                if (r->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> +                    new_addr = pci_get_quad(d->config + pci_bar(d, i));
>                  } else {
> -                no_mem_map:
> +                    new_addr = pci_get_long(d->config + pci_bar(d, i));
> +                }
> +                /* the ROM slot has a specific enable bit */
> +                if (i == PCI_ROM_SLOT && !(new_addr & PCI_ROM_ADDRESS_ENABLE))
> +                    goto no_mem_map;
> +                new_addr = new_addr & ~(r->size - 1);
> +                last_addr = new_addr + r->size - 1;
> +                /* NOTE: we do not support wrapping */
> +                /* XXX: as we cannot support really dynamic
> +                   mappings, we handle specific values as invalid
> +                   mappings. */
> +                if (last_addr <= new_addr || new_addr == 0 ||
> +                    last_addr == PCI_BAR_UNMAPPED ||
> +
> +                    /* Now pcibus_t is 64bit.
> +                     * Check if 32 bit BAR wrap around explicitly.
> +                     * Without this, PC ide doesn't work well.
> +                     * TODO: remove this work around.
> +                     */
> +                    (!(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64) &&
> +                     last_addr >= UINT32_MAX) ||
> +
> +                    /*
> +                     * OS is allowed to set BAR beyond its addressable
> +                     * bits. For example, 32 bit OS can set 64bit bar
> +                     * to >4G. Check it.
> +                     */
> +                    last_addr >= TARGET_PHYS_ADDR_MAX) {
>                      new_addr = PCI_BAR_UNMAPPED;
>                  }
> +            } else {
> +            no_mem_map:
> +                new_addr = PCI_BAR_UNMAPPED;

Grr goto into scope ...
Can't we get rid of this as well?
Let's just move the whole if() statement to a subfunction,
then we can return PCI_BAR_UNMAPPED instead of goto.



>              }
> -            /* now do the real mapping */
> -            if (new_addr != r->addr) {
> -                if (r->addr != PCI_BAR_UNMAPPED) {
> -                    if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> -                        int class;
> -                        /* NOTE: specific hack for IDE in PC case:
> -                           only one byte must be mapped. */
> -                        class = pci_get_word(d->config + PCI_CLASS_DEVICE);
> -                        if (class == 0x0101 && r->size == 4) {
> -                            isa_unassign_ioport(r->addr + 2, 1);
> -                        } else {
> -                            isa_unassign_ioport(r->addr, r->size);
> -                        }
> -                    } else {
> -                        cpu_register_physical_memory(pci_to_cpu_addr(r->addr),
> -                                                     r->size,
> -                                                     IO_MEM_UNASSIGNED);
> -                        qemu_unregister_coalesced_mmio(r->addr, r->size);
> -                    }
> -                }
> -                r->addr = new_addr;
> -                if (r->addr != PCI_BAR_UNMAPPED) {
> -                    r->map_func(d, i, r->addr, r->size, r->type);
> +        }
> +
> +        /* This bar isn't changed */
> +        if (new_addr == r->addr)
> +            continue;
> +
> +        /* now do the real mapping */
> +        if (r->addr != PCI_BAR_UNMAPPED) {
> +            if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> +                int class;
> +                /* NOTE: specific hack for IDE in PC case:
> +                   only one byte must be mapped. */
> +                class = pci_get_word(d->config + PCI_CLASS_DEVICE);
> +                if (class == 0x0101 && r->size == 4) {
> +                    isa_unassign_ioport(r->addr + 2, 1);
> +                } else {
> +                    isa_unassign_ioport(r->addr, r->size);
>                  }
> +            } else {
> +                cpu_register_physical_memory(pci_to_cpu_addr(r->addr),
> +                                             r->size,
> +                                             IO_MEM_UNASSIGNED);
> +                qemu_unregister_coalesced_mmio(r->addr, r->size);
>              }
>          }
> +        r->addr = new_addr;
> +        if (r->addr != PCI_BAR_UNMAPPED) {
> +            r->map_func(d, i, r->addr, r->size, r->type);
> +        }
>      }
>  }
>  
> -- 
> 1.6.0.2

Patch

diff --git a/hw/pci.c b/hw/pci.c
index beefae3..b43f6c6 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -637,86 +637,92 @@  static void pci_update_mappings(PCIDevice *d)
     cmd = pci_get_word(d->config + PCI_COMMAND);
     for(i = 0; i < PCI_NUM_REGIONS; i++) {
         r = &d->io_regions[i];
-        if (r->size != 0) {
-            if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
-                if (cmd & PCI_COMMAND_IO) {
-                    new_addr = pci_get_long(d->config + pci_bar(d, i));
-                    new_addr = new_addr & ~(r->size - 1);
-                    last_addr = new_addr + r->size - 1;
-                    /* NOTE: we have only 64K ioports on PC */
-                    if (last_addr <= new_addr || new_addr == 0 ||
-                        last_addr >= 0x10000) {
-                        new_addr = PCI_BAR_UNMAPPED;
-                    }
-                } else {
+
+        /* this region isn't registered */
+        if (r->size == 0)
+            continue;
+
+        if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
+            if (cmd & PCI_COMMAND_IO) {
+                new_addr = pci_get_long(d->config + pci_bar(d, i));
+                new_addr = new_addr & ~(r->size - 1);
+                last_addr = new_addr + r->size - 1;
+                /* NOTE: we have only 64K ioports on PC */
+                if (last_addr <= new_addr || new_addr == 0 ||
+                    last_addr >= 0x10000) {
                     new_addr = PCI_BAR_UNMAPPED;
                 }
             } else {
-                if (cmd & PCI_COMMAND_MEMORY) {
-                    if (r->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
-                        new_addr = pci_get_quad(d->config + pci_bar(d, i));
-                    } else {
-                        new_addr = pci_get_long(d->config + pci_bar(d, i));
-                    }
-                    /* the ROM slot has a specific enable bit */
-                    if (i == PCI_ROM_SLOT && !(new_addr & PCI_ROM_ADDRESS_ENABLE))
-                        goto no_mem_map;
-                    new_addr = new_addr & ~(r->size - 1);
-                    last_addr = new_addr + r->size - 1;
-                    /* NOTE: we do not support wrapping */
-                    /* XXX: as we cannot support really dynamic
-                       mappings, we handle specific values as invalid
-                       mappings. */
-                    if (last_addr <= new_addr || new_addr == 0 ||
-                        last_addr == PCI_BAR_UNMAPPED ||
-
-                        /* Now pcibus_t is 64bit.
-                         * Check if 32 bit BAR wrap around explicitly.
-                         * Without this, PC ide doesn't work well.
-                         * TODO: remove this work around.
-                         */
-                        (!(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64) &&
-                         last_addr >= UINT32_MAX) ||
-
-                        /*
-                         * OS is allowed to set BAR beyond its addressable
-                         * bits. For example, 32 bit OS can set 64bit bar
-                         * to >4G. Check it.
-                         */
-                        last_addr >= TARGET_PHYS_ADDR_MAX) {
-                        new_addr = PCI_BAR_UNMAPPED;
-                    }
+                new_addr = PCI_BAR_UNMAPPED;
+            }
+        } else {
+            if (cmd & PCI_COMMAND_MEMORY) {
+                if (r->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+                    new_addr = pci_get_quad(d->config + pci_bar(d, i));
                 } else {
-                no_mem_map:
+                    new_addr = pci_get_long(d->config + pci_bar(d, i));
+                }
+                /* the ROM slot has a specific enable bit */
+                if (i == PCI_ROM_SLOT && !(new_addr & PCI_ROM_ADDRESS_ENABLE))
+                    goto no_mem_map;
+                new_addr = new_addr & ~(r->size - 1);
+                last_addr = new_addr + r->size - 1;
+                /* NOTE: we do not support wrapping */
+                /* XXX: as we cannot support really dynamic
+                   mappings, we handle specific values as invalid
+                   mappings. */
+                if (last_addr <= new_addr || new_addr == 0 ||
+                    last_addr == PCI_BAR_UNMAPPED ||
+
+                    /* Now pcibus_t is 64bit.
+                     * Check if 32 bit BAR wrap around explicitly.
+                     * Without this, PC ide doesn't work well.
+                     * TODO: remove this work around.
+                     */
+                    (!(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64) &&
+                     last_addr >= UINT32_MAX) ||
+
+                    /*
+                     * OS is allowed to set BAR beyond its addressable
+                     * bits. For example, 32 bit OS can set 64bit bar
+                     * to >4G. Check it.
+                     */
+                    last_addr >= TARGET_PHYS_ADDR_MAX) {
                     new_addr = PCI_BAR_UNMAPPED;
                 }
+            } else {
+            no_mem_map:
+                new_addr = PCI_BAR_UNMAPPED;
             }
-            /* now do the real mapping */
-            if (new_addr != r->addr) {
-                if (r->addr != PCI_BAR_UNMAPPED) {
-                    if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
-                        int class;
-                        /* NOTE: specific hack for IDE in PC case:
-                           only one byte must be mapped. */
-                        class = pci_get_word(d->config + PCI_CLASS_DEVICE);
-                        if (class == 0x0101 && r->size == 4) {
-                            isa_unassign_ioport(r->addr + 2, 1);
-                        } else {
-                            isa_unassign_ioport(r->addr, r->size);
-                        }
-                    } else {
-                        cpu_register_physical_memory(pci_to_cpu_addr(r->addr),
-                                                     r->size,
-                                                     IO_MEM_UNASSIGNED);
-                        qemu_unregister_coalesced_mmio(r->addr, r->size);
-                    }
-                }
-                r->addr = new_addr;
-                if (r->addr != PCI_BAR_UNMAPPED) {
-                    r->map_func(d, i, r->addr, r->size, r->type);
+        }
+
+        /* This bar isn't changed */
+        if (new_addr == r->addr)
+            continue;
+
+        /* now do the real mapping */
+        if (r->addr != PCI_BAR_UNMAPPED) {
+            if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
+                int class;
+                /* NOTE: specific hack for IDE in PC case:
+                   only one byte must be mapped. */
+                class = pci_get_word(d->config + PCI_CLASS_DEVICE);
+                if (class == 0x0101 && r->size == 4) {
+                    isa_unassign_ioport(r->addr + 2, 1);
+                } else {
+                    isa_unassign_ioport(r->addr, r->size);
                 }
+            } else {
+                cpu_register_physical_memory(pci_to_cpu_addr(r->addr),
+                                             r->size,
+                                             IO_MEM_UNASSIGNED);
+                qemu_unregister_coalesced_mmio(r->addr, r->size);
             }
         }
+        r->addr = new_addr;
+        if (r->addr != PCI_BAR_UNMAPPED) {
+            r->map_func(d, i, r->addr, r->size, r->type);
+        }
     }
 }