linux-kernel - Re: [RFC PATCH 07/33] irqchip/gic-v3-its: Macro-ize its_send_single

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5c8c5655-3c65-da3d-9e4c-fc14098ae32d@redhat.com>
Date:   Thu, 16 Feb 2017 15:59:23 +0100
From:   Auger Eric <eric.auger@...hat.com>
To:     Marc Zyngier <marc.zyngier@....com>, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Christoffer Dall <christoffer.dall@...aro.org>
Subject: Re: [RFC PATCH 07/33] irqchip/gic-v3-its: Macro-ize
 its_send_single_command

Hi,
On 17/01/2017 11:20, Marc Zyngier wrote:
> Most ITS commands do operate on a collection object, and require
> a SYNC command to be performed on that collection in order to
> guarantee the execution of the first command.
> 
> With GICv4 ITS, another set of commands perform similar operations
> on a VPE object, and a VSYNC operations must be executed to guarantee
> their execution.
> 
> Given the similarities (post a command, perform a synchronization
> operation on a sync object), it makes sense to reuse the same
> mechanism for both class of commands.
> 
> Let's start with turning its_send_single_command into a huge macro
> that performs the bulk of the work, and a set of helpers that
> make this macro useeable for the GICv3 ITS commands.
s/useeable/usable

Eric
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@....com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 82 ++++++++++++++++++++++------------------
>  1 file changed, 45 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 99f6130..520b764 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -484,43 +484,51 @@ static void its_wait_for_range_completion(struct its_node *its,
>  	}
>  }
>  
> -static void its_send_single_command(struct its_node *its,
> -				    its_cmd_builder_t builder,
> -				    struct its_cmd_desc *desc)
> -{
> -	struct its_cmd_block *cmd, *sync_cmd, *next_cmd;
> -	struct its_collection *sync_col;
> -	unsigned long flags;
> -
> -	raw_spin_lock_irqsave(&its->lock, flags);
> -
> -	cmd = its_allocate_entry(its);
> -	if (!cmd) {		/* We're soooooo screewed... */
> -		pr_err_ratelimited("ITS can't allocate, dropping command\n");
> -		raw_spin_unlock_irqrestore(&its->lock, flags);
> -		return;
> -	}
> -	sync_col = builder(cmd, desc);
> -	its_flush_cmd(its, cmd);
> -
> -	if (sync_col) {
> -		sync_cmd = its_allocate_entry(its);
> -		if (!sync_cmd) {
> -			pr_err_ratelimited("ITS can't SYNC, skipping\n");
> -			goto post;
> -		}
> -		its_encode_cmd(sync_cmd, GITS_CMD_SYNC);
> -		its_encode_target(sync_cmd, sync_col->target_address);
> -		its_fixup_cmd(sync_cmd);
> -		its_flush_cmd(its, sync_cmd);
> -	}
> -
> -post:
> -	next_cmd = its_post_commands(its);
> -	raw_spin_unlock_irqrestore(&its->lock, flags);
> -
> -	its_wait_for_range_completion(its, cmd, next_cmd);
> -}
> +#define __its_send_single_cmd(name, buildtype, synctype, buildfn)	\
> +void name(struct its_node *its,						\
> +	  buildtype builder,						\
> +	  struct its_cmd_desc *desc)					\
> +{									\
> +	struct its_cmd_block *cmd, *sync_cmd, *next_cmd;		\
> +	synctype *sync_obj;						\
> +	unsigned long flags;						\
> +									\
> +	raw_spin_lock_irqsave(&its->lock, flags);			\
> +									\
> +	cmd = its_allocate_entry(its);					\
> +	if (!cmd) {		/* We're soooooo screewed... */		\
> +		raw_spin_unlock_irqrestore(&its->lock, flags);		\
> +		return;							\
> +	}								\
> +	sync_obj = builder(cmd, desc);					\
> +	its_flush_cmd(its, cmd);					\
> +									\
> +	if (sync_obj) {							\
> +		sync_cmd = its_allocate_entry(its);			\
> +		if (!sync_cmd)						\
> +			goto post;					\
> +									\
> +		buildfn(sync_cmd, sync_obj);				\
> +		its_fixup_cmd(sync_cmd);				\
> +		its_flush_cmd(its, sync_cmd);				\
> +	}								\
> +									\
> +post:									\
> +	next_cmd = its_post_commands(its);				\
> +	raw_spin_unlock_irqrestore(&its->lock, flags);			\
> +									\
> +	its_wait_for_range_completion(its, cmd, next_cmd);		\
> +}
> +
> +static void its_build_sync_cmd(struct its_cmd_block *sync_cmd,
> +			       struct its_collection *sync_col)
> +{
> +	its_encode_cmd(sync_cmd, GITS_CMD_SYNC);
> +	its_encode_target(sync_cmd, sync_col->target_address);
> +}
> +
> +static __its_send_single_cmd(its_send_single_command, its_cmd_builder_t,
> +			     struct its_collection, its_build_sync_cmd)
>  
>  static void its_send_inv(struct its_device *dev, u32 event_id)
>  {
>