2020-07-14 11:41:13

by Pierre Morel

[permalink] [raw]
Subject: [PATCH v6 0/2] s390: virtio: let arch validate VIRTIO features

Hi all,

The goal of the series is to give a chance to the architecture
to validate VIRTIO device features.

in this respin:

1) I kept removed the ack from Jason as I reworked the patch
@Jason, the nature and goal of the patch did not really changed
please can I get back your acked-by with these changes?

2) Rewording for warning messages

Regards,
Pierre


Pierre Morel (2):
virtio: let arch validate VIRTIO features
s390: virtio: PV needs VIRTIO I/O device protection

arch/s390/mm/init.c | 28 ++++++++++++++++++++++++++++
drivers/virtio/virtio.c | 19 +++++++++++++++++++
include/linux/virtio_config.h | 1 +
3 files changed, 48 insertions(+)

--
2.25.1

Changelog

to v6:

- rewording warning messages
(Connie, Halil)

to v5:

- return directly from S390 arch_validate_virtio_features()
when the guest is not protected.
(Connie)

- Somme rewording
(Connie, Michael)

- moved back code from arch/s390/ ...kernel/uv.c to ...mm/init.c
(Christian)

to v4:

- separate virtio and arch code
(Pierre)

- moved code from arch/s390/mm/init.c to arch/s390/kernel/uv.c
(as interpreted from Heiko's comment)

- moved validation inside the arch code
(Connie)

- moved the call to arch validation before VIRTIO_F_1 test
(Michael)

to v3:

- add warning
(Connie, Christian)

- add comment
(Connie)

- change hook name
(Halil, Connie)

to v2:

- put the test in virtio_finalize_features()
(Connie)

- put the test inside VIRTIO core
(Jason)

- pass a virtio device as parameter
(Halil)



2020-07-14 11:42:04

by Pierre Morel

[permalink] [raw]
Subject: [PATCH v6 2/2] s390: virtio: PV needs VIRTIO I/O device protection

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 <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
Acked-by: Halil Pasic <[email protected]>
---
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..26efb663bac2 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 virtualizatio\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)
{
--
2.25.1

2020-07-14 11:45:47

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v6 2/2] s390: virtio: PV needs VIRTIO I/O device protection


On 14.07.20 13:38, 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 <[email protected]>
> Reviewed-by: Cornelia Huck <[email protected]>
> Acked-by: Halil Pasic <[email protected]>

We will probably move this (and other related code) into a new file,
but we can do that later.
As for now:

Acked-by: Christian Borntraeger <[email protected]>

> ---
> 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..26efb663bac2 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 virtualizatio\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)
> {
>

2020-07-14 11:53:37

by Pierre Morel

[permalink] [raw]
Subject: Re: [PATCH v6 2/2] s390: virtio: PV needs VIRTIO I/O device protection



On 2020-07-14 13:42, Christian Borntraeger wrote:
>
> On 14.07.20 13:38, 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 <[email protected]>
>> Reviewed-by: Cornelia Huck <[email protected]>
>> Acked-by: Halil Pasic <[email protected]>
>
> We will probably move this (and other related code) into a new file,
> but we can do that later.
> As for now:
>
> Acked-by: Christian Borntraeger <[email protected]>
>

Thanks,
Pierre

--
Pierre Morel
IBM Lab Boeblingen

2020-07-14 11:57:39

by Cornelia Huck

[permalink] [raw]
Subject: Re: [PATCH v6 2/2] s390: virtio: PV needs VIRTIO I/O device protection

On Tue, 14 Jul 2020 13:38:02 +0200
Pierre Morel <[email protected]> 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 <[email protected]>
> Reviewed-by: Cornelia Huck <[email protected]>
> Acked-by: Halil Pasic <[email protected]>
> ---
> arch/s390/mm/init.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)

(...)

> +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 virtualizatio\n");

typo: s/virtualizatio/virtualization/

> + 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)
> {