Re: [Xen-devel] [PATCH 6/9] x86/amd: Allocate resources to cope with LS_CFG being per-core on Fam17h

["Woods, Brian" <Brian.Woods@xxxxxxx>]

On Thu, Dec 06, 2018 at 06:46:51PM +0000, Andy Cooper wrote:
> On 06/12/2018 08:54, Jan Beulich wrote:
> >>>> On 05.12.18 at 18:05, <andrew.cooper3@xxxxxxxxxx> wrote:
> >> On 05/12/2018 16:57, Jan Beulich wrote:
> >>>>>> On 03.12.18 at 17:18, <andrew.cooper3@xxxxxxxxxx> wrote:
> >>>> --- a/xen/arch/x86/cpu/amd.c
> >>>> +++ b/xen/arch/x86/cpu/amd.c
> >>>> @@ -419,6 +419,97 @@ static void __init noinline 
> >>>> amd_probe_legacy_ssbd(void)
> >>>>  }
> >>>>  
> >>>>  /*
> >>>> + * This is all a gross hack, but Xen really doesn't have flexible-enough
> >>>> + * per-cpu infrastructure to do it properly.  For Zen(v1) with SMT 
> >>>> active,
> >>>> + * MSR_AMD64_LS_CFG is per-core rather than per-thread, so we need a 
> >>>> per-core
> >>>> + * spinlock to synchronise updates of the MSR.
> >>>> + *
> >>>> + * We can't use per-cpu state because taking one CPU offline would free 
> >>>> state
> >>>> + * under the feet of another.  Ideally, we'd allocate memory on the AP 
> >>>> boot
> >>>> + * path, but by the time the sibling information is calculated 
> >>>> sufficiently
> >>>> + * for us to locate the per-core state, it's too late to fail the AP 
> >>>> boot.
> >>>> + *
> >>>> + * We also can't afford to end up in a heterogeneous scenario with some 
> >>>> CPUs
> >>>> + * unable to safely use LS_CFG.
> >>>> + *
> >>>> + * Therefore, we have to allocate for the worse-case scenario, which is
> >>>> + * believed to be 4 sockets.  Any allocation failure cause us to turn 
> >>>> LS_CFG
> >>>> + * off, as this is fractionally better than failing to boot.
> >>>> + */
> >>>> +static struct ssbd_ls_cfg {
> >>>> +        spinlock_t lock;
> >>>> +        unsigned int disable_count;
> >>>> +} *ssbd_ls_cfg[4];
> >>> Same question as to Brian for his original code: Instead of the
> >>> hard-coding of 4, can't you use nr_sockets here?
> >>> smp_prepare_cpus() runs before pre-SMP initcalls after all.
> >> nr_sockets has zero connection with reality as far as I can tell.
> >>
> >> On this particular box it reports 6 when the correct answer is 2.  I've
> >> got some Intel boxes where nr_sockets reports 15 and the correct answer
> >> is 4.
> > If you look back at when it was introduced, the main goal was
> > for it to never be too low. Any improvements to its calculation
> > are welcome, provided they maintain that guarantee. To high
> > a socket count is imo still better than a hard-coded one.
> 
> Even for the extra 2k of memory it will waste?
> 
> ~Andrew

Just as a side note, for processors using MSR LS_CFG and have SMT
enabled (F17h), there should only be 2 physical sockets.  The 4 was a
worst case (and before some other information was available).
Realistically, there should only be a max of 2 physical sockets when
this needed.  Although, having 4 could be nice as a safe buffer and
only costs 16 bytes.

-- 
Brian Woods

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel