[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] spapr_nvdimm.c: make 'label-size' mandatory
From: |
David Gibson |
Subject: |
Re: [PATCH] spapr_nvdimm.c: make 'label-size' mandatory |
Date: |
Tue, 14 Apr 2020 12:01:17 +1000 |
CCing Xiao, Michael and Igor for generic NVDIMM perspective.
On Mon, Apr 13, 2020 at 05:36:28PM -0300, Daniel Henrique Barboza wrote:
> The pseries machine does not support NVDIMM modules without label.
> Attempting to do so, even if the overall block size is aligned with
> 256MB, will seg fault the guest kernel during NVDIMM probe. This
> can be avoided by forcing 'label-size' to always be present for
> sPAPR NVDIMMs.
>
> The verification was put before the alignment check because the
> presence of label-size affects the alignment calculation, so
> it's not optimal to warn the user about an alignment error,
> then about the lack of label-size, then about a new alignment
> error when the user sets a label-size.
>
> Signed-off-by: Daniel Henrique Barboza <address@hidden>
So, this would get the job done, but it seems a bit inelegant compared
to having the device default to working settings. I'm looking at how
this interacts with the generic constraints on label-size.
The generic nvdimm code has a MIN_NAMESPACE_LABEL_SIZE of 128 kiB, and
values of label-size less than that are rejected. Except that if
label-size is not set at all, it is left as 0.
Is that intended behaviour? Do x86 (or whatever) NVDIMMs have a label
of at least 128kiB, unless they have no label at all? Or could we
make the default label-size 128kiB generically?
> ---
> hw/ppc/spapr_nvdimm.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
> index 25be8082d7..9abcdcc26b 100644
> --- a/hw/ppc/spapr_nvdimm.c
> +++ b/hw/ppc/spapr_nvdimm.c
> @@ -37,6 +37,12 @@ void spapr_nvdimm_validate_opts(NVDIMMDevice *nvdimm,
> uint64_t size,
> QemuUUID uuid;
> int ret;
>
> + if (object_property_get_int(OBJECT(nvdimm), NVDIMM_LABEL_SIZE_PROP,
> + &error_abort) == 0) {
> + error_setg(errp, "NVDIMM device requires label-size to be set");
> + return;
> + }
> +
> if (size % SPAPR_MINIMUM_SCM_BLOCK_SIZE) {
> error_setg(errp, "NVDIMM memory size excluding the label area"
> " must be a multiple of %" PRIu64 "MB",
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature