On Thu, Sep 30, 2021 at 10:52:13AM +0300, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <[email protected]>
>
> This is in preparation for dynamic assignment of the vpci register
> handlers depending on the domain: hwdom or guest.
>
> Signed-off-by: Oleksandr Andrushchenko <[email protected]>
> Reviewed-by: Michal Orzel <[email protected]>
> ---
> 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.
