diff mbox series

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

Message ID 20211102112041.551369-1-andr2000@gmail.com (mailing list archive)
State New, archived
Headers show
Series [PATCH-4.16,v2] xen/arm: fix SBDF calculation for vPCI MMIO handlers | expand

Commit Message

Oleksandr Andrushchenko Nov. 2, 2021, 11:20 a.m. UTC 
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(-)

Comments

Stefano Stabellini Nov. 2, 2021, 10:47 p.m. UTC | #1 
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
>
Oleksandr Andrushchenko Nov. 3, 2021, 12:08 p.m. UTC | #2 
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
>>
Julien Grall Nov. 3, 2021, 6:17 p.m. UTC | #3 
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,
Oleksandr Andrushchenko Nov. 4, 2021, 5:56 a.m. UTC | #4 
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,
>
diff mbox series

Patch

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