| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <PH0PR21MB3025C5DFB189B9609F7A601FD7199@PH0PR21MB3025.namprd21.prod.outlook.com>
Date: Thu, 24 Mar 2022 13:18:42 +0000
From: "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To: Robin Murphy <robin.murphy@....com>,
Stephen Hemminger <sthemmin@...rosoft.com>,
KY Srinivasan <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
"wei.liu@...nel.org" <wei.liu@...nel.org>,
Dexuan Cui <decui@...rosoft.com>,
"rafael@...nel.org" <rafael@...nel.org>,
"lenb@...nel.org" <lenb@...nel.org>,
"lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>,
"robh@...nel.org" <robh@...nel.org>, "kw@...ux.com" <kw@...ux.com>,
"bhelgaas@...gle.com" <bhelgaas@...gle.com>,
"hch@....de" <hch@....de>,
"m.szyprowski@...sung.com" <m.szyprowski@...sung.com>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>
Subject: RE: [PATCH v2 1/2] Drivers: hv: vmbus: Propagate VMbus coherence to
each VMbus device
From: Robin Murphy <robin.murphy@....com> Sent: Thursday, March 24, 2022 4:59 AM
>
> On 2022-03-23 20:31, Michael Kelley wrote:
> > VMbus synthetic devices are not represented in the ACPI DSDT -- only
> > the top level VMbus device is represented. As a result, on ARM64
> > coherence information in the _CCA method is not specified for
> > synthetic devices, so they default to not hardware coherent.
> > Drivers for some of these synthetic devices have been recently
> > updated to use the standard DMA APIs, and they are incurring extra
> > overhead of unneeded software coherence management.
> >
> > Fix this by propagating coherence information from the VMbus node
> > in ACPI to the individual synthetic devices. There's no effect on
> > x86/x64 where devices are always hardware coherent.
> >
> > Signed-off-by: Michael Kelley <mikelley@...rosoft.com>
> > ---
> > drivers/hv/hv_common.c | 11 +++++++++++
> > drivers/hv/vmbus_drv.c | 23 +++++++++++++++++++++++
> > include/asm-generic/mshyperv.h | 1 +
> > 3 files changed, 35 insertions(+)
> >
> > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> > index 181d16b..820e814 100644
> > --- a/drivers/hv/hv_common.c
> > +++ b/drivers/hv/hv_common.c
> > @@ -20,6 +20,7 @@
> > #include <linux/panic_notifier.h>
> > #include <linux/ptrace.h>
> > #include <linux/slab.h>
> > +#include <linux/dma-map-ops.h>
> > #include <asm/hyperv-tlfs.h>
> > #include <asm/mshyperv.h>
> >
> > @@ -216,6 +217,16 @@ bool hv_query_ext_cap(u64 cap_query)
> > }
> > EXPORT_SYMBOL_GPL(hv_query_ext_cap);
> >
> > +void hv_setup_dma_ops(struct device *dev, bool coherent)
> > +{
> > + /*
> > + * Hyper-V does not offer a vIOMMU in the guest
> > + * VM, so pass 0/NULL for the IOMMU settings
> > + */
> > + arch_setup_dma_ops(dev, 0, 0, NULL, coherent);
> > +}
> > +EXPORT_SYMBOL_GPL(hv_setup_dma_ops);
> > +
> > bool hv_is_hibernation_supported(void)
> > {
> > return !hv_root_partition && acpi_sleep_state_supported(ACPI_STATE_S4);
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index 12a2b37..2d2c54c 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -905,6 +905,14 @@ static int vmbus_probe(struct device *child_device)
> > struct hv_device *dev = device_to_hv_device(child_device);
> > const struct hv_vmbus_device_id *dev_id;
> >
> > + /*
> > + * On ARM64, propagate the DMA coherence setting from the top level
> > + * VMbus ACPI device to the child VMbus device being added here.
> > + * On x86/x64 coherence is assumed and these calls have no effect.
> > + */
> > + hv_setup_dma_ops(child_device,
> > + device_get_dma_attr(&hv_acpi_dev->dev) == DEV_DMA_COHERENT);
>
> Would you mind hooking up the hv_bus.dma_configure method to do this?
> Feel free to fold hv_setup_dma_ops entirely into that if you're not
> likely to need to call it from anywhere else.
I'm pretty sure using hv_bus.dma_configure() is doable. A separate
hv_setup_dma_ops() is still needed because arch_setup_dma_ops() isn't
exported and this VMbus driver can be built as a module.
>
> > +
> > dev_id = hv_vmbus_get_id(drv, dev);
> > if (drv->probe) {
> > ret = drv->probe(dev, dev_id);
> > @@ -2428,6 +2436,21 @@ static int vmbus_acpi_add(struct acpi_device *device)
> >
> > hv_acpi_dev = device;
> >
> > + /*
> > + * Older versions of Hyper-V for ARM64 fail to include the _CCA
> > + * method on the top level VMbus device in the DSDT. But devices
> > + * are hardware coherent in all current Hyper-V use cases, so fix
> > + * up the ACPI device to behave as if _CCA is present and indicates
> > + * hardware coherence.
> > + */
> > + ACPI_COMPANION_SET(&device->dev, device);
> > + if (IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED) &&
> > + device_get_dma_attr(&device->dev) == DEV_DMA_NOT_SUPPORTED) {
> > + pr_info("No ACPI _CCA found; assuming coherent device I/O\n");
> > + device->flags.cca_seen = true;
> > + device->flags.coherent_dma = true;
> > + }
>
> I'm not the biggest fan of this, especially since I'm not convinced that
> there are any out-of-support deployments of ARM64 Hyper-V that can't be
> updated. However I suppose it's not "real" firmware, and one Hyper-V
> component is at liberty to hack another Hyper-V component's data if it
> really wants to...
Agreed, it's a hack. But Hyper-V instances are out there as part of
Windows 10/11 on ARM64 PCs, and they run ARM64 VMs for the
Windows Subsystem for Linux. Microsoft gets pilloried for breaking
stuff, and this removes the potential for that happening if someone
runs a new Linux kernel version in that VM.
Michael
>
> If you can hook up .dma_configure, or clarify if it wouldn't work,
>
> Acked-by: Robin Murphy <robin.murphy@....com>
>
> Cheers,
> Robin.
>
> > +
> > result = acpi_walk_resources(device->handle, METHOD_NAME__CRS,
> > vmbus_walk_resources, NULL);
> >
> > diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> > index c08758b..c05d2ce 100644
> > --- a/include/asm-generic/mshyperv.h
> > +++ b/include/asm-generic/mshyperv.h
> > @@ -269,6 +269,7 @@ static inline int cpumask_to_vpset_noself(struct hv_vpset
> *vpset,
> > u64 hv_ghcb_hypercall(u64 control, void *input, void *output, u32 input_size);
> > void hyperv_cleanup(void);
> > bool hv_query_ext_cap(u64 cap_query);
> > +void hv_setup_dma_ops(struct device *dev, bool coherent);
> > void *hv_map_memory(void *addr, unsigned long size);
> > void hv_unmap_memory(void *addr);
> > #else /* CONFIG_HYPERV */
Powered by blists - more mailing lists