On Tue, 2020-07-07 at 21:15 +0200, Klaus Jensen wrote: > On Jul 7 19:27, Maxim Levitsky wrote: > > On Wed, 2020-07-01 at 14:48 -0700, Andrzej Jakowski wrote: > > > This patch sets CMBS bit in controller capabilities register when user > > > configures NVMe driver with CMB support, so capabilites are correctly > > > reported to guest OS. > > > > > > Signed-off-by: Andrzej Jakowski <andrzej.jakow...@linux.intel.com> > > > Reviewed-by: Klaus Jensen <k.jen...@samsung.com> > > > --- > > > hw/block/nvme.c | 2 +- > > > include/block/nvme.h | 6 +++++- > > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > > > index 1aee042d4c..9f11f3e9da 100644 > > > --- a/hw/block/nvme.c > > > +++ b/hw/block/nvme.c > > > @@ -1582,6 +1582,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice > > > *pci_dev) > > > NVME_CAP_SET_TO(n->bar.cap, 0xf); > > > NVME_CAP_SET_CSS(n->bar.cap, 1); > > > NVME_CAP_SET_MPSMAX(n->bar.cap, 4); > > > + NVME_CAP_SET_CMBS(n->bar.cap, n->params.cmb_size_mb ? 1 : 0); > > > > > > n->bar.vs = 0x00010200; > > > n->bar.intmc = n->bar.intms = 0; > > > @@ -1591,7 +1592,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error > > > **errp) > > > { > > > NvmeCtrl *n = NVME(pci_dev); > > > Error *local_err = NULL; > > > - > > > int i; > > > > > > nvme_check_constraints(n, &local_err); > > > diff --git a/include/block/nvme.h b/include/block/nvme.h > > > index 1720ee1d51..14cf398dfa 100644 > > > --- a/include/block/nvme.h > > > +++ b/include/block/nvme.h > > > @@ -35,6 +35,7 @@ enum NvmeCapShift { > > > CAP_MPSMIN_SHIFT = 48, > > > CAP_MPSMAX_SHIFT = 52, > > > CAP_PMR_SHIFT = 56, > > > + CAP_CMB_SHIFT = 57, > > > }; > > > > > > enum NvmeCapMask { > > > @@ -48,6 +49,7 @@ enum NvmeCapMask { > > > CAP_MPSMIN_MASK = 0xf, > > > CAP_MPSMAX_MASK = 0xf, > > > CAP_PMR_MASK = 0x1, > > > + CAP_CMB_MASK = 0x1, > > > }; > > > > > > #define NVME_CAP_MQES(cap) (((cap) >> CAP_MQES_SHIFT) & CAP_MQES_MASK) > > > @@ -78,8 +80,10 @@ enum NvmeCapMask { > > > << > > > CAP_MPSMIN_SHIFT) > > > #define NVME_CAP_SET_MPSMAX(cap, val) (cap |= (uint64_t)(val & > > > CAP_MPSMAX_MASK)\ > > > << > > > CAP_MPSMAX_SHIFT) > > > -#define NVME_CAP_SET_PMRS(cap, val) (cap |= (uint64_t)(val & > > > CAP_PMR_MASK)\ > > > +#define NVME_CAP_SET_PMRS(cap, val) (cap |= (uint64_t)(val & > > > CAP_PMR_MASK) \ > > > << > > > CAP_PMR_SHIFT) > > > +#define NVME_CAP_SET_CMBS(cap, val) (cap |= (uint64_t)(val & > > > CAP_CMB_MASK) \ > > > + << > > > CAP_CMB_SHIFT) > > > > > > enum NvmeCcShift { > > > CC_EN_SHIFT = 0, > > > > I wonder how this could have beeing forgotten. Hmm. > > I see that Linux kernel uses CMBSZ != for that. > > I guess this explains it. > > > > Reviewed-by: Maxim Levitsky <mlevit...@gmail.com> > > > > It is a v1.4 field. The CMB support was added when NVMe was at v1.2. > And the Linux kernel is also basically adhering to v1.3 wrt. CMB > support. In v1.4 the host actually needs to specifically enable the CMB > - and that is not something the kernel does currently IIRC. > Ah, makes sense! I by now have specs for each NVME revision, but I am getting lazy sometimes to cross-check them.
Best regards, Maxim Levitsky