Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Tue, 17 May 2022 09:33:28 -0700
From:   Fenghua Yu <fenghua.yu@intel.com>
To:     Stephane Eranian <eranian@google.com>
Cc:     linux-kernel@vger.kernel.org, reinette.chatre@intel.com,
        babu.moger@amd.com, x86@kernel.org
Subject: Re: [PATCH v2] x86/resctrl: Fix zero cbm for AMD in cbm_validate
Message-ID: <YoPOWC0waMuSlvI6@fyu1.sc.intel.com>
References: <20220517001234.3137157-1-eranian@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220517001234.3137157-1-eranian@google.com>
Precedence: bulk

Hi, Eranian,

On Mon, May 16, 2022 at 05:12:34PM -0700, Stephane Eranian wrote:
> AMD supports cbm with no bits set as reflected in rdt_init_res_defs_amd() by:
...
> @@ -107,6 +107,10 @@ static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
>  	first_bit = find_first_bit(&val, cbm_len);
>  	zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);
>  
> +	/* no need to check bits if arch supports no bits set */
> +	if (r->cache.arch_has_empty_bitmaps && val == 0)
> +		goto done;
> +
>  	/* Are non-contiguous bitmaps allowed? */
>  	if (!r->cache.arch_has_sparse_bitmaps &&
>  	    (find_next_bit(&val, cbm_len, zero_bit) < cbm_len)) {
> @@ -119,7 +123,7 @@ static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
>  				    r->cache.min_cbm_bits);
>  		return false;
>  	}
> -
> +done:
>  	*data = val;
>  	return true;
>  }

Isn't it AMD supports 0 minimal CBM bits? Then should set its min_cbm_bits as 0.
Is the following patch a better fix? I don't have AMD machine and cannot
test the patch.

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 6055d05af4cc..031d77dd982d 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -909,6 +909,7 @@ static __init void rdt_init_res_defs_amd(void)
 			r->cache.arch_has_sparse_bitmaps = true;
 			r->cache.arch_has_empty_bitmaps = true;
 			r->cache.arch_has_per_cpu_cfg = true;
+			r->cache.min_cbm_bits = 0;
 		} else if (r->rid == RDT_RESOURCE_MBA) {
 			hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
 			hw_res->msr_update = mba_wrmsr_amd;