[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 2/2] x86/spec-ctrl: add support for modifying SSBD via LS_CFG MSR



>>> On 30.08.18 at 20:09, <brian.woods@xxxxxxx> wrote:
> On Thu, Aug 30, 2018 at 09:49:58AM -0600, Jan Beulich wrote:
>> >>> On 27.08.18 at 18:55, <brian.woods@xxxxxxx> wrote:
>> > --- a/xen/arch/x86/spec_ctrl.c
>> > +++ b/xen/arch/x86/spec_ctrl.c
>> > @@ -20,6 +20,7 @@
>> >  #include <xen/init.h>
>> >  #include <xen/lib.h>
>> >  #include <xen/warning.h>
>> > +#include <xen/spinlock.h>
>> >  
>> >  #include <asm/microcode.h>
>> >  #include <asm/msr.h>
>> > @@ -58,8 +59,17 @@ paddr_t __read_mostly l1tf_addr_mask, __read_mostly 
>> > l1tf_safe_maddr;
>> >  static bool __initdata cpu_has_bug_l1tf;
>> >  static unsigned int __initdata l1d_maxphysaddr;
>> >  
>> > +/* for SSBD support for AMD via LS_CFG */
>> > +#define SSBD_AMD_MAX_SOCKET 4
>> 
>> Oh, went up from 2 to 4? But still not a dynamic upper bound?
>> 
> 
> Well, 4 was just to be safe.  2 is more than reasonable IMO.  Having it
> dynamic seems like a lot of work to save 8 bytes (or after the bump to
> 4, 24 bytes) when it doesn't get used.

Well, am I misremembering that I saw you say somewhere that you'd
switch to using nr_sockets?

>> > +struct ssbd_amd_ls_cfg_smt_status {
>> > +    spinlock_t lock;
>> > +    uint32_t mask;
>> > +} __attribute__ ((aligned (64)));
>> 
>> Ehem. See the respective comment on patch 1. To put it bluntly,
>> I'm not willing to look at patches where prior comments were not
>> addressed, the more that you had verbally agreed to use
>> SMP_CACHE_BYTES here.
> 
> Well, a rebasing failed  and I had to regenerate the patches from a
> diff of the v3 patches combined, and I missed fixing that one part.
> I'm terrible sorry I missed that one fix.

If it was just that one fix, I wouldn't have stopped review at that
point. But together with patch 1 it makes at least three missed
fixes, and that made me then consider it too much.

Jan



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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.