On Wed, Sep 24, 2025 at 11:50:02AM +0000, Andrew Cooper wrote:
> On 24/09/2025 4:00 am, Roger Pau Monne wrote:
> > Otherwise the check for the SS feature in
> > check_memory_type_self_snoop_errata() fails unconditionally, which leads to
> > X86_FEATURE_XEN_SELFSNOOP never being set.
> >
> > We could also avoid this by not doing the reset_cpuinfo() for the BSP in
> > identify_cpu(), because SS detection uses boot_cpu_data.
>
> Doesn't this, mean ...
Well, that's the reason for the rant here. The reset at the top of
identify_cpu() has been there since 2005. It's arguably to make sure
the BSP and the APs have the same empty state in the passed
cpuinfo_x86 struct, as for the BSP this would be already partially
initialized due to what's done in early_cpu_init().
The underlying question is whether we would rather prefer to not do
the reset for the BSP, but that would lead to differences in the
contents of cpuinfo_x86 struct between the BSP and the APs. In the
past we have arranged for leaves needed early to be populated in
generic_identify(), like FEATURESET_e21a, hence the proposed patch
does that for FEATURESET_1d.
> > However that
> > creates an imbalance on the state of the BSP versus the APs in the
> > identify_cpu() code.
> >
> > I've opted for the less controversial solution of populating FEATURESET_1d
> > in generic_identify(), as the value is already there. The same is done for
> > the AMD faulting probe code.
> >
> > Fixes: f2663ca2e520 ("x86/cpu/intel: Clear cache self-snoop capability in
> > CPUs with known errata")
> > Signed-off-by: Roger Pau Monné <[email protected]>
>
> ... this Fixes tag is incorrect?
I think the Fixes tag is accurate; the code was OK before that change.
Nothing in c_early_init hooks depended on (some of) the x86_capability
fields being populated, which is required after the change.
Thanks, Roger.
