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

Oleksandr Andrushchenko posted 1 patch 2 years, 4 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20211102112041.551369-1-andr2000@gmail.com
xen/arch/arm/vpci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH-4.16 v2] xen/arm: fix SBDF calculation for vPCI MMIO handlers
Posted by Oleksandr Andrushchenko 2 years, 4 months ago
From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

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@epam.com>

---
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


Re: [PATCH-4.16 v2] xen/arm: fix SBDF calculation for vPCI MMIO handlers
Posted by Stefano Stabellini 2 years, 4 months ago
On Tue, 2 Nov 2021, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> 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@epam.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

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
> 

Re: [PATCH-4.16 v2] xen/arm: fix SBDF calculation for vPCI MMIO handlers
Posted by Oleksandr Andrushchenko 2 years, 4 months ago

On 03.11.21 00:47, Stefano Stabellini wrote:
> On Tue, 2 Nov 2021, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>> 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@epam.com>
> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
>
> Also, Ian already gave his release-ack.
Thank you,
Do I need to resend the patch with Acks? Or hopefully those can be applied
on commit?
>
>
>> ---
>> 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
>>
Re: [PATCH-4.16 v2] xen/arm: fix SBDF calculation for vPCI MMIO handlers
Posted by Julien Grall 2 years, 4 months ago
Hi Oleksandr,

On 03/11/2021 12:08, Oleksandr Andrushchenko wrote:
> 
> 
> On 03.11.21 00:47, Stefano Stabellini wrote:
>> On Tue, 2 Nov 2021, Oleksandr Andrushchenko wrote:
>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>
>>> 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@epam.com>
>> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
>>
>> Also, Ian already gave his release-ack.
> Thank you,
> Do I need to resend the patch with Acks? Or hopefully those can be applied
> on commit?

I have committed with the two tags applied.

Next time, please remember to carry Release-acked-by tag.

Cheers,

-- 
Julien Grall

Re: [PATCH-4.16 v2] xen/arm: fix SBDF calculation for vPCI MMIO handlers
Posted by Oleksandr Andrushchenko 2 years, 4 months ago
Hi, Julien!

On 03.11.21 20:17, Julien Grall wrote:
> Hi Oleksandr,
>
> On 03/11/2021 12:08, Oleksandr Andrushchenko wrote:
>>
>>
>> On 03.11.21 00:47, Stefano Stabellini wrote:
>>> On Tue, 2 Nov 2021, Oleksandr Andrushchenko wrote:
>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>
>>>> 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@epam.com>
>>> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
>>>
>>> Also, Ian already gave his release-ack.
>> Thank you,
>> Do I need to resend the patch with Acks? Or hopefully those can be applied
>> on commit?
>
> I have committed with the two tags applied.
Thank you
>
> Next time, please remember to carry Release-acked-by tag.
The Release-acked-by tag was sent by Ian *after* I've sent v2 of the patch
(this one) and it was in v1 mail thread....
>
> Cheers,
>