From: Juergen Gross <jgross@suse.com>
To: Stefano Stabellini <sstabellini@kernel.org>,
Oleksandr <olekstysh@gmail.com>
Cc: xen-devel@lists.xenproject.org, x86@kernel.org,
linux-kernel@vger.kernel.org,
Dave Hansen <dave.hansen@linux.intel.com>,
Andy Lutomirski <luto@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
"H. Peter Anvin" <hpa@zytor.com>,
Boris Ostrovsky <boris.ostrovsky@oracle.com>,
Julien Grall <julien@xen.org>,
Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
linux-arm-kernel@lists.infradead.org,
Christoph Hellwig <hch@infradead.org>,
"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [RFC PATCH 2/6] virtio: add option to restrict memory access under Xen
Date: Tue, 19 Apr 2022 08:21:28 +0200 [thread overview]
Message-ID: <ef637f17-0e9c-2f86-218b-918297cb9930@suse.com> (raw)
In-Reply-To: <alpine.DEB.2.22.394.2204181151030.915916@ubuntu-linux-20-04-desktop>
[-- Attachment #1.1.1: Type: text/plain, Size: 8272 bytes --]
On 18.04.22 21:11, Stefano Stabellini wrote:
> On Sun, 17 Apr 2022, Oleksandr wrote:
>> On 16.04.22 01:01, Stefano Stabellini wrote:
>>> On Thu, 14 Apr 2022, Oleksandr Tyshchenko wrote:
>>>> From: Juergen Gross <jgross@suse.com>
>>>>
>>>> In order to support virtio in Xen guests add a config option enabling
>>>> the user to specify whether in all Xen guests virtio should be able to
>>>> access memory via Xen grant mappings only on the host side.
>>>>
>>>> This applies to fully virtualized guests only, as for paravirtualized
>>>> guests this is mandatory.
>>>>
>>>> This requires to switch arch_has_restricted_virtio_memory_access()
>>>> from a pure stub to a real function on x86 systems (Arm systems are
>>>> not covered by now).
>>>>
>>>> Add the needed functionality by providing a special set of DMA ops
>>>> handling the needed grant operations for the I/O pages.
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> ---
>>>> arch/x86/mm/init.c | 15 ++++
>>>> arch/x86/mm/mem_encrypt.c | 5 --
>>>> arch/x86/xen/Kconfig | 9 +++
>>>> drivers/xen/Kconfig | 20 ++++++
>>>> drivers/xen/Makefile | 1 +
>>>> drivers/xen/xen-virtio.c | 177
>>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>> include/xen/xen-ops.h | 8 +++
>>>> 7 files changed, 230 insertions(+), 5 deletions(-)
>>>> create mode 100644 drivers/xen/xen-virtio.c
>>>>
>>>> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
>>>> index d8cfce2..526a3b2 100644
>>>> --- a/arch/x86/mm/init.c
>>>> +++ b/arch/x86/mm/init.c
>>>> @@ -8,6 +8,8 @@
>>>> #include <linux/kmemleak.h>
>>>> #include <linux/sched/task.h>
>>>> +#include <xen/xen.h>
>>>> +
>>>> #include <asm/set_memory.h>
>>>> #include <asm/e820/api.h>
>>>> #include <asm/init.h>
>>>> @@ -1065,3 +1067,16 @@ unsigned long max_swapfile_size(void)
>>>> return pages;
>>>> }
>>>> #endif
>>>> +
>>>> +#ifdef CONFIG_ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
>>>> +int arch_has_restricted_virtio_memory_access(void)
>>>> +{
>>>> + if (IS_ENABLED(CONFIG_XEN_PV_VIRTIO) && xen_pv_domain())
>>>> + return 1;
>>>> + if (IS_ENABLED(CONFIG_XEN_HVM_VIRTIO_GRANT) && xen_hvm_domain())
>>>> + return 1;
>>> I think these two checks could be moved to a separate function in a Xen
>>> header, e.g. xen_restricted_virtio_memory_access, and here you could
>>> just
>>>
>>> if (xen_restricted_virtio_memory_access())
>>> return 1;
>>
>> Agree, will do
>>
>>
>>>
>>>
>>>
>>>> + return cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(arch_has_restricted_virtio_memory_access);
>>>> +#endif
>>>> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
>>>> index 50d2099..dda020f 100644
>>>> --- a/arch/x86/mm/mem_encrypt.c
>>>> +++ b/arch/x86/mm/mem_encrypt.c
>>>> @@ -77,8 +77,3 @@ void __init mem_encrypt_init(void)
>>>> print_mem_encrypt_feature_info();
>>>> }
>>>> -int arch_has_restricted_virtio_memory_access(void)
>>>> -{
>>>> - return cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT);
>>>> -}
>>>> -EXPORT_SYMBOL_GPL(arch_has_restricted_virtio_memory_access);
>>>> diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
>>>> index 85246dd..dffdffd 100644
>>>> --- a/arch/x86/xen/Kconfig
>>>> +++ b/arch/x86/xen/Kconfig
>>>> @@ -92,3 +92,12 @@ config XEN_DOM0
>>>> select X86_X2APIC if XEN_PVH && X86_64
>>>> help
>>>> Support running as a Xen Dom0 guest.
>>>> +
>>>> +config XEN_PV_VIRTIO
>>>> + bool "Xen virtio support for PV guests"
>>>> + depends on XEN_VIRTIO && XEN_PV
>>>> + default y
>>>> + help
>>>> + Support virtio for running as a paravirtualized guest. This will
>>>> + need support on the backend side (qemu or kernel, depending on the
>>>> + virtio device types used).
>>>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
>>>> index 120d32f..fc61f7a 100644
>>>> --- a/drivers/xen/Kconfig
>>>> +++ b/drivers/xen/Kconfig
>>>> @@ -335,4 +335,24 @@ config XEN_UNPOPULATED_ALLOC
>>>> having to balloon out RAM regions in order to obtain physical memory
>>>> space to create such mappings.
>>>> +config XEN_VIRTIO
>>>> + bool "Xen virtio support"
>>>> + default n
>>>> + depends on VIRTIO && DMA_OPS
>>>> + select ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
>>>> + help
>>>> + Enable virtio support for running as Xen guest. Depending on the
>>>> + guest type this will require special support on the backend side
>>>> + (qemu or kernel, depending on the virtio device types used).
>>>> +
>>>> +config XEN_HVM_VIRTIO_GRANT
>>>> + bool "Require virtio for fully virtualized guests to use grant
>>>> mappings"
>>>> + depends on XEN_VIRTIO && X86_64
>>>> + default y
>>>> + help
>>>> + Require virtio for fully virtualized guests to use grant mappings.
>>>> + This will avoid the need to give the backend the right to map all
>>>> + of the guest memory. This will need support on the backend side
>>>> + (qemu or kernel, depending on the virtio device types used).
>>> I don't think we need 3 visible kconfig options for this.
>>>
>>> In fact, I would only add one: XEN_VIRTIO. We can have any X86 (or ARM)
>>> specific dependencies in the "depends" line under XEN_VIRTIO. And I
>>> don't think we need XEN_HVM_VIRTIO_GRANT as a kconfig option
>>> necessarely. It doesn't seem like some we want as build time option. At
>>> most, it could be a runtime option (like a command line) or a debug
>>> option (like an #define at the top of the source file.)
>>
>>
>> I don't know what was the initial idea of having and extra XEN_HVM_VIRTIO and
>> XEN_PV_VIRTIO options, but taking into the account that
>> they are only used in arch_has_restricted_virtio_memory_access() currently, I
>> share your opinion regarding a single XEN_VIRTIO option.
>>
>> Looking ahead (including changes in the commit #4), we can imagine the
>> resulting option:
>>
>> config XEN_VIRTIO
>> bool "Xen virtio support"
>> default n
>> depends on VIRTIO && DMA_OPS
>> depends on (X86_64 || ARM || ARM64)
>> select ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
>> help
>> Enable virtio support for running as Xen guest. Depending on the
>> guest type this will require special support on the backend side
>> (qemu or kernel, depending on the virtio device types used).
>>
>>
>> and then arch_has_restricted_virtio_memory_access() per arch:
>>
>>
>> 1. x86:
>>
>> int arch_has_restricted_virtio_memory_access(void)
>> {
>> return (xen_has_restricted_virtio_memory_access() ||
>> cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT));
>> }
>>
>>
>> 2. Arm:
>>
>> int arch_has_restricted_virtio_memory_access(void)
>> {
>> return xen_has_restricted_virtio_memory_access();
>> }
>>
>>
>> 3. xen.h:
>>
>> static inline int xen_has_restricted_virtio_memory_access(void)
>> {
>> if (IS_ENABLED(CONFIG_XEN_VIRTIO) && (xen_pv_domain() ||
>> xen_hvm_domain()))
>> return 1;
>>
>> return 0;
>> }
>>
>>
>> Actually, as domain type on Arm is always XEN_HVM_DOMAIN, we could probably
>> have the following on Arm:
>>
>> int arch_has_restricted_virtio_memory_access(void)
>> {
>> return IS_ENABLED(CONFIG_XEN_VIRTIO);
>> }
>>
>> but I would prefer not to diverge and use common
>> xen_has_restricted_virtio_memory_access().
>>
>> Any thoughts?
>
> Yes, I would also prefer not to diverge between the x86 and arm versions
> of xen_has_restricted_virtio_memory_access. But what case are we trying
> to catch with (xen_pv_domain() || xen_hvm_domain()) ? Even on x86, it is
> not going to leave much out. Is it really meant only to exclude pvh
> domains?
It wouldn't exclude pvh domains.
>
> I have the feeling that we could turn this check into:
>
> static inline int xen_has_restricted_virtio_memory_access(void)
> {
> return IS_ENABLED(CONFIG_XEN_VIRTIO) && xen_domain();
> }
>
> even on x86, but one of the xen/x86 maintainers should confirm.
I do confirm this is better and functionally equivalent.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
next prev parent reply other threads:[~2022-04-19 6:21 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-14 19:19 [RFC PATCH 0/6] virtio: Solution to restrict memory access under Xen using xen-virtio DMA ops layer Oleksandr Tyshchenko
2022-04-14 19:19 ` [RFC PATCH 1/6] xen/grants: support allocating consecutive grants Oleksandr Tyshchenko
2022-04-14 19:19 ` [RFC PATCH 2/6] virtio: add option to restrict memory access under Xen Oleksandr Tyshchenko
2022-04-14 19:43 ` H. Peter Anvin
2022-04-15 15:20 ` Oleksandr
2022-04-15 22:01 ` Stefano Stabellini
2022-04-17 17:02 ` Oleksandr
2022-04-18 19:11 ` Stefano Stabellini
2022-04-19 6:21 ` Juergen Gross [this message]
2022-04-19 6:37 ` Oleksandr
2022-04-14 19:19 ` [RFC PATCH 3/6] dt-bindings: xen: Add xen,dev-domid property description for xen-virtio layer Oleksandr Tyshchenko
2022-04-15 22:01 ` Stefano Stabellini
2022-04-17 17:24 ` Oleksandr
2022-04-14 19:19 ` [RFC PATCH 4/6] virtio: Various updates to xen-virtio DMA ops layer Oleksandr Tyshchenko
2022-04-15 22:02 ` Stefano Stabellini
2022-04-17 18:21 ` Oleksandr
2022-04-18 19:11 ` Stefano Stabellini
2022-04-19 6:58 ` Juergen Gross
2022-04-19 7:07 ` Oleksandr
2022-04-16 6:05 ` Christoph Hellwig
2022-04-17 18:39 ` Oleksandr
2022-04-14 19:19 ` [RFC PATCH 5/6] arm/xen: Introduce xen_setup_dma_ops() Oleksandr Tyshchenko
2022-04-15 22:02 ` Stefano Stabellini
2022-04-17 18:43 ` Oleksandr
2022-04-14 19:19 ` [RFC PATCH 6/6] arm/xen: Assign xen-virtio DMA ops for virtio devices in Xen guests Oleksandr Tyshchenko
2022-04-15 22:02 ` Stefano Stabellini
2022-04-16 6:07 ` Christoph Hellwig
2022-04-17 21:05 ` Oleksandr
2022-04-18 19:11 ` Stefano Stabellini
2022-04-19 12:17 ` Oleksandr
2022-04-19 14:48 ` Juergen Gross
2022-04-19 17:11 ` Oleksandr
2022-04-20 0:23 ` Stefano Stabellini
2022-04-20 9:00 ` Oleksandr
2022-04-20 22:49 ` Stefano Stabellini
2022-04-17 19:20 ` Oleksandr
2022-04-15 7:41 ` [RFC PATCH 0/6] virtio: Solution to restrict memory access under Xen using xen-virtio DMA ops layer Christoph Hellwig
2022-04-15 10:04 ` Oleksandr
2022-04-15 8:44 ` Michael S. Tsirkin
2022-04-15 15:29 ` Oleksandr
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=ef637f17-0e9c-2f86-218b-918297cb9930@suse.com \
--to=jgross@suse.com \
--cc=boris.ostrovsky@oracle.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=hch@infradead.org \
--cc=hpa@zytor.com \
--cc=julien@xen.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=mst@redhat.com \
--cc=oleksandr_tyshchenko@epam.com \
--cc=olekstysh@gmail.com \
--cc=peterz@infradead.org \
--cc=sstabellini@kernel.org \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
--cc=xen-devel@lists.xenproject.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).