Re: [PATCH v1 kvmtool 4/7] vfio/pci: Rename PBA offset in device descriptor to fd_offset

[Andre Przywara <andre.przywara@arm.com>]

On Mon, 13 Sep 2021 16:44:10 +0100
Alexandru Elisei <alexandru.elisei@arm.com> wrote:

> The MSI-X capability defines a PBA offset, which is the offset of the PBA
> array in the BAR that holds the array.
> 
> kvmtool uses the field "pba_offset" in struct msix_cap (which represents
> the MSIX capability) to refer to the [PBA offset:BAR] field of the
> capability; and the field "offset" in the struct vfio_pci_msix_pba to refer
> to offset of the PBA array in the device descriptor created by the VFIO
> driver.
> 
> As we're getting ready to add yet another field that represents an offset
> to struct vfio_pci_msix_pba, try to avoid ambiguities by renaming the
> struct's "offset" field to "fd_offset".
> 
> No functional change intended.

Makes sense, too many offsets.

> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  include/kvm/vfio.h | 2 +-
>  vfio/pci.c         | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/kvm/vfio.h b/include/kvm/vfio.h
> index 28223cf..8cdf04f 100644
> --- a/include/kvm/vfio.h
> +++ b/include/kvm/vfio.h
> @@ -48,7 +48,7 @@ struct vfio_pci_msix_table {
>  
>  struct vfio_pci_msix_pba {
>  	size_t				size;
> -	off_t				offset; /* in VFIO device fd */
> +	off_t				fd_offset; /* in VFIO device fd */
>  	unsigned int			bar;
>  	u32				guest_phys_addr;
>  };
> diff --git a/vfio/pci.c b/vfio/pci.c
> index 10ff99e..cc18311 100644
> --- a/vfio/pci.c
> +++ b/vfio/pci.c
> @@ -256,7 +256,7 @@ static void vfio_pci_msix_pba_access(struct kvm_cpu *vcpu, u64 addr, u8 *data,
>  	 * TODO: emulate PBA. Hardware MSI-X is never masked, so reading the PBA
>  	 * is completely useless here. Note that Linux doesn't use PBA.
>  	 */
> -	if (pread(vdev->fd, data, len, pba->offset + offset) != (ssize_t)len)
> +	if (pread(vdev->fd, data, len, pba->fd_offset + offset) != (ssize_t)len)
>  		vfio_dev_err(vdev, "cannot access MSIX PBA\n");
>  }
>  
> @@ -815,8 +815,8 @@ static int vfio_pci_fixup_cfg_space(struct vfio_device *vdev)
>  	if (msix) {
>  		/* Add a shortcut to the PBA region for the MMIO handler */
>  		int pba_index = VFIO_PCI_BAR0_REGION_INDEX + pdev->msix_pba.bar;
> -		pdev->msix_pba.offset = vdev->regions[pba_index].info.offset +
> -					(msix->pba_offset & PCI_MSIX_PBA_OFFSET);
> +		pdev->msix_pba.fd_offset = vdev->regions[pba_index].info.offset +
> +					   (msix->pba_offset & PCI_MSIX_PBA_OFFSET);
>  
>  		/* Tidy up the capability */
>  		msix->table_offset &= PCI_MSIX_TABLE_BIR;