From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 170D73B9949 for ; Tue, 21 Jul 2026 08:40:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784623255; cv=none; b=U+kQ8gqDL/+U/4DJCx1sY80otsyiLCQHRHITOrJi/RKMYybTacvDFzwCtbOe8Y6lgObHGdPY8M5RGPltkMNbeHisIwWex/2D0Mo/KY3lNP1SFuZEaBGVlVQE9a5Uleov5aTAYMxLymlCo7ZO2j1KoAbdDTbOx0qBP1EUEmaMKro= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784623255; c=relaxed/simple; bh=U2/ZgVzfkBXw9PNW1p1aK6bNd/TBhsvKFpUPyLgI8VM=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=IPQZYl1OrHpv/dulDMagjtEIoWdKabArPikZgrGL6hlo+NSu3TlNFfAfwXPm7Y4yw+O7UcIVo511c8+Z+MexCBGL9bpLK95OWAxEzpxw4hQU8VCSXv/OxnHfhVBFZO0vqCxf7oYGKjhbjelTPw9kvKdMECEIUGWsqADhwUWWfys= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=BPrHIgOD; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="BPrHIgOD" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C28351595; Tue, 21 Jul 2026 01:40:47 -0700 (PDT) Received: from [10.2.212.8] (e134344.arm.com [10.2.212.8]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CFE233F66F; Tue, 21 Jul 2026 01:40:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1784623251; bh=U2/ZgVzfkBXw9PNW1p1aK6bNd/TBhsvKFpUPyLgI8VM=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=BPrHIgODqI7HpDaLwP++61j00cpszVF9l1evspNyEMAT3YKnkaBpJd5EMYo3d9DmP DTack0Hp8lrQciFotK5c0j3wbgJxFgggHL7S9/bnMBU3y36zRzfgd90ml5xzLS5I/p JKxB2euCWHHTQfITyQAxlAG2Mm/MvMeNsVNeW5oc= Message-ID: Date: Tue, 21 Jul 2026 09:40:49 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Thunderbird Daily Subject: Re: [PATCH v1 07/11] arm_mpam: Initialize all of struct mon_read in mpam_restore_mbwu_state() To: Lee Trager Cc: james.morse@arm.com, reinette.chatre@intel.com, fenghuay@nvidia.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, dave.martin@arm.com, andre.przywara@arm.com References: <20260710115546.29644-1-ben.horgan@arm.com> <20260710115546.29644-8-ben.horgan@arm.com> <0e117330-36ed-4917-8a63-384d1e4f77fc@trager.us> Content-Language: en-US From: Ben Horgan In-Reply-To: <0e117330-36ed-4917-8a63-384d1e4f77fc@trager.us> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hi Lee, On 7/21/26 00:05, Lee Trager wrote: > On 7/10/26 4:55 AM, Ben Horgan wrote: > >> m->err may be read before initialization in __ris_msmon_read() when called >> from mpam_restore_mbwu_state(). >> >> Initialize the whole struct mon_read in mpam_restore_mbwu_state() and fix >> the spelling of mbwu in the name. >> >> Fixes: 41e8a14950e1 ("arm_mpam: Track bandwidth counter state for power management") >> Signed-off-by: Ben Horgan >> --- >>   drivers/resctrl/mpam_devices.c | 13 +++++++------ >>   1 file changed, 7 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c >> index a49f426aefc0..c9adc450f087 100644 >> --- a/drivers/resctrl/mpam_devices.c >> +++ b/drivers/resctrl/mpam_devices.c >> @@ -1640,7 +1640,6 @@ static int mpam_restore_mbwu_state(void *_ris) >>   { >>       int i; >>       u64 val; > val is still uninitialized. Its passed to to __ris_mon_read() below which does *m->val += now; Ok, I don't think this causes any actual problems as an unitialized automatic variable has an indeterminate value, we're just adding to it rather than making any decisions or persisting the value. As it's unsigned the addition is defined even if it wraps around. Having said that, it's clearer just to initialize it. I'll set it to 0. Thanks, Ben >> -    struct mon_read mwbu_arg; >>       struct mpam_msc_ris *ris = _ris; >>       struct msmon_mbwu_state *mbwu_state; >>       struct mpam_msc *msc = ris->vmsc->msc; >> @@ -1653,16 +1652,18 @@ static int mpam_restore_mbwu_state(void *_ris) >>               return -EIO; >>             if (ris->mbwu_state[i].enabled) { >> -            mwbu_arg.ris = ris; >> -            mwbu_arg.ctx = &ris->mbwu_state[i].cfg; >> -            mwbu_arg.type = mpam_msmon_choose_counter(class); >> -            mwbu_arg.val = &val; >> +            struct mon_read mbwu_arg = { >> +                .ris = ris, >> +                .ctx = &ris->mbwu_state[i].cfg, >> +                .type = mpam_msmon_choose_counter(class), >> +                .val = &val >> +            }; >>                 mbwu_state->reset_on_next_read = true; >>                 mpam_mon_sel_unlock(msc); >>   -            __ris_msmon_read(&mwbu_arg); >> +            __ris_msmon_read(&mbwu_arg); >>           } else { >>               mpam_mon_sel_unlock(msc); >>           }