Re: [Qemu-devel] [PATCH v5 28/33] nvdimm acpi: support Get Namespace Label Size function

[Stefan Hajnoczi]

On Thu, Oct 29, 2015 at 10:16:14AM +0800, Xiao Guangrong wrote:
> 
> 
> On 10/29/2015 12:41 AM, Stefan Hajnoczi wrote:
> >On Wed, Oct 28, 2015 at 10:26:26PM +, Xiao Guangrong wrote:
> >>+struct nvdimm_func_in_get_label_data {
> >>+uint32_t offset; /* the offset in the namespace label data area. */
> >>+uint32_t length; /* the size of data is to be read via the function. */
> >>+} QEMU_PACKED;
> >>+typedef struct nvdimm_func_in_get_label_data nvdimm_func_in_get_label_data;
> >
> >./CODING_STYLE "3. Naming":
> >
> >   Structured type names are in CamelCase; harder to type but standing
> >   out.
> 
> Did not realize it before. Will change its name to:
> NVDIMMFuncInGetLabelData

Great, thanks!

> >>+/*
> >>+ * the max transfer size is the max size transferred by both a
> >>+ * 'Get Namespace Label Data' function and a 'Set Namespace Label Data'
> >>+ * function.
> >>+ */
> >>+static uint32_t nvdimm_get_max_xfer_label_size(void)
> >>+{
> >>+nvdimm_dsm_in *in;
> >>+uint32_t max_get_size, max_set_size, dsm_memory_size = getpagesize();
> >
> >Why is the host's page size relevant here?  Did you mean
> >TARGET_PAGE_SIZE?
> 
> Yes.
> 
> NVDIMM is the common code, unfortunately TARGET_PAGE_SIZE is platform
> specified and QEMU lacks a place to include this kind of specified definition:

Can you make NVDIMM a per-target object file?

Although we try to avoid it whenever possible, it means that
qemu-system-x86_64, qemu-system-arm, etc will build
x86_64-softmmu/hw/.../nvdimm.o, arm-softmmu/hw/.../nvdimm.o, etc.

In Makefile.objs put the nvdimm object file in obj-y instead of
common-obj-y.


signature.asc
Description: PGP signature

[PATCH v5 28/33] nvdimm acpi: support Get Namespace Label Size function

[Xiao Guangrong]

Function 4 is used to get Namespace label size

Signed-off-by: Xiao Guangrong 
---
 hw/acpi/nvdimm.c | 87 +++-
 1 file changed, 86 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 8efa640..72203d2 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -407,15 +407,48 @@ enum {
 NVDIMM_DSM_DEV_STATUS_VENDOR_SPECIFIC_ERROR = 4,
 };
 
+struct nvdimm_func_in_get_label_data {
+uint32_t offset; /* the offset in the namespace label data area. */
+uint32_t length; /* the size of data is to be read via the function. */
+} QEMU_PACKED;
+typedef struct nvdimm_func_in_get_label_data nvdimm_func_in_get_label_data;
+
+struct nvdimm_func_in_set_label_data {
+uint32_t offset; /* the offset in the namespace label data area. */
+uint32_t length; /* the size of data is to be written via the function. */
+uint8_t in_buf[0]; /* the data written to label data area. */
+} QEMU_PACKED;
+typedef struct nvdimm_func_in_set_label_data nvdimm_func_in_set_label_data;
+
 struct nvdimm_dsm_in {
 uint32_t handle;
 uint32_t revision;
 uint32_t function;
/* the remaining size in the page is used by arg3. */
-uint8_t arg3[0];
+union {
+uint8_t arg3[0];
+nvdimm_func_in_set_label_data func_set_label_data;
+};
 } QEMU_PACKED;
 typedef struct nvdimm_dsm_in nvdimm_dsm_in;
 
+struct nvdimm_func_out_label_size {
+uint32_t status; /* return status code. */
+uint32_t label_size; /* the size of label data area. */
+/*
+ * Maximum size of the namespace label data length supported by
+ * the platform in Get/Set Namespace Label Data functions.
+ */
+uint32_t max_xfer;
+} QEMU_PACKED;
+typedef struct nvdimm_func_out_label_size nvdimm_func_out_label_size;
+
+struct nvdimm_func_out_get_label_data {
+uint32_t status;/*return status code. */
+uint8_t out_buf[0]; /* the data got via Get Namesapce Label function. */
+} QEMU_PACKED;
+typedef struct nvdimm_func_out_get_label_data nvdimm_func_out_get_label_data;
+
 static void nvdimm_dsm_write_status(GArray *out, uint32_t status)
 {
 status = cpu_to_le32(status);
@@ -445,6 +478,55 @@ static void nvdimm_dsm_root(nvdimm_dsm_in *in, GArray *out)
 nvdimm_dsm_write_status(out, status);
 }
 
+/*
+ * the max transfer size is the max size transferred by both a
+ * 'Get Namespace Label Data' function and a 'Set Namespace Label Data'
+ * function.
+ */
+static uint32_t nvdimm_get_max_xfer_label_size(void)
+{
+nvdimm_dsm_in *in;
+uint32_t max_get_size, max_set_size, dsm_memory_size = getpagesize();
+
+/*
+ * the max data ACPI can read one time which is transferred by
+ * the response of 'Get Namespace Label Data' function.
+ */
+max_get_size = dsm_memory_size - sizeof(nvdimm_func_out_get_label_data);
+
+/*
+ * the max data ACPI can write one time which is transferred by
+ * 'Set Namespace Label Data' function.
+ */
+max_set_size = dsm_memory_size - offsetof(nvdimm_dsm_in, arg3) -
+   sizeof(in->func_set_label_data);
+
+return MIN(max_get_size, max_set_size);
+}
+
+/*
+ * DSM Spec Rev1 4.4 Get Namespace Label Size (Function Index 4).
+ *
+ * It gets the size of Namespace Label data area and the max data size
+ * that Get/Set Namespace Label Data functions can transfer.
+ */
+static void nvdimm_dsm_func_label_size(NVDIMMDevice *nvdimm, GArray *out)
+{
+nvdimm_func_out_label_size func_label_size;
+uint32_t label_size, mxfer;
+
+label_size = nvdimm->label_size;
+mxfer = nvdimm_get_max_xfer_label_size();
+
+nvdimm_debug("label_size %#x, max_xfer %#x.\n", label_size, mxfer);
+
+func_label_size.status = cpu_to_le32(NVDIMM_DSM_STATUS_SUCCESS);
+func_label_size.label_size = cpu_to_le32(label_size);
+func_label_size.max_xfer = cpu_to_le32(mxfer);
+
+g_array_append_vals(out, _label_size, sizeof(func_label_size));
+}
+
 static void nvdimm_dsm_device(nvdimm_dsm_in *in, GArray *out)
 {
 GSList *list = nvdimm_get_plugged_device_list();
@@ -469,6 +551,9 @@ static void nvdimm_dsm_device(nvdimm_dsm_in *in, GArray 
*out)
1 << 6 /* Set Namespace Label Data */);
 build_append_int_noprefix(out, cmd_list, sizeof(cmd_list));
 goto free;
+case 0x4 /* Get Namespace Label Size */:
+nvdimm_dsm_func_label_size(nvdimm, out);
+goto free;
 default:
 status = NVDIMM_DSM_STATUS_NOT_SUPPORTED;
 };
-- 
1.8.3.1

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

Re: [PATCH v5 28/33] nvdimm acpi: support Get Namespace Label Size function

[Stefan Hajnoczi]

On Wed, Oct 28, 2015 at 10:26:26PM +, Xiao Guangrong wrote:
> +struct nvdimm_func_in_get_label_data {
> +uint32_t offset; /* the offset in the namespace label data area. */
> +uint32_t length; /* the size of data is to be read via the function. */
> +} QEMU_PACKED;
> +typedef struct nvdimm_func_in_get_label_data nvdimm_func_in_get_label_data;

./CODING_STYLE "3. Naming":

  Structured type names are in CamelCase; harder to type but standing
  out.

I'm surprised that scripts/checkpatch.pl didn't warning about this.

> +/*
> + * the max transfer size is the max size transferred by both a
> + * 'Get Namespace Label Data' function and a 'Set Namespace Label Data'
> + * function.
> + */
> +static uint32_t nvdimm_get_max_xfer_label_size(void)
> +{
> +nvdimm_dsm_in *in;
> +uint32_t max_get_size, max_set_size, dsm_memory_size = getpagesize();

Why is the host's page size relevant here?  Did you mean
TARGET_PAGE_SIZE?


signature.asc
Description: PGP signature

Re: [PATCH v5 28/33] nvdimm acpi: support Get Namespace Label Size function

[Xiao Guangrong]

On 10/29/2015 12:41 AM, Stefan Hajnoczi wrote:

On Wed, Oct 28, 2015 at 10:26:26PM +, Xiao Guangrong wrote:

+struct nvdimm_func_in_get_label_data {
+uint32_t offset; /* the offset in the namespace label data area. */
+uint32_t length; /* the size of data is to be read via the function. */
+} QEMU_PACKED;
+typedef struct nvdimm_func_in_get_label_data nvdimm_func_in_get_label_data;


./CODING_STYLE "3. Naming":

   Structured type names are in CamelCase; harder to type but standing
   out.


Did not realize it before. Will change its name to:
NVDIMMFuncInGetLabelData

And the same as others.



I'm surprised that scripts/checkpatch.pl didn't warning about this.






+/*
+ * the max transfer size is the max size transferred by both a
+ * 'Get Namespace Label Data' function and a 'Set Namespace Label Data'
+ * function.
+ */
+static uint32_t nvdimm_get_max_xfer_label_size(void)
+{
+nvdimm_dsm_in *in;
+uint32_t max_get_size, max_set_size, dsm_memory_size = getpagesize();


Why is the host's page size relevant here?  Did you mean
TARGET_PAGE_SIZE?


Yes.

NVDIMM is the common code, unfortunately TARGET_PAGE_SIZE is platform
specified and QEMU lacks a place to include this kind of specified definition:

$ git grep "#define TARGET_PAGE_SIZE"
include/exec/cpu-all.h:#define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
[eric@xiaoreal1 qemu]$ git grep "#define TARGET_PAGE_BITS"
target-alpha/cpu.h:#define TARGET_PAGE_BITS 13
target-arm/cpu.h:#define TARGET_PAGE_BITS 12
target-arm/cpu.h:#define TARGET_PAGE_BITS 10
target-cris/cpu.h:#define TARGET_PAGE_BITS 13
target-i386/cpu.h:#define TARGET_PAGE_BITS 12
target-lm32/cpu.h:#define TARGET_PAGE_BITS 12
target-m68k/cpu.h:#define TARGET_PAGE_BITS 13
target-m68k/cpu.h:#define TARGET_PAGE_BITS 10
target-microblaze/cpu.h:#define TARGET_PAGE_BITS 12
target-mips/mips-defs.h:#define TARGET_PAGE_BITS 12
target-moxie/cpu.h:#define TARGET_PAGE_BITS 12 /* 4k */
target-openrisc/cpu.h:#define TARGET_PAGE_BITS 13
target-ppc/cpu.h:#define TARGET_PAGE_BITS 12
target-ppc/cpu.h:#define TARGET_PAGE_BITS_64K 16
target-ppc/cpu.h:#define TARGET_PAGE_BITS_16M 24
target-ppc/cpu.h:#define TARGET_PAGE_BITS 12
target-ppc/cpu.h:#define TARGET_PAGE_BITS 10
target-ppc/cpu.h:#define TARGET_PAGE_BITS 12
target-s390x/cpu.h:#define TARGET_PAGE_BITS 12
target-sh4/cpu.h:#define TARGET_PAGE_BITS 12/* 4k X */
target-sparc/cpu.h:#define TARGET_PAGE_BITS 12 /* 4k */
target-sparc/cpu.h:#define TARGET_PAGE_BITS 13 /* 8k */
target-tilegx/cpu.h:#define TARGET_PAGE_BITS 16  /* TILE-Gx uses 64KB page size 
*/
target-tricore/tricore-defs.h:#define TARGET_PAGE_BITS 14
target-unicore32/cpu.h:#define TARGET_PAGE_BITS12
target-xtensa/cpu.h:#define TARGET_PAGE_BITS 12


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