Hi Stefano,
On 06/08/2021 23:26, Stefano Stabellini wrote:
On Fri, 6 Aug 2021, Julien Grall wrote:
Hi Stefano,
On 06/08/2021 21:15, Stefano Stabellini wrote:
On Fri, 6 Aug 2021, Oleksandr Tyshchenko wrote:
Hello, all.
I would like to clarify some bits regarding a possible update for "Xen
device tree bindings for the guest" [1].
A bit of context:
We are considering extending "reg" property under the hypervisor node and
we would like to avoid breaking backward compatibility.
So far, the "reg" was used to carry a single region for the grant table
mapping only and it's size is quite small for the new improvement
we are currently working on.
What we want to do is to extend the current region [reg: 0] and add an
extra regions [reg: 1-N] to be used as a safe address space for any
Xen specific mappings. But, we need to be careful about running "new"
guests (with the improvement being built-in already) on "old" Xen
which is not aware of the extended regions, so we need the binding to be
extended in a backward compatible way. In order to detect whether
we are running on top of the "new" Xen (and it provides us enough space to
be used for improvement), we definitely need some sign to
indicate that.
Could you please clarify, how do you expect the binding to be changed in
the backward compatible way?
- by adding an extra compatible (as it is a change of the binding
technically)
- by just adding new property (xen,***) to indicate that "reg" contains
enough space
- other option
The current description is:
- reg: specifies the base physical address and size of a region in
memory where the grant table should be mapped to, using an
HYPERVISOR_memory_op hypercall [...]
Although it says "a region" I think that adding multiple ranges would be
fine and shouldn't break backward compatibility.
In addition, the purpose of the region was described as "where the grant
table should be mapped". In other words, it is a safe address range
where the OS can map Xen special pages.
Your proposal is to extend the region to be bigger to allow the OS to
map more Xen special pages. I think it is a natural extension to the
binding, which should be backward compatible.
I agree that extending the reg (or even adding a second region) should be fine
for older OS.
Rob, I am not sure what is commonly done in these cases. Maybe we just
need an update to the description of the binding? I am also fine with
adding a new compatible string if needed.
So the trouble is how a newer Linux version knows that the region is big
enough to deal with all the foreign/grant mapping?
If you run on older Xen, then the region will only be 16MB. This means the
Linux will have to fallback on stealing RAM as it is today.
IOW, XSA-300 will still be a thing. On newer Xen (or toolstack), we ideally
want the OS to not fallback on stealing RAM (and close XSA-300). This is where
we need a way to advertise it.
The question here is whether we want to use a property or a compatible for
this.
I am leaning towards the latter because this is an extension of the bindings.
However, I wasn't entirely whether this was a normal way to do it.
Although I think it would be OK to have a new compatible string, am I
not sure we need it.
Let's assume we don't add a new compatible string, property... How do
would you prevent the following two issues?
1) XSA-300: A frontend can DoS the backend
2) Existing Xen expects the grant-table to be mapped at the exact
same place.
2# could potentially be solved by reserved the first range for the grant
table. For 1#, I think we need a compatible string (or property).
What else do you have in mind?
FAOD, relying on the region to always be big enough would not be an
acceptable solution to me :). A frontend may find a new way for a
frontend to exhaust the region (*hint* virtio *hint*).
In any case, we'll have to be able to recognize and handle the case
where we run out of the space in the provided region. If the region is
too small (16MB) then it just means we'll run out of space immediately
after mapping the grant table. Then, we'll have to use other techniques.
Right, one of the other techniques is likely to steal RAM page. Which
means that a frontend could potentially DoS the backend. This will be a
lot easier to trigger with virtio as the DM tends to cache the mappings.
So I think we ought to prevent stealing the RAM if a new kernel is
running on a new Xen.
Or perhaps you think that if we had a new compatible string to say "Xen
binding with a larger region" then we could get away with a simpler
implementation in Linux, one that doesn't handle the case where we run
out of space in the region? If that was the case, then I agree that it
would be worthwhile adding a new compatible.
We will need to keep the code to steal RAM for the forseeable future
because a newer Linux may run on an older Xen setup. So simplicity is
not the reason here.
I have provided the reason above.
Cheers,
--
Julien Grall
