[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 2/2] xen-pciback: allow compiling on other archs than x86


  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
  • Date: Mon, 27 Sep 2021 05:13:43 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=0/znJzAAQQJ+c4hgRTIU8PEFLUy1MPlz0bgZAqDxVe4=; b=Yu2Xr+PiTjwk6skyW4OMk5pygP6ZBov+FiPsz5B3C0dxAXPJUoEBVtmxIUo+hwz2ZjnUHp6rmpHitifACL2mxB6bD+85lpT12/ynl+S4Z3p+jWoubsWGPwqwxvVVj5d8qf54eLulbx3nx7VonZ6cNP0oILRDN3BtGIVErhY+E2IuG5TN3Ukyx401yC2lBy42XEWMdUQ6ggMGT7GwcFwpfSEGfiTsi3Ewi5IPUxluAdWxkPq0GlC7puPNpLNnb8+Y4IrIDEro+tDVXKBhs9bjfQHztqQY/Gi7A83JznYPzT7+x5KL9fT5XsdEYX2FGR3AGfMRx+Z70ZDCibyWfZBBXA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=n5EiGtdaFQRcAnwiMxVokVCXYW7jOac1pWmuQvhh9jrRWXRMtTRxm7UXOsLxxfyJ85S2KGEYN1+5AGF+s2kQxDaNfWEttAr2YHzU/WtR0nkssLkpoVOTxiVafmA3/p+yCdU/gbNsACQP94oUGaNXgOTuY+G0gnbxab4fzFoNO0uCquKDh8waxD+Pga2DK5Tyf3lh9zUUIPR8r1shgmHQe3evHiA5BVtODfj5qptgOUpkS3xviVVgeizNIxMz8u4c9ArxC9GFi0dzMJru+4doxCUaF7+NLxqOFB+w9HPNdve+WPVnHmIkxUXNamCMNI9ERYZ9bnCr86zr7U/ItGgZrQ==
  • Authentication-results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=epam.com;
  • Cc: "jgross@xxxxxxxx" <jgross@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>, "boris.ostrovsky@xxxxxxxxxx" <boris.ostrovsky@xxxxxxxxxx>, "julien@xxxxxxx" <julien@xxxxxxx>, "jbeulich@xxxxxxxx" <jbeulich@xxxxxxxx>, Anastasiia Lukianenko <Anastasiia_Lukianenko@xxxxxxxx>
  • Delivery-date: Mon, 27 Sep 2021 05:13:55 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHXsGDrkewW5NG+ZkqXauJ2P2aIyauyCpkAgACjoYCAAGJfAIAAjV+AgAO+J4A=
  • Thread-topic: [PATCH v3 2/2] xen-pciback: allow compiling on other archs than x86

On 24.09.21 23:04, Stefano Stabellini wrote:
> On Fri, 24 Sep 2021, Oleksandr Andrushchenko wrote:
>> On 24.09.21 08:46, Oleksandr Andrushchenko wrote:
>>> On 23.09.21 23:00, Stefano Stabellini wrote:
>>>> On Thu, 23 Sep 2021, Oleksandr Andrushchenko wrote:
>>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>>>>>
>>>>> Xen-pciback driver was designed to be built for x86 only. But it
>>>>> can also be used by other architectures, e.g. Arm.
>>>>> Re-structure the driver in a way that it can be built for other
>>>>> platforms as well.
>>>>>
>>>>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>>>>> Signed-off-by: Anastasiia Lukianenko <anastasiia_lukianenko@xxxxxxxx>
>>>> The patch looks good to me. Only one thing: on ARM32 I get:
>>> WE do not yet support Xen PCI passthrough for ARM32
> Keep in mind that it is possible to run ARM32 guests on an ARM64
> hypervisor.
>
>
>>>> drivers/xen/xen-pciback/conf_space_header.c: In function ‘bar_init’:
>>>> drivers/xen/xen-pciback/conf_space_header.c:239:34: warning: right shift 
>>>> count >= width of type [-Wshift-count-overflow]
>>>>        bar->val = res[pos - 1].start >> 32;
>>>>                                      ^~
>>>> drivers/xen/xen-pciback/conf_space_header.c:240:49: warning: right shift 
>>>> count >= width of type [-Wshift-count-overflow]
>>>>        bar->len_val = -resource_size(&res[pos - 1]) >> 32;
>>>>     
>>>>     
>>>> resource_size_t is defined as phys_addr_t and it can be 32bit on arm32.
>>>>
>>>>
>>>> One fix is to surround:
>>>>
>>>>            if (pos && (res[pos - 1].flags & IORESOURCE_MEM_64)) {
>>>>                    bar->val = res[pos - 1].start >> 32;
>>>>                    bar->len_val = -resource_size(&res[pos - 1]) >> 32;
>>>>                    return bar;
>>>>            }
>>>>
>>>> with #ifdef PHYS_ADDR_T_64BIT
>>>>
>>> This might not be correct. We are dealing here with a 64-bit BAR on a 
>>> 32-bit OS.
>>>
>>> I think that this can still be valid use-case if BAR64.hi == 0. So, not sure
>>>
>>> we can just skip it with ifdef.
>>>
>>> Instead, to be on the safe side, we can have:
>>>
>>> config XEN_PCIDEV_STUB
>>>           tristate "Xen PCI-device stub driver"
>>>           depends on PCI && ARM64 && XEN
>>> e.g. only allow building the "stub" for ARM64 for now.
> This is a pretty drastic solution. I would be OK with it but I prefer
> the solution below >> 16 >> 16.
>
>
>> Or... there are couple of places in the kernel where PCI deals with the 32 
>> bit shift as:
>>
>> drivers/pci/setup-res.c:108:        new = region.start >> 16 >> 16;
>> drivers/pci/iov.c:949:        new = region.start >> 16 >> 16;
>>
>> commit cf7bee5a0bf270a4eace0be39329d6ac0136cc47
>> Date:   Sun Aug 7 13:49:59 *2005* +0400
>>
>> [snip]
>>
>>       Also make sure to write high bits - use "x >> 16 >> 16" (rather than 
>> the
>>       simpler ">> 32") to avoid warnings on 32-bit architectures where we're
>>       not going to have any high bits.
> I think this is the best option

Ok, so for both patches:

1. I'll move CONFIG_XEN_PCIDEV_STUB into the second patch

2. I'll use >> 16 >> 16 to fix 32-bit architectures


>
>
>> This might not be(?) immediately correct in case of LPAE though, e.g.
>>
>> 64-bit BAR may tolerate 40-bit address in some use-cases?
> It is correct for LPAE too, it is just that with LPAE it would be
> unnecessary.

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.