On Tue, 2022-03-29 at 16:10 +1300, Kai Huang wrote: > > > > > > > > *all cpus* is questionable. > > > > > > > > Say BIOS enabled 8 CPUs: [0 - 7] > > > > > > > > cpu_present_map covers [0 - 5], due to nr_cpus=6 > > > > > > > > You compared cpus_booted_once_mask to cpu_present_mask so if > > > maxcpus > > > > is set to a number < nr_cpus SEAMRR is considered disabled because you > > > > cannot verify CPUs between [max_cpus, nr_cpus). > > Sorry my bad. We can verify CPUs between [max_cpus, nr_cpus). When any cpu > within that range becomes online, the detection code is run. The paranoid > check > in seamrr_enabled() is used to check whether all cpus within [max_cpus, > nr_cpus) > (if there's any -- cpus within [0, max_cpus) are up during boot) have been > brought up at least once. > > > > > > > SEAMRR is not considered as disabled in this case, at least in my > > > intention. > > > > the function is called seamrr_enabled(), and false is returned if above > > check is not passed. So it is the intention from the code. > > The false is returned if something error is discovered among cpus [0 - > present_cpus]. It returns true even if we cannot verify [present_cpus, > bios_enabled_cpus). Sorry typo. Not "something error is discovered among cpus [0 - present_cpus)", but "when there's any cpu within [0 - present_cpus) hasn't been brought up once". > > > > > > My > > > understanding on the spec is if SEAMRR is configured as enabled on one > > > core > > > (the > > > SEAMRR MSRs are core-scope), the SEAMCALL instruction can work on that > > > core. It > > > is TDX's requirement that some SEAMCALL needs to be done on all BIOS- > > > enabled > > > CPUs to finish TDX initialization, but not SEAM's. > > > > > > From this perspective, if we forget TDX at this moment but talk about SEAM > > > alone, it might make sense to not just treat SEAMRR as disabled if kernel > > > usable > > > cpus are limited by 'nr_cpus'. The chance that BIOS misconfigured SEAMRR > > > is > > > really rare. If kernel can detect potential BIOS misconfiguration, it > > > should do > > > it. Otherwise, perhaps it's more reasonable not to just treat SEAM as > > > disabled. > > > > My problem is just that you didn't adopt consistency policy for CPUs in > > [maxcpus, nr_cpus) and CPUs in [nr_cpus, nr_bios_enabled_cpus). This is > > the only trouble to me no matter what policy you want to pursue... > > Let's separate SEAMRR detection and TDX initialization. The paranoid check is > only for SEAM detection, but not for TDX initialization. As I said, it is > TDX's > requirement that some SEAMCALL must be run on all bios-enabled cpus, but not > SEAM's. > >
