On Wed, May 21, 2025 at 09:54:48AM +0100, James Clark wrote: > On 20/05/2025 5:22 pm, Leo Yan wrote: [...] > I'm thinking I'd rather leave it consistent with PMSFCR_EL1.FT and > automatically enable PMSFCR_EL1.FDS for any non zero data-source filter. This is fine for me. Just a minor thing, for the case PMSDSFR_EL1 = 0xFFFF,FFFF,FFFF,FFFF, we might consider to clear the PMSFCR_EL1.FDS bit. This would be a bit performance benefit for disabling data source filter rather than enabling the filter with unaffecting all data sources. > This means we don't need a tool change to set some other flag when a filter > is provided (even if it's zero) and it's much simpler. It also doesn't > prevent the possibility of adding the enable flag in the future if someone > comes out with a need for it, but I don't think it needs to be done now. The question comes down to the complexity in user-space tools. Perf initializes the attribute configs to zeros. If we want to set all bits in config4 as a default value, we would need additional change in the perf tool. Also initializing config4 to all ones is likely to cause confusion if other tools want to enable the feature. I agree that a cleaner way would be to use an enable flag + mask, we can defer to add flag if needed. > TBH I can't imagine a case where someone would want to filter out any samples > that have any data source. Surely you'd only be looking for a selected set > of data sources, or no filtering at all. Agreed this is a rare case. Thanks, Leo
