xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Bobby Eshleman <bobbyeshleman@gmail.com>,
	<xen-devel@lists.xenproject.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Bobby Eshleman <bobby.eshleman@starlab.io>,
	Dan Robertson <dan@dlrobertson.com>,
	Alistair Francis <alistair.francis@wdc.com>
Subject: Re: [Xen-devel] [RFC XEN PATCH 00/23] xen: beginning support for RISC-V
Date: Fri, 24 Jan 2020 13:41:38 +0000	[thread overview]
Message-ID: <abbe5c9f-46e2-af76-a7ff-d98c55fa364f@citrix.com> (raw)
In-Reply-To: <cover.1579615303.git.bobbyeshleman@gmail.com>

On 22/01/2020 01:58, Bobby Eshleman wrote:
> Currently, this patchset really only sets up virtual memory for Xen and
> initializes UART to enable print output.  None of RISC-V's
> virtualization support has been implemented yet, although that is the
> next road to start going down.  Many functions only contain dummy
> implementations.  Many shortcuts have been taken and TODO's have been
> left accordingly.  It is very, very rough.  Be forewarned: you are quite
> likely to see some ungainly code here (despite my efforts to clean it up
> before sending this patchset out).  My intent with this RFC is to align
> early and gauge interest, as opposed to presenting a totally complete
> patchset.

I've taken a very quick look over the series.

Normally, we require that all patches be bisectable, and this series is
not because of the Makefile changes in patch 3 specifying object files
which are introduced in the following 20 patches.  Of course,
introducing a brand new architecture is a special case, but the
suggested plan of getting a "minimal build" together will help keep the
second phase of "making it boot".

To start with, I'd recommend a head.S with _start: and an infinite loop,
along with whatever makefile/kconfig infrastructure is required to get a
build.

Within that first phase however, there are some obvious tweaks we should
make to common code.  For one, the debugger_trap() infrastructure really
is x86-specific (and I haven't seen it used in a decade) so should have
its ARM stubs dropped so as not to be a burden on other architectures.

There are other aspects (the atomic_t infrastructure) where x86 and ARM
already have basically identical copies of the header file, and you've
taken a 3rd copy.

Other areas where you can reduce the minimum build would be to put some
defaults into the defconfig, such as disabling grant tables and mem
access.  There are almost certainly others which will help, so have a
search around menuconfig.

Disabling grant tables in particular will work around the fact that the
ARM snapshot you based your port on was pre-XSA-295, and the cmpxchg
loop against guest memory in gnttab_clear_flags() is reintroducing a
previously-fixed security vulnerability.

Some ARM-isms you've inherited would be much better if you didn't.  In
particular, I *really* hope RISC-V hasn't made the same backwards naming
bug in their pagetable levels which results in {second,first,zeroth}_*
et al.  In x86, we purposefully chose not to follow Intel's naming, and
instead went with L1, L2, L3, and for 64bit L4.

As a couple of general points from our coding style, please avoid
commented out code, and you are free to omit braces for single statement
blocks.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  parent reply	other threads:[~2020-01-24 13:42 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-22  1:58 [Xen-devel] [RFC XEN PATCH 00/23] xen: beginning support for RISC-V Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 01/23] HACK: OE Build changes Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 02/23] HACK: Makefile: Don't build Xen tools Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 03/23] riscv: makefiles and Kconfig Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 04/23] riscv: header files Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 05/23] riscv: early setup code Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 06/23] riscv: Add riscv to tools/libxc header files Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 07/23] riscv: Add asm-offsets.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 08/23] riscv: Add delay.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 09/23] riscv: Add domain.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 10/23] riscv: Add domctl.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 11/23] riscv: Add guestcopy.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 12/23] riscv: Add time.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 13/23] riscv: Add smp.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 14/23] riscv: Add shutdown.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 15/23] riscv: Add traps.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 16/23] riscv: Add irq.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 17/23] riscv: Add vm_event.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 18/23] riscv: Add p2m.c Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 19/23] riscv: Add the lib directory Bobby Eshleman
2020-06-22 11:38   ` Jan Beulich
2020-06-30  1:55     ` Bobby Eshleman
2020-01-22  1:58 ` [Xen-devel] [RFC XEN PATCH 20/23] riscv: Add smpboot.c Bobby Eshleman
2020-01-22  1:59 ` [Xen-devel] [RFC XEN PATCH 21/23] riscv: Add percpu.c Bobby Eshleman
2020-01-22  1:59 ` [Xen-devel] [RFC XEN PATCH 22/23] riscv: Add sysctl.c Bobby Eshleman
2020-06-22 11:43   ` Jan Beulich
2020-06-30  1:51     ` Bobby Eshleman
2020-01-22  1:59 ` [Xen-devel] [RFC XEN PATCH 23/23] riscv: Add iommu.c Bobby Eshleman
2020-01-22 14:57 ` [Xen-devel] [RFC XEN PATCH 00/23] xen: beginning support for RISC-V Andrew Cooper
2020-01-22 16:27   ` Lars Kurth
2020-01-23  5:31     ` Bobby Eshleman
2020-01-23 23:44       ` Lars Kurth
2020-01-25  1:59         ` Bobby Eshleman
2020-01-22 21:05   ` Stefano Stabellini
     [not found]     ` <20200123044527.GA5583@bobbye-pc>
2020-01-23  5:13       ` Bobby Eshleman
2020-01-23  5:19   ` Bobby Eshleman
2020-01-23 16:02     ` Andrew Cooper
2020-01-25  1:58       ` Bobby Eshleman
2020-01-23  8:25   ` Alistair Francis
2020-01-24 13:41 ` Andrew Cooper [this message]
2020-01-25  3:26   ` Bobby Eshleman
2020-01-25 17:11     ` Andrew Cooper
2020-01-28  3:37       ` Bobby Eshleman
2020-06-16  1:03 ` Stefano Stabellini
2020-06-16  1:08   ` Roman Shaposhnik
2020-06-16  1:10   ` Alistair Francis
2020-06-16  3:51     ` Bobby Eshleman
2020-06-16 20:16       ` Stefano Stabellini
2020-06-30  1:50         ` Bobby Eshleman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=abbe5c9f-46e2-af76-a7ff-d98c55fa364f@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=alistair.francis@wdc.com \
    --cc=bobby.eshleman@starlab.io \
    --cc=bobbyeshleman@gmail.com \
    --cc=dan@dlrobertson.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=julien@xen.org \
    --cc=konrad.wilk@oracle.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).