Re: [PATCH 12/15] nvdimm acpi: support Get Namespace Label Size function

From: Stefan Hajnoczi <stefanha@gmail.com>
To: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Cc: ehabkost@redhat.com, kvm@vger.kernel.org, mst@redhat.com,
	gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org,
	stefanha@redhat.com, imammedo@redhat.com, pbonzini@redhat.com,
	dan.j.williams@intel.com, rth@twiddle.net
Subject: Re: [PATCH 12/15] nvdimm acpi: support Get Namespace Label Size function
Date: Thu, 17 Mar 2016 10:58:45 +0000	[thread overview]
Message-ID: <20160317105845.GD14062@stefanha-x1.localdomain> (raw)
In-Reply-To: <1458203581-59143-13-git-send-email-guangrong.xiao@linux.intel.com>

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

On Thu, Mar 17, 2016 at 04:32:58PM +0800, Xiao Guangrong wrote:
> -    /* No function except function 0 is supported yet. */
> -    nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
> +    if (!nvdimm) {
> +        return nvdimm_dsm_no_payload(1 /* Non-Existing Memory Device */,

"Non-existing Memory Device" is 2 according to the spec.  1 would be
"Not Supported".

Please define constants for all these magic values instead of putting
literals into the code:

enum {
    NVDIMM_DSM_SUCCESS = 0,
    NVDIMM_DSM_NOT_SUPPORTED = 1,
    NVDIMM_DSM_NON_EXISTING_MEMORY_DEVICE = 2,
    NVDIMM_DSM_INVALID_INPUT_PARAMETERS = 3,
    NVDIMM_DSM_VENDOR_SPECIFIC_ERROR = 4,
};

> +                                     dsm_mem_addr);
> +    }
> +
> +    /* Encode DSM function according to DSM Spec Rev1. */
> +    switch (in->function) {
> +    case 4 /* Get Namespace Label Size */:
> +        if (nvdimm->reserve_label) {
> +            nvdimm_dsm_label_size(nvdimm, dsm_mem_addr);
> +        }
> +        break;

What is the expected return status code if the device has no labels?

This function must write a return status code, otherwise the guest will
read the out buffer and attempt to interpret uninitialized memory.

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