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

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

On 03.11.21 00:47, Stefano Stabellini wrote:
> 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.
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
>>