On 03/17/2016 06:58 PM, Stefan Hajnoczi wrote:
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".

Yes, indeed.


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,
};

Er, it seems Michael much prefers the style which uses raw number which
is followed by a comment. :(


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


Yes, my fault, will fix it. Thanks you for pointing it out.


Reply via email to