Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 9 Mar 2022 22:25:22 +0000
From:   Yazen Ghannam <yazen.ghannam@amd.com>
To:     Borislav Petkov <bp@alien8.de>
Cc:     linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
        mchehab@kernel.org, tony.luck@intel.com, james.morse@arm.com,
        rric@kernel.org, Smita.KoralahalliChannabasappa@amd.com
Subject: Re: [PATCH v4 16/24] EDAC/amd64: Define function to make space for
 CS ID
Message-ID: <YikpUrojo1cMFfOp@yaz-ubuntu>
References: <20220127204115.384161-1-yazen.ghannam@amd.com>
 <20220127204115.384161-17-yazen.ghannam@amd.com>
 <YgpQLnTd4TyiehS/@zn.tnic>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YgpQLnTd4TyiehS/@zn.tnic>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?aNa8/fruufm2Ye4W4lF7nmXjPgyTyhM3Fy13FAWxCWIYah0glq31YjrsjeMI?=
 =?us-ascii?Q?ee7vz9RYW1Oa5Cr7PfTmOffQy4jVSsr/vBaFy4LZxq4+wM/IjgHt4gARVL7f?=
 =?us-ascii?Q?X9G6quXV7YuWwjiWATiu02vzepGZkuRUqNA+91EcLnMIVKbCkI80fp36ICSD?=
 =?us-ascii?Q?e1ug4JKGA3+Bp07FcBKQu4L50fs/NhmDx7i/DEkInHLObmzTAKihtpYnEHJH?=
 =?us-ascii?Q?EbqbBtl2s2SJakJMdAMpNcaJJP010yyFUWJnStBb1EkX6qub9vcnLXPUC/ct?=
 =?us-ascii?Q?+PAWAXl/o8cmZcGdIcUbfFBG1ZJhDK49tLKXwS2OgItHmAV0E9z1ilh7Lld+?=
 =?us-ascii?Q?e7zp4T4BCfTVfplXfwHmnA2h40zLA1jQcrc8IyPgtGwnJTfwm/GAoDUbAhLp?=
 =?us-ascii?Q?hvfRGMjFQ2PTdO1ojiRYVFiDTeRvRCsUdgMt/imU483rbh9pTEEafQGTUHwt?=
 =?us-ascii?Q?j4JenG4pUgLj48lv39KtqOUkh+kM796xh0aOodUCmyvHfHwagS/mtkkvfzFI?=
 =?us-ascii?Q?+ThD1FK5bnvXP+HMGdHyEItjVJTklW5AXpitENJ6BnN7vtuCeCGYtOp/1PD0?=
 =?us-ascii?Q?wy/qWIqXS8Q1irjEPB3hFdm866qj/xcc7gyJV7YfoFtpwrUw/bVg2IClmvOn?=
 =?us-ascii?Q?tXYgGaMGRp3fa7mYf/33CAvKeUPTR3aiZbtGyJQJDr/8y28DfG4j7HdFe/xd?=
 =?us-ascii?Q?wW0y8gUOL/iFeBpf0goxL9VbyNFqlvgaXVA2W4BTwiURDH9hgs7uEJLeJoNE?=
 =?us-ascii?Q?nQwPS3IvOflOEx/jpx8nj+InBzkJ66MP+k0ZeVzrA0uw2CDAPy8D/JwWiX2T?=
 =?us-ascii?Q?3XUIS2jqC2hH7aJVl5xCsk/V45sye/AuPTxWoaO2EYIX4pCrBXNG6HJK0aL/?=
 =?us-ascii?Q?VCUFYBhxFsresYIBxy4MNqWSAnLi+YThqcA1xUZ/VZtXsOY5+FFFbXrpPSAb?=
 =?us-ascii?Q?OLpFjV6/mvfRDZSEcbIOjTPUEWzkzoP7IEywyRv2m47MbnC7eCTFLEeqRjjx?=
 =?us-ascii?Q?+w8TfLAMBZwnsc1hyAsCrkb5dSeyGQr1SMbYKqpw/6mc29hw/gjiKfE9jVMT?=
 =?us-ascii?Q?au9wDA55k2qPjd4a1Z5ct5HubY9jXGzKS0/VB3Kpp3C5DHGdbhR1XPMNlOjr?=
 =?us-ascii?Q?Pc+o8KcddcRYfOeuXbY3micu0z04lktkh/vGqT5sVwAs0/SJq047R55YmgxJ?=
 =?us-ascii?Q?N+6mz5DYjT3DvXjW+v7IGncfNQfc1haybB0ez9+J9EW4xHjVhHwqsJZPGlfO?=
 =?us-ascii?Q?E6ZeUrPUFRmR+oBw6nDZs1Iuj6dkPkdNEqDtAnyX/yH0glz6X9+7cyhZOkOM?=
 =?us-ascii?Q?a8LiLsRrHp9FIizK3p/v5vjoypg1fbU3Ft6kcEI2oCQWpnL5fFqMJVBw/J/t?=
 =?us-ascii?Q?GxZr5sFkVW3b63/sAjQbizixZ4MdecwSK6rXGt4TEfRjKUcFHi33U861dRTo?=
 =?us-ascii?Q?C5dDa3jNMbrKxCxL8R+oZou3+oYgyz8d23/7BxIzxgubrQaC7r/8LPnSF89v?=
 =?us-ascii?Q?ZHVccCv5NhuQK5iNxhZ9LgQ4l3egxT3CgyCui7q7W1IrrUTgHfktB54aXF/c?=
 =?us-ascii?Q?KOqIRn9INagyPvYgEpkjGboQJF0VCq4YqC8LoCwTSKhws5QyNYzkTFpFEVBy?=
 =?us-ascii?Q?ouujZb0CxVVgIRb76VStylQ=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9ad0964e-51df-4a60-f67d-08da021bb833
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Mar 2022 22:25:30.6163
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: otMbSIIDlyxAXy7Tgr/EHgDML4CmAswbQ8xlvv0l209lfiMwkQQNmdB6eb1A/1mZVsYlVmReQKX6JfR4AbI4cQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR1201MB0198
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Feb 14, 2022 at 01:50:54PM +0100, Borislav Petkov wrote:
> On Thu, Jan 27, 2022 at 08:41:07PM +0000, Yazen Ghannam wrote:
> > +static void expand_bits(u8 start_bit, u8 num_bits, u64 *value)
> > +{
> > +	u64 temp1, temp2;
> > +
> > +	if (start_bit == 0) {
> 
> As always
> 
> 	if (!<variable, etc>)
> 
> for 0/NULL tests.
>

Will fix.
 
> > +		*value <<= num_bits;
> > +		return;
> > +	}
> > +
> > +	temp1 = *value & GENMASK_ULL(start_bit - 1, 0);
> > +	temp2 = (*value & GENMASK_ULL(63, start_bit)) << num_bits;
> > +	*value = temp1 | temp2;
> > +}
> > +
> > +static void make_space_for_cs_id_simple(struct addr_ctx *ctx)
> > +{
> > +	u8 num_intlv_bits = ctx->intlv_num_chan;
> > +
> > +	num_intlv_bits += ctx->intlv_num_dies;
> > +	num_intlv_bits += ctx->intlv_num_sockets;
> > +	expand_bits(ctx->intlv_addr_bit, num_intlv_bits, &ctx->ret_addr);
> > +}
> 
> void functions but they return values through their pointer arguments?
> I'm sure you can design those better.
>

I guess this could be "addr = func(x, y, addr)", but why not just operate on
the value directly? There isn't an error condition here that's obvious to me.

Thanks,
Yazen