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

Re: [Xen-devel] [RFC v3 1/6] xen/arm: Add basic save/restore support for ARM

On 05/08/2014 05:11 PM, Andrew Cooper wrote:
On 08/05/2014 22:18, Wei Huang wrote:

diff --git a/xen/include/asm-arm/hvm/support.h 
new file mode 100644
index 0000000..fa5fe75
--- /dev/null
+++ b/xen/include/asm-arm/hvm/support.h
@@ -0,0 +1,29 @@
+ * HVM support routines
+ *
+ * Copyright (c) 2014, Samsung Electronics.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ */
+#ifndef __ASM_ARM_HVM_SUPPORT_H__
+#define __ASM_ARM_HVM_SUPPORT_H__
+#include <xen/types.h>
+#include <public/hvm/ioreq.h>
+#include <xen/sched.h>
+#include <xen/hvm/save.h>
+#include <asm/processor.h>
+#endif /* __ASM_ARM_HVM_SUPPORT_H__ */

This header file isn't touched by any subsequent patches, and just
having it as a list of includes is overkill.  Can it be dropped?  5
extra includes in a few .c files is hardly breaking the bank.
Last time I tried it quickly, the compilation broke. Will try again.

diff --git a/xen/include/public/arch-arm/hvm/save.h 
index 75b8e65..8312e7b 100644
--- a/xen/include/public/arch-arm/hvm/save.h
+++ b/xen/include/public/arch-arm/hvm/save.h
@@ -3,6 +3,7 @@
   * be saved along with the domain's memory and device-model state.
   * Copyright (c) 2012 Citrix Systems Ltd.
+ * Copyright (c) 2014 Samsung Electronics.
   * Permission is hereby granted, free of charge, to any person obtaining a 
   * of this software and associated documentation files (the "Software"), to
@@ -26,6 +27,24 @@

+#define HVM_ARM_FILE_MAGIC   0x92385520
+#define HVM_ARM_FILE_VERSION 0x00000001
+/* Note: For compilation purpose hvm_save_header name is the same as x86,
+ * but layout is different. */
+struct hvm_save_header
+    uint32_t magic;             /* Must be HVM_ARM_FILE_MAGIC */
+    uint32_t version;           /* File format version */
+    uint32_t cpuinfo;           /* Record MIDR_EL1 info of saving machine */
+DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header);
+ * Largest type-code in use
+ */
+#define HVM_SAVE_CODE_MAX 1


Hmm - it is quite poor to have this magically named "hvm_save_header".

If you were to redefine arch_hvm{save,load} as:

int arch_hvm_save(struct domain *d, struct hvm_domain_context_t *h);
int arch_hvm_load(struct domain *d, struct hvm_domain_context_t *h);

and pushed the hvm_save_entry(HEADER, 0, h, &hdr) into arch_hvm_save(),
you can remove all trace of "struct hvm_save_header" from common code.
This then removes any requirements to have an identically named struct.

It is probably worth having this all as a separate patch which just
cleans up the x86 and common code before introducing the ARM side of things.
Reasonable. I will fix next revision.

The rest of the patch is however looking fine.


Xen-devel mailing list

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.