On 03/02/2016 04:44 PM, Michael S. Tsirkin wrote:
On Wed, Mar 02, 2016 at 03:29:33PM +0800, Xiao Guangrong wrote:
On 03/02/2016 03:20 PM, Michael S. Tsirkin wrote:
On Wed, Mar 02, 2016 at 03:15:19PM +0800, Xiao Guangrong wrote:
On 03/02/2016 02:36 PM, Michael S. Tsirkin wrote:
On Wed, Mar 02, 2016 at 11:30:10AM +0800, Xiao Guangrong wrote:
On 03/02/2016 01:09 AM, Michael S. Tsirkin wrote:
Can't guest trigger this?
If yes, don't put such code in production please:
this will fill up disk on the host.
Okay, the evil guest can read the IO port freely. I will use nvdimm_debug() instead.
static void
nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
{
+ NvdimmDsmIn *in;
+ GArray *out;
+ uint32_t buf_size;
+ hwaddr dsm_mem_addr = val;
+
+ nvdimm_debug("dsm memory address %#lx.\n", dsm_mem_addr);
+
+ /*
+ * The DSM memory is mapped to guest address space so an evil guest
+ * can change its content while we are doing DSM emulation. Avoid
+ * this by copying DSM memory to QEMU local memory.
+ */
+ in = g_malloc(TARGET_PAGE_SIZE);
ugh. manual memory management :(
Hmm... Or use GArray? But it is :)
+ cpu_physical_memory_read(dsm_mem_addr, in, TARGET_PAGE_SIZE);
is there a requirement address is aligned?
if not this might cross page and crash qemu.
better read just what you need.
Yes, this memory is allocated by BIOS and we asked it to align the memory
with PAGE_SIZE:
bios_linker_loader_alloc(linker, NVDIMM_DSM_MEM_FILE, TARGET_PAGE_SIZE,
false /* high memory */);
+
+ le32_to_cpus(&in->revision);
+ le32_to_cpus(&in->function);
+ le32_to_cpus(&in->handle);
+
+ nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision,
+ in->handle, in->function);
+
+ out = g_array_new(false, true /* clear */, 1);
export build_alloc_array then, and reuse?
It is good to me, but as your suggestions, this code will be removed.
+
+ /*
+ * function 0 is called to inquire what functions are supported by
+ * OSPM
+ */
+ if (in->function == 0) {
+ build_append_int_noprefix(out, 0 /* No function Supported */,
+ sizeof(uint8_t));
What does this mean? Same comment here and below ...
If its the function 0, we return 0 that indicates no command is supported yet.
first comment says no function supported.
clearly function 0 is supported, is it not?
Yep, the comment is not clear here. It should be "No function Supported other
than function 0 "
Function 0 is the common function supported by all DSMs to inquire what functions are
supported by this DSM.
how exactly does 0 indicate no command is supported?
is it a bitmask of supported commands?
It is a bitmask. The spec said:
If Function Index is zero, the return is a buffer containing one bit for each function
index, starting with zero.
Why not start from 1?
So 0x1 - function 1 supported, 0x2 - function 2, 0x4 - function 3 etc.
It is defined by the spec in ACPI 6.0 "9.14.1 _DSM (Device Specific Method)" on page 532:
If set to zero, no functions are supported (other than function zero) for the specified UUID and
Revision ID. If set to one, at least one additional function is supported.
I audaciously guess the ACPI guys just want to reserve 0 to quickly identify if any function is
valid other than walking the bits one by one. :) but who knows...
--
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
