On 15.04.2025 18:54, Stewart Hildebrand wrote:
> In preparation for translating virtual PCI bus topology (where a pdev
> lookup will be needed in the vPCI I/O handlers), acquire d->pci_lock in
> the vPCI I/O handlers.
I find it concerning that the locked regions (it's a domain-wide lock
after all) are further grown.
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -440,6 +440,8 @@ uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg,
> unsigned int size)
> unsigned int data_offset = 0;
> uint32_t data = ~(uint32_t)0;
>
> + ASSERT(rw_is_locked(&d->pci_lock));
> +
> if ( !size )
> {
> ASSERT_UNREACHABLE();
> @@ -452,15 +454,11 @@ uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg,
> unsigned int size)
> * If this is hwdom and the device is assigned to DomXEN, acquiring
> hwdom's
> * pci_lock is sufficient.
> */
> - read_lock(&d->pci_lock);
The comment now becomes meaningless in its context.
> @@ -570,7 +569,6 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg,
> unsigned int size,
> * TODO: We need to take pci_locks in exclusive mode only if we
> * are modifying BARs, so there is a room for improvement.
> */
> - write_lock(&d->pci_lock);
Same here.
Jan
