| Message ID | 20220622063838.8854-4-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio: support requiring restricted access per device | expand |
On 22.06.22 09:38, Juergen Gross wrote: Hello Juergen > Commit fa1f57421e0b ("xen/virtio: Enable restricted memory access using > Xen grant mappings") introduced a new requirement for using virtio > devices: the backend now needs to support the VIRTIO_F_ACCESS_PLATFORM > feature. > > This is an undue requirement for non-PV guests, as those can be operated > with existing backends without any problem, as long as those backends > are running in dom0. > > Per default allow virtio devices without grant support for non-PV > guests. > > On Arm require VIRTIO_F_ACCESS_PLATFORM for devices having been listed > in the device tree to use grants. > > Add a new config item to always force use of grants for virtio. > > Fixes: fa1f57421e0b ("xen/virtio: Enable restricted memory access using Xen grant mappings") > Reported-by: Viresh Kumar <viresh.kumar@linaro.org> > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > V2: > - remove command line parameter (Christoph Hellwig) > V3: > - rebase to callback method Patch looks good, just one NIT ... > --- > arch/arm/xen/enlighten.c | 4 +++- > arch/x86/xen/enlighten_hvm.c | 4 +++- > arch/x86/xen/enlighten_pv.c | 5 ++++- > drivers/xen/Kconfig | 9 +++++++++ > drivers/xen/grant-dma-ops.c | 10 ++++++++++ > include/xen/xen-ops.h | 6 ++++++ > include/xen/xen.h | 8 -------- > 7 files changed, 35 insertions(+), 11 deletions(-) > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index 1f9c3ba32833..93c8ccbf2982 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -34,6 +34,7 @@ > #include <linux/timekeeping.h> > #include <linux/timekeeper_internal.h> > #include <linux/acpi.h> > +#include <linux/virtio_anchor.h> > > #include <linux/mm.h> > > @@ -443,7 +444,8 @@ static int __init xen_guest_init(void) > if (!xen_domain()) > return 0; > > - xen_set_restricted_virtio_memory_access(); > + if (IS_ENABLED(CONFIG_XEN_VIRTIO)) > + virtio_set_mem_acc_cb(xen_virtio_mem_acc); > > if (!acpi_disabled) > xen_acpi_guest_init(); > diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c > index 8b71b1dd7639..28762f800596 100644 > --- a/arch/x86/xen/enlighten_hvm.c > +++ b/arch/x86/xen/enlighten_hvm.c > @@ -4,6 +4,7 @@ > #include <linux/cpu.h> > #include <linux/kexec.h> > #include <linux/memblock.h> > +#include <linux/virtio_anchor.h> > > #include <xen/features.h> > #include <xen/events.h> > @@ -195,7 +196,8 @@ static void __init xen_hvm_guest_init(void) > if (xen_pv_domain()) > return; > > - xen_set_restricted_virtio_memory_access(); > + if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT)) > + virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc); > > init_hvm_pv_info(); > > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c > index e3297b15701c..5aaae8a77f55 100644 > --- a/arch/x86/xen/enlighten_pv.c > +++ b/arch/x86/xen/enlighten_pv.c > @@ -31,6 +31,7 @@ > #include <linux/gfp.h> > #include <linux/edd.h> > #include <linux/reboot.h> > +#include <linux/virtio_anchor.h> > > #include <xen/xen.h> > #include <xen/events.h> > @@ -109,7 +110,9 @@ static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc); > > static void __init xen_pv_init_platform(void) > { > - xen_set_restricted_virtio_memory_access(); > + /* PV guests can't operate virtio devices without grants. */ > + if (IS_ENABLED(CONFIG_XEN_VIRTIO)) > + virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc); > > populate_extra_pte(fix_to_virt(FIX_PARAVIRT_BOOTMAP)); > > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig > index bfd5f4f706bc..a65bd92121a5 100644 > --- a/drivers/xen/Kconfig > +++ b/drivers/xen/Kconfig > @@ -355,4 +355,13 @@ config XEN_VIRTIO > > If in doubt, say n. > > +config XEN_VIRTIO_FORCE_GRANT > + bool "Require Xen virtio support to use grants" > + depends on XEN_VIRTIO > + help > + Require virtio for Xen 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 > + (e.g. qemu or kernel, depending on the virtio device types used). > + > endmenu > diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > index fc0142484001..8973fc1e9ccc 100644 > --- a/drivers/xen/grant-dma-ops.c > +++ b/drivers/xen/grant-dma-ops.c > @@ -12,6 +12,8 @@ > #include <linux/of.h> > #include <linux/pfn.h> > #include <linux/xarray.h> > +#include <linux/virtio_anchor.h> > +#include <linux/virtio.h> > #include <xen/xen.h> > #include <xen/xen-ops.h> > #include <xen/grant_table.h> > @@ -287,6 +289,14 @@ bool xen_is_grant_dma_device(struct device *dev) > return has_iommu; > } > > +bool xen_virtio_mem_acc(struct virtio_device *dev) > +{ > + if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT)) > + return true; > + > + return xen_is_grant_dma_device(dev->dev.parent); > +} ... I am thinking would it be better to move this to xen/xen-ops.h as grant-dma-ops.c is generic (not only for virtio, although the virtio is the first use-case) diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c index 8973fc1..fc01424 100644 --- a/drivers/xen/grant-dma-ops.c +++ b/drivers/xen/grant-dma-ops.c @@ -12,8 +12,6 @@ #include <linux/of.h> #include <linux/pfn.h> #include <linux/xarray.h> -#include <linux/virtio_anchor.h> -#include <linux/virtio.h> #include <xen/xen.h> #include <xen/xen-ops.h> #include <xen/grant_table.h> @@ -289,14 +287,6 @@ bool xen_is_grant_dma_device(struct device *dev) return has_iommu; } -bool xen_virtio_mem_acc(struct virtio_device *dev) -{ - if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT)) - return true; - - return xen_is_grant_dma_device(dev->dev.parent); -} - void xen_grant_setup_dma_ops(struct device *dev) { struct xen_grant_dma_data *data; diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h index 98c399a..a9ae51b 100644 --- a/include/xen/xen-ops.h +++ b/include/xen/xen-ops.h @@ -6,6 +6,7 @@ #include <linux/notifier.h> #include <linux/efi.h> #include <linux/virtio_anchor.h> +#include <linux/virtio.h> #include <xen/features.h> #include <asm/xen/interface.h> #include <xen/interface/vcpu.h> @@ -218,7 +219,13 @@ static inline void xen_preemptible_hcall_end(void) { } #ifdef CONFIG_XEN_GRANT_DMA_OPS void xen_grant_setup_dma_ops(struct device *dev); bool xen_is_grant_dma_device(struct device *dev); -bool xen_virtio_mem_acc(struct virtio_device *dev); +static inline bool xen_virtio_mem_acc(struct virtio_device *dev) +{ + if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT)) + return true; + + return xen_is_grant_dma_device(dev->dev.parent); +} #else static inline void xen_grant_setup_dma_ops(struct device *dev) { > + > void xen_grant_setup_dma_ops(struct device *dev) > { > struct xen_grant_dma_data *data; > diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h > index 80546960f8b7..98c399a960a3 100644 > --- a/include/xen/xen-ops.h > +++ b/include/xen/xen-ops.h > @@ -5,6 +5,7 @@ > #include <linux/percpu.h> > #include <linux/notifier.h> > #include <linux/efi.h> > +#include <linux/virtio_anchor.h> > #include <xen/features.h> > #include <asm/xen/interface.h> > #include <xen/interface/vcpu.h> > @@ -217,6 +218,7 @@ static inline void xen_preemptible_hcall_end(void) { } > #ifdef CONFIG_XEN_GRANT_DMA_OPS > void xen_grant_setup_dma_ops(struct device *dev); > bool xen_is_grant_dma_device(struct device *dev); > +bool xen_virtio_mem_acc(struct virtio_device *dev); > #else > static inline void xen_grant_setup_dma_ops(struct device *dev) > { > @@ -225,6 +227,10 @@ static inline bool xen_is_grant_dma_device(struct device *dev) > { > return false; > } > +static inline bool xen_virtio_mem_acc(struct virtio_device *dev) > +{ > + return false; > +} > #endif /* CONFIG_XEN_GRANT_DMA_OPS */ > > #endif /* INCLUDE_XEN_OPS_H */ > diff --git a/include/xen/xen.h b/include/xen/xen.h > index ac5a144c6a65..a99bab817523 100644 > --- a/include/xen/xen.h > +++ b/include/xen/xen.h > @@ -52,14 +52,6 @@ bool xen_biovec_phys_mergeable(const struct bio_vec *vec1, > extern u64 xen_saved_max_mem_size; > #endif > > -#include <linux/virtio_anchor.h> > - > -static inline void xen_set_restricted_virtio_memory_access(void) > -{ > - if (IS_ENABLED(CONFIG_XEN_VIRTIO) && xen_domain()) > - virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc); > -} > - > #ifdef CONFIG_XEN_UNPOPULATED_ALLOC > int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages); > void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages);
On 22.06.22 11:03, Oleksandr wrote: > > On 22.06.22 09:38, Juergen Gross wrote: > > Hello Juergen > >> Commit fa1f57421e0b ("xen/virtio: Enable restricted memory access using >> Xen grant mappings") introduced a new requirement for using virtio >> devices: the backend now needs to support the VIRTIO_F_ACCESS_PLATFORM >> feature. >> >> This is an undue requirement for non-PV guests, as those can be operated >> with existing backends without any problem, as long as those backends >> are running in dom0. >> >> Per default allow virtio devices without grant support for non-PV >> guests. >> >> On Arm require VIRTIO_F_ACCESS_PLATFORM for devices having been listed >> in the device tree to use grants. >> >> Add a new config item to always force use of grants for virtio. >> >> Fixes: fa1f57421e0b ("xen/virtio: Enable restricted memory access using Xen >> grant mappings") >> Reported-by: Viresh Kumar <viresh.kumar@linaro.org> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> V2: >> - remove command line parameter (Christoph Hellwig) >> V3: >> - rebase to callback method > > > Patch looks good, just one NIT ... > > >> --- >> arch/arm/xen/enlighten.c | 4 +++- >> arch/x86/xen/enlighten_hvm.c | 4 +++- >> arch/x86/xen/enlighten_pv.c | 5 ++++- >> drivers/xen/Kconfig | 9 +++++++++ >> drivers/xen/grant-dma-ops.c | 10 ++++++++++ >> include/xen/xen-ops.h | 6 ++++++ >> include/xen/xen.h | 8 -------- >> 7 files changed, 35 insertions(+), 11 deletions(-) >> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c >> index 1f9c3ba32833..93c8ccbf2982 100644 >> --- a/arch/arm/xen/enlighten.c >> +++ b/arch/arm/xen/enlighten.c >> @@ -34,6 +34,7 @@ >> #include <linux/timekeeping.h> >> #include <linux/timekeeper_internal.h> >> #include <linux/acpi.h> >> +#include <linux/virtio_anchor.h> >> #include <linux/mm.h> >> @@ -443,7 +444,8 @@ static int __init xen_guest_init(void) >> if (!xen_domain()) >> return 0; >> - xen_set_restricted_virtio_memory_access(); >> + if (IS_ENABLED(CONFIG_XEN_VIRTIO)) >> + virtio_set_mem_acc_cb(xen_virtio_mem_acc); >> if (!acpi_disabled) >> xen_acpi_guest_init(); >> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c >> index 8b71b1dd7639..28762f800596 100644 >> --- a/arch/x86/xen/enlighten_hvm.c >> +++ b/arch/x86/xen/enlighten_hvm.c >> @@ -4,6 +4,7 @@ >> #include <linux/cpu.h> >> #include <linux/kexec.h> >> #include <linux/memblock.h> >> +#include <linux/virtio_anchor.h> >> #include <xen/features.h> >> #include <xen/events.h> >> @@ -195,7 +196,8 @@ static void __init xen_hvm_guest_init(void) >> if (xen_pv_domain()) >> return; >> - xen_set_restricted_virtio_memory_access(); >> + if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT)) >> + virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc); >> init_hvm_pv_info(); >> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c >> index e3297b15701c..5aaae8a77f55 100644 >> --- a/arch/x86/xen/enlighten_pv.c >> +++ b/arch/x86/xen/enlighten_pv.c >> @@ -31,6 +31,7 @@ >> #include <linux/gfp.h> >> #include <linux/edd.h> >> #include <linux/reboot.h> >> +#include <linux/virtio_anchor.h> >> #include <xen/xen.h> >> #include <xen/events.h> >> @@ -109,7 +110,9 @@ static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc); >> static void __init xen_pv_init_platform(void) >> { >> - xen_set_restricted_virtio_memory_access(); >> + /* PV guests can't operate virtio devices without grants. */ >> + if (IS_ENABLED(CONFIG_XEN_VIRTIO)) >> + virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc); >> populate_extra_pte(fix_to_virt(FIX_PARAVIRT_BOOTMAP)); >> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig >> index bfd5f4f706bc..a65bd92121a5 100644 >> --- a/drivers/xen/Kconfig >> +++ b/drivers/xen/Kconfig >> @@ -355,4 +355,13 @@ config XEN_VIRTIO >> If in doubt, say n. >> +config XEN_VIRTIO_FORCE_GRANT >> + bool "Require Xen virtio support to use grants" >> + depends on XEN_VIRTIO >> + help >> + Require virtio for Xen 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 >> + (e.g. qemu or kernel, depending on the virtio device types used). >> + >> endmenu >> diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c >> index fc0142484001..8973fc1e9ccc 100644 >> --- a/drivers/xen/grant-dma-ops.c >> +++ b/drivers/xen/grant-dma-ops.c >> @@ -12,6 +12,8 @@ >> #include <linux/of.h> >> #include <linux/pfn.h> >> #include <linux/xarray.h> >> +#include <linux/virtio_anchor.h> >> +#include <linux/virtio.h> >> #include <xen/xen.h> >> #include <xen/xen-ops.h> >> #include <xen/grant_table.h> >> @@ -287,6 +289,14 @@ bool xen_is_grant_dma_device(struct device *dev) >> return has_iommu; >> } >> +bool xen_virtio_mem_acc(struct virtio_device *dev) >> +{ >> + if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT)) >> + return true; >> + >> + return xen_is_grant_dma_device(dev->dev.parent); >> +} > > > ... I am thinking would it be better to move this to xen/xen-ops.h as > grant-dma-ops.c is generic (not only for virtio, although the virtio is the > first use-case) I dislike using a function marked as inline in a function vector. We could add another module "xen-virtio" for this purpose, but this seems to be overkill. I think we should just leave it here and move it later in case more real virtio dependent stuff is being added. Juergen
On 22.06.22 17:35, Juergen Gross wrote: Hello Juergen > On 22.06.22 11:03, Oleksandr wrote: >> >> On 22.06.22 09:38, Juergen Gross wrote: >> >> Hello Juergen >> >>> Commit fa1f57421e0b ("xen/virtio: Enable restricted memory access using >>> Xen grant mappings") introduced a new requirement for using virtio >>> devices: the backend now needs to support the VIRTIO_F_ACCESS_PLATFORM >>> feature. >>> >>> This is an undue requirement for non-PV guests, as those can be >>> operated >>> with existing backends without any problem, as long as those backends >>> are running in dom0. >>> >>> Per default allow virtio devices without grant support for non-PV >>> guests. >>> >>> On Arm require VIRTIO_F_ACCESS_PLATFORM for devices having been listed >>> in the device tree to use grants. >>> >>> Add a new config item to always force use of grants for virtio. >>> >>> Fixes: fa1f57421e0b ("xen/virtio: Enable restricted memory access >>> using Xen grant mappings") >>> Reported-by: Viresh Kumar <viresh.kumar@linaro.org> >>> Signed-off-by: Juergen Gross <jgross@suse.com> >>> --- >>> V2: >>> - remove command line parameter (Christoph Hellwig) >>> V3: >>> - rebase to callback method >> >> >> Patch looks good, just one NIT ... >> >> >>> --- >>> arch/arm/xen/enlighten.c | 4 +++- >>> arch/x86/xen/enlighten_hvm.c | 4 +++- >>> arch/x86/xen/enlighten_pv.c | 5 ++++- >>> drivers/xen/Kconfig | 9 +++++++++ >>> drivers/xen/grant-dma-ops.c | 10 ++++++++++ >>> include/xen/xen-ops.h | 6 ++++++ >>> include/xen/xen.h | 8 -------- >>> 7 files changed, 35 insertions(+), 11 deletions(-) >>> >>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c >>> index 1f9c3ba32833..93c8ccbf2982 100644 >>> --- a/arch/arm/xen/enlighten.c >>> +++ b/arch/arm/xen/enlighten.c >>> @@ -34,6 +34,7 @@ >>> #include <linux/timekeeping.h> >>> #include <linux/timekeeper_internal.h> >>> #include <linux/acpi.h> >>> +#include <linux/virtio_anchor.h> >>> #include <linux/mm.h> >>> @@ -443,7 +444,8 @@ static int __init xen_guest_init(void) >>> if (!xen_domain()) >>> return 0; >>> - xen_set_restricted_virtio_memory_access(); >>> + if (IS_ENABLED(CONFIG_XEN_VIRTIO)) >>> + virtio_set_mem_acc_cb(xen_virtio_mem_acc); >>> if (!acpi_disabled) >>> xen_acpi_guest_init(); >>> diff --git a/arch/x86/xen/enlighten_hvm.c >>> b/arch/x86/xen/enlighten_hvm.c >>> index 8b71b1dd7639..28762f800596 100644 >>> --- a/arch/x86/xen/enlighten_hvm.c >>> +++ b/arch/x86/xen/enlighten_hvm.c >>> @@ -4,6 +4,7 @@ >>> #include <linux/cpu.h> >>> #include <linux/kexec.h> >>> #include <linux/memblock.h> >>> +#include <linux/virtio_anchor.h> >>> #include <xen/features.h> >>> #include <xen/events.h> >>> @@ -195,7 +196,8 @@ static void __init xen_hvm_guest_init(void) >>> if (xen_pv_domain()) >>> return; >>> - xen_set_restricted_virtio_memory_access(); >>> + if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT)) >>> + virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc); >>> init_hvm_pv_info(); >>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c >>> index e3297b15701c..5aaae8a77f55 100644 >>> --- a/arch/x86/xen/enlighten_pv.c >>> +++ b/arch/x86/xen/enlighten_pv.c >>> @@ -31,6 +31,7 @@ >>> #include <linux/gfp.h> >>> #include <linux/edd.h> >>> #include <linux/reboot.h> >>> +#include <linux/virtio_anchor.h> >>> #include <xen/xen.h> >>> #include <xen/events.h> >>> @@ -109,7 +110,9 @@ static DEFINE_PER_CPU(struct tls_descs, >>> shadow_tls_desc); >>> static void __init xen_pv_init_platform(void) >>> { >>> - xen_set_restricted_virtio_memory_access(); >>> + /* PV guests can't operate virtio devices without grants. */ >>> + if (IS_ENABLED(CONFIG_XEN_VIRTIO)) >>> + virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc); >>> populate_extra_pte(fix_to_virt(FIX_PARAVIRT_BOOTMAP)); >>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig >>> index bfd5f4f706bc..a65bd92121a5 100644 >>> --- a/drivers/xen/Kconfig >>> +++ b/drivers/xen/Kconfig >>> @@ -355,4 +355,13 @@ config XEN_VIRTIO >>> If in doubt, say n. >>> +config XEN_VIRTIO_FORCE_GRANT >>> + bool "Require Xen virtio support to use grants" >>> + depends on XEN_VIRTIO >>> + help >>> + Require virtio for Xen 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 >>> + (e.g. qemu or kernel, depending on the virtio device types >>> used). >>> + >>> endmenu >>> diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c >>> index fc0142484001..8973fc1e9ccc 100644 >>> --- a/drivers/xen/grant-dma-ops.c >>> +++ b/drivers/xen/grant-dma-ops.c >>> @@ -12,6 +12,8 @@ >>> #include <linux/of.h> >>> #include <linux/pfn.h> >>> #include <linux/xarray.h> >>> +#include <linux/virtio_anchor.h> >>> +#include <linux/virtio.h> >>> #include <xen/xen.h> >>> #include <xen/xen-ops.h> >>> #include <xen/grant_table.h> >>> @@ -287,6 +289,14 @@ bool xen_is_grant_dma_device(struct device *dev) >>> return has_iommu; >>> } >>> +bool xen_virtio_mem_acc(struct virtio_device *dev) >>> +{ >>> + if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT)) >>> + return true; >>> + >>> + return xen_is_grant_dma_device(dev->dev.parent); >>> +} >> >> >> ... I am thinking would it be better to move this to >> xen/xen-ops.h as grant-dma-ops.c is generic (not only for virtio, >> although the virtio is the first use-case) > > I dislike using a function marked as inline in a function vector. > > We could add another module "xen-virtio" for this purpose, but this seems > to be overkill. > > I think we should just leave it here and move it later in case more real > virtio dependent stuff is being added. I am happy with that explanation. Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > > > Juergen
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index 1f9c3ba32833..93c8ccbf2982 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -34,6 +34,7 @@ #include <linux/timekeeping.h> #include <linux/timekeeper_internal.h> #include <linux/acpi.h> +#include <linux/virtio_anchor.h> #include <linux/mm.h> @@ -443,7 +444,8 @@ static int __init xen_guest_init(void) if (!xen_domain()) return 0; - xen_set_restricted_virtio_memory_access(); + if (IS_ENABLED(CONFIG_XEN_VIRTIO)) + virtio_set_mem_acc_cb(xen_virtio_mem_acc); if (!acpi_disabled) xen_acpi_guest_init(); diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c index 8b71b1dd7639..28762f800596 100644 --- a/arch/x86/xen/enlighten_hvm.c +++ b/arch/x86/xen/enlighten_hvm.c @@ -4,6 +4,7 @@ #include <linux/cpu.h> #include <linux/kexec.h> #include <linux/memblock.h> +#include <linux/virtio_anchor.h> #include <xen/features.h> #include <xen/events.h> @@ -195,7 +196,8 @@ static void __init xen_hvm_guest_init(void) if (xen_pv_domain()) return; - xen_set_restricted_virtio_memory_access(); + if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT)) + virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc); init_hvm_pv_info(); diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c index e3297b15701c..5aaae8a77f55 100644 --- a/arch/x86/xen/enlighten_pv.c +++ b/arch/x86/xen/enlighten_pv.c @@ -31,6 +31,7 @@ #include <linux/gfp.h> #include <linux/edd.h> #include <linux/reboot.h> +#include <linux/virtio_anchor.h> #include <xen/xen.h> #include <xen/events.h> @@ -109,7 +110,9 @@ static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc); static void __init xen_pv_init_platform(void) { - xen_set_restricted_virtio_memory_access(); + /* PV guests can't operate virtio devices without grants. */ + if (IS_ENABLED(CONFIG_XEN_VIRTIO)) + virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc); populate_extra_pte(fix_to_virt(FIX_PARAVIRT_BOOTMAP)); diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index bfd5f4f706bc..a65bd92121a5 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -355,4 +355,13 @@ config XEN_VIRTIO If in doubt, say n. +config XEN_VIRTIO_FORCE_GRANT + bool "Require Xen virtio support to use grants" + depends on XEN_VIRTIO + help + Require virtio for Xen 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 + (e.g. qemu or kernel, depending on the virtio device types used). + endmenu diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c index fc0142484001..8973fc1e9ccc 100644 --- a/drivers/xen/grant-dma-ops.c +++ b/drivers/xen/grant-dma-ops.c @@ -12,6 +12,8 @@ #include <linux/of.h> #include <linux/pfn.h> #include <linux/xarray.h> +#include <linux/virtio_anchor.h> +#include <linux/virtio.h> #include <xen/xen.h> #include <xen/xen-ops.h> #include <xen/grant_table.h> @@ -287,6 +289,14 @@ bool xen_is_grant_dma_device(struct device *dev) return has_iommu; } +bool xen_virtio_mem_acc(struct virtio_device *dev) +{ + if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT)) + return true; + + return xen_is_grant_dma_device(dev->dev.parent); +} + void xen_grant_setup_dma_ops(struct device *dev) { struct xen_grant_dma_data *data; diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h index 80546960f8b7..98c399a960a3 100644 --- a/include/xen/xen-ops.h +++ b/include/xen/xen-ops.h @@ -5,6 +5,7 @@ #include <linux/percpu.h> #include <linux/notifier.h> #include <linux/efi.h> +#include <linux/virtio_anchor.h> #include <xen/features.h> #include <asm/xen/interface.h> #include <xen/interface/vcpu.h> @@ -217,6 +218,7 @@ static inline void xen_preemptible_hcall_end(void) { } #ifdef CONFIG_XEN_GRANT_DMA_OPS void xen_grant_setup_dma_ops(struct device *dev); bool xen_is_grant_dma_device(struct device *dev); +bool xen_virtio_mem_acc(struct virtio_device *dev); #else static inline void xen_grant_setup_dma_ops(struct device *dev) { @@ -225,6 +227,10 @@ static inline bool xen_is_grant_dma_device(struct device *dev) { return false; } +static inline bool xen_virtio_mem_acc(struct virtio_device *dev) +{ + return false; +} #endif /* CONFIG_XEN_GRANT_DMA_OPS */ #endif /* INCLUDE_XEN_OPS_H */ diff --git a/include/xen/xen.h b/include/xen/xen.h index ac5a144c6a65..a99bab817523 100644 --- a/include/xen/xen.h +++ b/include/xen/xen.h @@ -52,14 +52,6 @@ bool xen_biovec_phys_mergeable(const struct bio_vec *vec1, extern u64 xen_saved_max_mem_size; #endif -#include <linux/virtio_anchor.h> - -static inline void xen_set_restricted_virtio_memory_access(void) -{ - if (IS_ENABLED(CONFIG_XEN_VIRTIO) && xen_domain()) - virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc); -} - #ifdef CONFIG_XEN_UNPOPULATED_ALLOC int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages); void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages);
Commit fa1f57421e0b ("xen/virtio: Enable restricted memory access using Xen grant mappings") introduced a new requirement for using virtio devices: the backend now needs to support the VIRTIO_F_ACCESS_PLATFORM feature. This is an undue requirement for non-PV guests, as those can be operated with existing backends without any problem, as long as those backends are running in dom0. Per default allow virtio devices without grant support for non-PV guests. On Arm require VIRTIO_F_ACCESS_PLATFORM for devices having been listed in the device tree to use grants. Add a new config item to always force use of grants for virtio. Fixes: fa1f57421e0b ("xen/virtio: Enable restricted memory access using Xen grant mappings") Reported-by: Viresh Kumar <viresh.kumar@linaro.org> Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - remove command line parameter (Christoph Hellwig) V3: - rebase to callback method --- arch/arm/xen/enlighten.c | 4 +++- arch/x86/xen/enlighten_hvm.c | 4 +++- arch/x86/xen/enlighten_pv.c | 5 ++++- drivers/xen/Kconfig | 9 +++++++++ drivers/xen/grant-dma-ops.c | 10 ++++++++++ include/xen/xen-ops.h | 6 ++++++ include/xen/xen.h | 8 -------- 7 files changed, 35 insertions(+), 11 deletions(-)