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

Re: [PATCH v5 08/11] xen/arm: Enable the existing x86 virtual PCI support for ARM.

  • To: Julien Grall <julien@xxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Tue, 12 Oct 2021 17:50:50 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.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:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=uTEBRcsMW//GFyUzf19PhZO+G+xGbKzglhWcA6XKiDo=; b=mMvkMcocUK3oC302diekuy752nchdxXaSjAqgoP8icmhhUYVURE3pFH3cs/XeP6vMjO354vNrlMB+PS/njiAlwZ9G3XlCeLhzAbArWz90ZVuUMT4vUwBHfOI6z868eO7pRapU1VfZZc9kIiqsfpvWFoL0wg+5h1XdgFy9wNQIWJh5kH+x/p65PnvKrL5zKepnH27y5txAnPmBW4DWdEhi/bhq+x0dyv2U/KROjYEWL7Z9SK/vs0WJgeVQwLaPm4rZLxPPJ+7V9xkstqo3xyh0CpWp7mqLTCi70XRaosv1nRbiIZp4WPZ68rZ8TOaQQVekuOK/4r7uZP/tHCD7TaSZg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YxwFx2AxeCy/uXutu+m3D5v98EitDEPLVdf5Xekp3TWjWgvIGUh3Y5Am3T2C7ABjH4V/qHk9rRW+sK6sHsfhENLg7ocxjNx7qmuNqSXsB0AKWwrgHEMCtkuvc2zMvWN40Og5y7DckeCcasXgYouqJEzjxVPixCGld2wVLcIRTMBHMblSWvhGJHel4dRKGW++QXDLvi+wz9jPfvJByUO0Bi74YsG5cUAvOf6LL8UsXKNFfuRoHRlDNUB2L88H35w6rU1Dnqom68T71Ivl6JdVnq53+b8snWUCvtsitZ4ep1253Y3gEaHURudFL2bdylSe7KXFSidG3/tLIvzoj5E5uw==
  • Authentication-results-original: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=arm.com;
  • Cc: Jan Beulich <jbeulich@xxxxxxxx>, Rahul Singh <Rahul.Singh@xxxxxxx>, Andre Przywara <Andre.Przywara@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Paul Durrant <paul@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 12 Oct 2021 17:51:09 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHXutl3YqS5dHXsYk6KpdKt4d14RavHjPqAgAY38oCAAbo3AIAAEwAAgAACbACAABkfgA==
  • Thread-topic: [PATCH v5 08/11] xen/arm: Enable the existing x86 virtual PCI support for ARM.

Hi Julien,

> On 12 Oct 2021, at 17:20, Julien Grall <julien@xxxxxxx> wrote:
> On 12/10/2021 17:12, Bertrand Marquis wrote:
>> Hi Julien,
>>> On 12 Oct 2021, at 16:04, Julien Grall <julien@xxxxxxx> wrote:
>>> On 11/10/2021 13:41, Bertrand Marquis wrote:
>>>> Hi Jan,
>>> Hi Bertrand,
>>>> As Rahul is on leave, I will answer you and make the changes needed.
>>>>> On 7 Oct 2021, at 14:43, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>> Independent of this - is bare metal Arm enforcing this same
>>>>> alignment restriction (unconditionally)? Iirc on x86 we felt we'd
>>>>> better synthesize misaligned accesses.
>>>> Unaligned IO access could be synthesise also on arm to but I would
>>>> rather not make such a change now without testing it (and there is
>>>> also a question of it making sense).
>>> Yes it makes sense. I actually have an item in my TODO list to forbid 
>>> unaligned access because they should not happen on any device we currently 
>>> emulate.
>>> Although, I am not aware of any issue other than the guest would shoot 
>>> itself in the foot if this happens.
>>>> So if it is ok with you I will keep that check and discuss it with Rahul
>>>> when he is back. I will add a comment in the code to make that clear.
>>> I am OK with it.
>>> [...]
>>>>> Throughout this series I haven't been able to spot where the HAS_VPCI
>>>>> Kconfig symbol would get selected. Hence I cannot tell whether all of
>>>>> this is Arm64-specific. Otherwise I wonder whether size 8 actually
>>>>> can occur on Arm32.
>>>> Dabt.size could be 3 even on ARM32 but we should not allow 64bit
>>>> access on mmio regions on arm32.
>>> Hmmm... Looking at the Armv7 and Armv8 spec, ldrd/strd (64-bit read) would 
>>> not present a valid ISV. So I think it is not be possible to have dabt.size 
>>> = 3 for 32-bit domain. But I agree we probably want to harden the code.
>>>> So I will surround this code with ifdef CONFIG_ARM_64 and add a test
>>>> for len > 4 to prevent this case on 32bit.
>>>> To be completely right we should disable this also for 32bit guests but
>>>> this change would be a bit more invasive.
>>> I think the following should be sufficient:
>>> if ( is_domain_32bit_domain() && len > 4 )
>>>  return ...;
>> With the last request from Roger to use the function implemented in 
>> arch/x86/hw/io.c, the function will move to vpci.h so using is_32bit_domain 
>> will not be possible without ifdefery CONFIG_ARM.
>> Also I have no access to the domain there.
>> So the best I can do for now would be something like:
>> #ifdef CONFIG_ARM_32
>> If (len == 8)
>>     return false
>> #endif
>> A 32bit guest on 64bit xen would not be checked.
>> Would that be ok for now ?
> I think the #ifdef is a bit pointless. My preference would be to not add the 
> #ifdef but instead add...
>> I could add a comment in the code to warn about the limitation.
> .. a comment for now as this is only an hardening problem.

Agree I will do that.


> Cheers,
> -- 
> Julien Grall



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