From 72b2aa38191bcba28389b0e20bf6b4f15017ff2b Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Thu, 13 Oct 2022 15:04:04 +0300 Subject: tools: iio: iio_utils: fix digit calculation The iio_utils uses a digit calculation in order to know length of the file name containing a buffer number. The digit calculation does not work for number 0. This leads to allocation of one character too small buffer for the file-name when file name contains value '0'. (Eg. buffer0). Fix digit calculation by returning one digit to be present for number '0'. Fixes: 096f9b862e60 ("tools:iio:iio_utils: implement digit calculation") Signed-off-by: Matti Vaittinen Link: https://lore.kernel.org/r/Y0f+tKCz+ZAIoroQ@dc75zzyyyyyyyyyyyyycy-3.rev.dnainternet.fi Cc: Signed-off-by: Jonathan Cameron --- tools/iio/iio_utils.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'tools') diff --git a/tools/iio/iio_utils.c b/tools/iio/iio_utils.c index aadee6d34c74..8d35893b2fa8 100644 --- a/tools/iio/iio_utils.c +++ b/tools/iio/iio_utils.c @@ -547,6 +547,10 @@ static int calc_digits(int num) { int count = 0; + /* It takes a digit to represent zero */ + if (!num) + return 1; + while (num != 0) { num /= 10; count++; -- cgit v1.2.3 From 9bfb09774ea40937b6c6d6e07858e0f7ad1991ec Mon Sep 17 00:00:00 2001 From: Todd Brandt Date: Thu, 20 Oct 2022 02:23:10 -0700 Subject: pm-graph v5.10 sleepgraph: - add -wifitrace argument for tracing all the way to wifi reconnect - include more data in ftrace to mark the end of kernel resume - add async_synchronize_full to the list of funcs to chart - add thermal zone info to the log data - include a check for s0ix support (s2idle is the default mem_sleep) - if s2idle does not support s0ix, remove the SYS%LPI turbostat var - fix -dev crash when kprobe caller is just an address (not a symbol) - fix the cpuexec data in -proc to display in resume sleepgraph.8: - add -wifitrace documentation README: - change links from 01.org to developer.intel.com Signed-off-by: Todd Brandt Signed-off-by: Rafael J. Wysocki --- tools/power/pm-graph/README | 12 +- tools/power/pm-graph/sleepgraph.8 | 3 + tools/power/pm-graph/sleepgraph.py | 225 ++++++++++++++++++------------------- 3 files changed, 118 insertions(+), 122 deletions(-) (limited to 'tools') diff --git a/tools/power/pm-graph/README b/tools/power/pm-graph/README index e6020c0d59ec..3213dbe63b74 100644 --- a/tools/power/pm-graph/README +++ b/tools/power/pm-graph/README @@ -6,22 +6,22 @@ |_| |___/ |_| pm-graph: suspend/resume/boot timing analysis tools - Version: 5.9 + Version: 5.10 Author: Todd Brandt - Home Page: https://01.org/pm-graph + Home Page: https://www.intel.com/content/www/us/en/developer/topic-technology/open/pm-graph/overview.html Report bugs/issues at bugzilla.kernel.org Tools/pm-graph - https://bugzilla.kernel.org/buglist.cgi?component=pm-graph&product=Tools Full documentation available online & in man pages - Getting Started: - https://01.org/pm-graph/documentation/getting-started + https://www.intel.com/content/www/us/en/developer/articles/technical/usage.html - - Config File Format: - https://01.org/pm-graph/documentation/3-config-file-format + - Feature Summary: + https://www.intel.com/content/www/us/en/developer/topic-technology/open/pm-graph/features.html - upstream version in git: - https://github.com/intel/pm-graph/ + git clone https://github.com/intel/pm-graph/ Table of Contents - Overview diff --git a/tools/power/pm-graph/sleepgraph.8 b/tools/power/pm-graph/sleepgraph.8 index 5126271de98a..643271b6fc6f 100644 --- a/tools/power/pm-graph/sleepgraph.8 +++ b/tools/power/pm-graph/sleepgraph.8 @@ -78,6 +78,9 @@ This helps maintain the consistency of test data for better comparison. If a wifi connection is available, check that it reconnects after resume. Include the reconnect time in the total resume time calculation and treat wifi timeouts as resume failures. +.TP +\fB-wifitrace\fR +Trace through the wifi reconnect time and include it in the timeline. .SS "advanced" .TP diff --git a/tools/power/pm-graph/sleepgraph.py b/tools/power/pm-graph/sleepgraph.py index 33981adcdd68..cfe343306e08 100755 --- a/tools/power/pm-graph/sleepgraph.py +++ b/tools/power/pm-graph/sleepgraph.py @@ -86,7 +86,7 @@ def ascii(text): # store system values and test parameters class SystemValues: title = 'SleepGraph' - version = '5.9' + version = '5.10' ansi = False rs = 0 display = '' @@ -100,6 +100,7 @@ class SystemValues: ftracelog = False acpidebug = True tstat = True + wifitrace = False mindevlen = 0.0001 mincglen = 0.0 cgphase = '' @@ -124,6 +125,7 @@ class SystemValues: epath = '/sys/kernel/debug/tracing/events/power/' pmdpath = '/sys/power/pm_debug_messages' s0ixpath = '/sys/module/intel_pmc_core/parameters/warn_on_s0ix_failures' + s0ixres = '/sys/devices/system/cpu/cpuidle/low_power_idle_system_residency_us' acpipath='/sys/module/acpi/parameters/debug_level' traceevents = [ 'suspend_resume', @@ -180,6 +182,7 @@ class SystemValues: tmstart = 'SUSPEND START %Y%m%d-%H:%M:%S.%f' tmend = 'RESUME COMPLETE %Y%m%d-%H:%M:%S.%f' tracefuncs = { + 'async_synchronize_full': {}, 'sys_sync': {}, 'ksys_sync': {}, '__pm_notifier_call_chain': {}, @@ -304,6 +307,7 @@ class SystemValues: [2, 'suspendstats', 'sh', '-c', 'grep -v invalid /sys/power/suspend_stats/*'], [2, 'cpuidle', 'sh', '-c', 'grep -v invalid /sys/devices/system/cpu/cpu*/cpuidle/state*/s2idle/*'], [2, 'battery', 'sh', '-c', 'grep -v invalid /sys/class/power_supply/*/*'], + [2, 'thermal', 'sh', '-c', 'grep . /sys/class/thermal/thermal_zone*/temp'], ] cgblacklist = [] kprobes = dict() @@ -777,7 +781,7 @@ class SystemValues: return if not quiet: sysvals.printSystemInfo(False) - pprint('INITIALIZING FTRACE...') + pprint('INITIALIZING FTRACE') # turn trace off self.fsetVal('0', 'tracing_on') self.cleanupFtrace() @@ -841,7 +845,7 @@ class SystemValues: for name in self.dev_tracefuncs: self.defaultKprobe(name, self.dev_tracefuncs[name]) if not quiet: - pprint('INITIALIZING KPROBES...') + pprint('INITIALIZING KPROBES') self.addKprobes(self.verbose) if(self.usetraceevents): # turn trace events on @@ -1133,6 +1137,15 @@ class SystemValues: self.cfgdef[file] = fp.read().strip() fp.write(value) fp.close() + def s0ixSupport(self): + if not os.path.exists(self.s0ixres) or not os.path.exists(self.mempowerfile): + return False + fp = open(sysvals.mempowerfile, 'r') + data = fp.read().strip() + fp.close() + if '[s2idle]' in data: + return True + return False def haveTurbostat(self): if not self.tstat: return False @@ -1146,7 +1159,7 @@ class SystemValues: self.vprint(out) return True return False - def turbostat(self): + def turbostat(self, s0ixready): cmd = self.getExec('turbostat') rawout = keyline = valline = '' fullcmd = '%s -q -S echo freeze > %s' % (cmd, self.powerfile) @@ -1173,6 +1186,8 @@ class SystemValues: for key in keyline: idx = keyline.index(key) val = valline[idx] + if key == 'SYS%LPI' and not s0ixready and re.match('^[0\.]*$', val): + continue out.append('%s=%s' % (key, val)) return '|'.join(out) def netfixon(self, net='both'): @@ -1183,14 +1198,6 @@ class SystemValues: out = ascii(fp.read()).strip() fp.close() return out - def wifiRepair(self): - out = self.netfixon('wifi') - if not out or 'error' in out.lower(): - return '' - m = re.match('WIFI \S* ONLINE (?P\S*)', out) - if not m: - return 'dead' - return m.group('action') def wifiDetails(self, dev): try: info = open('/sys/class/net/%s/device/uevent' % dev, 'r').read().strip() @@ -1220,11 +1227,6 @@ class SystemValues: return '%s reconnected %.2f' % \ (self.wifiDetails(dev), max(0, time.time() - start)) time.sleep(0.01) - if self.netfix: - res = self.wifiRepair() - if res: - timeout = max(0, time.time() - start) - return '%s %s %d' % (self.wifiDetails(dev), res, timeout) return '%s timeout %d' % (self.wifiDetails(dev), timeout) def errorSummary(self, errinfo, msg): found = False @@ -1346,6 +1348,20 @@ class SystemValues: for i in self.rslist: self.setVal(self.rstgt, i) pprint('runtime suspend settings restored on %d devices' % len(self.rslist)) + def start(self, pm): + if self.useftrace: + self.dlog('start ftrace tracing') + self.fsetVal('1', 'tracing_on') + if self.useprocmon: + self.dlog('start the process monitor') + pm.start() + def stop(self, pm): + if self.useftrace: + if self.useprocmon: + self.dlog('stop the process monitor') + pm.stop() + self.dlog('stop ftrace tracing') + self.fsetVal('0', 'tracing_on') sysvals = SystemValues() switchvalues = ['enable', 'disable', 'on', 'off', 'true', 'false', '1', '0'] @@ -1643,19 +1659,20 @@ class Data: ubiquitous = False if kprobename in dtf and 'ub' in dtf[kprobename]: ubiquitous = True - title = cdata+' '+rdata - mstr = '\(.*\) *(?P.*) *\((?P.*)\+.* arg1=(?P.*)' - m = re.match(mstr, title) - if m: - c = m.group('caller') - a = m.group('args').strip() - r = m.group('ret') + mc = re.match('\(.*\) *(?P.*)', cdata) + mr = re.match('\((?P\S*).* arg1=(?P.*)', rdata) + if mc and mr: + c = mr.group('caller').split('+')[0] + a = mc.group('args').strip() + r = mr.group('ret') if len(r) > 6: r = '' else: r = 'ret=%s ' % r if ubiquitous and c in dtf and 'ub' in dtf[c]: return False + else: + return False color = sysvals.kprobeColor(kprobename) e = DevFunction(displayname, a, c, r, start, end, ubiquitous, proc, pid, color) tgtdev['src'].append(e) @@ -1772,6 +1789,14 @@ class Data: e.time = self.trimTimeVal(e.time, t0, dT, left) e.end = self.trimTimeVal(e.end, t0, dT, left) e.length = e.end - e.time + if('cpuexec' in d): + cpuexec = dict() + for e in d['cpuexec']: + c0, cN = e + c0 = self.trimTimeVal(c0, t0, dT, left) + cN = self.trimTimeVal(cN, t0, dT, left) + cpuexec[(c0, cN)] = d['cpuexec'][e] + d['cpuexec'] = cpuexec for dir in ['suspend', 'resume']: list = [] for e in self.errorinfo[dir]: @@ -2086,75 +2111,43 @@ class Data: return d def addProcessUsageEvent(self, name, times): # get the start and end times for this process - maxC = 0 - tlast = 0 - start = -1 - end = -1 + cpuexec = dict() + tlast = start = end = -1 for t in sorted(times): - if tlast == 0: + if tlast < 0: tlast = t continue - if name in self.pstl[t]: - if start == -1 or tlast < start: + if name in self.pstl[t] and self.pstl[t][name] > 0: + if start < 0: start = tlast - if end == -1 or t > end: - end = t + end, key = t, (tlast, t) + maxj = (t - tlast) * 1024.0 + cpuexec[key] = min(1.0, float(self.pstl[t][name]) / maxj) tlast = t - if start == -1 or end == -1: - return 0 + if start < 0 or end < 0: + return # add a new action for this process and get the object out = self.newActionGlobal(name, start, end, -3) - if not out: - return 0 - phase, devname = out - dev = self.dmesg[phase]['list'][devname] - # get the cpu exec data - tlast = 0 - clast = 0 - cpuexec = dict() - for t in sorted(times): - if tlast == 0 or t <= start or t > end: - tlast = t - continue - list = self.pstl[t] - c = 0 - if name in list: - c = list[name] - if c > maxC: - maxC = c - if c != clast: - key = (tlast, t) - cpuexec[key] = c - tlast = t - clast = c - dev['cpuexec'] = cpuexec - return maxC + if out: + phase, devname = out + dev = self.dmesg[phase]['list'][devname] + dev['cpuexec'] = cpuexec def createProcessUsageEvents(self): - # get an array of process names - proclist = [] - for t in sorted(self.pstl): - pslist = self.pstl[t] - for ps in sorted(pslist): - if ps not in proclist: - proclist.append(ps) - # get a list of data points for suspend and resume - tsus = [] - tres = [] + # get an array of process names and times + proclist = {'sus': dict(), 'res': dict()} + tdata = {'sus': [], 'res': []} for t in sorted(self.pstl): - if t < self.tSuspended: - tsus.append(t) - else: - tres.append(t) + dir = 'sus' if t < self.tSuspended else 'res' + for ps in sorted(self.pstl[t]): + if ps not in proclist[dir]: + proclist[dir][ps] = 0 + tdata[dir].append(t) # process the events for suspend and resume - if len(proclist) > 0: + if len(proclist['sus']) > 0 or len(proclist['res']) > 0: sysvals.vprint('Process Execution:') - for ps in proclist: - c = self.addProcessUsageEvent(ps, tsus) - if c > 0: - sysvals.vprint('%25s (sus): %d' % (ps, c)) - c = self.addProcessUsageEvent(ps, tres) - if c > 0: - sysvals.vprint('%25s (res): %d' % (ps, c)) + for dir in ['sus', 'res']: + for ps in sorted(proclist[dir]): + self.addProcessUsageEvent(ps, tdata[dir]) def handleEndMarker(self, time, msg=''): dm = self.dmesg self.setEnd(time, msg) @@ -3218,7 +3211,7 @@ class ProcessMonitor: # markers, and/or kprobes required for primary parsing. def doesTraceLogHaveTraceEvents(): kpcheck = ['_cal: (', '_ret: ('] - techeck = ['suspend_resume', 'device_pm_callback'] + techeck = ['suspend_resume', 'device_pm_callback', 'tracing_mark_write'] tmcheck = ['SUSPEND START', 'RESUME COMPLETE'] sysvals.usekprobes = False fp = sysvals.openlog(sysvals.ftracefile, 'r') @@ -3241,7 +3234,7 @@ def doesTraceLogHaveTraceEvents(): check.remove(i) tmcheck = check fp.close() - sysvals.usetraceevents = True if len(techeck) < 2 else False + sysvals.usetraceevents = True if len(techeck) < 3 else False sysvals.usetracemarkers = True if len(tmcheck) == 0 else False # Function: appendIncompleteTraceLog @@ -3456,6 +3449,8 @@ def parseTraceLog(live=False): continue # process cpu exec line if t.type == 'tracing_mark_write': + if t.name == 'CMD COMPLETE' and data.tKernRes == 0: + data.tKernRes = t.time m = re.match(tp.procexecfmt, t.name) if(m): parts, msg = 1, m.group('ps') @@ -3674,6 +3669,9 @@ def parseTraceLog(live=False): e = next((x for x in reversed(tp.ktemp[key]) if x['end'] < 0), 0) if not e: continue + if (t.time - e['begin']) * 1000 < sysvals.mindevlen: + tp.ktemp[key].pop() + continue e['end'] = t.time e['rdata'] = kprobedata # end of kernel resume @@ -4213,6 +4211,8 @@ def callgraphHTML(sv, hf, num, cg, title, color, devid): fmt = '(%.3f ms @ '+sv.timeformat+')' flen = fmt % (line.length*1000, line.time) if line.isLeaf(): + if line.length * 1000 < sv.mincglen: + continue hf.write(html_func_leaf.format(line.name, flen)) elif line.freturn: hf.write(html_func_end) @@ -4827,14 +4827,11 @@ def createHTML(testruns, testfail): if('cpuexec' in dev): for t in sorted(dev['cpuexec']): start, end = t - j = float(dev['cpuexec'][t]) / 5 - if j > 1.0: - j = 1.0 height = '%.3f' % (rowheight/3) top = '%.3f' % (rowtop + devtl.scaleH + 2*rowheight/3) left = '%f' % (((start-m0)*100)/mTotal) width = '%f' % ((end-start)*100/mTotal) - color = 'rgba(255, 0, 0, %f)' % j + color = 'rgba(255, 0, 0, %f)' % dev['cpuexec'][t] devtl.html += \ html_cpuexec.format(left, top, height, width, color) if('src' not in dev): @@ -5453,17 +5450,9 @@ def executeSuspend(quiet=False): call('sync', shell=True) sv.dlog('read dmesg') sv.initdmesg() - # start ftrace - if sv.useftrace: - if not quiet: - pprint('START TRACING') - sv.dlog('start ftrace tracing') - sv.fsetVal('1', 'tracing_on') - if sv.useprocmon: - sv.dlog('start the process monitor') - pm.start() - sv.dlog('run the cmdinfo list before') + sv.dlog('cmdinfo before') sv.cmdinfo(True) + sv.start(pm) # execute however many s/r runs requested for count in range(1,sv.execcount+1): # x2delay in between test runs @@ -5500,6 +5489,7 @@ def executeSuspend(quiet=False): if res != 0: tdata['error'] = 'cmd returned %d' % res else: + s0ixready = sv.s0ixSupport() mode = sv.suspendmode if sv.memmode and os.path.exists(sv.mempowerfile): mode = 'mem' @@ -5509,9 +5499,10 @@ def executeSuspend(quiet=False): sv.testVal(sv.diskpowerfile, 'radio', sv.diskmode) if sv.acpidebug: sv.testVal(sv.acpipath, 'acpi', '0xe') - if mode == 'freeze' and sv.haveTurbostat(): + if ((mode == 'freeze') or (sv.memmode == 's2idle')) \ + and sv.haveTurbostat(): # execution will pause here - turbo = sv.turbostat() + turbo = sv.turbostat(s0ixready) if turbo: tdata['turbo'] = turbo else: @@ -5522,7 +5513,8 @@ def executeSuspend(quiet=False): pf.close() except Exception as e: tdata['error'] = str(e) - sv.dlog('system returned from resume') + sv.fsetVal('CMD COMPLETE', 'trace_marker') + sv.dlog('system returned') # reset everything sv.testVal('restoreall') if(sv.rtcwake): @@ -5535,33 +5527,29 @@ def executeSuspend(quiet=False): sv.fsetVal('WAIT END', 'trace_marker') # return from suspend pprint('RESUME COMPLETE') - sv.fsetVal(datetime.now().strftime(sv.tmend), 'trace_marker') + if(count < sv.execcount): + sv.fsetVal(datetime.now().strftime(sv.tmend), 'trace_marker') + elif(not sv.wifitrace): + sv.fsetVal(datetime.now().strftime(sv.tmend), 'trace_marker') + sv.stop(pm) if sv.wifi and wifi: tdata['wifi'] = sv.pollWifi(wifi) sv.dlog('wifi check, %s' % tdata['wifi']) - if sv.netfix: - netfixout = sv.netfixon('wired') - elif sv.netfix: - netfixout = sv.netfixon() - if sv.netfix and netfixout: - tdata['netfix'] = netfixout + if(count == sv.execcount and sv.wifitrace): + sv.fsetVal(datetime.now().strftime(sv.tmend), 'trace_marker') + sv.stop(pm) + if sv.netfix: + tdata['netfix'] = sv.netfixon() sv.dlog('netfix, %s' % tdata['netfix']) if(sv.suspendmode == 'mem' or sv.suspendmode == 'command'): sv.dlog('read the ACPI FPDT') tdata['fw'] = getFPDT(False) testdata.append(tdata) - sv.dlog('run the cmdinfo list after') + sv.dlog('cmdinfo after') cmdafter = sv.cmdinfo(False) - # stop ftrace - if sv.useftrace: - if sv.useprocmon: - sv.dlog('stop the process monitor') - pm.stop() - sv.fsetVal('0', 'tracing_on') # grab a copy of the dmesg output if not quiet: pprint('CAPTURING DMESG') - sysvals.dlog('EXECUTION TRACE END') sv.getdmesg(testdata) # grab a copy of the ftrace output if sv.useftrace: @@ -6350,6 +6338,8 @@ def data_from_html(file, outpath, issues, fulldetail=False): if not m: continue name, time, phase = m.group('n'), m.group('t'), m.group('p') + if name == 'async_synchronize_full': + continue if ' async' in name or ' sync' in name: name = ' '.join(name.split(' ')[:-1]) if phase.startswith('suspend'): @@ -6701,6 +6691,7 @@ def printHelp(): ' -skiphtml Run the test and capture the trace logs, but skip the timeline (default: disabled)\n'\ ' -result fn Export a results table to a text file for parsing.\n'\ ' -wifi If a wifi connection is available, check that it reconnects after resume.\n'\ + ' -wifitrace Trace kernel execution through wifi reconnect.\n'\ ' -netfix Use netfix to reset the network in the event it fails to resume.\n'\ ' [testprep]\n'\ ' -sync Sync the filesystems before starting the test\n'\ @@ -6828,6 +6819,8 @@ if __name__ == '__main__': sysvals.sync = True elif(arg == '-wifi'): sysvals.wifi = True + elif(arg == '-wifitrace'): + sysvals.wifitrace = True elif(arg == '-netfix'): sysvals.netfix = True elif(arg == '-gzip'): -- cgit v1.2.3 From a51abbbf25317c07cb00b40ae7d04a209d2a3d54 Mon Sep 17 00:00:00 2001 From: Michal Luczaj Date: Thu, 13 Oct 2022 21:12:33 +0000 Subject: KVM: selftests: Add tests in xen_shinfo_test to detect lock races Tests for races between shinfo_cache (de)activation and hypercall+ioctl() processing. KVM has had bugs where activating the shared info cache multiple times and/or with concurrent users results in lock corruption, NULL pointer dereferences, and other fun. For the timer injection testcase (#22), re-arm the timer until the IRQ is successfully injected. If the timer expires while the shared info is deactivated (invalid), KVM will drop the event. Signed-off-by: Michal Luczaj Co-developed-by: Sean Christopherson Signed-off-by: Sean Christopherson Message-Id: <20221013211234.1318131-16-seanjc@google.com> Signed-off-by: Paolo Bonzini --- .../testing/selftests/kvm/x86_64/xen_shinfo_test.c | 140 +++++++++++++++++++++ 1 file changed, 140 insertions(+) (limited to 'tools') diff --git a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c index 8a5cb800f50e..caa3f5ab9e10 100644 --- a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c +++ b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c @@ -15,9 +15,13 @@ #include #include #include +#include #include +/* Defined in include/linux/kvm_types.h */ +#define GPA_INVALID (~(ulong)0) + #define SHINFO_REGION_GVA 0xc0000000ULL #define SHINFO_REGION_GPA 0xc0000000ULL #define SHINFO_REGION_SLOT 10 @@ -44,6 +48,8 @@ #define MIN_STEAL_TIME 50000 +#define SHINFO_RACE_TIMEOUT 2 /* seconds */ + #define __HYPERVISOR_set_timer_op 15 #define __HYPERVISOR_sched_op 29 #define __HYPERVISOR_event_channel_op 32 @@ -148,6 +154,7 @@ static void guest_wait_for_irq(void) static void guest_code(void) { struct vcpu_runstate_info *rs = (void *)RUNSTATE_VADDR; + int i; __asm__ __volatile__( "sti\n" @@ -325,6 +332,49 @@ static void guest_code(void) guest_wait_for_irq(); GUEST_SYNC(21); + /* Racing host ioctls */ + + guest_wait_for_irq(); + + GUEST_SYNC(22); + /* Racing vmcall against host ioctl */ + + ports[0] = 0; + + p = (struct sched_poll) { + .ports = ports, + .nr_ports = 1, + .timeout = 0 + }; + +wait_for_timer: + /* + * Poll for a timer wake event while the worker thread is mucking with + * the shared info. KVM XEN drops timer IRQs if the shared info is + * invalid when the timer expires. Arbitrarily poll 100 times before + * giving up and asking the VMM to re-arm the timer. 100 polls should + * consume enough time to beat on KVM without taking too long if the + * timer IRQ is dropped due to an invalid event channel. + */ + for (i = 0; i < 100 && !guest_saw_irq; i++) + asm volatile("vmcall" + : "=a" (rax) + : "a" (__HYPERVISOR_sched_op), + "D" (SCHEDOP_poll), + "S" (&p) + : "memory"); + + /* + * Re-send the timer IRQ if it was (likely) dropped due to the timer + * expiring while the event channel was invalid. + */ + if (!guest_saw_irq) { + GUEST_SYNC(23); + goto wait_for_timer; + } + guest_saw_irq = false; + + GUEST_SYNC(24); } static int cmp_timespec(struct timespec *a, struct timespec *b) @@ -352,11 +402,36 @@ static void handle_alrm(int sig) TEST_FAIL("IRQ delivery timed out"); } +static void *juggle_shinfo_state(void *arg) +{ + struct kvm_vm *vm = (struct kvm_vm *)arg; + + struct kvm_xen_hvm_attr cache_init = { + .type = KVM_XEN_ATTR_TYPE_SHARED_INFO, + .u.shared_info.gfn = SHINFO_REGION_GPA / PAGE_SIZE + }; + + struct kvm_xen_hvm_attr cache_destroy = { + .type = KVM_XEN_ATTR_TYPE_SHARED_INFO, + .u.shared_info.gfn = GPA_INVALID + }; + + for (;;) { + __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_init); + __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_destroy); + pthread_testcancel(); + }; + + return NULL; +} + int main(int argc, char *argv[]) { struct timespec min_ts, max_ts, vm_ts; struct kvm_vm *vm; + pthread_t thread; bool verbose; + int ret; verbose = argc > 1 && (!strncmp(argv[1], "-v", 3) || !strncmp(argv[1], "--verbose", 10)); @@ -785,6 +860,71 @@ int main(int argc, char *argv[]) case 21: TEST_ASSERT(!evtchn_irq_expected, "Expected event channel IRQ but it didn't happen"); + alarm(0); + + if (verbose) + printf("Testing shinfo lock corruption (KVM_XEN_HVM_EVTCHN_SEND)\n"); + + ret = pthread_create(&thread, NULL, &juggle_shinfo_state, (void *)vm); + TEST_ASSERT(ret == 0, "pthread_create() failed: %s", strerror(ret)); + + struct kvm_irq_routing_xen_evtchn uxe = { + .port = 1, + .vcpu = vcpu->id, + .priority = KVM_IRQ_ROUTING_XEN_EVTCHN_PRIO_2LEVEL + }; + + evtchn_irq_expected = true; + for (time_t t = time(NULL) + SHINFO_RACE_TIMEOUT; time(NULL) < t;) + __vm_ioctl(vm, KVM_XEN_HVM_EVTCHN_SEND, &uxe); + break; + + case 22: + TEST_ASSERT(!evtchn_irq_expected, + "Expected event channel IRQ but it didn't happen"); + + if (verbose) + printf("Testing shinfo lock corruption (SCHEDOP_poll)\n"); + + shinfo->evtchn_pending[0] = 1; + + evtchn_irq_expected = true; + tmr.u.timer.expires_ns = rs->state_entry_time + + SHINFO_RACE_TIMEOUT * 1000000000ULL; + vcpu_ioctl(vcpu, KVM_XEN_VCPU_SET_ATTR, &tmr); + break; + + case 23: + /* + * Optional and possibly repeated sync point. + * Injecting the timer IRQ may fail if the + * shinfo is invalid when the timer expires. + * If the timer has expired but the IRQ hasn't + * been delivered, rearm the timer and retry. + */ + vcpu_ioctl(vcpu, KVM_XEN_VCPU_GET_ATTR, &tmr); + + /* Resume the guest if the timer is still pending. */ + if (tmr.u.timer.expires_ns) + break; + + /* All done if the IRQ was delivered. */ + if (!evtchn_irq_expected) + break; + + tmr.u.timer.expires_ns = rs->state_entry_time + + SHINFO_RACE_TIMEOUT * 1000000000ULL; + vcpu_ioctl(vcpu, KVM_XEN_VCPU_SET_ATTR, &tmr); + break; + case 24: + TEST_ASSERT(!evtchn_irq_expected, + "Expected event channel IRQ but it didn't happen"); + + ret = pthread_cancel(thread); + TEST_ASSERT(ret == 0, "pthread_cancel() failed: %s", strerror(ret)); + + ret = pthread_join(thread, 0); + TEST_ASSERT(ret == 0, "pthread_join() failed: %s", strerror(ret)); goto done; case 0x20: -- cgit v1.2.3 From 5addaf530995ac203fa46efde0d1ded4c15ff98e Mon Sep 17 00:00:00 2001 From: Sean Christopherson Date: Thu, 13 Oct 2022 21:12:34 +0000 Subject: KVM: selftests: Mark "guest_saw_irq" as volatile in xen_shinfo_test Tag "guest_saw_irq" as "volatile" to ensure that the compiler will never optimize away lookups. Relying on the compiler thinking that the flag is global and thus might change also works, but it's subtle, less robust, and looks like a bug at first glance, e.g. risks being "fixed" and breaking the test. Make the flag "static" as well since convincing the compiler it's global is no longer necessary. Alternatively, the flag could be accessed with {READ,WRITE}_ONCE(), but literally every access would need the wrappers, and eking out performance isn't exactly top priority for selftests. Signed-off-by: Sean Christopherson Message-Id: <20221013211234.1318131-17-seanjc@google.com> Signed-off-by: Paolo Bonzini --- tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c index caa3f5ab9e10..2a5727188c8d 100644 --- a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c +++ b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c @@ -132,7 +132,7 @@ struct { struct kvm_irq_routing_entry entries[2]; } irq_routes; -bool guest_saw_irq; +static volatile bool guest_saw_irq; static void evtchn_handler(struct ex_regs *regs) { -- cgit v1.2.3 From bfc3b0f05653a28c8d41067a2aa3875d1f982e3e Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Sun, 9 Oct 2022 20:29:36 +0200 Subject: tools/nolibc: Fix missing strlen() definition and infinite loop with gcc-12 When built at -Os, gcc-12 recognizes an strlen() pattern in nolibc_strlen() and replaces it with a jump to strlen(), which is not defined as a symbol and breaks compilation. Worse, when the function is called strlen(), the function is simply replaced with a jump to itself, hence becomes an infinite loop. One way to avoid this is to always set -ffreestanding, but the calling code doesn't know this and there's no way (either via attributes or pragmas) to globally enable it from include files, effectively leaving a painful situation for the caller. Alexey suggested to place an empty asm() statement inside the loop to stop gcc from recognizing a well-known pattern, which happens to work pretty fine. At least it allows us to make sure our local definition is not replaced with a self jump. The function only needs to be renamed back to strlen() so that the symbol exists, which implies that nolibc_strlen() which is used on variable strings has to be declared as a macro that points back to it before the strlen() macro is redifined. It was verified to produce valid code with gcc 3.4 to 12.1 at different optimization levels, and both with constant and variable strings. In case this problem surfaces again in the future, an alternate approach consisting in adding an optimize("no-tree-loop-distribute-patterns") function attribute for gcc>=12 worked as well but is less pretty. Reported-by: kernel test robot Link: https://lore.kernel.org/r/202210081618.754a77db-yujie.liu@intel.com Fixes: 66b6f755ad45 ("rcutorture: Import a copy of nolibc") Fixes: 96980b833a21 ("tools/nolibc/string: do not use __builtin_strlen() at -O0") Cc: "Paul E. McKenney" Cc: Alexey Dobriyan Signed-off-by: Willy Tarreau Signed-off-by: Paul E. McKenney --- tools/include/nolibc/string.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'tools') diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h index bef35bee9c44..718a405ffbc3 100644 --- a/tools/include/nolibc/string.h +++ b/tools/include/nolibc/string.h @@ -125,14 +125,18 @@ char *strcpy(char *dst, const char *src) } /* this function is only used with arguments that are not constants or when - * it's not known because optimizations are disabled. + * it's not known because optimizations are disabled. Note that gcc 12 + * recognizes an strlen() pattern and replaces it with a jump to strlen(), + * thus itself, hence the asm() statement below that's meant to disable this + * confusing practice. */ static __attribute__((unused)) -size_t nolibc_strlen(const char *str) +size_t strlen(const char *str) { size_t len; - for (len = 0; str[len]; len++); + for (len = 0; str[len]; len++) + asm(""); return len; } @@ -140,13 +144,12 @@ size_t nolibc_strlen(const char *str) * the two branches, then will rely on an external definition of strlen(). */ #if defined(__OPTIMIZE__) +#define nolibc_strlen(x) strlen(x) #define strlen(str) ({ \ __builtin_constant_p((str)) ? \ __builtin_strlen((str)) : \ nolibc_strlen((str)); \ }) -#else -#define strlen(str) nolibc_strlen((str)) #endif static __attribute__((unused)) -- cgit v1.2.3 From b3f4f51ea68a495f8a5956064c33dce711a2df91 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 21 Oct 2022 08:01:53 +0200 Subject: tools/nolibc/string: Fix memcmp() implementation The C standard says that memcmp() must treat the buffers as consisting of "unsigned chars". If char happens to be unsigned, the casts are ok, but then obviously the c1 variable can never contain a negative value. And when char is signed, the casts are wrong, and there's still a problem with using an 8-bit quantity to hold the difference, because that can range from -255 to +255. For example, assuming char is signed, comparing two 1-byte buffers, one containing 0x00 and another 0x80, the current implementation would return -128 for both memcmp(a, b, 1) and memcmp(b, a, 1), whereas one of those should of course return something positive. Signed-off-by: Rasmus Villemoes Fixes: 66b6f755ad45 ("rcutorture: Import a copy of nolibc") Cc: stable@vger.kernel.org # v5.0+ Signed-off-by: Willy Tarreau Signed-off-by: Paul E. McKenney --- tools/include/nolibc/string.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h index 718a405ffbc3..ad97c0d522b8 100644 --- a/tools/include/nolibc/string.h +++ b/tools/include/nolibc/string.h @@ -19,9 +19,9 @@ static __attribute__((unused)) int memcmp(const void *s1, const void *s2, size_t n) { size_t ofs = 0; - char c1 = 0; + int c1 = 0; - while (ofs < n && !(c1 = ((char *)s1)[ofs] - ((char *)s2)[ofs])) { + while (ofs < n && !(c1 = ((unsigned char *)s1)[ofs] - ((unsigned char *)s2)[ofs])) { ofs++; } return c1; -- cgit v1.2.3