Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [tpmdd-devel] [PATCH v7 07/10] tpm: TPM 2.0 baseline support
From: Stefan Berger @ 2014-11-26  0:42 UTC (permalink / raw)
  To: Jarkko Sakkinen, Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Will Arthur,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <1415713513-16524-8-git-send-email-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On 11/11/2014 08:45 AM, Jarkko Sakkinen wrote:
> TPM 2.0 devices are separated by adding a field 'flags' to struct
> tpm_chip and defining a flag TPM_CHIP_FLAG_TPM2 for tagging them.
>
> This patch adds the following internal functions:
>
> - tpm2_get_random()
> - tpm2_get_tpm_pt()
> - tpm2_pcr_extend()
> - tpm2_pcr_read()
> - tpm2_startup()
>
> Additionally, the following exported functions are implemented for
> implementing TPM 2.0 device drivers:
>
> - tpm2_do_selftest()
> - tpm2_calc_ordinal_durations()
> - tpm2_gen_interrupt()
>
> The existing functions that are exported for the use for existing
> subsystems have been changed to check the flags field in struct
> tpm_chip and use appropriate TPM 2.0 counterpart if
> TPM_CHIP_FLAG_TPM2 is est.
>
> The code for tpm2_calc_ordinal_duration() and tpm2_startup() were
> originally written by Will Arthur.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>   drivers/char/tpm/Makefile        |   2 +-
>   drivers/char/tpm/tpm-chip.c      |  21 +-
>   drivers/char/tpm/tpm-interface.c |  24 +-
>   drivers/char/tpm/tpm.h           |  67 +++++
>   drivers/char/tpm/tpm2-cmd.c      | 566 +++++++++++++++++++++++++++++++++++++++
>   5 files changed, 668 insertions(+), 12 deletions(-)
>   create mode 100644 drivers/char/tpm/tpm2-cmd.c
>
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 837da04..ae56af9 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -2,7 +2,7 @@
>   # Makefile for the kernel tpm device drivers.
>   #
>   obj-$(CONFIG_TCG_TPM) += tpm.o
> -tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o
> +tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o
>   tpm-$(CONFIG_ACPI) += tpm_ppi.o
>
>   ifdef CONFIG_ACPI
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 5d268ac..4d25b24 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -213,11 +213,14 @@ int tpm_chip_register(struct tpm_chip *chip)
>   	if (rc)
>   		return rc;
>
> -	rc = tpm_add_ppi(chip);
> -	if (rc)
> -		goto out_err;
> +	/* Populate sysfs for TPM1 devices. */
> +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> +		rc = tpm_add_ppi(chip);
> +		if (rc)
> +			goto out_err;
>
> -	chip->bios_dir = tpm_bios_log_setup(chip->devname);
> +		chip->bios_dir = tpm_bios_log_setup(chip->devname);
> +	}
>
>   	/* Make the chip available. */
>   	spin_lock(&driver_lock);
> @@ -248,10 +251,12 @@ void tpm_chip_unregister(struct tpm_chip *chip)
>   	spin_unlock(&driver_lock);
>   	synchronize_rcu();
>
> -	tpm_remove_ppi(chip);
> -
> -	if (chip->bios_dir)
> -		tpm_bios_log_teardown(chip->bios_dir);
> +	/* Clean up sysfs for TPM1 devices. */
> +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> +		if (chip->bios_dir)
> +			tpm_bios_log_teardown(chip->bios_dir);
> +		tpm_remove_ppi(chip);
> +	}
>
>   	tpm_dev_del_device(chip);
>   }
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 9e4ce4d..e62b835 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -360,7 +360,10 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
>   	if (chip->vendor.irq)
>   		goto out_recv;
>
> -	stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		stop = jiffies + tpm2_calc_ordinal_duration(chip, ordinal);
> +	else
> +		stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
>   	do {
>   		u8 status = chip->ops->status(chip);
>   		if ((status & chip->ops->req_complete_mask) ==
> @@ -483,7 +486,7 @@ static const struct tpm_input_header tpm_startup_header = {
>   static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
>   {
>   	struct tpm_cmd_t start_cmd;
> -	start_cmd.header.in = tpm_startup_header;
> +
>   	start_cmd.params.startup_in.startup_type = startup_type;
>   	return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
>   				"attempting to start the TPM");
> @@ -680,7 +683,10 @@ int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
>   	chip = tpm_chip_find_get(chip_num);
>   	if (chip == NULL)
>   		return -ENODEV;
> -	rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
> +	else
> +		rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
>   	tpm_chip_put(chip);
>   	return rc;
>   }
> @@ -714,6 +720,12 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
>   	if (chip == NULL)
>   		return -ENODEV;
>
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> +		rc = tpm2_pcr_extend(chip, pcr_idx, hash);
> +		tpm_chip_put(chip);
> +		return rc;
> +	}
> +
>   	cmd.header.in = pcrextend_header;
>   	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
>   	memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
> @@ -974,6 +986,12 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
>   	if (chip == NULL)
>   		return -ENODEV;
>
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> +		err = tpm2_get_random(chip, out, max);
> +		tpm_chip_put(chip);
> +		return err;
> +	}
> +
>   	do {
>   		tpm_cmd.header.in = tpm_getrandom_header;
>   		tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 9d062e6..8a434d2 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -62,6 +62,57 @@ enum tpm_duration {
>   #define TPM_ERR_INVALID_POSTINIT 38
>
>   #define TPM_HEADER_SIZE		10
> +
> +enum tpm2_const {
> +	TPM2_PLATFORM_PCR	= 24,
> +	TPM2_PCR_SELECT_MIN	= ((TPM2_PLATFORM_PCR + 7) / 8),
> +	TPM2_TIMEOUT_A		= 750 * 1000,
> +	TPM2_TIMEOUT_B		= 2000 * 1000,
> +	TPM2_TIMEOUT_C		= 200 * 1000,
> +	TPM2_TIMEOUT_D		= 30 * 1000,
> +	TPM2_DURATION_SHORT	= 20 * 1000,
> +	TPM2_DURATION_MEDIUM	= 750 * 1000,
> +	TPM2_DURATION_LONG	= 2000 * 1000,
> +};
> +
> +enum tpm2_structures {
> +	TPM2_ST_NO_SESSIONS	= 0x8001,
> +	TPM2_ST_SESSIONS	= 0x8002,
> +};
> +
> +enum tpm2_return_codes {
> +	TPM2_RC_TESTING		= 0x090A,
> +	TPM2_RC_DISABLED	= 0x0120,
> +};
> +
> +enum tpm2_algorithms {
> +	TPM2_ALG_SHA1		= 0x0004,
> +};
> +
> +enum tpm2_command_codes {
> +	TPM2_CC_FIRST		= 0x011F,
> +	TPM2_CC_SELF_TEST	= 0x0143,
> +	TPM2_CC_STARTUP		= 0x0144,
> +	TPM2_CC_GET_CAPABILITY	= 0x017A,
> +	TPM2_CC_GET_RANDOM	= 0x017B,
> +	TPM2_CC_PCR_READ	= 0x017E,
> +	TPM2_CC_PCR_EXTEND	= 0x0182,
> +	TPM2_CC_LAST		= 0x018F,
> +};
> +
> +enum tpm2_permanent_handles {
> +	TPM2_RS_PW		= 0x40000009,
> +};
> +
> +enum tpm2_capabilities {
> +	TPM2_CAP_TPM_PROPERTIES = 6,
> +};
> +
> +enum tpm2_startup_types {
> +	TPM2_SU_CLEAR	= 0x0000,
> +	TPM2_SU_STATE	= 0x0001,
> +};
> +
>   struct tpm_chip;
>
>   struct tpm_vendor_specific {
> @@ -96,12 +147,17 @@ struct tpm_vendor_specific {
>
>   #define TPM_PPI_VERSION_LEN		3
>
> +enum tpm_chip_flags {
> +	TPM_CHIP_FLAG_TPM2	= BIT(0),
> +};
> +
>   struct tpm_chip {
>   	struct device *pdev;	/* Device stuff */
>   	struct device dev;
>   	struct cdev cdev;
>
>   	const struct tpm_class_ops *ops;
> +	unsigned int flags;
>
>   	int dev_num;		/* /dev/tpm# */
>   	char devname[7];
> @@ -362,3 +418,14 @@ static inline void tpm_remove_ppi(struct tpm_chip *chip)
>   {
>   }
>   #endif
> +
> +int tpm2_startup(struct tpm_chip *chip, __be16 startup_type);
> +int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> +int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
> +int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> +
> +extern ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
> +			       u32 *value, const char *desc);
> +extern unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
> +extern int tpm2_do_selftest(struct tpm_chip *chip);
> +extern int tpm2_gen_interrupt(struct tpm_chip *chip);
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> new file mode 100644
> index 0000000..458a17d
> --- /dev/null
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -0,0 +1,566 @@
> +/*
> + * Copyright (C) 2014 Intel Corporation
> + *
> + * Authors:
> + * Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> + *
> + * Maintained by: <tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> + *
> + * This file contains TPM2 protocol implementations of the commands
> + * used by the kernel internally.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + */
> +
> +#include "tpm.h"
> +
> +struct tpm2_startup_in {
> +	__be16	startup_type;
> +} __packed;
> +
> +struct tpm2_self_test_in {
> +	u8	full_test;
> +} __packed;
> +
> +struct tpm2_pcr_read_in {
> +	__be32	pcr_selects_cnt;
> +	__be16	hash_alg;
> +	u8	pcr_select_size;
> +	u8	pcr_select[TPM2_PCR_SELECT_MIN];
> +} __packed;
> +
> +struct tpm2_pcr_read_out {
> +	__be32	update_cnt;
> +	__be32	pcr_selects_cnt;
> +	__be16	hash_alg;
> +	u8	pcr_select_size;
> +	u8	pcr_select[TPM2_PCR_SELECT_MIN];
> +	__be32	digests_cnt;
> +	__be16	digest_size;
> +	u8	digest[TPM_DIGEST_SIZE];
> +} __packed;
> +
> +struct tpm2_null_auth_area {
> +	__be32			handle;
> +	__be16			nonce_size;
> +	u8			attributes;
> +	__be16			auth_size;
> +} __packed;
> +
> +struct tpm2_pcr_extend_in {
> +	__be32				pcr_idx;
> +	__be32				auth_area_size;
> +	struct tpm2_null_auth_area	auth_area;
> +	__be32				digest_cnt;
> +	__be16				hash_alg;
> +	u8				digest[TPM_DIGEST_SIZE];
> +} __packed;
> +
> +struct tpm2_get_tpm_pt_in {
> +	__be32	cap_id;
> +	__be32	property_id;
> +	__be32	property_cnt;
> +} __packed;
> +
> +struct tpm2_get_tpm_pt_out {
> +	u8	more_data;
> +	__be32	subcap_id;
> +	__be32	property_cnt;
> +	__be32	property_id;
> +	__be32	value;
> +} __packed;
> +
> +struct tpm2_get_random_in {
> +	__be16	size;
> +} __packed;
> +
> +struct tpm2_get_random_out {
> +	__be16	size;
> +	u8	buffer[TPM_MAX_RNG_DATA];
> +} __packed;
> +
> +union tpm2_cmd_params {
> +	struct	tpm2_startup_in		startup_in;
> +	struct	tpm2_self_test_in	selftest_in;
> +	struct	tpm2_pcr_read_in	pcrread_in;
> +	struct	tpm2_pcr_read_out	pcrread_out;
> +	struct	tpm2_pcr_extend_in	pcrextend_in;
> +	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
> +	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
> +	struct	tpm2_get_random_in	getrandom_in;
> +	struct	tpm2_get_random_out	getrandom_out;
> +};
> +
> +struct tpm2_cmd {
> +	tpm_cmd_header		header;
> +	union tpm2_cmd_params	params;
> +} __packed;
> +
> +/*
> + * Array with one entry per ordinal defining the maximum amount
> + * of time the chip could take to return the result. The values
> + * of the SHORT, MEDIUM, and LONG durations are taken from the
> + * PC Client Profile (PTP) specification.
> + */
> +static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - TPM2_CC_FIRST + 1] = {
> +	TPM_UNDEFINED,		/* 11F */
> +	TPM_UNDEFINED,		/* 120 */
> +	TPM_LONG,		/* 121 */
> +	TPM_UNDEFINED,		/* 122 */
> +	TPM_UNDEFINED,		/* 123 */
> +	TPM_UNDEFINED,		/* 124 */
> +	TPM_UNDEFINED,		/* 125 */
> +	TPM_UNDEFINED,		/* 126 */
> +	TPM_UNDEFINED,		/* 127 */
> +	TPM_UNDEFINED,		/* 128 */
> +	TPM_LONG,		/* 129 */
> +	TPM_UNDEFINED,		/* 12a */
> +	TPM_UNDEFINED,		/* 12b */
> +	TPM_UNDEFINED,		/* 12c */
> +	TPM_UNDEFINED,		/* 12d */
> +	TPM_UNDEFINED,		/* 12e */
> +	TPM_UNDEFINED,		/* 12f */
> +	TPM_UNDEFINED,		/* 130 */
> +	TPM_UNDEFINED,		/* 131 */
> +	TPM_UNDEFINED,		/* 132 */
> +	TPM_UNDEFINED,		/* 133 */
> +	TPM_UNDEFINED,		/* 134 */
> +	TPM_UNDEFINED,		/* 135 */
> +	TPM_UNDEFINED,		/* 136 */
> +	TPM_UNDEFINED,		/* 137 */
> +	TPM_UNDEFINED,		/* 138 */
> +	TPM_UNDEFINED,		/* 139 */
> +	TPM_UNDEFINED,		/* 13a */
> +	TPM_UNDEFINED,		/* 13b */
> +	TPM_UNDEFINED,		/* 13c */
> +	TPM_UNDEFINED,		/* 13d */
> +	TPM_MEDIUM,		/* 13e */
> +	TPM_UNDEFINED,		/* 13f */
> +	TPM_UNDEFINED,		/* 140 */
> +	TPM_UNDEFINED,		/* 141 */
> +	TPM_UNDEFINED,		/* 142 */
> +	TPM_LONG,		/* 143 */
> +	TPM_MEDIUM,		/* 144 */
> +	TPM_UNDEFINED,		/* 145 */
> +	TPM_UNDEFINED,		/* 146 */
> +	TPM_UNDEFINED,		/* 147 */
> +	TPM_UNDEFINED,		/* 148 */
> +	TPM_UNDEFINED,		/* 149 */
> +	TPM_UNDEFINED,		/* 14a */
> +	TPM_UNDEFINED,		/* 14b */
> +	TPM_UNDEFINED,		/* 14c */
> +	TPM_UNDEFINED,		/* 14d */
> +	TPM_LONG,		/* 14e */
> +	TPM_UNDEFINED,		/* 14f */
> +	TPM_UNDEFINED,		/* 150 */
> +	TPM_UNDEFINED,		/* 151 */
> +	TPM_UNDEFINED,		/* 152 */
> +	TPM_UNDEFINED,		/* 153 */
> +	TPM_UNDEFINED,		/* 154 */
> +	TPM_UNDEFINED,		/* 155 */
> +	TPM_UNDEFINED,		/* 156 */
> +	TPM_UNDEFINED,		/* 157 */
> +	TPM_UNDEFINED,		/* 158 */
> +	TPM_UNDEFINED,		/* 159 */
> +	TPM_UNDEFINED,		/* 15a */
> +	TPM_UNDEFINED,		/* 15b */
> +	TPM_MEDIUM,		/* 15c */
> +	TPM_UNDEFINED,		/* 15d */
> +	TPM_UNDEFINED,		/* 15e */
> +	TPM_UNDEFINED,		/* 15f */
> +	TPM_UNDEFINED,		/* 160 */
> +	TPM_UNDEFINED,		/* 161 */
> +	TPM_UNDEFINED,		/* 162 */
> +	TPM_UNDEFINED,		/* 163 */
> +	TPM_UNDEFINED,		/* 164 */
> +	TPM_UNDEFINED,		/* 165 */
> +	TPM_UNDEFINED,		/* 166 */
> +	TPM_UNDEFINED,		/* 167 */
> +	TPM_UNDEFINED,		/* 168 */
> +	TPM_UNDEFINED,		/* 169 */
> +	TPM_UNDEFINED,		/* 16a */
> +	TPM_UNDEFINED,		/* 16b */
> +	TPM_UNDEFINED,		/* 16c */
> +	TPM_UNDEFINED,		/* 16d */
> +	TPM_UNDEFINED,		/* 16e */
> +	TPM_UNDEFINED,		/* 16f */
> +	TPM_UNDEFINED,		/* 170 */
> +	TPM_UNDEFINED,		/* 171 */
> +	TPM_UNDEFINED,		/* 172 */
> +	TPM_UNDEFINED,		/* 173 */
> +	TPM_UNDEFINED,		/* 174 */
> +	TPM_UNDEFINED,		/* 175 */
> +	TPM_UNDEFINED,		/* 176 */
> +	TPM_LONG,		/* 177 */
> +	TPM_UNDEFINED,		/* 178 */
> +	TPM_UNDEFINED,		/* 179 */
> +	TPM_MEDIUM,		/* 17a */
> +	TPM_LONG,		/* 17b */
> +	TPM_UNDEFINED,		/* 17c */
> +	TPM_UNDEFINED,		/* 17d */
> +	TPM_UNDEFINED,		/* 17e */
> +	TPM_UNDEFINED,		/* 17f */
> +	TPM_UNDEFINED,		/* 180 */
> +	TPM_UNDEFINED,		/* 181 */
> +	TPM_MEDIUM,		/* 182 */
> +	TPM_UNDEFINED,		/* 183 */
> +	TPM_UNDEFINED,		/* 184 */
> +	TPM_MEDIUM,		/* 185 */
> +	TPM_MEDIUM,		/* 186 */
> +	TPM_UNDEFINED,		/* 187 */
> +	TPM_UNDEFINED,		/* 188 */
> +	TPM_UNDEFINED,		/* 189 */
> +	TPM_UNDEFINED,		/* 18a */
> +	TPM_UNDEFINED,		/* 18b */
> +	TPM_UNDEFINED,		/* 18c */
> +	TPM_UNDEFINED,		/* 18d */
> +	TPM_UNDEFINED,		/* 18e */
> +	TPM_UNDEFINED		/* 18f */
> +};
> +
> +static const struct tpm_input_header tpm2_startup_header = {
> +	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> +	.length = cpu_to_be32(12),

12 -> sizeof(struct tpm_input_header) + sizeof(struct pm2_startup_in)

> +	.ordinal = cpu_to_be32(TPM2_CC_STARTUP)
> +};
> +
> +/**
> + * tpm2_startup() - send startup command to the TPM chip
> + * @chip:		TPM chip to use.
> + * @startup_type	startup type. The value is either
> + *			TPM_SU_CLEAR or TPM_SU_STATE.
> + *
> + * 0 is returned when the operation is successful. When a negative number is
> + * returned it remarks a POSIX error code. When a positive number is returned
> + * it remarks a TPM error.

Replace 'When's with 'If's. (when being 'temporal')

> + */
> +int tpm2_startup(struct tpm_chip *chip, __be16 startup_type)
> +{
> +	struct tpm2_cmd cmd;
> +
> +	cmd.header.in = tpm2_startup_header;
> +
> +	cmd.params.startup_in.startup_type = startup_type;
> +	return tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
> +				"attempting to start the TPM");
> +}
> +
> +#define TPM2_PCR_READ_IN_SIZE \
> +	(sizeof(struct tpm_input_header) + \
> +	 sizeof(struct tpm2_pcr_read_in))
> +

Ah! You could also use a #define above!

> +static const struct tpm_input_header tpm2_pcrread_header = {
> +	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> +	.length = cpu_to_be32(TPM2_PCR_READ_IN_SIZE),
> +	.ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
> +};
> +
> +/**
> + * tpm2_pcr_read() - read a PCR value
> + * @chip:	TPM chip to use.
> + * @pcr_idx:	index of the PCR to read.
> + * @ref_buf:	buffer to store the resulting hash,
> + *
> + * 0 is returned when the operation is successful. When a negative number is
> + * returned it remarks a POSIX error code. When a positive number is returned
> + * it remarks a TPM error.
> + */

Replace 'When's with 'If's. Also further below.

> +int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> +{
> +	int rc;
> +	struct tpm2_cmd cmd;
> +	u8 *buf;
> +	int i, j;
> +
> +	if (pcr_idx >= TPM2_PLATFORM_PCR)
> +		return -EINVAL;
> +
> +	cmd.header.in = tpm2_pcrread_header;
> +	cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
> +	cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
> +	cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
> +
> +	for (i = 0; i < TPM2_PCR_SELECT_MIN; i++) {
> +		j = pcr_idx - i * 8;
> +
> +		cmd.params.pcrread_in.pcr_select[i] =
> +			(j >= 0 && j < 8) ? 1 << j : 0;
> +	}

Umpf - what's this? You need to set the PCR index as an index in the 
bitfield?

pcr_idx >> 3  gives you the index into the array, assuming that [0] 
holds bits for PCR0 to 7.

1 << (pcr_idx & 0x7) gives you the bit to set, assuming bit 0 is to be 
set for PCR 0
1 << (7- (pcr_idx & 0x7)) gives you the bit to set, assuming bit 7 is to 
be set for PCR 0.

memset(cmd.params.pcrread_in.pcr_select, 0, 
sizeof(cmd.params.pcrread_in.pcr_select));
cmd.params.pcrread_in.pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);



> +
> +	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
> +			      "attempting to read a pcr value");
> +
> +	if (rc == 0) {
> +		buf = cmd.params.pcrread_out.digest;
> +		memcpy(res_buf, buf, TPM_DIGEST_SIZE);
> +	}
> +
> +	return rc;
> +}
> +
> +/**
> + * tpm2_pcr_extend() - extend a PCR value
> + * @chip:	TPM chip to use.
> + * @pcr_idx:	index of the PCR.
> + * @hash:	hash value to use for the extend operation.
> + *
> + * 0 is returned when the operation is successful. When a negative number is
> + * returned it remarks a POSIX error code. When a positive number is returned
> + * it remarks a TPM error.
> + */
> +static const struct tpm_input_header tpm2_pcrextend_header = {
> +	.tag = cpu_to_be16(TPM2_ST_SESSIONS),
> +	.length = cpu_to_be32(sizeof(struct tpm_input_header) +
> +			      sizeof(struct tpm2_pcr_extend_in)),
> +	.ordinal = cpu_to_be32(TPM2_CC_PCR_EXTEND)
> +};
> +
> +int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
> +{
> +	struct tpm2_cmd cmd;
> +	int rc;
> +
> +	cmd.header.in = tpm2_pcrextend_header;
> +	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
> +	cmd.params.pcrextend_in.auth_area_size =
> +		cpu_to_be32(sizeof(struct tpm2_null_auth_area));
> +	cmd.params.pcrextend_in.auth_area.handle =
> +		cpu_to_be32(TPM2_RS_PW);
> +	cmd.params.pcrextend_in.auth_area.nonce_size = 0;
> +	cmd.params.pcrextend_in.auth_area.attributes = 0;
> +	cmd.params.pcrextend_in.auth_area.auth_size = 0;
> +	cmd.params.pcrextend_in.digest_cnt = cpu_to_be32(1);
> +	cmd.params.pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
> +	memcpy(cmd.params.pcrextend_in.digest, hash, TPM_DIGEST_SIZE);
> +
> +	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
> +			      "attempting extend a PCR value");
> +
> +	return rc;
> +}
> +
> +static const struct tpm_input_header tpm2_getrandom_header = {
> +	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> +	.length = cpu_to_be32(sizeof(struct tpm_input_header) +
> +			      sizeof(struct tpm2_get_random_in)),
> +	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
> +};
> +
> +/**
> + * tpm2_get_random() - get random bytes from the TPM RNG
> + * @chip: TPM chip to use
> + * @out: destination buffer for the random bytes
> + * @max: the max number of bytes to write to @out
> + *
> + * 0 is returned when the operation is successful. When a negative number is
> + * returned it remarks a POSIX error code. When a positive number is returned
> + * it remarks a TPM error.
> + */
> +int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> +{
> +	struct tpm2_cmd cmd;
> +	u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
num_bytes = min_t(u32, max, sizeof(tpm2_cmd.params.getrandom_out.buffer);

This way you tie it to the actual buffer size of the buffer the TPM can fill

> +	int err, total = 0, retries = 5;
> +	u8 *dest = out;
> +
> +	if (!out || !num_bytes || max > TPM_MAX_RNG_DATA)

... || max > sizeof((tpm2_cmd.params.getrandom_out.buffer)


> +		return -EINVAL;
> +
> +	do {
> +		cmd.header.in = tpm2_getrandom_header;
> +		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
> +
> +		err = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
> +				       "attempting get random");
> +		if (err)
> +			break;
> +
> +		recd = be16_to_cpu(cmd.params.getrandom_out.size);
> +		memcpy(dest, cmd.params.getrandom_out.buffer, recd);

to be on the safe side, I would do

/* never accept more bytes than we asked for */
recd = min(recd, num_bytes);
memcpy(dest, cmd.params.getrandom_out.buffer, recd);

so that the destination buffer can never be overwritten beyond its 
boundaries by a TPM that just ends up sending you more bytes than you 
asked for (firmware bug).

> +
> +		dest += recd;
> +		total += recd;
> +		num_bytes -= recd;
> +	} while (retries-- && total < max);
> +
> +	return total ? total : -EIO;
> +}
> +
> +#define TPM2_GET_TPM_PT_IN_SIZE \
> +	(sizeof(struct tpm_input_header) + \
> +	 sizeof(struct tpm2_get_tpm_pt_in))
> +
> +static const struct tpm_input_header tpm2_get_tpm_pt_header = {
> +	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> +	.length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
> +	.ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
> +};
> +
> +/**
> + * tpm2_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
> + * @chip:		TPM chip to use.
> + * @property_id:	property ID.
> + * @value:		output variable.
> + * @desc:		passed to tpm_transmit_cmd()
> + *
> + * 0 is returned when the operation is successful. When a negative number is
> + * returned it remarks a POSIX error code. When a positive number is returned
> + * it remarks a TPM error.
> + */
> +ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
> +			const char *desc)
> +{
> +	struct tpm2_cmd cmd;
> +	int rc;
> +
> +	cmd.header.in = tpm2_get_tpm_pt_header;
> +	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
> +	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(property_id);
> +	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
> +
> +	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), desc);
> +	if (!rc)
> +		*value = cmd.params.get_tpm_pt_out.value;
> +
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
> +
> +/*
> + * tpm2_calc_ordinal_duration() - maximum duration for a command
> + * @chip:	TPM chip to use.
> + * @ordinal:	command code number.
> + *
> + * 0 is returned when the operation is successful. When a negative number is
> + * returned it remarks a POSIX error code. When a positive number is returned
> + * it remarks a TPM error.
> + */
> +unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
> +{
> +	int index = TPM_UNDEFINED;
> +	int duration = 0;
> +
> +	if (ordinal >= TPM2_CC_FIRST && ordinal <= TPM2_CC_LAST)
> +		index = tpm2_ordinal_duration[ordinal - TPM2_CC_FIRST];
> +
> +	if (index != TPM_UNDEFINED)
> +		duration = chip->vendor.duration[index];
> +	if (duration <= 0)
> +		return 2 * 60 * HZ;
> +	else
> +		return duration;
> +}
> +EXPORT_SYMBOL_GPL(tpm2_calc_ordinal_duration);
> +
> +static const struct tpm_input_header tpm2_selftest_header = {
> +	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> +	.length = cpu_to_be32(sizeof(struct tpm_input_header) +
> +			      sizeof(struct tpm2_self_test_in)),
> +	.ordinal = cpu_to_be32(TPM2_CC_SELF_TEST)
> +};
> +
> +#define TPM2_SELF_TEST_IN_SIZE \
> +	(sizeof(struct tpm_input_header) + sizeof(struct tpm2_self_test_in))
> +
> +/**
> + * tpm2_continue_selftest() - start a self test
> + * @chip: TPM chip to use
> + * @full: test all commands instead of testing only those that were not
> + *        previously tested.
> + *
> + * 0 is returned when the operation is successful. When a negative number is
> + * returned it remarks a POSIX error code. When a positive number is returned
> + * it remarks a TPM error.
> + */
> +static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
> +{
> +	int rc;
> +	struct tpm2_cmd cmd;
> +
> +	cmd.header.in = tpm2_selftest_header;
> +	cmd.params.selftest_in.full_test = full;
> +
> +	rc = tpm_transmit_cmd(chip, &cmd, TPM2_SELF_TEST_IN_SIZE,
> +			      "continue selftest");
> +
> +	return rc;
> +}
> +
> +/**
> + * tpm2_do_selftest() - run a full self test
> + * @chip: TPM chip to use
> + *
> + * During the self test TPM2 commands return with the error code RC_TESTING.
> + * Waiting is done by issuing PCR read until it executes successfully.
> + *
> + * 0 is returned when the operation is successful. When a negative number is
> + * returned it remarks a POSIX error code. When a positive number is returned
> + * it remarks a TPM error.
> + */
> +int tpm2_do_selftest(struct tpm_chip *chip)
> +{
> +	int rc;
> +	unsigned int loops;
> +	unsigned int delay_msec = 100;
> +	unsigned long duration;
> +	struct tpm2_cmd cmd;
> +	int i;
> +
> +	duration = tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST);
> +
> +	loops = jiffies_to_msecs(duration) / delay_msec;
> +
> +	rc = tpm2_start_selftest(chip, true);
> +	if (rc)
> +		return rc;
> +
> +	for (i = 0; i < loops; i++) {
> +		/* Attempt to read a PCR value */
> +		cmd.header.in = tpm2_pcrread_header;
> +		cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
> +		cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
> +		cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
> +		cmd.params.pcrread_in.pcr_select[0] = 0x01;
> +		cmd.params.pcrread_in.pcr_select[1] = 0x00;
> +		cmd.params.pcrread_in.pcr_select[2] = 0x00;
> +
> +		rc = tpm_transmit_cmd(chip, (u8 *) &cmd, sizeof(cmd), NULL);
> +		if (rc < 0)
> +			break;
> +
> +		rc = be32_to_cpu(cmd.header.out.return_code);
> +		if (rc != TPM2_RC_TESTING)
> +			break;
> +
> +		msleep(delay_msec);
> +	}
> +
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(tpm2_do_selftest);
> +
> +/**
> + * tpm2_gen_interrupt() - generate an interrupt
> + * @chip: TPM chip to use
> + *
> + * 0 is returned when the operation is successful. When a negative number is
> + * returned it remarks a POSIX error code. When a positive number is returned
> + * it remarks a TPM error.
> + */
> +
> +int tpm2_gen_interrupt(struct tpm_chip *chip)
> +{
> +	u32 dummy;
> +	int rc;
> +
> +	rc = tpm2_get_tpm_pt(chip,
> +			     TPM2_CAP_TPM_PROPERTIES,
> +			     &dummy,
> +			     "attempting to generate an interrupt");
> +
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(tpm2_gen_interrupt);

      Stefan

^ permalink raw reply

* Re: [PATCH v14 0/3] Add drm driver for Rockchip Socs
From: Heiko Stübner @ 2014-11-26  0:37 UTC (permalink / raw)
  To: Dave Airlie, Joerg Roedel
  Cc: Mark Yao, Boris BREZILLON, Rob Clark, Daniel Vetter, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Randy Dunlap,
	Grant Likely, Greg Kroah-Hartman, John Stultz, Rom Lemarchand,
	devicetree@vger.kernel.org, linux-doc, LKML, dri-devel, linux-api,
	linux-rockchip, Douglas Anderson, Stéphane Marchesin
In-Reply-To: <CAPM=9tzvvb5U+8KaCO+K6CJi=rwWOkoW+4ZSFW_Oo9gTncvkWA@mail.gmail.com>

Hi Joerg, Dave,

Am Mittwoch, 26. November 2014, 09:12:56 schrieb Dave Airlie:
> On 26 November 2014 at 02:38, Heiko Stübner <heiko@sntech.de> wrote:
> > Mark,
> > 
> > Am Donnerstag, 20. November 2014, 09:46:34 schrieb Mark Yao:
> >> This a series of patches is a DRM Driver for Rockchip Socs, add support
> >> for vop devices. Future patches will add additional encoders/connectors,
> >> such as eDP, HDMI.
> >> 
> >> The basic "crtc" for rockchip is a "VOP" - Video Output Processor.
> >> the vop devices found on Rockchip rk3288 Soc, rk3288 soc have two similar
> >> Vop devices. Vop devices support iommu mapping, we use dma-mapping API
> >> with
> >> ARM_DMA_USE_IOMMU.
> > 
> > it looks like everybody is more or less happy with this version - in past
> > versions responses voicing concerns where quite swift.
> > 
> > As David requested a pull request the last time, it might be time to do
> > so, so that we maybe still reach 3.19.
> 
> I need the iommu changes the drm driver depends on in a stable
> maintainer git tree,
> I'm not pulling in code I can't build.

Joerg, is your arm/rockchip branch [0] considered stable?

[0] https://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/log/?h=arm/rockchip


> I'm happy if the rockchip drm pull stays static and I can pull it in
> once the iommu bits
> are done.

While testing this I noticed a quite big kconfig warning
(on both 3.18-rc1 and 3.18-rc6):

drivers/video/fbdev/Kconfig:5:error: recursive dependency detected!
drivers/video/fbdev/Kconfig:5:  symbol FB is selected by DRM_KMS_FB_HELPER
drivers/gpu/drm/Kconfig:34:     symbol DRM_KMS_FB_HELPER is selected by DRM_ROCKCHIP
drivers/gpu/drm/rockchip/Kconfig:1:     symbol DRM_ROCKCHIP depends on ARM_DMA_USE_IOMMU
arch/arm/Kconfig:95:    symbol ARM_DMA_USE_IOMMU is selected by VIDEO_OMAP3
drivers/media/platform/Kconfig:96:      symbol VIDEO_OMAP3 depends on VIDEO_V4L2
drivers/media/v4l2-core/Kconfig:6:      symbol VIDEO_V4L2 depends on I2C
drivers/i2c/Kconfig:7:  symbol I2C is selected by FB_DDC
drivers/video/fbdev/Kconfig:59: symbol FB_DDC is selected by FB_CYBER2000_DDC
drivers/video/fbdev/Kconfig:374:        symbol FB_CYBER2000_DDC depends on FB_CYBER2000
drivers/video/fbdev/Kconfig:362:        symbol FB_CYBER2000 depends on FB

It looks like the VIDEO_OMAP3 is selecting ARM_DMA_USE_IOMMU and
OMAP_IOMMU instead of depending on it.

I'm not sure which driver should change but looking at the exynos drm
driver it is also depends on the iommu parts instead of selecting them
[like the rockchip drm currently does]


Heiko

^ permalink raw reply

* Re: [tpmdd-devel] [PATCH v7 10/10] tpm: TPM 2.0 sysfs attributes
From: Stefan Berger @ 2014-11-25 23:55 UTC (permalink / raw)
  To: Jarkko Sakkinen, Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <1415713513-16524-11-git-send-email-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On 11/11/2014 08:45 AM, Jarkko Sakkinen wrote:
> Manadatory sysfs attributes for TPM 2.0 devices so that it is easy
> to check whether storage hierarchies are enabled and use PPI
> interface.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>   Documentation/ABI/stable/sysfs-class-tpm2 |  57 +++++++++++
>   drivers/char/tpm/Makefile                 |   2 +-
>   drivers/char/tpm/tpm-chip.c               |  21 +++--
>   drivers/char/tpm/tpm.h                    |  19 ++++
>   drivers/char/tpm/tpm2-sysfs.c             | 152 ++++++++++++++++++++++++++++++
>   5 files changed, 241 insertions(+), 10 deletions(-)
>   create mode 100644 Documentation/ABI/stable/sysfs-class-tpm2
>   create mode 100644 drivers/char/tpm/tpm2-sysfs.c
>
> diff --git a/Documentation/ABI/stable/sysfs-class-tpm2 b/Documentation/ABI/stable/sysfs-class-tpm2
> new file mode 100644
> index 0000000..301ab2e
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-class-tpm2
> @@ -0,0 +1,57 @@
> +What:		/sys/class/misc/tpmX/device/
> +Date:		October 2014
> +KernelVersion:	3.19
> +Contact:	tpmdd-devel-TtF/mJH4Jtrk1uMJSBkQmQ@public.gmane.org
> +Description:	The device/ directory under a specific TPM instance exposes
> +		the properties of that TPM chip.
> +
> +What:		/sys/class/misc/tpmX/device/family
> +Date:		October 2014
> +KernelVersion:	3.19
> +Contact:	tpmdd-devel-TtF/mJH4Jtrk1uMJSBkQmQ@public.gmane.org
> +Description:	The protocol family in the major.minor format.
> +

What protocol ? The TPM protocol family .. ?

> +What:		/sys/class/misc/tpmX/device/sh_enabled
> +Date:		October 2014
> +KernelVersion:	3.19
> +Contact:	tpmdd-devel-TtF/mJH4Jtrk1uMJSBkQmQ@public.gmane.org
> +Description:	The "sh_enabled" property prints a '1' if the Storage Hierarchy
> +		is enabled, i.e. if PM_PT_STARTUP_CLEAR.shEnable is set.

Why capital letters for storage hierarchy?

> +
> +What:		/sys/class/misc/tpmX/device/sh_owned
> +Date:		October 2014
> +KernelVersion:	3.19
> +Contact:	tpmdd-devel-TtF/mJH4Jtrk1uMJSBkQmQ@public.gmane.org
> +Description:	The "sh_owned" property prints a '1' if the ownership of the
> +		Storage Hierarchy has been taken, i.e. if
> +		TPM_PT_PERMANENT.ownerAuthSet is set.
> +

Same her

> +What:		/sys/class/misc/tpmX/device/eh_enabled
> +Date:		October 2014
> +KernelVersion:	3.19
> +Contact:	tpmdd-devel-TtF/mJH4Jtrk1uMJSBkQmQ@public.gmane.org
> +Description:	The "eh_enabled" property prints a '1' if the Endorsement
> +		Hierarchy is enabled, i.e if PM_PT_STARTUP_CLEAR.ehEnable is
> +		set.


> +
> +What:		/sys/class/misc/tpmX/device/eh_owned
> +Date:		October 2014
> +KernelVersion:	3.19
> +Contact:	tpmdd-devel-TtF/mJH4Jtrk1uMJSBkQmQ@public.gmane.org
> +Description:	The "eh_owned" property prints a '1' if the ownership of the
> +		Endrosoment Hierarchy has been taken, i.e if
> +		TPM_PT_PERMANENT.endorsementAuthSet is set.
> +
> +What:		/sys/class/misc/tpmX/device/manufacturer
> +Date:		October 2014
> +KernelVersion:	3.19
> +Contact:	tpmdd-devel-TtF/mJH4Jtrk1uMJSBkQmQ@public.gmane.org
> +Description:	The "manufacturer" property prints the vendor ID of the TPM
> +		manufacturer.
> +
> +What:		/sys/class/misc/tpmX/device/firmware
> +Date:		October 2014
> +KernelVersion:	3.19
> +Contact:	tpmdd-devel-TtF/mJH4Jtrk1uMJSBkQmQ@public.gmane.org
> +Description:	The property prints the vendor-specific value indicating the
> +		version of the firmware.
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index e6d26dd..15e3b4c 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -2,7 +2,7 @@
>   # Makefile for the kernel tpm device drivers.
>   #
>   obj-$(CONFIG_TCG_TPM) += tpm.o
> -tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o
> +tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o tpm2-sysfs.o
>   tpm-$(CONFIG_ACPI) += tpm_ppi.o
>
>   ifdef CONFIG_ACPI
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 4d25b24..accd408 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -30,6 +30,7 @@
>   #include "tpm_eventlog.h"
>
>   ATTRIBUTE_GROUPS(tpm_dev);
> +ATTRIBUTE_GROUPS(tpm2_dev);
>
>   static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
>   static LIST_HEAD(tpm_chip_list);
> @@ -138,7 +139,10 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>   	else
>   		chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num);
>
> -	chip->dev.groups = tpm_dev_groups;
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		chip->dev.groups = tpm2_dev_groups;
> +	else
> +		chip->dev.groups = tpm_dev_groups;
>
>   	dev_set_name(&chip->dev, chip->devname);
>
> @@ -213,14 +217,12 @@ int tpm_chip_register(struct tpm_chip *chip)
>   	if (rc)
>   		return rc;
>
> -	/* Populate sysfs for TPM1 devices. */
> -	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> -		rc = tpm_add_ppi(chip);
> -		if (rc)
> -			goto out_err;
> +	rc = tpm_add_ppi(chip);
> +	if (rc)
> +		goto out_err;
>
> +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
>   		chip->bios_dir = tpm_bios_log_setup(chip->devname);
> -	}
>
>   	/* Make the chip available. */
>   	spin_lock(&driver_lock);
> @@ -251,8 +253,9 @@ void tpm_chip_unregister(struct tpm_chip *chip)
>   	spin_unlock(&driver_lock);
>   	synchronize_rcu();
>
> -	/* Clean up sysfs for TPM1 devices. */
> -	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> +		tpm_remove_ppi(chip);

Take the tpm_remove_ppi(chip) out of the if and else branches into a 
common path.

> +	} else {
>   		if (chip->bios_dir)
>   			tpm_bios_log_teardown(chip->bios_dir);
>   		tpm_remove_ppi(chip);
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 8a434d2..1548182 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -108,6 +108,24 @@ enum tpm2_capabilities {
>   	TPM2_CAP_TPM_PROPERTIES = 6,
>   };
>
> +enum tpm2_tpm_properties {
> +	TPM2_PT_MANUFACTURER		= 0x00000105,
> +	TPM2_PT_FIRMWARE_VERSION_1	= 0x0000010C,
> +	TPM2_PT_FIRMWARE_VERSION_2	= 0x0000010D,
> +	TPM2_PT_PERMANENT		= 0x00000200,
> +	TPM2_PT_STARTUP_CLEAR		= 0x00000201,
> +};
> +
> +enum tpm2_pt_startup_clear {
> +	TPM2_PT_SC_SH_ENABLE	= BIT(1),
> +	TPM2_PT_SC_EH_ENABLE	= BIT(2),
> +};
> +
> +enum tpm2_pt_permanent {
> +	TPM2_PT_PM_OWNER_AUTH_SET	= BIT(0),
> +	TPM2_PT_PM_ENDORSEMENT_AUTH_SET	= BIT(1),
> +};
> +
>   enum tpm2_startup_types {
>   	TPM2_SU_CLEAR	= 0x0000,
>   	TPM2_SU_STATE	= 0x0001,
> @@ -382,6 +400,7 @@ extern struct class *tpm_class;
>   extern dev_t tpm_devt;
>   extern const struct file_operations tpm_fops;
>   extern struct attribute *tpm_dev_attrs[];
> +extern struct attribute *tpm2_dev_attrs[];
>
>   ssize_t	tpm_getcap(struct device *, __be32, cap_t *, const char *);
>   ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
> diff --git a/drivers/char/tpm/tpm2-sysfs.c b/drivers/char/tpm/tpm2-sysfs.c
> new file mode 100644
> index 0000000..9e5e2e3
> --- /dev/null
> +++ b/drivers/char/tpm/tpm2-sysfs.c
> @@ -0,0 +1,152 @@
> +/*
> + * Copyright (C) 2014 Intel Corporation
> + *
> + * Authors:
> + * Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> + *
> + * TPM2 sysfs attributes
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2 of the
> + * License.
> + *
> + */
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include "tpm.h"
> +
> +static ssize_t sh_enabled_show(struct device *dev, struct device_attribute *attr,
> +		     char *buf)
> +{
> +	struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
> +	u32 value;
> +	ssize_t rc;
> +
> +	rc = tpm2_get_tpm_pt(chip, TPM2_PT_STARTUP_CLEAR, &value,
> +			     "could not retrieve STARTUP_CLEAR property");
> +	if (rc)
> +		return 0;
> +
> +	rc = sprintf(buf, "%d\n", (value & TPM2_PT_SC_SH_ENABLE) > 0);
> +	return rc;
> +}
> +static DEVICE_ATTR_RO(sh_enabled);
> +
> +static ssize_t sh_owned_show(struct device *dev, struct device_attribute *attr,
> +			  char *buf)
> +{
> +	struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
> +	u32 value;
> +	ssize_t rc;
> +
> +	rc = tpm2_get_tpm_pt(chip, TPM2_PT_PERMANENT, &value,
> +			     "could not retrieve PERMANENT property");
> +	if (rc)
> +		return 0;
> +
> +	rc = sprintf(buf, "%d\n", (value & TPM2_PT_PM_OWNER_AUTH_SET) > 0);
> +	return rc;
> +}
> +static DEVICE_ATTR_RO(sh_owned);
> +
> +static ssize_t eh_enabled_show(struct device *dev, struct device_attribute *attr,
> +		     char *buf)
> +{
> +	struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
> +	u32 value;
> +	ssize_t rc;
> +
> +	rc = tpm2_get_tpm_pt(chip, TPM2_PT_STARTUP_CLEAR, &value,
> +			     "could not retrieve STARTUP_CLEAR property");
> +	if (rc)
> +		return 0;
> +
> +	rc = sprintf(buf, "%d\n", (value & TPM2_PT_SC_EH_ENABLE) > 0);
> +	return rc;
> +}
> +static DEVICE_ATTR_RO(eh_enabled);
> +
> +static ssize_t eh_owned_show(struct device *dev, struct device_attribute *attr,
> +			  char *buf)
> +{
> +	struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
> +	u32 value;
> +	ssize_t rc;
> +
> +	rc = tpm2_get_tpm_pt(chip, TPM2_PT_PERMANENT, &value,
> +			     "could not retrieve PERMANENT property");
> +	if (rc)
> +		return 0;
> +
> +	rc = sprintf(buf, "%d\n", (value & TPM2_PT_PM_ENDORSEMENT_AUTH_SET) > 0);
> +	return rc;
> +}
> +static DEVICE_ATTR_RO(eh_owned);
> +
> +static ssize_t manufacturer_show(struct device *dev,
> +				 struct device_attribute *attr,
> +				 char *buf)
> +{
> +	struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
> +	u32 manufacturer;
> +	ssize_t rc;
> +	char *str = buf;
> +
> +	rc = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, (u32 *) &manufacturer,

seems an unnecessary cast.

> +			     "could not retrieve MANUFACTURER property");
> +	if (rc)
> +		return 0;
> +
> +	str += sprintf(str, "0x%08x\n", be32_to_cpu(manufacturer));
> +
rc = sprintf()
return rc;

Like above?
> +	return str - buf;
> +}
> +static DEVICE_ATTR_RO(manufacturer);
> +
> +static ssize_t firmware_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
> +	u32 firmware1;
> +	u32 firmware2;
> +	ssize_t rc;
> +	char *str = buf;
> +
> +	rc = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_1, (u32 *) &firmware1,
> +			     "could not retrieve FIRMWARE_VERSION_1 property");
> +	if (rc)
> +		return 0;
> +
> +	rc = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_2, (u32 *) &firmware2,
> +			     "could not retrieve FIRMWARE_VERSION_2 property");
> +	if (rc)
> +		return 0;
> +
> +	str += sprintf(str, "0x%08x.0x%08x\n", firmware1, firmware2);
> +
> +	return str - buf;

Same for here, why not again do rc = sprintf .. return rc; ?

> +}
> +static DEVICE_ATTR_RO(firmware);
> +
> +static ssize_t family_show(struct device *dev, struct device_attribute *attr,
> +			   char *buf)
> +{
> +	char *str = buf;
> +
> +	str += sprintf(str, "2.0\n");
> +
> +	return str - buf;

return sprintf(buf, "2.0\n");

> +}
> +static DEVICE_ATTR_RO(family);
> +
> +struct attribute *tpm2_dev_attrs[] = {
> +	&dev_attr_sh_enabled.attr,
> +	&dev_attr_sh_owned.attr,
> +	&dev_attr_eh_enabled.attr,
> +	&dev_attr_eh_owned.attr,
> +	&dev_attr_manufacturer.attr,
> +	&dev_attr_firmware.attr,
> +	&dev_attr_family.attr,
> +	NULL,
> +};

     Stefan

^ permalink raw reply

* Re: [PATCH v3] selftest: size: Add size test for Linux kernel
From: Tim Bird @ 2014-11-25 23:33 UTC (permalink / raw)
  To: Shuah Khan, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-embedded-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Josh Triplett
In-Reply-To: <5474E8F6.8010307-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>



On 11/25/2014 12:39 PM, Shuah Khan wrote:
> On 11/24/2014 11:20 AM, Tim Bird wrote:
>>
>> This test shows the amount of memory used by the system.
>> Note that this is dependent on the user-space that is loaded
>> when this program runs.  Optimally, this program would be
>> run as the init program itself.
>>
>> The program is optimized for size itself, to avoid conflating
>> its own execution with that of the system software.
>> The code is compiled statically, with no stdlibs. On my x86_64 system,
>> this results in a statically linked binary of less than 5K.
>>
> 
> This following version information shouldn't be part of changelog.
> 
>> Changes from v2:
>>   - add return values to print routines
>>   - add .gitignore file
>>
>> Changes from v1:
>>   - use more correct Copyright string in get_size.c
>>
>> Signed-off-by: Tim Bird <tim.bird-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
>> ---
> 
> Goes here.

OK - I'll change this.

> 
>>  tools/testing/selftests/Makefile        |   1 +
>>  tools/testing/selftests/size/.gitignore |   1 +
>>  tools/testing/selftests/size/Makefile   |  21 +++++++
>>  tools/testing/selftests/size/get_size.c | 105 ++++++++++++++++++++++++++++++++
>>  4 files changed, 128 insertions(+)
>>  create mode 100644 tools/testing/selftests/size/.gitignore
>>  create mode 100644 tools/testing/selftests/size/Makefile
>>  create mode 100644 tools/testing/selftests/size/get_size.c
>>
>> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
>> index 45f145c..fa91aef 100644
>> --- a/tools/testing/selftests/Makefile
>> +++ b/tools/testing/selftests/Makefile
>> @@ -15,6 +15,7 @@ TARGETS += user
>>  TARGETS += sysctl
>>  TARGETS += firmware
>>  TARGETS += ftrace
>> +TARGETS += size
>>  
>>  TARGETS_HOTPLUG = cpu-hotplug
>>  TARGETS_HOTPLUG += memory-hotplug
>> diff --git a/tools/testing/selftests/size/.gitignore b/tools/testing/selftests/size/.gitignore
>> new file mode 100644
>> index 0000000..189b781
>> --- /dev/null
>> +++ b/tools/testing/selftests/size/.gitignore
>> @@ -0,0 +1 @@
>> +get_size
>> diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
>> new file mode 100644
>> index 0000000..51e5fbd
>> --- /dev/null
>> +++ b/tools/testing/selftests/size/Makefile
>> @@ -0,0 +1,21 @@
>> +#ifndef CC
>> +	CC = $(CROSS_COMPILE)gcc
>> +#endif
>> +
>> +#ifndef STRIP
>> +	STRIP = $(CROSS_COMPILE)strip
>> +#endif
>> +
>> +all: get_size
>> +
>> +get_size: get_size.c
>> +	$(CC) --static -ffreestanding -nostartfiles \
>> +		-Wl,--entry=main get_size.c -o get_size \
>> +		`cc -print-libgcc-file-name`
>> +	$(STRIP) -s get_size
>> +
>> +run_tests: all
>> +	./get_size
>> +
>> +clean:
>> +	$(RM) get_size
>> diff --git a/tools/testing/selftests/size/get_size.c b/tools/testing/selftests/size/get_size.c
>> new file mode 100644
>> index 0000000..9c8d3cd
>> --- /dev/null
>> +++ b/tools/testing/selftests/size/get_size.c
>> @@ -0,0 +1,105 @@
>> +/*
>> + * Copyright 2014 Sony
>> + *
>> + * Licensed under the terms of the GNU GPL License version 2
>> + *
>> + * Selftest for runtime system size
>> + *
>> + * Prints the amount of RAM that the currently running system is using.
>> + *
>> + * This program tries to be as small as possible itself, to
>> + * avoid perturbing the system memory utilization with its
>> + * own execution.  It also attempts to have as few dependencies
>> + * on kernel features as possible.
>> + *
>> + * It should be statically linked, with startup libs avoided.
>> + * It uses no library calls, and only the following 3 syscalls:
>> + *   sysinfo(), write(), and _exit()
>> + *
>> + * For output, it avoids printf (which in some C libraries
>> + * has large external dependencies) by implementing its own
>> + * strlen(), number output and print() routines.
>> + */
>> +
>> +#include <sys/sysinfo.h>
>> +#include <unistd.h>
>> +
>> +#define STDOUT_FILENO 1
>> +
>> +int my_strlen(const char *s)
>> +{
>> +	int len = 0;
>> +
>> +	while (*s++)
>> +		len++;
>> +	return len;
>> +}
>> +
>> +/*
>> + * num_to_str - put digits from num into *s, left to right
>> + *   do this by dividing the number by powers of 10
>> + *   the tricky part is to omit leading zeros
>> + *   don't print zeros until we've started printing any numbers at all
>> + */
>> +void num_to_str(unsigned long num, char *s)
>> +{
>> +	unsigned long long temp, div;
>> +	int started;
>> +
>> +	temp = num;
>> +	div = 1000000000000000000LL;
>> +	started = 0;
>> +	while (div) {
>> +		if (temp/div || started) {
>> +			*s++ = (unsigned char)(temp/div + '0');
>> +			started = 1;
>> +		}
>> +		temp -= (temp/div)*div;
>> +		div /= 10;
>> +	}
>> +	*s = 0;
>> +}
>> +
>> +int print_num(unsigned long num)
>> +{
>> +	char num_buf[30];
>> +
>> +	num_to_str(num, num_buf);
>> +	return write(STDOUT_FILENO, num_buf, my_strlen(num_buf));
>> +}
>> +
>> +int print(char *s)
>> +{
>> +	return write(STDOUT_FILENO, s, my_strlen(s));
>> +}
>> +
>> +void main(int argc, char **argv)
>> +{
>> +	int ccode;
>> +	unsigned long used;
>> +	struct sysinfo info;
>> +	unsigned long long temp;
>> +
>> +	print("Testing system size.\n");
>> +	print("1..1\n");
> 
> I ran this test and the reporting could be improved.
> Could you change the above to say
> 
> System RAM in use instead of system size.
I'll have to think about this.  I'm not sure what the
correct phrasing would be for XIP systems (for which part of
the kernel resides in flash, not RAM, maybe "memory"?)

> Also can you also
> print total RAM and other information you already have:
> 
I can print it as part of total output, but for regression
testing and automation I want the tool to have a single
number result as output (well, one number relating to runtime
size, and I'm considering adding one number relating
to compile-time size.)

> Maybe something along the lines of:
> 
> System RAM report (units Kilobytes):
>    Total:
>    Free:
>    Bufferram:
>    In use:
> 
> I would remove the print("1..1\n");

This is part of the TAP output format.

Is there any (other) output format being standardized for selftest
programs?

TAP allows me to output more human-readable information to go
along with the "official" result, but I'd like to keep the
official result in TAP format so it can be handled by automated
tools.  Maybe it would be best to output both.

I'll try to make a counter-proposal tomorrow that we can discus.

>> +
>> +	ccode = sysinfo(&info);
>> +	if (ccode < 0) {
>> +		print("not ok 1 get size runtime size\n");
>> +		print("# could not get sysinfo\n");
>> +		_exit(ccode);
>> +	}
>> +
>> +	/* ignore cache complexities for now */
>> +	temp = info.totalram - info.freeram - info.bufferram;
>> +	temp = temp * info.mem_unit;
>> +	temp = temp / 1024;
>> +
>> +	used = temp;
>> +
>> +	print("ok 1 get runtime size # size = ");
>> +	print_num(used);
>> +	print(" K\n");
> 
> Please see above. You can get rid of print(" K\n"); at the end.
> 
> Sorry for not giving this feedback earlier. The not so clear
> reporting aspect stood out for me after running the test.

No problem.  Thanks for looking at it.
I have some additional changes, based on feedback from Josh Triplett,
so look for a v4 tomorrow.

 -- Tim

^ permalink raw reply

* Re: [PATCH v14 0/3] Add drm driver for Rockchip Socs
From: Dave Airlie @ 2014-11-25 23:12 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: Mark Yao, Boris BREZILLON, Rob Clark, Daniel Vetter, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Randy Dunlap,
	Grant Likely, Greg Kroah-Hartman, John Stultz, Rom Lemarchand,
	devicetree@vger.kernel.org, linux-doc, LKML, dri-devel, linux-api,
	linux-rockchip, Douglas Anderson, Stéphane Marchesin
In-Reply-To: <23617908.6oA5uRuO3l@diego>

On 26 November 2014 at 02:38, Heiko Stübner <heiko@sntech.de> wrote:
> Mark,
>
> Am Donnerstag, 20. November 2014, 09:46:34 schrieb Mark Yao:
>> This a series of patches is a DRM Driver for Rockchip Socs, add support
>> for vop devices. Future patches will add additional encoders/connectors,
>> such as eDP, HDMI.
>>
>> The basic "crtc" for rockchip is a "VOP" - Video Output Processor.
>> the vop devices found on Rockchip rk3288 Soc, rk3288 soc have two similar
>> Vop devices. Vop devices support iommu mapping, we use dma-mapping API with
>> ARM_DMA_USE_IOMMU.
>
> it looks like everybody is more or less happy with this version - in past
> versions responses voicing concerns where quite swift.
>
> As David requested a pull request the last time, it might be time to do so, so
> that we maybe still reach 3.19.
>

I need the iommu changes the drm driver depends on in a stable
maintainer git tree,
I'm not pulling in code I can't build.

I'm happy if the rockchip drm pull stays static and I can pull it in
once the iommu bits
are done.

Dave.

^ permalink raw reply

* Re: [PATCH v6 0/7] vfs: Non-blockling buffered fs read (page cache only)
From: Andrew Morton @ 2014-11-25 23:01 UTC (permalink / raw)
  To: Milosz Tanski
  Cc: linux-kernel, Christoph Hellwig, linux-fsdevel, linux-aio,
	Mel Gorman, Volker Lendecke, Tejun Heo, Jeff Moyer,
	Theodore Ts'o, Al Viro, linux-api, Michael Kerrisk,
	linux-arch
In-Reply-To: <cover.1415636409.git.milosz@adfin.com>

On Mon, 10 Nov 2014 11:40:23 -0500 Milosz Tanski <milosz@adfin.com> wrote:

> This patcheset introduces an ability to perform a non-blocking read from
> regular files in buffered IO mode. This works by only for those filesystems
> that have data in the page cache.
> 
> It does this by introducing new syscalls new syscalls preadv2/pwritev2. These
> new syscalls behave like the network sendmsg, recvmsg syscalls that accept an
> extra flag argument (RWF_NONBLOCK).
> 
> It's a very common patern today (samba, libuv, etc..) use a large threadpool to
> perform buffered IO operations. They submit the work form another thread
> that performs network IO and epoll or other threads that perform CPU work. This
> leads to increased latency for processing, esp. in the case of data that's
> already cached in the page cache.

It would be extremely useful if we could get input from the developers
of "samba, libuv, etc.." about this.  Do they think it will be useful,
will they actually use it, can they identify any shortcomings, etc.

Because it would be terrible if we were to merge this then discover
that major applications either don't use it, or require
userspace-visible changes.

Ideally, someone would whip up pread2() support into those apps and
report on the result.

> With the new interface the applications will now be able to fetch the data in
> their network / cpu bound thread(s) and only defer to a threadpool if it's not
> there. In our own application (VLDB) we've observed a decrease in latency for
> "fast" request by avoiding unnecessary queuing and having to swap out current
> tasks in IO bound work threads.

I haven't read the patches yet, but I'm scratching my head over
pwritev2().  There's much talk and testing results here about
preadv2(), but nothing about how pwritev() works, what its semantics
are, testing results, etc.

> Version 6 highlight:
>  - Compat syscall flag checks, per. Jeff.
>  - Minor stylistic suggestions.
> 
> Version 5 highlight:
>  - XFS support for RWF_NONBLOCK. from Christoph.
>  - RWF_DSYNC flag and support for pwritev2, from Christoph.
>  - Implemented compat syscalls, per. Jeff.
>  - Missing nfs, ceph changes from older patchset.
> 
> Version 4 highlight:
>  - Updated for 3.18-rc1.
>  - Performance data from our application.
>  - First stab at man page with Jeff's help. Patch is in-reply to.

I can't find that manpage.  It is important.  Please include it in the
patch series.

I'm particularly interested in details regarding

- behaviour and userspace return values when data is not found in pagecache

- how it handles partially uptodate pages (blocksize < pagesize). 
  For both reads and writes.  This sort of thing gets intricate so
  let's spell the design out with great specificity.

- behaviour at EOF.

- details regarding handling of file holes.

> RFC Version 3 highlights:
>  - Down to 2 syscalls from 4; can user fp or argument position.
>  - RWF_NONBLOCK value flag is not the same O_NONBLOCK, per Jeff.
> 
> RFC Version 2 highlights:
>  - Put the flags argument into kiocb (less noise), per. Al Viro
>  - O_DIRECT checking early in the process, per. Jeff Moyer
>  - Resolved duplicate (c&p) code in syscall code, per. Jeff
>  - Included perf data in thread cover letter, per. Jeff
>  - Created a new flag (not O_NONBLOCK) for readv2, perf Jeff
> 
> 
> Some perf data generated using fio comparing the posix aio engine to a version
> of the posix AIO engine that attempts to performs "fast" reads before
> submitting the operations to the queue. This workflow is on ext4 partition on
> raid0 (test / build-rig.) Simulating our database access patern workload using
> 16kb read accesses. Our database uses a home-spun posix aio like queue (samba
> does the same thing.)
> 
> f1: ~73% rand read over mostly cached data (zipf med-size dataset)
> f2: ~18% rand read over mostly un-cached data (uniform large-dataset)
> f3: ~9% seq-read over large dataset
> 
> before:
> 
> f1:
>     bw (KB  /s): min=   11, max= 9088, per=0.56%, avg=969.54, stdev=827.99
>     lat (msec) : 50=0.01%, 100=1.06%, 250=5.88%, 500=4.08%, 750=12.48%
>     lat (msec) : 1000=17.27%, 2000=49.86%, >=2000=9.42%
> f2:
>     bw (KB  /s): min=    2, max= 1882, per=0.16%, avg=273.28, stdev=220.26
>     lat (msec) : 250=5.65%, 500=3.31%, 750=15.64%, 1000=24.59%, 2000=46.56%
>     lat (msec) : >=2000=4.33%
> f3:
>     bw (KB  /s): min=    0, max=265568, per=99.95%, avg=174575.10,
>                  stdev=34526.89
>     lat (usec) : 2=0.01%, 4=0.01%, 10=0.02%, 20=0.27%, 50=10.82%
>     lat (usec) : 100=50.34%, 250=5.05%, 500=7.12%, 750=6.60%, 1000=4.55%
>     lat (msec) : 2=8.73%, 4=3.49%, 10=1.83%, 20=0.89%, 50=0.22%
>     lat (msec) : 100=0.05%, 250=0.02%, 500=0.01%
> total:
>    READ: io=102365MB, aggrb=174669KB/s, minb=240KB/s, maxb=173599KB/s,
>          mint=600001msec, maxt=600113msec
> 
> after (with fast read using preadv2 before submit):
> 
> f1:
>     bw (KB  /s): min=    3, max=14897, per=1.28%, avg=2276.69, stdev=2930.39
>     lat (usec) : 2=70.63%, 4=0.01%
>     lat (msec) : 250=0.20%, 500=2.26%, 750=1.18%, 2000=0.22%, >=2000=25.53%
> f2:
>     bw (KB  /s): min=    2, max= 2362, per=0.14%, avg=249.83, stdev=222.00
>     lat (msec) : 250=6.35%, 500=1.78%, 750=9.29%, 1000=20.49%, 2000=52.18%
>     lat (msec) : >=2000=9.99%
> f3:
>     bw (KB  /s): min=    1, max=245448, per=100.00%, avg=177366.50,
>                  stdev=35995.60
>     lat (usec) : 2=64.04%, 4=0.01%, 10=0.01%, 20=0.06%, 50=0.43%
>     lat (usec) : 100=0.20%, 250=1.27%, 500=2.93%, 750=3.93%, 1000=7.35%
>     lat (msec) : 2=14.27%, 4=2.88%, 10=1.54%, 20=0.81%, 50=0.22%
>     lat (msec) : 100=0.05%, 250=0.02%
> total:
>    READ: io=103941MB, aggrb=177339KB/s, minb=213KB/s, maxb=176375KB/s,
>          mint=600020msec, maxt=600178msec
> 
> Interpreting the results you can see total bandwidth stays the same but overall
> request latency is decreased in f1 (random, mostly cached) and f3 (sequential)
> workloads. There is a slight bump in latency for since it's random data that's

s/for/for f2/

> unlikely to be cached but we're always trying "fast read".
> 
> In our application we have starting keeping track of "fast read" hits/misses
> and for files / requests that have a lot hit ratio we don't do "fast reads"
> mostly getting rid of extra latency in the uncached cases. In our real world
> work load we were able to reduce average response time by 20 to 30% (depends
> on amount of IO done by request).
> 
> I've performed other benchmarks and I have no observed any perf regressions in
> any of the normal (old) code paths.
> 
> I have co-developed these changes with Christoph Hellwig.
> 

There have been several incomplete attempts to implement fincore().  If
we were to complete those attempts, preadv2() could be implemented
using fincore()+pread().  Plus we get fincore(), which is useful for
other (but probably similar) reasons.  Probably fincore()+pwrite() could
be used to implement pwritev2(), but I don't know what pwritev2() does
yet.

Implementing fincore() is more flexible, requires less code and is less
likely to have bugs.  So why not go that way?  Yes, it's more CPU
intensive, but how much?  Is the difference sufficient to justify the
preadv2()/pwritev2() approach?

^ permalink raw reply

* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: josh-iaAMLnmF4UmaiuxdJuQwMA @ 2014-11-25 22:08 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Pieter Smith, Alexander Duyck, Alexander Viro, Alexei Starovoitov,
	Andrew Morton, Bertrand Jacquin, Catalina Mocanu, Daniel Borkmann,
	David S. Miller, Eric Dumazet, Eric W. Biederman,
	Fabian Frederick, open list:FUSE: FILESYSTEM...,
	Geert Uytterhoeven, Hugh Dickins, Iulia Manda, Jan Beulich,
	J. Bruce Fields, Jeff Layton, linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, open list
In-Reply-To: <5474ABB6.3030400-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>

[Resending this mail due to some email encoding brokenness that
prevented it from reaching LKML the first time; sorry to anyone who
receives two copies.]

On Tue, Nov 25, 2014 at 08:17:58AM -0800, Randy Dunlap wrote:
> On 11/24/2014 03:00 PM, Pieter Smith wrote:
> >REPO: https://github.com/smipi1/linux-tinification.git
> >
> >BRANCH: tiny/config-syscall-splice
> >
> >BACKGROUND: This patch-set forms part of the Linux Kernel Tinification effort (
> >   https://tiny.wiki.kernel.org/).
> >
> >GOAL: Support compiling out the splice family of syscalls (splice, vmsplice,
> >   tee and sendfile) along with all supporting infrastructure if not needed.
> >   Many embedded systems will not need the splice-family syscalls. Omitting them
> >   saves space.
> 
> Hi,
> 
> Is the splice family of syscalls the only one that tiny has identified
> for optional building or can we expect similar treatment for other
> syscalls?

Pretty much any system call that you could conceive of writing a
userspace without.

There's a partial project list at https://tiny.wiki.kernel.org/projects.

> Why will many embedded systems not need these syscalls?  You know
> exactly what apps they run and you are positive that those apps do
> not use splice?

Yes, precisely.  We're talking about embedded systems small enough that
you're booting with init=/your/app and don't even call fork(), where you
know exactly what code you're putting in and what libraries you use.
And they're almost certainly not running glibc.

> >RESULTS: A tinyconfig bloat-o-meter score for the entire patch-set:
> >
> >add/remove: 0/41 grow/shrink: 5/7 up/down: 23/-8422 (-8399)
> 
> The summary is that this patch saves around 8 KB of code space --
> is that correct?

Right.  For reference, we're talking about kernels where the *total*
size is a few hundred kB.

> How much storage space do embedded systems have nowadays?

For the embedded systems we're targeting for the tinification effort, in
a first pass: 512k-2M of storage (often for an *uncompressed* kernel, to
support execute-in-place), and 128k-512k of memory.  We've successfully
built useful kernels and userspaces for such environments, and we'd like
to go even smaller.

- Josh Triplett
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [tpmdd-devel] [PATCH v7 09/10] tpm: TPM 2.0 FIFO Interface
From: Stefan Berger @ 2014-11-25 21:52 UTC (permalink / raw)
  To: Jarkko Sakkinen, Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Will Arthur,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <1415713513-16524-10-git-send-email-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On 11/11/2014 08:45 AM, Jarkko Sakkinen wrote:
> From: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> Detect TPM 2.0 by using the extended STS (STS3) register. For TPM 2.0,
> instead of calling tpm_get_timeouts(), assign duration and timeout
> values defined in the TPM 2.0 PTP specification.
>
> Signed-off-by: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>   drivers/char/tpm/tpm_tis.c | 71 ++++++++++++++++++++++++++++++++++++----------
>   1 file changed, 56 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 7a2c59b..0b3c089 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -1,5 +1,6 @@
>   /*
>    * Copyright (C) 2005, 2006 IBM Corporation
> + * Copyright (C) 2014 Intel Corporation
>    *
>    * Authors:
>    * Leendert van Doorn <leendert-aZOuKsOsJu3MbYB6QlFGEg@public.gmane.org>
> @@ -44,6 +45,10 @@ enum tis_status {
>   	TPM_STS_DATA_EXPECT = 0x08,
>   };
>
> +enum tis_status3 {
> +	TPM_STS3_TPM2_FAM = 0x04,
> +};
> +
>   enum tis_int_flags {
>   	TPM_GLOBAL_INT_ENABLE = 0x80000000,
>   	TPM_INTF_BURST_COUNT_STATIC = 0x100,
> @@ -70,6 +75,7 @@ enum tis_defaults {
>   #define	TPM_INT_STATUS(l)		(0x0010 | ((l) << 12))
>   #define	TPM_INTF_CAPS(l)		(0x0014 | ((l) << 12))
>   #define	TPM_STS(l)			(0x0018 | ((l) << 12))
> +#define	TPM_STS3(l)			(0x001b | ((l) << 12))
>   #define	TPM_DATA_FIFO(l)		(0x0024 | ((l) << 12))
>
>   #define	TPM_DID_VID(l)			(0x0F00 | ((l) << 12))
> @@ -344,6 +350,7 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
>   {
>   	int rc;
>   	u32 ordinal;
> +	unsigned long dur;
>
>   	rc = tpm_tis_send_data(chip, buf, len);
>   	if (rc < 0)
> @@ -355,9 +362,14 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
>
>   	if (chip->vendor.irq) {
>   		ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> +
> +		if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +			dur = tpm_calc_ordinal_duration(chip, ordinal);
> +		else
> +			dur = tpm_calc_ordinal_duration(chip, ordinal);
> +

Is this right? Shouldn't you call tpm2_calc_ordinal_duration for TPM2? 
If not don't check the flag but leave comment for why the same function 
can be called for both TPM versions.


>   		if (wait_for_tpm_stat
> -		    (chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> -		     tpm_calc_ordinal_duration(chip, ordinal),
> +		    (chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID, dur,
>   		     &chip->vendor.read_queue, false) < 0) {
>   			rc = -ETIME;
>   			goto out_err;
> @@ -543,6 +555,7 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
>   	u32 vendor, intfcaps, intmask;
>   	int rc, i, irq_s, irq_e, probe;
>   	struct tpm_chip *chip;
> +	u8 sts3;
>
>   	chip = tpmm_chip_alloc(dev, &tpm_tis);
>   	if (IS_ERR(chip))
> @@ -554,11 +567,28 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
>   	if (!chip->vendor.iobase)
>   		return -EIO;
>
> +	sts3 = ioread8(chip->vendor.iobase + TPM_STS3(1));
> +	if (sts3 & TPM_STS3_TPM2_FAM)
> +		chip->flags = TPM_CHIP_FLAG_TPM2;
> +
>   	/* Default timeouts */
> -	chip->vendor.timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> -	chip->vendor.timeout_b = msecs_to_jiffies(TIS_LONG_TIMEOUT);
> -	chip->vendor.timeout_c = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> -	chip->vendor.timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> +		chip->vendor.timeout_a = usecs_to_jiffies(TPM2_TIMEOUT_A);
> +		chip->vendor.timeout_b = usecs_to_jiffies(TPM2_TIMEOUT_B);
> +		chip->vendor.timeout_c = usecs_to_jiffies(TPM2_TIMEOUT_C);
> +		chip->vendor.timeout_d = usecs_to_jiffies(TPM2_TIMEOUT_D);
> +		chip->vendor.duration[TPM_SHORT] =
> +			usecs_to_jiffies(TPM2_DURATION_SHORT);
> +		chip->vendor.duration[TPM_MEDIUM] =
> +			usecs_to_jiffies(TPM2_DURATION_MEDIUM);
> +		chip->vendor.duration[TPM_LONG] =
> +			usecs_to_jiffies(TPM2_DURATION_LONG);
> +	} else {
> +		chip->vendor.timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> +		chip->vendor.timeout_b = msecs_to_jiffies(TIS_LONG_TIMEOUT);
> +		chip->vendor.timeout_c = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> +		chip->vendor.timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> +	}
>
>   	if (wait_startup(chip, 0) != 0) {
>   		rc = -ENODEV;
> @@ -573,8 +603,8 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
>   	vendor = ioread32(chip->vendor.iobase + TPM_DID_VID(0));
>   	chip->vendor.manufacturer_id = vendor;
>
> -	dev_info(dev,
> -		 "1.2 TPM (device-id 0x%X, rev-id %d)\n",
> +	dev_info(dev, "%s TPM (device-id 0x%X, rev-id %d)\n",
> +		 (chip->flags & TPM_CHIP_FLAG_TPM2) ? "2.0" : "1.2",
>   		 vendor >> 16, ioread8(chip->vendor.iobase + TPM_RID(0)));
>
>   	if (!itpm) {
> @@ -616,13 +646,17 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
>   		dev_dbg(dev, "\tData Avail Int Support\n");
>
>   	/* get the timeouts before testing for irqs */
> -	if (tpm_get_timeouts(chip)) {
> +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2) && tpm_get_timeouts(chip)) {
>   		dev_err(dev, "Could not get TPM timeouts and durations\n");
>   		rc = -ENODEV;
>   		goto out_err;
>   	}
>
> -	if (tpm_do_selftest(chip)) {
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		rc = tpm2_do_selftest(chip);
> +	else
> +		rc = tpm_do_selftest(chip);
> +	if (rc) {
>   		dev_err(dev, "TPM self test failed\n");
>   		rc = -ENODEV;
>   		goto out_err;
> @@ -683,7 +717,10 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
>   			chip->vendor.probed_irq = 0;
>
>   			/* Generate Interrupts */
> -			tpm_gen_interrupt(chip);
> +			if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +				tpm2_gen_interrupt(chip);
> +			else
> +				tpm_gen_interrupt(chip);
>
>   			chip->vendor.irq = chip->vendor.probed_irq;
>
> @@ -759,14 +796,18 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
>   static int tpm_tis_resume(struct device *dev)
>   {
>   	struct tpm_chip *chip = dev_get_drvdata(dev);
> -	int ret;
> +	int ret = 0;
>
>   	if (chip->vendor.irq)
>   		tpm_tis_reenable_interrupts(chip);
>
> -	ret = tpm_pm_resume(dev);
> -	if (!ret)
> -		tpm_do_selftest(chip);
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		tpm2_do_selftest(chip);
> +	else {
> +		ret = tpm_pm_resume(dev);
> +		if (!ret)
> +			tpm_do_selftest(chip);
> +	}

Only a self test necessary for TPM2 if resuming from ACPI sleep?

Other parts look good to me.
    Stefan

^ permalink raw reply

* Re: [tpmdd-devel] [PATCH v7 04/10] tpm: rename chip->dev to chip->pdev
From: Stefan Berger @ 2014-11-25 21:44 UTC (permalink / raw)
  To: Jarkko Sakkinen, Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <1415713513-16524-5-git-send-email-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On 11/11/2014 08:45 AM, Jarkko Sakkinen wrote:
> Rename chip->dev to chip->pdev to make it explicit that this not the
> character device but actually represents the platform device.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>


> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 69f4003..b3a7c76 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -98,7 +98,7 @@ struct tpm_vendor_specific {
>   #define TPM_PPI_VERSION_LEN		3
>
>   struct tpm_chip {
> -	struct device *dev;	/* Device stuff */
> +	struct device *pdev;	/* Device stuff */
>   	const struct tpm_class_ops *ops;
>
>   	int dev_num;		/* /dev/tpm# */

So this is the core requiring the renamings.  I assume you got them all 
and none were hidden in #if's or so.

Reviewed-by: Stefan Berger <stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

^ permalink raw reply

* Re: [PATCH v4 13/42] virtio_blk: v1.0 support
From: Michael S. Tsirkin @ 2014-11-25 21:43 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Thomas Huth, rusty, linux-api, linux-kernel, virtualization,
	David Hildenbrand, pbonzini, David Miller
In-Reply-To: <20141125185516.7570c6e7.cornelia.huck@de.ibm.com>

On Tue, Nov 25, 2014 at 06:55:16PM +0100, Cornelia Huck wrote:
> On Tue, 25 Nov 2014 18:42:18 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > Based on patch by Cornelia Huck.
> > 
> > Note: for consistency, and to avoid sparse errors,
> >       convert all fields, even those no longer in use
> >       for virtio v1.0.
> > 
> > Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> > Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  include/uapi/linux/virtio_blk.h | 15 ++++-----
> >  drivers/block/virtio_blk.c      | 70 ++++++++++++++++++++++++-----------------
> >  2 files changed, 49 insertions(+), 36 deletions(-)
> > 
> 
> Is this sufficiently unchanged so that the original r-bs still hold?

You are right, I'm afraid I'll have to drop these.
Thomas, David, care to comment?
Even better - have the cycles to review the latest version?

-- 
MST

^ permalink raw reply

* Re: [tpmdd-devel] [PATCH v7 03/10] tpm: fix multiple race conditions in tpm_ppi.c
From: Stefan Berger @ 2014-11-25 21:40 UTC (permalink / raw)
  To: Jarkko Sakkinen, Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: christophe.ricard, josh.triplett, linux-api, linux-kernel,
	tpmdd-devel, jason.gunthorpe, trousers-tech
In-Reply-To: <1415713513-16524-4-git-send-email-jarkko.sakkinen@linux.intel.com>

On 11/11/2014 08:45 AM, Jarkko Sakkinen wrote:
> Traversal of the ACPI device tree was not done right. It should lookup
> PPI only under the ACPI device that it is associated. Otherwise, it could
> match to a wrong PPI interface if there are two TPM devices in the device
> tree.
>
> Removed global ACPI handle and version string from tpm_ppi.c as this
> is racy. Instead they should be associated with the chip.
>
> Additionally, added the missing copyright platter to tpm_ppi.c.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>   drivers/char/tpm/tpm-chip.c |   4 +-
>   drivers/char/tpm/tpm.h      |  16 ++++--
>   drivers/char/tpm/tpm_ppi.c  | 136 +++++++++++++++++++++++++++-----------------
>   drivers/char/tpm/tpm_tis.c  |  15 +++--
>   4 files changed, 108 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index cf3ad24..647867c 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -148,7 +148,7 @@ int tpm_chip_register(struct tpm_chip *chip)
>   	if (rc)
>   		goto del_misc;
>
> -	rc = tpm_add_ppi(&chip->dev->kobj);
> +	rc = tpm_add_ppi(chip);
>   	if (rc)
>   		goto del_sysfs;
>
> @@ -186,7 +186,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
>   	synchronize_rcu();
>
>   	tpm_sysfs_del_device(chip);
> -	tpm_remove_ppi(&chip->dev->kobj);
> +	tpm_remove_ppi(chip);
>
>   	if (chip->bios_dir)
>   		tpm_bios_log_teardown(chip->bios_dir);
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 9880681..69f4003 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -27,6 +27,7 @@
>   #include <linux/platform_device.h>
>   #include <linux/io.h>
>   #include <linux/tpm.h>
> +#include <linux/acpi.h>
>
>   enum tpm_const {
>   	TPM_MINOR = 224,	/* officially assigned */
> @@ -94,6 +95,8 @@ struct tpm_vendor_specific {
>   #define TPM_VID_WINBOND  0x1050
>   #define TPM_VID_STM      0x104A
>
> +#define TPM_PPI_VERSION_LEN		3
> +
>   struct tpm_chip {
>   	struct device *dev;	/* Device stuff */
>   	const struct tpm_class_ops *ops;
> @@ -109,6 +112,11 @@ struct tpm_chip {
>
>   	struct dentry **bios_dir;
>
> +#ifdef CONFIG_ACPI
> +	acpi_handle acpi_dev_handle;
> +	char ppi_version[TPM_PPI_VERSION_LEN + 1];
> +#endif /* CONFIG_ACPI */
> +
>   	struct list_head list;
>   };
>
> @@ -340,15 +348,15 @@ void tpm_sysfs_del_device(struct tpm_chip *chip);
>   int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>
>   #ifdef CONFIG_ACPI
> -extern int tpm_add_ppi(struct kobject *);
> -extern void tpm_remove_ppi(struct kobject *);
> +extern int tpm_add_ppi(struct tpm_chip *chip);
> +extern void tpm_remove_ppi(struct tpm_chip *chip);
>   #else
> -static inline int tpm_add_ppi(struct kobject *parent)
> +static inline int tpm_add_ppi(struct tpm_chip *chip)
>   {
>   	return 0;
>   }
>
> -static inline void tpm_remove_ppi(struct kobject *parent)
> +static inline void tpm_remove_ppi(struct tpm_chip *chip)
>   {
>   }
>   #endif
> diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
> index 61dcc80..6acdb17 100644
> --- a/drivers/char/tpm/tpm_ppi.c
> +++ b/drivers/char/tpm/tpm_ppi.c
> @@ -1,3 +1,22 @@
> +/*
> + * Copyright (C) 2012-2014 Intel Corporation
> + *
> + * Authors:
> + * Xiaoyan Zhang <xiaoyan.zhang@intel.com>
> + * Jiang Liu <jiang.liu@linux.intel.com>
> + * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> + *
> + * Maintained by: <tpmdd-devel@lists.sourceforge.net>
> + *
> + * This file contains implementation of the sysfs interface for PPI.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + */
> +
> +
>   #include <linux/acpi.h>
>   #include "tpm.h"
>
> @@ -22,45 +41,22 @@ static const u8 tpm_ppi_uuid[] = {
>   	0x8D, 0x10, 0x08, 0x9D, 0x16, 0x53
>   };
>
> -static char tpm_ppi_version[PPI_VERSION_LEN + 1];
> -static acpi_handle tpm_ppi_handle;
> -
> -static acpi_status ppi_callback(acpi_handle handle, u32 level, void *context,
> -				void **return_value)
> -{
> -	union acpi_object *obj;
> -
> -	if (!acpi_check_dsm(handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
> -			    1 << TPM_PPI_FN_VERSION))
> -		return AE_OK;
> -
> -	/* Cache version string */
> -	obj = acpi_evaluate_dsm_typed(handle, tpm_ppi_uuid,
> -				      TPM_PPI_REVISION_ID, TPM_PPI_FN_VERSION,
> -				      NULL, ACPI_TYPE_STRING);
> -	if (obj) {
> -		strlcpy(tpm_ppi_version, obj->string.pointer,
> -			PPI_VERSION_LEN + 1);
> -		ACPI_FREE(obj);
> -	}
> -
> -	*return_value = handle;
> -
> -	return AE_CTRL_TERMINATE;
> -}
> -
>   static inline union acpi_object *
> -tpm_eval_dsm(int func, acpi_object_type type, union acpi_object *argv4)
> +tpm_eval_dsm(acpi_handle dev_handle, int func, acpi_object_type type,
> +	     union acpi_object *argv4)
>   {
> -	BUG_ON(!tpm_ppi_handle);
> -	return acpi_evaluate_dsm_typed(tpm_ppi_handle, tpm_ppi_uuid,
> -				       TPM_PPI_REVISION_ID, func, argv4, type);
> +	BUG_ON(!dev_handle);
> +	return acpi_evaluate_dsm_typed(dev_handle, tpm_ppi_uuid,
> +				       TPM_PPI_REVISION_ID,
> +				       func, argv4, type);
>   }
>
>   static ssize_t tpm_show_ppi_version(struct device *dev,
>   				    struct device_attribute *attr, char *buf)
>   {
> -	return scnprintf(buf, PAGE_SIZE, "%s\n", tpm_ppi_version);
> +	struct tpm_chip *chip = dev_get_drvdata(dev);
> +
> +	return scnprintf(buf, PAGE_SIZE, "%s\n", chip->ppi_version);
>   }
>
>   static ssize_t tpm_show_ppi_request(struct device *dev,
> @@ -68,8 +64,10 @@ static ssize_t tpm_show_ppi_request(struct device *dev,
>   {
>   	ssize_t size = -EINVAL;
>   	union acpi_object *obj;
> +	struct tpm_chip *chip = dev_get_drvdata(dev);
>
> -	obj = tpm_eval_dsm(TPM_PPI_FN_GETREQ, ACPI_TYPE_PACKAGE, NULL);
> +	obj = tpm_eval_dsm(chip->acpi_dev_handle, TPM_PPI_FN_GETREQ,
> +			   ACPI_TYPE_PACKAGE, NULL);
>   	if (!obj)
>   		return -ENXIO;
>
> @@ -103,14 +101,15 @@ static ssize_t tpm_store_ppi_request(struct device *dev,
>   	int func = TPM_PPI_FN_SUBREQ;
>   	union acpi_object *obj, tmp;
>   	union acpi_object argv4 = ACPI_INIT_DSM_ARGV4(1, &tmp);
> +	struct tpm_chip *chip = dev_get_drvdata(dev);
>
>   	/*
>   	 * the function to submit TPM operation request to pre-os environment
>   	 * is updated with function index from SUBREQ to SUBREQ2 since PPI
>   	 * version 1.1
>   	 */
> -	if (acpi_check_dsm(tpm_ppi_handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
> -			   1 << TPM_PPI_FN_SUBREQ2))
> +	if (acpi_check_dsm(chip->acpi_dev_handle, tpm_ppi_uuid,
> +			   TPM_PPI_REVISION_ID, 1 << TPM_PPI_FN_SUBREQ2))
>   		func = TPM_PPI_FN_SUBREQ2;
>
>   	/*
> @@ -119,7 +118,7 @@ static ssize_t tpm_store_ppi_request(struct device *dev,
>   	 * string/package type. For PPI version 1.0 and 1.1, use buffer type
>   	 * for compatibility, and use package type since 1.2 according to spec.
>   	 */
> -	if (strcmp(tpm_ppi_version, "1.2") < 0) {
> +	if (strcmp(chip->ppi_version, "1.2") < 0) {
>   		if (sscanf(buf, "%d", &req) != 1)
>   			return -EINVAL;
>   		argv4.type = ACPI_TYPE_BUFFER;
> @@ -131,7 +130,8 @@ static ssize_t tpm_store_ppi_request(struct device *dev,
>   			return -EINVAL;
>   	}
>
> -	obj = tpm_eval_dsm(func, ACPI_TYPE_INTEGER, &argv4);
> +	obj = tpm_eval_dsm(chip->acpi_dev_handle, func, ACPI_TYPE_INTEGER,
> +			   &argv4);
>   	if (!obj) {
>   		return -ENXIO;
>   	} else {
> @@ -157,6 +157,7 @@ static ssize_t tpm_show_ppi_transition_action(struct device *dev,
>   		.buffer.length = 0,
>   		.buffer.pointer = NULL
>   	};
> +	struct tpm_chip *chip = dev_get_drvdata(dev);
>
>   	static char *info[] = {
>   		"None",
> @@ -171,9 +172,10 @@ static ssize_t tpm_show_ppi_transition_action(struct device *dev,
>   	 * (e.g. Capella with PPI 1.0) need integer/string/buffer type, so for
>   	 * compatibility, define params[3].type as buffer, if PPI version < 1.2
>   	 */
> -	if (strcmp(tpm_ppi_version, "1.2") < 0)
> +	if (strcmp(chip->ppi_version, "1.2") < 0)
>   		obj = &tmp;
> -	obj = tpm_eval_dsm(TPM_PPI_FN_GETACT, ACPI_TYPE_INTEGER, obj);
> +	obj = tpm_eval_dsm(chip->acpi_dev_handle, TPM_PPI_FN_GETACT,
> +			   ACPI_TYPE_INTEGER, obj);
>   	if (!obj) {
>   		return -ENXIO;
>   	} else {
> @@ -196,8 +198,10 @@ static ssize_t tpm_show_ppi_response(struct device *dev,
>   	acpi_status status = -EINVAL;
>   	union acpi_object *obj, *ret_obj;
>   	u64 req, res;
> +	struct tpm_chip *chip = dev_get_drvdata(dev);
>
> -	obj = tpm_eval_dsm(TPM_PPI_FN_GETRSP, ACPI_TYPE_PACKAGE, NULL);
> +	obj = tpm_eval_dsm(chip->acpi_dev_handle, TPM_PPI_FN_GETRSP,
> +			   ACPI_TYPE_PACKAGE, NULL);
>   	if (!obj)
>   		return -ENXIO;
>
> @@ -248,7 +252,8 @@ cleanup:
>   	return status;
>   }
>
> -static ssize_t show_ppi_operations(char *buf, u32 start, u32 end)
> +static ssize_t show_ppi_operations(acpi_handle dev_handle, char *buf, u32 start,
> +				   u32 end)
>   {
>   	int i;
>   	u32 ret;
> @@ -264,14 +269,15 @@ static ssize_t show_ppi_operations(char *buf, u32 start, u32 end)
>   		"User not required",
>   	};
>
> -	if (!acpi_check_dsm(tpm_ppi_handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
> +	if (!acpi_check_dsm(dev_handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
>   			    1 << TPM_PPI_FN_GETOPR))
>   		return -EPERM;
>
>   	tmp.integer.type = ACPI_TYPE_INTEGER;
>   	for (i = start; i <= end; i++) {
>   		tmp.integer.value = i;
> -		obj = tpm_eval_dsm(TPM_PPI_FN_GETOPR, ACPI_TYPE_INTEGER, &argv);
> +		obj = tpm_eval_dsm(dev_handle, TPM_PPI_FN_GETOPR,
> +				   ACPI_TYPE_INTEGER, &argv);
>   		if (!obj) {
>   			return -ENOMEM;
>   		} else {
> @@ -291,14 +297,20 @@ static ssize_t tpm_show_ppi_tcg_operations(struct device *dev,
>   					   struct device_attribute *attr,
>   					   char *buf)
>   {
> -	return show_ppi_operations(buf, 0, PPI_TPM_REQ_MAX);
> +	struct tpm_chip *chip = dev_get_drvdata(dev);
> +
> +	return show_ppi_operations(chip->acpi_dev_handle, buf, 0,
> +				   PPI_TPM_REQ_MAX);
>   }
>
>   static ssize_t tpm_show_ppi_vs_operations(struct device *dev,
>   					  struct device_attribute *attr,
>   					  char *buf)
>   {
> -	return show_ppi_operations(buf, PPI_VS_REQ_START, PPI_VS_REQ_END);
> +	struct tpm_chip *chip = dev_get_drvdata(dev);
> +
> +	return show_ppi_operations(chip->acpi_dev_handle, buf, PPI_VS_REQ_START,
> +				   PPI_VS_REQ_END);
>   }
>
>   static DEVICE_ATTR(version, S_IRUGO, tpm_show_ppi_version, NULL);
> @@ -323,16 +335,34 @@ static struct attribute_group ppi_attr_grp = {
>   	.attrs = ppi_attrs
>   };
>
> -int tpm_add_ppi(struct kobject *parent)
> +int tpm_add_ppi(struct tpm_chip *chip)
>   {
> -	/* Cache TPM ACPI handle and version string */
> -	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX,
> -			    ppi_callback, NULL, NULL, &tpm_ppi_handle);
> -	return tpm_ppi_handle ? sysfs_create_group(parent, &ppi_attr_grp) : 0;
> +	union acpi_object *obj;
> +
> +	if (!chip->acpi_dev_handle)
> +		return 0;
> +
> +	if (!acpi_check_dsm(chip->acpi_dev_handle, tpm_ppi_uuid,
> +			    TPM_PPI_REVISION_ID, 1 << TPM_PPI_FN_VERSION))
> +		return 0;
> +
> +	/* Cache PPI version string. */
> +	obj = acpi_evaluate_dsm_typed(chip->acpi_dev_handle, tpm_ppi_uuid,
> +				      TPM_PPI_REVISION_ID, TPM_PPI_FN_VERSION,
> +				      NULL, ACPI_TYPE_STRING);
> +	if (!obj)
> +		return -ENOMEM;
> +
> +	strlcpy(chip->ppi_version, obj->string.pointer,
> +		PPI_VERSION_LEN + 1);

  sizeof(chip->ppi_version) would be better than PPI_VERSION_LEN + 1.

Rest looks ok to me.

     Stefan

^ permalink raw reply

* Re: [PATCH v4 09/42] virtio: set FEATURES_OK
From: Michael S. Tsirkin @ 2014-11-25 21:38 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: rusty, linux-api, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <20141125184811.3191f8db.cornelia.huck@de.ibm.com>

On Tue, Nov 25, 2014 at 06:48:11PM +0100, Cornelia Huck wrote:
> On Tue, 25 Nov 2014 18:42:01 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > set FEATURES_OK as per virtio 1.0 spec
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  include/uapi/linux/virtio_config.h |  2 ++
> >  drivers/virtio/virtio.c            | 29 ++++++++++++++++++++++-------
> >  2 files changed, 24 insertions(+), 7 deletions(-)
> > 
> 
> > +	if (virtio_has_feature(dev, VIRTIO_F_VERSION_1)) {
> > +		add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
> > +		status = dev->config->get_status(dev);
> > +		if (!(status & VIRTIO_CONFIG_S_FEATURES_OK)) {
> > +			printk(KERN_ERR "virtio: device refuses features: %x\n",
> > +			       status);
> 
> Can you use dev_err() to include the information which device failed?

I'm not sure - is dev_err OK to use from probe?

If yes then sure ...

I guess I'll experiment with this.

> > +			err = -ENODEV;
> > +			goto err;
> > +		}
> > +	}

^ permalink raw reply

* Re: [tpmdd-devel] [PATCH v7 01/10] tpm: merge duplicate transmit_cmd() functions
From: Stefan Berger @ 2014-11-25 21:18 UTC (permalink / raw)
  To: Jarkko Sakkinen, Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: christophe.ricard, josh.triplett, linux-api, linux-kernel,
	tpmdd-devel, jason.gunthorpe, trousers-tech
In-Reply-To: <1415713513-16524-2-git-send-email-jarkko.sakkinen@linux.intel.com>

On 11/11/2014 08:45 AM, Jarkko Sakkinen wrote:
> Merged transmit_cmd() functions in tpm-interface.c and tpm-sysfs.c.
> Added "tpm_" prefix for consistency sake. Changed cmd parameter as
> opaque. This enables to use separate command structures for TPM1
> and TPM2 commands in future. Loose coupling works fine here.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>   drivers/char/tpm/tpm-interface.c | 49 +++++++++++++++++++++-------------------
>   drivers/char/tpm/tpm-sysfs.c     | 23 ++-----------------
>   drivers/char/tpm/tpm.h           |  3 ++-
>   3 files changed, 30 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 6af1700..0150b7c 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -398,9 +398,10 @@ out:
>   #define TPM_DIGEST_SIZE 20
>   #define TPM_RET_CODE_IDX 6
>
> -static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
> -			    int len, const char *desc)
> +ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
> +			 int len, const char *desc)
>   {
> +	struct tpm_output_header *header;
>   	int err;
>
>   	len = tpm_transmit(chip, (u8 *) cmd, len);
> @@ -409,7 +410,9 @@ static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
>   	else if (len < TPM_HEADER_SIZE)
>   		return -EFAULT;
>
> -	err = be32_to_cpu(cmd->header.out.return_code);
> +	header = (struct tpm_output_header *) cmd;

The cast should not be necessary -- and this change doesn't buy much...

header = &cmd->header.out;

Should do the trick without cast.


> +
> +	err = be32_to_cpu(header->return_code);
>   	if (err != 0 && desc)
>   		dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
>
> @@ -448,7 +451,7 @@ ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
>   		tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
>   		tpm_cmd.params.getcap_in.subcap = subcap_id;
>   	}
> -	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, desc);
> +	rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, desc);
>   	if (!rc)
>   		*cap = tpm_cmd.params.getcap_out.cap;
>   	return rc;
> @@ -464,8 +467,8 @@ void tpm_gen_interrupt(struct tpm_chip *chip)
>   	tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
>   	tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
>
> -	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> -			"attempting to determine the timeouts");
> +	rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> +			      "attempting to determine the timeouts");
>   }
>   EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
>
> @@ -484,8 +487,8 @@ static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
>   	struct tpm_cmd_t start_cmd;
>   	start_cmd.header.in = tpm_startup_header;
>   	start_cmd.params.startup_in.startup_type = startup_type;
> -	return transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
> -			    "attempting to start the TPM");
> +	return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
> +				"attempting to start the TPM");
>   }
>
>   int tpm_get_timeouts(struct tpm_chip *chip)
> @@ -500,7 +503,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>   	tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
>   	tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
>   	tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
> -	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
> +	rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
>
>   	if (rc == TPM_ERR_INVALID_POSTINIT) {
>   		/* The TPM is not started, we are the first to talk to it.
> @@ -513,7 +516,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>   		tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
>   		tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
>   		tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
> -		rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> +		rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
>   				  NULL);
>   	}
>   	if (rc) {
> @@ -575,8 +578,8 @@ duration:
>   	tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
>   	tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_DURATION;
>
> -	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> -			"attempting to determine the durations");
> +	rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> +			      "attempting to determine the durations");
>   	if (rc)
>   		return rc;
>
> @@ -631,8 +634,8 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
>   	struct tpm_cmd_t cmd;
>
>   	cmd.header.in = continue_selftest_header;
> -	rc = transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
> -			  "continue selftest");
> +	rc = tpm_transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
> +			      "continue selftest");
>   	return rc;
>   }
>
> @@ -672,8 +675,8 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
>
>   	cmd.header.in = pcrread_header;
>   	cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
> -	rc = transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE,
> -			  "attempting to read a pcr value");
> +	rc = tpm_transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE,
> +			      "attempting to read a pcr value");
>
>   	if (rc == 0)
>   		memcpy(res_buf, cmd.params.pcrread_out.pcr_result,
> @@ -737,8 +740,8 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
>   	cmd.header.in = pcrextend_header;
>   	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
>   	memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
> -	rc = transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
> -			  "attempting extend a PCR value");
> +	rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
> +			      "attempting extend a PCR value");
>
>   	tpm_chip_put(chip);
>   	return rc;
> @@ -817,7 +820,7 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
>   	if (chip == NULL)
>   		return -ENODEV;
>
> -	rc = transmit_cmd(chip, cmd, buflen, "attempting tpm_cmd");
> +	rc = tpm_transmit_cmd(chip, cmd, buflen, "attempting tpm_cmd");
>
>   	tpm_chip_put(chip);
>   	return rc;
> @@ -938,14 +941,14 @@ int tpm_pm_suspend(struct device *dev)
>   		cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(tpm_suspend_pcr);
>   		memcpy(cmd.params.pcrextend_in.hash, dummy_hash,
>   		       TPM_DIGEST_SIZE);
> -		rc = transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
> -				  "extending dummy pcr before suspend");
> +		rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
> +				      "extending dummy pcr before suspend");
>   	}
>
>   	/* now do the actual savestate */
>   	for (try = 0; try < TPM_RETRY; try++) {
>   		cmd.header.in = savestate_header;
> -		rc = transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, NULL);
> +		rc = tpm_transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, NULL);
>
>   		/*
>   		 * If the TPM indicates that it is too busy to respond to
> @@ -1022,7 +1025,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
>   		tpm_cmd.header.in = tpm_getrandom_header;
>   		tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
>
> -		err = transmit_cmd(chip, &tpm_cmd,
> +		err = tpm_transmit_cmd(chip, &tpm_cmd,
>   				   TPM_GETRANDOM_RESULT_SIZE + num_bytes,
>   				   "attempting get random");
>   		if (err)
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index 01730a2..8ecb052 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -20,25 +20,6 @@
>   #include <linux/device.h>
>   #include "tpm.h"
>
> -/* XXX for now this helper is duplicated in tpm-interface.c */
> -static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
> -			    int len, const char *desc)
> -{
> -	int err;
> -
> -	len = tpm_transmit(chip, (u8 *) cmd, len);
> -	if (len <  0)
> -		return len;
> -	else if (len < TPM_HEADER_SIZE)
> -		return -EFAULT;
> -
> -	err = be32_to_cpu(cmd->header.out.return_code);
> -	if (err != 0 && desc)
> -		dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
> -
> -	return err;
> -}
> -
>   #define READ_PUBEK_RESULT_SIZE 314
>   #define TPM_ORD_READPUBEK cpu_to_be32(124)
>   static struct tpm_input_header tpm_readpubek_header = {
> @@ -58,8 +39,8 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
>   	struct tpm_chip *chip = dev_get_drvdata(dev);
>
>   	tpm_cmd.header.in = tpm_readpubek_header;
> -	err = transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
> -			   "attempting to read the PUBEK");
> +	err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
> +			       "attempting to read the PUBEK");
>   	if (err)
>   		goto out;
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index e4d0888..e638eb0 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -314,9 +314,10 @@ struct tpm_cmd_t {
>   } __packed;
>
>   ssize_t	tpm_getcap(struct device *, __be32, cap_t *, const char *);
> -
>   ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
>   		     size_t bufsiz);

Delete this prototype ?

> +ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
> +			 const char *desc);
>   extern int tpm_get_timeouts(struct tpm_chip *);
>   extern void tpm_gen_interrupt(struct tpm_chip *);
>   extern int tpm_do_selftest(struct tpm_chip *);


    Stefan

^ permalink raw reply

* Re: [PATCH v3] selftest: size: Add size test for Linux kernel
From: Shuah Khan @ 2014-11-25 20:39 UTC (permalink / raw)
  To: Tim Bird, linux-api-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-embedded-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Shuah Khan
In-Reply-To: <547376D3.7070709-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>

On 11/24/2014 11:20 AM, Tim Bird wrote:
> 
> This test shows the amount of memory used by the system.
> Note that this is dependent on the user-space that is loaded
> when this program runs.  Optimally, this program would be
> run as the init program itself.
> 
> The program is optimized for size itself, to avoid conflating
> its own execution with that of the system software.
> The code is compiled statically, with no stdlibs. On my x86_64 system,
> this results in a statically linked binary of less than 5K.
> 

This following version information shouldn't be part of changelog.

> Changes from v2:
>   - add return values to print routines
>   - add .gitignore file
> 
> Changes from v1:
>   - use more correct Copyright string in get_size.c
> 
> Signed-off-by: Tim Bird <tim.bird-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
> ---

Goes here.

>  tools/testing/selftests/Makefile        |   1 +
>  tools/testing/selftests/size/.gitignore |   1 +
>  tools/testing/selftests/size/Makefile   |  21 +++++++
>  tools/testing/selftests/size/get_size.c | 105 ++++++++++++++++++++++++++++++++
>  4 files changed, 128 insertions(+)
>  create mode 100644 tools/testing/selftests/size/.gitignore
>  create mode 100644 tools/testing/selftests/size/Makefile
>  create mode 100644 tools/testing/selftests/size/get_size.c
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 45f145c..fa91aef 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -15,6 +15,7 @@ TARGETS += user
>  TARGETS += sysctl
>  TARGETS += firmware
>  TARGETS += ftrace
> +TARGETS += size
>  
>  TARGETS_HOTPLUG = cpu-hotplug
>  TARGETS_HOTPLUG += memory-hotplug
> diff --git a/tools/testing/selftests/size/.gitignore b/tools/testing/selftests/size/.gitignore
> new file mode 100644
> index 0000000..189b781
> --- /dev/null
> +++ b/tools/testing/selftests/size/.gitignore
> @@ -0,0 +1 @@
> +get_size
> diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
> new file mode 100644
> index 0000000..51e5fbd
> --- /dev/null
> +++ b/tools/testing/selftests/size/Makefile
> @@ -0,0 +1,21 @@
> +#ifndef CC
> +	CC = $(CROSS_COMPILE)gcc
> +#endif
> +
> +#ifndef STRIP
> +	STRIP = $(CROSS_COMPILE)strip
> +#endif
> +
> +all: get_size
> +
> +get_size: get_size.c
> +	$(CC) --static -ffreestanding -nostartfiles \
> +		-Wl,--entry=main get_size.c -o get_size \
> +		`cc -print-libgcc-file-name`
> +	$(STRIP) -s get_size
> +
> +run_tests: all
> +	./get_size
> +
> +clean:
> +	$(RM) get_size
> diff --git a/tools/testing/selftests/size/get_size.c b/tools/testing/selftests/size/get_size.c
> new file mode 100644
> index 0000000..9c8d3cd
> --- /dev/null
> +++ b/tools/testing/selftests/size/get_size.c
> @@ -0,0 +1,105 @@
> +/*
> + * Copyright 2014 Sony
> + *
> + * Licensed under the terms of the GNU GPL License version 2
> + *
> + * Selftest for runtime system size
> + *
> + * Prints the amount of RAM that the currently running system is using.
> + *
> + * This program tries to be as small as possible itself, to
> + * avoid perturbing the system memory utilization with its
> + * own execution.  It also attempts to have as few dependencies
> + * on kernel features as possible.
> + *
> + * It should be statically linked, with startup libs avoided.
> + * It uses no library calls, and only the following 3 syscalls:
> + *   sysinfo(), write(), and _exit()
> + *
> + * For output, it avoids printf (which in some C libraries
> + * has large external dependencies) by implementing its own
> + * strlen(), number output and print() routines.
> + */
> +
> +#include <sys/sysinfo.h>
> +#include <unistd.h>
> +
> +#define STDOUT_FILENO 1
> +
> +int my_strlen(const char *s)
> +{
> +	int len = 0;
> +
> +	while (*s++)
> +		len++;
> +	return len;
> +}
> +
> +/*
> + * num_to_str - put digits from num into *s, left to right
> + *   do this by dividing the number by powers of 10
> + *   the tricky part is to omit leading zeros
> + *   don't print zeros until we've started printing any numbers at all
> + */
> +void num_to_str(unsigned long num, char *s)
> +{
> +	unsigned long long temp, div;
> +	int started;
> +
> +	temp = num;
> +	div = 1000000000000000000LL;
> +	started = 0;
> +	while (div) {
> +		if (temp/div || started) {
> +			*s++ = (unsigned char)(temp/div + '0');
> +			started = 1;
> +		}
> +		temp -= (temp/div)*div;
> +		div /= 10;
> +	}
> +	*s = 0;
> +}
> +
> +int print_num(unsigned long num)
> +{
> +	char num_buf[30];
> +
> +	num_to_str(num, num_buf);
> +	return write(STDOUT_FILENO, num_buf, my_strlen(num_buf));
> +}
> +
> +int print(char *s)
> +{
> +	return write(STDOUT_FILENO, s, my_strlen(s));
> +}
> +
> +void main(int argc, char **argv)
> +{
> +	int ccode;
> +	unsigned long used;
> +	struct sysinfo info;
> +	unsigned long long temp;
> +
> +	print("Testing system size.\n");
> +	print("1..1\n");

I ran this test and the reporting could be improved.
Could you change the above to say

System RAM in use instead of system size. Also can you also
print total RAM and other information you already have:

Maybe something along the lines of:

System RAM report (units Kilobytes):
   Total:
   Free:
   Bufferram:
   In use:

I would remove the print("1..1\n");

> +
> +	ccode = sysinfo(&info);
> +	if (ccode < 0) {
> +		print("not ok 1 get size runtime size\n");
> +		print("# could not get sysinfo\n");
> +		_exit(ccode);
> +	}
> +
> +	/* ignore cache complexities for now */
> +	temp = info.totalram - info.freeram - info.bufferram;
> +	temp = temp * info.mem_unit;
> +	temp = temp / 1024;
> +
> +	used = temp;
> +
> +	print("ok 1 get runtime size # size = ");
> +	print_num(used);
> +	print(" K\n");

Please see above. You can get rid of print(" K\n"); at the end.

Sorry for not giving this feedback earlier. The not so clear
reporting aspect stood out for me after running the test.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978

^ permalink raw reply

* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: Pieter Smith @ 2014-11-25 20:11 UTC (permalink / raw)
  To: David Miller
  Cc: josh-iaAMLnmF4UmaiuxdJuQwMA, rdunlap-wEGCiKHe2LqWVfeAwA7xHQ,
	alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	beber-2YnHqweIUXrk1uMJSBkQmQ,
	catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, fabf-AgBVmzD5pcezQB+pC5nmwQ,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
	iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
	mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
	mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
	netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
	Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
	therbert-hpIqsD4AKlfQT0dZR+AlfA,
	trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	willemb-hpIqsD4AKlfQT0dZR+AlfA,
	xiaoguangrong-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
In-Reply-To: <20141125.140441.401150380839514113.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

On Tue, Nov 25, 2014 at 02:04:41PM -0500, David Miller wrote:
> From: josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org
> Date: Tue, 25 Nov 2014 10:53:10 -0800
> 
> > It's not a "slippery slope"; it's been our standard practice for ages.
> 
> We've never put an entire class of generic system calls behind
> a config option.

I would have loved to make them optional individually, but they all are
semantic variations of the same thing: Moving data between fd's without that
data passing through userspace. It therefore isn't surprising that these
syscalls share an underlying entanglement of code (which is where the bulk of
the space saving is to be had).

What a tiny product developer should be asking himself, is: "Do I really need
to efficiently move data between file descriptors?". If the answer no, he can
disable CONFIG_SYSCALL_SPLICE to squeeze an extra 8KB out of his kernel.

^ permalink raw reply

* Re: [PATCH v3 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support
From: Greg KH @ 2014-11-25 20:03 UTC (permalink / raw)
  To: Chunyan Zhang
  Cc: grant.likely, robh+dt, catalin.marinas, ijc+devicetree, jslaby,
	galak, broonie, mark.rutland, m-karicheri2, pawel.moll, artagnon,
	rrichter, will.deacon, arnd, gnomes, corbet, jason, broonie,
	heiko, shawn.guo, florian.vaussard, andrew, hytszk, orsonzhai,
	geng.ren, zhizhou.zhang, lanqing.liu, zhang.lyra, wei.qiao,
	devicetree, linux-arm-kernel, linux-kernel, sprdlinux, linux-doc,
	linux-serial, linux-api
In-Reply-To: <1416917818-10506-6-git-send-email-chunyan.zhang@spreadtrum.com>

On Tue, Nov 25, 2014 at 08:16:58PM +0800, Chunyan Zhang wrote:
> Add a full sc9836-uart driver for SC9836 SoC which is based on the
> spreadtrum sharkl64 platform.
> This driver also support earlycon.
> 
> Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
> Signed-off-by: Orson Zhai <orson.zhai@spreadtrum.com>
> Originally-by: Lanqing Liu <lanqing.liu@spreadtrum.com>

Some objections:

> +static struct platform_driver sprd_platform_driver = {
> +	.probe = sprd_probe,
> +	.remove = sprd_remove,
> +	.suspend = sprd_suspend,
> +	.resume = sprd_resume,
> +	.driver = {
> +		   .name = "sprd_serial",
> +		   .owner = THIS_MODULE,

platform drivers don't need .owner in them anymore, it's handled by the
driver core automatically.


> +		   .of_match_table = of_match_ptr(serial_ids),
> +		   },
> +};
> +
> +static int __init sprd_serial_init(void)
> +{
> +	int ret = 0;
> +
> +	ret = uart_register_driver(&sprd_uart_driver);
> +	if (unlikely(ret != 0))
> +		return ret;

NEVER use unlikely unless you can measure the difference without it.
And even then, you better be able to justify it.  For something as dumb
as this type of check, youare working against the complier and cpu which
already knows that 0 is the usual response.

So please remove all usages of likely/unlikely in this patch series.

> +
> +	ret = platform_driver_register(&sprd_platform_driver);
> +	if (unlikely(ret != 0))
> +		uart_unregister_driver(&sprd_uart_driver);
> +
> +	return ret;
> +}
> +
> +static void __exit sprd_serial_exit(void)
> +{
> +	platform_driver_unregister(&sprd_platform_driver);
> +	uart_unregister_driver(&sprd_uart_driver);
> +}
> +
> +module_init(sprd_serial_init);
> +module_exit(sprd_serial_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Spreadtrum SoC serial driver series");
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index 16ad852..d9a8c88 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -247,4 +247,7 @@
>  /* MESON */
>  #define PORT_MESON	109
>  
> +/* SPRD SERIAL  */
> +#define PORT_SPRD   110

Please use a tab character here.

^ permalink raw reply

* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: Eric W. Biederman @ 2014-11-25 20:01 UTC (permalink / raw)
  To: David Miller
  Cc: josh-iaAMLnmF4UmaiuxdJuQwMA, rdunlap-wEGCiKHe2LqWVfeAwA7xHQ,
	pieter-qeJ+1H9vRZbz+pZb47iToQ,
	alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	beber-2YnHqweIUXrk1uMJSBkQmQ,
	catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
	fabf-AgBVmzD5pcezQB+pC5nmwQ,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
	iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
	mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
	mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
	netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
	Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
	therbert-hpIqsD4AKlfQT0dZR+AlfA,
	trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	willemb-hpIqsD4AKlfQT0dZR+AlfA,
	xiaoguangrong-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, zhe
In-Reply-To: <20141125.142741.1620673255148724338.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:

> From: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman)
> Date: Tue, 25 Nov 2014 13:16:44 -0600
>
>> David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:
>> 
>>> From: josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org
>>> Date: Tue, 25 Nov 2014 10:53:10 -0800
>>>
>>>> It's not a "slippery slope"; it's been our standard practice for ages.
>>>
>>> We've never put an entire class of generic system calls behind
>>> a config option.
>> 
>> CONFIG_SYSVIPC has been in the kernel as long as I can remember.
>> 
>> I seem to remember a plan to remove that code once userspace had
>> finished migrating to more unixy interfaces to ipc.  But in 20 years
>> that migration does does not seem to have finished, or even look
>> like it ever will.
>> 
>> But if we started a slippery slope it was long long ago.
>
> Fair enough.
>
> Would be amusing if these tiny systems have it enabled.

It would.

In practice when I was playing in that space I had a hard time
justifying CONFIG_NET and CONFIG_INET.  Despite writing a network
bootloader to use with kexec.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3] sched/fair: Add advisory flag for borrowing a timeslice
From: Davidlohr Bueso @ 2014-11-25 19:40 UTC (permalink / raw)
  To: Khalid Aziz
  Cc: Thomas Gleixner, corbet-T1hC0tSOHrs, mingo-H+wXaHxf7aLQT0dZR+AlfA,
	hpa-YMNOUZJC4hwAvxtiuMwx3w, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	riel-H+wXaHxf7aLQT0dZR+AlfA,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	rientjes-hpIqsD4AKlfQT0dZR+AlfA, ak-VuQAYsv1563Yd54FQh9/CA,
	mgorman-l3A5Bk7waGM, raistlin-k2GhghHVRtY,
	kirill.shutemov-VuQAYsv1563Yd54FQh9/CA,
	atomlin-H+wXaHxf7aLQT0dZR+AlfA, avagin-GEFAQzZX7r8dnm+yROfE0A,
	gorcunov-GEFAQzZX7r8dnm+yROfE0A,
	serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw, athorlton-sJ/iWh9BUns,
	oleg-H+wXaHxf7aLQT0dZR+AlfA, vdavydov-bzQdu9zFT3WakBO8gow8eQ,
	daeseok.youn-Re5JQEeQqe8AvxtiuMwx3w,
	keescook-F7+t8E8rja9g9hUCZPvPmw,
	yangds.fnst-BthXqXjhjHXQFUHtdCDX3A, sbauer-F61uvSdQLzf2fBVCVOL8/A,
	vishnu.ps-Sze3O3UU22JBDgjK7y7TUQ, axboe-b10kYP2dOMg,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5474D71E.8070603-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

On Tue, 2014-11-25 at 12:23 -0700, Khalid Aziz wrote:
> On 11/25/2014 11:27 AM, Davidlohr Bueso wrote:
> > On Tue, 2014-11-25 at 07:45 -0700, Khalid Aziz wrote:
> >> This solution has been used by both database and java on other OSs and
> >> has shown performance improvement. Andrew had asked for performance
> >> numbers on Linux with this patch last time I sent this out and it took
> >> me a while to get performance folks to run a full TPC-C workload. They
> >> did see a 3% improvement in tpcc as I noted in commit log and that is a
> >> significant improvement.
> >
> > 3% for such a change seems pretty worthless... I would have expected
> > this having to impact performance much more.
> >
> 
> Performance impact will depend upon how big a bottleneck the spinlock 
> was creating and how severe the resulting convoy problem was. Database 
> guys try to squeeze every bit out of the system and 3% is considered to 
> be very good gain. 10% gain would have been nicer :)

Right, but my point is that 3% indicates that this isn't really a
problem in the first place. It would be good to know that hw
characteristics as well. We've tackled Oracle related performance issues
in the past with on OLTP benchmarks, with much more noticeable
improvements -- which is why I'm not impressed with your numbers and
particularly for a patch of this nature. That said, I do realize that
Oracle is not the only workload that can potentially gain with userspace
spinlocks.

Thanks,
Davidlohr

^ permalink raw reply

* Re: [PATCH v3] sched/fair: Add advisory flag for borrowing a timeslice
From: Khalid Aziz @ 2014-11-25 19:38 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: Thomas Gleixner, corbet-T1hC0tSOHrs, mingo-H+wXaHxf7aLQT0dZR+AlfA,
	hpa-YMNOUZJC4hwAvxtiuMwx3w, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	riel-H+wXaHxf7aLQT0dZR+AlfA,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	rientjes-hpIqsD4AKlfQT0dZR+AlfA, ak-VuQAYsv1563Yd54FQh9/CA,
	mgorman-l3A5Bk7waGM, liwanp-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	raistlin-k2GhghHVRtY, kirill.shutemov-VuQAYsv1563Yd54FQh9/CA,
	atomlin-H+wXaHxf7aLQT0dZR+AlfA, avagin-GEFAQzZX7r8dnm+yROfE0A,
	gorcunov-GEFAQzZX7r8dnm+yROfE0A,
	serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw, athorlton-sJ/iWh9BUns,
	oleg-H+wXaHxf7aLQT0dZR+AlfA, vdavydov-bzQdu9zFT3WakBO8gow8eQ,
	daeseok.youn-Re5JQEeQqe8AvxtiuMwx3w,
	keescook-F7+t8E8rja9g9hUCZPvPmw,
	yangds.fnst-BthXqXjhjHXQFUHtdCDX3A, sbauer-F61uvSdQLzf2fBVCVOL8/A,
	vishnu.ps-Sze3O3UU22JBDgjK7y7TUQ, axboe-b10kYP2dOMg,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1416937564.3512.15.camel-sZ+7a5bGyC/1wTEvPJ5Q0F6hYfS7NtTn@public.gmane.org>

On 11/25/2014 10:46 AM, Mike Galbraith wrote:
> On Tue, 2014-11-25 at 07:50 -0700, Khalid Aziz wrote:
>
>> It is definitely not an attempt to solve any kind of RT problem.
>
> No no, I'm saying that giving certain tasks special dispensations
> effectively elevates them.  Temporarily or otherwise, they play by
> different rules, will block more deserving tasks, and it's not cut and
> dried that that blocking will not do more harm than good.
>
> Is it a clear win to make say some kworker or other global asset wait
> when it could have preempted and been gone in usecs?  Nope.

I understand. You are right, this allows some apps to gain special 
dispensation. On a general purpose system, I agree this can be 
problematic and it is important that it be easy to disable this. This is 
why I added sysctl tunable and made "disabled" the default state for 
this feature. Allowing temporary elevation of a task as part of the 
overall system design is ok when it is intentional and done after 
considering impact on other tasks running on the system. A large 
database server typically is not a general purpose server that runs any 
arbitrary tasks. These systems are tweaked in many ways to ensure 
optimal performance for database and not necessarily other apps. This 
patch gives admins one more knob to turn when maximizing performance. 
Any general purpose system that sees no use for this feature can leave 
this feature in its default state of disabled. I can see usefulness of 
this patch for other servers used in telecommunication infrastructure 
for instance, where the server is dedicated to specific task(s) and 
needs to update critical database with minimal contention, for example 
switch map on a telco switch controller or channel allocation map on a 
base station controller. I am sure people more familiar with other 
industries can see usefulness in other dedicated applications.

Thanks,
Khalid

^ permalink raw reply

* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: David Miller @ 2014-11-25 19:27 UTC (permalink / raw)
  To: ebiederm-aS9lmoZGLiVWk0Htik3J/w
  Cc: josh-iaAMLnmF4UmaiuxdJuQwMA, rdunlap-wEGCiKHe2LqWVfeAwA7xHQ,
	pieter-qeJ+1H9vRZbz+pZb47iToQ,
	alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	beber-2YnHqweIUXrk1uMJSBkQmQ,
	catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
	fabf-AgBVmzD5pcezQB+pC5nmwQ,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
	iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
	mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
	mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
	netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
	Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
	therbert-hpIqsD4AKlfQT0dZR+AlfA,
	trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	willemb-hpIqsD4AKlfQT0dZR+AlfA,
	xiaoguangrong-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, zhe
In-Reply-To: <87egsr9jkz.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>

From: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman)
Date: Tue, 25 Nov 2014 13:16:44 -0600

> David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:
> 
>> From: josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org
>> Date: Tue, 25 Nov 2014 10:53:10 -0800
>>
>>> It's not a "slippery slope"; it's been our standard practice for ages.
>>
>> We've never put an entire class of generic system calls behind
>> a config option.
> 
> CONFIG_SYSVIPC has been in the kernel as long as I can remember.
> 
> I seem to remember a plan to remove that code once userspace had
> finished migrating to more unixy interfaces to ipc.  But in 20 years
> that migration does does not seem to have finished, or even look
> like it ever will.
> 
> But if we started a slippery slope it was long long ago.

Fair enough.

Would be amusing if these tiny systems have it enabled.

^ permalink raw reply

* Re: [PATCH v3] sched/fair: Add advisory flag for borrowing a timeslice
From: Khalid Aziz @ 2014-11-25 19:23 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: Thomas Gleixner, corbet, mingo, hpa, peterz, riel, akpm, rientjes,
	ak, mgorman, raistlin, kirill.shutemov, atomlin, avagin, gorcunov,
	serge.hallyn, athorlton, oleg, vdavydov, daeseok.youn, keescook,
	yangds.fnst, sbauer, vishnu.ps, axboe, paulmck, linux-kernel,
	linux-doc, linux-api
In-Reply-To: <1416940024.26209.2.camel@linux-t7sj.site>

On 11/25/2014 11:27 AM, Davidlohr Bueso wrote:
> On Tue, 2014-11-25 at 07:45 -0700, Khalid Aziz wrote:
>> This solution has been used by both database and java on other OSs and
>> has shown performance improvement. Andrew had asked for performance
>> numbers on Linux with this patch last time I sent this out and it took
>> me a while to get performance folks to run a full TPC-C workload. They
>> did see a 3% improvement in tpcc as I noted in commit log and that is a
>> significant improvement.
>
> 3% for such a change seems pretty worthless... I would have expected
> this having to impact performance much more.
>

Performance impact will depend upon how big a bottleneck the spinlock 
was creating and how severe the resulting convoy problem was. Database 
guys try to squeeze every bit out of the system and 3% is considered to 
be very good gain. 10% gain would have been nicer :)

--
Khalid

^ permalink raw reply

* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: Eric W. Biederman @ 2014-11-25 19:16 UTC (permalink / raw)
  To: David Miller
  Cc: josh-iaAMLnmF4UmaiuxdJuQwMA, rdunlap-wEGCiKHe2LqWVfeAwA7xHQ,
	pieter-qeJ+1H9vRZbz+pZb47iToQ,
	alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	beber-2YnHqweIUXrk1uMJSBkQmQ,
	catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
	fabf-AgBVmzD5pcezQB+pC5nmwQ,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
	iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
	mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
	mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
	netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
	Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
	therbert-hpIqsD4AKlfQT0dZR+AlfA,
	trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	willemb-hpIqsD4AKlfQT0dZR+AlfA,
	xiaoguangrong-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, zhe
In-Reply-To: <20141125.140441.401150380839514113.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:

> From: josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org
> Date: Tue, 25 Nov 2014 10:53:10 -0800
>
>> It's not a "slippery slope"; it's been our standard practice for ages.
>
> We've never put an entire class of generic system calls behind
> a config option.

CONFIG_SYSVIPC has been in the kernel as long as I can remember.

I seem to remember a plan to remove that code once userspace had
finished migrating to more unixy interfaces to ipc.  But in 20 years
that migration does does not seem to have finished, or even look
like it ever will.

But if we started a slippery slope it was long long ago.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: David Miller @ 2014-11-25 19:05 UTC (permalink / raw)
  To: tytso-3s7WtUTddSA
  Cc: paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	rdunlap-wEGCiKHe2LqWVfeAwA7xHQ, pieter-qeJ+1H9vRZbz+pZb47iToQ,
	josh-iaAMLnmF4UmaiuxdJuQwMA,
	alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	beber-2YnHqweIUXrk1uMJSBkQmQ,
	catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, fabf-AgBVmzD5pcezQB+pC5nmwQ,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
	iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
	mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
	mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
	netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
	Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
	pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
	therbert-hpIqsD4AKlfQT0dZR+AlfA,
	trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	willemb-hpIqsD4AKlfQT0dZR+AlfA, xiaoguangrong@
In-Reply-To: <20141125185806.GA28116-AKGzg7BKzIDYtjvyW6yDsg@public.gmane.org>

From: Theodore Ts'o <tytso-3s7WtUTddSA@public.gmane.org>
Date: Tue, 25 Nov 2014 13:58:06 -0500

> On Tue, Nov 25, 2014 at 01:24:45PM -0500, David Miller wrote:
>> 
>> And then if some fundamental part of userland (glibc, klibc, etc.) finds
>> a useful way to use splice for a fundamental operation, we're back to
>> square one.
> 
> I'll note that the applications for these super-tiny kernels are
> places where it's not likely they would be using glibc at all; think
> very tiny embedded systems.  The userspace tends to be highly
> restricted for the same space reasons why there is an effort to make
> the kernel as small as possible.

This is why I mentioned klibc, in order to avoid replies like your's,
it seems I have failed.

^ permalink raw reply

* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: David Miller @ 2014-11-25 19:04 UTC (permalink / raw)
  To: josh
  Cc: rdunlap, pieter, alexander.h.duyck, viro, ast, akpm, beber,
	catalina.mocanu, dborkman, edumazet, ebiederm, fabf, fuse-devel,
	geert, hughd, iulia.manda21, JBeulich, bfields, jlayton,
	linux-api, linux-fsdevel, linux-kernel, linux-nfs, mcgrof,
	mattst88, mgorman, mst, miklos, netdev, oleg, Paul.Durrant,
	paulmck, pefoley2, tgraf, therbert, trond.myklebust, willemb,
	xiaoguangrong, zhe
In-Reply-To: <20141125185310.GA24891@cloud>

From: josh@joshtriplett.org
Date: Tue, 25 Nov 2014 10:53:10 -0800

> It's not a "slippery slope"; it's been our standard practice for ages.

We've never put an entire class of generic system calls behind
a config option.

^ permalink raw reply

* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: josh-iaAMLnmF4UmaiuxdJuQwMA @ 2014-11-25 19:00 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Michael S. Tsirkin, Trond Myklebust, Bertrand Jacquin,
	Oleg Nesterov, J. Bruce Fields, Eric Dumazet,
	蔡正龙, Jeff Layton, Tom Herbert,
	Alexei Starovoitov, Miklos Szeredi, Peter Foley, Hugh Dickins,
	Xiao Guangrong, Geert Uytterhoeven, Mel Gorman, Matt Turner,
	Paul E. McKenney, Alexander Duyck, Pieter Smith,
	open list:FUSE: FILESYSTEM...
In-Reply-To: <5474ABB6.3030400-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>

On Tue, Nov 25, 2014 at 08:17:58AM -0800, Randy Dunlap wrote:
> On 11/24/2014 03:00 PM, Pieter Smith wrote:
> >REPO: https://github.com/smipi1/linux-tinification.git
> >
> >BRANCH: tiny/config-syscall-splice
> >
> >BACKGROUND: This patch-set forms part of the Linux Kernel Tinification effort (
> >   https://tiny.wiki.kernel.org/).
> >
> >GOAL: Support compiling out the splice family of syscalls (splice, vmsplice,
> >   tee and sendfile) along with all supporting infrastructure if not needed.
> >   Many embedded systems will not need the splice-family syscalls. Omitting them
> >   saves space.
> 
> Hi,
> 
> Is the splice family of syscalls the only one that tiny has identified
> for optional building or can we expect similar treatment for other
> syscalls?

Pretty much any system call that you could conceive of writing a
userspace without.

There's a partial project list at https://tiny.wiki.kernel.org/projects.

> Why will many embedded systems not need these syscalls?  You know
> exactly what apps they run and you are positive that those apps do
> not use splice?

Yes, precisely.  We're talking about embedded systems small enough that
you're booting with init=/your/app and don't even call fork(), where you
know exactly what code you're putting in and what libraries you use.
And they're almost certainly not running glibc.

> >RESULTS: A tinyconfig bloat-o-meter score for the entire patch-set:
> >
> >add/remove: 0/41 grow/shrink: 5/7 up/down: 23/-8422 (-8399)
> 
> The summary is that this patch saves around 8 KB of code space --
> is that correct?

Right.  For reference, we're talking about kernels where the *total*
size is a few hundred kB.

> How much storage space do embedded systems have nowadays?

For the embedded systems we're targeting for the tinification effort, in
a first pass: 512k-2M of storage (often for an *uncompressed* kernel, to
support execute-in-place), and 128k-512k of memory.  We've successfully
built useful kernels and userspaces for such environments, and we'd like
to go even smaller.

- Josh Triplett

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox