Re: [RFC PATCH v12 11/33] KVM: Introduce per-page memory attributes
On 9/21/2023 5:03 AM, Sean Christopherson wrote:
On Mon, Sep 18, 2023, Binbin Wu wrote:
On 9/14/2023 9:55 AM, Sean Christopherson wrote:
From: Chao Peng
[...]
+#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+/*
+ * Returns true if _all_ gfns in the range [@start, @end) have attributes
+ * matching @attrs.
+ */
+bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
+unsigned long attrs)
+{
+ XA_STATE(xas, >mem_attr_array, start);
+ unsigned long index;
+ bool has_attrs;
+ void *entry;
+
+ rcu_read_lock();
+
+ if (!attrs) {
+ has_attrs = !xas_find(, end);
IIUIC, xas_find() is inclusive for "end", so here should be "end - 1" ?
Yes, that does appear to be the case. Inclusive vs. exclusive on gfn ranges has
is the bane of my existence.
Seems this one is not included in the "KVM: guest_memfd fixes" patch series?
https://lore.kernel.org/kvm/20230921203331.3746712-1-sea...@google.com/
Re: linux-next: Tree for Sep 26 (arch/powerpc/platforms/86xx/pic.o)
On 9/25/23 22:50, Stephen Rothwell wrote: > Hi all, > > Changes since 20230925: > on powerpc 32BIT: powerpc-linux-ld: arch/powerpc/platforms/86xx/pic.o: in function `mpc86xx_init_irq': pic.c:(.init.text+0x38): undefined reference to `mpic_alloc' powerpc-linux-ld: pic.c:(.init.text+0x58): undefined reference to `mpic_init' Full randconfig file is attached. -- ~Randy config-r7757.gz Description: application/gzip
Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events
> On 27-Sep-2023, at 4:07 AM, Namhyung Kim wrote:
>
> Hello,
>
> On Mon, Sep 25, 2023 at 10:37 AM Arnaldo Carvalho de Melo
> wrote:
>>
>>
>>
>> On Wed, Sep 13, 2023, 7:40 AM Athira Rajeev
>> wrote:
>>>
>>>
>>>
On 08-Sep-2023, at 7:48 PM, Athira Rajeev
wrote:
> On 08-Sep-2023, at 11:04 AM, Sachin Sant wrote:
>
>
>
>> On 07-Sep-2023, at 10:29 PM, Athira Rajeev
>> wrote:
>>
>> Testcase "Parsing of all PMU events from sysfs" parse events for
>> all PMUs, and not just cpu. In case of powerpc, the PowerVM
>> environment supports events from hv_24x7 and hv_gpci PMU which
>> is of example format like below:
>>
>> - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
>> - hv_gpci/event,partition_id=?/
>>
>> The value for "?" needs to be filled in depending on system
>> configuration. It is better to skip these parametrized events
>> in this test as it is done in:
>> 'commit b50d691e50e6 ("perf test: Fix "all PMU test" to skip
>> parametrized events")' which handled a simialr instance with
>> "all PMU test".
>>
>> Fix parse-events test to skip parametrized events since
>> it needs proper setup of the parameters.
>>
>> Signed-off-by: Athira Rajeev
>> ---
>> Changelog:
>> v1 -> v2:
>> Addressed review comments from Ian. Updated size of
>> pmu event name variable and changed bool name which is
>> used to skip the test.
>>
>
> The patch fixes the reported issue.
>
> 6.2: Parsing of all PMU events from sysfs : Ok
> 6.3: Parsing of given PMU events from sysfs: Ok
>
> Tested-by: Sachin Sant
>
> - Sachin
Hi Sachin, Ian
Thanks for testing the patch
>>>
>>> Hi Arnaldo
>>>
>>> Can you please check and pull this if it looks good to go .
>>
>>
>> Namhyung, can you please take a look?
>
> Yep sure. I think it needs to close the file when getline() fails.
>
> Athira, can you please send v3 with that?
Sure, I will post V3 with this change
Athira
>
> Thanks,
> Namhyung
Re: [PATCH 0/3] Fix for shellcheck issues with version "0.6"
> On 25-Sep-2023, at 1:34 PM, kajoljain wrote: > > > > On 9/7/23 22:45, Athira Rajeev wrote: >> From: root >> >> shellcheck was run on perf tool shell scripts s a pre-requisite >> to include a build option for shellcheck discussed here: >> https://www.spinics.net/lists/linux-perf-users/msg25553.html >> >> And fixes were added for the coding/formatting issues in >> two patchsets: >> https://lore.kernel.org/linux-perf-users/20230613164145.50488-1-atraj...@linux.vnet.ibm.com/ >> https://lore.kernel.org/linux-perf-users/20230709182800.53002-1-atraj...@linux.vnet.ibm.com/ >> >> Three additional issues are observed with shellcheck "0.6" and >> this patchset covers those. With this patchset, >> >> # for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck -S >> warning $F; done >> # echo $? >> 0 >> > > Patchset looks good to me. > > Reviewed-by: Kajol Jain > > Thanks, > Kajol Jain > Hi Namhyunbg, Can you please check for this patchset also Thanks Athira >> Athira Rajeev (3): >> tests/shell: Fix shellcheck SC1090 to handle the location of sourced >>files >> tests/shell: Fix shellcheck issues in tests/shell/stat+shadow_stat.sh >>tetscase >> tests/shell: Fix shellcheck warnings for SC2153 in multiple scripts >> >> tools/perf/tests/shell/coresight/asm_pure_loop.sh| 4 >> tools/perf/tests/shell/coresight/memcpy_thread_16k_10.sh | 4 >> tools/perf/tests/shell/coresight/thread_loop_check_tid_10.sh | 4 >> tools/perf/tests/shell/coresight/thread_loop_check_tid_2.sh | 4 >> tools/perf/tests/shell/coresight/unroll_loop_thread_10.sh| 4 >> tools/perf/tests/shell/probe_vfs_getname.sh | 2 ++ >> tools/perf/tests/shell/record+probe_libc_inet_pton.sh| 2 ++ >> tools/perf/tests/shell/record+script_probe_vfs_getname.sh| 2 ++ >> tools/perf/tests/shell/record.sh | 1 + >> tools/perf/tests/shell/stat+csv_output.sh| 1 + >> tools/perf/tests/shell/stat+csv_summary.sh | 4 ++-- >> tools/perf/tests/shell/stat+shadow_stat.sh | 4 ++-- >> tools/perf/tests/shell/stat+std_output.sh| 1 + >> tools/perf/tests/shell/test_intel_pt.sh | 1 + >> tools/perf/tests/shell/trace+probe_vfs_getname.sh| 1 + >> 15 files changed, 35 insertions(+), 4 deletions(-)
Re: [PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf
> On 27-Sep-2023, at 5:25 AM, Namhyung Kim wrote:
>
> On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev
> wrote:
>>
>> Add rule in new Makefile "tests/Makefile.tests" for running
>> shellcheck on shell test scripts. This automates below shellcheck
>> into the build.
>>
>>$ for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do
>> shellcheck -S warning $F; done
>
> I think you can do it if $(shell command -v shellcheck) returns
> non-empty string (the path to the shellcheck). Then the feature
> test logic can be gone.
Ok, I will try this.
>
>>
>> CONFIG_SHELLCHECK check is added to avoid build breakage in
>> the absence of shellcheck binary. Update Makefile.perf to contain
>> new rule for "SHELLCHECK_TEST" which is for making shellcheck
>> test as a dependency on perf binary. Added "tests/Makefile.tests"
>> to run shellcheck on shellscripts in tests/shell. The make rule
>> "SHLLCHECK_RUN" ensures that, every time during make, shellcheck
>> will be run only on modified files during subsequent invocations.
>> By this, if any newly added shell scripts or fixes in existing
>> scripts breaks coding/formatting style, it will get captured
>> during the perf build.
>
> Can you show me the example output?
Sure, I will add it.
>
>>
>> Signed-off-by: Athira Rajeev
>> ---
>> tools/perf/Makefile.perf| 12 +++-
>> tools/perf/tests/Makefile.tests | 24
>> 2 files changed, 35 insertions(+), 1 deletion(-)
>> create mode 100644 tools/perf/tests/Makefile.tests
>>
>> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
>> index f6fdc2d5a92f..c27f54771e90 100644
>> --- a/tools/perf/Makefile.perf
>> +++ b/tools/perf/Makefile.perf
>> @@ -667,7 +667,16 @@ $(PERF_IN): prepare FORCE
>> $(PMU_EVENTS_IN): FORCE prepare
>>$(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=pmu-events
>> obj=pmu-events
>>
>> -$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN)
>> +# Runs shellcheck on perf test shell scripts
>> +ifeq ($(CONFIG_SHELLCHECK),y)
>> +SHELLCHECK_TEST: FORCE prepare
>> + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests
>> +else
>> +SHELLCHECK_TEST:
>> + @:
>> +endif
>> +
>> +$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN) SHELLCHECK_TEST
>>$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) \
>>$(PERF_IN) $(PMU_EVENTS_IN) $(LIBS) -o $@
>>
>> @@ -1129,6 +1138,7 @@ bpf-skel-clean:
>>$(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
>>
>> clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean
>> $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean
>> tests-coresight-targets-clean
>> + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
>>$(call QUIET_CLEAN, core-objs) $(RM) $(LIBPERF_A)
>> $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
>>$(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd'
>> -delete -o -name '\.*.d' -delete
>>$(Q)$(RM) $(OUTPUT).config-detected
>> diff --git a/tools/perf/tests/Makefile.tests
>> b/tools/perf/tests/Makefile.tests
>> new file mode 100644
>> index ..e74575559e83
>> --- /dev/null
>> +++ b/tools/perf/tests/Makefile.tests
>> @@ -0,0 +1,24 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Athira Rajeev , 2023
>> +-include $(OUTPUT).config-detected
>> +
>> +log_file = $(OUTPUT)shellcheck_test.log
>> +PROGS = $(subst ./,,$(shell find tests/shell -perm -o=x -type f -name
>> '*.sh'))
>> +DEPS = $(addprefix output/,$(addsuffix .dep,$(basename $(PROGS
>> +DIRS = $(shell echo $(dir $(DEPS)) | xargs -n1 | sort -u | xargs)
>> +
>> +.PHONY: all
>> +all: SHELLCHECK_RUN
>> + @:
>> +
>> +SHELLCHECK_RUN: $(DEPS) $(DIRS)
>> +
>> +output/%.dep: %.sh | $(DIRS)
>> + $(call rule_mkdir)
>> + $(Q)$(call frecho-cmd,test)@touch $@
>> + $(Q)$(call frecho-cmd,test)@shellcheck -S warning $(subst
>> output/,./,$(patsubst %.dep, %.sh, $@)) 1> ${log_file} && ([[ ! -s
>> ${log_file} ]])
>
> This line is too long, please wrap it with some backslashes.
Ok
I will address all the comments in next version
Thanks
Athira
>
> Thanks,
> Namhyung
>
>
>> +$(DIRS):
>> + @mkdir -p $@
>> +
>> +clean:
>> + @rm -rf $(log_file) output
>> --
>> 2.31.1
Re: [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary
> On 27-Sep-2023, at 5:21 AM, Namhyung Kim wrote: > > Hello, > > On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev > wrote: >> >> shellcheck tool can detect coding/formatting issues on >> shell scripts. In perf directory "tests/shell", there are lot >> of shell test scripts and this tool can detect coding/formatting >> issues on these scripts. >> >> Example to use shellcheck for severity level for >> errors and warnings, below command is used: >> >> # for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck -S >> warning $F; done >> # echo $? >> 0 >> >> This testing needs to be automated into the build so that it >> can avoid regressions and also run the check for newly added >> during build test itself. Add a new feature check to detect >> presence of shellcheck. Add CONFIG_SHELLCHECK feature check in >> the build to avoid not having shellcheck breaking the build. >> >> Signed-off-by: Athira Rajeev >> --- >> tools/build/Makefile.feature | 6 -- >> tools/build/feature/Makefile | 8 +++- >> tools/perf/Makefile.config | 10 ++ >> 3 files changed, 21 insertions(+), 3 deletions(-) >> >> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature >> index 934e2777a2db..23f56b95babf 100644 >> --- a/tools/build/Makefile.feature >> +++ b/tools/build/Makefile.feature >> @@ -72,7 +72,8 @@ FEATURE_TESTS_BASIC := \ >> libzstd\ >> disassembler-four-args \ >> disassembler-init-styled \ >> -file-handle >> +file-handle\ >> +shellcheck >> >> # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list >> # of all feature tests >> @@ -138,7 +139,8 @@ FEATURE_DISPLAY ?= \ >> get_cpuid \ >> bpf \ >> libaio\ >> - libzstd >> + libzstd \ >> + shellcheck >> >> # >> # Declare group members of a feature to display the logical OR of the >> detection >> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile >> index 3184f387990a..44ba6d0c98d0 100644 >> --- a/tools/build/feature/Makefile >> +++ b/tools/build/feature/Makefile >> @@ -76,7 +76,8 @@ FILES= \ >> test-libzstd.bin \ >> test-clang-bpf-co-re.bin \ >> test-file-handle.bin \ >> - test-libpfm4.bin >> + test-libpfm4.bin \ >> + test-shellcheck.bin >> >> FILES := $(addprefix $(OUTPUT),$(FILES)) >> >> @@ -92,6 +93,8 @@ __BUILD = $(CC) $(CFLAGS) -MD -Wall -Werror -o $@ >> $(patsubst %.bin,%.c,$(@F)) $( >> __BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall -Werror -o $@ $(patsubst >> %.bin,%.cpp,$(@F)) $(LDFLAGS) >> BUILDXX = $(__BUILDXX) > $(@:.bin=.make.output) 2>&1 >> >> + BUILD_BINARY = sh -c $1 > $(@:.bin=.make.output) 2>&1 >> + >> ### >> >> $(OUTPUT)test-all.bin: >> @@ -207,6 +210,9 @@ $(OUTPUT)test-libslang-include-subdir.bin: >> $(OUTPUT)test-libtraceevent.bin: >>$(BUILD) -ltraceevent >> >> +$(OUTPUT)test-shellcheck.bin: >> + $(BUILD_BINARY) "shellcheck --version" > > I don't think it'd generate the .bin file. > > Anyway, it's a binary file already. Can we check it with > `command -v` and get rid of the feature test? > > Thanks, > Namhyung Hi Namhyung, Thanks for the review. Sure, I will check on this Athira > > >> + >> $(OUTPUT)test-libtracefs.bin: >> $(BUILD) $(shell $(PKG_CONFIG) --cflags libtraceevent 2>/dev/null) >> -ltracefs >> >> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config >> index d66b52407e19..e71fe95ad865 100644 >> --- a/tools/perf/Makefile.config >> +++ b/tools/perf/Makefile.config >> @@ -779,6 +779,16 @@ ifndef NO_SLANG >> endif >> endif >> >> +ifneq ($(NO_SHELLCHECK),1) >> + $(call feature_check,shellcheck) >> + ifneq ($(feature-shellcheck), 1) >> +msg := $(warning No shellcheck found. please install ShellCheck); >> + else >> +$(call detected,CONFIG_SHELLCHECK) >> +NO_SHELLCHECK := 0 >> + endif >> +endif >> + >> ifdef GTK2 >> FLAGS_GTK2=$(CFLAGS) $(LDFLAGS) $(EXTLIBS) $(shell $(PKG_CONFIG) --libs >> --cflags gtk+-2.0 2>/dev/null) >> $(call feature_check,gtk2) >> -- >> 2.31.1
Re: Questions: Should kernel panic when PCIe fatal error occurs?
On Wed, Sep 27, 2023 at 9:03 AM Bjorn Helgaas wrote: > > On Fri, Sep 22, 2023 at 10:46:36AM +0800, Shuai Xue wrote: > > ... > > > Actually, this is a question from my colleague from firmware team. > > The original question is that: > > > > "Should I set CPER_SEV_FATAL for Generic Error Status Block when a > > PCIe fatal error is detected? If set, kernel will always panic. > > Otherwise, kernel will always not panic." > > > > So I pull a question about desired behavior of Linux kernel first :) > > From the perspective of the kernel, CPER_SEV_FATAL for Generic Error > > Status Block is not reasonable. The kernel will attempt to recover > > Fatal errors, although recovery may fail. > > I don't know the semantics of CPER_SEV_FATAL or why it's there. > With CPER, we have *two* error severities: a "native" one defined by > the PCIe spec and another defined by the platform via CPER. > > I speculate that the reason for the CPER severity could be to provide > a severity for error sources that don't have a "native" severity like > AER does, or for the vendor to force the OS to restart (for > CPER_SEV_FATAL, anyway) in cases where it might not otherwise. > > In the native case, we only have the PCIe severity and don't have the > CPER severity at all, and I suspect that unless there's uncontained > data corruption, we would rather handle even the most severe PCIe > fatal error by disabling the specific device(s) instead of panicking > and restarting the whole machine. >From a user's point of view disabling a device is often worse than a reboot. If you get a fatal error from a system's only network card then disabling the card may result in the system being uncontactable until someone manually recovers it. Similarly, if the disk hosting the root filesystem disappears the system may not crash immediately (most of what it needs will be in page cache), but there's no guarantee that it can do anything useful in that state. In both cases forcing a reboot will probably bring the system back into a usable state. > So for PCIe errors, I'm not sure setting CPER_SEV_FATAL is beneficial > unless the platform wants to force the OS to panic, e.g., maybe the > platform knows about data corruption and/or the vendor wants the OS to > panic as part of a reliability story. The PCIe spec is (intentionally?) vague about the causes of fatal errors. For all we know a device is reporting one because the embedded OS it was running crashed and as a side effect it's been DMAing junk into system memory for the past hour. If you know something about the device in question maybe you can make those assumptions, but in general it's impossible to assess the actual severity of an error. Even in the case of a noisy link causing bit flips (it's possible, LCRC is only 16bit and ECEC is optional) if we get corruption of the address bits of the TLP header then the DMA might have overwritten something important to the OS. From a hardware vendor's point of view just forcing a reboot makes a lot of sense. Paranoia aside, in a lot of cases PCI device errors are nothing major and can be resolved by just restarting the device. However, there's no way for generic kernel code to make that assessment and we probably shouldn't have the kernel guess. I'd say the safest option would be to punt that decision to userspace and provide some way to whitelist devices that we can ignore errors from. I'm not familiar enough with the ACPI to know if we have enough details in the notification it sends to actually implement that though. Oliver
[PATCH] powerpc/pseries/vas: Migration suspend waits for no in-progress open windows
The hypervisor returns migration failure if all VAS windows are not
closed. During pre-migration stage, vas_migration_handler() sets
migration_in_progress flag and closes all windows from the list.
The allocate VAS window routine checks the migration flag, setup
the window and then add it to the list. So there is possibility of
the migration handler missing the window that is still in the
process of setup.
t1: Allocate and open VAS t2: Migration event
window
lock vas_pseries_mutex
If migration_in_progress set
unlock vas_pseries_mutex
return
open window HCALL
unlock vas_pseries_mutex
Modify window HCALL lock vas_pseries_mutex
setup windowmigration_in_progress=true
Closes all windows from
the list
unlock vas_pseries_mutex
lock vas_pseries_mutex return
if nr_closed_windows == 0
// No DLPAR CPU or migration
add to the list
unlock vas_pseries_mutex
return
unlock vas_pseries_mutex
Close VAS window
// due to DLPAR CPU or migration
return -EBUSY
This patch resolves the issue with the following steps:
- Define migration_in_progress as atomic so that the migration
handler sets this flag without holding mutex.
- Introduce nr_open_wins_progress counter in VAS capabilities
struct
- This counter tracks the number of open windows are still in
progress
- The allocate setup window thread closes windows if the migration
is set and decrements nr_open_window_progress counter
- The migration handler waits for no in-progress open windows.
Fixes: 37e6764895ef ("powerpc/pseries/vas: Add VAS migration handler")
Signed-off-by: Haren Myneni
---
arch/powerpc/platforms/pseries/vas.c | 51 ++--
arch/powerpc/platforms/pseries/vas.h | 2 ++
2 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/vas.c
b/arch/powerpc/platforms/pseries/vas.c
index 15d958e38eca..efdaf12ffe49 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -31,7 +31,8 @@ static struct hv_vas_cop_feat_caps hv_cop_caps;
static struct vas_caps vascaps[VAS_MAX_FEAT_TYPE];
static DEFINE_MUTEX(vas_pseries_mutex);
-static bool migration_in_progress;
+static atomic_t migration_in_progress = ATOMIC_INIT(0);
+static DECLARE_WAIT_QUEUE_HEAD(open_win_progress_wq);
static long hcall_return_busy_check(long rc)
{
@@ -384,11 +385,15 @@ static struct vas_window *vas_allocate_window(int vas_id,
u64 flags,
* same fault IRQ is not freed by the OS before.
*/
mutex_lock(_pseries_mutex);
- if (migration_in_progress)
+ if (atomic_read(_in_progress)) {
rc = -EBUSY;
- else
+ } else {
rc = allocate_setup_window(txwin, (u64 *)[0],
cop_feat_caps->win_type);
+ if (!rc)
+ atomic_inc(>nr_open_wins_progress);
+ }
+
mutex_unlock(_pseries_mutex);
if (rc)
goto out;
@@ -403,8 +408,17 @@ static struct vas_window *vas_allocate_window(int vas_id,
u64 flags,
goto out_free;
txwin->win_type = cop_feat_caps->win_type;
- mutex_lock(_pseries_mutex);
+
/*
+* The migration SUSPEND thread sets migration_in_progress and
+* closes all open windows from the list. But the window is
+* added to the list after open and modify HCALLs. So possible
+* that migration_in_progress is set before modify HCALL which
+* may cause some windows are still open when the hypervisor
+* initiates the migration.
+* So checks the migration_in_progress flag again and close all
+* open windows.
+*
* Possible to lose the acquired credit with DLPAR core
* removal after the window is opened. So if there are any
* closed windows (means with lost credits), do not give new
@@ -412,9 +426,11 @@ static struct vas_window *vas_allocate_window(int vas_id,
u64 flags,
* after the existing windows are reopened when credits are
* available.
*/
- if (!caps->nr_close_wins) {
+ mutex_lock(_pseries_mutex);
+ if (!caps->nr_close_wins && !atomic_read(_in_progress)) {
list_add(>win_list, >list);
caps->nr_open_windows++;
+ atomic_dec(>nr_open_wins_progress);
mutex_unlock(_pseries_mutex);
vas_user_win_add_mm_context(>vas_win.task_ref);
return >vas_win;
@@ -432,6 +448,8 @@ static struct vas_window *vas_allocate_window(int vas_id,
u64 flags,
*/
free_irq_setup(txwin);
h_deallocate_vas_window(txwin->vas_win.winid);
+ atomic_dec(>nr_open_wins_progress);
+ wake_up(_win_progress_wq);
out:
atomic_dec(_feat_caps->nr_used_credits);
kfree(txwin);
@@ -936,18 +954,18 @@ int
[PATCH] powerpc/pseries/vas: Migration suspend waits for no in-progress open windows
The hypervisor returns migration failure if all VAS windows are not
closed. During pre-migration stage, vas_migration_handler() sets
migration_in_progress flag and closes all windows from the list.
The allocate VAS window routine checks the migration flag, setup
the window and then add it to the list. So there is possibility of
the migration handler missing the window that is still in the
process of setup.
t1: Allocate and open VAS t2: Migration event
window
lock vas_pseries_mutex
If migration_in_progress set
unlock vas_pseries_mutex
return
open window HCALL
unlock vas_pseries_mutex
Modify window HCALL lock vas_pseries_mutex
setup windowmigration_in_progress=true
Closes all windows from
the list
unlock vas_pseries_mutex
lock vas_pseries_mutex return
if nr_closed_windows == 0
// No DLPAR CPU or migration
add to the list
unlock vas_pseries_mutex
return
unlock vas_pseries_mutex
Close VAS window
// due to DLPAR CPU or migration
return -EBUSY
This patch resolves the issue with the following steps:
- Define migration_in_progress as atomic so that the migration
handler sets this flag without holding mutex.
- Introduce nr_open_wins_progress counter in VAS capabilities
struct
- This counter tracks the number of open windows are still in
progress
- The allocate setup window thread closes windows if the migration
is set and decrements nr_open_window_progress counter
- The migration handler waits for no in-progress open windows.
Fixes: 37e6764895ef ("powerpc/pseries/vas: Add VAS migration handler")
Signed-off-by: Haren Myneni
---
arch/powerpc/platforms/pseries/vas.c | 51 ++--
arch/powerpc/platforms/pseries/vas.h | 2 ++
2 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/vas.c
b/arch/powerpc/platforms/pseries/vas.c
index 15d958e38eca..efdaf12ffe49 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -31,7 +31,8 @@ static struct hv_vas_cop_feat_caps hv_cop_caps;
static struct vas_caps vascaps[VAS_MAX_FEAT_TYPE];
static DEFINE_MUTEX(vas_pseries_mutex);
-static bool migration_in_progress;
+static atomic_t migration_in_progress = ATOMIC_INIT(0);
+static DECLARE_WAIT_QUEUE_HEAD(open_win_progress_wq);
static long hcall_return_busy_check(long rc)
{
@@ -384,11 +385,15 @@ static struct vas_window *vas_allocate_window(int vas_id,
u64 flags,
* same fault IRQ is not freed by the OS before.
*/
mutex_lock(_pseries_mutex);
- if (migration_in_progress)
+ if (atomic_read(_in_progress)) {
rc = -EBUSY;
- else
+ } else {
rc = allocate_setup_window(txwin, (u64 *)[0],
cop_feat_caps->win_type);
+ if (!rc)
+ atomic_inc(>nr_open_wins_progress);
+ }
+
mutex_unlock(_pseries_mutex);
if (rc)
goto out;
@@ -403,8 +408,17 @@ static struct vas_window *vas_allocate_window(int vas_id,
u64 flags,
goto out_free;
txwin->win_type = cop_feat_caps->win_type;
- mutex_lock(_pseries_mutex);
+
/*
+* The migration SUSPEND thread sets migration_in_progress and
+* closes all open windows from the list. But the window is
+* added to the list after open and modify HCALLs. So possible
+* that migration_in_progress is set before modify HCALL which
+* may cause some windows are still open when the hypervisor
+* initiates the migration.
+* So checks the migration_in_progress flag again and close all
+* open windows.
+*
* Possible to lose the acquired credit with DLPAR core
* removal after the window is opened. So if there are any
* closed windows (means with lost credits), do not give new
@@ -412,9 +426,11 @@ static struct vas_window *vas_allocate_window(int vas_id,
u64 flags,
* after the existing windows are reopened when credits are
* available.
*/
- if (!caps->nr_close_wins) {
+ mutex_lock(_pseries_mutex);
+ if (!caps->nr_close_wins && !atomic_read(_in_progress)) {
list_add(>win_list, >list);
caps->nr_open_windows++;
+ atomic_dec(>nr_open_wins_progress);
mutex_unlock(_pseries_mutex);
vas_user_win_add_mm_context(>vas_win.task_ref);
return >vas_win;
@@ -432,6 +448,8 @@ static struct vas_window *vas_allocate_window(int vas_id,
u64 flags,
*/
free_irq_setup(txwin);
h_deallocate_vas_window(txwin->vas_win.winid);
+ atomic_dec(>nr_open_wins_progress);
+ wake_up(_win_progress_wq);
out:
atomic_dec(_feat_caps->nr_used_credits);
kfree(txwin);
@@ -936,18 +954,18 @@ int
RE: Re: [PATCH v2 1/2] ASoC: dt-bindings: fsl_rpmsg: List DAPM endpoints ignoring system suspend
> > + fsl,lpa-widgets: > > +$ref: /schemas/types.yaml#/definitions/non-unique-string-array > > +description: | > > + A list of DAPM endpoints which mark paths between these > endpoints should > > + not be disabled when system enters in suspend state. LPA means low power > > + audio case. On asymmetric multiprocessor, there are Cortex-A core > and > > + Cortex-M core, Linux is running on Cortex-A core, RTOS or other OS is > > + running on Cortex-M core. The audio hardware devices can be > controlled by > > + Cortex-M. LPA can be explained as a mechanism that Cortex-A > allocates a > > + large buffer and fill audio data, then Cortex-A can enter into > > suspend > > + for the purpose of power saving. Cortex-M continues to play the > sound > > + during suspend phase of Cortex-A. When the data in buffer is > consumed, > > + Cortex-M will trigger the Cortex-A to wakeup to fill data. LPA > requires > > + some audio paths still enabled when Cortex-A enters into suspend. > > This is a fairly standard DSP playback case as far as I can see so it > should work with DAPM without needing this obviously use case specific > stuff peering into the Linux implementation. Generally this is done by > tagging endpoint widgets and DAIs as ignore_suspend, DAPM will then > figure out the rest of the widgets in the path. Yes, indeed I meant to let driver get DAPM endpoints from the "fsl,lpa-widgets" property and then set these endpoints as ignore_suspend if the sound card is running in this use case. Do you think the description for the use case can be simplified since it's a common use case? Regards, Chancel Liu
Re: Questions: Should kernel panic when PCIe fatal error occurs?
On 2023/9/27 07:02, Bjorn Helgaas wrote: > On Fri, Sep 22, 2023 at 10:46:36AM +0800, Shuai Xue wrote: >> ... > >> Actually, this is a question from my colleague from firmware team. >> The original question is that: >> >> "Should I set CPER_SEV_FATAL for Generic Error Status Block when a >> PCIe fatal error is detected? If set, kernel will always panic. >> Otherwise, kernel will always not panic." >> >> So I pull a question about desired behavior of Linux kernel first :) >> From the perspective of the kernel, CPER_SEV_FATAL for Generic Error >> Status Block is not reasonable. The kernel will attempt to recover >> Fatal errors, although recovery may fail. > > I don't know the semantics of CPER_SEV_FATAL or why it's there. > With CPER, we have *two* error severities: a "native" one defined by > the PCIe spec and another defined by the platform via CPER. > > I speculate that the reason for the CPER severity could be to provide > a severity for error sources that don't have a "native" severity like > AER does, or for the vendor to force the OS to restart (for > CPER_SEV_FATAL, anyway) in cases where it might not otherwise. Agreed, it is the key point. Per ACPI 6.5 18.1 Hardware Errors and Error Sources[1]: - An uncorrected error is a hardware error condition that cannot be corrected by the hardware or by the firmware. Uncorrected errors are either fatal or non-fatal. - A fatal hardware error is an uncorrected or uncontained error condition that is determined to be unrecoverable by the hardware. When a fatal uncorrected error occurs, the system is restarted to prevent propagation of the error. A non-fatal hardware error is an uncorrected error condition from which OSPM can attempt recovery by trying to correct the error. These are also referred to as correctable or recoverable errors. Based on our discussion and the PCIe and APCI Spec: - Native AER fatal error defined in PCIe does not indate that there's uncontained data corruption. - The kernel is capable of handle native AER fatal and non-fatal errors. - When a CPER_SEV_FATAL error nofitied by firmware, it indicates the platform wants to force the OS to restart, and the APEI/GHES driver follows the Spec now. (Please correct me if I misunderstand any) > > In the native case, we only have the PCIe severity and don't have the > CPER severity at all, and I suspect that unless there's uncontained > data corruption, we would rather handle even the most severe PCIe > fatal error by disabling the specific device(s) instead of panicking > and restarting the whole machine. > > So for PCIe errors, I'm not sure setting CPER_SEV_FATAL is beneficial > unless the platform wants to force the OS to panic, e.g., maybe the > platform knows about data corruption and/or the vendor wants the OS to > panic as part of a reliability story. So back to the original question, I think your above comments are clear enough. > > Presumably the platform has already logged the error, and I assume the > platform *could* restart without even returning to the OS, but maybe > it wants the OS to do a crashdump or shutdown in a more orderly way. > If the system is reset in platform without even returning to the OS, it is not visible to end user. IMHO, it always a bad choice. The OS can provide enhanced debuggability, for example: - providing details about the runtime context through crashdump - saving error information to persistent storage Thank you for your patience and valuable feedback. It is greatly appreciated and truly helpful. Best Regards and Cheers. Shuai [1] https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#hardware-errors-and-error-sources
Re: [PATCH 00/40] soc: Convert to platform remove callback returning void
On Mon, 25 Sept 2023 at 09:55, Uwe Kleine-König
wrote:
>
> Hello,
>
> this series converts all platform drivers below drivers/soc to use
> .remove_new(). The motivation is to get rid of an integer return code
> that is (mostly) ignored by the platform driver core and error prone on
> the driver side.
>
> See commit 5c5a7680e67b ("platform: Provide a remove callback that
> returns no value") for an extended explanation and the eventual goal.
>
> As there is no single maintainer team for drivers/soc, I suggest the
> individual maintainers to pick up "their" patches.
I'd be happy if Arnd merged the lot at once. Arnd, what do you think?
If that will be too messy then I understand. I have queued the aspeed
ones locally and will push that out if we decide that's the best way
to go.
Thanks for doing this cleanup Uwe.
Cheers,
Joel
Re: [PATCH V4 2/2] tools/perf/tests: Fix object code reading to skip address that falls out of text section
On Thu, Sep 14, 2023 at 10:40 PM Athira Rajeev
wrote:
>
> The testcase "Object code reading" fails in somecases
> for "fs_something" sub test as below:
>
> Reading object code for memory address: 0xc00807f0142c
> File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko
> On file address is: 0x1114cc
> Objdump command is: objdump -z -d --start-address=0x11142c
> --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko
> objdump read too few bytes: 128
> test child finished with -1
>
> This can alo be reproduced when running perf record with
> workload that exercises fs_something() code. In the test
> setup, this is exercising xfs code since root is xfs.
>
> # perf record ./a.out
> # perf report -v |grep "xfs.ko"
> 0.76% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko
> 0xc00807de5efc B [k] xlog_cil_commit
> 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko
> 0xc00807d5ae18 B [k] xfs_btree_key_offset
> 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko
> 0xc00807e11fd4 B [k] 0x00112074
>
> Here addr "0xc00807e11fd4" is not resolved. since this is a
> kernel module, its offset is from the DSO. Xfs module is loaded
> at 0xc00807d0
>
># cat /proc/modules | grep xfs
> xfs 2228224 3 - Live 0xc00807d0
>
> And size is 0x22. So its loaded between 0xc00807d0
> and 0xc00807f2. From objdump, text section is:
> text 0010f7bc 00a0 2**4
>
> Hence perf captured ip maps to 0x112074 which is:
> ( ip - start of module ) + a0
>
> This offset 0x112074 falls out .text section which is up to 0x10f7bc
> In this case for module, the address 0xc00807e11fd4 is pointing
> to stub instructions. This address range represents the module stubs
> which is allocated on module load and hence is not part of DSO offset.
>
> To address this issue in "object code reading", skip the sample if
> address falls out of text section and is within the module end.
> Use the "text_end" member of "struct dso" to do this check.
>
> To address this issue in "perf report", exploring an option of
> having stubs range as part of the /proc/kallsyms, so that perf
> report can resolve addresses in stubs range
>
> However this patch uses text_end to skip the stub range for
> Object code reading testcase.
>
> Reported-by: Disha Goel
> Signed-off-by: Athira Rajeev
> Tested-by: Disha Goel
> Reviewed-by: Adrian Hunter
> ---
> Changelog:
> v3 -> v4:
> Fixed indent in V3
>
> v2 -> v3:
> Used strtailcmp in comparison for module check and added Reviewed-by
> from Adrian, Tested-by from Disha.
>
> v1 -> v2:
> Updated comment to add description on which arch has stub and
> reason for skipping as suggested by Adrian
>
> tools/perf/tests/code-reading.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index ed3815163d1b..9e6e6c985840 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -269,6 +269,16 @@ static int read_object_code(u64 addr, size_t len, u8
> cpumode,
> if (addr + len > map__end(al.map))
> len = map__end(al.map) - addr;
>
> + /*
> +* Some architectures (ex: powerpc) have stubs (trampolines) in kernel
> +* modules to manage long jumps. Check if the ip offset falls in stubs
> +* sections for kernel modules. And skip module address after text end
> +*/
> + if (!strtailcmp(dso->long_name, ".ko") && al.addr > dso->text_end) {
There's a is_kernel_module() that can check compressed modules
too but I think we need a simpler way to check it like dso->kernel.
Thanks,
Namhyung
> + pr_debug("skipping the module address %#"PRIx64" after text
> end\n", al.addr);
> + goto out;
> + }
> +
> /* Read the object code using perf */
> ret_len = dso__data_read_offset(dso,
> maps__machine(thread__maps(thread)),
> al.addr, buf1, len);
> --
> 2.31.1
>
Re: [PATCH v10 13/15] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler
Terry Bowman wrote: > Hi Dan, > > On 8/31/23 15:35, Dan Williams wrote: > > Terry Bowman wrote: > >> From: Robert Richter > >> > >> In Restricted CXL Device (RCD) mode a CXL device is exposed as an > >> RCiEP, but CXL downstream and upstream ports are not enumerated and > >> not visible in the PCIe hierarchy. [1] Protocol and link errors from > >> these non-enumerated ports are signaled as internal AER errors, either > >> Uncorrectable Internal Error (UIE) or Corrected Internal Errors (CIE) > >> via an RCEC. > >> > >> Restricted CXL host (RCH) downstream port-detected errors have the > >> Requester ID of the RCEC set in the RCEC's AER Error Source ID > >> register. A CXL handler must then inspect the error status in various > >> CXL registers residing in the dport's component register space (CXL > >> RAS capability) or the dport's RCRB (PCIe AER extended > >> capability). [2] > >> > >> Errors showing up in the RCEC's error handler must be handled and > >> connected to the CXL subsystem. Implement this by forwarding the error > >> to all CXL devices below the RCEC. Since the entire CXL device is > >> controlled only using PCIe Configuration Space of device 0, function > >> 0, only pass it there [3]. The error handling is limited to currently > >> supported devices with the Memory Device class code set (CXL Type 3 > >> Device, PCI_CLASS_MEMORY_CXL, 502h), handle downstream port errors in > >> the device's cxl_pci driver. Support for other CXL Device Types > >> (e.g. a CXL.cache Device) can be added later. > >> > >> To handle downstream port errors in addition to errors directed to the > >> CXL endpoint device, a handler must also inspect the CXL RAS and PCIe > >> AER capabilities of the CXL downstream port the device is connected > >> to. > >> > >> Since CXL downstream port errors are signaled using internal errors, > >> the handler requires those errors to be unmasked. This is subject of a > >> follow-on patch. > >> > >> The reason for choosing this implementation is that the AER service > >> driver claims the RCEC device, but does not allow it to register a > >> custom specific handler to support CXL. Connecting the RCEC hard-wired > >> with a CXL handler does not work, as the CXL subsystem might not be > >> present all the time. The alternative to add an implementation to the > >> portdrv to allow the registration of a custom RCEC error handler isn't > >> worth doing it as CXL would be its only user. Instead, just check for > >> an CXL RCEC and pass it down to the connected CXL device's error > >> handler. With this approach the code can entirely be implemented in > >> the PCIe AER driver and is independent of the CXL subsystem. The CXL > >> driver only provides the handler. > >> > >> [1] CXL 3.0 spec: 9.11.8 CXL Devices Attached to an RCH > >> [2] CXL 3.0 spec, 12.2.1.1 RCH Downstream Port-detected Errors > >> [3] CXL 3.0 spec, 8.1.3 PCIe DVSEC for CXL Devices > >> > >> Co-developed-by: Terry Bowman > >> Signed-off-by: Terry Bowman > >> Signed-off-by: Robert Richter > >> Cc: "Oliver O'Halloran" > >> Cc: Bjorn Helgaas > >> Cc: linuxppc-dev@lists.ozlabs.org > >> Cc: linux-...@vger.kernel.org > >> Acked-by: Bjorn Helgaas > >> Reviewed-by: Jonathan Cameron > >> Reviewed-by: Dave Jiang > >> --- > >> drivers/pci/pcie/Kconfig | 12 + > >> drivers/pci/pcie/aer.c | 96 +++- > >> 2 files changed, 106 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig > >> index 228652a59f27..4f0e70fafe2d 100644 > >> --- a/drivers/pci/pcie/Kconfig > >> +++ b/drivers/pci/pcie/Kconfig > >> @@ -49,6 +49,18 @@ config PCIEAER_INJECT > >> gotten from: > >> > >> https://git.kernel.org/cgit/linux/kernel/git/gong.chen/aer-inject.git/ > >> > >> +config PCIEAER_CXL > >> + bool "PCI Express CXL RAS support for Restricted Hosts (RCH)" > > > > Why the "for Restricted Hosts (RCH)" clarification? I am seeing nothing > > that prevents this from working with RCECs on VH topologies. > > > > The same option can be used in VH mode. Will remove the RCH reference. > > > > >> + default y > > > > Minor, but I think "default PCIEAER" makes it slightly clearer that CXL > > error handling comes along for the ride with PCIE AER. > > > > We found Kconfig entries do not typically list a dependancy and the default > to be the same. We prefer to leave as 'default y'. If you want we can make > your requested change. The tie breaker would be to follow whatever the local precedent is. Indeed, it looks like "depends with default y" is consistent with other drivers/pci/pcie/Kconfig entries. So I retract my comment. > > >> + depends on PCIEAER && CXL_PCI > >> + help > >> +Enables error handling of downstream ports of a CXL host > >> +that is operating in RCD mode (Restricted CXL Host, RCH). > >> +The downstream port reports AER errors to a given RCEC. > >> +Errors are handled by the CXL memory device driver. > >> +
Re: [PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf
On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev
wrote:
>
> Add rule in new Makefile "tests/Makefile.tests" for running
> shellcheck on shell test scripts. This automates below shellcheck
> into the build.
>
> $ for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do
> shellcheck -S warning $F; done
I think you can do it if $(shell command -v shellcheck) returns
non-empty string (the path to the shellcheck). Then the feature
test logic can be gone.
>
> CONFIG_SHELLCHECK check is added to avoid build breakage in
> the absence of shellcheck binary. Update Makefile.perf to contain
> new rule for "SHELLCHECK_TEST" which is for making shellcheck
> test as a dependency on perf binary. Added "tests/Makefile.tests"
> to run shellcheck on shellscripts in tests/shell. The make rule
> "SHLLCHECK_RUN" ensures that, every time during make, shellcheck
> will be run only on modified files during subsequent invocations.
> By this, if any newly added shell scripts or fixes in existing
> scripts breaks coding/formatting style, it will get captured
> during the perf build.
Can you show me the example output?
>
> Signed-off-by: Athira Rajeev
> ---
> tools/perf/Makefile.perf| 12 +++-
> tools/perf/tests/Makefile.tests | 24
> 2 files changed, 35 insertions(+), 1 deletion(-)
> create mode 100644 tools/perf/tests/Makefile.tests
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index f6fdc2d5a92f..c27f54771e90 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -667,7 +667,16 @@ $(PERF_IN): prepare FORCE
> $(PMU_EVENTS_IN): FORCE prepare
> $(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=pmu-events
> obj=pmu-events
>
> -$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN)
> +# Runs shellcheck on perf test shell scripts
> +ifeq ($(CONFIG_SHELLCHECK),y)
> +SHELLCHECK_TEST: FORCE prepare
> + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests
> +else
> +SHELLCHECK_TEST:
> + @:
> +endif
> +
> +$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN) SHELLCHECK_TEST
> $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) \
> $(PERF_IN) $(PMU_EVENTS_IN) $(LIBS) -o $@
>
> @@ -1129,6 +1138,7 @@ bpf-skel-clean:
> $(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
>
> clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean
> $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean
> tests-coresight-targets-clean
> + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
> $(call QUIET_CLEAN, core-objs) $(RM) $(LIBPERF_A)
> $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
> $(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd'
> -delete -o -name '\.*.d' -delete
> $(Q)$(RM) $(OUTPUT).config-detected
> diff --git a/tools/perf/tests/Makefile.tests b/tools/perf/tests/Makefile.tests
> new file mode 100644
> index ..e74575559e83
> --- /dev/null
> +++ b/tools/perf/tests/Makefile.tests
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Athira Rajeev , 2023
> +-include $(OUTPUT).config-detected
> +
> +log_file = $(OUTPUT)shellcheck_test.log
> +PROGS = $(subst ./,,$(shell find tests/shell -perm -o=x -type f -name
> '*.sh'))
> +DEPS = $(addprefix output/,$(addsuffix .dep,$(basename $(PROGS
> +DIRS = $(shell echo $(dir $(DEPS)) | xargs -n1 | sort -u | xargs)
> +
> +.PHONY: all
> +all: SHELLCHECK_RUN
> + @:
> +
> +SHELLCHECK_RUN: $(DEPS) $(DIRS)
> +
> +output/%.dep: %.sh | $(DIRS)
> + $(call rule_mkdir)
> + $(Q)$(call frecho-cmd,test)@touch $@
> + $(Q)$(call frecho-cmd,test)@shellcheck -S warning $(subst
> output/,./,$(patsubst %.dep, %.sh, $@)) 1> ${log_file} && ([[ ! -s
> ${log_file} ]])
This line is too long, please wrap it with some backslashes.
Thanks,
Namhyung
> +$(DIRS):
> + @mkdir -p $@
> +
> +clean:
> + @rm -rf $(log_file) output
> --
> 2.31.1
>
Re: [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary
Hello, On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev wrote: > > shellcheck tool can detect coding/formatting issues on > shell scripts. In perf directory "tests/shell", there are lot > of shell test scripts and this tool can detect coding/formatting > issues on these scripts. > > Example to use shellcheck for severity level for > errors and warnings, below command is used: > ># for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck -S > warning $F; done ># echo $? > 0 > > This testing needs to be automated into the build so that it > can avoid regressions and also run the check for newly added > during build test itself. Add a new feature check to detect > presence of shellcheck. Add CONFIG_SHELLCHECK feature check in > the build to avoid not having shellcheck breaking the build. > > Signed-off-by: Athira Rajeev > --- > tools/build/Makefile.feature | 6 -- > tools/build/feature/Makefile | 8 +++- > tools/perf/Makefile.config | 10 ++ > 3 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature > index 934e2777a2db..23f56b95babf 100644 > --- a/tools/build/Makefile.feature > +++ b/tools/build/Makefile.feature > @@ -72,7 +72,8 @@ FEATURE_TESTS_BASIC := \ > libzstd\ > disassembler-four-args \ > disassembler-init-styled \ > -file-handle > +file-handle\ > +shellcheck > > # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list > # of all feature tests > @@ -138,7 +139,8 @@ FEATURE_DISPLAY ?= \ > get_cpuid \ > bpf \ > libaio\ > - libzstd > + libzstd \ > + shellcheck > > # > # Declare group members of a feature to display the logical OR of the > detection > diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile > index 3184f387990a..44ba6d0c98d0 100644 > --- a/tools/build/feature/Makefile > +++ b/tools/build/feature/Makefile > @@ -76,7 +76,8 @@ FILES= \ > test-libzstd.bin \ > test-clang-bpf-co-re.bin \ > test-file-handle.bin \ > - test-libpfm4.bin > + test-libpfm4.bin \ > + test-shellcheck.bin > > FILES := $(addprefix $(OUTPUT),$(FILES)) > > @@ -92,6 +93,8 @@ __BUILD = $(CC) $(CFLAGS) -MD -Wall -Werror -o $@ > $(patsubst %.bin,%.c,$(@F)) $( > __BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall -Werror -o $@ $(patsubst > %.bin,%.cpp,$(@F)) $(LDFLAGS) >BUILDXX = $(__BUILDXX) > $(@:.bin=.make.output) 2>&1 > > + BUILD_BINARY = sh -c $1 > $(@:.bin=.make.output) 2>&1 > + > ### > > $(OUTPUT)test-all.bin: > @@ -207,6 +210,9 @@ $(OUTPUT)test-libslang-include-subdir.bin: > $(OUTPUT)test-libtraceevent.bin: > $(BUILD) -ltraceevent > > +$(OUTPUT)test-shellcheck.bin: > + $(BUILD_BINARY) "shellcheck --version" I don't think it'd generate the .bin file. Anyway, it's a binary file already. Can we check it with `command -v` and get rid of the feature test? Thanks, Namhyung > + > $(OUTPUT)test-libtracefs.bin: > $(BUILD) $(shell $(PKG_CONFIG) --cflags libtraceevent 2>/dev/null) > -ltracefs > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config > index d66b52407e19..e71fe95ad865 100644 > --- a/tools/perf/Makefile.config > +++ b/tools/perf/Makefile.config > @@ -779,6 +779,16 @@ ifndef NO_SLANG >endif > endif > > +ifneq ($(NO_SHELLCHECK),1) > + $(call feature_check,shellcheck) > + ifneq ($(feature-shellcheck), 1) > +msg := $(warning No shellcheck found. please install ShellCheck); > + else > +$(call detected,CONFIG_SHELLCHECK) > +NO_SHELLCHECK := 0 > + endif > +endif > + > ifdef GTK2 >FLAGS_GTK2=$(CFLAGS) $(LDFLAGS) $(EXTLIBS) $(shell $(PKG_CONFIG) --libs > --cflags gtk+-2.0 2>/dev/null) >$(call feature_check,gtk2) > -- > 2.31.1 >
[linux-next:master] BUILD REGRESSION 4ae73bba62a367f2314f6ce69e3085a941983d8b
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 4ae73bba62a367f2314f6ce69e3085a941983d8b Add linux-next specific files for 20230926 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202308282000.2xnh0k6d-...@intel.com https://lore.kernel.org/oe-kbuild-all/202308301211.2hhbgs2n-...@intel.com https://lore.kernel.org/oe-kbuild-all/202309101830.7uqv4smc-...@intel.com https://lore.kernel.org/oe-kbuild-all/202309122047.cri9yjrq-...@intel.com https://lore.kernel.org/oe-kbuild-all/202309130213.msr7x2jz-...@intel.com https://lore.kernel.org/oe-kbuild-all/202309192314.vbsjiim5-...@intel.com Error/Warning: (recently discovered and may have been fixed) ERROR: modpost: "ice_cgu_get_pin_freq_supp" [drivers/net/ethernet/intel/ice/ice.ko] undefined! ERROR: modpost: "ice_cgu_get_pin_name" [drivers/net/ethernet/intel/ice/ice.ko] undefined! ERROR: modpost: "ice_cgu_get_pin_type" [drivers/net/ethernet/intel/ice/ice.ko] undefined! ERROR: modpost: "ice_get_cgu_rclk_pin_info" [drivers/net/ethernet/intel/ice/ice.ko] undefined! ERROR: modpost: "ice_get_cgu_state" [drivers/net/ethernet/intel/ice/ice.ko] undefined! ERROR: modpost: "ice_is_cgu_present" [drivers/net/ethernet/intel/ice/ice.ko] undefined! ERROR: modpost: "ice_is_clock_mux_present_e810t" [drivers/net/ethernet/intel/ice/ice.ko] undefined! ERROR: modpost: "ice_is_phy_rclk_present" [drivers/net/ethernet/intel/ice/ice.ko] undefined! arc-elf-ld: xfrm_algo.c:(.text+0x46c): undefined reference to `crypto_has_aead' drivers/cpufreq/sti-cpufreq.c:215:50: warning: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 2 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:3887: warning: Function parameter or member 'srf_updates' not described in 'could_mpcc_tree_change_for_active_pipes' drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c:60:52: warning: '%s' directive output may be truncated writing up to 63 bytes into a region of size 57 [-Wformat-truncation=] drivers/net/ethernet/sfc/ethtool_common.c:278:32: warning: '%-24s' directive output may be truncated writing between 24 and 31 bytes into a region of size 25 [-Wformat-truncation=] drivers/net/ethernet/sfc/falcon/ethtool.c:229:32: warning: '%-24s' directive output may be truncated writing between 24 and 31 bytes into a region of size 25 [-Wformat-truncation=] drivers/net/ethernet/sfc/siena/ethtool_common.c:229:32: warning: '%-24s' directive output may be truncated writing between 24 and 31 bytes into a region of size 25 [-Wformat-truncation=] drivers/usb/gadget/udc/fsl_udc_core.c:2491:36: warning: 'out' directive writing 3 bytes into a region of size between 2 and 11 [-Wformat-overflow=] drivers/usb/gadget/udc/fsl_udc_core.c:2493:38: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=] fs/bcachefs/bcachefs_format.h:215:25: warning: 'p' offset 3 in 'struct bkey' isn't aligned to 4 [-Wpacked-not-aligned] fs/bcachefs/bcachefs_format.h:217:25: warning: 'version' offset 27 in 'struct bkey' isn't aligned to 4 [-Wpacked-not-aligned] ice_dpll.c:(.text.ice_dpll_init_info+0x160): undefined reference to `ice_get_cgu_rclk_pin_info' ice_dpll.c:(.text.ice_dpll_init_info_direct_pins+0xc4): undefined reference to `ice_cgu_get_pin_freq_supp' ice_dpll.c:(.text.ice_dpll_update_state+0x48): undefined reference to `ice_get_cgu_state' ice_lib.c:(.text.ice_init_feature_support+0x7c): undefined reference to `ice_is_phy_rclk_present' include/linux/fortify-string.h:57:33: warning: writing 8 bytes into a region of size 0 [-Wstringop-overflow=] include/linux/netlink.h:116:13: warning: ') out of range, only support...' directive output truncated writing 60 bytes into a region of size between 46 and 55 [-Wformat-truncation=] include/linux/netlink.h:116:13: warning: 'sfc: Unsupported: only suppo...' directive output truncated writing 104 bytes into a region of size 80 [-Wformat-truncation=] powerpc-linux-ld: ice_dpll.c:(.text.ice_dpll_init_info_direct_pins+0x110): undefined reference to `ice_cgu_get_pin_type' powerpc-linux-ld: ice_dpll.c:(.text.ice_dpll_init_info_direct_pins+0xfc): undefined reference to `ice_cgu_get_pin_name' powerpc-linux-ld: ice_lib.c:(.text.ice_init_feature_support+0xd0): undefined reference to `ice_is_cgu_present' powerpc-linux-ld: ice_lib.c:(.text.ice_init_feature_support+0xe0): undefined reference to `ice_is_clock_mux_present_e810t' s390-linux-ld: drivers/net/ethernet/intel/ice/ice_dpll.c:1647:(.text+0xa6c): undefined reference to `ice_cgu_get_pin_type' s390-linux-ld: drivers/net/ethernet/intel/ice/ice_dpll.c:1666:(.text+0xad0): undefined reference to `ice_cgu_get_pin_freq_supp' sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c:44:50: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 10 [-Wformat
Re: Questions: Should kernel panic when PCIe fatal error occurs?
On Fri, Sep 22, 2023 at 10:46:36AM +0800, Shuai Xue wrote: > ... > Actually, this is a question from my colleague from firmware team. > The original question is that: > > "Should I set CPER_SEV_FATAL for Generic Error Status Block when a > PCIe fatal error is detected? If set, kernel will always panic. > Otherwise, kernel will always not panic." > > So I pull a question about desired behavior of Linux kernel first :) > From the perspective of the kernel, CPER_SEV_FATAL for Generic Error > Status Block is not reasonable. The kernel will attempt to recover > Fatal errors, although recovery may fail. I don't know the semantics of CPER_SEV_FATAL or why it's there. With CPER, we have *two* error severities: a "native" one defined by the PCIe spec and another defined by the platform via CPER. I speculate that the reason for the CPER severity could be to provide a severity for error sources that don't have a "native" severity like AER does, or for the vendor to force the OS to restart (for CPER_SEV_FATAL, anyway) in cases where it might not otherwise. In the native case, we only have the PCIe severity and don't have the CPER severity at all, and I suspect that unless there's uncontained data corruption, we would rather handle even the most severe PCIe fatal error by disabling the specific device(s) instead of panicking and restarting the whole machine. So for PCIe errors, I'm not sure setting CPER_SEV_FATAL is beneficial unless the platform wants to force the OS to panic, e.g., maybe the platform knows about data corruption and/or the vendor wants the OS to panic as part of a reliability story. Presumably the platform has already logged the error, and I assume the platform *could* restart without even returning to the OS, but maybe it wants the OS to do a crashdump or shutdown in a more orderly way. Bjorn
Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events
Hello,
On Mon, Sep 25, 2023 at 10:37 AM Arnaldo Carvalho de Melo
wrote:
>
>
>
> On Wed, Sep 13, 2023, 7:40 AM Athira Rajeev
> wrote:
>>
>>
>>
>> > On 08-Sep-2023, at 7:48 PM, Athira Rajeev
>> > wrote:
>> >
>> >
>> >
>> >> On 08-Sep-2023, at 11:04 AM, Sachin Sant wrote:
>> >>
>> >>
>> >>
>> >>> On 07-Sep-2023, at 10:29 PM, Athira Rajeev
>> >>> wrote:
>> >>>
>> >>> Testcase "Parsing of all PMU events from sysfs" parse events for
>> >>> all PMUs, and not just cpu. In case of powerpc, the PowerVM
>> >>> environment supports events from hv_24x7 and hv_gpci PMU which
>> >>> is of example format like below:
>> >>>
>> >>> - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
>> >>> - hv_gpci/event,partition_id=?/
>> >>>
>> >>> The value for "?" needs to be filled in depending on system
>> >>> configuration. It is better to skip these parametrized events
>> >>> in this test as it is done in:
>> >>> 'commit b50d691e50e6 ("perf test: Fix "all PMU test" to skip
>> >>> parametrized events")' which handled a simialr instance with
>> >>> "all PMU test".
>> >>>
>> >>> Fix parse-events test to skip parametrized events since
>> >>> it needs proper setup of the parameters.
>> >>>
>> >>> Signed-off-by: Athira Rajeev
>> >>> ---
>> >>> Changelog:
>> >>> v1 -> v2:
>> >>> Addressed review comments from Ian. Updated size of
>> >>> pmu event name variable and changed bool name which is
>> >>> used to skip the test.
>> >>>
>> >>
>> >> The patch fixes the reported issue.
>> >>
>> >> 6.2: Parsing of all PMU events from sysfs : Ok
>> >> 6.3: Parsing of given PMU events from sysfs: Ok
>> >>
>> >> Tested-by: Sachin Sant
>> >>
>> >> - Sachin
>> >
>> > Hi Sachin, Ian
>> >
>> > Thanks for testing the patch
>>
>> Hi Arnaldo
>>
>> Can you please check and pull this if it looks good to go .
>
>
> Namhyung, can you please take a look?
Yep sure. I think it needs to close the file when getline() fails.
Athira, can you please send v3 with that?
Thanks,
Namhyung
[PATCH] uapi/auxvec: Define AT_HWCAP3 and AT_HWCAP4 aux vector, entries
The powerpc toolchain keeps a copy of the HWCAP bit masks in our TCB for fast access by our __builtin_cpu_supports built-in function. The TCB space for the HWCAP entries - which are created in pairs - is an ABI extension, so waiting to create the space for HWCAP3 and HWCAP4 until we need them is problematical, given distro unwillingness to apply ABI modifying patches to distro point releases. Define AT_HWCAP3 and AT_HWCAP4 in the generic uapi header so they can be used in GLIBC to reserve space in the powerpc TCB for their future use. I scanned both the Linux and GLIBC source codes looking for unused AT_* values and 29 and 30 did not seem to be used, so they are what I went with. If anyone sees a problem with using those specific values, I'm amenable to using other values, just let me know what would be better. Peter Signed-off-by: Peter Bergner --- include/uapi/linux/auxvec.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/uapi/linux/auxvec.h b/include/uapi/linux/auxvec.h index 6991c4b8ab18..cc61cb9b3e9a 100644 --- a/include/uapi/linux/auxvec.h +++ b/include/uapi/linux/auxvec.h @@ -32,6 +32,8 @@ #define AT_HWCAP2 26 /* extension of AT_HWCAP */ #define AT_RSEQ_FEATURE_SIZE 27 /* rseq supported feature size */ #define AT_RSEQ_ALIGN 28 /* rseq allocation alignment */ +#define AT_HWCAP3 29 /* extension of AT_HWCAP */ +#define AT_HWCAP4 30 /* extension of AT_HWCAP */ #define AT_EXECFN 31 /* filename of program */ -- 2.39.3
Re: [PATCH 8/8] iommu/dart: Call apple_dart_finalize_domain() as part of alloc_paging()
On Fri, Sep 22, 2023 at 02:07:59PM -0300, Jason Gunthorpe wrote:
> In many cases the dev argument will now be !NULL so we should use it to
> finalize the domain at allocation.
>
> Make apple_dart_finalize_domain() accept the correct type.
>
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/iommu/apple-dart.c | 26 ++
> 1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
> index 62efe0aa056f60..2c1832e357c7c6 100644
> --- a/drivers/iommu/apple-dart.c
> +++ b/drivers/iommu/apple-dart.c
> @@ -568,10 +568,9 @@ apple_dart_setup_translation(struct apple_dart_domain
> *domain,
> stream_map->dart->hw->invalidate_tlb(stream_map);
> }
>
> -static int apple_dart_finalize_domain(struct iommu_domain *domain,
> +static int apple_dart_finalize_domain(struct apple_dart_domain *dart_domain,
> struct apple_dart_master_cfg *cfg)
> {
> - struct apple_dart_domain *dart_domain = to_dart_domain(domain);
> struct apple_dart *dart = cfg->stream_maps[0].dart;
> struct io_pgtable_cfg pgtbl_cfg;
> int ret = 0;
> @@ -597,16 +596,17 @@ static int apple_dart_finalize_domain(struct
> iommu_domain *domain,
> .iommu_dev = dart->dev,
> };
>
> - dart_domain->pgtbl_ops =
> - alloc_io_pgtable_ops(dart->hw->fmt, _cfg, domain);
> + dart_domain->pgtbl_ops = alloc_io_pgtable_ops(dart->hw->fmt, _cfg,
> + _domain->domain);
> if (!dart_domain->pgtbl_ops) {
> ret = -ENOMEM;
> goto done;
> }
>
> - domain->pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
> - domain->geometry.aperture_start = 0;
> - domain->geometry.aperture_end = (dma_addr_t)DMA_BIT_MASK(dart->ias);
> + dart_domain->domain.pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
> + dart_domain->domain.geometry.aperture_start = 0;
> + dart_domain->domain.geometry.aperture_end =
> + (dma_addr_t)DMA_BIT_MASK(dart->ias);
>
> dart_domain->finalized = true;
>
> @@ -661,7 +661,7 @@ static int apple_dart_attach_dev_paging(struct
> iommu_domain *domain,
> if (cfg->stream_maps[0].dart->force_bypass)
> return -EINVAL;
>
> - ret = apple_dart_finalize_domain(domain, cfg);
> + ret = apple_dart_finalize_domain(dart_domain, cfg);
> if (ret)
> return ret;
>
> @@ -757,6 +757,16 @@ static struct iommu_domain
> *apple_dart_domain_alloc_paging(struct device *dev)
>
> mutex_init(_domain->init_lock);
>
> + if (dev) {
> + struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
> + int ret;
> +
> + ret = apple_dart_finalize_domain(dart_domain, cfg);
> + if (ret) {
> + kfree(dart_domain);
> + return ERR_PTR(ret);
> + }
> + }
> return _domain->domain;
> }
>
> --
> 2.42.0
Reviewed-by: Janne Grunau
best regards
Janne
Re: [PATCH 7/8] iommu/dart: Convert to domain_alloc_paging()
On Fri, Sep 22, 2023 at 02:07:58PM -0300, Jason Gunthorpe wrote:
> Since the IDENTITY and BLOCKED behaviors were moved to global statics all
> that remains is the paging domain. Rename to
> apple_dart_attach_dev_paging() and remove the left over cruft.
>
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/iommu/apple-dart.c | 33 +++--
> 1 file changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
> index 376f4c5461e8f7..62efe0aa056f60 100644
> --- a/drivers/iommu/apple-dart.c
> +++ b/drivers/iommu/apple-dart.c
> @@ -650,8 +650,8 @@ static int apple_dart_domain_add_streams(struct
> apple_dart_domain *domain,
> true);
> }
>
> -static int apple_dart_attach_dev(struct iommu_domain *domain,
> - struct device *dev)
> +static int apple_dart_attach_dev_paging(struct iommu_domain *domain,
> + struct device *dev)
> {
> int ret, i;
> struct apple_dart_stream_map *stream_map;
> @@ -665,21 +665,13 @@ static int apple_dart_attach_dev(struct iommu_domain
> *domain,
> if (ret)
> return ret;
>
> - switch (domain->type) {
> - case IOMMU_DOMAIN_DMA:
> - case IOMMU_DOMAIN_UNMANAGED:
> - ret = apple_dart_domain_add_streams(dart_domain, cfg);
> - if (ret)
> - return ret;
> + ret = apple_dart_domain_add_streams(dart_domain, cfg);
> + if (ret)
> + return ret;
>
> - for_each_stream_map(i, cfg, stream_map)
> - apple_dart_setup_translation(dart_domain, stream_map);
> - break;
> - default:
> - return -EINVAL;
> - }
> -
> - return ret;
> + for_each_stream_map(i, cfg, stream_map)
> + apple_dart_setup_translation(dart_domain, stream_map);
> + return 0;
> }
>
> static int apple_dart_attach_dev_identity(struct iommu_domain *domain,
> @@ -755,13 +747,10 @@ static void apple_dart_release_device(struct device
> *dev)
> kfree(cfg);
> }
>
> -static struct iommu_domain *apple_dart_domain_alloc(unsigned int type)
> +static struct iommu_domain *apple_dart_domain_alloc_paging(struct device
> *dev)
> {
> struct apple_dart_domain *dart_domain;
>
> - if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED)
> - return NULL;
> -
> dart_domain = kzalloc(sizeof(*dart_domain), GFP_KERNEL);
> if (!dart_domain)
> return NULL;
> @@ -982,7 +971,7 @@ static void apple_dart_get_resv_regions(struct device
> *dev,
> static const struct iommu_ops apple_dart_iommu_ops = {
> .identity_domain = _dart_identity_domain,
> .blocked_domain = _dart_blocked_domain,
> - .domain_alloc = apple_dart_domain_alloc,
> + .domain_alloc_paging = apple_dart_domain_alloc_paging,
> .probe_device = apple_dart_probe_device,
> .release_device = apple_dart_release_device,
> .device_group = apple_dart_device_group,
> @@ -992,7 +981,7 @@ static const struct iommu_ops apple_dart_iommu_ops = {
> .pgsize_bitmap = -1UL, /* Restricted during dart probe */
> .owner = THIS_MODULE,
> .default_domain_ops = &(const struct iommu_domain_ops) {
> - .attach_dev = apple_dart_attach_dev,
> + .attach_dev = apple_dart_attach_dev_paging,
> .map_pages = apple_dart_map_pages,
> .unmap_pages= apple_dart_unmap_pages,
> .flush_iotlb_all = apple_dart_flush_iotlb_all,
> --
> 2.42.0
Reviewed-by: Janne Grunau
best regards
Janne
Re: [PATCH 6/8] iommu/dart: Move the blocked domain support to a global static
Hej,
On Fri, Sep 22, 2023 at 02:07:57PM -0300, Jason Gunthorpe wrote:
> Move to the new static global for blocked domains. Move the blocked
> specific code to apple_dart_attach_dev_blocked().
>
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/iommu/apple-dart.c | 36 ++--
> 1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
> index 424f779ccc34df..376f4c5461e8f7 100644
> --- a/drivers/iommu/apple-dart.c
> +++ b/drivers/iommu/apple-dart.c
> @@ -675,10 +675,6 @@ static int apple_dart_attach_dev(struct iommu_domain
> *domain,
> for_each_stream_map(i, cfg, stream_map)
> apple_dart_setup_translation(dart_domain, stream_map);
> break;
> - case IOMMU_DOMAIN_BLOCKED:
> - for_each_stream_map(i, cfg, stream_map)
> - apple_dart_hw_disable_dma(stream_map);
> - break;
> default:
> return -EINVAL;
> }
> @@ -710,6 +706,30 @@ static struct iommu_domain apple_dart_identity_domain = {
> .ops = _dart_identity_ops,
> };
>
> +static int apple_dart_attach_dev_blocked(struct iommu_domain *domain,
> + struct device *dev)
> +{
> + struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
> + struct apple_dart_stream_map *stream_map;
> + int i;
> +
> + if (cfg->stream_maps[0].dart->force_bypass)
> + return -EINVAL;
unrelated to this change as this keeps the current behavior but I think
force_bypass should not override IOMMU_DOMAIN_BLOCKED. It is set if the
CPU page size is smaller than dart's page size. Obviously dart can't
translate in that situation but it should be still possible to block it
completely.
How do we manage this? I can write a patch either to the current state
or based on this series.
> +
> + for_each_stream_map(i, cfg, stream_map)
> + apple_dart_hw_disable_dma(stream_map);
> + return 0;
> +}
> +
> +static const struct iommu_domain_ops apple_dart_blocked_ops = {
> + .attach_dev = apple_dart_attach_dev_blocked,
> +};
> +
> +static struct iommu_domain apple_dart_blocked_domain = {
> + .type = IOMMU_DOMAIN_BLOCKED,
> + .ops = _dart_blocked_ops,
> +};
> +
> static struct iommu_device *apple_dart_probe_device(struct device *dev)
> {
> struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
> @@ -739,8 +759,7 @@ static struct iommu_domain
> *apple_dart_domain_alloc(unsigned int type)
> {
> struct apple_dart_domain *dart_domain;
>
> - if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED &&
> - type != IOMMU_DOMAIN_BLOCKED)
> + if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED)
> return NULL;
>
> dart_domain = kzalloc(sizeof(*dart_domain), GFP_KERNEL);
> @@ -749,10 +768,6 @@ static struct iommu_domain
> *apple_dart_domain_alloc(unsigned int type)
>
> mutex_init(_domain->init_lock);
>
> - /* no need to allocate pgtbl_ops or do any other finalization steps */
> - if (type == IOMMU_DOMAIN_BLOCKED)
> - dart_domain->finalized = true;
> -
> return _domain->domain;
> }
>
> @@ -966,6 +981,7 @@ static void apple_dart_get_resv_regions(struct device
> *dev,
>
> static const struct iommu_ops apple_dart_iommu_ops = {
> .identity_domain = _dart_identity_domain,
> + .blocked_domain = _dart_blocked_domain,
> .domain_alloc = apple_dart_domain_alloc,
> .probe_device = apple_dart_probe_device,
> .release_device = apple_dart_release_device,
> --
> 2.42.0
Reviewed-by: Janne Grunau
best regards
Janne
ps: I sent the reply to [Patch 4/8] accidentally with an incorrect from
address but the correct Reviewed-by:. I can resend if necessary.
Re: [PATCH v6 08/30] dt-bindings: soc: fsl: cpm_qe: cpm1-scc-qmc: Add support for QMC HDLC
On 25/09/2023 15:50, Herve Codina wrote: > With these details, do you still think I need to change the child > (channel) > compatible ? From OS point of view, you have a driver binding to this child-level compatible. How do you enforce Linux driver binding based on parent compatible? I looked at your next patch and I did not see it. >>> >>> We do not need to have the child driver binding based on parent. >> >> Exactly, that's what I said. >> >>> We have to ensure that the child handles a QMC channel and the parent >>> provides >>> a QMC channel. >>> >>> A QMC controller (parent) has to implement the QMC API >>> (include/soc/fsl/qe/qmc.h) >>> and a QMC channel driver (child) has to use the QMC API. >> >> How does this solve my concerns? Sorry, I do not understand. Your driver >> is a platform driver and binds to the generic compatible. How do you >> solve regular compatibility issues (need for quirks) if parent >> compatible is not used? >> >> How does being QMC compliant affects driver binding and >> compatibility/quirks? >> >> We are back to my original question and I don't think you answered to >> any of the concerns. > > Well, to be sure that I understand correctly, do you mean that I should > provide a compatible for the child (HDLC) with something like this: > --- 8< --- > compatible: > items: > - enum: > - fsl,mpc885-qmc-hdlc > - fsl,mpc866-qmc-hdlc > - const: fsl,cpm1-qmc-hdlc > - const: fsl,qmc-hdlc > --- 8< --- Yes, more or less, depending on actual compatibility and SoC-family. Maybe "fsl,cpm1-qmc-hdlc" item in the middle is not needed. > > If so, I didn't do that because a QMC channel consumer (driver matching > fsl,qmc-hdlc) doesn't contains any SoC specific part. Just like hundreds of other drivers. :) There is a paragraph about specific compatibles here: https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-schema.html > It uses the channel as a communication channel to send/receive HDLC frames > to/from this communication channel. > All the specific SoC part is handled by the QMC controller (parent) itself and > not by any consumer (child). OK, so you guarantee in 100% for this hardware and all future (including designs unknown currently), that they will be 100% compatible with existing QMC channel consumer (child, matching fsl,qmc-hdlc) driver, thus there will be no need for any quirk. Specifically, there will be no chances that it would be reasonable to re-use the same driver for child (currently fsl,qmc-hdlc) in different parent. P.S. If you received this email twice, apologies, I have here troubles with internet. Best regards, Krzysztof
Re: [PATCH 6/8] iommu/dart: Move the blocked domain support to a global static
On 2023-09-26 20:05, Janne Grunau wrote:
Hej,
On Fri, Sep 22, 2023 at 02:07:57PM -0300, Jason Gunthorpe wrote:
Move to the new static global for blocked domains. Move the blocked
specific code to apple_dart_attach_dev_blocked().
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/apple-dart.c | 36 ++--
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index 424f779ccc34df..376f4c5461e8f7 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -675,10 +675,6 @@ static int apple_dart_attach_dev(struct iommu_domain
*domain,
for_each_stream_map(i, cfg, stream_map)
apple_dart_setup_translation(dart_domain, stream_map);
break;
- case IOMMU_DOMAIN_BLOCKED:
- for_each_stream_map(i, cfg, stream_map)
- apple_dart_hw_disable_dma(stream_map);
- break;
default:
return -EINVAL;
}
@@ -710,6 +706,30 @@ static struct iommu_domain apple_dart_identity_domain = {
.ops = _dart_identity_ops,
};
+static int apple_dart_attach_dev_blocked(struct iommu_domain *domain,
+struct device *dev)
+{
+ struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
+ struct apple_dart_stream_map *stream_map;
+ int i;
+
+ if (cfg->stream_maps[0].dart->force_bypass)
+ return -EINVAL;
unrelated to this change as this keeps the current behavior but I think
force_bypass should not override IOMMU_DOMAIN_BLOCKED. It is set if the
CPU page size is smaller than dart's page size. Obviously dart can't
translate in that situation but it should be still possible to block it
completely.
How do we manage this? I can write a patch either to the current state
or based on this series.
The series is queued already, so best to send a patch based on
iommu/core (I guess just removing these lines?). It won't be
super-useful in practice since the blocking domain is normally only used
to transition to an unmanaged domain which in the force_bypass situation
can't be used anyway, but it's still nice on principle not to have
unnecessary reasons for attach to fail.
Thanks,
Robin.
+
+ for_each_stream_map(i, cfg, stream_map)
+ apple_dart_hw_disable_dma(stream_map);
+ return 0;
+}
+
+static const struct iommu_domain_ops apple_dart_blocked_ops = {
+ .attach_dev = apple_dart_attach_dev_blocked,
+};
+
+static struct iommu_domain apple_dart_blocked_domain = {
+ .type = IOMMU_DOMAIN_BLOCKED,
+ .ops = _dart_blocked_ops,
+};
+
static struct iommu_device *apple_dart_probe_device(struct device *dev)
{
struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
@@ -739,8 +759,7 @@ static struct iommu_domain
*apple_dart_domain_alloc(unsigned int type)
{
struct apple_dart_domain *dart_domain;
- if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED &&
- type != IOMMU_DOMAIN_BLOCKED)
+ if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED)
return NULL;
dart_domain = kzalloc(sizeof(*dart_domain), GFP_KERNEL);
@@ -749,10 +768,6 @@ static struct iommu_domain
*apple_dart_domain_alloc(unsigned int type)
mutex_init(_domain->init_lock);
- /* no need to allocate pgtbl_ops or do any other finalization steps */
- if (type == IOMMU_DOMAIN_BLOCKED)
- dart_domain->finalized = true;
-
return _domain->domain;
}
@@ -966,6 +981,7 @@ static void apple_dart_get_resv_regions(struct device *dev,
static const struct iommu_ops apple_dart_iommu_ops = {
.identity_domain = _dart_identity_domain,
+ .blocked_domain = _dart_blocked_domain,
.domain_alloc = apple_dart_domain_alloc,
.probe_device = apple_dart_probe_device,
.release_device = apple_dart_release_device,
--
2.42.0
Reviewed-by: Janne Grunau
best regards
Janne
ps: I sent the reply to [Patch 4/8] accidentally with an incorrect from
address but the correct Reviewed-by:. I can resend if necessary.
Re: [PATCH 6/8] iommu/dart: Move the blocked domain support to a global static
On 2023-09-26 20:34, Robin Murphy wrote:
On 2023-09-26 20:05, Janne Grunau wrote:
Hej,
On Fri, Sep 22, 2023 at 02:07:57PM -0300, Jason Gunthorpe wrote:
Move to the new static global for blocked domains. Move the blocked
specific code to apple_dart_attach_dev_blocked().
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/apple-dart.c | 36 ++--
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index 424f779ccc34df..376f4c5461e8f7 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -675,10 +675,6 @@ static int apple_dart_attach_dev(struct
iommu_domain *domain,
for_each_stream_map(i, cfg, stream_map)
apple_dart_setup_translation(dart_domain, stream_map);
break;
- case IOMMU_DOMAIN_BLOCKED:
- for_each_stream_map(i, cfg, stream_map)
- apple_dart_hw_disable_dma(stream_map);
- break;
default:
return -EINVAL;
}
@@ -710,6 +706,30 @@ static struct iommu_domain
apple_dart_identity_domain = {
.ops = _dart_identity_ops,
};
+static int apple_dart_attach_dev_blocked(struct iommu_domain *domain,
+ struct device *dev)
+{
+ struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
+ struct apple_dart_stream_map *stream_map;
+ int i;
+
+ if (cfg->stream_maps[0].dart->force_bypass)
+ return -EINVAL;
unrelated to this change as this keeps the current behavior but I think
force_bypass should not override IOMMU_DOMAIN_BLOCKED. It is set if the
CPU page size is smaller than dart's page size. Obviously dart can't
translate in that situation but it should be still possible to block it
completely.
How do we manage this? I can write a patch either to the current state
or based on this series.
The series is queued already, so best to send a patch based on
iommu/core (I guess just removing these lines?).
Um, what? This isn't the domain_alloc_paging series itself, Robin you
fool. Clearly it's time to close the computer and try again tomorrow...
Cheers,
Robin.
It won't be
super-useful in practice since the blocking domain is normally only used
to transition to an unmanaged domain which in the force_bypass situation
can't be used anyway, but it's still nice on principle not to have
unnecessary reasons for attach to fail.
Thanks,
Robin.
+
+ for_each_stream_map(i, cfg, stream_map)
+ apple_dart_hw_disable_dma(stream_map);
+ return 0;
+}
+
+static const struct iommu_domain_ops apple_dart_blocked_ops = {
+ .attach_dev = apple_dart_attach_dev_blocked,
+};
+
+static struct iommu_domain apple_dart_blocked_domain = {
+ .type = IOMMU_DOMAIN_BLOCKED,
+ .ops = _dart_blocked_ops,
+};
+
static struct iommu_device *apple_dart_probe_device(struct device
*dev)
{
struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
@@ -739,8 +759,7 @@ static struct iommu_domain
*apple_dart_domain_alloc(unsigned int type)
{
struct apple_dart_domain *dart_domain;
- if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED &&
- type != IOMMU_DOMAIN_BLOCKED)
+ if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED)
return NULL;
dart_domain = kzalloc(sizeof(*dart_domain), GFP_KERNEL);
@@ -749,10 +768,6 @@ static struct iommu_domain
*apple_dart_domain_alloc(unsigned int type)
mutex_init(_domain->init_lock);
- /* no need to allocate pgtbl_ops or do any other finalization
steps */
- if (type == IOMMU_DOMAIN_BLOCKED)
- dart_domain->finalized = true;
-
return _domain->domain;
}
@@ -966,6 +981,7 @@ static void apple_dart_get_resv_regions(struct
device *dev,
static const struct iommu_ops apple_dart_iommu_ops = {
.identity_domain = _dart_identity_domain,
+ .blocked_domain = _dart_blocked_domain,
.domain_alloc = apple_dart_domain_alloc,
.probe_device = apple_dart_probe_device,
.release_device = apple_dart_release_device,
--
2.42.0
Reviewed-by: Janne Grunau
best regards
Janne
ps: I sent the reply to [Patch 4/8] accidentally with an incorrect from
address but the correct Reviewed-by:. I can resend if necessary.
Re: [Bisected] PowerMac G4 getting "BUG: Unable to handle kernel data access on write at 0x00001ff0" at boot with CONFIG_VMAP_STACK=y on kernels 6.5.x (regression over 6.4.x)
* Bagas Sanjaya [230925 20:03]: > On Tue, Sep 26, 2023 at 01:01:59AM +0200, Erhard Furtner wrote: > > Greetings! > > > > Had a chat on #gentoo-powerpc with another user whose G4 Mini fails booting > > kernel 6.5.0 when CONFIG_VMAP_STACK=y is enabled. I was able to replicate > > the issue on my PowerMac G4. Also I was able to bisect the issue. > > > > Kernels 6.4.x boot ok with CONFIG_VMAP_STACK=y but on 6.5.5 I get: > > > > [...] > > Kernel attempted to write user page (1ff0) - exploit attempt? (uid: 0) > > BUG: Unable to handle kernel data access on write at 0x1ff0 > > Faulting instruction address: 0xc0008750 > > Oops: Kernel access of bad area, sig: 11 [#1] > > BE PAGE_SIZE=4K MMU=Hash PowerMac > > Modules linked in: > > CPU: 0 PID: 0 Comm: swapper Not tainted 6.5.5-PMacG4 #5 > > Hardware name: PowerMac3,6 7455 0x80010303 PowerMac > > NIP: c0008750 LR: c0041848 CTR: c0070988 > > REGS: c0d3dcd0 TRAP: 0300 Not tainted (6.5.5-PMacG4) > > MSR: 1032 CR: 22d3ddc0 XER: 2000 > > DAR: 1ff0 DSISR: 4200 > > GPR00: c0041848 c0d3dd90 c0d06360 c0d3ddd0 c0d06360 c0d3dea8 c0d3adc0 > > > > GPR08: c0d4 c0d3ddc0 > > 0004 > > GPR16: 0002 0002 00402dc2 00402dc2 2000 f1004000 > > > > GPR24: c0d45220 c0d06644 c0843c34 0002 c0d06360 c0d0ce00 c0d06360 > > > > NIP [c0008750] do_softirq_own_stack+0x18/0x3c > > LR [c0041848] irq_exit+0x98/0xc4 > > Call Trace: > > [c0d3dd90] [c0d69564] 0xc0d69564 (unreliable) > > [c0d3ddb0] [c0041848] irq_exit+0x98/0xc4 > > [c0d3ddc0] [c0004a98] Decrementer_virt+0x108/0x10c > > --- interrupt: 900 at __schedule+0x43c/0x4e0 > > NIP: c0843940 LR: c084398c CTR: c0070988 > > REGS: c0d3ddd0 TRAP: 0900 Not tainted (6.5.5-PMacG4) > > MSR: 9032 CR: 22024484 XER: > > > > GPR00: c0843574 c0d3de90 c0d06360 c0d06360 c0d06360 c0d3dea8 0001 > > > > GPR08: 9032 c099ce2c 0007ffbf 22024484 > > 0004 > > GPR16: 0002 0002 00402dc2 00402dc2 2000 f1004000 > > > > GPR24: c0d45220 c0d06644 c0843c34 0002 c0d06360 c0d0ce00 c0d06360 > > c0d063ac > > NIP [c0843940] __schedule+0x43c/0x4e0 > > LR [c084390c] __schedule+0x408/0x4e0 > > --- interrupt: 900 > > [c0d3de90] [c0843574] __schedule+0x70/0x4e0 (unreliable) > > [c0d3ded0] [c0843c34] __cond_resched+0x34/0x54 > > [c0d3dee0] [c0141068] __vmalloc_node_range+0x27c/0x64c > > [c0d3de60] [c0141794] __vmalloc_node+0x44/0x54 > > [c8d3df80] [c0c06510] init_IRQ+0x34/0xd4 > > [c8d3dfa0] [c0c03440] start_kernel+0x424/0x558 > > [c8d3dff0] [3540] 0x3540 > > Code: 39490999 7d4901a4 39290aaa 7d2a01a4 4c00012c 4b20 9421ffe0 > > 7c08002a6 3d20c0d4 93e1001c 90010024 83e95278 <943f1ff0> 7fe1fb78 48840c6d > > 8021 > > ---[ end trace ]--- > > > > Kernel panic - not syncing: Attempted to kill the idle task! > > Rebooting in 48 seconds.. > > > > > > The bisect revealed this commit: > > # git bisect good > > cfeb6ae8bcb96ccf674724f223661bbcef7b0d0b is the first bad commit > > commit cfeb6ae8bcb96ccf674724f223661bbcef7b0d0b > > Author: Liam R. Howlett > > Date: Fri Aug 18 20:43:55 2023 -0400 > > > > maple_tree: disable mas_wr_append() when other readers are possible > > > > The current implementation of append may cause duplicate data and/or > > incorrect ranges to be returned to a reader during an update. Although > > this has not been reported or seen, disable the append write operation > > while the tree is in rcu mode out of an abundance of caution. > > > > During the analysis of the mas_next_slot() the following was > > artificially created by separating the writer and reader code: > > > > Writer: reader: > > mas_wr_append > > set end pivot > > updates end metata > > Detects write to last slot > > last slot write is to start of slot > > store current contents in slot > > overwrite old end pivot > > mas_next_slot(): > > read end metadata > > read old end pivot > > return with incorrect > > range > > store new value > > > > Alternatively: > > > > Writer: reader: > > mas_wr_append > > set end pivot > > updates end metata > > Detects write to last slot > > last lost write to end of slot > > store value > > mas_next_slot(): > > read end metadata > > read old end pivot > > read new end pivot > >
Re: [PATCH] ASoC: dt-bindings: Add missing (unevaluated|additional)Properties on child node schemas
On Mon, 25 Sep 2023 17:09:28 -0500, Rob Herring wrote: > Just as unevaluatedProperties or additionalProperties are required at > the top level of schemas, they should (and will) also be required for > child node schemas. That ensures only documented properties are > present for any node. > > Add unevaluatedProperties or additionalProperties as appropriate. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: dt-bindings: Add missing (unevaluated|additional)Properties on child node schemas commit: 7b71da59122c3ab82908910abf78db1fd6340cac All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
[PATCH v3 1/1] PCI: layerscape-ep: set 64-bit DMA mask
From: Guanhua Gao Set DMA mask and coherent DMA mask to enable 64-bit addressing. Signed-off-by: Guanhua Gao Signed-off-by: Hou Zhiqiang Signed-off-by: Frank Li --- Notes: change from v2 to v3 - remove check return value of dma_set_mask_and_coherent. 64bit mask always return success. - remove redundate comments change from v1 to v2 - Remove 32bit DMA mask set. drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c index de4c1758a6c3..2c2c9aaa8700 100644 --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c @@ -249,6 +249,8 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev) pcie->big_endian = of_property_read_bool(dev->of_node, "big-endian"); + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); + platform_set_drvdata(pdev, pcie); ret = dw_pcie_ep_init(>ep); -- 2.34.1
Re: [PATCH v2 1/1] PCI: layerscape-ep: set 64-bit DMA mask
On Tue, Sep 26, 2023 at 12:27:32AM -0700, Christoph Hellwig wrote: > > + /* set 64-bit DMA mask and coherent DMA mask */ > > + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); > > The comment is a bit silly :) > > > + if (ret) > > + return ret; > > Also no need to check the return value when setting a 64-bit mask, > but I guess it desn't hurt here. > You are right, let me remove check.
Re: [PATCH] ASoC: dt-bindings: Add missing (unevaluated|additional)Properties on child node schemas
On Mon, Sep 25, 2023 at 05:09:28PM -0500, Rob Herring wrote: > Just as unevaluatedProperties or additionalProperties are required at > the top level of schemas, they should (and will) also be required for > child node schemas. That ensures only documented properties are > present for any node. > > Add unevaluatedProperties or additionalProperties as appropriate. > > Signed-off-by: Rob Herring Acked-by: Conor Dooley Thanks, Conor. > --- > Documentation/devicetree/bindings/sound/dialog,da7219.yaml | 1 + > Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml | 1 + > Documentation/devicetree/bindings/sound/ti,pcm3168a.yaml | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/dialog,da7219.yaml > b/Documentation/devicetree/bindings/sound/dialog,da7219.yaml > index eb7d219e2c86..19137abdba3e 100644 > --- a/Documentation/devicetree/bindings/sound/dialog,da7219.yaml > +++ b/Documentation/devicetree/bindings/sound/dialog,da7219.yaml > @@ -89,6 +89,7 @@ properties: > >da7219_aad: > type: object > +additionalProperties: false > description: >Configuration of advanced accessory detection. > properties: > diff --git a/Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml > b/Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml > index ff5cd9241941..b522ed7dcc51 100644 > --- a/Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml > +++ b/Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml > @@ -33,6 +33,7 @@ patternProperties: > description: >A DAI managed by this controller > type: object > +additionalProperties: false > > properties: >reg: > diff --git a/Documentation/devicetree/bindings/sound/ti,pcm3168a.yaml > b/Documentation/devicetree/bindings/sound/ti,pcm3168a.yaml > index b6a4360ab845..0b4f003989a4 100644 > --- a/Documentation/devicetree/bindings/sound/ti,pcm3168a.yaml > +++ b/Documentation/devicetree/bindings/sound/ti,pcm3168a.yaml > @@ -60,6 +60,7 @@ properties: > >ports: > $ref: audio-graph-port.yaml#/definitions/port-base > +unevaluatedProperties: false > properties: >port@0: > $ref: audio-graph-port.yaml# > -- > 2.40.1 > signature.asc Description: PGP signature
Re: [PATCH] ASoC: fsl-asoc-card: use integer type for fll_id and pll_id
On Wed, 20 Sep 2023 17:43:12 +0800, Shengjiu Wang wrote:
> As the pll_id and pll_id can be zero (WM8960_SYSCLK_AUTO)
> with the commit 2bbc2df46e67 ("ASoC: wm8960: Make automatic the
> default clocking mode")
>
> Then the machine driver will skip to call set_sysclk() and set_pll()
> for codec, when the sysclk rate is different with what wm8960 read
> at probe, the output sound frequency is wrong.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: fsl-asoc-card: use integer type for fll_id and pll_id
commit: 2b21207afd06714986a3d22442ed4860ba4f9ced
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
Re: [PATCH v3 10/13] arch: make execmem setup available regardless of CONFIG_MODULES
Hi Arnd,
On Tue, Sep 26, 2023 at 09:33:48AM +0200, Arnd Bergmann wrote:
> On Mon, Sep 18, 2023, at 09:29, Mike Rapoport wrote:
> > index a42e4cd11db2..c0b536e398b4 100644
> > --- a/arch/arm/mm/init.c
> > +++ b/arch/arm/mm/init.c
> > +#ifdef CONFIG_XIP_KERNEL
> > +/*
> > + * The XIP kernel text is mapped in the module area for modules and
> > + * some other stuff to work without any indirect relocations.
> > + * MODULES_VADDR is redefined here and not in asm/memory.h to avoid
> > + * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned
> > on/off.
> > + */
> > +#undef MODULES_VADDR
> > +#define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
> > PMD_MASK)
> > +#endif
> > +
> > +#if defined(CONFIG_MMU) && defined(CONFIG_EXECMEM)
> > +static struct execmem_params execmem_params __ro_after_init = {
> > + .ranges = {
> > + [EXECMEM_DEFAULT] = {
> > + .start = MODULES_VADDR,
> > + .end = MODULES_END,
> > + .alignment = 1,
> > + },
>
> This causes a randconfig build failure for me on linux-next now:
>
> arch/arm/mm/init.c:499:25: error: initializer element is not constant
> 499 | #define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
> PMD_MASK)
> | ^
> arch/arm/mm/init.c:506:34: note: in expansion of macro 'MODULES_VADDR'
> 506 | .start = MODULES_VADDR,
> | ^
> arch/arm/mm/init.c:499:25: note: (near initialization for
> 'execmem_params.ranges[0].start')
> 499 | #define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
> PMD_MASK)
> | ^
> arch/arm/mm/init.c:506:34: note: in expansion of macro 'MODULES_VADDR'
> 506 | .start = MODULES_VADDR,
> | ^
>
> I have not done any analysis on the issue so far, I hope
> you can see the problem directly. See
> https://pastebin.com/raw/xVqAyakH for a .config that runs into
> this problem with gcc-13.2.0.
The first patch that breaks XIP build is rather patch 04/13, currently
commit 52a34d45419f ("mm/execmem, arch: convert remaining overrides of
module_alloc to execmem") in mm.git/mm-unstable.
The hunk below is a fix for that and the attached patch is the updated
version of 835bc9685f45 ("arch: make execmem setup available regardless of
CONFIG_MODULES")
Andrew, please let me know if you'd like to me to resend these differently.
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index 2c7651a2d84c..096cc1ead635 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -38,8 +38,6 @@
static struct execmem_params execmem_params __ro_after_init = {
.ranges = {
[EXECMEM_DEFAULT] = {
- .start = MODULES_VADDR,
- .end = MODULES_END,
.alignment = 1,
},
},
@@ -49,6 +47,8 @@ struct execmem_params __init *execmem_arch_params(void)
{
struct execmem_range *r = _params.ranges[EXECMEM_DEFAULT];
+ r->start = MODULES_VADDR;
+ r->end = MODULES_END;
r->pgprot = PAGE_KERNEL_EXEC;
if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS)) {
>
> Arnd
--
Sincerely yours,
Mike.
>From a2dae5a88d172d54e7f074799a286faedd2cdb6a Mon Sep 17 00:00:00 2001
From: "Mike Rapoport (IBM)"
Date: Wed, 31 May 2023 14:58:24 +0300
Subject: [PATCH] arch: make execmem setup available regardless of
CONFIG_MODULES
execmem does not depend on modules, on the contrary modules use
execmem.
To make execmem available when CONFIG_MODULES=n, for instance for
kprobes, split execmem_params initialization out from
arch/kernel/module.c and compile it when CONFIG_EXECMEM=y
Signed-off-by: Mike Rapoport (IBM)
---
arch/arm/kernel/module.c | 38 --
arch/arm/mm/init.c | 38 ++
arch/arm64/kernel/module.c | 130
arch/arm64/mm/init.c | 132 +
arch/loongarch/kernel/module.c | 18 -
arch/loongarch/mm/init.c | 20 +
arch/mips/kernel/module.c | 19 -
arch/mips/mm/init.c| 20 +
arch/parisc/kernel/module.c| 17 -
arch/parisc/mm/init.c | 22 +-
arch/powerpc/kernel/module.c | 60 ---
arch/powerpc/mm/mem.c | 62
arch/riscv/kernel/module.c | 39 --
arch/riscv/mm/init.c | 39 ++
arch/s390/kernel/module.c | 25 ---
arch/s390/mm/init.c| 28 +++
arch/sparc/kernel/module.c | 23 --
arch/sparc/mm/Makefile | 2 +
arch/sparc/mm/execmem.c| 25 +++
arch/x86/kernel/module.c | 27 ---
arch/x86/mm/init.c | 29
21 files changed, 416 insertions(+), 397 deletions(-)
create mode 100644 arch/sparc/mm/execmem.c
Re: [PATCH] ASoC: fsl_sai: Don't disable bitclock for i.MX8MP
On Tue, 19 Sep 2023 17:42:13 +0800, Shengjiu Wang wrote: > On i.MX8MP, the BCE and TERE bit are binding with mclk > enablement, if BCE and TERE are cleared the MCLK also be > disabled on output pin, that cause the external codec (wm8960) > in wrong state. > > Codec (wm8960) is using the mclk to generate PLL clock, > if mclk is disabled before disabling PLL, the codec (wm8960) > won't generate bclk and frameclk when sysclk switch to > MCLK source in next test case. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: fsl_sai: Don't disable bitclock for i.MX8MP commit: 197c53c8ecb34f2cd5922f4bdcffa8f701a134eb All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Re: [PATCH v3 02/13] mm: introduce execmem_text_alloc() and execmem_free()
On Sat, Sep 23, 2023 at 03:36:01PM -0700, Song Liu wrote:
> On Sat, Sep 23, 2023 at 8:39 AM Mike Rapoport wrote:
> >
> > On Thu, Sep 21, 2023 at 03:34:18PM -0700, Song Liu wrote:
> > > On Mon, Sep 18, 2023 at 12:30 AM Mike Rapoport wrote:
> > > >
> > >
> > > [...]
> > >
> > > > diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
> > > > index 42215f9404af..db5561d0c233 100644
> > > > --- a/arch/s390/kernel/module.c
> > > > +++ b/arch/s390/kernel/module.c
> > > > @@ -21,6 +21,7 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > @@ -76,7 +77,7 @@ void *module_alloc(unsigned long size)
> > > > #ifdef CONFIG_FUNCTION_TRACER
> > > > void module_arch_cleanup(struct module *mod)
> > > > {
> > > > - module_memfree(mod->arch.trampolines_start);
> > > > + execmem_free(mod->arch.trampolines_start);
> > > > }
> > > > #endif
> > > >
> > > > @@ -510,7 +511,7 @@ static int
> > > > module_alloc_ftrace_hotpatch_trampolines(struct module *me,
> > > >
> > > > size = FTRACE_HOTPATCH_TRAMPOLINES_SIZE(s->sh_size);
> > > > numpages = DIV_ROUND_UP(size, PAGE_SIZE);
> > > > - start = module_alloc(numpages * PAGE_SIZE);
> > > > + start = execmem_text_alloc(EXECMEM_FTRACE, numpages *
> > > > PAGE_SIZE);
> > >
> > > This should be EXECMEM_MODULE_TEXT?
> >
> > This is an ftrace trampoline, so I think it should be FTRACE type of
> > allocation.
>
> Yeah, I was aware of the ftrace trampoline. My point was, ftrace trampoline
> doesn't seem to have any special requirements. Therefore, it is probably not
> necessary to have a separate type just for it.
Since ftrace trampolines are currently used only on s390 and x86 which
enforce the same range for all executable allocations there are no special
requirements indeed. But I think that explicitly marking these allocations
as FTRACE makes it clearer what are they used for and I don't see downsides
to having a type for FTRACE.
> AFAICT, kprobe, ftrace, and BPF (JIT and trampoline) can share the same
> execmem_type. We may need some work for some archs, but nothing is
> fundamentally different among these.
Using the same type for all generated code implies that all types of the
generated code must live in the same range and I don't think we want to
impose this limitation on architectures.
For example, RISC-V deliberately added a range for BPF code to allow
relative addressing, see commit 7f3631e88ee6 ("riscv, bpf: Provide RISC-V
specific JIT image alloc/free").
> Thanks,
> Song
--
Sincerely yours,
Mike.
Re: [PATCH v3 10/13] arch: make execmem setup available regardless of CONFIG_MODULES
On Mon, Sep 18, 2023, at 09:29, Mike Rapoport wrote:
> index a42e4cd11db2..c0b536e398b4 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> +#ifdef CONFIG_XIP_KERNEL
> +/*
> + * The XIP kernel text is mapped in the module area for modules and
> + * some other stuff to work without any indirect relocations.
> + * MODULES_VADDR is redefined here and not in asm/memory.h to avoid
> + * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned
> on/off.
> + */
> +#undef MODULES_VADDR
> +#define MODULES_VADDR(((unsigned long)_exiprom + ~PMD_MASK) &
> PMD_MASK)
> +#endif
> +
> +#if defined(CONFIG_MMU) && defined(CONFIG_EXECMEM)
> +static struct execmem_params execmem_params __ro_after_init = {
> + .ranges = {
> + [EXECMEM_DEFAULT] = {
> + .start = MODULES_VADDR,
> + .end = MODULES_END,
> + .alignment = 1,
> + },
This causes a randconfig build failure for me on linux-next now:
arch/arm/mm/init.c:499:25: error: initializer element is not constant
499 | #define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
PMD_MASK)
| ^
arch/arm/mm/init.c:506:34: note: in expansion of macro 'MODULES_VADDR'
506 | .start = MODULES_VADDR,
| ^
arch/arm/mm/init.c:499:25: note: (near initialization for
'execmem_params.ranges[0].start')
499 | #define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
PMD_MASK)
| ^
arch/arm/mm/init.c:506:34: note: in expansion of macro 'MODULES_VADDR'
506 | .start = MODULES_VADDR,
| ^
I have not done any analysis on the issue so far, I hope
you can see the problem directly. See
https://pastebin.com/raw/xVqAyakH for a .config that runs into
this problem with gcc-13.2.0.
Arnd
Re: [PATCH v2 1/1] PCI: layerscape-ep: set 64-bit DMA mask
> + /* set 64-bit DMA mask and coherent DMA mask */ > + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); The comment is a bit silly :) > + if (ret) > + return ret; Also no need to check the return value when setting a 64-bit mask, but I guess it desn't hurt here.
Re: [PATCH v4 4/5] powerpc/code-patching: introduce patch_instructions()
Le 26/09/2023 à 00:50, Song Liu a écrit :
> On Fri, Sep 8, 2023 at 6:28 AM Hari Bathini wrote:
>>
>> patch_instruction() entails setting up pte, patching the instruction,
>> clearing the pte and flushing the tlb. If multiple instructions need
>> to be patched, every instruction would have to go through the above
>> drill unnecessarily. Instead, introduce function patch_instructions()
>> that sets up the pte, clears the pte and flushes the tlb only once per
>> page range of instructions to be patched. This adds a slight overhead
>> to patch_instruction() call while improving the patching time for
>> scenarios where more than one instruction needs to be patched.
>>
>> Signed-off-by: Hari Bathini
>
> I didn't see this one when I reviewed 1/5. Please ignore that comment.
If I remember correctry, patch 1 introduces a huge performance
degradation, which gets then improved with this patch.
As I said before, I'd expect patch 4 to go first then get
bpf_arch_text_copy() be implemented with patch_instructions() directly.
Christophe
>
> [...]
>
>> @@ -307,11 +312,22 @@ static int __do_patch_instruction_mm(u32 *addr,
>> ppc_inst_t instr)
>>
>> orig_mm = start_using_temp_mm(patching_mm);
>>
>> - err = __patch_instruction(addr, instr, patch_addr);
>> + while (len > 0) {
>> + instr = ppc_inst_read(code);
>> + ilen = ppc_inst_len(instr);
>> + err = __patch_instruction(addr, instr, patch_addr);
>
> It appears we are still repeating a lot of work here. For example, with
> fill_insn == true, we don't need to repeat ppc_inst_read().
>
> Can we do this with a memcpy or memset like functions?
>
>> + /* hwsync performed by __patch_instruction (sync) if
>> successful */
>> + if (err) {
>> + mb(); /* sync */
>> + break;
>> + }
>>
>> - /* hwsync performed by __patch_instruction (sync) if successful */
>> - if (err)
>> - mb(); /* sync */
>> + len -= ilen;
>> + patch_addr = patch_addr + ilen;
>> + addr = (void *)addr + ilen;
>> + if (!fill_insn)
>> + code = code + ilen;
>
> It took me a while to figure out what "fill_insn" means. Maybe call it
> "repeat_input" or something?
>
> Thanks,
> Song
>
>> + }
>>
>> /* context synchronisation performed by __patch_instruction (isync
>> or exception) */
>> stop_using_temp_mm(patching_mm, orig_mm);
>> @@ -328,16 +344,21 @@ static int __do_patch_instruction_mm(u32 *addr,
>> ppc_inst_t instr)
>> return err;
>> }
>>
Re: [PATCH] ASoC: dt-bindings: Add missing (unevaluated|additional)Properties on child node schemas
On Mon, 25 Sep 2023 17:09:28 -0500 Rob Herring wrote: > Just as unevaluatedProperties or additionalProperties are required at > the top level of schemas, they should (and will) also be required for > child node schemas. That ensures only documented properties are > present for any node. > > Add unevaluatedProperties or additionalProperties as appropriate. > > Signed-off-by: Rob Herring > --- > Documentation/devicetree/bindings/sound/dialog,da7219.yaml | 1 + > Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml | 1 + > Documentation/devicetree/bindings/sound/ti,pcm3168a.yaml | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/dialog,da7219.yaml > b/Documentation/devicetree/bindings/sound/dialog,da7219.yaml > index eb7d219e2c86..19137abdba3e 100644 > --- a/Documentation/devicetree/bindings/sound/dialog,da7219.yaml > +++ b/Documentation/devicetree/bindings/sound/dialog,da7219.yaml > @@ -89,6 +89,7 @@ properties: > >da7219_aad: > type: object > +additionalProperties: false > description: >Configuration of advanced accessory detection. > properties: > diff --git a/Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml > b/Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml > index ff5cd9241941..b522ed7dcc51 100644 > --- a/Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml > +++ b/Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml > @@ -33,6 +33,7 @@ patternProperties: > description: >A DAI managed by this controller > type: object > +additionalProperties: false > > properties: >reg: > diff --git a/Documentation/devicetree/bindings/sound/ti,pcm3168a.yaml > b/Documentation/devicetree/bindings/sound/ti,pcm3168a.yaml > index b6a4360ab845..0b4f003989a4 100644 > --- a/Documentation/devicetree/bindings/sound/ti,pcm3168a.yaml > +++ b/Documentation/devicetree/bindings/sound/ti,pcm3168a.yaml > @@ -60,6 +60,7 @@ properties: > >ports: > $ref: audio-graph-port.yaml#/definitions/port-base > +unevaluatedProperties: false > properties: >port@0: > $ref: audio-graph-port.yaml# At least for sound/fsl,qmc-audio.yaml: Acked-by: Herve Codina Best regards, Hervé
