Re: [Xen-devel] How to deal with hypercalls returning -EFAULT

2018-06-14 Thread Jan Beulich 
>>> On 14.06.18 at 10:48,  wrote:
> On 14/06/18 10:17, Julien Grall wrote:
>> On Thu, 14 Jun 2018, 07:40 Juergen Gross, > > wrote:
>> 
>> On 13/06/18 18:25, Andrew Cooper wrote:
>> > On 13/06/18 16:27, Juergen Gross wrote:
>> 
>> Looking more into the privcmd driver I'm rather sure now I can solve
>> this via mmap() plus an ioctl(). So the kernel would allocate the
>> bounce buffers and map them into user space. This will require changes
>> only in libxencall without having to modify its external interface.
>> 
>> 
>> It looks like David Vrabel sent in 2016 a patch implementing that
>> solution [1]. It seems it has not been merged. Do you have any insight
>> why it was not merged?
> 
> No. It had all needed Acks and there were no open questions.
> 
> Thanks for the reference, I think I can just reuse the patch after some
> rebasing.

But I still wonder whether retaining references obtained through
get_user_pages_fast() across system calls is a legitimate thing to do.

Jan



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

Re: [Xen-devel] How to deal with hypercalls returning -EFAULT

2018-06-14 Thread Jan Beulich 
>>> On 13.06.18 at 18:25,  wrote:
> On 13/06/18 16:27, Juergen Gross wrote:
>> 4. Modify the interface between hypervisor and kernel: instead of just
>>returning -EFAULT let the hypervisor behave more like copy_to_user by
>>raising a page fault which can then be fixed up in the kernel. This
>>change must be activated by the kernel, of course.
>>Pros: - rather simple change in the kernel "doing the right thing"
>>  - hypercall bounce buffer handling in libxc/libxencall can be
>>switched off for a kernel supporting this chnage
>>Cons: - split solution between kernel and Xen, both must be changed
>>  - not sure how complex the required hypervisor change will be
> 
> Sadly, as I've just realised...
> 
> Con: Cannot be used to replace all -EFAULTs.
> 
> Faults when copying data in can be resolved by passing #PF to the
> kernel, but faults when trying to update guest state (continuation, or
> completion information) cannot be safely resumed at a later point.

Yeah, I've too realized this meanwhile. This is not an option, at least
not in simple enough a shape as would be necessary here.

Jan



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

Re: [Xen-devel] How to deal with hypercalls returning -EFAULT

2018-06-14 Thread Juergen Gross 
On 14/06/18 10:17, Julien Grall wrote:
> Hi Juergen,
> 
> Sorry for the formatting.
> 
> On Thu, 14 Jun 2018, 07:40 Juergen Gross,  > wrote:
> 
> On 13/06/18 18:25, Andrew Cooper wrote:
> > On 13/06/18 16:27, Juergen Gross wrote:
> 
> Looking more into the privcmd driver I'm rather sure now I can solve
> this via mmap() plus an ioctl(). So the kernel would allocate the
> bounce buffers and map them into user space. This will require changes
> only in libxencall without having to modify its external interface.
> 
> 
> It looks like David Vrabel sent in 2016 a patch implementing that
> solution [1]. It seems it has not been merged. Do you have any insight
> why it was not merged?

No. It had all needed Acks and there were no open questions.

Thanks for the reference, I think I can just reuse the patch after some
rebasing.


Juergen

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

Re: [Xen-devel] How to deal with hypercalls returning -EFAULT

2018-06-14 Thread Julien Grall 
Hi Juergen,

Sorry for the formatting.

On Thu, 14 Jun 2018, 07:40 Juergen Gross,  wrote:

> On 13/06/18 18:25, Andrew Cooper wrote:
> > On 13/06/18 16:27, Juergen Gross wrote:
>
> Looking more into the privcmd driver I'm rather sure now I can solve
> this via mmap() plus an ioctl(). So the kernel would allocate the
> bounce buffers and map them into user space. This will require changes
> only in libxencall without having to modify its external interface.
>

It looks like David Vrabel sent in 2016 a patch implementing that solution
[1]. It seems it has not been merged. Do you have any insight why it was
not merged?

Cheers,

[1]
https://lists.xenproject.org/archives/html/xen-devel/2016-08/msg00743.html
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] How to deal with hypercalls returning -EFAULT

2018-06-14 Thread Juergen Gross 
On 13/06/18 18:06, Ian Jackson wrote:
> Juergen Gross writes ("How to deal with hypercalls returning -EFAULT"):
>> 3. Modify the interface to the privcmd driver to pass information about
>>used buffers to the kernel in order to lock them there. Either add a
>>new interface for hypercall buffer management or add the list of
>>buffers to the privcmd ioctl data structure.
>>Pros: - problem is really solved
>>Cons: - split solution between kernel and Xen, both must be changed
> 
> I think this is the best approach.  There are presumably already
> internal kernel interfaces which could be used by privcmd to implement
> this.  All that's needed is to decide what the kernel API should look
> like and implement it.  libxc doesn't really care very much what that
> interface looks like, so it can be whatever is convenient for the
> kernel.
> 
>> 2. Add a bandaid to Xen tools by retrying hypercalls which have failed
>>with -EFAULT (either for all or only for some hypercalls)
>>Pros: - no interface change necessary
>>Cons: - not all hypercalls might be just repeatable
>>  - problem isn't solved but just worked around
> 
> This may allow us to make some kind of progress in systems which are
> fundamentally broken, but I don't think it is a tolerable long term
> approach.
> 
> If we do this, we should also do your option (3), and the workaround
> should only be enabled if the proper interface is not available.

Of course.


Juergen

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

Re: [Xen-devel] How to deal with hypercalls returning -EFAULT

2018-06-14 Thread Juergen Gross 
On 13/06/18 18:25, Andrew Cooper wrote:
> On 13/06/18 16:27, Juergen Gross wrote:
>> Currently the release of Xen 4.11 is blocked due to a sporadic failure
>> of the OSSTEST guest-saverestore[.2]. During that test a hypercall
>> issued by libxc via the Linux privcmd driver returns -EFAULT in spite
>> of all hypercall buffers locked in memory via mlock() (or similar flags
>> specified in a mmap() call).
>>
>> My analysis has revealed that modern Linux kernels might make such
>> locked user pages unaccessible for very short periods of time. This can
>> happen e.g. when pages are subject to compaction or migration.
>>
>> There are multiple ways to mitigate this problem:
>>
>> 1. Trying to switch page migration or compaction off in dom0.
>>Pros: - no change in Xen necessary
> 
> Pro: can likely retrofitted to existing environments without further
> code changes.
> 
> (Not that I disagree with your Con's in this case)
> 
>>Cons: - new cases might come up in the future
>>  - easy to miss, failures are really very sporadic and might
>>happen only after updating the kernel
>>
>> 2. Add a bandaid to Xen tools by retrying hypercalls which have failed
>>with -EFAULT (either for all or only for some hypercalls)
>>Pros: - no interface change necessary
>>Cons: - not all hypercalls might be just repeatable
>>  - problem isn't solved but just worked around
> 
> We'd have to whitelist hypercalls which are safe to repeat like this. 

Right.

> Most wont be.  Any mutable operation which -EFAULTs can't safely be
> restarted, because we can't distinguish an early fault (Xen reading the
> parameters) from a late fault (Xen trying to update a userspace pointer
> with the result).

Even some of the late fault cases are repeatable.

> 
>>
>> 3. Modify the interface to the privcmd driver to pass information about
>>used buffers to the kernel in order to lock them there. Either add a
>>new interface for hypercall buffer management or add the list of
>>buffers to the privcmd ioctl data structure.
>>Pros: - problem is really solved
>>Cons: - split solution between kernel and Xen, both must be changed
> 
> To be clear, you mean suggesting changing libxc here, rather than the
> hypervisor?

Yes.

> Getting this problem fixed properly would be a distinct improvement over
> the whack-a-mole which has been played in the past.

Looking more into the privcmd driver I'm rather sure now I can solve
this via mmap() plus an ioctl(). So the kernel would allocate the
bounce buffers and map them into user space. This will require changes
only in libxencall without having to modify its external interface.

> 
>>
>> 4. Modify the interface between hypervisor and kernel: instead of just
>>returning -EFAULT let the hypervisor behave more like copy_to_user by
>>raising a page fault which can then be fixed up in the kernel. This
>>change must be activated by the kernel, of course.
>>Pros: - rather simple change in the kernel "doing the right thing"
>>  - hypercall bounce buffer handling in libxc/libxencall can be
>>switched off for a kernel supporting this chnage
>>Cons: - split solution between kernel and Xen, both must be changed
>>  - not sure how complex the required hypervisor change will be
> 
> Sadly, as I've just realised...
> 
> Con: Cannot be used to replace all -EFAULTs.
> 
> Faults when copying data in can be resolved by passing #PF to the
> kernel, but faults when trying to update guest state (continuation, or
> completion information) cannot be safely resumed at a later point.

Hmm, seems you are right. Sigh.

> 
>>
>> It should be noted that we can either select only one of above solutions
>> or one of 3/4 and additionally one of 1/2 as a fallback for old kernels.
>>
>> How to proceed?
> 
> Much as I hate to say it (as I do like this idea), I don't idea 4 is a
> viable alternative to 3.

Yeah, but idea 3 via mmap() isn't too bad in the end...


Juergen


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

Re: [Xen-devel] How to deal with hypercalls returning -EFAULT

2018-06-13 Thread Andrew Cooper 
On 13/06/18 16:27, Juergen Gross wrote:
> Currently the release of Xen 4.11 is blocked due to a sporadic failure
> of the OSSTEST guest-saverestore[.2]. During that test a hypercall
> issued by libxc via the Linux privcmd driver returns -EFAULT in spite
> of all hypercall buffers locked in memory via mlock() (or similar flags
> specified in a mmap() call).
>
> My analysis has revealed that modern Linux kernels might make such
> locked user pages unaccessible for very short periods of time. This can
> happen e.g. when pages are subject to compaction or migration.
>
> There are multiple ways to mitigate this problem:
>
> 1. Trying to switch page migration or compaction off in dom0.
>Pros: - no change in Xen necessary

Pro: can likely retrofitted to existing environments without further
code changes.

(Not that I disagree with your Con's in this case)

>Cons: - new cases might come up in the future
>  - easy to miss, failures are really very sporadic and might
>happen only after updating the kernel
>
> 2. Add a bandaid to Xen tools by retrying hypercalls which have failed
>with -EFAULT (either for all or only for some hypercalls)
>Pros: - no interface change necessary
>Cons: - not all hypercalls might be just repeatable
>  - problem isn't solved but just worked around

We'd have to whitelist hypercalls which are safe to repeat like this. 
Most wont be.  Any mutable operation which -EFAULTs can't safely be
restarted, because we can't distinguish an early fault (Xen reading the
parameters) from a late fault (Xen trying to update a userspace pointer
with the result).

>
> 3. Modify the interface to the privcmd driver to pass information about
>used buffers to the kernel in order to lock them there. Either add a
>new interface for hypercall buffer management or add the list of
>buffers to the privcmd ioctl data structure.
>Pros: - problem is really solved
>Cons: - split solution between kernel and Xen, both must be changed

To be clear, you mean suggesting changing libxc here, rather than the
hypervisor?

Getting this problem fixed properly would be a distinct improvement over
the whack-a-mole which has been played in the past.

>
> 4. Modify the interface between hypervisor and kernel: instead of just
>returning -EFAULT let the hypervisor behave more like copy_to_user by
>raising a page fault which can then be fixed up in the kernel. This
>change must be activated by the kernel, of course.
>Pros: - rather simple change in the kernel "doing the right thing"
>  - hypercall bounce buffer handling in libxc/libxencall can be
>switched off for a kernel supporting this chnage
>Cons: - split solution between kernel and Xen, both must be changed
>  - not sure how complex the required hypervisor change will be

Sadly, as I've just realised...

Con: Cannot be used to replace all -EFAULTs.

Faults when copying data in can be resolved by passing #PF to the
kernel, but faults when trying to update guest state (continuation, or
completion information) cannot be safely resumed at a later point.

>
> It should be noted that we can either select only one of above solutions
> or one of 3/4 and additionally one of 1/2 as a fallback for old kernels.
>
> How to proceed?

Much as I hate to say it (as I do like this idea), I don't idea 4 is a
viable alternative to 3.

~Andrew

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

Re: [Xen-devel] How to deal with hypercalls returning -EFAULT

2018-06-13 Thread Ian Jackson 
Juergen Gross writes ("How to deal with hypercalls returning -EFAULT"):
> 3. Modify the interface to the privcmd driver to pass information about
>used buffers to the kernel in order to lock them there. Either add a
>new interface for hypercall buffer management or add the list of
>buffers to the privcmd ioctl data structure.
>Pros: - problem is really solved
>Cons: - split solution between kernel and Xen, both must be changed

I think this is the best approach.  There are presumably already
internal kernel interfaces which could be used by privcmd to implement
this.  All that's needed is to decide what the kernel API should look
like and implement it.  libxc doesn't really care very much what that
interface looks like, so it can be whatever is convenient for the
kernel.

> 2. Add a bandaid to Xen tools by retrying hypercalls which have failed
>with -EFAULT (either for all or only for some hypercalls)
>Pros: - no interface change necessary
>Cons: - not all hypercalls might be just repeatable
>  - problem isn't solved but just worked around

This may allow us to make some kind of progress in systems which are
fundamentally broken, but I don't think it is a tolerable long term
approach.

If we do this, we should also do your option (3), and the workaround
should only be enabled if the proper interface is not available.

Thanks,
Ian.

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