| Message ID | 20210324200907.408996-2-its@irrelevant.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/block/nvme: misc fixes | expand |
On Wed, Mar 24, 2021 at 09:09:01PM +0100, Klaus Jensen wrote: >From: Klaus Jensen <k.jensen@samsung.com> > >Protection Information can only be enabled if there is at least 8 bytes >of metadata. > >Signed-off-by: Klaus Jensen <k.jensen@samsung.com> >--- > hw/block/nvme-ns.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c >index 7f8d139a8663..ca04ee1bacfb 100644 >--- a/hw/block/nvme-ns.c >+++ b/hw/block/nvme-ns.c >@@ -394,7 +394,7 @@ static int nvme_ns_check_constraints(NvmeNamespace *ns, Error **errp) > return -1; > } > >- if (ns->params.pi && !ns->params.ms) { >+ if (ns->params.pi && ns->params.ms < 8) { and also it is good check that "metadata size" is power of 2 or not? > error_setg(errp, "at least 8 bytes of metadata required to enable " > "protection information"); > return -1; >-- >2.31.0 > >
On Mar 29 19:52, Gollu Appalanaidu wrote: > On Wed, Mar 24, 2021 at 09:09:01PM +0100, Klaus Jensen wrote: > > From: Klaus Jensen <k.jensen@samsung.com> > > > > Protection Information can only be enabled if there is at least 8 bytes > > of metadata. > > > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > > --- > > hw/block/nvme-ns.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c > > index 7f8d139a8663..ca04ee1bacfb 100644 > > --- a/hw/block/nvme-ns.c > > +++ b/hw/block/nvme-ns.c > > @@ -394,7 +394,7 @@ static int nvme_ns_check_constraints(NvmeNamespace *ns, Error **errp) > > return -1; > > } > > > > - if (ns->params.pi && !ns->params.ms) { > > + if (ns->params.pi && ns->params.ms < 8) { > and also it is good check that "metadata size" is power of 2 or not? > While I don't expect a lot of real-world devices having metadata sizes that are not power of twos, there is no requirement in the spec for that. And the implementation here also does not require it :)
On Tue, Mar 30, 2021 at 09:24:59AM +0200, Klaus Jensen wrote: >On Mar 29 19:52, Gollu Appalanaidu wrote: >> On Wed, Mar 24, 2021 at 09:09:01PM +0100, Klaus Jensen wrote: >> > From: Klaus Jensen <k.jensen@samsung.com> >> > >> > Protection Information can only be enabled if there is at least 8 bytes >> > of metadata. >> > >> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> >> > --- >> > hw/block/nvme-ns.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c >> > index 7f8d139a8663..ca04ee1bacfb 100644 >> > --- a/hw/block/nvme-ns.c >> > +++ b/hw/block/nvme-ns.c >> > @@ -394,7 +394,7 @@ static int nvme_ns_check_constraints(NvmeNamespace *ns, Error **errp) >> > return -1; >> > } >> > >> > - if (ns->params.pi && !ns->params.ms) { >> > + if (ns->params.pi && ns->params.ms < 8) { >> and also it is good check that "metadata size" is power of 2 or not? >> > >While I don't expect a lot of real-world devices having metadata sizes >that are not power of twos, there is no requirement in the spec for >that. > >And the implementation here also does not require it :) Reviewed-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c index 7f8d139a8663..ca04ee1bacfb 100644 --- a/hw/block/nvme-ns.c +++ b/hw/block/nvme-ns.c @@ -394,7 +394,7 @@ static int nvme_ns_check_constraints(NvmeNamespace *ns, Error **errp) return -1; } - if (ns->params.pi && !ns->params.ms) { + if (ns->params.pi && ns->params.ms < 8) { error_setg(errp, "at least 8 bytes of metadata required to enable " "protection information"); return -1;