com/kernel/2007-q4/msg19926.html
Yes it makes sense.
-Andi
>
> Out of curiosity, how much is Intel PT used purely for control flow tracing,
> i.e.
> without caring _at all_ about perceived execution time?
It is very common, e.g. one major use of PT is control flow discovery in
feedback fuzzers.
-Andi
me... if this works, you could pioneer a sequence
of simiar changes :-)
Merged to i2c/i2c-host.
Thanks,
Andi
Hi Wolfram,
On Fri, Jul 12, 2024 at 08:34:11AM GMT, Wolfram Sang wrote:
> On Fri, Jul 12, 2024 at 01:19:24AM +0200, Andi Shyti wrote:
> > Hi,
> >
> > while reviewing Wolfram's series, I received some delivery
> > failure notifications for e-mails that don
2c), I didn't
really know what to do with them, so that I marked them as
Orphan.
Andi
Cc: Andy Shevchenko
Cc: Krzysztof Kozlowski
Cc: Lee Jones
Cc: Linus Walleij
Cc: Philipp Zabel
Cc: Viresh Kumar
Cc: Wolfram Sang
Cc: linux-g...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: virtu
E-mails to Conghui Chen have bounced back:
: host mgamail.eglb.intel.com[198.175.65.14] said: 550
#5.1.0 Address rejected. (in reply to RCPT TO command)
Remove him as maintainer of the i2c Virtio driver in the
MAINTAINERS file.
Signed-off-by: Andi Shyti
Cc: Viresh Kumar
Cc: Wolfram
Hi Wolfram,
pushed in i2c/i2c-host.
Thanks for this big work, at the end it turned out quite nice and
I'm happy of the outcome!
Thanks
Andi
On Sat, Jul 06, 2024 at 01:20:00PM GMT, Wolfram Sang wrote:
> Start changing the wording of the I2C main header wrt. the newest I2C
> v7 an
Hi Wolfram,
On Sat, Jul 06, 2024 at 01:20:58PM GMT, Wolfram Sang wrote:
> Change the wording of this driver wrt. the newest I2C v7 and SMBus 3.2
> specifications and replace "master/slave" with more appropriate terms.
>
> Signed-off-by: Wolfram Sang
Reviewed-by: Andi Shyti
Thanks,
Andi
Hi Wolfram,
> > @Andi: are you okay with this approach? It means you'd need to merge
> > -rc2 into your for-next branch. Or rebase if all fails.
>
> I think it's a good plan, I'll try to support you with it.
Do you feel more comfortable if I take the patches as
P. To be
> able to work on this with minimal dependencies, I'd like to apply this
> series between -rc1 and -rc2.
>
> I hope this will work for you guys. The changes are really minimal. If
> you are not comfortable with changes to your driver or need more time to
> review
> __typeof__(*(ptr)) _prev_; \
> unsigned long __flags; \
> \
> - BUILD_BUG_ON(sizeof(_p_) != 4); \
> -
eplacing everything.
Of course long term it might be a problem to maintain too many
different ways to do things, but I suppose short term it's a reasonable
strategy.
-Andi
> The CNA code, if enabled, will be in vmlinux, not in a kernel module. As a
> result, I think a module parameter will be no different from a kernel
> command line parameter in this regard.
You can still change it in /sys at runtime, even if it's in the vmlinux.
-Andi
> Now imagine we have an 8 node system, and memory
> pressure in the DMA32 zone of node 0.
The question is how much do we still care about DMA32.
If there are problems they can probably just turn on the IOMMU for
these IO mappings.
-Andi
used,
so this PTE skipping trick doesn't work?
-Andi
to have to reboot just to play with this parameter
> Yes, good suggestion, thanks.
>
> > This would also make the code a lot shorter I guess.
> So you don’t think we need the command-line parameter, just the module_param?
module_params can be changed at the command line too, so yes.
-Andi
Andi Kleen writes:
> Alex Kogan writes:
>>
>> +numa_spinlock_threshold=[NUMA, PV_OPS]
>> +Set the time threshold in milliseconds for the
>> +number of intra-node lock hand-offs before the
>> +
ms granularity seems very coarse grained for this. Surely
at some point of spinning you can afford a ktime_get? But ok.
Could you turn that into a moduleparm which can be changed at runtime?
Would be strange to have to reboot just to play with this parameter
This would also make the code a lot shorter I guess.
-Andi
> The reason why soft lockup happens may be the unmapped EPT pages. So, do we
> have a way to map all gpa
> before we use pebs on Skylake?
Can you configure a VT-d device, that will implicitly pin all pages for the
IOMMU. I *think* that should be enough for testing.
-Andi
> I have actually seen real user programs poke MSR_SYSCALL_MASK.
Hmm, what was the use case?
-Andi
ith existing sandbox and lockdown mechanisms. But
on a non locked down system fully accessible MSRs are really useful for
all kind of debugging and tuning, and anything that prevents that
is bad.
-Andi
Except where I commented, for the series
Acked-by: Andi Kleen
-Andi
> +--threads=::
> +Write collected trace data into several data files using parallel threads.
> + value can be user defined list of masks. Masks separated by colon
> +define cpus to be monitored by a thread and affinity mask of that thread
> +is separated by slash. For example user specification li
> + err = write(thread->pipes.ack[1], &msg, sizeof(msg));
> + if (err == -1)
> + pr_err("threads[%d]: failed to notify on start. Error %m",
> thread->tid);
It might be safer to not use %m. I'm not sure if all the non glibc
libcs that people use support it.
> + } else {
> + thread_data[t].tid = syscall(SYS_gettid);
That always fills in the tid of the setup thread instead of the target
threads?
> Hmm, I forgot about that quirk. I would expect the TDX Module to inject a #GP
> for that case. I can't find anything in the spec that confirms or denies
> that,
> but injecting #VE would be weird and pointless.
>
> Andi/Sathya, the TDX Module spec should be updated to
David Hildenbrand writes:
> I have no idea how expensive would be bouncing writes (and reads?)
> through the kernel. Did you ever experiment with that/evaluate that?
I would expect it to be quite expensive, as in virtio IO performance
tanking.
-Andi
On Wed, Apr 07, 2021 at 11:05:20AM +0800, Liuxiangdong (Aven, Cloud
Infrastructure Service Product Dept.) wrote:
>
>
> On 2021/4/6 20:47, Andi Kleen wrote:
> > > AFAIK， Icelake supports adaptive PEBS and extended PEBS which Skylake
> > > doesn't.
> > >
. It's in principle possible, but currently
not implemented.
In general host swap without balloning is usually a bad idea anyways
because it often just swaps a lot of cache data that could easily be
thrown away instead.
-andi
> Now the hottest block is reported at the top of output.
>
> Fixes: b65a7d372b1a ("perf hist: Support block formats with
> compare/sort/display")
> Signed-off-by: Jin Yao
Reviewed-by: Andi Kleen
-Andi
t; when guest is kernel-5.11, but can't when kernel-4.18. Is there a minimum
> guest kernel version requirement?
You would need a guest kernel that supports Icelake server PEBS. 4.18
would need backports for tht.
-Andi
On Sat, Apr 03, 2021 at 09:26:24AM -0700, Dave Hansen wrote:
> On 4/2/21 2:32 PM, Andi Kleen wrote:
> >> If we go this route, what are the rules and restrictions? Do we have to
> >> say "no MMIO in #VE"?
> >
> > All we have to say is "No M
is a lot of code to audit.
But you can always disable the filtering with a command line option and
then it will always work for debugging.
-Andi
On Sat, Dec 19, 2020 at 09:08:05AM -0800, Randy Dunlap wrote:
> Give the user a clue about the problem along with the 35 lines of
> usage/help text.
Reviewed-by: Andi Kleen
-Andi
ct, blowing up code side everywhere
and adding complicated self modifying code, when it's only needed for very
few drivers. But we also don't want to patch every MMIO to be special cased
even those few drivers.
#VE based MMIO avoids all that cleanly while being nicely non intrusive.
-Andi
On Wed, Mar 31, 2021 at 08:46:18PM -0700, Dave Hansen wrote:
> On 3/31/21 8:28 PM, Andi Kleen wrote:
> >> The hardware (and VMMs and SEAM) have ways of telling the guest kernel
> >> what is supported: CPUID. If it screws up, and the guest gets an
> >> unexpected #VE,
e. Why should TDX be any different?
That's what the original patch did -- no unnecessary checks -- but reviewers
keep asking for the extra checks, so Sathya added more. We have the not
unusual problem here that reviewers don't agree among themselves.
-Andi
; Signed-off-by: Kuppuswamy Sathyanarayanan
> >
> > Reviewed-by: Andi Kleen
> > ---
> >
> > Changes since previous series:
> > * Suppressed MWAIT feature as per Andi's comment.
> > * Added warning debug log for MWAIT #VE exception.
> >
&
> > No, if these instructions take a #VE then they were executed at CPL=0.
> > MONITOR
> > and MWAIT will #UD without VM-Exit->#VE. Same for WBINVD, s/#UD/#GP.
>
> Dare I ask about XSETBV?
XGETBV does not cause a #VE, it just works normally. The guest has full
AVX capabilities.
-Andi
> > But is this a real problem?
>
> perhaps not, Andi, any idea about this?
It's not a problem for my tools which don't use the unit,
but I could imagine one for other parsers. I would recommend
to not change it for CSV, which is expected to be parsed
by tools.
-Andi
Jim Cromie writes:
> CONFIG_DYNAMIC_DEBUG creates a struct _ddebug (56 bytes) for each
> callsite, which includes 3 pointers to: module, filename, function.
> These are repetetive, and compressible, this patch series goes about
> doing that, it:
So how much memory does it actually save?
-Andi
CPUs not getting profiled.
It's a similar strategy as we do in the source code when semantics
change.
ARM did this right.
-Andi
previous mess. It's simply not
valid CSV.
That's why I'm arguing that keeping compatibility is not useful here.
We would be stuck with the broken mess as default forever.
-Andi
of being on the command line.
-Andi
On Tue, Mar 16, 2021 at 04:05:13PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 16, 2021 at 09:34:21AM -0700, Andi Kleen escreveu:
> > > looks ok, but maybe make the option more related to CVS, like:
> > >
> > > --x-summary, --cvs-summary ...?
> >
> looks ok, but maybe make the option more related to CVS, like:
>
> --x-summary, --cvs-summary ...?
Actually I don't think it should be a new option. I doubt
anyone could parse the previous mess. So just make it default
with -x
-Andi
it is.
But Folio? Huh?
-Andi
> I still think that there is value in taking those measurements after we
> enable the counters, as, for instance, for interval mode we want
> measurements with the counters enabled, whatever happens before the
> counters are enabled is just startup time, etc. Jiri, Andi?
Yes I agr
ld_flags & ~LRU_GEN_MASK) | ((new_gen + 1UL) <<
> LRU_GEN_PGOFF);
> + /* mark page for reclaim if pending writeback */
> + if (front)
> + new_flags |= BIT(PG_reclaim);
> + } while (cmpxchg(&page->flags, old_flags, new_flags) !=
> old_flags);
I see this cmpxchg flags pattern a lot. Could there be some common code
factoring?
-Andi
> A more concise syntax would be -e +event
>
> The + would apply to the whole event list if there are multiple.
>
> and maybe -event too to remove something from the default list.
Sorry that was an old email. Please ignore.
-Andi
is '--add-default'.
A more concise syntax would be -e +event
The + would apply to the whole event list if there are multiple.
and maybe -event too to remove something from the default list.
-Andi
umbers are not stable though)
Looks all good to me. The VmPeak assumption might be slightly
fragile, but I guess there's nothing better currently.
Reviewed-by: Andi Kleen
-Andi
nd configuration from the same base kernel
release.
-Andi
rences to other Golden Cove or Gracemont implementations.
For non event list usages, the model numbers also indicate a lot of things
in the SOC, so even you knew it was a somewhat similar core as Sapphire
Rapids, it wouldn't tell the complete story. Even on the cores there are
differences.
In the end you need to know that Alderlake is Alderlake.
-Andi
erhaps add some way
that someone doing wait*() can know the exit was due this mitigation
(and not something way) Then they could disable respawning of that daemon.
-Andi
Thanks.
Okay but that means that the brute force attack can just continue
because the attacked daemon will be respawned?
You need some way to stop the respawning, otherwise the
mitigation doesn't work for daemons.
-Andi
> AFAICT we could register them all here. That instantly fixes that
> CPU_STARTING / CPU_DEAD fail elsewhere in this patch.
This would mean a system that only has Atoms or only has big cores
would still show the other CPU's PMU. We expect those to exist.
-Andi
. Presumably most people already
running with newer microcodes, so that functional impact is small.
But it should speed up the newer systems by the 2-4% claimed in
the original patch.
Cc: jmatt...@google.com
Fixes: 9b545c04abd4 ("perf/x86/kvm: Avoid unnecessary work ...")
Signed-off-by:
useful security mitigation you need a threat model first I would say.
So you need to have at least some idea how an attack with branch
tracing would work.
> Initial change was to restrict this only to HW assisted instruction tracing
> [1]
I don't think it's needed for instruction tracing.
-Andi
queue
> buffers correctly anyway, so the check is not needed.
>
> In addition, the check gets erroneously hit when using sample mode
> to trace multiple threads.
>
> Consequently, fix that case by removing the check.
Thanks!
Reviewed-by: Andi Kleen
-Andi
On Sun, Mar 07, 2021 at 07:05:41PM +0100, John Wood wrote:
> On Sun, Mar 07, 2021 at 09:25:40AM -0800, Andi Kleen wrote:
> > > processes created from it will be killed. If the systemd restart the
> > > network
> > > daemon and it will crash again, then the s
might be ok depending on the use case,
but people certainly need to be aware of it.
It's probably not something you want to have enabled by default ever.
-Andi
eed some user space configuration
changes too.
-Andi
support for the Samsung S6SY761
> touchscreen")
> Signed-off-by: Caleb Connolly
Reviewed-by: Andi Shyti
Thanks,
Andi
[2] << 3) | (event[3] & S6SY761_MASK_Y);
> + u16 x = (event[1] << 4) | ((event[3] & S6SY761_MASK_X) >> 4);
> + u16 y = (event[2] << 4) | (event[3] & S6SY761_MASK_Y);
the devil knows how that '3' has ended up there :)
Thanks for catching it!
Reviewed-by: Andi Shyti
Andi
rXXX which one it is,
it may not be obvious with offsetted fields (like umask=0x121212),
and hard to find in a long command line.
-Andi
Andi Kleen writes:
>
> Normally disk encryption is in specialized work queues. It's total
> overkill to restrict all of the kernel if you just want to restrict
> those work queues.
>
> I would suggest some more analysis where secrets are actually stored
> and handled f
It's total
overkill to restrict all of the kernel if you just want to restrict
those work queues.
I would suggest some more analysis where secrets are actually stored
and handled first.
-Andi
ese "high level"
problems here.
-Andi
possibly add the extra columns from there just to stay compatible,
but being a subset is ok too.
-Andi
It would be ok if it fixed something serious, but as it seems to be
more cosmetic I don't think there's a good reason to devivate.
-Andi
On Thu, Feb 18, 2021 at 11:57:50AM +0200, Adrian Hunter wrote:
> Hi
>
> Currently, only kernel tracing is supported and only with "timeless" decoding
> i.e. no TSC timestamps
Patches look good to me. That will be quite useful.
Acked-by: Andi Kleen
Thanks,
-Andi
re future asynchronous #VEs they would only happen
with IF=1, which would also protect the gap.
So no need to make #VE an IST.
-Andi
ely.
#DB is trickier because it will happen every time, so simply reexecuting
won't work. I guess it would need the ve info stack, or some care in
kprobes/kernel
debugger that it cannot happen. I think I would prefer the later.
-Andi
for that.
Or alternatively the NMI always gets the VE information and puts
it on some internal stack, but that would seem clunkier.
-Andi
ll microcode versions.
>
> Add the Cascade Lake Xeon steppings (5, 6, and 7) to the
> isolation_ucodes[] table so that these parts benefit from Andi's
> optimization in commit 9b545c04abd4f ("perf/x86/kvm: Avoid unnecessary
> work in guest filtering").
Reviewed-by: Andi Kleen
-Andi
On Fri, Feb 05, 2021 at 07:53:46PM +0200, Adrian Hunter wrote:
> Hi
>
> Here are 3 fixes and 1 minor new feature, for Intel PT.
For the series:
Reviewed-by: Andi Kleen
-Andi
ink overloading LOST is not so very nice
> > for this.
>
> But anything can get lost in case of no space.
> Do you want to use something other than the LOST event?
Could always reserve the last entry in the ring buffer for a LOST event,
that would guarantee you can always get one out.
-Andi
On Fri, Jan 15, 2021 at 10:51:38AM -0800, Sean Christopherson wrote:
> On Fri, Jan 15, 2021, Andi Kleen wrote:
> > > I'm asking about ucode/hardare. Is the "guest pebs buffer write -> PEBS
> > > PMI"
> > > guaranteed to be atomic?
> >
>
how bad do things get for
> the
> guest if we simply disable guest counters if they can't have a 1:1 association
> with their physical counter?
This would completely break perfmon for the guest, likely with no way to
recover.
-Andi
so no bug.
That said of course the change is the right thing for main line, but probably
doesn't
need to be backported.
-Andi
oesn't make a lot of sense.
-Andi
> My first thought was: Why not have a 'perf iiostat' subcommand?
Same would apply to a lot of options in perf stat.
I guess you could add some aliases to "perf" that give shortcuts
for common perf stat command lines.
-Andi
On Wed, Dec 02, 2020 at 11:47:25AM -0800, Stephane Eranian wrote:
> On Wed, Dec 2, 2020 at 11:28 AM Andi Kleen wrote:
> >
> > > + prev_cgrp = task_css_check(prev, perf_event_cgrp_id, 1)->cgroup;
> > > + next_cgrp = task_css_check(next,
perf cgroup only, not all cgroups.
That's a big difference and needs to be documented properly.
Probably would make sense to have two events for both, one for
all cgroups and one for perf only.
-Andi
;ll do a v5 with a macro :-/
I suppose you could inlines with callbacks too, with one liner functions for
the operands.
-Andi
} else {
> + /* LHS and/or RHS need computing from event IDs so union. */
> + $$ = union_expr($1, $3);
> + }
Can't this be all in a macro? It's still a lot of duplication.
I'm still not a fan, but I think with a macro it wouldn't be too bad.
-Andi
roblem with debugfs is security,
or is it no convenient process that could do cron like functionality?
If it's the first, perhaps what they really need is a way to get
a partial debugfs?
-Andi
#x27;t really
needed for perf, but presumably if we want to do more complicated
things it might be useful.
In theory it could speed up performance slightly when an expression needs
to be computed multiple times in interval mode.
-Andi
#include
#include
#include
#include
#include "code.h&
> You mean change event-converter-for-linux-perf to add this as JSON
> comments at the start of the generated files?
JSON doesn't support comments unfortunately
(unless they're somehow part of the data schema)
-Andi
kernels.
In principle the scripts could be included, but without the raw files it would
be somewhat pointless.
-andi
> Can I get ACK for this patch?
>
> It's mainly for fixing the perf-test issue and MEM_Parallel_Reads issue.
Acked-by: Andi Kleen
The JSON updates should be imho just merged automatically. Not much
point in doing code review on them. If there's a problem it has
to be
Actually thinking about it more you should probably pass around ctx/cgroup
in a single abstract argument. Otherwise have to change all the metrics
functions for the next filter too.
b->cgrp ? -1 : +1;
This means the sort order will depend on heap randomization,
which will make it harder to debug.
Better use something stable like the inode number of the cgroup.
Do we have the same problem with other filters?
The rest of the patch looks good to me.
-Andi
: expr { reset flag }
scanner checks flag and ignores the event if false
This would need changing the JSON files but that should be easy enough
with a script.
Or maybe at some point need to bite the bullet and build
an AST, but for now we probably can get away of not doing it.
-Andi
0.76 insn per cycle
> CPU2 44,763,978 instructions #0.45 insn per cycle
> CPU3 69,049,079 instructions #0.56 insn per cycle
>
>1.001910444 seconds time elapsed
Yes makes a lot of sense. Thanks for tracking that down.
Reviewed-by: Andi Kleen
-Andi
ood idea
because it will not be very usable, or worse even misleading.
Even for level 2/3 we should probably have some simple thresholding at least,
but maybe could get away with not having it yet.
While it's of course possible to add this too it would be significant surgery.
I actually considered doing it all in perf at some point, but it was quite
complicated.
I personally found it simpler to handle those higher level algorithms in a
separate tool.
-Andi
re levels is probably ok, but doing all
> levels
> without these mechanisms might be difficult to use in the end.
>
>Thanks Andi, I'm working from the optimization manual and trying to
>understand this. With this contribution we have both metrics and
>gr
ght.
I'm not saying it's not doable, but it will be a lot of additional work
to work out all the quirks using the metrics infrastructure.
I think adding one or two more levels is probably ok, but doing all levels
without these mechanisms might be difficult to use in the end.
-Andi
hy not enable it by default
if the kernel supports it?
With the option most user won't get the benefit.
The only reason I can think of for an option would be to disable
so that old tools can still process.
-Andi
1 - 100 of 8120 matches
Mail list logo
