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 64B6914E2DE for ; Thu, 11 Apr 2024 14:17:07 +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=1712845028; cv=none; b=VkZQZyemuehClQXX4k4uTa6ry7Cq6eKV0ZZlzEoQKZ+/T5sQpfoATB9tVMhErYjbC+/EMcTHTtLiHxHxDPrqC59FbyqNxTcwjeb9vH95B7EbPRnL7HxzIr3jhNhvxnkwRkDpkSm/VM6RtFErd4q/gp3c81WVWRfEu1B0dsZY1jo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712845028; c=relaxed/simple; bh=hFpPbCUdGEDxZtHZpIbjnwRnJOUcczaR3P0MhRNYSRY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Rg9nNH42zzAumwvOxtUc13GtngiT4aKemCGnFd7FpKcb0T4vMasJMTLn8KsKkL9BhCEO7AmjHQ2jXE3OVDeoEHqq9NARxI1bTnTetJG+GOr1qAGtTb9S8G4etYwaHC9ePySEW3goJp3qAuxMJ5/Y/SQdS0nlqHCyka8SGdaUiLg= 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; 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 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 443A4339; Thu, 11 Apr 2024 07:17:36 -0700 (PDT) Received: from e133380.arm.com (e133380.arm.com [10.1.197.52]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id F1C953F64C; Thu, 11 Apr 2024 07:17:03 -0700 (PDT) Date: Thu, 11 Apr 2024 15:17:01 +0100 From: Dave Martin To: Reinette Chatre Cc: James Morse , x86@kernel.org, linux-kernel@vger.kernel.org, Fenghua Yu , Thomas Gleixner , Ingo Molnar , Borislav Petkov , H Peter Anvin , Babu Moger , shameerali.kolothum.thodi@huawei.com, D Scott Phillips OS , carl@os.amperecomputing.com, lcherian@marvell.com, bobo.shaobowang@huawei.com, tan.shaopeng@fujitsu.com, baolin.wang@linux.alibaba.com, Jamie Iles , Xin Hao , peternewman@google.com, dfustini@baylibre.com, amitsinght@marvell.com, David Hildenbrand , Rex Nie Subject: Re: [PATCH v1 18/31] x86/resctrl: Allow resctrl_arch_mon_event_config_write() to return an error Message-ID: References: <20240321165106.31602-1-james.morse@arm.com> <20240321165106.31602-19-james.morse@arm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Mon, Apr 08, 2024 at 08:23:36PM -0700, Reinette Chatre wrote: > Hi James, > > On 3/21/2024 9:50 AM, James Morse wrote: > > resctrl_arch_mon_event_config_write() writes a bitmap of events provided > > by user-space into the configuration register for the monitors. > > > > This assumes that all architectures support all the features each bit > > corresponds to. > > > > MPAM can filter monitors based on read, write, or both, but there are > > many more options in the existing bitmap. To allow this interface to > > work for machines with MPAM, allow the architecture helper to return > > an error if an incompatible bitmap is set. > > > > When valid values are provided, there is no change in behaviour. If > > an invalid value is provided, currently it is silently ignored, but > > last_cmd_status is updated. After this change, the parser will stop > > at the first invalid value and return an error to user-space. This > > matches the way changes to the schemata file are made. > > > > Is this needed? With move of mbm_cfg_mask to rdt_resource I expect > MPAM would use it to set what the valid values are. With that done, > when user space provides a value, mon_config_write() compares user > provided value against mbm_cfg_mask and will already return early > (before attempting to write to hardware) with error > if value is not supported. This seems to accomplish the goal of this > patch? This sounds plausible. In a recent snapshot of James' MPAM code, it looks like we could be initialising rdt_resource::mbm_cfg_mask when setting up the rdt_resource struct for resctrl, though in fact this information is captured differently right now. I'm sure why (though James may have a reason). [1] I don't see an obvious reason though why we couldn't set mbm_cfg_mask and detect bad config values globally in mon_config_write(), the same as for the existing AMD BMEC case. Nothing in the MPAM architecture stops hardware vendors from randomly implementing different capabilities in different components of the system, but provided that we only expose the globally supported subset of event filtering capabilities to resctrl this approach looks workable. This consistent with the James' MPAM code deals with other feature mismatches across the system today. [1] https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/tree/drivers/platform/mpam/mpam_resctrl.c?h=mpam/snapshot/v6.7-rc2#n730 > > > Signed-off-by: James Morse > > --- > > .. > > diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h > > index 8a7367d1ce45..6705d7960dfd 100644 > > --- a/include/linux/resctrl.h > > +++ b/include/linux/resctrl.h > > @@ -200,6 +200,7 @@ struct resctrl_mon_config_info { > > struct rdt_domain *d; > > u32 evtid; > > u32 mon_config; > > + int err; > > }; > > Please take care to use consistent spacing. > > Reinette Noted FWIW (though I guess this code might change or go away). Cheers ---Dave