On 2020/2/24 下午9:56, Halil Pasic wrote:
On Mon, 24 Feb 2020 17:26:20 +0800
Jason Wang <jasowang@xxxxxxxxxx> wrote:
That's better.
How about attached?
Thanks
Thanks Jason! It does avoid the translation overhead in vhost.
Tested-by: Halil Pasic <pasic@xxxxxxxxxxxxx>
Regarding the code, you fence it in virtio-net.c, but AFAIU this feature
has relevance for other vhost devices as well. E.g. what about vhost
user? Would it be the responsibility of each virtio device to fence this
on its own?
Yes, it looks to me it's better to do that in virtio_set_features_nocheck()
I'm also a bit confused about the semantics of the vhost feature bit
F_ACCESS_PLATFORM. What we have specified on virtio level is:
"""
This feature indicates that the device can be used on a platform where
device access to data in memory is limited and/or translated. E.g. this
is the case if the device can be located behind an IOMMU that translates
bus addresses from the device into physical addresses in memory, if the
device can be limited to only access certain memory addresses or if
special commands such as a cache flush can be needed to synchronise data
in memory with the device. Whether accesses are actually limited or
translated is described by platform-specific means. If this feature bit
is set to 0, then the device has same access to memory addresses
supplied to it as the driver has. In particular, the device will always
use physical addresses matching addresses used by the driver (typically
meaning physical addresses used by the CPU) and not translated further,
and can access any address supplied to it by the driver. When clear,
this overrides any platform-specific description of whether device
access is limited or translated in any way, e.g. whether an IOMMU may be
present.
"""
I read this like the addresses may be IOVAs which require
IMMU translation or GPAs which don't.
On the vhost level however, it seems that F_IOMMU_PLATFORM means that
vhost has to do the translation (via IOTLB API).
Yes.
Do I understand this correctly? If yes, I believe we should document
this properly.
Good point. I think it was probably wrong to tie F_IOMMU_PLATFORM to
IOTLB API. Technically IOTLB can work with GPA->HVA mapping. I
originally use a dedicated feature bit (you can see that from commit
log), but for some reason Michael tweak it to virtio feature bit. I
guess it was probably because at that time there's no way to specify e.g
backend capability but now we have VHOST_GET_BACKEND_FEATURES.
For now, it was probably too late to fix that but document or we can add
the support of enabling IOTLB via new backend features.
BTW I'm still not 100% on the purpose and semantics of the
F_ACCESS_PLATFORM feature bit. But that is a different problem.
Yes, I aggree that we should decouple the features that does not belongs
to device (protected, encrypted, swiotlb etc) from F_IOMMU_PLATFORM. But
Michael and other have their points as well.
Thanks
Regards,
Halil