Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [RFC PATCH 6/6] arm/xen: Assign xen-virtio DMA ops for virtio
 devices in Xen guests
To:     Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>
Cc:     Christoph Hellwig <hch@infradead.org>,
        xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        Julien Grall <julien@xen.org>,
        "Michael S. Tsirkin" <mst@redhat.com>
References: <1649963973-22879-1-git-send-email-olekstysh@gmail.com>
 <1649963973-22879-7-git-send-email-olekstysh@gmail.com>
 <alpine.DEB.2.22.394.2204151305050.915916@ubuntu-linux-20-04-desktop>
 <YlpdBHKT1bYzZe2e@infradead.org>
 <f879622e-5656-deb1-1930-f0cd180a4ab1@gmail.com>
 <alpine.DEB.2.22.394.2204181202080.915916@ubuntu-linux-20-04-desktop>
 <6a04cc34-fbb3-44d8-c1a4-03bda5b3deb1@gmail.com>
 <b68163be-ad43-7773-22ff-e83191886626@suse.com>
From:   Oleksandr <olekstysh@gmail.com>
Message-ID: <5afb9e61-4164-9cc9-278a-911fc21f4f6c@gmail.com>
Date:   Tue, 19 Apr 2022 20:11:12 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <b68163be-ad43-7773-22ff-e83191886626@suse.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk


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