Re: [Xen-devel] [RFC 12/16] gic-v2: Write HCR only on change

[Julien Grall <julien.grall@xxxxxxx>]

On 28/11/2018 21:32, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@xxxxxxxx>
>
> This saves one write to peripheral HCR register per hypervisor entry for
> most cases.
>
> Signed-off-by: Andrii Anisov <andrii_anisov@xxxxxxxx>
> ---
>   xen/arch/arm/gic-v2.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 1a744c5..25147bd 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -579,14 +579,17 @@ static void gicv2_write_lr(int lr, const struct gic_lr 
> *lr_reg)
>   
>   static void gicv2_hcr_status(uint32_t flag, bool status)
>   {
> -    uint32_t hcr = readl_gich(GICH_HCR);
> +    uint32_t hcr, ohcr;
> +
> +    ohcr = hcr = readl_gich(GICH_HCR);
>   
>       if ( status )
>           hcr |= flag;
>       else
>           hcr &= (~flag);
>   
> -    writel_gich(hcr, GICH_HCR);
> +    if ( hcr != ohcr )
> +        writel_gich(hcr, GICH_HCR);
While I understand that theoretically this is an issue. Is it actually a real 
performance benefits?
I would actually expect the read to be more expensive than the write because we 
implement writel_gich using writel_relaxed. So there are no barrier afterwards 
to "block" the processor to go forward.
>   }
>   
>   static unsigned int gicv2_read_vmcr_priority(void)
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel