Re: [PATCH] spapr_nvdimm.c: make 'label-size' mandatory

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2210 bytes --]

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 <danielhb413@gmail.com>

So, I still think it's kind of bogus that the guest side driver falls
over so messily (more on that elswhere in the thread).

However, regardless of that, it does make sense to enforce the PAPR
restriction that all NVDIMMs have labels.  And it fixes the visible
problem with a minimal change.

So, I've applied to ppc-for-5.1.  I am going to update the error
messages a little to make it clearer that these are PAPR specific
restrictions.

> ---
>  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]