On Wed, Nov 19, 2025 at 01:55:15PM +0000, James Clark wrote:
[...]
static ssize_t format_attr_contextid_show(struct device *dev, struct device_attribute *attr, char *page) { #if IS_ENABLED(CONFIG_ARM64) if (is_kernel_in_hyp_mode()) return contextid2_show(dev, attr, page); #endif
return contextid1_show(dev, attr, page);Not having an #else implies that the contextid1_show() part is valid when !CONFIG_ARM64, but that isn't right. That's why I had the WARN_ON because it's dead code.
Based on ETMv3/v4 spec, would contextid1 always be valid ? (Though we do not support context ID for ETMv3 yet).
Personally I would drop the is_visible(). It makes sense for dynamically hidden things, but these are all compile time. IMO it's cleaner to just not include them to begin with, rather than include and then hide them. Then the extra condition in format_attr_contextid_show() isn't needed because the function doesn't exist:
This is fine for me, though in general I think the dynamic approach is readable and extendable than the compile-time approach.
Thanks, Leo