linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4 RESEND] Fix coherence for VMbus and PCI pass-thru devices in Hyper-V VM
@ 2022-03-17 16:25 Michael Kelley
  2022-03-17 16:25 ` [PATCH 1/4 RESEND] ACPI: scan: Export acpi_get_dma_attr() Michael Kelley
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Michael Kelley @ 2022-03-17 16:25 UTC (permalink / raw)
  To: sthemmin, kys, haiyangz, wei.liu, decui, rafael, lenb,
	lorenzo.pieralisi, robh, kw, bhelgaas, hch, m.szyprowski,
	robin.murphy, linux-acpi, linux-kernel, linux-hyperv, linux-pci,
	iommu
  Cc: mikelley

[Resend to fix an email address typo for Bjorn Helgaas]

Hyper-V VMs have VMbus synthetic devices and PCI pass-thru devices that are added
dynamically via the VMbus protocol and are not represented in the ACPI DSDT. Only
the top level VMbus node exists in the DSDT. As such, on ARM64 these devices don't
pick up coherence information and default to not hardware coherent.  This results
in extra software coherence management overhead since the synthetic devices are
always hardware coherent. PCI pass-thru devices are also hardware coherent in all
current usage scenarios.

Fix this by propagating coherence information from the top level VMbus node in
the DSDT to all VMbus synthetic devices and PCI pass-thru devices. While smaller
granularity of control would be better, basing on the VMbus node in the DSDT
gives as escape path if a future scenario arises with devices that are not
hardware coherent.

The first two patches are prep to allow manipulating device coherence from a
module (since the VMbus driver can be built as a module) and from architecture
independent code without having a bunch of #ifdef's.

The third patch propagates the VMbus node coherence to VMbus synthetic devices.

The fourth patch propagates the coherence to PCI pass-thru devices.

Michael Kelley (4):
  ACPI: scan: Export acpi_get_dma_attr()
  dma-mapping: Add wrapper function to set dma_coherent
  Drivers: hv: vmbus: Propagate VMbus coherence to each VMbus device
  PCI: hv: Propagate coherence from VMbus device to PCI device

 drivers/acpi/scan.c                 |  1 +
 drivers/hv/vmbus_drv.c              | 15 +++++++++++++++
 drivers/pci/controller/pci-hyperv.c | 17 +++++++++++++----
 include/linux/dma-map-ops.h         |  9 +++++++++
 4 files changed, 38 insertions(+), 4 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/4 RESEND] ACPI: scan: Export acpi_get_dma_attr()
  2022-03-17 16:25 [PATCH 0/4 RESEND] Fix coherence for VMbus and PCI pass-thru devices in Hyper-V VM Michael Kelley
@ 2022-03-17 16:25 ` Michael Kelley
  2022-03-17 16:31   ` Robin Murphy
  2022-03-17 16:25 ` [PATCH 2/4 RESEND] dma-mapping: Add wrapper function to set dma_coherent Michael Kelley
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Michael Kelley @ 2022-03-17 16:25 UTC (permalink / raw)
  To: sthemmin, kys, haiyangz, wei.liu, decui, rafael, lenb,
	lorenzo.pieralisi, robh, kw, bhelgaas, hch, m.szyprowski,
	robin.murphy, linux-acpi, linux-kernel, linux-hyperv, linux-pci,
	iommu
  Cc: mikelley

Export acpi_get_dma_attr() so that it can be used by the Hyper-V
VMbus driver, which may be built as a module. The related function
acpi_dma_configure_id() is already exported.

Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
 drivers/acpi/scan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 1331756..9f3c88f 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1489,6 +1489,7 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
 	else
 		return DEV_DMA_NON_COHERENT;
 }
+EXPORT_SYMBOL_GPL(acpi_get_dma_attr);
 
 /**
  * acpi_dma_get_range() - Get device DMA parameters.
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/4 RESEND] dma-mapping: Add wrapper function to set dma_coherent
  2022-03-17 16:25 [PATCH 0/4 RESEND] Fix coherence for VMbus and PCI pass-thru devices in Hyper-V VM Michael Kelley
  2022-03-17 16:25 ` [PATCH 1/4 RESEND] ACPI: scan: Export acpi_get_dma_attr() Michael Kelley
@ 2022-03-17 16:25 ` Michael Kelley
  2022-03-17 17:19   ` Robin Murphy
  2022-03-17 16:25 ` [PATCH 3/4 RESEND] Drivers: hv: vmbus: Propagate VMbus coherence to each VMbus device Michael Kelley
  2022-03-17 16:25 ` [PATCH 4/4 RESEND] PCI: hv: Propagate coherence from VMbus device to PCI device Michael Kelley
  3 siblings, 1 reply; 16+ messages in thread
From: Michael Kelley @ 2022-03-17 16:25 UTC (permalink / raw)
  To: sthemmin, kys, haiyangz, wei.liu, decui, rafael, lenb,
	lorenzo.pieralisi, robh, kw, bhelgaas, hch, m.szyprowski,
	robin.murphy, linux-acpi, linux-kernel, linux-hyperv, linux-pci,
	iommu
  Cc: mikelley

Add a wrapper function to set dma_coherent, avoiding the need for
complex #ifdef's when setting it in architecture independent code.

Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
 include/linux/dma-map-ops.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index 0d5b06b..3350e7a 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -254,11 +254,20 @@ static inline bool dev_is_dma_coherent(struct device *dev)
 {
 	return dev->dma_coherent;
 }
+static inline void dev_set_dma_coherent(struct device *dev,
+		bool coherent)
+{
+	dev->dma_coherent = coherent;
+}
 #else
 static inline bool dev_is_dma_coherent(struct device *dev)
 {
 	return true;
 }
+static inline void dev_set_dma_coherent(struct device *dev,
+		bool coherent)
+{
+}
 #endif /* CONFIG_ARCH_HAS_DMA_COHERENCE_H */
 
 void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/4 RESEND] Drivers: hv: vmbus: Propagate VMbus coherence to each VMbus device
  2022-03-17 16:25 [PATCH 0/4 RESEND] Fix coherence for VMbus and PCI pass-thru devices in Hyper-V VM Michael Kelley
  2022-03-17 16:25 ` [PATCH 1/4 RESEND] ACPI: scan: Export acpi_get_dma_attr() Michael Kelley
  2022-03-17 16:25 ` [PATCH 2/4 RESEND] dma-mapping: Add wrapper function to set dma_coherent Michael Kelley
@ 2022-03-17 16:25 ` Michael Kelley
  2022-03-17 16:52   ` Robin Murphy
  2022-03-17 16:25 ` [PATCH 4/4 RESEND] PCI: hv: Propagate coherence from VMbus device to PCI device Michael Kelley
  3 siblings, 1 reply; 16+ messages in thread
From: Michael Kelley @ 2022-03-17 16:25 UTC (permalink / raw)
  To: sthemmin, kys, haiyangz, wei.liu, decui, rafael, lenb,
	lorenzo.pieralisi, robh, kw, bhelgaas, hch, m.szyprowski,
	robin.murphy, linux-acpi, linux-kernel, linux-hyperv, linux-pci,
	iommu
  Cc: mikelley

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@microsoft.com>
---
 drivers/hv/vmbus_drv.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 12a2b37..c0e993ad 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -904,6 +904,21 @@ static int vmbus_probe(struct device *child_device)
 			drv_to_hv_drv(child_device->driver);
 	struct hv_device *dev = device_to_hv_device(child_device);
 	const struct hv_vmbus_device_id *dev_id;
+	enum dev_dma_attr coherent;
+
+	/*
+	 * On ARM64, propagate the DMA coherence setting from the top level
+	 * VMbus ACPI device to the child VMbus device being added here.
+	 * Older Hyper-V ARM64 versions don't set the _CCA method on the
+	 * top level VMbus ACPI device as they should.  Treat these cases
+	 * as DMA coherent since that's the assumption made by Hyper-V.
+	 *
+	 * On x86/x64 these calls assume coherence and have no effect.
+	 */
+	coherent = acpi_get_dma_attr(hv_acpi_dev);
+	if (coherent == DEV_DMA_NOT_SUPPORTED)
+		coherent = DEV_DMA_COHERENT;
+	acpi_dma_configure(child_device, coherent);
 
 	dev_id = hv_vmbus_get_id(drv, dev);
 	if (drv->probe) {
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/4 RESEND] PCI: hv: Propagate coherence from VMbus device to PCI device
  2022-03-17 16:25 [PATCH 0/4 RESEND] Fix coherence for VMbus and PCI pass-thru devices in Hyper-V VM Michael Kelley
                   ` (2 preceding siblings ...)
  2022-03-17 16:25 ` [PATCH 3/4 RESEND] Drivers: hv: vmbus: Propagate VMbus coherence to each VMbus device Michael Kelley
@ 2022-03-17 16:25 ` Michael Kelley
  2022-03-17 17:15   ` Robin Murphy
  3 siblings, 1 reply; 16+ messages in thread
From: Michael Kelley @ 2022-03-17 16:25 UTC (permalink / raw)
  To: sthemmin, kys, haiyangz, wei.liu, decui, rafael, lenb,
	lorenzo.pieralisi, robh, kw, bhelgaas, hch, m.szyprowski,
	robin.murphy, linux-acpi, linux-kernel, linux-hyperv, linux-pci,
	iommu
  Cc: mikelley

PCI pass-thru devices in a Hyper-V VM are represented as a VMBus
device and as a PCI device.  The coherence of the VMbus device is
set based on the VMbus node in ACPI, but the PCI device has no
ACPI node and defaults to not hardware coherent.  This results
in extra software coherence management overhead on ARM64 when
devices are hardware coherent.

Fix this by propagating the coherence of the VMbus device to the
PCI device.  There's no effect on x86/x64 where devices are
always hardware coherent.

Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
 drivers/pci/controller/pci-hyperv.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index ae0bc2f..14276f5 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -49,6 +49,7 @@
 #include <linux/refcount.h>
 #include <linux/irqdomain.h>
 #include <linux/acpi.h>
+#include <linux/dma-map-ops.h>
 #include <asm/mshyperv.h>
 
 /*
@@ -2142,9 +2143,9 @@ static void hv_pci_remove_slots(struct hv_pcibus_device *hbus)
 }
 
 /*
- * Set NUMA node for the devices on the bus
+ * Set NUMA node and DMA coherence for the devices on the bus
  */
-static void hv_pci_assign_numa_node(struct hv_pcibus_device *hbus)
+static void hv_pci_assign_properties(struct hv_pcibus_device *hbus)
 {
 	struct pci_dev *dev;
 	struct pci_bus *bus = hbus->bridge->bus;
@@ -2167,6 +2168,14 @@ static void hv_pci_assign_numa_node(struct hv_pcibus_device *hbus)
 				     numa_map_to_online_node(
 					     hv_dev->desc.virtual_numa_node));
 
+		/*
+		 * On ARM64, propagate the DMA coherence from the VMbus device
+		 * to the corresponding PCI device. On x86/x64, these calls
+		 * have no effect because DMA is always hardware coherent.
+		 */
+		dev_set_dma_coherent(&dev->dev,
+			dev_is_dma_coherent(&hbus->hdev->device));
+
 		put_pcichild(hv_dev);
 	}
 }
@@ -2191,7 +2200,7 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
 		return error;
 
 	pci_lock_rescan_remove();
-	hv_pci_assign_numa_node(hbus);
+	hv_pci_assign_properties(hbus);
 	pci_bus_assign_resources(bridge->bus);
 	hv_pci_assign_slots(hbus);
 	pci_bus_add_devices(bridge->bus);
@@ -2458,7 +2467,7 @@ static void pci_devices_present_work(struct work_struct *work)
 		 */
 		pci_lock_rescan_remove();
 		pci_scan_child_bus(hbus->bridge->bus);
-		hv_pci_assign_numa_node(hbus);
+		hv_pci_assign_properties(hbus);
 		hv_pci_assign_slots(hbus);
 		pci_unlock_rescan_remove();
 		break;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4 RESEND] ACPI: scan: Export acpi_get_dma_attr()
  2022-03-17 16:25 ` [PATCH 1/4 RESEND] ACPI: scan: Export acpi_get_dma_attr() Michael Kelley
@ 2022-03-17 16:31   ` Robin Murphy
  2022-03-17 18:56     ` Michael Kelley (LINUX)
  0 siblings, 1 reply; 16+ messages in thread
From: Robin Murphy @ 2022-03-17 16:31 UTC (permalink / raw)
  To: Michael Kelley, sthemmin, kys, haiyangz, wei.liu, decui, rafael,
	lenb, lorenzo.pieralisi, robh, kw, bhelgaas, hch, m.szyprowski,
	linux-acpi, linux-kernel, linux-hyperv, linux-pci, iommu

On 2022-03-17 16:25, Michael Kelley wrote:
> Export acpi_get_dma_attr() so that it can be used by the Hyper-V
> VMbus driver, which may be built as a module. The related function
> acpi_dma_configure_id() is already exported.

No. Use device_get_dma_attr() like everyone else, please.

Robin.

> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
>   drivers/acpi/scan.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 1331756..9f3c88f 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1489,6 +1489,7 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
>   	else
>   		return DEV_DMA_NON_COHERENT;
>   }
> +EXPORT_SYMBOL_GPL(acpi_get_dma_attr);
>   
>   /**
>    * acpi_dma_get_range() - Get device DMA parameters.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/4 RESEND] Drivers: hv: vmbus: Propagate VMbus coherence to each VMbus device
  2022-03-17 16:25 ` [PATCH 3/4 RESEND] Drivers: hv: vmbus: Propagate VMbus coherence to each VMbus device Michael Kelley
@ 2022-03-17 16:52   ` Robin Murphy
  0 siblings, 0 replies; 16+ messages in thread
From: Robin Murphy @ 2022-03-17 16:52 UTC (permalink / raw)
  To: Michael Kelley, sthemmin, kys, haiyangz, wei.liu, decui, rafael,
	lenb, lorenzo.pieralisi, robh, kw, bhelgaas, hch, m.szyprowski,
	linux-acpi, linux-kernel, linux-hyperv, linux-pci, iommu

On 2022-03-17 16:25, Michael Kelley via iommu 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@microsoft.com>
> ---
>   drivers/hv/vmbus_drv.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 12a2b37..c0e993ad 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -904,6 +904,21 @@ static int vmbus_probe(struct device *child_device)
>   			drv_to_hv_drv(child_device->driver);
>   	struct hv_device *dev = device_to_hv_device(child_device);
>   	const struct hv_vmbus_device_id *dev_id;
> +	enum dev_dma_attr coherent;
> +
> +	/*
> +	 * On ARM64, propagate the DMA coherence setting from the top level
> +	 * VMbus ACPI device to the child VMbus device being added here.
> +	 * Older Hyper-V ARM64 versions don't set the _CCA method on the
> +	 * top level VMbus ACPI device as they should.  Treat these cases
> +	 * as DMA coherent since that's the assumption made by Hyper-V.
> +	 *
> +	 * On x86/x64 these calls assume coherence and have no effect.
> +	 */
> +	coherent = acpi_get_dma_attr(hv_acpi_dev);
> +	if (coherent == DEV_DMA_NOT_SUPPORTED)
> +		coherent = DEV_DMA_COHERENT;
> +	acpi_dma_configure(child_device, coherent);

acpi_dma_configure is for devices represented in ACPI. The commit 
message implies that these VMBus devices aren't represented in ACPI. 
What gives?

Robin.

>   
>   	dev_id = hv_vmbus_get_id(drv, dev);
>   	if (drv->probe) {

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/4 RESEND] PCI: hv: Propagate coherence from VMbus device to PCI device
  2022-03-17 16:25 ` [PATCH 4/4 RESEND] PCI: hv: Propagate coherence from VMbus device to PCI device Michael Kelley
@ 2022-03-17 17:15   ` Robin Murphy
  2022-03-18  5:12     ` Michael Kelley (LINUX)
  0 siblings, 1 reply; 16+ messages in thread
From: Robin Murphy @ 2022-03-17 17:15 UTC (permalink / raw)
  To: Michael Kelley, sthemmin, kys, haiyangz, wei.liu, decui, rafael,
	lenb, lorenzo.pieralisi, robh, kw, bhelgaas, hch, m.szyprowski,
	linux-acpi, linux-kernel, linux-hyperv, linux-pci, iommu

On 2022-03-17 16:25, Michael Kelley via iommu wrote:
> PCI pass-thru devices in a Hyper-V VM are represented as a VMBus
> device and as a PCI device.  The coherence of the VMbus device is
> set based on the VMbus node in ACPI, but the PCI device has no
> ACPI node and defaults to not hardware coherent.  This results
> in extra software coherence management overhead on ARM64 when
> devices are hardware coherent.
> 
> Fix this by propagating the coherence of the VMbus device to the
> PCI device.  There's no effect on x86/x64 where devices are
> always hardware coherent.
> 
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
>   drivers/pci/controller/pci-hyperv.c | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index ae0bc2f..14276f5 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -49,6 +49,7 @@
>   #include <linux/refcount.h>
>   #include <linux/irqdomain.h>
>   #include <linux/acpi.h>
> +#include <linux/dma-map-ops.h>
>   #include <asm/mshyperv.h>
>   
>   /*
> @@ -2142,9 +2143,9 @@ static void hv_pci_remove_slots(struct hv_pcibus_device *hbus)
>   }
>   
>   /*
> - * Set NUMA node for the devices on the bus
> + * Set NUMA node and DMA coherence for the devices on the bus
>    */
> -static void hv_pci_assign_numa_node(struct hv_pcibus_device *hbus)
> +static void hv_pci_assign_properties(struct hv_pcibus_device *hbus)
>   {
>   	struct pci_dev *dev;
>   	struct pci_bus *bus = hbus->bridge->bus;
> @@ -2167,6 +2168,14 @@ static void hv_pci_assign_numa_node(struct hv_pcibus_device *hbus)
>   				     numa_map_to_online_node(
>   					     hv_dev->desc.virtual_numa_node));
>   
> +		/*
> +		 * On ARM64, propagate the DMA coherence from the VMbus device
> +		 * to the corresponding PCI device. On x86/x64, these calls
> +		 * have no effect because DMA is always hardware coherent.
> +		 */
> +		dev_set_dma_coherent(&dev->dev,
> +			dev_is_dma_coherent(&hbus->hdev->device));

Eww... if you really have to do this, I'd prefer to see a proper 
hv_dma_configure() helper implemented and wired up to 
pci_dma_configure(). Although since it's a generic property I guess at 
worst pci_dma_configure could perhaps propagate coherency from the host 
bridge to its children by itself in the absence of any other firmware 
info. And it's built-in so could use arch_setup_dma_ops() like everyone 
else.

Robin.

> +
>   		put_pcichild(hv_dev);
>   	}
>   }
> @@ -2191,7 +2200,7 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
>   		return error;
>   
>   	pci_lock_rescan_remove();
> -	hv_pci_assign_numa_node(hbus);
> +	hv_pci_assign_properties(hbus);
>   	pci_bus_assign_resources(bridge->bus);
>   	hv_pci_assign_slots(hbus);
>   	pci_bus_add_devices(bridge->bus);
> @@ -2458,7 +2467,7 @@ static void pci_devices_present_work(struct work_struct *work)
>   		 */
>   		pci_lock_rescan_remove();
>   		pci_scan_child_bus(hbus->bridge->bus);
> -		hv_pci_assign_numa_node(hbus);
> +		hv_pci_assign_properties(hbus);
>   		hv_pci_assign_slots(hbus);
>   		pci_unlock_rescan_remove();
>   		break;

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4 RESEND] dma-mapping: Add wrapper function to set dma_coherent
  2022-03-17 16:25 ` [PATCH 2/4 RESEND] dma-mapping: Add wrapper function to set dma_coherent Michael Kelley
@ 2022-03-17 17:19   ` Robin Murphy
  2022-03-17 19:13     ` Michael Kelley (LINUX)
  0 siblings, 1 reply; 16+ messages in thread
From: Robin Murphy @ 2022-03-17 17:19 UTC (permalink / raw)
  To: Michael Kelley, sthemmin, kys, haiyangz, wei.liu, decui, rafael,
	lenb, lorenzo.pieralisi, robh, kw, bhelgaas, hch, m.szyprowski,
	linux-acpi, linux-kernel, linux-hyperv, linux-pci, iommu

On 2022-03-17 16:25, Michael Kelley via iommu wrote:
> Add a wrapper function to set dma_coherent, avoiding the need for
> complex #ifdef's when setting it in architecture independent code.

No. It might happen to work out on the architectures you're looking at, 
but if Hyper-V were ever to support, say, AArch32 VMs you might see the 
problem. arch_setup_dma_ops() is the tool for this job.

Robin.

> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
>   include/linux/dma-map-ops.h | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
> index 0d5b06b..3350e7a 100644
> --- a/include/linux/dma-map-ops.h
> +++ b/include/linux/dma-map-ops.h
> @@ -254,11 +254,20 @@ static inline bool dev_is_dma_coherent(struct device *dev)
>   {
>   	return dev->dma_coherent;
>   }
> +static inline void dev_set_dma_coherent(struct device *dev,
> +		bool coherent)
> +{
> +	dev->dma_coherent = coherent;
> +}
>   #else
>   static inline bool dev_is_dma_coherent(struct device *dev)
>   {
>   	return true;
>   }
> +static inline void dev_set_dma_coherent(struct device *dev,
> +		bool coherent)
> +{
> +}
>   #endif /* CONFIG_ARCH_HAS_DMA_COHERENCE_H */
>   
>   void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/4 RESEND] ACPI: scan: Export acpi_get_dma_attr()
  2022-03-17 16:31   ` Robin Murphy
@ 2022-03-17 18:56     ` Michael Kelley (LINUX)
  0 siblings, 0 replies; 16+ messages in thread
From: Michael Kelley (LINUX) @ 2022-03-17 18:56 UTC (permalink / raw)
  To: Robin Murphy, Stephen Hemminger, KY Srinivasan, Haiyang Zhang,
	wei.liu, Dexuan Cui, rafael, lenb, lorenzo.pieralisi, robh, kw,
	bhelgaas, hch, m.szyprowski, linux-acpi, linux-kernel,
	linux-hyperv, linux-pci, iommu

From: Robin Murphy <robin.murphy@arm.com> Sent: Thursday, March 17, 2022 9:31 AM
> 
> On 2022-03-17 16:25, Michael Kelley wrote:
> > Export acpi_get_dma_attr() so that it can be used by the Hyper-V
> > VMbus driver, which may be built as a module. The related function
> > acpi_dma_configure_id() is already exported.
> 
> No. Use device_get_dma_attr() like everyone else, please.
>

Got it.  That works.

Michael


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 2/4 RESEND] dma-mapping: Add wrapper function to set dma_coherent
  2022-03-17 17:19   ` Robin Murphy
@ 2022-03-17 19:13     ` Michael Kelley (LINUX)
  2022-03-18 11:07       ` Robin Murphy
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Kelley (LINUX) @ 2022-03-17 19:13 UTC (permalink / raw)
  To: Robin Murphy, Stephen Hemminger, KY Srinivasan, Haiyang Zhang,
	wei.liu, Dexuan Cui, rafael, lenb, lorenzo.pieralisi, robh, kw,
	bhelgaas, hch, m.szyprowski, linux-acpi, linux-kernel,
	linux-hyperv, linux-pci, iommu

From: Robin Murphy <robin.murphy@arm.com> Sent: Thursday, March 17, 2022 10:20 AM
> 
> On 2022-03-17 16:25, Michael Kelley via iommu wrote:
> > Add a wrapper function to set dma_coherent, avoiding the need for
> > complex #ifdef's when setting it in architecture independent code.
> 
> No. It might happen to work out on the architectures you're looking at,
> but if Hyper-V were ever to support, say, AArch32 VMs you might see the
> problem. arch_setup_dma_ops() is the tool for this job.
>

OK.   There's currently no vIOMMU in a Hyper-V guest, so presumably the
code would call arch_setup_dma_ops() with the dma_base, size, and iommu
parameters set to 0 and NULL.  This call can then be used in Patch 3 instead
of acpi_dma_configure(), and in the Patch 4 hv_dma_configure() function
as you suggested.  arch_setup_dma_ops() is not exported, so I'd need to
wrap it in a Hyper-V specific function in built-in code that is exported.

But at some point in the future if there's a vIOMMU in Hyper-V guests,
this approach will need some rework.

Does that make sense?  Thanks for your input and suggestions ...

Michael



^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 4/4 RESEND] PCI: hv: Propagate coherence from VMbus device to PCI device
  2022-03-17 17:15   ` Robin Murphy
@ 2022-03-18  5:12     ` Michael Kelley (LINUX)
  2022-03-18 10:57       ` Robin Murphy
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Kelley (LINUX) @ 2022-03-18  5:12 UTC (permalink / raw)
  To: Robin Murphy, Stephen Hemminger, KY Srinivasan, Haiyang Zhang,
	wei.liu, Dexuan Cui, rafael, lenb, lorenzo.pieralisi, robh, kw,
	bhelgaas, hch, m.szyprowski, linux-acpi, linux-kernel,
	linux-hyperv, linux-pci, iommu

From: Robin Murphy <robin.murphy@arm.com> Sent: Thursday, March 17, 2022 10:15 AM
> 
> On 2022-03-17 16:25, Michael Kelley via iommu wrote:
> > PCI pass-thru devices in a Hyper-V VM are represented as a VMBus
> > device and as a PCI device.  The coherence of the VMbus device is
> > set based on the VMbus node in ACPI, but the PCI device has no
> > ACPI node and defaults to not hardware coherent.  This results
> > in extra software coherence management overhead on ARM64 when
> > devices are hardware coherent.
> >
> > Fix this by propagating the coherence of the VMbus device to the
> > PCI device.  There's no effect on x86/x64 where devices are
> > always hardware coherent.
> >
> > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> > ---
> >   drivers/pci/controller/pci-hyperv.c | 17 +++++++++++++----
> >   1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> > index ae0bc2f..14276f5 100644
> > --- a/drivers/pci/controller/pci-hyperv.c
> > +++ b/drivers/pci/controller/pci-hyperv.c
> > @@ -49,6 +49,7 @@
> >   #include <linux/refcount.h>
> >   #include <linux/irqdomain.h>
> >   #include <linux/acpi.h>
> > +#include <linux/dma-map-ops.h>
> >   #include <asm/mshyperv.h>
> >
> >   /*
> > @@ -2142,9 +2143,9 @@ static void hv_pci_remove_slots(struct hv_pcibus_device
> *hbus)
> >   }
> >
> >   /*
> > - * Set NUMA node for the devices on the bus
> > + * Set NUMA node and DMA coherence for the devices on the bus
> >    */
> > -static void hv_pci_assign_numa_node(struct hv_pcibus_device *hbus)
> > +static void hv_pci_assign_properties(struct hv_pcibus_device *hbus)
> >   {
> >   	struct pci_dev *dev;
> >   	struct pci_bus *bus = hbus->bridge->bus;
> > @@ -2167,6 +2168,14 @@ static void hv_pci_assign_numa_node(struct
> hv_pcibus_device *hbus)
> >   				     numa_map_to_online_node(
> >   					     hv_dev->desc.virtual_numa_node));
> >
> > +		/*
> > +		 * On ARM64, propagate the DMA coherence from the VMbus device
> > +		 * to the corresponding PCI device. On x86/x64, these calls
> > +		 * have no effect because DMA is always hardware coherent.
> > +		 */
> > +		dev_set_dma_coherent(&dev->dev,
> > +			dev_is_dma_coherent(&hbus->hdev->device));
> 
> Eww... if you really have to do this, I'd prefer to see a proper
> hv_dma_configure() helper implemented and wired up to
> pci_dma_configure(). Although since it's a generic property I guess at
> worst pci_dma_configure could perhaps propagate coherency from the host
> bridge to its children by itself in the absence of any other firmware
> info. And it's built-in so could use arch_setup_dma_ops() like everyone
> else.
> 

I'm not seeing an existing mechanism to provide a "helper" or override
of pci_dma_configure().   Could you elaborate?  Or is this something
that needs to be created?

Michael

> Robin.
> 
> > +
> >   		put_pcichild(hv_dev);
> >   	}
> >   }
> > @@ -2191,7 +2200,7 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device
> *hbus)
> >   		return error;
> >
> >   	pci_lock_rescan_remove();
> > -	hv_pci_assign_numa_node(hbus);
> > +	hv_pci_assign_properties(hbus);
> >   	pci_bus_assign_resources(bridge->bus);
> >   	hv_pci_assign_slots(hbus);
> >   	pci_bus_add_devices(bridge->bus);
> > @@ -2458,7 +2467,7 @@ static void pci_devices_present_work(struct work_struct
> *work)
> >   		 */
> >   		pci_lock_rescan_remove();
> >   		pci_scan_child_bus(hbus->bridge->bus);
> > -		hv_pci_assign_numa_node(hbus);
> > +		hv_pci_assign_properties(hbus);
> >   		hv_pci_assign_slots(hbus);
> >   		pci_unlock_rescan_remove();
> >   		break;

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/4 RESEND] PCI: hv: Propagate coherence from VMbus device to PCI device
  2022-03-18  5:12     ` Michael Kelley (LINUX)
@ 2022-03-18 10:57       ` Robin Murphy
  2022-03-18 20:36         ` Michael Kelley (LINUX)
  0 siblings, 1 reply; 16+ messages in thread
From: Robin Murphy @ 2022-03-18 10:57 UTC (permalink / raw)
  To: Michael Kelley (LINUX),
	Stephen Hemminger, KY Srinivasan, Haiyang Zhang, wei.liu,
	Dexuan Cui, rafael, lenb, lorenzo.pieralisi, robh, kw, bhelgaas,
	hch, m.szyprowski, linux-acpi, linux-kernel, linux-hyperv,
	linux-pci, iommu

On 2022-03-18 05:12, Michael Kelley (LINUX) wrote:
> From: Robin Murphy <robin.murphy@arm.com> Sent: Thursday, March 17, 2022 10:15 AM
>>
>> On 2022-03-17 16:25, Michael Kelley via iommu wrote:
>>> PCI pass-thru devices in a Hyper-V VM are represented as a VMBus
>>> device and as a PCI device.  The coherence of the VMbus device is
>>> set based on the VMbus node in ACPI, but the PCI device has no
>>> ACPI node and defaults to not hardware coherent.  This results
>>> in extra software coherence management overhead on ARM64 when
>>> devices are hardware coherent.
>>>
>>> Fix this by propagating the coherence of the VMbus device to the
>>> PCI device.  There's no effect on x86/x64 where devices are
>>> always hardware coherent.
>>>
>>> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
>>> ---
>>>    drivers/pci/controller/pci-hyperv.c | 17 +++++++++++++----
>>>    1 file changed, 13 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
>>> index ae0bc2f..14276f5 100644
>>> --- a/drivers/pci/controller/pci-hyperv.c
>>> +++ b/drivers/pci/controller/pci-hyperv.c
>>> @@ -49,6 +49,7 @@
>>>    #include <linux/refcount.h>
>>>    #include <linux/irqdomain.h>
>>>    #include <linux/acpi.h>
>>> +#include <linux/dma-map-ops.h>
>>>    #include <asm/mshyperv.h>
>>>
>>>    /*
>>> @@ -2142,9 +2143,9 @@ static void hv_pci_remove_slots(struct hv_pcibus_device
>> *hbus)
>>>    }
>>>
>>>    /*
>>> - * Set NUMA node for the devices on the bus
>>> + * Set NUMA node and DMA coherence for the devices on the bus
>>>     */
>>> -static void hv_pci_assign_numa_node(struct hv_pcibus_device *hbus)
>>> +static void hv_pci_assign_properties(struct hv_pcibus_device *hbus)
>>>    {
>>>    	struct pci_dev *dev;
>>>    	struct pci_bus *bus = hbus->bridge->bus;
>>> @@ -2167,6 +2168,14 @@ static void hv_pci_assign_numa_node(struct
>> hv_pcibus_device *hbus)
>>>    				     numa_map_to_online_node(
>>>    					     hv_dev->desc.virtual_numa_node));
>>>
>>> +		/*
>>> +		 * On ARM64, propagate the DMA coherence from the VMbus device
>>> +		 * to the corresponding PCI device. On x86/x64, these calls
>>> +		 * have no effect because DMA is always hardware coherent.
>>> +		 */
>>> +		dev_set_dma_coherent(&dev->dev,
>>> +			dev_is_dma_coherent(&hbus->hdev->device));
>>
>> Eww... if you really have to do this, I'd prefer to see a proper
>> hv_dma_configure() helper implemented and wired up to
>> pci_dma_configure(). Although since it's a generic property I guess at
>> worst pci_dma_configure could perhaps propagate coherency from the host
>> bridge to its children by itself in the absence of any other firmware
>> info. And it's built-in so could use arch_setup_dma_ops() like everyone
>> else.
>>
> 
> I'm not seeing an existing mechanism to provide a "helper" or override
> of pci_dma_configure().   Could you elaborate?  Or is this something
> that needs to be created?

I mean something like the diff below (other #includes omitted for 
clarity). Essentially if VMBus has its own way of describing parts of 
the system, then for those parts it's nice if it could fit into the same 
abstractions we use for firmware-based system description.

Cheers,
Robin.

----->8-----
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 588588cfda48..7d92ccad1569 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -20,6 +20,7 @@
  #include <linux/of_device.h>
  #include <linux/acpi.h>
  #include <linux/dma-map-ops.h>
+#include <linux/hyperv.h>
  #include "pci.h"
  #include "pcie/portdrv.h"

@@ -1602,6 +1603,8 @@ static int pci_dma_configure(struct device *dev)
  		struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);

  		ret = acpi_dma_configure(dev, acpi_get_dma_attr(adev));
+	} else if (is_vmbus_dev(bridge)) {
+		ret = hv_dma_configure(dev, device_get_dma_attr(bridge));
  	}

  	pci_put_host_bridge_device(bridge);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index f565a8938836..d1d4dd3d5a3a 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1764,4 +1764,19 @@ static inline unsigned long virt_to_hvpfn(void *addr)
  #define HVPFN_DOWN(x)	((x) >> HV_HYP_PAGE_SHIFT)
  #define page_to_hvpfn(page)	(page_to_pfn(page) * NR_HV_HYP_PAGES_IN_PAGE)

+static inline bool is_vmbus_dev(struct device *dev)
+{
+	/*
+	 * dev->bus == &hv_bus would break when the caller is built-in
+	 * and CONFIG_HYPERV=m, so look for it by name instead.
+	 */
+	return !strcmp(dev->bus->name, "vmbus");
+}
+
+static inline int hv_dma_configure(struct device *dev, enum 
dev_dma_attr attr)
+{
+	arch_setup_dma_ops(dev, 0, 0, NULL, attr == DEV_DMA_COHERENT);
+	return 0;
+}
+
  #endif /* _HYPERV_H */

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4 RESEND] dma-mapping: Add wrapper function to set dma_coherent
  2022-03-17 19:13     ` Michael Kelley (LINUX)
@ 2022-03-18 11:07       ` Robin Murphy
  2022-03-18 20:37         ` Michael Kelley (LINUX)
  0 siblings, 1 reply; 16+ messages in thread
From: Robin Murphy @ 2022-03-18 11:07 UTC (permalink / raw)
  To: Michael Kelley (LINUX),
	Stephen Hemminger, KY Srinivasan, Haiyang Zhang, wei.liu,
	Dexuan Cui, rafael, lenb, lorenzo.pieralisi, robh, kw, bhelgaas,
	hch, m.szyprowski, linux-acpi, linux-kernel, linux-hyperv,
	linux-pci, iommu

On 2022-03-17 19:13, Michael Kelley (LINUX) wrote:
> From: Robin Murphy <robin.murphy@arm.com> Sent: Thursday, March 17, 2022 10:20 AM
>>
>> On 2022-03-17 16:25, Michael Kelley via iommu wrote:
>>> Add a wrapper function to set dma_coherent, avoiding the need for
>>> complex #ifdef's when setting it in architecture independent code.
>>
>> No. It might happen to work out on the architectures you're looking at,
>> but if Hyper-V were ever to support, say, AArch32 VMs you might see the
>> problem. arch_setup_dma_ops() is the tool for this job.
>>
> 
> OK.   There's currently no vIOMMU in a Hyper-V guest, so presumably the
> code would call arch_setup_dma_ops() with the dma_base, size, and iommu
> parameters set to 0 and NULL.  This call can then be used in Patch 3 instead
> of acpi_dma_configure(), and in the Patch 4 hv_dma_configure() function
> as you suggested.  arch_setup_dma_ops() is not exported, so I'd need to
> wrap it in a Hyper-V specific function in built-in code that is exported.
> 
> But at some point in the future if there's a vIOMMU in Hyper-V guests,
> this approach will need some rework.
> 
> Does that make sense?  Thanks for your input and suggestions ...

Yes, that's essentially what I had in mind. If you did present a vIOMMU 
to the guest, presumably you'd either have to construct a regular 
IORT/VIOT, and thus end up adding the root complex to the ACPI namespace 
too so it can be referenced, at which point it would all get picked up 
by the standard machinery, or come up with some magic VMBus mechanism 
that would need a whole load of work to wire up in all the relevant 
places anyway.

(But please lean extremely heavily towards the former option!)

Thanks,
Robin.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 4/4 RESEND] PCI: hv: Propagate coherence from VMbus device to PCI device
  2022-03-18 10:57       ` Robin Murphy
@ 2022-03-18 20:36         ` Michael Kelley (LINUX)
  0 siblings, 0 replies; 16+ messages in thread
From: Michael Kelley (LINUX) @ 2022-03-18 20:36 UTC (permalink / raw)
  To: Robin Murphy, Stephen Hemminger, KY Srinivasan, Haiyang Zhang,
	wei.liu, Dexuan Cui, rafael, lenb, lorenzo.pieralisi, robh, kw,
	bhelgaas, hch, m.szyprowski, linux-acpi, linux-kernel,
	linux-hyperv, linux-pci, iommu

From: Robin Murphy <robin.murphy@arm.com> Sent: Friday, March 18, 2022 3:58 AM
> 
> On 2022-03-18 05:12, Michael Kelley (LINUX) wrote:
> > From: Robin Murphy <robin.murphy@arm.com> Sent: Thursday, March 17, 2022
> 10:15 AM
> >>
> >> On 2022-03-17 16:25, Michael Kelley via iommu wrote:
> >>> PCI pass-thru devices in a Hyper-V VM are represented as a VMBus
> >>> device and as a PCI device.  The coherence of the VMbus device is
> >>> set based on the VMbus node in ACPI, but the PCI device has no
> >>> ACPI node and defaults to not hardware coherent.  This results
> >>> in extra software coherence management overhead on ARM64 when
> >>> devices are hardware coherent.
> >>>
> >>> Fix this by propagating the coherence of the VMbus device to the
> >>> PCI device.  There's no effect on x86/x64 where devices are
> >>> always hardware coherent.
> >>>
> >>> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> >>> ---
> >>>    drivers/pci/controller/pci-hyperv.c | 17 +++++++++++++----
> >>>    1 file changed, 13 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> >>> index ae0bc2f..14276f5 100644
> >>> --- a/drivers/pci/controller/pci-hyperv.c
> >>> +++ b/drivers/pci/controller/pci-hyperv.c
> >>> @@ -49,6 +49,7 @@
> >>>    #include <linux/refcount.h>
> >>>    #include <linux/irqdomain.h>
> >>>    #include <linux/acpi.h>
> >>> +#include <linux/dma-map-ops.h>
> >>>    #include <asm/mshyperv.h>
> >>>
> >>>    /*
> >>> @@ -2142,9 +2143,9 @@ static void hv_pci_remove_slots(struct hv_pcibus_device
> >> *hbus)
> >>>    }
> >>>
> >>>    /*
> >>> - * Set NUMA node for the devices on the bus
> >>> + * Set NUMA node and DMA coherence for the devices on the bus
> >>>     */
> >>> -static void hv_pci_assign_numa_node(struct hv_pcibus_device *hbus)
> >>> +static void hv_pci_assign_properties(struct hv_pcibus_device *hbus)
> >>>    {
> >>>    	struct pci_dev *dev;
> >>>    	struct pci_bus *bus = hbus->bridge->bus;
> >>> @@ -2167,6 +2168,14 @@ static void hv_pci_assign_numa_node(struct
> >> hv_pcibus_device *hbus)
> >>>    				     numa_map_to_online_node(
> >>>    					     hv_dev->desc.virtual_numa_node));
> >>>
> >>> +		/*
> >>> +		 * On ARM64, propagate the DMA coherence from the VMbus device
> >>> +		 * to the corresponding PCI device. On x86/x64, these calls
> >>> +		 * have no effect because DMA is always hardware coherent.
> >>> +		 */
> >>> +		dev_set_dma_coherent(&dev->dev,
> >>> +			dev_is_dma_coherent(&hbus->hdev->device));
> >>
> >> Eww... if you really have to do this, I'd prefer to see a proper
> >> hv_dma_configure() helper implemented and wired up to
> >> pci_dma_configure(). Although since it's a generic property I guess at
> >> worst pci_dma_configure could perhaps propagate coherency from the host
> >> bridge to its children by itself in the absence of any other firmware
> >> info. And it's built-in so could use arch_setup_dma_ops() like everyone
> >> else.
> >>
> >
> > I'm not seeing an existing mechanism to provide a "helper" or override
> > of pci_dma_configure().   Could you elaborate?  Or is this something
> > that needs to be created?
> 
> I mean something like the diff below (other #includes omitted for
> clarity). Essentially if VMBus has its own way of describing parts of
> the system, then for those parts it's nice if it could fit into the same
> abstractions we use for firmware-based system description.

OK, got it.  Adding the VMbus special case into pci_dma_configure()
would work.  But after investigating further, let me throw out two
other possible solutions as well.

1)  It's easy to make the top-level VMbus node in the DSDT be
the ACPI companion for the pci_host bridge.  The function
pcibios_root_bridge_prepare() will do this if the pci-hyperv.c
driver sets sysdata->parent to that top-level VMbus node.  Then
pci_dma_configure()works as is.  Also, commit 7d40c0f70 that
special cases pcibios_root_bridge_prepare() for Hyper-V becomes
unnecessary.   I've coded this approach and it seems to work, but
I don't know all the implications of making that VMbus node be
the ACPI companion, potentially for multiple pci_host bridges.

2)  Since there's no vIOMMU and we don't know how it will be
described if there should be one in the future, it's a bit premature
to try to set things up "correctly" now to handle it.  A simple
approach is to set  dma_default_coherent to true if the top-level
VMbus node in the DSDT indicates coherent.  No other changes are
needed.  If there's a vIOMMU at some later time, then we add the
use of arch_setup_dma_ops() for each device.

Any thoughts on these two approaches vs. adding the VMbus
special case into pci_dma_configure()?  My preference would be
to avoid adding that special case if one of the other two approaches
is reasonable.

Michael

> 
> Cheers,
> Robin.
> 
> ----->8-----
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 588588cfda48..7d92ccad1569 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -20,6 +20,7 @@
>   #include <linux/of_device.h>
>   #include <linux/acpi.h>
>   #include <linux/dma-map-ops.h>
> +#include <linux/hyperv.h>
>   #include "pci.h"
>   #include "pcie/portdrv.h"
> 
> @@ -1602,6 +1603,8 @@ static int pci_dma_configure(struct device *dev)
>   		struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
> 
>   		ret = acpi_dma_configure(dev, acpi_get_dma_attr(adev));
> +	} else if (is_vmbus_dev(bridge)) {
> +		ret = hv_dma_configure(dev, device_get_dma_attr(bridge));
>   	}
> 
>   	pci_put_host_bridge_device(bridge);
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index f565a8938836..d1d4dd3d5a3a 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1764,4 +1764,19 @@ static inline unsigned long virt_to_hvpfn(void *addr)
>   #define HVPFN_DOWN(x)	((x) >> HV_HYP_PAGE_SHIFT)
>   #define page_to_hvpfn(page)	(page_to_pfn(page) *
> NR_HV_HYP_PAGES_IN_PAGE)
> 
> +static inline bool is_vmbus_dev(struct device *dev)
> +{
> +	/*
> +	 * dev->bus == &hv_bus would break when the caller is built-in
> +	 * and CONFIG_HYPERV=m, so look for it by name instead.
> +	 */
> +	return !strcmp(dev->bus->name, "vmbus");
> +}
> +
> +static inline int hv_dma_configure(struct device *dev, enum
> dev_dma_attr attr)
> +{
> +	arch_setup_dma_ops(dev, 0, 0, NULL, attr == DEV_DMA_COHERENT);
> +	return 0;
> +}
> +
>   #endif /* _HYPERV_H */

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 2/4 RESEND] dma-mapping: Add wrapper function to set dma_coherent
  2022-03-18 11:07       ` Robin Murphy
@ 2022-03-18 20:37         ` Michael Kelley (LINUX)
  0 siblings, 0 replies; 16+ messages in thread
From: Michael Kelley (LINUX) @ 2022-03-18 20:37 UTC (permalink / raw)
  To: Robin Murphy, Stephen Hemminger, KY Srinivasan, Haiyang Zhang,
	wei.liu, Dexuan Cui, rafael, lenb, lorenzo.pieralisi, robh, kw,
	bhelgaas, hch, m.szyprowski, linux-acpi, linux-kernel,
	linux-hyperv, linux-pci, iommu

From: Robin Murphy <robin.murphy@arm.com> Sent: Friday, March 18, 2022 4:08 AM
> 
> On 2022-03-17 19:13, Michael Kelley (LINUX) wrote:
> > From: Robin Murphy <robin.murphy@arm.com> Sent: Thursday, March 17, 2022
> 10:20 AM
> >>
> >> On 2022-03-17 16:25, Michael Kelley via iommu wrote:
> >>> Add a wrapper function to set dma_coherent, avoiding the need for
> >>> complex #ifdef's when setting it in architecture independent code.
> >>
> >> No. It might happen to work out on the architectures you're looking at,
> >> but if Hyper-V were ever to support, say, AArch32 VMs you might see the
> >> problem. arch_setup_dma_ops() is the tool for this job.
> >>
> >
> > OK.   There's currently no vIOMMU in a Hyper-V guest, so presumably the
> > code would call arch_setup_dma_ops() with the dma_base, size, and iommu
> > parameters set to 0 and NULL.  This call can then be used in Patch 3 instead
> > of acpi_dma_configure(), and in the Patch 4 hv_dma_configure() function
> > as you suggested.  arch_setup_dma_ops() is not exported, so I'd need to
> > wrap it in a Hyper-V specific function in built-in code that is exported.
> >
> > But at some point in the future if there's a vIOMMU in Hyper-V guests,
> > this approach will need some rework.
> >
> > Does that make sense?  Thanks for your input and suggestions ...
> 
> Yes, that's essentially what I had in mind. If you did present a vIOMMU
> to the guest, presumably you'd either have to construct a regular
> IORT/VIOT, and thus end up adding the root complex to the ACPI namespace
> too so it can be referenced, at which point it would all get picked up
> by the standard machinery, or come up with some magic VMBus mechanism
> that would need a whole load of work to wire up in all the relevant
> places anyway.
> 
> (But please lean extremely heavily towards the former option!)

Agreed!

> 
> Thanks,
> Robin.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-03-18 20:37 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-17 16:25 [PATCH 0/4 RESEND] Fix coherence for VMbus and PCI pass-thru devices in Hyper-V VM Michael Kelley
2022-03-17 16:25 ` [PATCH 1/4 RESEND] ACPI: scan: Export acpi_get_dma_attr() Michael Kelley
2022-03-17 16:31   ` Robin Murphy
2022-03-17 18:56     ` Michael Kelley (LINUX)
2022-03-17 16:25 ` [PATCH 2/4 RESEND] dma-mapping: Add wrapper function to set dma_coherent Michael Kelley
2022-03-17 17:19   ` Robin Murphy
2022-03-17 19:13     ` Michael Kelley (LINUX)
2022-03-18 11:07       ` Robin Murphy
2022-03-18 20:37         ` Michael Kelley (LINUX)
2022-03-17 16:25 ` [PATCH 3/4 RESEND] Drivers: hv: vmbus: Propagate VMbus coherence to each VMbus device Michael Kelley
2022-03-17 16:52   ` Robin Murphy
2022-03-17 16:25 ` [PATCH 4/4 RESEND] PCI: hv: Propagate coherence from VMbus device to PCI device Michael Kelley
2022-03-17 17:15   ` Robin Murphy
2022-03-18  5:12     ` Michael Kelley (LINUX)
2022-03-18 10:57       ` Robin Murphy
2022-03-18 20:36         ` Michael Kelley (LINUX)

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).