On 2017-04-03 13:59, Henning Schild wrote:
> Make the "used, dirty" firmware image visible in sysfs. The data segment
> contains data that could be useful for all sorts of debugging tools.
> This feature is introduced to extract code coverage information (gcov).
>
> Signed-off-by: Henning Schild <[email protected]>
>
> diff --git a/driver/main.c b/driver/main.c
> --- a/driver/main.c
> +++ b/driver/main.c
> @@ -81,9 +81,9 @@ struct console_state {
>
> DEFINE_MUTEX(jailhouse_lock);
> bool jailhouse_enabled;
> +void *hypervisor_mem;
>
> static struct device *jailhouse_dev;
> -static void *hypervisor_mem;
> static unsigned long hv_core_and_percpu_size;
> static atomic_t call_done;
> static int error_code;
> @@ -279,6 +279,7 @@ static int __jailhouse_console_dump_delt
>
> static void jailhouse_firmware_free(void)
> {
> + jailhouse_sysfs_firmware_exit(jailhouse_dev);
> vunmap(hypervisor_mem);
> hypervisor_mem = NULL;
> }
> @@ -419,6 +420,10 @@ static int jailhouse_cmd_enable(struct j
> header = (struct jailhouse_header *)hypervisor_mem;
> header->max_cpus = max_cpus;
>
> + err = jailhouse_sysfs_firmware_init(jailhouse_dev, hypervisor->size);
> + if (err)
> + goto error_unmap;
> +
> /*
> * ARMv8 requires to clean D-cache and invalidate I-cache for memory
> * containing new instructions. On x86 this is a NOP. On ARMv7 the
> diff --git a/driver/main.h b/driver/main.h
> --- a/driver/main.h
> +++ b/driver/main.h
> @@ -19,6 +19,7 @@
>
> extern struct mutex jailhouse_lock;
> extern bool jailhouse_enabled;
> +extern void *hypervisor_mem;
>
> void *jailhouse_ioremap(phys_addr_t phys, unsigned long virt,
> unsigned long size);
> diff --git a/driver/sysfs.c b/driver/sysfs.c
> --- a/driver/sysfs.c
> +++ b/driver/sysfs.c
> @@ -379,6 +379,29 @@ static ssize_t remap_pool_used_show(stru
> return info_show(dev, buffer, JAILHOUSE_INFO_REMAP_POOL_USED);
> }
>
> +static ssize_t firmware_show(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *a, char *buf, loff_t off,
No single-character variable names please if the variable is as complex
as "a".
> + size_t count)
> +{
> + if (mutex_lock_interruptible(&jailhouse_lock) != 0)
> + return -EINTR;
> +
> + if (jailhouse_enabled ||
> + hypervisor_mem == NULL || a->size == 0) {
Why is that check for a->size == 0 required? Not seeing this in other
drivers.
> + mutex_unlock(&jailhouse_lock);
> + return -EIO;
-EBUSY?
> + }
> + if (off >= a->size) {
> + count = 0;
> + } else {
> + if (off + count > a->size)
> + count = a->size - off;
> + memcpy(buf, hypervisor_mem + off, count);
Would memory_read_from_buffer be useful here?
> + }
> + mutex_unlock(&jailhouse_lock);
> + return count;
> +}
> +
> static DEVICE_ATTR_RO(console);
For my understanding: We need a special binary entry because attributes
like console are limited to a single page, right?
> static DEVICE_ATTR_RO(enabled);
> static DEVICE_ATTR_RO(mem_pool_size);
> @@ -401,6 +424,24 @@ static struct attribute_group jailhouse_
> .attrs = jailhouse_sysfs_entries,
> };
>
> +static struct bin_attribute bin_attr_firmware = {
> + .attr.name = "core",
> + .attr.mode = S_IRUSR,
> + .size = 0,
> + .read = firmware_show
> +};
> +
> +int jailhouse_sysfs_firmware_init(struct device *dev, size_t hypervisor_size)
> +{
> + bin_attr_firmware.size = hypervisor_size;
> + return(sysfs_create_bin_file(&dev->kobj, &bin_attr_firmware));
Coding style: return is not a function.
> +}
> +
> +void jailhouse_sysfs_firmware_exit(struct device *dev)
> +{
> + sysfs_remove_bin_file(&dev->kobj, &bin_attr_firmware);
> +}
What speaks against registering via jailhouse_sysfs_init? I mean, you
already register earlier than the data is actually available, so there
is no strict correlation with the attribute being visible.
Jan
> +
> int jailhouse_sysfs_init(struct device *dev)
> {
> int err;
> diff --git a/driver/sysfs.h b/driver/sysfs.h
> --- a/driver/sysfs.h
> +++ b/driver/sysfs.h
> @@ -19,6 +19,8 @@ int jailhouse_sysfs_cell_create(struct c
> void jailhouse_sysfs_cell_register(struct cell *cell);
> void jailhouse_sysfs_cell_delete(struct cell *cell);
>
> +int jailhouse_sysfs_firmware_init(struct device *dev, size_t
> hypervisor_size);
> +void jailhouse_sysfs_firmware_exit(struct device *dev);
> int jailhouse_sysfs_init(struct device *dev);
> void jailhouse_sysfs_exit(struct device *dev);
>
>
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
