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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux