Re: [PATCH v4 2/4] vpci: allow queueing of mapping operations

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.04.2026 21:11, Stewart Hildebrand wrote:
> Introduce vPCI BAR mapping task queue. Store information needed to
> map/unmap BARs in struct vpci_map_task. Allow queueing of BAR map/unmap
> operations in a list, thus making it possible to perform multiple p2m
> operations associated with single PCI device.
> 
> This is preparatory work for further changes that need to perform
> multiple unmap/map operations before returning to guest.
> 
> At the moment, only a single operation will be queued. However, when
> multiple operations are queued, there is a check in modify_bars() to
> skip BARs already in the requested state that will no longer be
> accurate. Remove this check in preparation of upcoming changes.
> 
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
> ---
> apply_map() and vpci_process_map_task() are very similar. Should we try
> to combine them into a single function?
> 
> I concede that the dynamic allocation/deallocation of struct
> vpci_map_task is not ideal. However, to support SR-IOV, there will be a
> need to queue many mapping operations (one per VF), and statically
> pre-allocating that much would seem wasteful. Only the hardware and/or
> control domain would need to queue many operations, and only when
> configuring SR-IOV.

The address ranges are pretty regular though across the VFs, so I wonder
whether getting away with less than one instance per VF (perhaps one per
PF) wouldn't be possible by adjusting what data is being put into the
"task" structure.

> --- a/xen/include/xen/vpci.h
> +++ b/xen/include/xen/vpci.h
> @@ -19,8 +19,7 @@
>   */
>  #define VPCI_MAX_VIRT_DEV       (PCI_SLOT(~0) + 1)
>  
> -void vpci_vcpu_destroy(struct vcpu *v);
> -int vpci_vcpu_init(struct vcpu *v);
> +void vpci_vcpu_init(struct vcpu *v);
>  
>  /* Assign vPCI to device by adding handlers. */
>  int __must_check vpci_assign_device(struct pci_dev *pdev);
> @@ -155,14 +154,23 @@ struct vpci {
>  };
>  
>  #ifdef __XEN__
> -struct vpci_vcpu {
> +struct vpci_map_task {
>      /* Per-vcpu structure to store state while {un}mapping of PCI BARs. */

This comment needs adjusting then, too, doesn't it?

Jan