Re: [PATCH v2 10/12] hw/block/nvme: move cmb logic to v1.4
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.
Re: [PATCH v2 10/12] hw/block/nvme: move cmb logic to v1.4
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.
Maybe the best course of action actually *is* an 'x-legacy-cmb'
parameter.
signature.asc
Description: PGP signature
Re: [PATCH v2 10/12] hw/block/nvme: move cmb logic to v1.4
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?
signature.asc
Description: PGP signature
Re: [PATCH v2 10/12] hw/block/nvme: move cmb logic to v1.4
> > 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?
Re: [PATCH v2 10/12] hw/block/nvme: move cmb logic to v1.4
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 :) signature.asc Description: PGP signature
Re: [PATCH v2 10/12] hw/block/nvme: move cmb logic to v1.4
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 ? :)
Re: [PATCH v2 10/12] hw/block/nvme: move cmb logic to v1.4
On Jan 18 21:58, Minwoo Im wrote: > Hello, > > On 21-01-18 10:47:03, Klaus Jensen wrote: > > From: Padmakar Kalghatgi > > > > Implement v1.4 logic for configuring the Controller Memory Buffer. This > > is not backward compatible with v1.3, so drivers that only support v1.3 > > will not be able to use the CMB anymore. > > > > Signed-off-by: Padmakar Kalghatgi > > Signed-off-by: Klaus Jensen > > 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. signature.asc Description: PGP signature
Re: [PATCH v2 10/12] hw/block/nvme: move cmb logic to v1.4
Hello, On 21-01-18 10:47:03, Klaus Jensen wrote: > From: Padmakar Kalghatgi > > Implement v1.4 logic for configuring the Controller Memory Buffer. This > is not backward compatible with v1.3, so drivers that only support v1.3 > will not be able to use the CMB anymore. > > Signed-off-by: Padmakar Kalghatgi > Signed-off-by: Klaus Jensen 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?
[PATCH v2 10/12] hw/block/nvme: move cmb logic to v1.4
From: Padmakar Kalghatgi
Implement v1.4 logic for configuring the Controller Memory Buffer. This
is not backward compatible with v1.3, so drivers that only support v1.3
will not be able to use the CMB anymore.
Signed-off-by: Padmakar Kalghatgi
Signed-off-by: Klaus Jensen
---
hw/block/nvme.h | 1 +
include/block/nvme.h | 107 +-
hw/block/nvme.c | 78 +++---
hw/block/trace-events | 2 +
4 files changed, 159 insertions(+), 29 deletions(-)
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 5988d9b36e12..de9164dd52e6 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -145,6 +145,7 @@ typedef struct NvmeCtrl {
uint32_tmax_q_ents;
uint8_t outstanding_aers;
uint8_t *cmbuf;
+boolcmb_cmse;
uint32_tirq_status;
uint64_thost_timestamp; /* Timestamp sent by the host
*/
uint64_ttimestamp_set_qemu_clock_ms;/* QEMU clock time */
diff --git a/include/block/nvme.h b/include/block/nvme.h
index 183dc5c0ecf6..7dcd8f9b4e78 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -15,14 +15,19 @@ typedef struct QEMU_PACKED NvmeBar {
uint64_tacq;
uint32_tcmbloc;
uint32_tcmbsz;
-uint8_t padding[3520]; /* not used by QEMU */
+uint32_tbpinfo;
+uint32_tbprsel;
+uint64_tbpmbl;
+uint64_tcmbmsc;
+uint32_tcmbsts;
+uint8_t rsvd92[3492];
uint32_tpmrcap;
uint32_tpmrctl;
uint32_tpmrsts;
uint32_tpmrebs;
uint32_tpmrswtp;
uint64_tpmrmsc;
-uint8_t reserved[484];
+uint8_t css[484];
} NvmeBar;
enum NvmeCapShift {
@@ -63,6 +68,7 @@ enum NvmeCapMask {
#define NVME_CAP_MPSMIN(cap)(((cap) >> CAP_MPSMIN_SHIFT) & CAP_MPSMIN_MASK)
#define NVME_CAP_MPSMAX(cap)(((cap) >> CAP_MPSMAX_SHIFT) & CAP_MPSMAX_MASK)
#define NVME_CAP_PMRS(cap) (((cap) >> CAP_PMRS_SHIFT) & CAP_PMRS_MASK)
+#define NVME_CAP_CMBS(cap) (((cap) >> CAP_CMBS_SHIFT) & CAP_CMBS_MASK)
#define NVME_CAP_SET_MQES(cap, val) (cap |= (uint64_t)(val & CAP_MQES_MASK)
\
<< CAP_MQES_SHIFT)
@@ -184,25 +190,64 @@ enum NvmeAqaMask {
#define NVME_AQA_ACQS(aqa) ((aqa >> AQA_ACQS_SHIFT) & AQA_ACQS_MASK)
enum NvmeCmblocShift {
-CMBLOC_BIR_SHIFT = 0,
-CMBLOC_OFST_SHIFT = 12,
+CMBLOC_BIR_SHIFT = 0,
+CMBLOC_CQMMS_SHIFT = 3,
+CMBLOC_CQPDS_SHIFT = 4,
+CMBLOC_CDPMLS_SHIFT = 5,
+CMBLOC_CDPCILS_SHIFT = 6,
+CMBLOC_CDMMMS_SHIFT = 7,
+CMBLOC_CQDA_SHIFT= 8,
+CMBLOC_OFST_SHIFT= 12,
};
enum NvmeCmblocMask {
-CMBLOC_BIR_MASK = 0x7,
-CMBLOC_OFST_MASK = 0xf,
+CMBLOC_BIR_MASK = 0x7,
+CMBLOC_CQMMS_MASK = 0x1,
+CMBLOC_CQPDS_MASK = 0x1,
+CMBLOC_CDPMLS_MASK = 0x1,
+CMBLOC_CDPCILS_MASK = 0x1,
+CMBLOC_CDMMMS_MASK = 0x1,
+CMBLOC_CQDA_MASK= 0x1,
+CMBLOC_OFST_MASK= 0xf,
};
-#define NVME_CMBLOC_BIR(cmbloc) ((cmbloc >> CMBLOC_BIR_SHIFT) & \
- CMBLOC_BIR_MASK)
-#define NVME_CMBLOC_OFST(cmbloc)((cmbloc >> CMBLOC_OFST_SHIFT) & \
- CMBLOC_OFST_MASK)
+#define NVME_CMBLOC_BIR(cmbloc) \
+((cmbloc >> CMBLOC_BIR_SHIFT) & CMBLOC_BIR_MASK)
+#define NVME_CMBLOC_CQMMS(cmbloc) \
+((cmbloc >> CMBLOC_CQMMS_SHIFT) & CMBLOC_CQMMS_MASK)
+#define NVME_CMBLOC_CQPDS(cmbloc) \
+((cmbloc >> CMBLOC_CQPDS_SHIFT) & CMBLOC_CQPDS_MASK)
+#define NVME_CMBLOC_CDPMLS(cmbloc) \
+((cmbloc >> CMBLOC_CDPMLS_SHIFT) & CMBLOC_CDPMLS_MASK)
+#define NVME_CMBLOC_CDPCILS(cmbloc) \
+((cmbloc >> CMBLOC_CDPCILS_SHIFT) & CMBLOC_CDPCILS_MASK)
+#define NVME_CMBLOC_CDMMMS(cmbloc) \
+((cmbloc >> CMBLOC_CDMMMS_SHIFT) & CMBLOC_CDMMMS_MASK)
+#define NVME_CMBLOC_CQDA(cmbloc) \
+((cmbloc >> CMBLOC_CQDA_SHIFT) & CMBLOC_CQDA_MASK)
+#define NVME_CMBLOC_OFST(cmbloc) \
+((cmbloc >> CMBLOC_OFST_SHIFT) & CMBLOC_OFST_MASK)
-#define NVME_CMBLOC_SET_BIR(cmbloc, val) \
+#define NVME_CMBLOC_SET_BIR(cmbloc, val) \
(cmbloc |= (uint64_t)(val & CMBLOC_BIR_MASK) << CMBLOC_BIR_SHIFT)
+#define NVME_CMBLOC_SET_CQMMS(cmbloc, val) \
+(cmbloc |= (uint64_t)(val & CMBLOC_CQMMS_MASK) << CMBLOC_CQMMS_SHIFT)
+#define NVME_CMBLOC_SET_CQPDS(cmbloc, val) \
+(cmbloc |= (uint64_t)(val & CMBLOC_CQPDS_MASK) << CMBLOC_CQPDS_SHIFT)
+#define NVME_CMBLOC_SET_CDPMLS(cmbloc, val) \
+(cmbloc |= (uint64_t)(val & CMBLOC_CDPMLS_MASK) << CMBLOC_CDPMLS_SHIFT)
+#define NVME_CMBLOC_SET_CDPCILS(cmbloc, val) \
+(cmbloc |= (uint64_t)(val & CMBLOC_CDPCILS_MASK) << CMBLOC_CDPCILS_SHIFT)
+#define NVME_CMBLOC_SET_CDMMMS(cmbloc, val) \
+(cmbloc |= (uint64_t)(val & CMBLOC_CDMMMS_MASK) << CMBLOC_CDMMMS_SHIFT)
+#define NVME_CMBLOC_SET_CQDA(cmbloc, val) \
+(cmbloc |= (uint64_t)(val & CMBLOC_CQDA_MASK) << CMBLOC_CQDA_SHIFT)
#define NVME_CMBLOC_SET_OFST(cmbloc, val) \
(cmbloc
