Move some logic to a new function to enable code reuse.

Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
 xen/drivers/vpci/header.c | 56 ++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index c1463d2ce076..b09ccc5e6be6 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -173,11 +173,38 @@ static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
         ASSERT_UNREACHABLE();
 }
 
+static int map_bars(struct vpci_header *header, struct domain *d, bool map)
+{
+    unsigned int i;
+
+    for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
+    {
+        struct vpci_bar *bar = &header->bars[i];
+        struct map_data data = {
+            .d = d,
+            .map = map,
+            .bar = bar,
+        };
+        int rc;
+
+        if ( rangeset_is_empty(bar->mem) )
+            continue;
+
+        rc = rangeset_consume_ranges(bar->mem, map_range, &data);
+
+        if ( rc )
+            return rc;
+    }
+
+    return 0;
+}
+
 bool vpci_process_pending(struct vcpu *v)
 {
     const struct pci_dev *pdev = v->vpci.pdev;
     struct vpci_header *header = NULL;
     unsigned int i;
+    int rc;
 
     if ( !pdev )
         return false;
@@ -192,30 +219,17 @@ bool vpci_process_pending(struct vcpu *v)
     }
 
     header = &pdev->vpci->header;
-    for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
-    {
-        struct vpci_bar *bar = &header->bars[i];
-        struct map_data data = {
-            .d = v->domain,
-            .map = v->vpci.cmd & PCI_COMMAND_MEMORY,
-            .bar = bar,
-        };
-        int rc;
-
-        if ( rangeset_is_empty(bar->mem) )
-            continue;
+    rc = map_bars(header, v->domain, v->vpci.cmd & PCI_COMMAND_MEMORY);
 
-        rc = rangeset_consume_ranges(bar->mem, map_range, &data);
+    if ( rc == -ERESTART )
+    {
+        read_unlock(&v->domain->pci_lock);
+        return true;
+    }
 
-        if ( rc == -ERESTART )
-        {
-            read_unlock(&v->domain->pci_lock);
-            return true;
-        }
+    if ( rc )
+        goto fail;
 
-        if ( rc )
-            goto fail;
-    }
     v->vpci.pdev = NULL;
 
     spin_lock(&pdev->vpci->lock);
-- 
2.49.0

On Sat, May 31, 2025 at 08:54:01AM -0400, Stewart Hildebrand wrote:
> Move some logic to a new function to enable code reuse.

Like with the previous changes, it's helpful if you explicitly note
that no functional change is intended in the commit message (which I
think it's the case here).

> 
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> ---
>  xen/drivers/vpci/header.c | 56 ++++++++++++++++++++++++---------------
>  1 file changed, 35 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index c1463d2ce076..b09ccc5e6be6 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -173,11 +173,38 @@ static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
>          ASSERT_UNREACHABLE();
>  }
>  
> +static int map_bars(struct vpci_header *header, struct domain *d, bool map)
> +{
> +    unsigned int i;
> +
> +    for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
> +    {
> +        struct vpci_bar *bar = &header->bars[i];
> +        struct map_data data = {
> +            .d = d,
> +            .map = map,
> +            .bar = bar,
> +        };
> +        int rc;
> +
> +        if ( rangeset_is_empty(bar->mem) )
> +            continue;
> +
> +        rc = rangeset_consume_ranges(bar->mem, map_range, &data);
> +
> +        if ( rc )
> +            return rc;
> +    }
> +
> +    return 0;
> +}
> +
>  bool vpci_process_pending(struct vcpu *v)
>  {
>      const struct pci_dev *pdev = v->vpci.pdev;
>      struct vpci_header *header = NULL;
>      unsigned int i;

Maybe I'm missing something, but don't you get complains from the
compiler here about i being unused after this change?

Thanks, Roger.

On 6/5/25 06:16, Roger Pau Monné wrote:
> On Sat, May 31, 2025 at 08:54:01AM -0400, Stewart Hildebrand wrote:
>> Move some logic to a new function to enable code reuse.
> 
> Like with the previous changes, it's helpful if you explicitly note
> that no functional change is intended in the commit message (which I
> think it's the case here).

OK, will do.

>>
>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>> ---
>>  xen/drivers/vpci/header.c | 56 ++++++++++++++++++++++++---------------
>>  1 file changed, 35 insertions(+), 21 deletions(-)
>>
>> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
>> index c1463d2ce076..b09ccc5e6be6 100644
>> --- a/xen/drivers/vpci/header.c
>> +++ b/xen/drivers/vpci/header.c
>> @@ -173,11 +173,38 @@ static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
>>          ASSERT_UNREACHABLE();
>>  }
>>  
>> +static int map_bars(struct vpci_header *header, struct domain *d, bool map)
>> +{
>> +    unsigned int i;
>> +
>> +    for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
>> +    {
>> +        struct vpci_bar *bar = &header->bars[i];
>> +        struct map_data data = {
>> +            .d = d,
>> +            .map = map,
>> +            .bar = bar,
>> +        };
>> +        int rc;
>> +
>> +        if ( rangeset_is_empty(bar->mem) )
>> +            continue;
>> +
>> +        rc = rangeset_consume_ranges(bar->mem, map_range, &data);
>> +
>> +        if ( rc )
>> +            return rc;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>  bool vpci_process_pending(struct vcpu *v)
>>  {
>>      const struct pci_dev *pdev = v->vpci.pdev;
>>      struct vpci_header *header = NULL;
>>      unsigned int i;
> 
> Maybe I'm missing something, but don't you get complains from the
> compiler here about i being unused after this change?

No, i is still used in the error path.