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

Re: [PATCH v2] xen/arm: Using unsigned long for arm64 MPIDR mask


  • To: Julien Grall <julien@xxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Wed, 20 Jan 2021 18:01:58 +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-SenderADCheck; bh=61sc3qh+iXzYr93Lkty1KxubI6uEJzno85wSQt9WbOk=; b=RWoPw7GUaXi+Zm4NZX5WTD8dcUeCCgZ4gY/RbifgWTQmOW+dEYQhfCQ/vo87Uiq/VWLaLPHSVShVlsmjVSzDhL/VCt2bcesyRmwSO5pShZlhywE88E6tOluy0cI+Fd93mQWMQVjj549GGM1p3GrVpHwPow6HiB01XdXmKIZlQqXWWwTPa3iTBhqfbLicMPjHatFgv3jlui7q6bl0UxlvBhzqhcYEZZ3mvjRNjeY2jquJs91ZzYE4LqQmK4M4avRGjsjYRlFDC7zz/jnZkwrTx6RKQCdo7dc7JxEsjFxd3IP4lLZCVTQsDc90aDhttJCVDzJdqXW+g0JeX57MjtYeDA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kxuW3dRRuybSVB1fcfBF3Y82G/MWgO48bQv3gPkeCP6IAXrV+iczTyGFeQ79gmkWt5+J7DLvf+n9rzJKcdz1769gZ8yfJvNKGgELWC0MMO0nV4PlqxGxM1wsR8Zene5TuAhLh8QiT40OPghxBArPufMxqDIyVl9UEApYKRtz5wY/q4Y/ySdnQCze9M+JL+aXnCk8jxuP5s0GwCk1vZn+cQKsI6DyodQuTnrSKAbfdUlidq0PT1Ts9f8RqNjrzE/q2ksSou/DSqKbUiWm/FHfBZSujYuL4EUCOWBZJ8DfFnIw6LmzSE0EwOh38gIu/78VXHX4a7jp02vSaTIrIr81wA==
  • Authentication-results-original: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=arm.com;
  • Cc: Wei Chen <Wei.Chen@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, Penny Zheng <Penny.Zheng@xxxxxxx>, Jiamei Xie <Jiamei.Xie@xxxxxxx>, nd <nd@xxxxxxx>
  • Delivery-date: Wed, 20 Jan 2021 18:02:16 +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: AQHW5Ye9qAqBzHV/jEeIa7+b+bK7L6odnJIAgAABQgCAE0LNAIAAAP0A
  • Thread-topic: [PATCH v2] xen/arm: Using unsigned long for arm64 MPIDR mask

Hi,

> On 20 Jan 2021, at 17:58, Julien Grall <julien@xxxxxxx> wrote:
> 
> On 08/01/2021 11:50, Wei Chen wrote:
>> Hi Julien
> 
> Hi Wei,
> 
> Sorry for the late answer. While cleaning my inbox today, I noticied that I 
> didn't reply to this thread :(.
> 
>>>> integer will do unsigned extend while doing some operations
>>>> with 64-bit unsigned integer. This can lead to unexpected
>>>> result in some use cases.
>>>> 
>>>> For example, in gicv3_send_sgi_list of GICv3 driver:
>>>> uint64_t cluster_id = cpu_logical_map(cpu) & ~MPIDR_AFF0_MASK;
>>>> 
>>>> When MPIDR_AFF0_MASK is 0xFFU, compiler output:
>>>>      f7c: 92785c16 and x22, x0, #0xffffff00
>>>> When MPIDR_AFF0_MASK is 0xFFUL, compiler output:
>>>>      f88: 9278dc75 and x21, x3, #0xffffffffffffff00
>>>> 
>>>> If cpu_logical_map(cpu) = 0x100000000UL and MPIDR_AFF0_MASK is
>>>> 0xFFU, the cluster_id returns 0. But the expected value should
>>>> be 0x100000000.
>>>> 
>>>> So, in this patch, we force aarch64 to use unsigned long
>>>> as MPIDR mask to avoid such unexpected results.
>>> 
>>> How about the following commit message:
>>> 
>>> "Currently, Xen is considering that all the affinity bits are defined
>>> below 32-bit. However, Arm64 define a 3rd level affinity in bits 32-39.
>>> 
>>> The function gicv3_send_sgi_list in the GICv3 driver will compute the
>>> cluser using the following code:
>>> 
>>> uint64_t cluster_id = cpu_logical_map(cpu) & ~MPIDR_AFF0_MASK;
>>> 
>>> Because MPIDR_AFF0_MASK is defined as a 32-bit value, we will miss out
>>> the 3rd level affinity. As a consequence, the IPI would not be sent to
>>> the correct vCPU.
>>> 
>>> This particular error can be solved by switching MPIDR_AFF0_MASK to use
>>> unsigned long. However, take the opportunity to switch all the MPIDR_*
>>> define to use unsigned long to avoid anymore issue.
>>> "
>>> 
>>> I can update the commit message while committing if you are happy with it.
>>> 
>> Yes, that would be good, thank you very much : )
> 
> Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Also tested on a platform where Xen was not booting without this patch and i can
confirm it fixed the issue :-)

Cheers
Bertrand

> 
> And committed.
> 
> Cheers,
> 
> -- 
> Julien Grall




 


Rackspace

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