Re: RT Xen on ARM - R-Car series

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Andrii Anisov <andrii.anisov@gmail.com>
Cc: Julien Grall <julien.grall@arm.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	"LOPEZ, FUENTES NACARINO Jairo Eduardo" <jairo@ruri.waseda.jp>,
	xen-devel@lists.xen.org
Subject: Re: RT Xen on ARM - R-Car series
Date: Thu, 14 Feb 2019 17:29:09 +0100	[thread overview]
Message-ID: <20190214162909.ylnjnarbkfkrpwyk@mac> (raw)
In-Reply-To: <316d93d5-7b4d-ace3-9bfa-7680057e92bd@gmail.com>

On Tue, Feb 12, 2019 at 08:21:32PM +0200, Andrii Anisov wrote:
> Hello Roger,
> 
> Sorry for a delayed response.
> 
> On 07.02.19 12:35, Roger Pau Monné wrote:
> > I've been thinking about this with other Citrix folks, and I'm not
> > sure the proposed patch is a good solution. It's not possible for us
> > to know whether there's a kernel somewhere relying on changing the
> > virtual address of the runtime state area without issuing a new
> > hypercall.
> 
> Do you mean allocating another buffer by VM and mapping it to a virtual address known to XEN? Or remapping existing buffer to a different virtual address?

I meant that with the current interface a user could change the
backing memory behind the virtual address passed in the runstate
register hypercall and expect Xen to write to the new physical memory
area without having to do anything else.

Attempting to do that with my proposed patch can result in hard to
debug guest memory corruption.

> > If such kernel existed by making this change we would introduce random
> > memory corruption to that kernel, which would be very hard to track
> > and considered a regression.
> 
> I guess you actually mean that VM is trying to map another physical buffer to a vaddr known to XEN. As I said here [1], even current implementation looks problematic, because VM's changes in PT are not atomic from the hypervisor point of view.
> I stated that for ARM, but x86 does not seem to differ here.

OSes use atomic operations to update a PTE, so I'm not sure how that
could be problematic. Xen will either get the new or the old address
from the PTE, but never a half-written value. Any OS wanting to do
this should be very careful about how it's page tables are updated.

> Actually VM trying to make changes behind a hypervisor's back is a really bad idea. Because the hypervisor *is* always behind the VM's back.
> 
> > I think the best way to move forward is to pick my patch and introduce
> > a new hypercall that instead of a virtual address takes a guest
> > physical address. Will you be OK with this Andrii?
> It might work better for this. And introducing a new interface is a chance to get rid of a mixed legacy.

Yes, I think introducing a new hypercall is the best way to move
forward.

> > Note that the Linux kernel would also need to be modified to make use
> > of this new hypercall, but that's likely close to a 1 line change.I would not say it is a 1 line change.
> I think about a page alignment for the runstate area, I'd like to have it directly accessed from XEN. I do not like `update_runstate_area()` with its all kind of copy_to_guest, done twice a context switch.

In order to simplify stuff the new interface could require runstate
areas to be page aligned, but I think the check can be relaxed to
simply require runstate areas to not cross a page boundary.

If you use a guest physical address for the interface you don't have
to use copy_to_guest because that expects a guest virtual addresses.

Thanks, Roger.

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