From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 15796A00BE; Tue, 29 Oct 2019 11:02:30 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 326C81BEBF; Tue, 29 Oct 2019 11:02:29 +0100 (CET) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 120021BE96 for ; Tue, 29 Oct 2019 11:02:28 +0100 (CET) Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1A47F8553D for ; Tue, 29 Oct 2019 10:02:27 +0000 (UTC) Received: by mail-wm1-f69.google.com with SMTP id 14so315374wmk.4 for ; Tue, 29 Oct 2019 03:02:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:cc:references:from:autocrypt:subject :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=pwefXM/a/tDw2WLzDr5TYeAaIeZdk3cb4y9nt9ywDAk=; b=AhcsdosMTVKrg1zg690GiNo1HzqNTfw0fPyvnT06nfHZ2UTHUnbQ2tugbOmINMA4Lg 8lEz7WcK/QLfTrQ7ckgLEayewyhR+rpWhU82v8Q7YIrhQwhgBuxI20Au12FjY2TAyqh2 hEJTO1y9s9cCOwxG/t7JNQQRHDClDz6aa96CntqJ+6g1KV0mbhD4VIDOynDWFrL0Ofnn 3jQ/bEA7itrIPoErdv+j9JfTUHjjQIXbWn9Z9e5pZMh6TMItCbKxOex5ghTCqWLggf++ 8U+su2vr3YYME+iGPIx5f/EWmKoxeUmQoXSaXH5pfRZf1PlbnMhK9CqjpQ0yVegb1Azj 8H2g== X-Gm-Message-State: APjAAAVPRKRgx9o5JcDoXAokJ9eWQVIgc2DcPSKHOCyHsOZcSqHmjnz0 NT7Bh+vlBTIUVc26PSuUrlC2MMIH3FSfyisgsllkkaGputo8gaCdurAAdoMO0an2PWELZKyHksK CKUk= X-Received: by 2002:a5d:4285:: with SMTP id k5mr18671557wrq.344.1572343345733; Tue, 29 Oct 2019 03:02:25 -0700 (PDT) X-Google-Smtp-Source: APXvYqxcmSOXoIE0rd03Pv5gt5gS6sVPnr8Fre7BVtsq37PV9uQesEU4qR3iWIecVJhc0dTM3Jdn6A== X-Received: by 2002:a5d:4285:: with SMTP id k5mr18671542wrq.344.1572343345541; Tue, 29 Oct 2019 03:02:25 -0700 (PDT) Received: from amorenoz.users.ipa.redhat.com ([46.6.233.201]) by smtp.gmail.com with ESMTPSA id l15sm2022054wmh.18.2019.10.29.03.02.23 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 29 Oct 2019 03:02:24 -0700 (PDT) To: "Liu, Yong" Cc: "dev@dpdk.org" , "maxime.coquelin@redhat.com" , "Bie, Tiwei" , "Wang, Zhihong" References: <20191025162042.71687-1-yong.liu@intel.com> <66e350e2-4d34-7a17-afa2-d87eea47a37e@redhat.com> <86228AFD5BCD8E4EBFD2B90117B5E81E633E5471@SHSMSX103.ccr.corp.intel.com> From: Adrian Moreno Autocrypt: addr=amorenoz@redhat.com; prefer-encrypt=mutual; keydata= mQENBF1syNUBCADQ9dk3fDMxOZ/+OQpmbanpodYxEv8IRtDz8PXw8YX7UyGfozOpLjQ8Fftj ZxuubYNbt2QVbSgviFilFdNWu2eTnN/JaGtfhmTOLPVoakkPHZF8lbgImMoch7L0fH8wN2IM KPxQyPNlX+K9FD5brHsV1lfe1TwAxmhcvLW8yNrVq+9eDIDykxc7tH4exIqXgZroahGxMHKy c8Ti2kJka/t6pDfRaY0J+6J7I1nrn6GXXSMNA45EH8+0N/QlcXhP3rfftnoPeVmpjswzvJqY FNjf/Q5VPLx7RX0Qx+y8mMB2JcChV5Bl7D7x5EUbItj6+Sy7QfOgCtPegk9HSrBCNYaLABEB AAG0I0FkcmlhbiBNb3Jlbm8gPGFtb3Jlbm96QHJlZGhhdC5jb20+iQFUBBMBCAA+FiEEogUD gihhmbOPHy26d5C5fbYeFsUFAl1syNUCGwMFCQHhM4AFCwkIBwIGFQoJCAsCBBYCAwECHgEC F4AACgkQd5C5fbYeFsX7qwgArGHSkX+ILNcujkVzjTG4OtkpJMPFlkn/1PxSEKD0jLuzx14B COzpg/Mqj3Re/QBuOas+ci9bsUA0/2nORtmmEBvzDOJpR5FH1jaGCx8USlY4WM6QqEDNZgTw hsy9KhjFzFjMk+oo3HyItXA+Uq9yrRBTjNBGTXxezMRcMuUZ4MIAfY0IRBglL2BufiuL43jD BvTENNFLoQ/wFV7qkFWSkv+8IjTsxr7M6XUo1QLd29Hn0dvwssN579HL1+BP46i2REpzeBEG L75iVChi+YnIQQNMJ9NYarVabZx4Y1Gn8+7B/1SNArDV+IDgnYgt7E58otoV2Ap310dmtuvE VbxGpbkBDQRdbMjVAQgAqyp9oA7WDu7/Y9T4Ommt69iZx8os7shUIfdgPEy5xrcPn6qGwN1/ HQ4j8nWfBG9uuX1X0RXUZIUEtYTxtED4yaCQMTqDUf9cBAwAA2mYxBfoiNYx8YqxM+sT0/J4 2qmDd+y+20UR4yzHE8AmIbspTzDFIJDAi+jKSR8F355z0sfW7CIMDC4ZWrPsskjEy1YN/U10 r6tRRH1kNyrCSbTG0d9MtcQO58h7DLXuzUhErB+BtG52A04t5cweIJTJC+koV5XPeilzlHnm RFoj0ncruGa9Odns21BNt3cy9wLfK+aUnWuAB1uc6bJGQPiAwjkilz7g7MBRUuIQ2Zt7HGLc SwARAQABiQE8BBgBCAAmFiEEogUDgihhmbOPHy26d5C5fbYeFsUFAl1syNUCGwwFCQHhM4AA CgkQd5C5fbYeFsUlSwf8CH+u/IXaE7WeWxwFkMaORfW8cM4q0xrL3M6yRGuQNW+kMjnrvK9U J9G+L1/5uTRbDQ/4LdoKqize8LjehA+iF6ba4t9Npikh8fLKWgaJfQ/hPhH4C3O5gWPOLTW6 ylGxiuER4CdFwQIoAMMslhFA7G+teeOKBq36E+1+zrybI6Xy1UBSlpDK9j4CtTnMQejjuSQb Qhle+l8VroaUHq869wjAhRHHhqmtJKggI+OvzgQpDIwfHIDypb1BuKydi2W6cVYEALUYyCLS dTBDhzj8zR5tPCsga8J7+TclQzkWOiI2C6ZtiWrMsL/Uym3uXk5nsoc7lSj7yLd/MrBRhYfP JQ== Message-ID: Date: Tue, 29 Oct 2019 11:02:20 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.0 MIME-Version: 1.0 In-Reply-To: <86228AFD5BCD8E4EBFD2B90117B5E81E633E5471@SHSMSX103.ccr.corp.intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] vhost: fix vhost user virtqueue not accessable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/28/19 3:13 AM, Liu, Yong wrote: > > >> -----Original Message----- >> From: Adrian Moreno [mailto:amorenoz@redhat.com] >> Sent: Friday, October 25, 2019 8:21 PM >> To: Liu, Yong ; maxime.coquelin@redhat.com; Bie, Tiwei >> ; Wang, Zhihong >> Cc: dev@dpdk.org >> Subject: Re: [PATCH] vhost: fix vhost user virtqueue not accessable >> >> Hi Marvin, >> >> On 10/25/19 6:20 PM, Marvin Liu wrote: >>> Log feature is disabled in vhost user, so that log address was invalid >>> when checking. Add feature bit check can skip useless address check. >>> >> Just so I understand, what conditions is the log address invalid? >> >>> Fixes: 04cfc7fdbfca ("vhost: translate incoming log address to gpa") >>> >>> Signed-off-by: Marvin Liu >>> --- >>> lib/librte_vhost/vhost_user.c | 16 +++++++++------- >>> 1 file changed, 9 insertions(+), 7 deletions(-) >>> >>> diff --git a/lib/librte_vhost/vhost_user.c >> b/lib/librte_vhost/vhost_user.c >>> index 61ef699ac..0407fdc29 100644 >>> --- a/lib/librte_vhost/vhost_user.c >>> +++ b/lib/librte_vhost/vhost_user.c >>> @@ -741,13 +741,15 @@ translate_ring_addresses(struct virtio_net *dev, >> int vq_index) >>> vq->last_avail_idx = vq->used->idx; >>> } >>> >>> - vq->log_guest_addr = >>> - translate_log_addr(dev, vq, addr->log_guest_addr); >>> - if (vq->log_guest_addr == 0) { >>> - RTE_LOG(DEBUG, VHOST_CONFIG, >>> - "(%d) failed to map log_guest_addr .\n", >>> - dev->vid); >>> - return dev; >>> + if (dev->features & (1ULL << VHOST_F_LOG_ALL)) { >>> + vq->log_guest_addr = >>> + translate_log_addr(dev, vq, addr->log_guest_addr); >> >> VHOST_F_LOG_ALL is only negotiated once the migration has started (at least >> from >> qemu's perspective). >> That means that we will postponing the translation of the log address to >> the >> vhost_user_set_vring_addr() call that follows the VHOST_F_LOG_ALL enabling. >> In >> that call there are (at least) two things that could go wrong and lead to a >> migration failure: >> - If VHOST_USER_F_PROTOCOL_FEATURES is not enabled, the address won't be >> translated: >> >> vhost_user:795 >> if ((vq->enabled && (dev->features & >> (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) || >> access_ok) { >> dev = translate_ring_addresses(dev, msg->payload.addr.index); >> if (!dev) >> return RTE_VHOST_MSG_RESULT_ERR; >> >> *pdev = dev; >> } >> >> - If the IOMMU is enabled and there's a miss, we would have to wait for the >> IOTLB_UPDATE and during that time, there would be failed accesses to the >> (still >> untranslated) log address. >> >> > > Thanks, Adrian. > Log address can be zero when logging is not enabled. > How about add other criteria after translation? Log address will be translated anyway and not affect vq status. > > vq->log_guest_addr = > translate_log_addr(dev, vq, addr->log_guest_addr); > - if (vq->log_guest_addr == 0) { > + if (vq->log_guest_addr == 0 && addr->flags) { > RTE_LOG(DEBUG, VHOST_CONFIG, > "(%d) failed to map log_guest_addr .\n", > dev->vid); > return dev; > } > That makes perfect sense IMHO. Thank you. > Meanwhile, log address of packed ring is fixed to zero. Is any special reason to do that? > No idea to be honest. Thanks. Adrian > Regards, > Marvin > >> >>> + if (vq->log_guest_addr == 0) { >>> + RTE_LOG(DEBUG, VHOST_CONFIG, >>> + "(%d) failed to map log_guest_addr .\n", >>> + dev->vid); >>> + return dev; >>> + } >>> } >>> vq->access_ok = 1; >>> >>> >> Thanks, >> Adrian