qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH v2 1/3] memory: add a sparse memory device for fuzzing


From: Darren Kenny
Subject: Re: [PATCH v2 1/3] memory: add a sparse memory device for fuzzing
Date: Mon, 15 Mar 2021 12:09:27 +0000

Hi Alex,

On Saturday, 2021-03-13 at 18:18:57 -05, Alexander Bulekov wrote:
> For testing, it can be useful to simulate an enormous amount of memory
> (e.g. 2^64 RAM). This adds an MMIO device that acts as sparse memory.
> When something writes a nonzero value to a sparse-mem address, we
> allocate a block of memory. This block is kept around, until all of the
> bytes within the block are zero-ed. The device has a very low priority

I don't see code below that actually checks if a block is zero-ed and
removes it from the hash table, so is this comment correct?

> (so it can be mapped beneath actual RAM, and virtual device MMIO
> regions).
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  MAINTAINERS                 |   1 +
>  hw/mem/meson.build          |   1 +
>  hw/mem/sparse-mem.c         | 152 ++++++++++++++++++++++++++++++++++++
>  include/hw/mem/sparse-mem.h |  19 +++++
>  4 files changed, 173 insertions(+)
>  create mode 100644 hw/mem/sparse-mem.c
>  create mode 100644 include/hw/mem/sparse-mem.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f22d83c178..9e3d8b1401 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2618,6 +2618,7 @@ R: Thomas Huth <thuth@redhat.com>
>  S: Maintained
>  F: tests/qtest/fuzz/
>  F: scripts/oss-fuzz/
> +F: hw/mem/sparse-mem.c
>  F: docs/devel/fuzzing.rst
>  
>  Register API
> diff --git a/hw/mem/meson.build b/hw/mem/meson.build
> index 0d22f2b572..ef79e04678 100644
> --- a/hw/mem/meson.build
> +++ b/hw/mem/meson.build
> @@ -1,5 +1,6 @@
>  mem_ss = ss.source_set()
>  mem_ss.add(files('memory-device.c'))
> +mem_ss.add(when: 'CONFIG_FUZZ', if_true: files('sparse-mem.c'))
>  mem_ss.add(when: 'CONFIG_DIMM', if_true: files('pc-dimm.c'))
>  mem_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_mc.c'))
>  mem_ss.add(when: 'CONFIG_NVDIMM', if_true: files('nvdimm.c'))
> diff --git a/hw/mem/sparse-mem.c b/hw/mem/sparse-mem.c
> new file mode 100644
> index 0000000000..575a287f59
> --- /dev/null
> +++ b/hw/mem/sparse-mem.c
> @@ -0,0 +1,152 @@
> +/*
> + * A sparse memory device. Useful for fuzzing
> + *
> + * Copyright Red Hat Inc., 2021
> + *
> + * Authors:
> + *  Alexander Bulekov   <alxndr@bu.edu>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "exec/address-spaces.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/sysbus.h"
> +#include "qapi/error.h"
> +#include "qemu/units.h"
> +#include "sysemu/qtest.h"
> +#include "hw/mem/sparse-mem.h"
> +
> +#define SPARSE_MEM(obj) OBJECT_CHECK(SparseMemState, (obj), TYPE_SPARSE_MEM)
> +#define SPARSE_BLOCK_SIZE 0x1000

This is assuming a 4K block size, should that be the same as the system
pagesize is? Or will it not matter w.r.t. how this is being consumed?

> +
> +typedef struct SparseMemState {
> +    SysBusDevice parent_obj;
> +    MemoryRegion mmio;
> +    uint64_t baseaddr;
> +    uint64_t length;
> +    uint64_t size_used;
> +    uint64_t maxsize;
> +    GHashTable *mapped;
> +} SparseMemState;
> +
> +typedef struct sparse_mem_block {
> +    uint8_t data[SPARSE_BLOCK_SIZE];
> +} sparse_mem_block;
> +
> +static uint64_t sparse_mem_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    printf("SPARSEREAD %lx\n", addr);

Should this printf() be a logging/trace call? Or do you really want it to be
printed all the time? Also seems out of place before the declaration of
the variables.

Thanks,

Darren.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]