Re: [PATCH v2 01/42] mm:gup/writeback: add callbacks for inaccessible pages

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 17.02.20 10:14, David Hildenbrand wrote:
> On 14.02.20 23:26, Christian Borntraeger wrote:
>> From: Claudio Imbrenda <imbrenda@linux.ibm.com>
>>
>> With the introduction of protected KVM guests on s390 there is now a
>> concept of inaccessible pages. These pages need to be made accessible
>> before the host can access them.
>>
>> While cpu accesses will trigger a fault that can be resolved, I/O
>> accesses will just fail.  We need to add a callback into architecture
>> code for places that will do I/O, namely when writeback is started or
>> when a page reference is taken.
>>
>> This is not only to enable paging, file backing etc, it is also
>> necessary to protect the host against a malicious user space. For
>> example a bad QEMU could simply start direct I/O on such protected
>> memory.  We do not want userspace to be able to trigger I/O errors and
>> thus we the logic is "whenever somebody accesses that page (gup) or
>> doing I/O, make sure that this page can be accessed. When the guest
>> tries to access that page we will wait in the page fault handler for
>> writeback to have finished and for the page_ref to be the expected
>> value.
>>
>> If wanted by others, the callbacks can be extended with error handlin
>> and a parameter from where this is called.
> 
> s/handlin/handling/

ack. 
> 
> One last question from my side:
> 
> Why is it OK to ignore errors here. IOW, why not squash "[PATCH v2
> 39/42] example for future extension: mm:gup/writeback: add callbacks for
> inaccessible pages: error cases" into this patch.

I can certainly squash this in. But I have never heard feedback from the 
memory management people if they would prefer the patch as-is (no overhead
at all for !s390) or already with the error handling. 
> 
> I can see in patch "[PATCH v2 05/42] s390/mm: provide memory management
> functions for protected KVM guests", that the call can fail for various
> reasons. That puzzles me a bit - what would happen if any of that fails?

The convert _to_ secure has error handling. uv_convert_from_secure (the
one that is used for arch_make_page_accessible can only fail if we mess up,
e.g. an "export" on memory that we have donated to the ultravisor.


> Or will it actually never fail for s390x (and all that error handling in
> arch_make_page_accessible() is essentially dead code in real life?)

So yes, if everything is setup properly this should not fail in real life
and only we have a kernel (or firmware) bug.