From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::241; helo=mail-it0-x241.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x241.google.com (mail-it0-x241.google.com [IPv6:2607:f8b0:4001:c0b::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B3D38210C5140 for ; Fri, 3 Aug 2018 00:33:07 -0700 (PDT) Received: by mail-it0-x241.google.com with SMTP id g141-v6so7111221ita.4 for ; Fri, 03 Aug 2018 00:33:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=qJoYdlIwfTeeRU6nQbY+HRImEL+ulf9LaaJbbbwbGss=; b=BmIZq5/iBuAaDg6qHcIWpvNIlMW+h3ttqNDwuWiD7fiBxZ5Yr523JpVpWdasorQpDM tCIcMfBn3YWElA5D/2dJ/yJslOLDvIBZqOSk/De2vl4252tL6BNbzUN4RUYw2A5kdXgN xbnafjRWLzPFvxsD8LZ4xHltdZVKfuzwvQvHQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=qJoYdlIwfTeeRU6nQbY+HRImEL+ulf9LaaJbbbwbGss=; b=Ufe0kd5lZ+JrD5zDchBz0tnpIsAfU/QmYBAqMV1z1cIM2FLQrr5PVb2IqDI2rihB4G YJoMKVaBg0CRZXAEV6RXHAdCfz4pKEus246k87fuLh/gpSkKfT8uyQNQm2lC8ivzCUOx 9Wk5EpHBSBptRpsG59Eh4Z/CqPeVSIU266MxaKkKxt+lHx+KFfwVBlLzpJibv5of05r9 yFNyD/K0cR1zA8Zd4CuEki0TCrZb+1eGHKeGXaIyAGP4WHJxWzSX3wARFKvFH2VAC9+f lcVgSECIhP3ZuwClxxhDTCkBkJ59bpXWJU2qBrUe3fY4RpCKT4QgHD71AVeOjdDMe547 x7Aw== X-Gm-Message-State: AOUpUlHte7iHurQJ87KdYZZ96DwwrSzOfTgGQGx0cKOAM0tCbT1JhBEW XG8bq7VB6LT5M8j+tTitb0MwOZ0wtMrmx2grysHKrA== X-Google-Smtp-Source: AAOMgpe/yhwFIeNjFDI+uw/gy6Hg4HV/oTy9fEUtVVHlFnbMKS+GZ46QpcCsoja3Bofpa5QaadmzmtWRDEPLo5w3liw= X-Received: by 2002:a24:148c:: with SMTP id 134-v6mr5371911itg.50.1533281586602; Fri, 03 Aug 2018 00:33:06 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:ac05:0:0:0:0:0 with HTTP; Fri, 3 Aug 2018 00:33:06 -0700 (PDT) In-Reply-To: <1533243054-14251-1-git-send-email-mw@semihalf.com> References: <1533243054-14251-1-git-send-email-mw@semihalf.com> From: Ard Biesheuvel Date: Fri, 3 Aug 2018 09:33:06 +0200 Message-ID: To: Marcin Wojtas Cc: "edk2-devel@lists.01.org" , "Tian, Feng" , "Kinney, Michael D" , "Gao, Liming" , Leif Lindholm , Nadav Haklai , =?UTF-8?B?SmFuIETEhWJyb8Wb?= Subject: Re: [PATCH v2 1/1] ArmPkg/GenericWatchdogDxe: Split 64bit register write to 2x32bit X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Aug 2018 07:33:07 -0000 Content-Type: text/plain; charset="UTF-8" On 2 August 2018 at 22:50, Marcin Wojtas wrote: > According to the SBSA specification the Watchdog Compare > Register is split into two separate 32bit registers. > EDK2 code uses a single 64bit transaction to update > them, which can be problematic, depending on the SoC > implementation and could result in an unpredicted behavior. > > Fix this by modifying WatchdogWriteCompareRegister routine to > use two consecutive 32bit writes to the Watchdog Compare Register > Low and High, using new dedicated macros. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas > Reviewed-by: Leif Lindholm Pushed as dd4cae4d82c7 Thanks > --- > Changelog v1 -> v2: > - use separate macros for WCV register low and high > - improve commit message > - add Leif's RB > > ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h | 3 ++- > ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h > index 9e2aebc..4f42c56 100644 > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h > @@ -20,7 +20,8 @@ > // Control Frame: > #define GENERIC_WDOG_CONTROL_STATUS_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x000) > #define GENERIC_WDOG_OFFSET_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x008) > -#define GENERIC_WDOG_COMPARE_VALUE_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x010) > +#define GENERIC_WDOG_COMPARE_VALUE_REG_LOW ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x010) > +#define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x014) > > // Values of bit 0 of the Control/Status Register > #define GENERIC_WDOG_ENABLED 1 > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > index 3180f01..8ccf153 100644 > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > @@ -56,7 +56,8 @@ WatchdogWriteCompareRegister ( > UINT64 Value > ) > { > - MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value); > + MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_LOW, Value & MAX_UINT32); > + MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_HIGH, (Value >> 32) & MAX_UINT32); > } > > VOID > -- > 2.7.4 >