|
From: | Maxime Coquelin |
Subject: | Re: [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier |
Date: | Thu, 18 May 2017 16:45:23 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0 |
Hi Michael, On 05/18/2017 09:35 AM, Maxime Coquelin wrote:
On 05/17/2017 06:41 PM, Michael S. Tsirkin wrote:On Fri, May 12, 2017 at 01:21:18PM +0200, Maxime Coquelin wrote:On 05/11/2017 07:33 PM, Michael S. Tsirkin wrote:On Thu, May 11, 2017 at 02:32:43PM +0200, Maxime Coquelin wrote:Vhost-kernel backend need to receive IOTLB entries for rings information early, but vhost-user need the same information earlier, before VHOST_USER_SET_VRING_ADDR is sent.Weird. What does VHOST_USER_SET_VRING_ADDR have to do with it? According to Starting and stopping rings in vhost user spec, vhost user does not access anything until ring is started and enabled.This patch also trigger IOTLB miss for all rings informations for robustness, even if in practice these adresses are on the same page.Actually, the DPDK vhost-user backend is compliant with the spec, but when handling VHOST_USER_SET_VRING_ADDR request, it translates theguest addresses into backend VAs, and check they are valid. I will make thecommit message clearer about this in next revision. The check could be done later, for example when the ring are started, but it wouldn't change the need to trigger a miss at some point.I think it should be done later, yes. As long as ring is not started addresses should not be interpreted.Ok, then I'll move these addresses translations in the VHOST_USER_SET_VRING_KICK handler.
s/VHOST_USER_SET_VRING_KICK/VHOST_USER_SET_VRING_ENABLE/ I just looked at implementing this change, but I'm not convinced this is the right thing to do. On backend side, it means saving temporarily the vhost_vring_addr struct into the vq struct, and moving all what is done currently in SET_VRING_ADDR handler to SET_VRING_ENABLE one. My understanding of the "Starting and stopping rings" chapter of the spec is that the ring must not be processed as long as not started and enabled, not that the addresses passed should not be checked/translated as it is done today both in DPDK and libvhost-user. If the addresses are invalid, isn't it better to know as soon as possible? Cheers, Maxime
[Prev in Thread] | Current Thread | [Next in Thread] |