Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 20 Jul 2020 08:19:40 +0300
From:   Eli Cohen <eli@mellanox.com>
To:     Jason Wang <jasowang@redhat.com>
Cc:     mst@redhat.com, virtualization@lists.linux-foundation.org,
        linux-kernel@vger.kernel.org, shahafs@mellanox.com,
        saeedm@mellanox.com, Parav Pandit <parav@mellanox.com>,
        Eugenio Perez Martin <eperezma@redhat.com>
Subject: Re: [PATCH vhost next 10/10] vdpa/mlx5: Add VDPA driver for
 supported mlx5 devices
Message-ID: <20200720051940.GA116013@mtl-vdi-166.wap.labs.mlnx>
References: <20200716072327.5359-1-eli@mellanox.com>
 <20200716072327.5359-11-eli@mellanox.com>
 <454850cc-269b-72aa-4511-2bf793a215cc@redhat.com>
 <20200716115428.GC186790@mtl-vdi-166.wap.labs.mlnx>
 <b85ecb97-2120-f63f-f5f4-01d90f88d666@redhat.com>
 <20200718194902.GA52765@mtl-vdi-166.wap.labs.mlnx>
 <ed10cb66-174f-ec5c-8043-4e18de65fb99@redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ed10cb66-174f-ec5c-8043-4e18de65fb99@redhat.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from mtl-vdi-166.wap.labs.mlnx (94.188.199.18) by AM3PR07CA0075.eurprd07.prod.outlook.com (2603:10a6:207:4::33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3216.17 via Frontend Transport; Mon, 20 Jul 2020 05:19:43 +0000
X-Originating-IP: [94.188.199.18]
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-HT: Tenant
X-MS-Office365-Filtering-Correlation-Id: 95094744-bbe6-4d2d-0ba1-08d82c6c8340
X-MS-TrafficTypeDiagnostic: AM4PR0501MB2228:
X-MS-Exchange-Transport-Forked: True
X-Microsoft-Antispam-PRVS: <AM4PR0501MB22288E2767EE01AA762B0BE7C57B0@AM4PR0501MB2228.eurprd05.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:10000;
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: DGQ5gbndmgYXrpQxZItLu5Mvqm57+CcAVi7uwPMo79WgW7qIFh4yMbRSfm3up+Upavgne6ziXvhDdL7T8WjRQXWdEsHrZN3a8RSortO/ZDsJDD3RmF8gkso8rsmLNM6/cd3+fg1oe09DHzJI7P8MILDhjGA8GpnI+neRK7p25K7YYcR+z1kOXrUbRddhFZ4iu2cDL4OFMNPh7uoC3WlEewejLict6SlWRKXfx5kV4T5amfbgsFyxmU8hFsNLp13MqwkekgKkt8gEfHt3Rtm1n4LPCGjeJj6qsLZCtVl3egX+9HicDXNMImElu5B6dFjKNbuNhdccJPwg8qDTZGUcbQ==
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AM0PR05MB4786.eurprd05.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(136003)(346002)(39860400002)(396003)(376002)(366004)(26005)(5660300002)(8676002)(8936002)(316002)(55016002)(66556008)(66946007)(54906003)(9686003)(478600001)(186003)(16526019)(66476007)(52116002)(7696005)(4326008)(2906002)(86362001)(1076003)(956004)(6916009)(6506007)(83380400001)(33656002);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData: fGNzDJXk7fJvomHs6MjDrugU0VZGjxRdzQHU+QMNtBS1gZXMtCWLAU2WkhCfQ9IkS8J98+R2RHg7gdQ8sSozqamXsZC55CrB4wTxhg4rVDuIUIloNuO0Y5AhbDiKtAfQ/oeTL4QXLqad3Fv0mfWzqjkNQGIuxK3t2Gz/r5ZR+kSjtWwcXvqkMFigZA7Sy3+M7aclghW1TGwaJZDwC3NHwMHiqvPfM0SOvGJYfzwR35IzFwXY0PgHE0/Ng/MT/DucTuyPRB+Cu03cN632rIf/7Ab+zF7V+nnYFOrGh4IpY4UviW1GVTP2icJcalETFahNFmnEP8v0qw+UQ9IGS+lRzEtBMy9vLuy5L4PCITAClp//DA7ULFyIwnsuI5Og3L3qwjGrQhkJtQI/lG/uxTJTaN6i6HxesVk/Bj3RCFap3zBA5U+GuOKq0GuUx7ayGoovfMLsHD47iLSBYAW9rVkSLko8iXaxUTx/Wrjdzq82qCw=
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 95094744-bbe6-4d2d-0ba1-08d82c6c8340
X-MS-Exchange-CrossTenant-AuthSource: AM0PR05MB4786.eurprd05.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Jul 2020 05:19:44.5076
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: KJGHsou9TtS7Jtnpx3378LLsZkOJTeQINvImhLl1RLqafeKDCyzl6OhUk4EY9Ed9E9Yg2S8GvLquzLi7q5usPw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0501MB2228
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Jul 20, 2020 at 12:12:30PM +0800, Jason Wang wrote:
> 
> On 2020/7/19 上午3:49, Eli Cohen wrote:
> >On Fri, Jul 17, 2020 at 04:57:29PM +0800, Jason Wang wrote:
> >>>Looks like checking intialized is enough. Will fix this.
> >>>>>+
> >>>>>+static void mlx5_vdpa_set_vq_ready(struct vdpa_device *vdev, u16 idx, bool ready)
> >>>>>+{
> >>>>>+	struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
> >>>>>+	struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> >>>>>+	struct mlx5_vdpa_virtqueue *mvq = &ndev->vqs[idx];
> >>>>>+	int err;
> >>>>>+
> >>>>>+	if (!mvq->ready && ready && mvq->fw_state != MLX5_VIRTIO_NET_Q_OBJECT_STATE_RDY) {
> >>>>>+		err = modify_virtqueue(ndev, mvq, MLX5_VIRTIO_NET_Q_OBJECT_STATE_RDY);
> >>>>>+		if (err) {
> >>>>>+			mlx5_vdpa_warn(mvdev, "failed to modify virtqueue(%d)\n", err);
> >>>>>+			return;
> >>>>>+		}
> >>>>>+	}
> >>>>I wonder what's the reason of changing vq state on the hardware
> >>>>here. I think we can defer it to set_status().
> >>>>
> >>>I can defer this to set status.
> >>>
> >>>I just wonder if it is possible that the core vdpa driver may call this
> >>>function with ready equals false and after some time call it with ready
> >>>equals true.
> >>
> >>Good point, so I think we can keep the logic. But looks like the
> >>code can not work if ready equals false since it only tries to
> >>modify vq state to RDY.
> >>
> >The point is that you cannot modify the virtqueue to "not ready".
> 
> 
> Is this a hardware limitation of software one?

Sorry, but I was not accruate in my statement above. You can suspend the
hardware VQ but you cannot mover out of suspend back to ready.
> 
> I'm asking since we need support live migration. But a questions is
> how to stop the device but not reset, since we need get e.g
> last_avail_idx from the device.
> 
> It could be either:
> 
> 1) set_status(0)
> 2) get_vq_state()
> 
> or
> 
> 1) set_queue_ready(0)
> 2) get_vq_state()
> 

This can work.

> Set_status(0) means reset the virtio device but last_avail_idx is
> something out of virtio spec. I guess using set_queue_ready() is
> better.
> 
> What's you opinion?

So if the intention to set ready(0) as a preliminary state for live
migration then we're ok. We just need to keep in mind that there's no
way of suspend but destroy the hardware vq.

> 
> Thanks
> 
> 
> >  The
> >only option is to destroy it and create a new one. This means that if I
> >get ready equals false after the virtqueue has been created I need to
> >teardown the driver and set it up again.
> >
> >Given that, I think your original suggestion to defer this logic is
> >reasonable.
> >
>