From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by mx.groups.io with SMTP id smtpd.web08.34571.1613998333908761550 for ; Mon, 22 Feb 2021 04:52:14 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=w5Rci3mR; spf=pass (domain: nuviainc.com, ip: 209.85.128.49, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f49.google.com with SMTP id p3so3294696wmc.2 for ; Mon, 22 Feb 2021 04:52:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=vg6eDP0BjCpqqYHht8oQ4LY67JDg4uHMCHGk9t03Hg0=; b=w5Rci3mRUi+fyL9+qw/QpOBhve24/QRzftZsGwPxUQ7osfrtiYr2QaXfdTGbJkC5SP AUJ7xYK5wyp9j2hL+i9UgXZ7Th8EZ5V2AVTYvSGvqdbQwxid2omKIelEH4G8tvw7wYc4 kX8vu9+G8nsGXtRJNG6C5EGKeO4h5tfBCujFE2qxBDC3/MuI0k9ydduVaNO3++Di+A7L paJRgdT/4K5gVlsUDrz5h/jFQ1GVna1ATSxkv35RmNCLQJCoyTYzAPIvcKw8tmSphF42 5BXmigmSvDsesFGEDMECQCAQaqly7fcHrlxD3nEuI13L8SJ2IxS492BejLy+/0OGSfdJ hcRQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=vg6eDP0BjCpqqYHht8oQ4LY67JDg4uHMCHGk9t03Hg0=; b=UbYIjQHLGW/M1lOflYzC2YwTUlFgWkOzPdb7TuE0t/dDFWSjWdQ8Mg7KIqdXg0YWkd Lvqw5j9uHs3HbFSsy/aFOP995/+5s5iaAhs9YYf5Fn0XYJIP/nGNr4LLNDmnun+PPiGb rahJK7PE2zkqpHSHcEKnRVM4bRumLfgcl5AnfQJE7sShQx7TDeX99jp9Xcatfoe8onsh c3uYIct9RH6mMCgxn5KILmcZexW4cigM3ItRBTDci8NuJV7PnuLL5+FndkBDUiobKvUp Yq8lAjf39QYN/MTXqL7FTjCd06e2PvW2SY+YCXm2BX/S28+bEG5gyvV+gAoqA/l9vT7d 8qtg== X-Gm-Message-State: AOAM532mqFfxE4D40ZfOoT5Ag3JBZUKhHx/YG++sRyjZ0LAXOsUzJ6T3 WMcDvwMz4xQY0CK2L+Xwv0eKuw== X-Google-Smtp-Source: ABdhPJzgKmsirIgVByM1xKr1HRdvQq2cBonOLcC7/0D5DVQhhZTUw8fswMuzlAfUJaAO3S4ig5j8Cw== X-Received: by 2002:a7b:c341:: with SMTP id l1mr19661272wmj.182.1613998332435; Mon, 22 Feb 2021 04:52:12 -0800 (PST) Return-Path: Received: from vanye (cpc1-cmbg19-2-0-cust915.5-4.cable.virginm.net. [82.27.183.148]) by smtp.gmail.com with ESMTPSA id q18sm17697668wrw.91.2021.02.22.04.52.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 22 Feb 2021 04:52:12 -0800 (PST) Date: Mon, 22 Feb 2021 12:52:10 +0000 From: "Leif Lindholm" To: Ming Huang Cc: devel@edk2.groups.io, ardb+tianocore@kernel.org, guoheyi@linux.alibaba.com Subject: Re: [PATCH edk2 v1 1/1] ArmPkg/ArmGicLib: Fix GICR_IPRIORITYR address wrong issue Message-ID: <20210222125210.GS1664@vanye> References: <20210220070839.29988-1-huangming@linux.alibaba.com> MIME-Version: 1.0 In-Reply-To: <20210220070839.29988-1-huangming@linux.alibaba.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Ming, On Sat, Feb 20, 2021 at 15:08:39 +0800, Ming Huang wrote: > The address of GICR_IPRIORITYR is in SGI_base frame. ARM_GICR_CTLR_FRAME_SIZE > should add to GicCpuRedistributorBase for GICR_IPRIORITYR. Otherwise RAS > error(Uncorrected software error) will reported in ArmGicDxe. > > Signed-off-by: Ming Huang > --- > ArmPkg/Drivers/ArmGic/ArmGicLib.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c > index 8ef32b33a1..7a54972455 100644 > --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c > +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c > @@ -235,6 +235,9 @@ ArmGicSetInterruptPriority ( > return; > } > > + // The address of GICR_IPRIORITYR is in SGI_base frame. > + // ARM_GICR_CTLR_FRAME_SIZE should add to GicCpuRedistributorBase for GICR_IPRIORITYR. > + GicCpuRedistributorBase += ARM_GICR_CTLR_FRAME_SIZE; I agree with the error report, but not the fix. Changing the value of a variable called GicCpuRedistributorBase to something that is not the base address of the redistributor makes the code confusing. If you look at the subsequent function, ArmGicEnableInterrupt, it resolvess the same situation using the ISENABLER_ADDRESS macro defined at the top of the file: #define ISENABLER_ADDRESS(base,offset) ((base) + \ ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_ISENABLER + (4 * offset)) I would suggest creating an IPRIORITY_ADDRESS macro by the same pattern and using that. Would that solution be OK with you? Best Regards, Leif > MmioAndThenOr32 ( > GicCpuRedistributorBase + ARM_GIC_ICDIPR + (4 * RegOffset), > ~(0xff << RegShift), > -- > 2.17.1 >