Re: [PATCH-4.16 v2] xen/arm: fix SBDF calculation for vPCI MMIO handlers

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 2 Nov 2021, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> 
> While in vPCI MMIO trap handlers for the guest PCI host bridge it is not
> enough for SBDF translation to simply call VPCI_ECAM_BDF(info->gpa) as
> the base address may not be aligned in the way that the translation
> always work. If not adjusted with respect to the base address it may not be
> able to properly convert SBDF.
> Fix this by adjusting the gpa with respect to the host bridge base address
> in a way as it is done for x86.
> 
> Please note, that this change is not strictly required given the current
> value of GUEST_VPCI_ECAM_BASE which has bits 0 to 27 clear, but could cause
> issues if such value is changed, or when handlers for dom0 ECAM
> regions are added as those will be mapped over existing hardware
> regions that could use non-aligned base addresses.
> 
> Fixes: d59168dc05a5 ("xen/arm: Enable the existing x86 virtual PCI support 
> for ARM")
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

Also, Ian already gave his release-ack.


> ---
> Since v1:
>  - updated commit message (Roger)
> 
> This patch aims for 4.16 release.
> Benefits:
> Fix potential bug and clear the way for further PCI passthrough
> development.
> Risks:
> None as the change doesn't change the behaviour of the current code,
> but brings clarity into SBDF calculation.
> ---
>  xen/arch/arm/vpci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
> index 8f40a0dec6d2..23f45386f4b3 100644
> --- a/xen/arch/arm/vpci.c
> +++ b/xen/arch/arm/vpci.c
> @@ -24,7 +24,7 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
>      unsigned long data;
>  
>      /* We ignore segment part and always handle segment 0 */
> -    sbdf.sbdf = VPCI_ECAM_BDF(info->gpa);
> +    sbdf.sbdf = VPCI_ECAM_BDF(info->gpa - GUEST_VPCI_ECAM_BASE);
>  
>      if ( vpci_ecam_read(sbdf, ECAM_REG_OFFSET(info->gpa),
>                          1U << info->dabt.size, &data) )
> @@ -44,7 +44,7 @@ static int vpci_mmio_write(struct vcpu *v, mmio_info_t 
> *info,
>      pci_sbdf_t sbdf;
>  
>      /* We ignore segment part and always handle segment 0 */
> -    sbdf.sbdf = VPCI_ECAM_BDF(info->gpa);
> +    sbdf.sbdf = VPCI_ECAM_BDF(info->gpa - GUEST_VPCI_ECAM_BASE);
>  
>      return vpci_ecam_write(sbdf, ECAM_REG_OFFSET(info->gpa),
>                             1U << info->dabt.size, r);
> -- 
> 2.25.1
>