On 17.02.20 10:14, David Hildenbrand wrote: > On 14.02.20 23:26, Christian Borntraeger wrote: >> From: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx> >> >> 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.