From: Jason Wang <jasowang@redhat.com> To: "Michael S. Tsirkin" <mst@redhat.com>, Pierre Morel <pmorel@linux.ibm.com> Cc: linux-kernel@vger.kernel.org, pasic@linux.ibm.com, borntraeger@de.ibm.com, frankja@linux.ibm.com, cohuck@redhat.com, kvm@vger.kernel.org, linux-s390@vger.kernel.org, virtualization@lists.linux-foundation.org, thomas.lendacky@amd.com, david@gibson.dropbear.id.au, linuxram@us.ibm.com, hca@linux.ibm.com, gor@linux.ibm.com Subject: Re: [PATCH v7 2/2] s390: virtio: PV needs VIRTIO I/O device protection Date: Wed, 15 Jul 2020 18:16:59 +0800 [thread overview] Message-ID: <bc5e09ad-faaf-8b38-83e0-5f4a4b1daeb0@redhat.com> (raw) In-Reply-To: <20200715054807-mutt-send-email-mst@kernel.org> On 2020/7/15 下午5:50, Michael S. Tsirkin wrote: > On Wed, Jul 15, 2020 at 10:31:09AM +0200, Pierre Morel wrote: >> If protected virtualization is active on s390, the virtio queues are >> not accessible to the host, unless VIRTIO_F_IOMMU_PLATFORM has been >> negotiated. Use the new arch_validate_virtio_features() interface to >> fail probe if that's not the case, preventing a host error on access >> attempt. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >> Acked-by: Halil Pasic <pasic@linux.ibm.com> >> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> arch/s390/mm/init.c | 28 ++++++++++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> >> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c >> index 6dc7c3b60ef6..d39af6554d4f 100644 >> --- a/arch/s390/mm/init.c >> +++ b/arch/s390/mm/init.c >> @@ -45,6 +45,7 @@ >> #include <asm/kasan.h> >> #include <asm/dma-mapping.h> >> #include <asm/uv.h> >> +#include <linux/virtio_config.h> >> >> pgd_t swapper_pg_dir[PTRS_PER_PGD] __section(.bss..swapper_pg_dir); >> >> @@ -161,6 +162,33 @@ bool force_dma_unencrypted(struct device *dev) >> return is_prot_virt_guest(); >> } >> >> +/* >> + * arch_validate_virtio_features >> + * @dev: the VIRTIO device being added >> + * >> + * Return an error if required features are missing on a guest running >> + * with protected virtualization. >> + */ >> +int arch_validate_virtio_features(struct virtio_device *dev) >> +{ >> + if (!is_prot_virt_guest()) >> + return 0; >> + >> + if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1)) { >> + dev_warn(&dev->dev, >> + "legacy virtio not supported with protected virtualization\n"); >> + return -ENODEV; >> + } >> + >> + if (!virtio_has_feature(dev, VIRTIO_F_IOMMU_PLATFORM)) { >> + dev_warn(&dev->dev, >> + "support for limited memory access required for protected virtualization\n"); >> + return -ENODEV; >> + } >> + >> + return 0; >> +} >> + >> /* protected virtualization */ >> static void pv_init(void) >> { > What bothers me here is that arch code depends on virtio now. > It works even with a modular virtio when functions are inline, > but it seems fragile: e.g. it breaks virtio as an out of tree module, > since layout of struct virtio_device can change. The code was only called from virtio.c so it should be fine. And my understanding is that we don't need to care about the kABI issue during upstream development? Thanks > > I'm not sure what to do with this yet, will try to think about it > over the weekend. Thanks! > > >> -- >> 2.25.1
WARNING: multiple messages have this Message-ID (diff)
From: Jason Wang <jasowang@redhat.com> To: "Michael S. Tsirkin" <mst@redhat.com>, Pierre Morel <pmorel@linux.ibm.com> Cc: gor@linux.ibm.com, linux-s390@vger.kernel.org, frankja@linux.ibm.com, kvm@vger.kernel.org, thomas.lendacky@amd.com, hca@linux.ibm.com, cohuck@redhat.com, linuxram@us.ibm.com, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, pasic@linux.ibm.com, borntraeger@de.ibm.com, david@gibson.dropbear.id.au Subject: Re: [PATCH v7 2/2] s390: virtio: PV needs VIRTIO I/O device protection Date: Wed, 15 Jul 2020 18:16:59 +0800 [thread overview] Message-ID: <bc5e09ad-faaf-8b38-83e0-5f4a4b1daeb0@redhat.com> (raw) In-Reply-To: <20200715054807-mutt-send-email-mst@kernel.org> On 2020/7/15 下午5:50, Michael S. Tsirkin wrote: > On Wed, Jul 15, 2020 at 10:31:09AM +0200, Pierre Morel wrote: >> If protected virtualization is active on s390, the virtio queues are >> not accessible to the host, unless VIRTIO_F_IOMMU_PLATFORM has been >> negotiated. Use the new arch_validate_virtio_features() interface to >> fail probe if that's not the case, preventing a host error on access >> attempt. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >> Acked-by: Halil Pasic <pasic@linux.ibm.com> >> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> arch/s390/mm/init.c | 28 ++++++++++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> >> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c >> index 6dc7c3b60ef6..d39af6554d4f 100644 >> --- a/arch/s390/mm/init.c >> +++ b/arch/s390/mm/init.c >> @@ -45,6 +45,7 @@ >> #include <asm/kasan.h> >> #include <asm/dma-mapping.h> >> #include <asm/uv.h> >> +#include <linux/virtio_config.h> >> >> pgd_t swapper_pg_dir[PTRS_PER_PGD] __section(.bss..swapper_pg_dir); >> >> @@ -161,6 +162,33 @@ bool force_dma_unencrypted(struct device *dev) >> return is_prot_virt_guest(); >> } >> >> +/* >> + * arch_validate_virtio_features >> + * @dev: the VIRTIO device being added >> + * >> + * Return an error if required features are missing on a guest running >> + * with protected virtualization. >> + */ >> +int arch_validate_virtio_features(struct virtio_device *dev) >> +{ >> + if (!is_prot_virt_guest()) >> + return 0; >> + >> + if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1)) { >> + dev_warn(&dev->dev, >> + "legacy virtio not supported with protected virtualization\n"); >> + return -ENODEV; >> + } >> + >> + if (!virtio_has_feature(dev, VIRTIO_F_IOMMU_PLATFORM)) { >> + dev_warn(&dev->dev, >> + "support for limited memory access required for protected virtualization\n"); >> + return -ENODEV; >> + } >> + >> + return 0; >> +} >> + >> /* protected virtualization */ >> static void pv_init(void) >> { > What bothers me here is that arch code depends on virtio now. > It works even with a modular virtio when functions are inline, > but it seems fragile: e.g. it breaks virtio as an out of tree module, > since layout of struct virtio_device can change. The code was only called from virtio.c so it should be fine. And my understanding is that we don't need to care about the kABI issue during upstream development? Thanks > > I'm not sure what to do with this yet, will try to think about it > over the weekend. Thanks! > > >> -- >> 2.25.1 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next prev parent reply other threads:[~2020-07-15 10:17 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-15 8:31 [PATCH v7 0/2] s390: virtio: let arch validate VIRTIO features Pierre Morel 2020-07-15 8:31 ` [PATCH v7 1/2] " Pierre Morel 2020-07-15 8:31 ` [PATCH v7 2/2] s390: virtio: PV needs VIRTIO I/O device protection Pierre Morel 2020-07-15 9:50 ` Michael S. Tsirkin 2020-07-15 10:16 ` Jason Wang [this message] 2020-07-15 10:16 ` Jason Wang 2020-07-15 11:51 ` Michael S. Tsirkin 2020-07-16 11:19 ` Christian Borntraeger 2020-07-16 21:46 ` Michael S. Tsirkin 2020-07-22 11:48 ` Pierre Morel 2020-07-30 11:31 ` Pierre Morel 2020-08-06 14:19 ` Pierre Morel 2020-08-06 14:19 ` Pierre Morel 2020-07-15 8:36 ` [PATCH v7 0/2] s390: virtio: let arch validate VIRTIO features Jason Wang 2020-07-15 9:20 ` Cornelia Huck
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=bc5e09ad-faaf-8b38-83e0-5f4a4b1daeb0@redhat.com \ --to=jasowang@redhat.com \ --cc=borntraeger@de.ibm.com \ --cc=cohuck@redhat.com \ --cc=david@gibson.dropbear.id.au \ --cc=frankja@linux.ibm.com \ --cc=gor@linux.ibm.com \ --cc=hca@linux.ibm.com \ --cc=kvm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-s390@vger.kernel.org \ --cc=linuxram@us.ibm.com \ --cc=mst@redhat.com \ --cc=pasic@linux.ibm.com \ --cc=pmorel@linux.ibm.com \ --cc=thomas.lendacky@amd.com \ --cc=virtualization@lists.linux-foundation.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.