Re: [PATCH v3 13/13] perf/x86: add syfs entry to disable HT bug workaround

[Stephane Eranian]

Hi,

On Tue, Nov 18, 2014 at 1:31 AM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>
> On Tue, 18 Nov 2014, Borislav Petkov wrote:
> > On Tue, Nov 18, 2014 at 12:38:14AM +0100, Thomas Gleixner wrote:
> > > Well a bitmask is a pretty indescriptive item as well. Putting my user
> > > hat on: Where is the documentation for the bits?
> >
> > $ cat /sys/devices/system/cpu/bugs
> > 0xXXXXXX - currently enabled workarounds are the set bits.
> > bit 0: workaround for bug#blabla
> > bit 1: workaround for bug#1
> > bit 2: workaround for bug#2; remember to do <bla> before disabling workaround
> > ...
> > bits n-63 are reserved, cannot be set and RAZ.
>
> You sure that 64 are enough?
>
> You need to create stable but numbers, i.e. each bug gets a fixed but
> number whethr it affects the machine or not. Otherwise you will drive
> admins completely nuts.
>
> > This will be issued when user cats the sysfs file.
>
> That might work as well, though you want that to be:
>
>   /sys/devices/system/cpu/bugs/
>
>   /sys/devices/system/cpu/bugs/status
>
>   /sys/devices/system/cpu/bugs/enable_workaround
>
>   /sys/devices/system/cpu/bugs/disable_workaround
>
>   The latter two take a bit number rather than a magic mask.
>
I am trying to get a better understanding of this scheme.

status:
  - a summary of what is enabled/disabled?
  - With description (as suggested by Boris)?
  - File is readonly
  - is that printing a variable length bitmask?

enable_workaround:
   - provide the bit number (of the workaround) to enable the workaround
   - File is write-only

disable_workaround:
   - provide the bit number (of the workaround) to disable the workaround
   - File is write-only

The split enable/disable is to avoid the read-modify-write issue.

Am I getting this right?

I understand the value of this proposition. But, I feel,  it is beyond the scope
of the patch series to workaround the PMU bug. Initially, we had
talked about not
even providing the sysfs file. Now, the series adjusts the workaround
on boot. The series is restructured so that the sysfs patch is the last
one and is totally optional. I think we should implement the proposed scheme
but we should not delay the review and merge of the rest of the patch series
for this. But I can propose a separate patch series to implement the proposed
scheme.

> So while it looks less effort to implement and extend in the first
> place I think, that a bit of infrastructure work will make the
> explicit scheme I proposed before a no brainer to maintain and extend,
> but I cannot judge what's more intuitive to use.
>
> Thanks,
>
>         tglx
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/