1. Patchew
  2. Xen
  3. [PATCH v3 00/11] PCI devices passthrough on Arm, part 3
  4. View patch

[PATCH v3 01/11] vpci: Make vpci registers removal a dedicated function

Oleksandr Andrushchenko posted 11 patches 4 years, 4 months ago
There is a newer version of this series
[PATCH v3 01/11] vpci: Make vpci registers removal a dedicated function
Posted by Oleksandr Andrushchenko 4 years, 4 months ago 
From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

This is in preparation for dynamic assignment of the vpci register
handlers depending on the domain: hwdom or guest.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Reviewed-by: Michal Orzel <michal.orzel@arm.com>
---
Since v1:
 - constify struct pci_dev where possible
---
 xen/drivers/vpci/vpci.c | 7 ++++++-
 xen/include/xen/vpci.h  | 2 ++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index cbd1bac7fc33..1666402d55b8 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -35,7 +35,7 @@ extern vpci_register_init_t *const __start_vpci_array[];
 extern vpci_register_init_t *const __end_vpci_array[];
 #define NUM_VPCI_INIT (__end_vpci_array - __start_vpci_array)
 
-void vpci_remove_device(struct pci_dev *pdev)
+void vpci_remove_device_registers(const struct pci_dev *pdev)
 {
     spin_lock(&pdev->vpci->lock);
     while ( !list_empty(&pdev->vpci->handlers) )
@@ -48,6 +48,11 @@ void vpci_remove_device(struct pci_dev *pdev)
         xfree(r);
     }
     spin_unlock(&pdev->vpci->lock);
+}
+
+void vpci_remove_device(struct pci_dev *pdev)
+{
+    vpci_remove_device_registers(pdev);
     xfree(pdev->vpci->msix);
     xfree(pdev->vpci->msi);
     xfree(pdev->vpci);
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index 9f5b5d52e159..2e910d0b1f90 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -28,6 +28,8 @@ int __must_check vpci_add_handlers(struct pci_dev *dev);
 
 /* Remove all handlers and free vpci related structures. */
 void vpci_remove_device(struct pci_dev *pdev);
+/* Remove all handlers for the device given. */
+void vpci_remove_device_registers(const struct pci_dev *pdev);
 
 /* Add/remove a register handler. */
 int __must_check vpci_add_register(struct vpci *vpci,
-- 
2.25.1
Re: [PATCH v3 01/11] vpci: Make vpci registers removal a dedicated function
Posted by Roger Pau Monné 4 years, 3 months ago 
On Thu, Sep 30, 2021 at 10:52:13AM +0300, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> This is in preparation for dynamic assignment of the vpci register
> handlers depending on the domain: hwdom or guest.
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Reviewed-by: Michal Orzel <michal.orzel@arm.com>
> ---
> Since v1:
>  - constify struct pci_dev where possible
> ---
>  xen/drivers/vpci/vpci.c | 7 ++++++-
>  xen/include/xen/vpci.h  | 2 ++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> index cbd1bac7fc33..1666402d55b8 100644
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -35,7 +35,7 @@ extern vpci_register_init_t *const __start_vpci_array[];
>  extern vpci_register_init_t *const __end_vpci_array[];
>  #define NUM_VPCI_INIT (__end_vpci_array - __start_vpci_array)
>  
> -void vpci_remove_device(struct pci_dev *pdev)
> +void vpci_remove_device_registers(const struct pci_dev *pdev)

Making this const is kind of misleading, as you end up modifying
contents of the pdev, is just that vpci data is stored as a pointer
inside the struct so you avoid the effects of the constification.

>  {
>      spin_lock(&pdev->vpci->lock);
>      while ( !list_empty(&pdev->vpci->handlers) )
> @@ -48,6 +48,11 @@ void vpci_remove_device(struct pci_dev *pdev)
>          xfree(r);
>      }
>      spin_unlock(&pdev->vpci->lock);
> +}
> +
> +void vpci_remove_device(struct pci_dev *pdev)
> +{
> +    vpci_remove_device_registers(pdev);
>      xfree(pdev->vpci->msix);
>      xfree(pdev->vpci->msi);
>      xfree(pdev->vpci);
> diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
> index 9f5b5d52e159..2e910d0b1f90 100644
> --- a/xen/include/xen/vpci.h
> +++ b/xen/include/xen/vpci.h
> @@ -28,6 +28,8 @@ int __must_check vpci_add_handlers(struct pci_dev *dev);
>  
>  /* Remove all handlers and free vpci related structures. */
>  void vpci_remove_device(struct pci_dev *pdev);
> +/* Remove all handlers for the device given. */

I would drop the 'given' form the end of the sentence...

> +void vpci_remove_device_registers(const struct pci_dev *pdev);

...and maybe name this vpci_remove_device_handlers as it's clearer
IMO.

Thanks, Roger.
Re: [PATCH v3 01/11] vpci: Make vpci registers removal a dedicated function
Posted by Oleksandr Andrushchenko 4 years, 3 months ago 
Hi, Roger!

On 13.10.21 14:11, Roger Pau Monné wrote:
> On Thu, Sep 30, 2021 at 10:52:13AM +0300, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>> This is in preparation for dynamic assignment of the vpci register
>> handlers depending on the domain: hwdom or guest.
>>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> Reviewed-by: Michal Orzel <michal.orzel@arm.com>
>> ---
>> Since v1:
>>   - constify struct pci_dev where possible
>> ---
>>   xen/drivers/vpci/vpci.c | 7 ++++++-
>>   xen/include/xen/vpci.h  | 2 ++
>>   2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
>> index cbd1bac7fc33..1666402d55b8 100644
>> --- a/xen/drivers/vpci/vpci.c
>> +++ b/xen/drivers/vpci/vpci.c
>> @@ -35,7 +35,7 @@ extern vpci_register_init_t *const __start_vpci_array[];
>>   extern vpci_register_init_t *const __end_vpci_array[];
>>   #define NUM_VPCI_INIT (__end_vpci_array - __start_vpci_array)
>>   
>> -void vpci_remove_device(struct pci_dev *pdev)
>> +void vpci_remove_device_registers(const struct pci_dev *pdev)
> Making this const is kind of misleading, as you end up modifying
> contents of the pdev, is just that vpci data is stored as a pointer
> inside the struct so you avoid the effects of the constification.
Ok, I will remove const
>
>>   {
>>       spin_lock(&pdev->vpci->lock);
>>       while ( !list_empty(&pdev->vpci->handlers) )
>> @@ -48,6 +48,11 @@ void vpci_remove_device(struct pci_dev *pdev)
>>           xfree(r);
>>       }
>>       spin_unlock(&pdev->vpci->lock);
>> +}
>> +
>> +void vpci_remove_device(struct pci_dev *pdev)
>> +{
>> +    vpci_remove_device_registers(pdev);
>>       xfree(pdev->vpci->msix);
>>       xfree(pdev->vpci->msi);
>>       xfree(pdev->vpci);
>> diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
>> index 9f5b5d52e159..2e910d0b1f90 100644
>> --- a/xen/include/xen/vpci.h
>> +++ b/xen/include/xen/vpci.h
>> @@ -28,6 +28,8 @@ int __must_check vpci_add_handlers(struct pci_dev *dev);
>>   
>>   /* Remove all handlers and free vpci related structures. */
>>   void vpci_remove_device(struct pci_dev *pdev);
>> +/* Remove all handlers for the device given. */
> I would drop the 'given' form the end of the sentence...
Sure
>
>> +void vpci_remove_device_registers(const struct pci_dev *pdev);
> ...and maybe name this vpci_remove_device_handlers as it's clearer
> IMO.
Ok, will rename
>
> Thanks, Roger.
Thank you,
Oleksandr
Re: [PATCH v3 01/11] vpci: Make vpci registers removal a dedicated function
Posted by Roger Pau Monné 4 years, 3 months ago 
On Wed, Oct 27, 2021 at 09:12:14AM +0000, Oleksandr Andrushchenko wrote:
> Hi, Roger!
> 
> On 13.10.21 14:11, Roger Pau Monné wrote:
> > On Thu, Sep 30, 2021 at 10:52:13AM +0300, Oleksandr Andrushchenko wrote:
> >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> >>
> >> This is in preparation for dynamic assignment of the vpci register
> >> handlers depending on the domain: hwdom or guest.
> >>
> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> >> Reviewed-by: Michal Orzel <michal.orzel@arm.com>
> >> ---
> >> Since v1:
> >>   - constify struct pci_dev where possible
> >> ---
> >>   xen/drivers/vpci/vpci.c | 7 ++++++-
> >>   xen/include/xen/vpci.h  | 2 ++
> >>   2 files changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> >> index cbd1bac7fc33..1666402d55b8 100644
> >> --- a/xen/drivers/vpci/vpci.c
> >> +++ b/xen/drivers/vpci/vpci.c
> >> @@ -35,7 +35,7 @@ extern vpci_register_init_t *const __start_vpci_array[];
> >>   extern vpci_register_init_t *const __end_vpci_array[];
> >>   #define NUM_VPCI_INIT (__end_vpci_array - __start_vpci_array)
> >>   
> >> -void vpci_remove_device(struct pci_dev *pdev)
> >> +void vpci_remove_device_registers(const struct pci_dev *pdev)
> > Making this const is kind of misleading, as you end up modifying
> > contents of the pdev, is just that vpci data is stored as a pointer
> > inside the struct so you avoid the effects of the constification.
> Ok, I will remove const

Jan prefers the const, so please leave it.

Thanks, Roger.
Re: [PATCH v3 01/11] vpci: Make vpci registers removal a dedicated function
Posted by Oleksandr Andrushchenko 4 years, 3 months ago 

On 27.10.21 12:24, Roger Pau Monné wrote:
> On Wed, Oct 27, 2021 at 09:12:14AM +0000, Oleksandr Andrushchenko wrote:
>> Hi, Roger!
>>
>> On 13.10.21 14:11, Roger Pau Monné wrote:
>>> On Thu, Sep 30, 2021 at 10:52:13AM +0300, Oleksandr Andrushchenko wrote:
>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>
>>>> This is in preparation for dynamic assignment of the vpci register
>>>> handlers depending on the domain: hwdom or guest.
>>>>
>>>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>> Reviewed-by: Michal Orzel <michal.orzel@arm.com>
>>>> ---
>>>> Since v1:
>>>>    - constify struct pci_dev where possible
>>>> ---
>>>>    xen/drivers/vpci/vpci.c | 7 ++++++-
>>>>    xen/include/xen/vpci.h  | 2 ++
>>>>    2 files changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
>>>> index cbd1bac7fc33..1666402d55b8 100644
>>>> --- a/xen/drivers/vpci/vpci.c
>>>> +++ b/xen/drivers/vpci/vpci.c
>>>> @@ -35,7 +35,7 @@ extern vpci_register_init_t *const __start_vpci_array[];
>>>>    extern vpci_register_init_t *const __end_vpci_array[];
>>>>    #define NUM_VPCI_INIT (__end_vpci_array - __start_vpci_array)
>>>>    
>>>> -void vpci_remove_device(struct pci_dev *pdev)
>>>> +void vpci_remove_device_registers(const struct pci_dev *pdev)
>>> Making this const is kind of misleading, as you end up modifying
>>> contents of the pdev, is just that vpci data is stored as a pointer
>>> inside the struct so you avoid the effects of the constification.
>> Ok, I will remove const
> Jan prefers the const, so please leave it.
Ooook )
>
> Thanks, Roger.
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.