Hello Stefano, Juergen
On 19.04.22 17:48, Juergen Gross wrote:
On 19.04.22 14:17, Oleksandr wrote:
Hello Stefano, Juergen
On 18.04.22 22:11, Stefano Stabellini wrote:
On Mon, 18 Apr 2022, Oleksandr wrote:
On 16.04.22 09:07, Christoph Hellwig wrote:
Hello Christoph
On Fri, Apr 15, 2022 at 03:02:45PM -0700, Stefano Stabellini wrote:
This makes sense overall. Considering that the swiotlb-xen case
and the
virtio case are mutually exclusive, I would write it like this:
Curious question: Why can't the same grant scheme also be used for
non-virtio devices? I really hate having virtio hooks in the arch
dma code. Why can't Xen just say in DT/ACPI that grants can be used
for a given device?
[...]
This patch series tries to make things work with "virtio" devices
in Xen
system without introducing any modifications to code under
drivers/virtio.
Actually, I think Christoph has a point.
There is nothing inherently virtio specific in this patch series or in
the "xen,dev-domid" device tree binding.
Although the main intention of this series was to enable using virtio
devices in Xen guests, I agree that nothing in new DMA ops layer
(xen-virtio.c) is virtio specific (at least at the moment). Regarding
the whole patch series I am not quite sure, as it uses
arch_has_restricted_virtio_memory_access(). >
Assuming a given device is
emulated by a Xen backend, it could be used with grants as well.
For instance, we could provide an emulated e1000 NIC with a
"xen,dev-domid" property in device tree. Linux could use grants with it
and the backend could map the grants. It would work the same way as
virtio-net/block/etc. Passthrough devices wouldn't have the
"xen,dev-domid" property, so no problems.
So I think we could easily generalize this work and expand it to any
device. We just need to hook on the "xen,dev-domid" device tree
property.
I think it is just a matter of:
- remove the "virtio,mmio" check from xen_is_virtio_device
- rename xen_is_virtio_device to something more generic, like
xen_is_grants_device
xen_is_grants_dma_device, please. Normal Xen PV devices are covered by
grants, too, and I'd like to avoid the confusion arising from this.
yes, this definitely makes sense as we need to distinguish
- rename xen_virtio_setup_dma_ops to something more generic, like
xen_grants_setup_dma_ops
And that's pretty much it.
+ likely renaming everything in that patch series not to mention
virtio (mostly related to xen-virtio.c internals).
Stefano, thank you for clarifying Christoph's point.
Well, I am not against going this direction. Could we please make a
decision on this? @Juergen, what is your opinion?
Yes, why not.
ok, thank you for confirming.
Maybe rename xen-virtio.c to grant-dma.c?
Personally I don't mind.
I'd keep the XEN_VIRTIO related config option, as this will be the
normal use
case. grant-dma.c should be covered by a new hidden config option
XEN_GRANT_DMA
selected by XEN_VIRTIO.
I got it, ok
CONFIG_XEN_VIRTIO should still guard
xen_has_restricted_virtio_memory_access().
ok
So a few questions to clarify:
1. What is the best place to keep "xen,dev-domid" binding's description
now? I think that proposed in current series place
(Documentation/devicetree/bindings/virtio/) is not good fit now.
2. I assume the logic in the current patch will remain the same, I mean
we will still assign Xen grant DMA ops from xen_setup_dma_ops() here?
Juergen
--
Regards,
Oleksandr Tyshchenko
