On Tue, Mar 20, 2018 at 07:35:18AM -0400, Artemis Tosini wrote: > This patch ensures that the client OS does not cause the host to read invalid > memory from the NVDIMM DSM. It is not tested, since the Linux NVDIMM driver > will not cause an invalid memory read.
Thanks for the patch!
Please move this line into the patch description...
>
> This patch is for my outreachy assignment, and is my first open source patch.
>
> From bcb717b761ac62adeda145e895f92e4bde1003af Mon Sep 17 00:00:00 2001
> From: Artemis Tosini <m...@artemis.re>
> Date: Sat, 10 Mar 2018 20:38:07 +0000
> Subject: [PATCH] nvdimm: ensure that dsm memory is read in nvdimm_dsm_write
...here so that git-log(1) will retain it after your patch is merged.
>
> Signed-off-by: Artemis Tosini <m...@artemis.re>
> ---
> hw/acpi/nvdimm.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 59d6e4254c..67dda723a7 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -838,7 +838,12 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t
> val, unsigned size)
> * this by copying DSM memory to QEMU local memory.
> */
> in = g_new(NvdimmDsmIn, 1);
> - cpu_physical_memory_read(dsm_mem_addr, in, sizeof(*in));
> + if (address_space_read(&address_space_memory, dsm_mem_addr,
> + MEMTXATTRS_UNSPECIFIED, in,
> + sizeof(*in)) != MEMTX_OK) {
> + nvdimm_debug("Failed to read DSM memory");
> + goto exit;
> + }
Please follow QEMU coding style (see ./CODING_STYLE in the source tree)
with 4 space indentation. You can use scripts/checkpatch.pl to check
for coding style violations. Try this:
if (address_space_read(&address_space_memory, dsm_mem_addr,
MEMTXATTRS_UNSPECIFIED, in,
sizeof(*in)) != MEMTX_OK) {
nvdimm_debug("Failed to read DSM memory");
goto exit;
}
Please also look at the compiler error that patchew (the continuous
integration system used by QEMU) has reported.
The actual change looks good to me - it stops the device from processing
*in when we were unable to copy bytes from guest RAM.
Stefan
signature.asc
Description: PGP signature
