[PATCH v3 06/14] hw/pci-host/raven: Simplify direct config access address decoding

BALATON Zoltan posted 14 patches 1 week, 1 day ago
Maintainers: "Hervé Poussineau" <hpoussin@reactos.org>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
[PATCH v3 06/14] hw/pci-host/raven: Simplify direct config access address decoding
Posted by BALATON Zoltan 1 week, 1 day ago
Use ctz instead of an open coded version and rename function to better
show what it does.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/pci-host/raven.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index a400a22df3..66dab28a29 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -57,16 +57,9 @@ struct PREPPCIState {
 
 #define PCI_IO_BASE_ADDR    0x80000000  /* Physical address on main bus */
 
-static inline uint32_t raven_pci_io_config(hwaddr addr)
+static inline uint32_t raven_idsel_to_addr(hwaddr addr)
 {
-    int i;
-
-    for (i = 0; i < 11; i++) {
-        if ((addr & (1 << (11 + i))) != 0) {
-            break;
-        }
-    }
-    return (addr & 0x7ff) |  (i << 11);
+    return (ctz16(addr >> 11) << 11) | (addr & 0x7ff);
 }
 
 static void raven_pci_io_write(void *opaque, hwaddr addr,
@@ -74,7 +67,7 @@ static void raven_pci_io_write(void *opaque, hwaddr addr,
 {
     PREPPCIState *s = opaque;
     PCIHostState *phb = PCI_HOST_BRIDGE(s);
-    pci_data_write(phb->bus, raven_pci_io_config(addr), val, size);
+    pci_data_write(phb->bus, raven_idsel_to_addr(addr), val, size);
 }
 
 static uint64_t raven_pci_io_read(void *opaque, hwaddr addr,
@@ -82,7 +75,7 @@ static uint64_t raven_pci_io_read(void *opaque, hwaddr addr,
 {
     PREPPCIState *s = opaque;
     PCIHostState *phb = PCI_HOST_BRIDGE(s);
-    return pci_data_read(phb->bus, raven_pci_io_config(addr), size);
+    return pci_data_read(phb->bus, raven_idsel_to_addr(addr), size);
 }
 
 static const MemoryRegionOps raven_pci_io_ops = {
-- 
2.41.3
Re: [PATCH v3 06/14] hw/pci-host/raven: Simplify direct config access address decoding
Posted by Mark Cave-Ayland 1 week ago
On 18/09/2025 19:50, BALATON Zoltan wrote:

> Use ctz instead of an open coded version and rename function to better
> show what it does.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/pci-host/raven.c | 15 ++++-----------
>   1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
> index a400a22df3..66dab28a29 100644
> --- a/hw/pci-host/raven.c
> +++ b/hw/pci-host/raven.c
> @@ -57,16 +57,9 @@ struct PREPPCIState {
>   
>   #define PCI_IO_BASE_ADDR    0x80000000  /* Physical address on main bus */
>   
> -static inline uint32_t raven_pci_io_config(hwaddr addr)
> +static inline uint32_t raven_idsel_to_addr(hwaddr addr)
>   {
> -    int i;
> -
> -    for (i = 0; i < 11; i++) {
> -        if ((addr & (1 << (11 + i))) != 0) {
> -            break;
> -        }
> -    }
> -    return (addr & 0x7ff) |  (i << 11);
> +    return (ctz16(addr >> 11) << 11) | (addr & 0x7ff);
>   }

This looks like a good rework of the logic.

>   static void raven_pci_io_write(void *opaque, hwaddr addr,
> @@ -74,7 +67,7 @@ static void raven_pci_io_write(void *opaque, hwaddr addr,
>   {
>       PREPPCIState *s = opaque;
>       PCIHostState *phb = PCI_HOST_BRIDGE(s);
> -    pci_data_write(phb->bus, raven_pci_io_config(addr), val, size);
> +    pci_data_write(phb->bus, raven_idsel_to_addr(addr), val, size);
>   }
>   
>   static uint64_t raven_pci_io_read(void *opaque, hwaddr addr,
> @@ -82,7 +75,7 @@ static uint64_t raven_pci_io_read(void *opaque, hwaddr addr,
>   {
>       PREPPCIState *s = opaque;
>       PCIHostState *phb = PCI_HOST_BRIDGE(s);
> -    return pci_data_read(phb->bus, raven_pci_io_config(addr), size);
> +    return pci_data_read(phb->bus, raven_idsel_to_addr(addr), size);
>   }
>   
>   static const MemoryRegionOps raven_pci_io_ops = {

It's a bit confusing during review that pci_data_read()/pci_data_write() access 
config space, but that's unrelated to this patch.

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.