From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <npickito@gmail.com>
Received: from mail-wm1-x342.google.com (mail-wm1-x342.google.com
 [IPv6:2a00:1450:4864:20::342])
 by sourceware.org (Postfix) with ESMTPS id 332223857C69
 for <newlib@sourceware.org>; Wed,  9 Dec 2020 08:21:51 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 332223857C69
Received: by mail-wm1-x342.google.com with SMTP id x22so625035wmc.5
 for <newlib@sourceware.org>; Wed, 09 Dec 2020 00:21:51 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc;
 bh=gMmPUB6SLDbJqM+85qfQLMeK9MIlPtEyczMxcuVPMQg=;
 b=c4whSQeQwB+bNIKObpYV3MyLbZk4jOXRh91up0dhLvjOCRBlVob0cy41tRnBotWXwW
 VLE7X/qLajNVXqIL+Nu7pa3u1Y3UQSLwxc/TVQFYXPqsUXONP7ytphiuvKlf9lqMvm50
 ReOHSXBi7zOQeMe35zR4ME/R0OqZOWICdElxyglTbotvUPjTHWFcYAOh7upanrguxzTA
 D+NWeAgl1GKcOErbMvkypmlr94Lui4ynih+2h+St9sWqCbFmi0poSXIxfRdPQSnF/bUu
 gzOPLn2dlgYHp2HW1VB6Vt1IpKMisd1x2X4NU5rlxmSRkFO0NohaT8v3aja8uaFFSaY9
 FkaA==
X-Gm-Message-State: AOAM531BFtGGV1fjpB89uAsgWpv++POjSCHs43bXjY4FFVmnXUiHp5xS
 tjCPCAK07ENbqlk4U+C3lyjgm38mTcTXCKlSX+7G8NtTcu6TBQ==
X-Google-Smtp-Source: ABdhPJy0odtxq/CkdZeaoHUrHGKn2hQvgJsS7hTImI+9czrcocrodBcmldIDvWcca3Edxpdy0eOrkEnJXHcNeijteu4=
X-Received: by 2002:a1c:49c2:: with SMTP id w185mr1445819wma.184.1607502110099; 
 Wed, 09 Dec 2020 00:21:50 -0800 (PST)
MIME-Version: 1.0
References: <694d497b-bc07-a3ba-2643-a7336927e9a7@embecosm.com>
In-Reply-To: <694d497b-bc07-a3ba-2643-a7336927e9a7@embecosm.com>
From: Kito Cheng <kito.cheng@gmail.com>
Date: Wed, 9 Dec 2020 16:21:38 +0800
Message-ID: <CA+yXCZD0ZNUNob0srvDQvefwO7C+AvqOfwSrLgZ0c76-ZDBg3A@mail.gmail.com>
Subject: Re: [PATCH] RISC-V: Add semihosting support
To: Craig Blackmore <craig.blackmore@embecosm.com>
Cc: Newlib <newlib@sourceware.org>
Content-Type: text/plain; charset="UTF-8"
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00, DKIM_SIGNED,
 DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,
 SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on
 server2.sourceware.org
X-BeenThere: newlib@sourceware.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Newlib mailing list <newlib.sourceware.org>
List-Unsubscribe: <https://sourceware.org/mailman/options/newlib>,
 <mailto:newlib-request@sourceware.org?subject=unsubscribe>
List-Archive: <https://sourceware.org/pipermail/newlib/>
List-Post: <mailto:newlib@sourceware.org>
List-Help: <mailto:newlib-request@sourceware.org?subject=help>
List-Subscribe: <https://sourceware.org/mailman/listinfo/newlib>,
 <mailto:newlib-request@sourceware.org?subject=subscribe>
X-List-Received-Date: Wed, 09 Dec 2020 08:21:52 -0000

Hi Craig:

I verified with GCC testsuite on qemu with Keith's semihosting patch,
And that's LGTM, only two minor review comment, see below:

> +++ b/libgloss/riscv/semihost-sys_exit.c
> @@ -0,0 +1,19 @@
> +#include <machine/syscall.h>
> +#include "semihost_syscall.h"
> +
> +#define ADP_Stopped_ApplicationExit 0x20026
> +
> +/* Exit a program without cleaning up files.  */
> +void
> +_exit (int exit_status)
> +{
> +#if __riscv_xlen == 32
> +  syscall_errno (SEMIHOST_exit, (long *) ADP_Stopped_ApplicationExit);

Could you use SEMIHOST_exit_extended here, so that we could have
return value on rv32.

> +/* Return the fdentry for file or NULL if not found.  */
> +
> +struct fdentry *
> +__get_fdentry (int file)
> +{
> +  if (file<0 || file>RISCV_MAX_OPEN_FILES || fdtable[file].handle == -1)

Should be >= RISCV_MAX_OPEN_FILES here?