| Message ID | 20190404231622.52531-1-pasic@linux.ibm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | s390: virtio: support protected virtualization | expand |
On Fri, 5 Apr 2019 01:16:10 +0200 Halil Pasic <pasic@linux.ibm.com> wrote: > Enhanced virtualization protection technology may require the use of > bounce buffers for I/O. While support for this was built into the virtio > core, virtio-ccw wasn't changed accordingly. > > Thus what needs to be done to bring virtio-ccw up to speed with respect > to this is: > * use some 'new' common virtio stuff > * make sure that virtio-ccw specific stuff uses shared memory when > talking to the hypervisor (except communication blocks like ORB, these > are handled by the hypervisor) > * make sure the DMA API does what is necessary to talk through shared > memory if we are a protected virtualization guest. > * make sure the common IO layer plays along as well (airqs, sense). It would be good to have a summary somewhere in the code (or Documentation/) as to what needs the dma treatment and what doesn't, for later reference. We don't want people to accidentally break things (especially if they cannot refer to architecture documentation - or will at least some of that be published?) > > The series is structured in incremental fashion: some of the changes are > overridden by following patches. The main reason why is that this is how I > developed. But I think it ain't bad for the didactic and we are a bit more > flexible with regards to throwing out some of the stuff in the end. FWIW, I think reshuffling the patches in the next iteration would ease review. > > Important notes: > > * This is an early (WIP) RFC that does not add any function to the > kernel at his stage, as the ultravisor interactions are left out. > The purpose is getting some early feedback ASAP. I would like some comments from people who have experience with the dma api. > > * In future these patches will depend on some code interacting with the > ultravisor (WIP by Vasily and Janosch). > > * The s390 names are by no means final, and are not properly explained. Should > not hamper understanding too much. If it does please ask. > > * The existing naming in the common infrastructure (kernel internal > interfaces) is pretty much based on the AMD SEV terminology. Thus the > names aren't always perfect. There might be merit to changing these > names to more abstract ones. I did not put much thought into that at > the current stage. If we can find some generic names that work well for everyone, converting seems like a good idea. But following SEV is not that bad, either (you'll probably find more people who have heard about SEV than folks familiar with s390 ;) > > > Testing: > > Please use iommu_platform=on for any virtio devices you are going > to test this code with (so virtio actually uses the DMA API). > > Looking forward to your review or any other type of input. I have now read through the whole series and commented in some places. But I'd really like to see comments from others as well.
On Wed, 10 Apr 2019 11:20:48 +0200 Cornelia Huck <cohuck@redhat.com> wrote: > On Fri, 5 Apr 2019 01:16:10 +0200 > Halil Pasic <pasic@linux.ibm.com> wrote: > > > Enhanced virtualization protection technology may require the use of > > bounce buffers for I/O. While support for this was built into the > > virtio core, virtio-ccw wasn't changed accordingly. > > > > Thus what needs to be done to bring virtio-ccw up to speed with > > respect to this is: > > * use some 'new' common virtio stuff > > * make sure that virtio-ccw specific stuff uses shared memory when > > talking to the hypervisor (except communication blocks like ORB, > > these are handled by the hypervisor) > > * make sure the DMA API does what is necessary to talk through shared > > memory if we are a protected virtualization guest. > > * make sure the common IO layer plays along as well (airqs, sense). > > It would be good to have a summary somewhere in the code (or > Documentation/) as to what needs the dma treatment and what doesn't, > for later reference. We don't want people to accidentally break things > (especially if they cannot refer to architecture documentation - or > will at least some of that be published?) > I can put documentation on my TODO list. This cover letter was also supposed to provide a bird's-eye view on what needs to be done. > > > > The series is structured in incremental fashion: some of the changes > > are overridden by following patches. The main reason why is that > > this is how I developed. But I think it ain't bad for the didactic > > and we are a bit more flexible with regards to throwing out some of > > the stuff in the end. > > FWIW, I think reshuffling the patches in the next iteration would ease > review. > Can you please tell me more about what is desired here? I mean, I made some tentative proposals on squashing some patches together. I don't remember any requests to reorder patches or split. > > > > Important notes: > > > > * This is an early (WIP) RFC that does not add any function to the > > kernel at his stage, as the ultravisor interactions are left out. > > The purpose is getting some early feedback ASAP. > > I would like some comments from people who have experience with the dma > api. > I can only second that! From the Linux on Z folks Sebastian seems to be the most likely suspect. I've briefly shown this series to him before posting here, and I asked him to have another look today. Unfortunately he seems quite busy at the moment, but he promised me that he will make some time for this (soonish). Getting some more feedback from Christoph Hellwig would be nice as well. > > > > * In future these patches will depend on some code interacting with > > the ultravisor (WIP by Vasily and Janosch). > > > > * The s390 names are by no means final, and are not properly > > explained. Should not hamper understanding too much. If it does > > please ask. > > > > * The existing naming in the common infrastructure (kernel internal > > interfaces) is pretty much based on the AMD SEV terminology. Thus > > the names aren't always perfect. There might be merit to changing > > these names to more abstract ones. I did not put much thought into > > that at the current stage. > > If we can find some generic names that work well for everyone, > converting seems like a good idea. But following SEV is not that bad, > either (you'll probably find more people who have heard about SEV than > folks familiar with s390 ;) > I agree. We should not swap out the names for worse ones for sure. :) I think the names discussion can be postponed. I would like to get the code right first. > > > > > > Testing: > > > > Please use iommu_platform=on for any virtio devices you are going > > to test this code with (so virtio actually uses the DMA API). > > > > Looking forward to your review or any other type of input. > > I have now read through the whole series and commented in some places. > But I'd really like to see comments from others as well. > Thank you so much for the quick review. Thanks to your quality review I will have a bunch of things to change (for the better) for v1. I intend to wait a bit for more feedback and also for the my dependencies to get published. I hope to send out a functional v1 mid next week. Thank you again! Regards, Halil
On Wed, 10 Apr 2019 17:57:50 +0200 Halil Pasic <pasic@linux.ibm.com> wrote: > On Wed, 10 Apr 2019 11:20:48 +0200 > Cornelia Huck <cohuck@redhat.com> wrote: > > > On Fri, 5 Apr 2019 01:16:10 +0200 > > Halil Pasic <pasic@linux.ibm.com> wrote: > > > > > Enhanced virtualization protection technology may require the use of > > > bounce buffers for I/O. While support for this was built into the > > > virtio core, virtio-ccw wasn't changed accordingly. > > > > > > Thus what needs to be done to bring virtio-ccw up to speed with > > > respect to this is: > > > * use some 'new' common virtio stuff > > > * make sure that virtio-ccw specific stuff uses shared memory when > > > talking to the hypervisor (except communication blocks like ORB, > > > these are handled by the hypervisor) > > > * make sure the DMA API does what is necessary to talk through shared > > > memory if we are a protected virtualization guest. > > > * make sure the common IO layer plays along as well (airqs, sense). > > > > It would be good to have a summary somewhere in the code (or > > Documentation/) as to what needs the dma treatment and what doesn't, > > for later reference. We don't want people to accidentally break things > > (especially if they cannot refer to architecture documentation - or > > will at least some of that be published?) > > > > I can put documentation on my TODO list. This cover letter was also > supposed to provide a bird's-eye view on what needs to be done. Some comments in the code to prevent further headscratching are also a good idea. > > > > > > > The series is structured in incremental fashion: some of the changes > > > are overridden by following patches. The main reason why is that > > > this is how I developed. But I think it ain't bad for the didactic > > > and we are a bit more flexible with regards to throwing out some of > > > the stuff in the end. > > > > FWIW, I think reshuffling the patches in the next iteration would ease > > review. > > > > Can you please tell me more about what is desired here? I mean, I made > some tentative proposals on squashing some patches together. I don't > remember any requests to reorder patches or split. Just try to avoid to rewrite things multiple times -- if we agree on the end result, we should be able to go there directly :)
On 05.04.19 01:16, Halil Pasic wrote: > Enhanced virtualization protection technology may require the use of > bounce buffers for I/O. While support for this was built into the virtio > core, virtio-ccw wasn't changed accordingly. Can you elaborate some more about the general approach (Enhanced virtualization protection technology, ultravisor, concept, issues, how to squeeze it into QEMU/KVM/kernel) etc = For my taste, this cover letter misses some important context :) > > Thus what needs to be done to bring virtio-ccw up to speed with respect > to this is: > * use some 'new' common virtio stuff > * make sure that virtio-ccw specific stuff uses shared memory when > talking to the hypervisor (except communication blocks like ORB, these > are handled by the hypervisor) > * make sure the DMA API does what is necessary to talk through shared > memory if we are a protected virtualization guest. > * make sure the common IO layer plays along as well (airqs, sense). > > The series is structured in incremental fashion: some of the changes are > overridden by following patches. The main reason why is that this is how I > developed. But I think it ain't bad for the didactic and we are a bit more > flexible with regards to throwing out some of the stuff in the end. > > Important notes: > > * This is an early (WIP) RFC that does not add any function to the > kernel at his stage, as the ultravisor interactions are left out. > The purpose is getting some early feedback ASAP. > > * In future these patches will depend on some code interacting with the > ultravisor (WIP by Vasily and Janosch). > > * The s390 names are by no means final, and are not properly explained. Should > not hamper understanding too much. If it does please ask. > > * The existing naming in the common infrastructure (kernel internal > interfaces) is pretty much based on the AMD SEV terminology. Thus the > names aren't always perfect. There might be merit to changing these > names to more abstract ones. I did not put much thought into that at > the current stage. > > > Testing: > > Please use iommu_platform=on for any virtio devices you are going > to test this code with (so virtio actually uses the DMA API). > > Looking forward to your review or any other type of input. > > Halil Pasic (12): > virtio/s390: use vring_create_virtqueue > virtio/s390: DMA support for virtio-ccw > s390/mm: force swiotlb for protected virtualization > s390/cio: introduce cio DMA pool > s390/cio: add protected virtualization support to cio > s390/airq: use DMA memory for adapter interrupts > virtio/s390: use DMA memory for ccw I/O > virtio/s390: add indirection to indicators access > virtio/s390: use DMA memory for notifiers > virtio/s390: consolidate DMA allocations > virtio/s390: use the cio DMA pool > virtio/s390: make airq summary indicators DMA > > arch/s390/Kconfig | 5 + > arch/s390/include/asm/Kbuild | 1 - > arch/s390/include/asm/airq.h | 2 + > arch/s390/include/asm/cio.h | 4 + > arch/s390/include/asm/dma-mapping.h | 13 ++ > arch/s390/include/asm/mem_encrypt.h | 18 +++ > arch/s390/mm/init.c | 44 +++++ > drivers/s390/cio/airq.c | 18 ++- > drivers/s390/cio/ccwreq.c | 8 +- > drivers/s390/cio/css.c | 63 ++++++++ > drivers/s390/cio/device.c | 46 ++++-- > drivers/s390/cio/device_fsm.c | 40 ++--- > drivers/s390/cio/device_id.c | 18 +-- > drivers/s390/cio/device_ops.c | 4 +- > drivers/s390/cio/device_pgid.c | 20 +-- > drivers/s390/cio/device_status.c | 24 +-- > drivers/s390/cio/io_sch.h | 19 ++- > drivers/s390/virtio/virtio_ccw.c | 310 ++++++++++++++++++++---------------- > 18 files changed, 444 insertions(+), 213 deletions(-) > create mode 100644 arch/s390/include/asm/dma-mapping.h > create mode 100644 arch/s390/include/asm/mem_encrypt.h >
On Fri, 12 Apr 2019 15:47:50 +0200 David Hildenbrand <david@redhat.com> wrote: > On 05.04.19 01:16, Halil Pasic wrote: > > Enhanced virtualization protection technology may require the use of > > bounce buffers for I/O. While support for this was built into the virtio > > core, virtio-ccw wasn't changed accordingly. > > Can you elaborate some more about the general approach (Enhanced > virtualization protection technology, ultravisor, concept, issues, how > to squeeze it into QEMU/KVM/kernel) etc = > > For my taste, this cover letter misses some important context :) > I'm aware. Unfortunately we don't have a decision yet about which parts of the protected virtualization architecture are going to be PoP material. You can get some more context immediately by having a look at Martin's features branch and looking at the s390/protvirt and s390/uv patches. I will try to provide more background information for v1. But having a remotely complete and reliable documentation will take some time. What I can offer at the moment is answers to specific questions. Regards, Halil
On 16.04.19 13:10, Halil Pasic wrote: > On Fri, 12 Apr 2019 15:47:50 +0200 > David Hildenbrand <david@redhat.com> wrote: > >> On 05.04.19 01:16, Halil Pasic wrote: >>> Enhanced virtualization protection technology may require the use of >>> bounce buffers for I/O. While support for this was built into the virtio >>> core, virtio-ccw wasn't changed accordingly. >> >> Can you elaborate some more about the general approach (Enhanced >> virtualization protection technology, ultravisor, concept, issues, how >> to squeeze it into QEMU/KVM/kernel) etc = >> >> For my taste, this cover letter misses some important context :) >> > > I'm aware. Unfortunately we don't have a decision yet about which parts > of the protected virtualization architecture are going to be PoP > material. Oh, okay. > > You can get some more context immediately by having a look at Martin's > features branch and looking at the s390/protvirt and s390/uv patches. As I don't have time to dig through random branches to discover/reverse-engineer the obvious, I won't be reviewing this patch series. But as I am not an I/O expert, this might not be bad at all :) Maybe other people can help. > > I will try to provide more background information for v1. But having > a remotely complete and reliable documentation will take some time. What > I can offer at the moment is answers to specific questions. Waiting for v1 then. Cheers!