Re: [RFC 2/4] virtio: Override device's DMA OPS with virtio_direct_dma_ops selectively
On Sat, Jul 28, 2018 at 02:26:24PM +0530, Anshuman Khandual wrote:
> On 07/20/2018 09:29 AM, Anshuman Khandual wrote:
> > Now that virtio core always needs all virtio devices to have DMA OPS, we
> > need to make sure that the structure it points is the right one. In the
> > absence of VIRTIO_F_IOMMU_PLATFORM flag QEMU expects GPA from guest kernel.
> > In such case, virtio device must use default virtio_direct_dma_ops DMA OPS
> > structure which transforms scatter gather buffer addresses as GPA. This
> > DMA OPS override must happen as early as possible during virtio device
> > initializatin sequence before virtio core starts using given device's DMA
> > OPS callbacks for I/O transactions. This change detects device's IOMMU flag
> > and does the override in case the flag is cleared.
> >
> > Signed-off-by: Anshuman Khandual
> > ---
> > drivers/virtio/virtio.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> > index 7907ad3..6b13987 100644
> > --- a/drivers/virtio/virtio.c
> > +++ b/drivers/virtio/virtio.c
> > @@ -166,6 +166,8 @@ void virtio_add_status(struct virtio_device *dev,
> > unsigned int status)
> > }
> > EXPORT_SYMBOL_GPL(virtio_add_status);
> >
> > +const struct dma_map_ops virtio_direct_dma_ops;
> > +
> > int virtio_finalize_features(struct virtio_device *dev)
> > {
> > int ret = dev->config->finalize_features(dev);
> > @@ -174,6 +176,9 @@ int virtio_finalize_features(struct virtio_device *dev)
> > if (ret)
> > return ret;
>
>
> The previous patch removed the code block for XEN guests which forced
> the use of DMA API all the time irrespective of VIRTIO_F_IOMMU_PLATFORM
> flag on the device. Here is what I have removed with patch 2/4 which
> breaks the existing semantics on XEN guests.
>
> -static bool vring_use_dma_api(struct virtio_device *vdev)
> -{
> - if (!virtio_has_iommu_quirk(vdev))
> - return true;
> -
> - /* Otherwise, we are left to guess. */
> - /*
> - * In theory, it's possible to have a buggy QEMU-supposed
> - * emulated Q35 IOMMU and Xen enabled at the same time. On
> - * such a configuration, virtio has never worked and will
> - * not work without an even larger kludge. Instead, enable
> - * the DMA API if we're a Xen guest, which at least allows
> - * all of the sensible Xen configurations to work correctly.
> - */
> - if (xen_domain())
> - return true;
> -
> - return false;
> -}
>
> XEN guests would not like override with virtio_direct_dma_ops in any
> case irrespective of the flag VIRTIO_F_IOMMU_PLATFORM. So the existing
> semantics can be preserved with something like this. It just assumes
> that dev->dma_ops is non-NULL and a valid one set by the architecture.
> If required we can add those tests here before skipping the override.
>
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 7907ad3..6b13987 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -166,6 +166,8 @@ void virtio_add_status(struct virtio_device *dev,
> unsigned int status)
> }
> EXPORT_SYMBOL_GPL(virtio_add_status);
>
> +const struct dma_map_ops virtio_direct_dma_ops;
> +
> int virtio_finalize_features(struct virtio_device *dev)
> {
> int ret = dev->config->finalize_features(dev);
> @@ -174,6 +176,9 @@ int virtio_finalize_features(struct virtio_device *dev)
> if (ret)
> return ret;
> +
> + if (xen_domain())
> + goto skip_override;
> +
> + if (virtio_has_iommu_quirk(dev))
> + set_dma_ops(dev->dev.parent, _direct_dma_ops);
> +
> + skip_override:
> +
I prefer normal if scoping as opposed to goto spaghetti pls.
Better yet move vring_use_dma_api here and use it.
Less of a chance something will break.
> if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1))
> return 0
>
> Will incorporate these changes in the next version.
Re: [RFC 2/4] virtio: Override device's DMA OPS with virtio_direct_dma_ops selectively
On 07/20/2018 09:29 AM, Anshuman Khandual wrote:
> Now that virtio core always needs all virtio devices to have DMA OPS, we
> need to make sure that the structure it points is the right one. In the
> absence of VIRTIO_F_IOMMU_PLATFORM flag QEMU expects GPA from guest kernel.
> In such case, virtio device must use default virtio_direct_dma_ops DMA OPS
> structure which transforms scatter gather buffer addresses as GPA. This
> DMA OPS override must happen as early as possible during virtio device
> initializatin sequence before virtio core starts using given device's DMA
> OPS callbacks for I/O transactions. This change detects device's IOMMU flag
> and does the override in case the flag is cleared.
>
> Signed-off-by: Anshuman Khandual
> ---
> drivers/virtio/virtio.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 7907ad3..6b13987 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -166,6 +166,8 @@ void virtio_add_status(struct virtio_device *dev,
> unsigned int status)
> }
> EXPORT_SYMBOL_GPL(virtio_add_status);
>
> +const struct dma_map_ops virtio_direct_dma_ops;
> +
> int virtio_finalize_features(struct virtio_device *dev)
> {
> int ret = dev->config->finalize_features(dev);
> @@ -174,6 +176,9 @@ int virtio_finalize_features(struct virtio_device *dev)
> if (ret)
> return ret;
The previous patch removed the code block for XEN guests which forced
the use of DMA API all the time irrespective of VIRTIO_F_IOMMU_PLATFORM
flag on the device. Here is what I have removed with patch 2/4 which
breaks the existing semantics on XEN guests.
-static bool vring_use_dma_api(struct virtio_device *vdev)
-{
- if (!virtio_has_iommu_quirk(vdev))
- return true;
-
- /* Otherwise, we are left to guess. */
- /*
-* In theory, it's possible to have a buggy QEMU-supposed
-* emulated Q35 IOMMU and Xen enabled at the same time. On
-* such a configuration, virtio has never worked and will
-* not work without an even larger kludge. Instead, enable
-* the DMA API if we're a Xen guest, which at least allows
-* all of the sensible Xen configurations to work correctly.
-*/
- if (xen_domain())
- return true;
-
- return false;
-}
XEN guests would not like override with virtio_direct_dma_ops in any
case irrespective of the flag VIRTIO_F_IOMMU_PLATFORM. So the existing
semantics can be preserved with something like this. It just assumes
that dev->dma_ops is non-NULL and a valid one set by the architecture.
If required we can add those tests here before skipping the override.
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 7907ad3..6b13987 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -166,6 +166,8 @@ void virtio_add_status(struct virtio_device *dev, unsigned
int status)
}
EXPORT_SYMBOL_GPL(virtio_add_status);
+const struct dma_map_ops virtio_direct_dma_ops;
+
int virtio_finalize_features(struct virtio_device *dev)
{
int ret = dev->config->finalize_features(dev);
@@ -174,6 +176,9 @@ int virtio_finalize_features(struct virtio_device *dev)
if (ret)
return ret;
+
+ if (xen_domain())
+ goto skip_override;
+
+ if (virtio_has_iommu_quirk(dev))
+ set_dma_ops(dev->dev.parent, _direct_dma_ops);
+
+ skip_override:
+
if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1))
return 0
Will incorporate these changes in the next version.
Re: [RFC 0/4] Virtio uses DMA API for all devices
On 07/27/2018 05:01 PM, Michael S. Tsirkin wrote: > On Wed, Jul 25, 2018 at 08:56:23AM +0530, Anshuman Khandual wrote: >> Results with and without the patches are similar. > > Thanks! And another thing to try is virtio-net with > a fast NIC backend (40G and up). Unfortunately > at this point loopback tests stress the host > scheduler too much. > Sure. Will look around for a 40G NIC system. BTW I have been testing virtio-net with a TAP device as back end. ip tuntap add dev tap0 mode tap user $(whoami) ip link set tap0 master virbr0 ip link set dev virbr0 up ip link set dev tap0 up which is exported into the guest as follows -device virtio-net,netdev=network0,mac=52:55:00:d1:55:01 \ -netdev tap,id=network0,ifname=tap0,script=no,downscript=no \ But I have not run any network benchmarks on it though.
Re: [PATCH v4 00/11] macintosh: Resolve various PMU driver problems
No regressions on my PowerBook G4, so for this series: Tested-by: Michael Schmitz Am 02.07.2018 um 20:21 schrieb Finn Thain: This series of patches has the following aims. 1) Eliminate duplicated code. Linux presently has two drivers for the 68HC05-based PMU devices found in Macs: via-pmu and via-pmu68k. There's no value in having separate PMU drivers for each architecture. 2) Avoid further work on via-pmu68k that's not needed for via-pmu. 3) Fix some bugs in the via-pmu driver. 4) Enable the /dev/pmu and /proc/pmu/* userspace APIs on m68k Macs by adopting via-pmu. 5) Improve stability on early 100-series PowerBooks by loading no PMU driver at all. Neither via-pmu nor via-pmu68k supports the early M50753-based PMU device found in these models. 6) Assist the out-of-tree NuBus PowerMac port to support PMU designs shared with the m68k Mac port (e.g. PowerBooks 190 and 5300). This patch series has been regression tested on various PowerBooks (190, 520, 3400, Pismo G3) and PowerMacs (Beige G3, G5). These patches did not affect userland utilities. (Note that there is a userland- visible change to the contents of /proc/pmu/interrupts.) Changed since v1: 1) Added blank lines after 'break' statements in patch 10. 2) Improved patch description for patch 3. 3) Added reviewed-by tags. 4) Split patch 8 to make code review easier. Changed since v2: 1) Added reviewed-by tag. 2) Retained PMU_68K_V1 and PMU_68K_V2 symbols. Changed since v3: 1) Rebased on v4.18-rc2. 2) Omitted patch 10/12, since these RTC changes now conflict with mainline. It will be reworked once the mainline m68k/powerpc RTC code stabilizes. Finn Thain (11): macintosh/via-pmu: Fix section mismatch warning macintosh/via-pmu: Add missing mmio accessors macintosh/via-pmu: Don't clear shift register interrupt flag twice macintosh/via-pmu: Enhance state machine with new 'uninitialized' state macintosh/via-pmu: Replace via pointer with via1 and via2 pointers macintosh/via-pmu: Add support for m68k PowerBooks macintosh/via-pmu: Explicitly specify CONFIG_PPC_PMAC dependencies macintosh/via-pmu68k: Don't load driver on unsupported hardware macintosh/via-pmu: Replace via-pmu68k driver with via-pmu driver macintosh/via-pmu: Clean up interrupt statistics macintosh/via-pmu: Disambiguate interrupt statistics arch/m68k/configs/mac_defconfig | 2 +- arch/m68k/configs/multi_defconfig | 2 +- arch/m68k/mac/config.c| 2 +- arch/m68k/mac/misc.c | 54 +-- drivers/macintosh/Kconfig | 19 +- drivers/macintosh/Makefile| 1 - drivers/macintosh/adb.c | 2 +- drivers/macintosh/via-pmu.c | 346 ++-- drivers/macintosh/via-pmu68k.c| 850 -- include/uapi/linux/pmu.h | 4 +- 10 files changed, 235 insertions(+), 1047 deletions(-) delete mode 100644 drivers/macintosh/via-pmu68k.c
