[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/3] xen/riscv: initialize .bss section
- To: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Thu, 2 Mar 2023 14:12:28 +0000
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.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=FySKrIWGJRmZKRcC6QD8aWgdn/j/04v2B3EY29nCpno=; b=docb78HzA6OtfECD/c2AHUUyql8RMwaUiJIca9/JIaQopdv8JJPfEGVxLXJW+7ad1nV2Ss9LQ8R5NWYsXE63nuPUu7oJZII96R+paDB4KOKH/2fU87FPdwy6XmNeW4Hrq73V5IVyiS1R80mmnfo5o0ED5955SsZCpTgVDLOUOk4SLYuvSsrVag5dL1J+u43HUrYF4O4jxWTvMVTrKStw9rLzkGn3Lrg9zcMGE9nwxc6YzhtyRFs5NOnyZ4UZ0yJPtIQz5FdgO2JM5U7TjJKzOHd6XVf6qeflYpKofmYVHuoAinGBCODyHr1mGQ1361rmXaxBXnaf+emP/4ORa0by8g==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FX43nkMp1dyCEf1Txm9tAjCUlFPL0BRwaQcdAPa7361eogfhlw+gk+OlOkhysGhR+q6lVj4hjIdmVFjSD+551VPVRRFU4uZHZ52CUEyurQECu+GeeX5lFAc02yQo8Uwv0Hs/c9nVoK101qsB8sMudpkDJjps82hxShXENa8VmqvZEibVyD7UlLZ9NQ4OFfB4Qp+RS3jFn9eV2DMha+1ppUzA3Q3i5APrgCM2/7bcsomXN3ScykF+4gnQ3fGOLPviP2Go/340I3IfikQFRgdBj5IIG7rj6OIWtTltZYfKvPmgXZG+ov3uZbbaSust7yp8+mDe030ZYnT84vepUz8Dgg==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Jan Beulich <jbeulich@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Gianluca Guida <gianluca@xxxxxxxxxxxx>, Bob Eshleman <bobbyeshleman@xxxxxxxxx>, Alistair Francis <alistair.francis@xxxxxxx>, Connor Davis <connojdavis@xxxxxxxxx>
- Delivery-date: Thu, 02 Mar 2023 14:12:50 +0000
- Ironport-data: A9a23:SPsGwakcYCR6D/Zct/EiCsLo5gw0J0RdPkR7XQ2eYbSJt1+Wr1Gzt xIYCGqFPf+JN2WgfttwO96yoU1SvZHSm9AwQQY4qyE2FCMWpZLJC+rCIxarNUt+DCFhoGFPt JxCN4aafKjYaleG+39B55C49SEUOZmgH+a6U6icfHgqH2eIcQ954Tp7gek1n4V0ttawBgKJq LvartbWfVSowFaYCEpNg064gE4p7aSaVA8w5ARkPqgQ5AWGzRH5MbpETU2PByqgKmVrNrbSq 9brlNmR4m7f9hExPdKp+p6TnpoiG+O60aCm0xK6aoD66vRwjnVaPpUTbZLwXXx/mTSR9+2d/ f0W3XCGpaXFCYWX8AgVe0Ew/yiTpsSq8pefSZS0mZT7I0Er7xIAahihZa07FdRwxwp5PY1B3 fgodDcRYC6ivtPo8qmUGspVp90ScfC+aevzulk4pd3YJdAPZMmaBo7tvJpf1jp2gd1SF/HDY cZfcSBocBnLfxxIPBEQFY46m+CrwHL4dlW0qnrM/fZxvzeVkVI3ieewWDbWUoXiqcF9t0CUv G/ZuU/+BQkXLoe3wjuZ6HO8wOTImEsXXapDROXgq6Q62TV/wEQcFwNKbhy/kMPmtWmsQctFJ BQI1SUX+P1aGEuDC4OVsweDiHmAsx0HWtsWEPAg7wqNya387AOQB2xCRTlEAPQ2uclzSTE02 1uhm9LyGScpoLCTUWia9LqfsXW1Iyd9BW0IaDIATAAFy8L+u4x1hRXKJv54C7K8hNDxHTD2w hiJoTI4irFVitQEv420/FnBhy+nr7DTTxAy/QTRVSSu6QYRTISibYa55EPb6ftFJYCxQVyIv XxCkM+bhMgHCZCWiCWMWqMDBriv7PeeGCLQihhkGJxJ3z+q/Xikf4xZ/jBlDEhsO8cAPzTuZ SfuVRh54ZZSOD6havZxaofoUsAyl/G/SJLiS+zeacdIbt5pbgib8SpyZEmWmWfwjEwrlqJ5M pCeGSqxMUsn5W1c5GLeb48gPXUDnEjSGUu7qUjH8ima
- Ironport-hdrordr: A9a23:OOIW0aoK24+ajKY1gZ6VPGUaV5oleYIsimQD101hICG9E/b1qy nKpp8mPHDP5wr5NEtPpTnjAsm9qALnlKKdiLN5Vd3OYOCMghrKEGgN1/qG/xTQXwH46+5Bxe NBXsFFebnN5IFB/KTH3DU=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 02/03/2023 1:23 pm, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
> Changes since v1:
> * initialization of .bss was moved to head.S
> ---
> xen/arch/riscv/include/asm/asm.h | 4 ++++
> xen/arch/riscv/riscv64/head.S | 9 +++++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/xen/arch/riscv/include/asm/asm.h
> b/xen/arch/riscv/include/asm/asm.h
> index 6d426ecea7..5208529cb4 100644
> --- a/xen/arch/riscv/include/asm/asm.h
> +++ b/xen/arch/riscv/include/asm/asm.h
> @@ -26,14 +26,18 @@
> #if __SIZEOF_POINTER__ == 8
> #ifdef __ASSEMBLY__
> #define RISCV_PTR .dword
> +#define RISCV_SZPTR 8
> #else
> #define RISCV_PTR ".dword"
> +#define RISCV_SZPTR 8
> #endif
> #elif __SIZEOF_POINTER__ == 4
> #ifdef __ASSEMBLY__
> #define RISCV_PTR .word
> +#define RISCV_SZPTR 4
> #else
> #define RISCV_PTR ".word"
> +#define RISCV_SZPTR 4
This an extremely verbose way of saying that __SIZEOF_POINTER__ is the
right value to use...
Just drop the change here. The code is better without this indirection.
> #endif
> #else
> #error "Unexpected __SIZEOF_POINTER__"
> diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
> index 851b4691a5..b139976b7a 100644
> --- a/xen/arch/riscv/riscv64/head.S
> +++ b/xen/arch/riscv/riscv64/head.S
> @@ -13,6 +13,15 @@ ENTRY(start)
> lla a6, _dtb_base
> REG_S a1, (a6)
>
/* Clear the BSS */
The comments (even just oneliners) will become increasingly useful as
the logic here grows.
> + la a3, __bss_start
> + la a4, __bss_end
> + ble a4, a3, clear_bss_done
> +clear_bss:
> + REG_S zero, (a3)
> + add a3, a3, RISCV_SZPTR
> + blt a3, a4, clear_bss
> +clear_bss_done:
You should use t's here, not a's. What we are doing here is temporary
and not constructing arguments to a function. Furthermore we want to
preserve the a's where possible to avoid spilling the parameters.
Finally, the symbols should have an .L_ prefix to make the local symbols.
It really doesn't matter now, but will when you're retrofitting elf
metadata to asm code to make livepatching work. (I'm doing this on x86
and it would have been easier if people had written the code nicely the
first time around.)
~Andrew
|
