Re: [PATCH v2 10/12] hw/block/nvme: move cmb logic to v1.4

From: Minwoo Im <minwoo.im.dev@gmail.com>
To: Klaus Jensen <its@irrelevant.dk>
Cc: Fam Zheng <fam@euphon.net>, Kevin Wolf <kwolf@redhat.com>,
	Padmakar Kalghatgi <p.kalghatgi@samsung.com>,
	qemu-block@nongnu.org, Klaus Jensen <k.jensen@samsung.com>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Keith Busch <kbusch@kernel.org>
Subject: Re: [PATCH v2 10/12] hw/block/nvme: move cmb logic to v1.4
Date: Tue, 19 Jan 2021 11:11:42 +0900	[thread overview]
Message-ID: <20210119021142.GB5939@localhost.localdomain> (raw)
In-Reply-To: <YAXgMluXNBuIaoPo@apples.localdomain>

On 21-01-18 20:23:30, Klaus Jensen wrote:
> On Jan 18 14:22, Klaus Jensen wrote:
> > On Jan 18 22:12, Minwoo Im wrote:
> > > On 21-01-18 14:10:50, Klaus Jensen wrote:
> > > > On Jan 18 22:09, Minwoo Im wrote:
> > > > > > > Yes, CMB in v1.4 is not backward-compatible, but is it okay to move onto
> > > > > > > the CMB v1.4 from v1.3 without supporting the v1.3 usage on this device
> > > > > > > model?
> > > > > > 
> > > > > > Next patch moves to v1.4. I wanted to split it because the "bump" patch
> > > > > > also adds a couple of other v1.4 requires fields. But I understand that
> > > > > > this is slightly wrong.
> > > > > 
> > > > > Sorry, I meant I'd like to have CMB for v1.3 support along with the v1.4
> > > > > support in this device model separately.  Maybe I missed the linux-nvme
> > > > > mailing list for CMB v1.4, but is there no plan to keep supportin CMB
> > > > > v1.3 in NVMe driver?
> > > > 
> > > > I posted a patch on linux-nvme for v1.4 support in the kernel. It will
> > > > support both the v1.3 and v1.4 behavior :)
> > > 
> > > Then, can we maintain CMB v1.3 also in QEMU also along with v1.4 ? :)
> > 
> > My first version of this patch actually included compatibility support
> > for v1.3 ("legacy cmb"), but Keith suggested we just drop that and keep
> > this device compliant.
> > 
> > What we could do is allow the spec version to be chosen with a
> > parameter?
> 
> Uhm. Maybe not. I gave this some more thought.
> 
> Adding a device parameter to choose the specification version requires
> us to maintain QEMU "compat" properties across different QEMU version.
> I'm not sure we want that for something like this.

Agreed.  The default should head for latest one.

> 
> Maybe the best course of action actually *is* an 'x-legacy-cmb'
> parameter.

This looks fine.

As kernel driver does not remove the v1.3 CMB support, then it would be
great if QEMU suports that also.