| Message ID | 1647534311-2349-3-git-send-email-mikelley@microsoft.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix coherence for VMbus and PCI pass-thru devices in Hyper-V VM | expand |
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,
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
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.
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.
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,
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(+)