[linux-5.4 test] 172943: regressions - FAIL
flight 172943 linux-5.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/172943/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 172128 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172128 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 172128 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 172128 Tests which are failing intermittently (not blocking): test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail in 172934 pass in 172943 test-armhf-armhf-examine 8 reboot fail in 172934 pass in 172943 test-arm64-arm64-xl-vhd 13 guest-startfail pass in 172934 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-xl-vhd 14 migrate-support-check fail in 172934 never pass test-arm64-arm64-xl-vhd 15 saverestore-support-check fail in 172934 never pass test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 172108 test-armhf-armhf-xl-credit2 14 guest-start fail like 172128 test-armhf-armhf-xl-credit1 14 guest-start fail like 172128 test-armhf-armhf-xl-multivcpu 14 guest-start fail like 172128 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 172128 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 172128 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 172128 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 172128 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 172128 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 172128 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 172128 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeatfail like 172128 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 172128 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass
[ovmf test] 172949: regressions - FAIL
flight 172949 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/172949/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 172136 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172136 version targeted for testing: ovmf ec87181192f013f4f7ff916b2a39ff2c87b079f3 baseline version: ovmf 444260d45ec2a84e8f8c192b3539a3cd5591d009 Last test of basis 172136 2022-08-04 06:43:42 Z 29 days Failing since172151 2022-08-05 02:40:28 Z 29 days 230 attempts Testing same since 172926 2022-09-02 02:30:44 Z1 days8 attempts People who touched revisions under test: Abdul Lateef Attar Abner Chang Ard Biesheuvel Bob Feng Chasel Chiu Chen, Xiao X Czajkowski, Maciej Dimitrije Pavlov Dun Tan Edward Pickup Foster Nong Gregx Yeh Guo Dong Igor Kulchytskyy James Lu Jeff Brasen Jiaxin Wu Jose Marinho KasimX Liu Kavya Konstantin Aladyshev Kun Qin Liming Gao Liu, Zhiguang Maciej Czajkowski Michael D Kinney Michael Kubacki Pierre Gondois Ray Ni Rebecca Cran Rebecca Cran Sainadh Nagolu Sami Mujawar Shengfengx Xue Wu, Jiaxin Xiao X Chen Yuanhao Xie Zhiguang Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 1457 lines long.)
[ovmf test] 172946: regressions - FAIL
flight 172946 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/172946/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 172136 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172136 version targeted for testing: ovmf ec87181192f013f4f7ff916b2a39ff2c87b079f3 baseline version: ovmf 444260d45ec2a84e8f8c192b3539a3cd5591d009 Last test of basis 172136 2022-08-04 06:43:42 Z 29 days Failing since172151 2022-08-05 02:40:28 Z 28 days 229 attempts Testing same since 172926 2022-09-02 02:30:44 Z0 days7 attempts People who touched revisions under test: Abdul Lateef Attar Abner Chang Ard Biesheuvel Bob Feng Chasel Chiu Chen, Xiao X Czajkowski, Maciej Dimitrije Pavlov Dun Tan Edward Pickup Foster Nong Gregx Yeh Guo Dong Igor Kulchytskyy James Lu Jeff Brasen Jiaxin Wu Jose Marinho KasimX Liu Kavya Konstantin Aladyshev Kun Qin Liming Gao Liu, Zhiguang Maciej Czajkowski Michael D Kinney Michael Kubacki Pierre Gondois Ray Ni Rebecca Cran Rebecca Cran Sainadh Nagolu Sami Mujawar Shengfengx Xue Wu, Jiaxin Xiao X Chen Yuanhao Xie Zhiguang Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 1457 lines long.)
[linux-linus test] 172940: regressions - FAIL
flight 172940 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/172940/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 172133 build-i386-libvirt6 libvirt-buildfail REGR. vs. 172133 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172133 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 172133 Tests which are failing intermittently (not blocking): test-amd64-amd64-dom0pvh-xl-amd 20 guest-localmigrate/x10 fail pass in 172931 test-armhf-armhf-xl-credit2 18 guest-start/debian.repeat fail pass in 172931 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-raw 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 172133 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 172133 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 172133 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 172133 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 172133 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: linux42e66b1cc3a070671001f8a1e933a80818a192bf baseline version: linuxb44f2fd87919b5ae6e1756d4c7ba2cbba22238e1 Last test of basis 172133 2022-08-04 05:14:48 Z 29 days Failing since172152 2022-08-05 04:01:26 Z 28 days 66 attempts Testing same since 172922 2022-09-01 22:42:52 Z1 days3 attempts 1629 people touched revisions under test, not listing them all jobs: build-amd64-xsm
Re: [PATCH 0/7] Fix MISRA C 2012 Rule 20.7 violations
+Roberto I think we need Roberto's advice on Rule 20.7. (Full thread below.) The question is on the interpretation of Rule 20.7. Are parenthesis required by Rule 20.7 in the following cases: - macro parameters used as function arguments - macro parameters used as macro arguments - macro parameter used as array index - macro parameter used as lhs in assignment Some of these cases are interesting because they should function correctly even without parenthesis, hence the discussion. In particular parenthesis don't seem necessary at least for the function argument case. Regardless of the MISRA C interpretation, Xenia noticed that Eclair reports violations on these cases (cppcheck does not, I don't know other checkers). On Fri, 2 Sep 2022, Xenia Ragiadakou wrote: > On 9/2/22 05:07, Stefano Stabellini wrote: > > On Thu, 1 Sep 2022, Bertrand Marquis wrote: > > > Hi Xenia, > > > > > > > On 1 Sep 2022, at 10:27, Xenia Ragiadakou wrote: > > > > > > > > > > > > On 9/1/22 01:35, Stefano Stabellini wrote: > > > > > Patches 1, 4, and 6 are already committed. I plan to commit patches 2, > > > > > 3 > > > > > and 5 in the next couple of days. > > > > > Patch 7 needs further discussions and it is best addressed during the > > > > > next MISRA C sync-up. > > > > > > > > I would like to share here, before the next MISRA C sync, my > > > > understandings that will hopefully resolve a wrong impression of mine, > > > > that I may have spread around, regarding this rule. > > > > There was a misunderstanding regarding the rule 20.7 from my part and I > > > > think that Jan is absolutely right that parenthesizing macro parameters > > > > used as function arguments is not required by the rule. > > > > > > > > The rule 20.7 states "Expressions resulting from the expansion of macro > > > > parameters shall be enclosed in parentheses" and in the rationale of the > > > > rule states "If a macro parameter is not being used as an expression > > > > then the parentheses are not necessary because no operators are > > > > involved.". > > > > > > > > Initially, based on the title, my understanding was that it requires for > > > > the expression resulting from the expansion of the macro to be enclosed > > > > in parentheses. Then, based on the rule explanation and the examples > > > > given, my understanding was that it requires the macro parameters that > > > > are used as expressions to be enclosed in parentheses. > > > > But, after re-thinking about it, the most probable and what makes more > > > > sense, is that it require parentheses around the macro parameters that > > > > are part of an expression and not around those that are used as > > > > expressions. > > > > > > > > Therefore, macro parameters being used as function arguments are not > > > > required to be enclosed in parentheses, because the function arguments > > > > are part of an expression list, not of an expression (comma is evaluated > > > > as separator, not as operator). > > > > While, macro parameters used as rhs and lhs expressions of the > > > > assignment operator are required to be enclosed in parentheses because > > > > they are part of an assignment expression. > > > > > > > > I verified that the violation reported by cppcheck is not due to missing > > > > parentheses around the function argument (though still I have not > > > > understood the origin of the warning). Also, Eclair does not report it. > > > > > > > > Hence, it was a misunderstanding of mine and there is no inconsistency, > > > > with respect to this rule, in adding parentheses around macro parameters > > > > used as rhs of assignments. The rule does not require adding parentheses > > > > around macro parameters used as function arguments and neither cppcheck > > > > nor Eclair report violation for missing parentheses around macro > > > > parameters used as function arguments. > > > > > > > > > Thanks a lot for the detailed explanation :-) > > > > > > What you say does make sense and I agree with your analysis here, only > > > protect when part of an expression and not use as a subsequent parameter > > > (for a function or an other macro). > > > > Yeah I also agree with your analysis, and many thanks for > > double-checking the cppcheck and Eclair's reports. > > Unfortunately in the specific case that I checked, it was not reported because > it was actually an argument to a macro, not a function. > Eclair does report as violations of Rule 20.7 the macro parameters that are > used as function arguments and are not enclosed in parentheses. > > So, one tool reports it as violation and the other one not. > > The same goes, also, for the case where a macro parameter is used as index to > an array. Eclair reports it as violation while cppcheck does not.
Re: [PATCH v3 2/2] xen/pci: replace call to is_memory_hole to pci_check_bar
On Thu, 1 Sep 2022, Rahul Singh wrote: > Replace is_memory_hole call to pci_check_bar as function should check > if device BAR is in defined memory range. Also, add an implementation > for ARM which is required for PCI passthrough. > > On x86, pci_check_bar will call is_memory_hole which will check if BAR > is not overlapping with any memory region defined in the memory map. > > On ARM, pci_check_bar will go through the host bridge ranges and check > if the BAR is in the range of defined ranges. > > Signed-off-by: Rahul Singh > --- > Changes in v3: > - fix minor comments > --- > xen/arch/arm/include/asm/pci.h | 2 ++ > xen/arch/arm/pci/pci-host-common.c | 43 ++ > xen/arch/x86/include/asm/pci.h | 10 +++ > xen/drivers/passthrough/pci.c | 8 +++--- > 4 files changed, 59 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h > index 80a2431804..8cb46f6b71 100644 > --- a/xen/arch/arm/include/asm/pci.h > +++ b/xen/arch/arm/include/asm/pci.h > @@ -126,6 +126,8 @@ int pci_host_iterate_bridges_and_count(struct domain *d, > > int pci_host_bridge_mappings(struct domain *d); > > +bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end); > + > #else /*!CONFIG_HAS_PCI*/ > > struct arch_pci_dev { }; > diff --git a/xen/arch/arm/pci/pci-host-common.c > b/xen/arch/arm/pci/pci-host-common.c > index 89ef30028e..0eb121666d 100644 > --- a/xen/arch/arm/pci/pci-host-common.c > +++ b/xen/arch/arm/pci/pci-host-common.c > @@ -24,6 +24,16 @@ > > #include > > +/* > + * struct to hold pci device bar. > + */ > +struct pdev_bar > +{ > +mfn_t start; > +mfn_t end; > +bool is_valid; > +}; > + > /* > * List for all the pci host bridges. > */ > @@ -363,6 +373,39 @@ int __init pci_host_bridge_mappings(struct domain *d) > return 0; > } > > +static int is_bar_valid(const struct dt_device_node *dev, > +uint64_t addr, uint64_t len, void *data) > +{ > +struct pdev_bar *bar_data = data; > +unsigned long s = mfn_x(bar_data->start); > +unsigned long e = mfn_x(bar_data->end); > + > +if ( (s <= e) && (s >= PFN_DOWN(addr)) && (e <= PFN_UP(addr + len - 1)) ) > +bar_data->is_valid = true; This patch looks good and you addressed all Jan's comment well. Before I ack it, one question. I know that you made this change to address Jan's comment but using PFN_DOWN for the (s >= PFN_DOWN(addr)) check and PFN_UP for the (e <= PFN_UP(addr + len - 1)) check means that we are relaxing the requirements, aren't we? I know that this discussion is a bit pointless because addr and len should always be page aligned, and if they weren't it would be a mistake. But assuming that they are not page aligned, wouldn't we want this check to be a strict as possible? Wouldn't we want to ensure that the [s,e] range is a strict subset of [addr,addr+len-1] ? If so we would need to do the following instead: if ( (s <= e) && (s >= PFN_UP(addr)) && (e <= PFN_DOWN(addr + len - 1)) ) bar_data->is_valid = true;
Re: [for-4.17 3/3] automation: Add a new job for testing boot time cpupools on arm64
On Fri, 2 Sep 2022, Michal Orzel wrote: > Add a new test job qemu-smoke-arm64-gcc-boot-cpupools that will execute > script qemu-smoke-arm64.sh to test boot time cpupools feature. > Enable CONFIG_BOOT_TIME_CPUPOOLS for the arm64 build and add a new test > case in qemu-smoke-arm64.sh that if selected will: > - create a device tree cpupool node with cpu@1 > - assign created cpupool to domU0 > - add a check in dom0 xen.start to see if domU is assigned a Pool-1 > > Take the opportunity to refactor the qemu-smoke-arm64.sh script as > follows: > - use domU_check to store the test's commands to be run from domU > - use dom0_check to store the test's commands to be run from dom0 > - use fdtput instead of sed to perform dtb modifications > - use more meaningful messages for "passed" variable. This way we can >grep for messages reported either by domU or dom0 and get rid of >assumption that tests can only be run from domU > > Signed-off-by: Michal Orzel > --- > automation/gitlab-ci/test.yaml | 19 +++ > automation/scripts/build | 3 ++- > automation/scripts/qemu-smoke-arm64.sh | 33 +++--- > 3 files changed, 45 insertions(+), 10 deletions(-) > > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > index 07209820b474..d899b3bdbf7a 100644 > --- a/automation/gitlab-ci/test.yaml > +++ b/automation/gitlab-ci/test.yaml > @@ -100,6 +100,25 @@ qemu-smoke-arm64-gcc-staticmem: >tags: > - arm64 > > +qemu-smoke-arm64-gcc-boot-cpupools: > + extends: .test-jobs-common > + variables: > +CONTAINER: debian:unstable-arm64v8 > + script: > +- ./automation/scripts/qemu-smoke-arm64.sh boot-cpupools 2>&1 | tee > qemu-smoke-arm64.log > + needs: > +- alpine-3.12-gcc-arm64 > +- alpine-3.12-arm64-rootfs-export > +- kernel-5.19-arm64-export > +- qemu-system-aarch64-6.0.0-arm64-export > + artifacts: > +paths: > + - smoke.serial > + - '*.log' > +when: always > + tags: > +- arm64 > + > qemu-smoke-arm32-gcc: >extends: .test-jobs-common >variables: > diff --git a/automation/scripts/build b/automation/scripts/build > index 2b9f2d2b541a..2f15ab3198e6 100755 > --- a/automation/scripts/build > +++ b/automation/scripts/build > @@ -19,7 +19,8 @@ else > echo " > CONFIG_EXPERT=y > CONFIG_UNSUPPORTED=y > -CONFIG_STATIC_MEMORY=y" > xen/.config > +CONFIG_STATIC_MEMORY=y > +CONFIG_BOOT_TIME_CPUPOOLS=y" > xen/.config > make -j$(nproc) -C xen olddefconfig > else > make -j$(nproc) -C xen defconfig > diff --git a/automation/scripts/qemu-smoke-arm64.sh > b/automation/scripts/qemu-smoke-arm64.sh > index 7ac96027760d..c2184850293c 100755 > --- a/automation/scripts/qemu-smoke-arm64.sh > +++ b/automation/scripts/qemu-smoke-arm64.sh > @@ -4,20 +4,22 @@ set -ex > > test_variant=$1 > > -passed="passed" > -check=" > +if [ -z "${test_variant}" ]; then > +passed="ping test passed" > +domU_check=" > until ifconfig eth0 192.168.0.2 &> /dev/null && ping -c 10 192.168.0.1; do > sleep 30 > done > echo \"${passed}\" > " > +fi > > if [[ "${test_variant}" == "static-mem" ]]; then > # Memory range that is statically allocated to DOM1 > domu_base="5000" > domu_size="1000" > passed="${test_variant} test passed" > -check=" > +domU_check=" > current=\$(hexdump -e '16/1 \"%02x\"' > /proc/device-tree/memory@${domu_base}/reg 2>/dev/null) > expected=$(printf \"%016x%016x\" 0x${domu_base} 0x${domu_size}) > if [[ \"\${expected}\" == \"\${current}\" ]]; then > @@ -42,11 +44,23 @@ curl -fsSLO > https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom > -cpu cortex-a57 -machine type=virt \ > -m 1024 -smp 2 -display none \ > -machine dumpdtb=binaries/virt-gicv2.dtb > -# XXX disable pl061 to avoid Linux crash > -dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts > -sed 's/compatible = "arm,pl061.*/status = "disabled";/g' > binaries/virt-gicv2.dts > binaries/virt-gicv2-edited.dts > -dtc -I dts -O dtb binaries/virt-gicv2-edited.dts > binaries/virt-gicv2.dtb > > +# XXX disable pl061 to avoid Linux crash > +fdtput binaries/virt-gicv2.dtb -p -t s /pl061@903 status disabled Currently this test fails with: + fdtput binaries/virt-gicv2.dtb -p -t s /pl061@903 status disabled + [[ boot-cpupools == \b\o\o\t\-\c\p\u\p\o\o\l\s ]] ++ fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle Error at 'phandle': FDT_ERR_NOTFOUND Given my other comment below, I would leave this code as is. > +if [[ "${test_variant}" == "boot-cpupools" ]]; then > +# Create cpupool node and assign it to domU0 > +cpu_phandle="$(fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle)" > +cpupool_phandle="0xff" > +fdtput binaries/virt-gicv2.dtb -p -t s /chosen/cpupool compatible > xen,cpupool > +fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool cpupool-cpus > $cpu_phandle > +fdtput
[ovmf test] 172944: regressions - FAIL
flight 172944 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/172944/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 172136 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172136 version targeted for testing: ovmf ec87181192f013f4f7ff916b2a39ff2c87b079f3 baseline version: ovmf 444260d45ec2a84e8f8c192b3539a3cd5591d009 Last test of basis 172136 2022-08-04 06:43:42 Z 29 days Failing since172151 2022-08-05 02:40:28 Z 28 days 228 attempts Testing same since 172926 2022-09-02 02:30:44 Z0 days6 attempts People who touched revisions under test: Abdul Lateef Attar Abner Chang Ard Biesheuvel Bob Feng Chasel Chiu Chen, Xiao X Czajkowski, Maciej Dimitrije Pavlov Dun Tan Edward Pickup Foster Nong Gregx Yeh Guo Dong Igor Kulchytskyy James Lu Jeff Brasen Jiaxin Wu Jose Marinho KasimX Liu Kavya Konstantin Aladyshev Kun Qin Liming Gao Liu, Zhiguang Maciej Czajkowski Michael D Kinney Michael Kubacki Pierre Gondois Ray Ni Rebecca Cran Rebecca Cran Sainadh Nagolu Sami Mujawar Shengfengx Xue Wu, Jiaxin Xiao X Chen Yuanhao Xie Zhiguang Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 1457 lines long.)
Re: [PATCH v2 00/10] xen/arm: smmuv3: Merge Linux fixes to Xen
I checked all the patches against the originals. I had comments on patches 3,4,5. You can add: Acked-by: Stefano Stabellini to all the others (1,2,6,7,8,9,10). On Fri, 2 Sep 2022, Rahul Singh wrote: > This patch series merge the applicable Linux fixes to Xen. > > Bixuan Cui (1): > xen/arm: smmuv3: Change *array into *const array > > Christophe JAILLET (1): > xen/arm: smmuv3: Avoid open coded arithmetic in memory allocation > > Gustavo A. R. Silva (1): > xen/arm: smmuv3: Fix fall-through warning for Clang > > Jean-Philippe Brucker (2): > xen/arm: smmuv3: Fix endianness annotations > xen/arm: smmuv3: Move definitions to a header > > Robin Murphy (1): > xen/arm: smmuv3: Remove the page 1 fixup > > Zenghui Yu (2): > xen/arm: smmuv3: Fix l1 stream table size in the error message > xen/arm: smmuv3: Remove the unused fields for PREFETCH_CONFIG command > > Zhen Lei (1): > xen/arm: smmuv3: Remove unnecessary oom message > > Zhou Wang (1): > xen/arm: smmuv3: Ensure queue is read after updating prod pointer > > xen/arch/arm/include/asm/system.h | 1 + > xen/drivers/passthrough/arm/smmu-v3.c | 742 ++ > xen/drivers/passthrough/arm/smmu-v3.h | 672 +++ > 3 files changed, 708 insertions(+), 707 deletions(-) > create mode 100644 xen/drivers/passthrough/arm/smmu-v3.h > > -- > 2.25.1 >
Re: [PATCH v2 04/10] xen/arm: smmuv3: Move definitions to a header
On Fri, 2 Sep 2022, Rahul Singh wrote: > From: Jean-Philippe Brucker > > Backport Linux commit e881e7839fba. This is the clean backport without > any changes. I don't think we can say that this is a clean backport because there are differences between the two patches. That said, it is just code movement, it is similar to the original patch, and it still compiles. So I think we should change the commit message not to say that it is a clean backport, but other than that is fine. > Allow sharing structure definitions with the upcoming SVA support for > Arm SMMUv3, by moving them to a separate header. We could surgically > extract only what is needed but keeping all definitions in one place > looks nicer. > > Signed-off-by: Jean-Philippe Brucker > Reviewed-by: Eric Auger > Reviewed-by: Jonathan Cameron > Link: > https://lore.kernel.org/r/20200918101852.582559-8-jean-phili...@linaro.org > Signed-off-by: Will Deacon > Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > e881e7839fba > Signed-off-by: Rahul Singh > --- > Changes in v2: > - fix commit msg > - also move struct definition to header file to sync with Linux patch > --- > xen/drivers/passthrough/arm/smmu-v3.c | 666 + > xen/drivers/passthrough/arm/smmu-v3.h | 674 ++ > 2 files changed, 675 insertions(+), 665 deletions(-) > create mode 100644 xen/drivers/passthrough/arm/smmu-v3.h > > diff --git a/xen/drivers/passthrough/arm/smmu-v3.c > b/xen/drivers/passthrough/arm/smmu-v3.c > index cee13d1fc7..85ad066266 100644 > --- a/xen/drivers/passthrough/arm/smmu-v3.c > +++ b/xen/drivers/passthrough/arm/smmu-v3.c > @@ -90,6 +90,7 @@ > #include > #include > > +#include "smmu-v3.h" > > #define ARM_SMMU_VTCR_SH_IS 3 > #define ARM_SMMU_VTCR_RGN_WBWA 1 > @@ -102,21 +103,10 @@ > #define ARM_SMMU_VTCR_PS_48_BIT 0x5ULL > #define ARM_SMMU_VTCR_PS_52_BIT 0x6ULL > > -/* Linux compatibility functions. */ > -typedef paddr_t dma_addr_t; > -typedef paddr_t phys_addr_t; > -typedef unsigned int gfp_t; > - > #define platform_device device > > #define GFP_KERNEL 0 > > -/* Alias to Xen lock functions */ > -#define mutex spinlock > -#define mutex_init spin_lock_init > -#define mutex_lock spin_lock > -#define mutex_unlock spin_unlock > - > /* Device logger functions */ > #define dev_name(dev)dt_node_full_name(dev->of_node) > #define dev_dbg(dev, fmt, ...) \ > @@ -157,12 +147,6 @@ typedef unsigned int gfp_t; > #define readl_relaxed_poll_timeout(addr, val, cond, delay_us, timeout_us) > \ > readx_poll_timeout(readl_relaxed, addr, val, cond, delay_us, timeout_us) > > -#define FIELD_PREP(_mask, _val) \ > - (((typeof(_mask))(_val) << (ffs64(_mask) - 1)) & (_mask)) > - > -#define FIELD_GET(_mask, _reg) \ > - ((typeof(_mask))(((_reg) & (_mask)) >> (ffs64(_mask) - 1))) > - > /* > * Helpers for DMA allocation. Just the function name is reused for > * porting code, these allocation are not managed allocations > @@ -195,27 +179,6 @@ static void *dmam_alloc_coherent(struct device *dev, > size_t size, > return vaddr; > } > > - > -/* Xen specific code. */ > -struct iommu_domain { > - /* Runtime SMMU configuration for this iommu_domain */ > - atomic_tref; > - /* > - * Used to link iommu_domain contexts for a same domain. > - * There is at least one per-SMMU to used by the domain. > - */ > - struct list_headlist; > -}; > - > -/* Describes information required for a Xen domain */ > -struct arm_smmu_xen_domain { > - spinlock_t lock; > - > - /* List of iommu domains associated to this domain */ > - struct list_headcontexts; > -}; > - > - > /* Keep a list of devices associated with this driver */ > static DEFINE_SPINLOCK(arm_smmu_devices_lock); > static LIST_HEAD(arm_smmu_devices); > @@ -259,635 +222,8 @@ static int platform_get_irq_byname_optional(struct > device *dev, > } > > /* Start of Linux SMMUv3 code */ > -/* MMIO registers */ > -#define ARM_SMMU_IDR00x0 > -#define IDR0_ST_LVL GENMASK(28, 27) > -#define IDR0_ST_LVL_2LVL 1 > -#define IDR0_STALL_MODEL GENMASK(25, 24) > -#define IDR0_STALL_MODEL_STALL 0 > -#define IDR0_STALL_MODEL_FORCE 2 > -#define IDR0_TTENDIANGENMASK(22, 21) > -#define IDR0_TTENDIAN_MIXED 0 > -#define IDR0_TTENDIAN_LE 2 > -#define IDR0_TTENDIAN_BE 3 > -#define IDR0_CD2L(1 << 19) > -#define IDR0_VMID16 (1 << 18) > -#define IDR0_PRI (1 << 16) > -#define IDR0_SEV (1 << 14) > -#define IDR0_MSI (1 << 13) >
Re: [PATCH v2 03/10] xen/arm: smmuv3: Ensure queue is read after updating prod pointer
On Fri, 2 Sep 2022, Rahul Singh wrote: > From: Zhou Wang > > Backport Linux commit a76a3f2c. This is the clean backport without > any changes. > > Reading the 'prod' MMIO register in order to determine whether or > not there is valid data beyond 'cons' for a given queue does not > provide sufficient dependency ordering, as the resulting access is > address dependent only on 'cons' and can therefore be speculated > ahead of time, potentially allowing stale data to be read by the > CPU. > > Use readl() instead of readl_relaxed() when updating the shadow copy > of the 'prod' pointer, so that all speculated memory reads from the > corresponding queue can occur only from valid slots. > > Signed-off-by: Zhou Wang > Link: > https://lore.kernel.org/r/1601281922-117296-1-git-send-email-wangzh...@hisilicon.com > [will: Use readl() instead of explicit barrier. Update 'cons' side to match.] > Signed-off-by: Will Deacon > Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > a76a3f2c > Signed-off-by: Rahul Singh > --- > Changes in v2: > - fix commit msg > - add _iomb changes also from the origin patch > --- > xen/arch/arm/include/asm/system.h | 1 + > xen/drivers/passthrough/arm/smmu-v3.c | 11 +-- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/include/asm/system.h > b/xen/arch/arm/include/asm/system.h > index 65d5c8e423..fe27cf8c5e 100644 > --- a/xen/arch/arm/include/asm/system.h > +++ b/xen/arch/arm/include/asm/system.h > @@ -29,6 +29,7 @@ > #endif > > #define smp_wmb() dmb(ishst) > +#define __iomb()dmb(osh) We don't have any other #define starting with __ in system.h. I wonder if we should call this macro differently or simply iomb(). > #define smp_mb__before_atomic()smp_mb() > #define smp_mb__after_atomic() smp_mb() > diff --git a/xen/drivers/passthrough/arm/smmu-v3.c > b/xen/drivers/passthrough/arm/smmu-v3.c > index 64d39bb4d3..cee13d1fc7 100644 > --- a/xen/drivers/passthrough/arm/smmu-v3.c > +++ b/xen/drivers/passthrough/arm/smmu-v3.c > @@ -951,7 +951,7 @@ static void queue_sync_cons_out(struct arm_smmu_queue *q) >* Ensure that all CPU accesses (reads and writes) to the queue >* are complete before we update the cons pointer. >*/ > - mb(); > + __iomb(); > writel_relaxed(q->llq.cons, q->cons_reg); > } > > @@ -963,8 +963,15 @@ static void queue_inc_cons(struct arm_smmu_ll_queue *q) > > static int queue_sync_prod_in(struct arm_smmu_queue *q) > { > + u32 prod; > int ret = 0; > - u32 prod = readl_relaxed(q->prod_reg); > + > + /* > + * We can't use the _relaxed() variant here, as we must prevent > + * speculative reads of the queue before we have determined that > + * prod has indeed moved. > + */ > + prod = readl(q->prod_reg); > > if (Q_OVF(prod) != Q_OVF(q->llq.prod)) > ret = -EOVERFLOW; > -- > 2.25.1 >
Re: [PATCH v2 05/10] xen/arm: smmuv3: Remove the page 1 fixup
On Fri, 2 Sep 2022, Rahul Singh wrote: > From: Robin Murphy > > Backport Linux commit 86d2d9214880. This is the clean backport without > any changes. > > Since we now keep track of page 1 via a separate pointer that > already encapsulates aliasing to page 0 as necessary, we can remove > the clunky fixup routine and simply use the relevant bases directly. > The current architecture spec (IHI0070D.a) defines > SMMU_{EVENTQ,PRIQ}_{PROD,CONS} as offsets relative to page 1, so the > cleanup represents a little bit of convergence as well as just > lines of code saved. > > Signed-off-by: Robin Murphy > Signed-off-by: Will Deacon > Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > 86d2d9214880 > Signed-off-by: Rahul Singh > --- > Changes in v2: > - fix commit msg. > --- > xen/drivers/passthrough/arm/smmu-v3.c | 42 ++- > xen/drivers/passthrough/arm/smmu-v3.h | 8 ++--- > 2 files changed, 20 insertions(+), 30 deletions(-) > > diff --git a/xen/drivers/passthrough/arm/smmu-v3.c > b/xen/drivers/passthrough/arm/smmu-v3.c > index 85ad066266..f5485a8a1c 100644 > --- a/xen/drivers/passthrough/arm/smmu-v3.c > +++ b/xen/drivers/passthrough/arm/smmu-v3.c > @@ -235,15 +235,6 @@ static struct arm_smmu_option_prop arm_smmu_options[] = { > { 0, NULL}, > }; > > -static inline void __iomem *arm_smmu_page1_fixup(unsigned long offset, > - struct arm_smmu_device *smmu) > -{ > - if (offset > SZ_64K) > - return smmu->page1 + offset - SZ_64K; > - > - return smmu->base + offset; > -} > - > static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom) > { > return container_of(dom, struct arm_smmu_domain, domain); > @@ -1578,6 +1569,7 @@ static int arm_smmu_dt_xlate(struct device *dev, > /* Probing and initialisation functions */ > static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu, > struct arm_smmu_queue *q, > +void __iomem *page, > unsigned long prod_off, > unsigned long cons_off, > size_t dwords, const char *name) > @@ -1606,8 +1598,8 @@ static int arm_smmu_init_one_queue(struct > arm_smmu_device *smmu, >1 << q->llq.max_n_shift, name); > } > > - q->prod_reg = arm_smmu_page1_fixup(prod_off, smmu); > - q->cons_reg = arm_smmu_page1_fixup(cons_off, smmu); > + q->prod_reg = page + prod_off; > + q->cons_reg = page + prod_off; In the original patch it was: - q->prod_reg = arm_smmu_page1_fixup(prod_off, smmu); - q->cons_reg = arm_smmu_page1_fixup(cons_off, smmu); + q->prod_reg = page + prod_off; + q->cons_reg = page + cons_off; Specifically the second line seems to be wrong here?
[qemu-mainline test] 172937: regressions - FAIL
flight 172937 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/172937/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172123 build-i386-libvirt6 libvirt-buildfail REGR. vs. 172123 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 172123 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 172123 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 172123 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 172123 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 172123 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 172123 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 172123 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: qemuu7dd9d7e0bd29abf590d1ac235c0a00606ef81153 baseline version: qemuu2480f3bbd03814b0651a1f74959f5c6631ee5819 Last test of basis 172123 2022-08-03 18:10:07 Z 30 days Failing since172148 2022-08-04 21:39:38 Z 29 days 66 attempts Testing same since 172928 2022-09-02 02:44:00 Z0 days2 attempts
Re: [RFC PATCH 00/30] Code tagging framework and applications
On 9/2/22 2:05 PM, Kent Overstreet wrote: > On Fri, Sep 02, 2022 at 01:53:53PM -0600, Jens Axboe wrote: >> I've complained about memcg accounting before, the slowness of it is why >> io_uring works around it by caching. Anything we account we try NOT do >> in the fast path because of it, the slowdown is considerable. > > I'm with you on that, it definitely raises an eyebrow. > >> You care about efficiency now? I thought that was relegated to >> irrelevant 10M IOPS cases. > > I always did, it's just not the only thing I care about. It's not the only thing anyone cares about. -- Jens Axboe
Re: [RFC PATCH 00/30] Code tagging framework and applications
On Fri, Sep 02, 2022 at 01:53:53PM -0600, Jens Axboe wrote: > I've complained about memcg accounting before, the slowness of it is why > io_uring works around it by caching. Anything we account we try NOT do > in the fast path because of it, the slowdown is considerable. I'm with you on that, it definitely raises an eyebrow. > You care about efficiency now? I thought that was relegated to > irrelevant 10M IOPS cases. I always did, it's just not the only thing I care about.
[linux-5.4 test] 172934: regressions - FAIL
flight 172934 linux-5.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/172934/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 172128 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172128 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 172128 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 172128 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-debianhvm-amd64 20 guest-start/debianhvm.repeat fail in 172924 pass in 172934 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail pass in 172924 test-armhf-armhf-examine 8 reboot fail pass in 172924 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 18 guest-start/debian.repeat fail in 172924 like 172108 test-armhf-armhf-xl-vhd 13 guest-start fail in 172924 like 172108 test-armhf-armhf-xl-credit2 15 migrate-support-check fail in 172924 never pass test-armhf-armhf-xl-credit2 16 saverestore-support-check fail in 172924 never pass test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 172108 test-armhf-armhf-xl-credit2 14 guest-start fail like 172128 test-armhf-armhf-xl-credit1 14 guest-start fail like 172128 test-armhf-armhf-xl-multivcpu 14 guest-start fail like 172128 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 172128 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 172128 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 172128 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 172128 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 172128 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 172128 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 172128 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeatfail like 172128 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 172128 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never
Re: [RFC PATCH 00/30] Code tagging framework and applications
On 9/2/22 1:48 PM, Kent Overstreet wrote: > On Fri, Sep 02, 2022 at 06:02:12AM -0600, Jens Axboe wrote: >> On 9/1/22 7:04 PM, Roman Gushchin wrote: >>> On Thu, Sep 01, 2022 at 08:17:47PM -0400, Kent Overstreet wrote: On Thu, Sep 01, 2022 at 03:53:57PM -0700, Roman Gushchin wrote: > I'd suggest to run something like iperf on a fast hardware. And maybe some > io_uring stuff too. These are two places which were historically most > sensitive > to the (kernel) memory accounting speed. I'm getting wildly inconsistent results with iperf. io_uring-echo-server and rust_echo_bench gets me: Benchmarking: 127.0.0.1:12345 50 clients, running 512 bytes, 60 sec. Without alloc tagging: 120547 request/sec With: 116748 request/sec https://github.com/frevib/io_uring-echo-server https://github.com/haraldh/rust_echo_bench How's that look to you? Close enough? :) >>> >>> Yes, this looks good (a bit too good). >>> >>> I'm not that familiar with io_uring, Jens and Pavel should have a better >>> idea >>> what and how to run (I know they've workarounded the kernel memory >>> accounting >>> because of the performance in the past, this is why I suspect it might be an >>> issue here as well). >> >> io_uring isn't alloc+free intensive on a per request basis anymore, it >> would not be a good benchmark if the goal is to check for regressions in >> that area. > > Good to know. The benchmark is still a TCP benchmark though, so still useful. > > Matthew suggested > while true; do echo 1 >/tmp/foo; rm /tmp/foo; done > > I ran that on tmpfs, and the numbers with and without alloc tagging were > statistically equal - there was a fair amount of variation, it wasn't a super > controlled test, anywhere from 38-41 seconds with 10 iterations (and alloc > tagging was some of the faster runs). > > But with memcg off, it ran in 32-33 seconds. We're piggybacking on the same > mechanism memcg uses for stashing per-object pointers, so it looks like that's > the bigger cost. I've complained about memcg accounting before, the slowness of it is why io_uring works around it by caching. Anything we account we try NOT do in the fast path because of it, the slowdown is considerable. You care about efficiency now? I thought that was relegated to irrelevant 10M IOPS cases. -- Jens Axboe
[ovmf test] 172942: regressions - FAIL
flight 172942 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/172942/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 172136 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172136 version targeted for testing: ovmf ec87181192f013f4f7ff916b2a39ff2c87b079f3 baseline version: ovmf 444260d45ec2a84e8f8c192b3539a3cd5591d009 Last test of basis 172136 2022-08-04 06:43:42 Z 29 days Failing since172151 2022-08-05 02:40:28 Z 28 days 227 attempts Testing same since 172926 2022-09-02 02:30:44 Z0 days5 attempts People who touched revisions under test: Abdul Lateef Attar Abner Chang Ard Biesheuvel Bob Feng Chasel Chiu Chen, Xiao X Czajkowski, Maciej Dimitrije Pavlov Dun Tan Edward Pickup Foster Nong Gregx Yeh Guo Dong Igor Kulchytskyy James Lu Jeff Brasen Jiaxin Wu Jose Marinho KasimX Liu Kavya Konstantin Aladyshev Kun Qin Liming Gao Liu, Zhiguang Maciej Czajkowski Michael D Kinney Michael Kubacki Pierre Gondois Ray Ni Rebecca Cran Rebecca Cran Sainadh Nagolu Sami Mujawar Shengfengx Xue Wu, Jiaxin Xiao X Chen Yuanhao Xie Zhiguang Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 1457 lines long.)
Re: [RFC PATCH 00/30] Code tagging framework and applications
On Fri, Sep 02, 2022 at 06:02:12AM -0600, Jens Axboe wrote: > On 9/1/22 7:04 PM, Roman Gushchin wrote: > > On Thu, Sep 01, 2022 at 08:17:47PM -0400, Kent Overstreet wrote: > >> On Thu, Sep 01, 2022 at 03:53:57PM -0700, Roman Gushchin wrote: > >>> I'd suggest to run something like iperf on a fast hardware. And maybe some > >>> io_uring stuff too. These are two places which were historically most > >>> sensitive > >>> to the (kernel) memory accounting speed. > >> > >> I'm getting wildly inconsistent results with iperf. > >> > >> io_uring-echo-server and rust_echo_bench gets me: > >> Benchmarking: 127.0.0.1:12345 > >> 50 clients, running 512 bytes, 60 sec. > >> > >> Without alloc tagging: 120547 request/sec > >> With: 116748 request/sec > >> > >> https://github.com/frevib/io_uring-echo-server > >> https://github.com/haraldh/rust_echo_bench > >> > >> How's that look to you? Close enough? :) > > > > Yes, this looks good (a bit too good). > > > > I'm not that familiar with io_uring, Jens and Pavel should have a better > > idea > > what and how to run (I know they've workarounded the kernel memory > > accounting > > because of the performance in the past, this is why I suspect it might be an > > issue here as well). > > io_uring isn't alloc+free intensive on a per request basis anymore, it > would not be a good benchmark if the goal is to check for regressions in > that area. Good to know. The benchmark is still a TCP benchmark though, so still useful. Matthew suggested while true; do echo 1 >/tmp/foo; rm /tmp/foo; done I ran that on tmpfs, and the numbers with and without alloc tagging were statistically equal - there was a fair amount of variation, it wasn't a super controlled test, anywhere from 38-41 seconds with 10 iterations (and alloc tagging was some of the faster runs). But with memcg off, it ran in 32-33 seconds. We're piggybacking on the same mechanism memcg uses for stashing per-object pointers, so it looks like that's the bigger cost.
Re: [PATCH 0/7] Fix MISRA C 2012 Rule 20.7 violations
On 9/2/22 05:07, Stefano Stabellini wrote: On Thu, 1 Sep 2022, Bertrand Marquis wrote: Hi Xenia, On 1 Sep 2022, at 10:27, Xenia Ragiadakou wrote: On 9/1/22 01:35, Stefano Stabellini wrote: Patches 1, 4, and 6 are already committed. I plan to commit patches 2, 3 and 5 in the next couple of days. Patch 7 needs further discussions and it is best addressed during the next MISRA C sync-up. I would like to share here, before the next MISRA C sync, my understandings that will hopefully resolve a wrong impression of mine, that I may have spread around, regarding this rule. There was a misunderstanding regarding the rule 20.7 from my part and I think that Jan is absolutely right that parenthesizing macro parameters used as function arguments is not required by the rule. The rule 20.7 states "Expressions resulting from the expansion of macro parameters shall be enclosed in parentheses" and in the rationale of the rule states "If a macro parameter is not being used as an expression then the parentheses are not necessary because no operators are involved.". Initially, based on the title, my understanding was that it requires for the expression resulting from the expansion of the macro to be enclosed in parentheses. Then, based on the rule explanation and the examples given, my understanding was that it requires the macro parameters that are used as expressions to be enclosed in parentheses. But, after re-thinking about it, the most probable and what makes more sense, is that it require parentheses around the macro parameters that are part of an expression and not around those that are used as expressions. Therefore, macro parameters being used as function arguments are not required to be enclosed in parentheses, because the function arguments are part of an expression list, not of an expression (comma is evaluated as separator, not as operator). While, macro parameters used as rhs and lhs expressions of the assignment operator are required to be enclosed in parentheses because they are part of an assignment expression. I verified that the violation reported by cppcheck is not due to missing parentheses around the function argument (though still I have not understood the origin of the warning). Also, Eclair does not report it. Hence, it was a misunderstanding of mine and there is no inconsistency, with respect to this rule, in adding parentheses around macro parameters used as rhs of assignments. The rule does not require adding parentheses around macro parameters used as function arguments and neither cppcheck nor Eclair report violation for missing parentheses around macro parameters used as function arguments. Thanks a lot for the detailed explanation :-) What you say does make sense and I agree with your analysis here, only protect when part of an expression and not use as a subsequent parameter (for a function or an other macro). Yeah I also agree with your analysis, and many thanks for double-checking the cppcheck and Eclair's reports. Unfortunately in the specific case that I checked, it was not reported because it was actually an argument to a macro, not a function. Eclair does report as violations of Rule 20.7 the macro parameters that are used as function arguments and are not enclosed in parentheses. So, one tool reports it as violation and the other one not. The same goes, also, for the case where a macro parameter is used as index to an array. Eclair reports it as violation while cppcheck does not. -- Xenia
[ovmf test] 172938: regressions - FAIL
flight 172938 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/172938/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 172136 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172136 version targeted for testing: ovmf ec87181192f013f4f7ff916b2a39ff2c87b079f3 baseline version: ovmf 444260d45ec2a84e8f8c192b3539a3cd5591d009 Last test of basis 172136 2022-08-04 06:43:42 Z 29 days Failing since172151 2022-08-05 02:40:28 Z 28 days 226 attempts Testing same since 172926 2022-09-02 02:30:44 Z0 days4 attempts People who touched revisions under test: Abdul Lateef Attar Abner Chang Ard Biesheuvel Bob Feng Chasel Chiu Chen, Xiao X Czajkowski, Maciej Dimitrije Pavlov Dun Tan Edward Pickup Foster Nong Gregx Yeh Guo Dong Igor Kulchytskyy James Lu Jeff Brasen Jiaxin Wu Jose Marinho KasimX Liu Kavya Konstantin Aladyshev Kun Qin Liming Gao Liu, Zhiguang Maciej Czajkowski Michael D Kinney Michael Kubacki Pierre Gondois Ray Ni Rebecca Cran Rebecca Cran Sainadh Nagolu Sami Mujawar Shengfengx Xue Wu, Jiaxin Xiao X Chen Yuanhao Xie Zhiguang Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 1457 lines long.)
Re: [PATCH] xen-pcifront: Handle missed Connected state
The conventional style for subject (from "git log --oneline") is: xen/pcifront: Handle ... On Mon, Aug 29, 2022 at 11:15:36AM -0400, Jason Andryuk wrote: > An HVM guest with linux stubdom and 2 PCI devices failed to start as "stubdom" might be handy shorthand in the Xen world, but I think it would be nice to consistently spell out "stubdomain" since you use both forms randomly in this commit log and newbies like me have to wonder whether they're the same or different. > libxl timed out waiting for the PCI devices to be added. It happens > intermittently but with some regularity. libxl wrote the two xenstore > entries for the devices, but then timed out waiting for backend state 4 > (Connected) - the state stayed at 7 (Reconfiguring). (PCI passthrough > to an HVM with stubdomain is PV passthrough to the stubdomain and then > HVM passthrough with the QEMU inside the stubdomain.) > > The stubdom kernel never printed "pcifront pci-0: Installing PCI > frontend", so it seems to have missed state 4 which would have > called pcifront_try_connect -> pcifront_connect_and_init_dma Add "()" after function names for clarity. > Have pcifront_detach_devices special-case state Initialised and call > pcifront_connect_and_init_dma. Don't use pcifront_try_connect because > that sets the xenbus state which may throw off the backend. After > connecting, skip the remainder of detach_devices since none have been > initialized yet. When the backend switches to Reconfigured, > pcifront_attach_devices will pick them up again. Bjorn
[linux-linus test] 172931: regressions - FAIL
flight 172931 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/172931/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 172133 build-i386-libvirt6 libvirt-buildfail REGR. vs. 172133 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172133 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 172133 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-raw 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 172133 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 172133 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 172133 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 172133 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 172133 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: linux42e66b1cc3a070671001f8a1e933a80818a192bf baseline version: linuxb44f2fd87919b5ae6e1756d4c7ba2cbba22238e1 Last test of basis 172133 2022-08-04 05:14:48 Z 29 days Failing since172152 2022-08-05 04:01:26 Z 28 days 65 attempts Testing same since 172922 2022-09-01 22:42:52 Z0 days2 attempts 1629 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64
Re: [PATCH v3 7/7] xen/arm: introduce new xen,enhanced property value
On 02/09/2022 16:54, Rahul Singh wrote: Hi Julien, Hi Rahul, On 2 Sep 2022, at 4:05 pm, Julien Grall wrote: Hi Bertrand, On 02/09/2022 15:51, Bertrand Marquis wrote: On 1 Sep 2022, at 19:15, Julien Grall wrote: AFAIU, it is not possible to have *_xenstore = true and *_enhanced = false. I think it would be clearer if ``dom0less_enhanced`` is turned to an enum with 3 values: - None - NOXENSTORE/BASIC - FULLY_ENHANCED If we want to be future proof, I would use a field 'flags' where non-zero means enhanced. Each bit would indicate which features of Xen is exposed. I think that could be a good solution if we do it this way: - define a dom0less feature field and have defines like the following: #define DOM0LESS_GNTTAB #define DOM0LESS_EVENTCHN #define DOM0LESS_XENSTORE > - define dom0less enhanced as the right combination: #define DOM0LESS_ENHANCED = (DOM0LESS_GNTTAB| DOM0LESS_EVENTCHN| DOM0LESS_XENSTORE) I would rather introduce DOM0LESS_ENHANCED_BASIC (or similar) instead of defining a bit for gnttab and evtchn. This will avoid the question of why we are introducing bits for both features but not the hypercall... As this is an internal interface, it would be easier to modify afterwards. How about this? /* * List of possible features for dom0less domUs * * DOM0LESS_ENHANCED_BASIC: Xen PV interfaces, including grant-table and * evtchn, will be enabled for the VM. Technically, the guest can already use the grant-table and evtchn interfaces. This also reads quite odd to me because "including" doesn't tell what's not enabled. So one could assume Xenstored is also enabled. In fact the wording for ``DOM0LESS_ENHANCED`` is what makes it a lot more confusing. So I would suggest the following wording: "Notify the OS it is running on top of Xen. All the default features but Xenstore will be available. Note that an OS *must* not rely on the availability of Xen features if this is not set. " The wording can be updated once we properly disable event channel/grant table when the flag is not set. * DOM0LESS_XENSTORE: Xenstore will be enabled for the VM. I would make clear this can't be used without the first one. * DOM0LESS_ENHANCED: Xen PV interfaces, including grant-table xenstore > * and evtchn, will be enabled for the VM. See above about "PV interfaces". So I would suggest to reword to: "Notify the OS it is running on top of Xen. All the default features (including Xenstore) will be available". */ #define DOM0LESS_ENHANCED_BASIC BIT(0, UL) #define DOM0LESS_XENSTORE BIT(1, UL) Based on the comment above, I would consider to define DOM0LESS_XENSTORE as bit 0 and 1 set. #define DOM0LESS_ENHANCED (DOM0LESS_ENHANCED_BASIC | DOM0LESS_XENSTORE) Cheers, -- Julien Grall
Re: [PATCH v3 7/7] xen/arm: introduce new xen,enhanced property value
Hi Julien, > On 2 Sep 2022, at 4:05 pm, Julien Grall wrote: > > Hi Bertrand, > > On 02/09/2022 15:51, Bertrand Marquis wrote: >>> On 1 Sep 2022, at 19:15, Julien Grall wrote: >>> AFAIU, it is not possible to have *_xenstore = true and *_enhanced = false. >>> I think it would be clearer if ``dom0less_enhanced`` is turned to an enum >>> with 3 values: >>> - None >>> - NOXENSTORE/BASIC >>> - FULLY_ENHANCED >>> >>> If we want to be future proof, I would use a field 'flags' where non-zero >>> means enhanced. Each bit would indicate which features of Xen is exposed. >> I think that could be a good solution if we do it this way: >> - define a dom0less feature field and have defines like the following: >> #define DOM0LESS_GNTTAB >> #define DOM0LESS_EVENTCHN >> #define DOM0LESS_XENSTORE > >> - define dom0less enhanced as the right combination: >> #define DOM0LESS_ENHANCED = (DOM0LESS_GNTTAB| DOM0LESS_EVENTCHN| >> DOM0LESS_XENSTORE) > > I would rather introduce DOM0LESS_ENHANCED_BASIC (or similar) instead of > defining a bit for gnttab and evtchn. This will avoid the question of why we > are introducing bits for both features but not the hypercall... > > As this is an internal interface, it would be easier to modify afterwards. How about this? /* * List of possible features for dom0less domUs * * DOM0LESS_ENHANCED_BASIC: Xen PV interfaces, including grant-table and * evtchn, will be enabled for the VM. * DOM0LESS_XENSTORE: Xenstore will be enabled for the VM. * DOM0LESS_ENHANCED: Xen PV interfaces, including grant-table xenstore * and evtchn, will be enabled for the VM. */ #define DOM0LESS_ENHANCED_BASIC BIT(0, UL) #define DOM0LESS_XENSTORE BIT(1, UL) #define DOM0LESS_ENHANCED (DOM0LESS_ENHANCED_BASIC | DOM0LESS_XENSTORE) Regards, Rahul
[ovmf test] 172935: regressions - FAIL
flight 172935 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/172935/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 172136 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172136 version targeted for testing: ovmf ec87181192f013f4f7ff916b2a39ff2c87b079f3 baseline version: ovmf 444260d45ec2a84e8f8c192b3539a3cd5591d009 Last test of basis 172136 2022-08-04 06:43:42 Z 29 days Failing since172151 2022-08-05 02:40:28 Z 28 days 225 attempts Testing same since 172926 2022-09-02 02:30:44 Z0 days3 attempts People who touched revisions under test: Abdul Lateef Attar Abner Chang Ard Biesheuvel Bob Feng Chasel Chiu Chen, Xiao X Czajkowski, Maciej Dimitrije Pavlov Dun Tan Edward Pickup Foster Nong Gregx Yeh Guo Dong Igor Kulchytskyy James Lu Jeff Brasen Jiaxin Wu Jose Marinho KasimX Liu Kavya Konstantin Aladyshev Kun Qin Liming Gao Liu, Zhiguang Maciej Czajkowski Michael D Kinney Michael Kubacki Pierre Gondois Ray Ni Rebecca Cran Rebecca Cran Sainadh Nagolu Sami Mujawar Shengfengx Xue Wu, Jiaxin Xiao X Chen Yuanhao Xie Zhiguang Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 1457 lines long.)
Re: [PATCH v3 6/7] xen/arm: introduce xen-evtchn dom0less property
Hi Julien, > On 1 Sep 2022, at 7:07 pm, Julien Grall wrote: > > Hi Rahul, > > On 01/09/2022 10:13, Rahul Singh wrote: >> Introduce a new sub-node under /chosen node to establish static event >> channel communication between domains on dom0less systems. >> An event channel will be created beforehand to allow the domains to >> send notifications to each other. >> Signed-off-by: Rahul Singh >> --- >> Changes in v3: >> - use device-tree used_by to find the domain id of the evtchn node. >> - add new static_evtchn_create variable in struct dt_device_node to >>hold the information if evtchn is already created. >> - fix minor comments >> Changes in v2: >> - no change >> --- >> docs/misc/arm/device-tree/booting.txt | 64 - >> xen/arch/arm/domain_build.c | 128 ++ >> xen/arch/arm/include/asm/setup.h | 1 + >> xen/arch/arm/setup.c | 2 + >> xen/include/xen/device_tree.h | 13 +++ >> 5 files changed, 207 insertions(+), 1 deletion(-) >> diff --git a/docs/misc/arm/device-tree/booting.txt >> b/docs/misc/arm/device-tree/booting.txt >> index 98253414b8..edef98e6d1 100644 >> --- a/docs/misc/arm/device-tree/booting.txt >> +++ b/docs/misc/arm/device-tree/booting.txt >> @@ -212,7 +212,7 @@ with the following properties: >> enable only selected interfaces. >>Under the "xen,domain" compatible node, one or more sub-nodes are present >> -for the DomU kernel and ramdisk. >> +for the DomU kernel, ramdisk and static event channel. >>The kernel sub-node has the following properties: >> @@ -254,11 +254,44 @@ The ramdisk sub-node has the following properties: >> property because it will be created by the UEFI stub on boot. >> This option is needed only when UEFI boot is used. >> +The static event channel sub-node has the following properties: >> + >> +- compatible >> + >> +"xen,evtchn" >> + >> +- xen,evtchn >> + >> +The property is tuples of two numbers >> +(local-evtchn link-to-foreign-evtchn) where: >> + >> +local-evtchn is an integer value that will be used to allocate local >> port >> +for a domain to send and receive event notifications to/from the remote >> +domain. Maximum supported value is 2^17 for FIFO ABI and 4096 for 2L >> ABI. >> +It is recommended to use low event channel IDs. >> + >> +link-to-foreign-evtchn is a single phandle to a remote evtchn to which >> +local-evtchn will be connected. >>Example >> === >>chosen { >> + >> +module@0 { >> +compatible = "multiboot,kernel", "multiboot,module"; >> +xen,uefi-binary = "..."; >> +bootargs = "..."; >> + >> +}; > > NIT: Describing this node in the example seems unnecessary. Ack. I will remove this. > >> + >> +/* one sub-node per local event channel */ >> +ec1: evtchn@1 { >> + compatible = "xen,evtchn-v1"; >> + /* local-evtchn link-to-foreign-evtchn */ >> + xen,evtchn = <0xa >; >> +}; >> + > > Here you provide an example for dom0. But the position where you describe the > binding suggests this binding only exists for domUs. > > Either we duplicate the binding or we re-order the documentation to have > common binding in a single place. My preference would be the latter. > Ack. >> domU1 { >> compatible = "xen,domain"; >> #address-cells = <0x2>; >> @@ -277,6 +310,23 @@ chosen { >> compatible = "multiboot,ramdisk", "multiboot,module"; >> reg = <0x0 0x4b00 0xff>; >> }; >> + >> +/* one sub-node per local event channel */ >> +ec2: evtchn@2 { >> +compatible = "xen,evtchn-v1"; >> +/* local-evtchn link-to-foreign-evtchn */ >> +xen,evtchn = <0xa >; >> +}; >> + >> +ec3: evtchn@3 { >> +compatible = "xen,evtchn-v1"; >> +xen,evtchn = <0xb >; >> +}; >> + >> +ec4: evtchn@4 { >> +compatible = "xen,evtchn-v1"; >> +xen,evtchn = <0xc >; >> +}; >> }; >>domU2 { >> @@ -296,6 +346,18 @@ chosen { >> compatible = "multiboot,ramdisk", "multiboot,module"; >> reg = <0x0 0x4d00 0xff>; >> }; >> + >> +/* one sub-node per local event channel */ >> +ec5: evtchn@5 { >> +compatible = "xen,evtchn-v1"; >> +/* local-evtchn link-to-foreign-evtchn */ >> +xen,evtchn = <0xb >; >> +}; >> + >> +ec6: evtchn@6 { >> +compatible = "xen,evtchn-v1"; >> +xen,evtchn = <0xd >; >> +}; >> }; >> }; >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 707e247f6a..4b24261825 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -33,6 +33,8 @@ >> #include >> #include >> +#define STATIC_EVTCHN_NODE_SIZE_CELLS 2 >> + >> static unsigned int __initdata
Re: [PATCH v3 7/7] xen/arm: introduce new xen,enhanced property value
Hi Bertrand, On 02/09/2022 15:51, Bertrand Marquis wrote: On 1 Sep 2022, at 19:15, Julien Grall wrote: AFAIU, it is not possible to have *_xenstore = true and *_enhanced = false. I think it would be clearer if ``dom0less_enhanced`` is turned to an enum with 3 values: - None - NOXENSTORE/BASIC - FULLY_ENHANCED If we want to be future proof, I would use a field 'flags' where non-zero means enhanced. Each bit would indicate which features of Xen is exposed. I think that could be a good solution if we do it this way: - define a dom0less feature field and have defines like the following: #define DOM0LESS_GNTTAB #define DOM0LESS_EVENTCHN #define DOM0LESS_XENSTORE > - define dom0less enhanced as the right combination: #define DOM0LESS_ENHANCED = (DOM0LESS_GNTTAB| DOM0LESS_EVENTCHN| DOM0LESS_XENSTORE) I would rather introduce DOM0LESS_ENHANCED_BASIC (or similar) instead of defining a bit for gnttab and evtchn. This will avoid the question of why we are introducing bits for both features but not the hypercall... As this is an internal interface, it would be easier to modify afterwards. Cheers, -- Julien Grall
Re: [PATCH v3 7/7] xen/arm: introduce new xen,enhanced property value
Hi Julien, > On 1 Sep 2022, at 7:15 pm, Julien Grall wrote: > > Hi Rahul, > > On 01/09/2022 10:13, Rahul Singh wrote: >> Introduce a new "xen,enhanced" dom0less property value "no-xenstore" to >> disable xenstore interface for dom0less guests. >> Signed-off-by: Rahul Singh >> --- >> Changes in v3: >> - new patch in this version >> --- >> docs/misc/arm/device-tree/booting.txt | 4 >> xen/arch/arm/domain_build.c | 10 +++--- >> xen/arch/arm/include/asm/kernel.h | 3 +++ >> 3 files changed, 14 insertions(+), 3 deletions(-) >> diff --git a/docs/misc/arm/device-tree/booting.txt >> b/docs/misc/arm/device-tree/booting.txt >> index edef98e6d1..87f57f8889 100644 >> --- a/docs/misc/arm/device-tree/booting.txt >> +++ b/docs/misc/arm/device-tree/booting.txt >> @@ -204,6 +204,10 @@ with the following properties: >> - "disabled" >> Xen PV interfaces are disabled. >> +- no-xenstore >> +Xen PV interfaces, including grant-table will be enabled for the VM but >> +xenstore will be disabled for the VM. > > NIT: I would drop one of the "for the VM" as it seems to be redundant. Ack. > >> + >> If the xen,enhanced property is present with no value, it defaults >> to "enabled". If the xen,enhanced property is not present, PV >> interfaces are disabled. >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 4b24261825..8dd9984225 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -3336,10 +3336,14 @@ static int __init construct_domU(struct domain *d, >> (rc == 0 && !strcmp(dom0less_enhanced, "enabled")) ) >> { >> if ( hardware_domain ) >> -kinfo.dom0less_enhanced = true; >> +kinfo.dom0less_xenstore = true; >> else >> -panic("Tried to use xen,enhanced without dom0\n"); >> +panic("Tried to use xen,enhanced without dom0 without >> no-xenstore\n"); > > This is a bit hard to parse. How about: > > "At the moment, Xenstore support requires dom0 to be present" Ack. > >> } >> +else if ( rc == 0 && !strcmp(dom0less_enhanced, "no-xenstore") ) >> +kinfo.dom0less_xenstore = false; >> + >> +kinfo.dom0less_enhanced = true; > > Wouldn't this now set dom0less_enhanced unconditionally? Yes , I will fix this in next version. Regards, Rahul
Re: [PATCH v3 7/7] xen/arm: introduce new xen,enhanced property value
Hi Julien, > On 1 Sep 2022, at 19:15, Julien Grall wrote: > > Hi Rahul, > > On 01/09/2022 10:13, Rahul Singh wrote: >> Introduce a new "xen,enhanced" dom0less property value "no-xenstore" to >> disable xenstore interface for dom0less guests. >> Signed-off-by: Rahul Singh >> --- >> Changes in v3: >> - new patch in this version >> --- >> docs/misc/arm/device-tree/booting.txt | 4 >> xen/arch/arm/domain_build.c | 10 +++--- >> xen/arch/arm/include/asm/kernel.h | 3 +++ >> 3 files changed, 14 insertions(+), 3 deletions(-) >> diff --git a/docs/misc/arm/device-tree/booting.txt >> b/docs/misc/arm/device-tree/booting.txt >> index edef98e6d1..87f57f8889 100644 >> --- a/docs/misc/arm/device-tree/booting.txt >> +++ b/docs/misc/arm/device-tree/booting.txt >> @@ -204,6 +204,10 @@ with the following properties: >> - "disabled" >> Xen PV interfaces are disabled. >> +- no-xenstore >> +Xen PV interfaces, including grant-table will be enabled for the VM but >> +xenstore will be disabled for the VM. > > NIT: I would drop one of the "for the VM" as it seems to be redundant. > >> + >> If the xen,enhanced property is present with no value, it defaults >> to "enabled". If the xen,enhanced property is not present, PV >> interfaces are disabled. >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 4b24261825..8dd9984225 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -3336,10 +3336,14 @@ static int __init construct_domU(struct domain *d, >> (rc == 0 && !strcmp(dom0less_enhanced, "enabled")) ) >> { >> if ( hardware_domain ) >> -kinfo.dom0less_enhanced = true; >> +kinfo.dom0less_xenstore = true; >> else >> -panic("Tried to use xen,enhanced without dom0\n"); >> +panic("Tried to use xen,enhanced without dom0 without >> no-xenstore\n"); > > This is a bit hard to parse. How about: > > "At the moment, Xenstore support requires dom0 to be present" > >> } >> +else if ( rc == 0 && !strcmp(dom0less_enhanced, "no-xenstore") ) >> +kinfo.dom0less_xenstore = false; >> + >> +kinfo.dom0less_enhanced = true; > > Wouldn't this now set dom0less_enhanced unconditionally? > >>if ( vcpu_create(d, 0) == NULL ) >> return -ENOMEM; >> @@ -3379,7 +3383,7 @@ static int __init construct_domU(struct domain *d, >> if ( rc < 0 ) >> return rc; >> -if ( kinfo.dom0less_enhanced ) >> +if ( kinfo.dom0less_xenstore ) >> { >> ASSERT(hardware_domain); >> rc = alloc_xenstore_evtchn(d); >> diff --git a/xen/arch/arm/include/asm/kernel.h >> b/xen/arch/arm/include/asm/kernel.h >> index c4dc039b54..3d7fa94910 100644 >> --- a/xen/arch/arm/include/asm/kernel.h >> +++ b/xen/arch/arm/include/asm/kernel.h >> @@ -39,6 +39,9 @@ struct kernel_info { >> /* Enable PV drivers */ >> bool dom0less_enhanced; >> +/* Enable Xenstore */ >> +bool dom0less_xenstore; >> + > > AFAIU, it is not possible to have *_xenstore = true and *_enhanced = false. I > think it would be clearer if ``dom0less_enhanced`` is turned to an enum with > 3 values: > - None > - NOXENSTORE/BASIC > - FULLY_ENHANCED > > If we want to be future proof, I would use a field 'flags' where non-zero > means enhanced. Each bit would indicate which features of Xen is exposed. I think that could be a good solution if we do it this way: - define a dom0less feature field and have defines like the following: #define DOM0LESS_GNTTAB #define DOM0LESS_EVENTCHN #define DOM0LESS_XENSTORE - define dom0less enhanced as the right combination: #define DOM0LESS_ENHANCED = (DOM0LESS_GNTTAB| DOM0LESS_EVENTCHN| DOM0LESS_XENSTORE) This way we have a proper feature bitset and ENHANCED is properly defined as a combination of the 3 features. What do you guys think ? Cheers Bertrand > > Cheers, > > -- > Julien Grall
Re: [PATCH v2 1/3] xen-blkback: Advertise feature-persistent as user requested
On 02/09/22 01:08PM, Juergen Gross wrote: > On 02.09.22 11:53, Pratyush Yadav wrote: > > On 31/08/22 04:58PM, SeongJae Park wrote: > > > The advertisement of the persistent grants feature (writing > > > 'feature-persistent' to xenbus) should mean not the decision for using > > > the feature but only the availability of the feature. However, commit > > > aac8a70db24b ("xen-blkback: add a parameter for disabling of persistent > > > grants") made a field of blkback, which was a place for saving only the > > > negotiation result, to be used for yet another purpose: caching of the > > > 'feature_persistent' parameter value. As a result, the advertisement, > > > which should follow only the parameter value, becomes inconsistent. > > > > > > This commit fixes the misuse of the semantic by making blkback saves the > > > parameter value in a separate place and advertises the support based on > > > only the saved value. > > > > > > Fixes: aac8a70db24b ("xen-blkback: add a parameter for disabling of > > > persistent grants") > > > Cc: # 5.10.x > > > Suggested-by: Juergen Gross > > > Signed-off-by: SeongJae Park > > > --- > > > drivers/block/xen-blkback/common.h | 3 +++ > > > drivers/block/xen-blkback/xenbus.c | 6 -- > > > 2 files changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/block/xen-blkback/common.h > > > b/drivers/block/xen-blkback/common.h > > > index bda5c815e441..a28473470e66 100644 > > > --- a/drivers/block/xen-blkback/common.h > > > +++ b/drivers/block/xen-blkback/common.h > > > @@ -226,6 +226,9 @@ struct xen_vbd { > > > sector_tsize; > > > unsigned intflush_support:1; > > > unsigned intdiscard_secure:1; > > > + /* Connect-time cached feature_persistent parameter value */ > > > + unsigned intfeature_gnt_persistent_parm:1; > > > > Continuing over from the previous version: > > > > > > If feature_gnt_persistent_parm is always going to be equal to > > > > feature_persistent, then why introduce it at all? Why not just use > > > > feature_persistent directly? This way you avoid adding an extra flag > > > > whose purpose is not immediately clear, and you also avoid all the > > > > mess with setting this flag at the right time. > > > > > > Mainly because the parameter should read twice (once for > > > advertisement, and once later just before the negotitation, for > > > checking if we advertised or not), and the user might change the > > > parameter value between the two reads. > > > > > > For the detailed available sequence of the race, you could refer to the > > > prior conversation[1]. > > > > > > [1] > > > https://lore.kernel.org/linux-block/20200922111259.GJ19254@Air-de-Roger/ > > > > Okay, I see. Thanks for the pointer. But still, I think it would be > > better to not maintain two copies of the value. How about doing: > > > > blkif->vbd.feature_gnt_persistent = > > xenbus_read_unsigned(dev->nodename, "feature-persistent", 0) && > > xenbus_read_unsigned(dev->otherend, "feature-persistent", 0); > > > > This makes it quite clear that we only enable persistent grants if > > _both_ ends support it. We can do the same for blkfront. > > I prefer it as is, as it will not rely on nobody having modified the > Xenstore node (which would in theory be possible). Okay. In that case, Reviewed-by: Pratyush Yadav -- Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
[PATCH v2 10/10] xen/arm: smmuv3: Avoid open coded arithmetic in memory allocation
From: Christophe JAILLET Backport Linux commit 98b64741d611. This is the clean backport without any changes kmalloc_array()/kcalloc() should be used to avoid potential overflow when a multiplication is needed to compute the size of the requested memory. So turn a devm_kzalloc()+explicit size computation into an equivalent devm_kcalloc(). Signed-off-by: Christophe JAILLET Acked-by: Robin Murphy Link: https://lore.kernel.org/r/3f7b9b202c6b6f5edc234ab7af5f208fbf8bc944.1644274051.git.christophe.jail...@wanadoo.fr Signed-off-by: Will Deacon Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 98b64741d611 Signed-off-by: Rahul Singh --- Changes in v2: - fix commit msg --- xen/drivers/passthrough/arm/smmu-v3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index 6fb74d864e..9e4815f455 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -1641,10 +1641,10 @@ static int arm_smmu_init_l1_strtab(struct arm_smmu_device *smmu) { unsigned int i; struct arm_smmu_strtab_cfg *cfg = >strtab_cfg; - size_t size = sizeof(*cfg->l1_desc) * cfg->num_l1_ents; void *strtab = smmu->strtab_cfg.strtab; - cfg->l1_desc = _xzalloc(size, sizeof(void *)); + cfg->l1_desc = _xzalloc_array(sizeof(*cfg->l1_desc), sizeof(void *), + cfg->num_l1_ents); if (!cfg->l1_desc) return -ENOMEM; -- 2.25.1
[PATCH v2 04/10] xen/arm: smmuv3: Move definitions to a header
From: Jean-Philippe Brucker Backport Linux commit e881e7839fba. This is the clean backport without any changes. Allow sharing structure definitions with the upcoming SVA support for Arm SMMUv3, by moving them to a separate header. We could surgically extract only what is needed but keeping all definitions in one place looks nicer. Signed-off-by: Jean-Philippe Brucker Reviewed-by: Eric Auger Reviewed-by: Jonathan Cameron Link: https://lore.kernel.org/r/20200918101852.582559-8-jean-phili...@linaro.org Signed-off-by: Will Deacon Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e881e7839fba Signed-off-by: Rahul Singh --- Changes in v2: - fix commit msg - also move struct definition to header file to sync with Linux patch --- xen/drivers/passthrough/arm/smmu-v3.c | 666 + xen/drivers/passthrough/arm/smmu-v3.h | 674 ++ 2 files changed, 675 insertions(+), 665 deletions(-) create mode 100644 xen/drivers/passthrough/arm/smmu-v3.h diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index cee13d1fc7..85ad066266 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -90,6 +90,7 @@ #include #include +#include "smmu-v3.h" #define ARM_SMMU_VTCR_SH_IS3 #define ARM_SMMU_VTCR_RGN_WBWA 1 @@ -102,21 +103,10 @@ #define ARM_SMMU_VTCR_PS_48_BIT0x5ULL #define ARM_SMMU_VTCR_PS_52_BIT0x6ULL -/* Linux compatibility functions. */ -typedef paddr_tdma_addr_t; -typedef paddr_tphys_addr_t; -typedef unsigned int gfp_t; - #define platform_devicedevice #define GFP_KERNEL 0 -/* Alias to Xen lock functions */ -#define mutex spinlock -#define mutex_init spin_lock_init -#define mutex_lock spin_lock -#define mutex_unlock spin_unlock - /* Device logger functions */ #define dev_name(dev) dt_node_full_name(dev->of_node) #define dev_dbg(dev, fmt, ...) \ @@ -157,12 +147,6 @@ typedef unsigned int gfp_t; #define readl_relaxed_poll_timeout(addr, val, cond, delay_us, timeout_us) \ readx_poll_timeout(readl_relaxed, addr, val, cond, delay_us, timeout_us) -#define FIELD_PREP(_mask, _val)\ - (((typeof(_mask))(_val) << (ffs64(_mask) - 1)) & (_mask)) - -#define FIELD_GET(_mask, _reg) \ - ((typeof(_mask))(((_reg) & (_mask)) >> (ffs64(_mask) - 1))) - /* * Helpers for DMA allocation. Just the function name is reused for * porting code, these allocation are not managed allocations @@ -195,27 +179,6 @@ static void *dmam_alloc_coherent(struct device *dev, size_t size, return vaddr; } - -/* Xen specific code. */ -struct iommu_domain { - /* Runtime SMMU configuration for this iommu_domain */ - atomic_tref; - /* -* Used to link iommu_domain contexts for a same domain. -* There is at least one per-SMMU to used by the domain. -*/ - struct list_headlist; -}; - -/* Describes information required for a Xen domain */ -struct arm_smmu_xen_domain { - spinlock_t lock; - - /* List of iommu domains associated to this domain */ - struct list_headcontexts; -}; - - /* Keep a list of devices associated with this driver */ static DEFINE_SPINLOCK(arm_smmu_devices_lock); static LIST_HEAD(arm_smmu_devices); @@ -259,635 +222,8 @@ static int platform_get_irq_byname_optional(struct device *dev, } /* Start of Linux SMMUv3 code */ -/* MMIO registers */ -#define ARM_SMMU_IDR0 0x0 -#define IDR0_ST_LVLGENMASK(28, 27) -#define IDR0_ST_LVL_2LVL 1 -#define IDR0_STALL_MODEL GENMASK(25, 24) -#define IDR0_STALL_MODEL_STALL 0 -#define IDR0_STALL_MODEL_FORCE 2 -#define IDR0_TTENDIAN GENMASK(22, 21) -#define IDR0_TTENDIAN_MIXED0 -#define IDR0_TTENDIAN_LE 2 -#define IDR0_TTENDIAN_BE 3 -#define IDR0_CD2L (1 << 19) -#define IDR0_VMID16(1 << 18) -#define IDR0_PRI (1 << 16) -#define IDR0_SEV (1 << 14) -#define IDR0_MSI (1 << 13) -#define IDR0_ASID16(1 << 12) -#define IDR0_ATS (1 << 10) -#define IDR0_HYP (1 << 9) -#define IDR0_COHACC(1 << 4) -#define IDR0_TTF GENMASK(3, 2) -#define IDR0_TTF_AARCH64 2 -#define IDR0_TTF_AARCH32_643 -#define IDR0_S1P (1 << 1) -#define IDR0_S2P (1 << 0) - -#define ARM_SMMU_IDR1 0x4 -#define IDR1_TABLES_PRESET (1 << 30) -#define IDR1_QUEUES_PRESET (1 << 29) -#define IDR1_REL
[PATCH v2 07/10] xen/arm: smmuv3: Change *array into *const array
From: Bixuan Cui Backport Linux commit d56d5162e317. This is the clean backport without any changes. Fix checkpatch warning in arm-smmu-v3.c: static const char * array should probably be static const char * const Signed-off-by: Bixuan Cui Signed-off-by: Will Deacon Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git d56d5162e317 Signed-off-by: Rahul Singh --- Changes in v2: - fix commit msg --- xen/drivers/passthrough/arm/smmu-v3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index 2b04f73b87..9bd0c211c0 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -467,7 +467,7 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent) static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu) { - static const char *cerror_str[] = { + static const char * const cerror_str[] = { [CMDQ_ERR_CERROR_NONE_IDX] = "No error", [CMDQ_ERR_CERROR_ILL_IDX] = "Illegal command", [CMDQ_ERR_CERROR_ABT_IDX] = "Abort on command fetch", -- 2.25.1
[PATCH v2 09/10] xen/arm: smmuv3: Fix fall-through warning for Clang
From: "Gustavo A. R. Silva" Backport Linux commit 5a1ab5c0299a. This is the clean backport without any changes. Fix the following fallthrough warning (arm64-randconfig with Clang): drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:382:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough] Reported-by: kernel test robot Link: https://lore.kernel.org/lkml/60edca25.k00ut905ifbjpyt5%25...@intel.com/ Signed-off-by: Gustavo A. R. Silva Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5a1ab5c0299a Signed-off-by: Rahul Singh --- Changes in v2: - fix commit msg. --- xen/drivers/passthrough/arm/smmu-v3.c | 1 + 1 file changed, 1 insertion(+) diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index f8f0eeb528..6fb74d864e 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -489,6 +489,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu) switch (idx) { case CMDQ_ERR_CERROR_ABT_IDX: dev_err(smmu->dev, "retrying command fetch\n"); + return; case CMDQ_ERR_CERROR_NONE_IDX: return; case CMDQ_ERR_CERROR_ATC_INV_IDX: -- 2.25.1
[PATCH v2 08/10] xen/arm: smmuv3: Remove unnecessary oom message
From: Zhen Lei Backport Linux commit affa909571b0. This is the clean backport without any changes. Fixes scripts/checkpatch.pl warning: WARNING: Possible unnecessary 'out of memory' message Remove it can help us save a bit of memory. Signed-off-by: Zhen Lei Link: https://lore.kernel.org/r/20210609125438.14369-1-thunder.leiz...@huawei.com Signed-off-by: Will Deacon Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git affa909571b0 Signed-off-by: Rahul Singh --- Changes in v2: - fix commit msg --- xen/drivers/passthrough/arm/smmu-v3.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index 9bd0c211c0..f8f0eeb528 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -1644,10 +1644,8 @@ static int arm_smmu_init_l1_strtab(struct arm_smmu_device *smmu) void *strtab = smmu->strtab_cfg.strtab; cfg->l1_desc = _xzalloc(size, sizeof(void *)); - if (!cfg->l1_desc) { - dev_err(smmu->dev, "failed to allocate l1 stream table desc\n"); + if (!cfg->l1_desc) return -ENOMEM; - } for (i = 0; i < cfg->num_l1_ents; ++i) { arm_smmu_write_strtab_l1_desc(strtab, >l1_desc[i]); @@ -2432,10 +2430,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev) struct arm_smmu_device *smmu; smmu = xzalloc(struct arm_smmu_device); - if (!smmu) { - dev_err(pdev, "failed to allocate arm_smmu_device\n"); + if (!smmu) return -ENOMEM; - } smmu->dev = pdev; if (pdev->of_node) { -- 2.25.1
[PATCH v2 06/10] xen/arm: smmuv3: Remove the unused fields for PREFETCH_CONFIG command
From: Zenghui Yu Backport Linux commit e0bb4b735404. This is the clean backport without any changes. Per SMMUv3 spec, there is no Size and Addr field in the PREFETCH_CONFIG command and they're not used by the driver. Remove them. We can add them back if we're going to use PREFETCH_ADDR in the future. Signed-off-by: Zenghui Yu Link: https://lore.kernel.org/r/20210407084448.1838-1-yuzeng...@huawei.com Signed-off-by: Will Deacon Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e0bb4b735404 Signed-off-by: Rahul Singh --- Changes in v2: - fix commit msg --- xen/drivers/passthrough/arm/smmu-v3.c | 2 -- xen/drivers/passthrough/arm/smmu-v3.h | 2 -- 2 files changed, 4 deletions(-) diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index f5485a8a1c..2b04f73b87 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -402,8 +402,6 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent) break; case CMDQ_OP_PREFETCH_CFG: cmd[0] |= FIELD_PREP(CMDQ_PREFETCH_0_SID, ent->prefetch.sid); - cmd[1] |= FIELD_PREP(CMDQ_PREFETCH_1_SIZE, ent->prefetch.size); - cmd[1] |= ent->prefetch.addr & CMDQ_PREFETCH_1_ADDR_MASK; break; case CMDQ_OP_CFGI_STE: cmd[0] |= FIELD_PREP(CMDQ_CFGI_0_SID, ent->cfgi.sid); diff --git a/xen/drivers/passthrough/arm/smmu-v3.h b/xen/drivers/passthrough/arm/smmu-v3.h index 0742bc393f..b381ad3738 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.h +++ b/xen/drivers/passthrough/arm/smmu-v3.h @@ -456,8 +456,6 @@ struct arm_smmu_cmdq_ent { #define CMDQ_OP_PREFETCH_CFG0x1 struct { u32 sid; - u8 size; - u64 addr; } prefetch; #define CMDQ_OP_CFGI_STE0x3 -- 2.25.1
[PATCH v2 05/10] xen/arm: smmuv3: Remove the page 1 fixup
From: Robin Murphy Backport Linux commit 86d2d9214880. This is the clean backport without any changes. Since we now keep track of page 1 via a separate pointer that already encapsulates aliasing to page 0 as necessary, we can remove the clunky fixup routine and simply use the relevant bases directly. The current architecture spec (IHI0070D.a) defines SMMU_{EVENTQ,PRIQ}_{PROD,CONS} as offsets relative to page 1, so the cleanup represents a little bit of convergence as well as just lines of code saved. Signed-off-by: Robin Murphy Signed-off-by: Will Deacon Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 86d2d9214880 Signed-off-by: Rahul Singh --- Changes in v2: - fix commit msg. --- xen/drivers/passthrough/arm/smmu-v3.c | 42 ++- xen/drivers/passthrough/arm/smmu-v3.h | 8 ++--- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index 85ad066266..f5485a8a1c 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -235,15 +235,6 @@ static struct arm_smmu_option_prop arm_smmu_options[] = { { 0, NULL}, }; -static inline void __iomem *arm_smmu_page1_fixup(unsigned long offset, -struct arm_smmu_device *smmu) -{ - if (offset > SZ_64K) - return smmu->page1 + offset - SZ_64K; - - return smmu->base + offset; -} - static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom) { return container_of(dom, struct arm_smmu_domain, domain); @@ -1578,6 +1569,7 @@ static int arm_smmu_dt_xlate(struct device *dev, /* Probing and initialisation functions */ static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu, struct arm_smmu_queue *q, + void __iomem *page, unsigned long prod_off, unsigned long cons_off, size_t dwords, const char *name) @@ -1606,8 +1598,8 @@ static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu, 1 << q->llq.max_n_shift, name); } - q->prod_reg = arm_smmu_page1_fixup(prod_off, smmu); - q->cons_reg = arm_smmu_page1_fixup(cons_off, smmu); + q->prod_reg = page + prod_off; + q->cons_reg = page + prod_off; q->ent_dwords = dwords; q->q_base = Q_BASE_RWA; @@ -1624,16 +1616,16 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu) /* cmdq */ spin_lock_init(>cmdq.lock); - ret = arm_smmu_init_one_queue(smmu, >cmdq.q, ARM_SMMU_CMDQ_PROD, - ARM_SMMU_CMDQ_CONS, CMDQ_ENT_DWORDS, - "cmdq"); + ret = arm_smmu_init_one_queue(smmu, >cmdq.q, smmu->base, + ARM_SMMU_CMDQ_PROD, ARM_SMMU_CMDQ_CONS, + CMDQ_ENT_DWORDS, "cmdq"); if (ret) return ret; /* evtq */ - ret = arm_smmu_init_one_queue(smmu, >evtq.q, ARM_SMMU_EVTQ_PROD, - ARM_SMMU_EVTQ_CONS, EVTQ_ENT_DWORDS, - "evtq"); + ret = arm_smmu_init_one_queue(smmu, >evtq.q, smmu->page1, + ARM_SMMU_EVTQ_PROD, ARM_SMMU_EVTQ_CONS, + EVTQ_ENT_DWORDS, "evtq"); if (ret) return ret; @@ -1641,9 +1633,9 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu) if (!(smmu->features & ARM_SMMU_FEAT_PRI)) return 0; - return arm_smmu_init_one_queue(smmu, >priq.q, ARM_SMMU_PRIQ_PROD, - ARM_SMMU_PRIQ_CONS, PRIQ_ENT_DWORDS, - "priq"); + return arm_smmu_init_one_queue(smmu, >priq.q, smmu->page1, + ARM_SMMU_PRIQ_PROD, ARM_SMMU_PRIQ_CONS, + PRIQ_ENT_DWORDS, "priq"); } static int arm_smmu_init_l1_strtab(struct arm_smmu_device *smmu) @@ -2087,10 +2079,8 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu) /* Event queue */ writeq_relaxed(smmu->evtq.q.q_base, smmu->base + ARM_SMMU_EVTQ_BASE); - writel_relaxed(smmu->evtq.q.llq.prod, - arm_smmu_page1_fixup(ARM_SMMU_EVTQ_PROD, smmu)); - writel_relaxed(smmu->evtq.q.llq.cons, - arm_smmu_page1_fixup(ARM_SMMU_EVTQ_CONS, smmu)); + writel_relaxed(smmu->evtq.q.llq.prod, smmu->page1 + ARM_SMMU_EVTQ_PROD); + writel_relaxed(smmu->evtq.q.llq.cons, smmu->page1 + ARM_SMMU_EVTQ_CONS); enables |= CR0_EVTQEN; ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0, @@
[PATCH v2 03/10] xen/arm: smmuv3: Ensure queue is read after updating prod pointer
From: Zhou Wang Backport Linux commit a76a3f2c. This is the clean backport without any changes. Reading the 'prod' MMIO register in order to determine whether or not there is valid data beyond 'cons' for a given queue does not provide sufficient dependency ordering, as the resulting access is address dependent only on 'cons' and can therefore be speculated ahead of time, potentially allowing stale data to be read by the CPU. Use readl() instead of readl_relaxed() when updating the shadow copy of the 'prod' pointer, so that all speculated memory reads from the corresponding queue can occur only from valid slots. Signed-off-by: Zhou Wang Link: https://lore.kernel.org/r/1601281922-117296-1-git-send-email-wangzh...@hisilicon.com [will: Use readl() instead of explicit barrier. Update 'cons' side to match.] Signed-off-by: Will Deacon Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a76a3f2c Signed-off-by: Rahul Singh --- Changes in v2: - fix commit msg - add _iomb changes also from the origin patch --- xen/arch/arm/include/asm/system.h | 1 + xen/drivers/passthrough/arm/smmu-v3.c | 11 +-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/include/asm/system.h b/xen/arch/arm/include/asm/system.h index 65d5c8e423..fe27cf8c5e 100644 --- a/xen/arch/arm/include/asm/system.h +++ b/xen/arch/arm/include/asm/system.h @@ -29,6 +29,7 @@ #endif #define smp_wmb() dmb(ishst) +#define __iomb()dmb(osh) #define smp_mb__before_atomic()smp_mb() #define smp_mb__after_atomic() smp_mb() diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index 64d39bb4d3..cee13d1fc7 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -951,7 +951,7 @@ static void queue_sync_cons_out(struct arm_smmu_queue *q) * Ensure that all CPU accesses (reads and writes) to the queue * are complete before we update the cons pointer. */ - mb(); + __iomb(); writel_relaxed(q->llq.cons, q->cons_reg); } @@ -963,8 +963,15 @@ static void queue_inc_cons(struct arm_smmu_ll_queue *q) static int queue_sync_prod_in(struct arm_smmu_queue *q) { + u32 prod; int ret = 0; - u32 prod = readl_relaxed(q->prod_reg); + + /* +* We can't use the _relaxed() variant here, as we must prevent +* speculative reads of the queue before we have determined that +* prod has indeed moved. +*/ + prod = readl(q->prod_reg); if (Q_OVF(prod) != Q_OVF(q->llq.prod)) ret = -EOVERFLOW; -- 2.25.1
[PATCH v2 02/10] xen/arm: smmuv3: Fix endianness annotations
From: Jean-Philippe Brucker Backport Linux commit 376cdf66f624. This is the clean backport without any changes. When building with C=1, sparse reports some issues regarding endianness annotations: arm-smmu-v3.c:221:26: warning: cast to restricted __le64 arm-smmu-v3.c:221:24: warning: incorrect type in assignment (different base types) arm-smmu-v3.c:221:24:expected restricted __le64 [usertype] arm-smmu-v3.c:221:24:got unsigned long long [usertype] arm-smmu-v3.c:229:20: warning: incorrect type in argument 1 (different base types) arm-smmu-v3.c:229:20:expected restricted __le64 [usertype] *[assigned] dst arm-smmu-v3.c:229:20:got unsigned long long [usertype] *ent arm-smmu-v3.c:229:25: warning: incorrect type in argument 2 (different base types) arm-smmu-v3.c:229:25:expected unsigned long long [usertype] *[assigned] src arm-smmu-v3.c:229:25:got restricted __le64 [usertype] * arm-smmu-v3.c:396:20: warning: incorrect type in argument 1 (different base types) arm-smmu-v3.c:396:20:expected restricted __le64 [usertype] *[assigned] dst arm-smmu-v3.c:396:20:got unsigned long long * arm-smmu-v3.c:396:25: warning: incorrect type in argument 2 (different base types) arm-smmu-v3.c:396:25:expected unsigned long long [usertype] *[assigned] src arm-smmu-v3.c:396:25:got restricted __le64 [usertype] * arm-smmu-v3.c:1349:32: warning: invalid assignment: |= arm-smmu-v3.c:1349:32:left side has type restricted __le64 arm-smmu-v3.c:1349:32:right side has type unsigned long arm-smmu-v3.c:1396:53: warning: incorrect type in argument 3 (different base types) arm-smmu-v3.c:1396:53:expected restricted __le64 [usertype] *dst arm-smmu-v3.c:1396:53:got unsigned long long [usertype] *strtab arm-smmu-v3.c:1424:39: warning: incorrect type in argument 1 (different base types) arm-smmu-v3.c:1424:39:expected unsigned long long [usertype] *[assigned] strtab arm-smmu-v3.c:1424:39:got restricted __le64 [usertype] *l2ptr While harmless, they are incorrect and could hide actual errors during development. Fix them. Signed-off-by: Jean-Philippe Brucker Reviewed-by: Robin Murphy Link: https://lore.kernel.org/r/20200918141856.629722-1-jean-phili...@linaro.org Signed-off-by: Will Deacon Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 376cdf66f624 Signed-off-by: Rahul Singh --- Changes in v2: - fix commit msg --- xen/drivers/passthrough/arm/smmu-v3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index 340609264d..64d39bb4d3 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -1037,7 +1037,7 @@ static int queue_insert_raw(struct arm_smmu_queue *q, u64 *ent) return 0; } -static void queue_read(__le64 *dst, u64 *src, size_t n_dwords) +static void queue_read(u64 *dst, __le64 *src, size_t n_dwords) { int i; @@ -1436,7 +1436,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid, arm_smmu_cmdq_issue_cmd(smmu, _cmd); } -static void arm_smmu_init_bypass_stes(u64 *strtab, unsigned int nent) +static void arm_smmu_init_bypass_stes(__le64 *strtab, unsigned int nent) { unsigned int i; -- 2.25.1
[PATCH v2 01/10] xen/arm: smmuv3: Fix l1 stream table size in the error message
From: Zenghui Yu Backport Linux commit dc898eb84b25. This is the clean backport without any changes. The actual size of level-1 stream table is l1size. This looks like an oversight on commit d2e88e7c081ef ("iommu/arm-smmu: Fix LOG2SIZE setting for 2-level stream tables") which forgot to update the @size in error message as well. As memory allocation failure is already bad enough, nothing worse would happen. But let's be careful. Signed-off-by: Zenghui Yu Link: https://lore.kernel.org/r/20200826141758.341-1-yuzeng...@huawei.com Signed-off-by: Will Deacon Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dc898eb84b25 Signed-off-by: Rahul Singh --- Changes in v2: - fix commit msg --- xen/drivers/passthrough/arm/smmu-v3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index f2562acc38..340609264d 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -2348,7 +2348,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu) if (!strtab) { dev_err(smmu->dev, "failed to allocate l1 stream table (%u bytes)\n", - size); + l1size); return -ENOMEM; } cfg->strtab = strtab; -- 2.25.1
[PATCH v2 00/10] xen/arm: smmuv3: Merge Linux fixes to Xen
This patch series merge the applicable Linux fixes to Xen. Bixuan Cui (1): xen/arm: smmuv3: Change *array into *const array Christophe JAILLET (1): xen/arm: smmuv3: Avoid open coded arithmetic in memory allocation Gustavo A. R. Silva (1): xen/arm: smmuv3: Fix fall-through warning for Clang Jean-Philippe Brucker (2): xen/arm: smmuv3: Fix endianness annotations xen/arm: smmuv3: Move definitions to a header Robin Murphy (1): xen/arm: smmuv3: Remove the page 1 fixup Zenghui Yu (2): xen/arm: smmuv3: Fix l1 stream table size in the error message xen/arm: smmuv3: Remove the unused fields for PREFETCH_CONFIG command Zhen Lei (1): xen/arm: smmuv3: Remove unnecessary oom message Zhou Wang (1): xen/arm: smmuv3: Ensure queue is read after updating prod pointer xen/arch/arm/include/asm/system.h | 1 + xen/drivers/passthrough/arm/smmu-v3.c | 742 ++ xen/drivers/passthrough/arm/smmu-v3.h | 672 +++ 3 files changed, 708 insertions(+), 707 deletions(-) create mode 100644 xen/drivers/passthrough/arm/smmu-v3.h -- 2.25.1
[PATCH v6 06/10] drivers/char: mark DMA buffers as reserved for the XHCI
The important part is to include those buffers in IOMMU page table relevant for the USB controller. Otherwise, DbC will stop working as soon as IOMMU is enabled, regardless of to which domain device assigned (be it xen or dom0). If the device is passed through to dom0 or other domain (see later patches), that domain will effectively have access to those buffers too. It does give such domain yet another way to DoS the system (as is the case when having PCI device assigned already), but also possibly steal the console ring content. Thus, such domain should be a trusted one. In any case, prevent anything else being placed on those pages by adding artificial padding. Signed-off-by: Marek Marczykowski-Górecki Acked-by: Jan Beulich --- Changes in v5: - add missing alignment Changes in v3: - adjust for xhci-dbc rename - do not raise MAX_USER_RMRR_PAGES - adjust alignment of DMA buffers --- xen/drivers/char/xhci-dbc.c | 43 +- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c index 8da76282259a..fc9745f7c2ac 100644 --- a/xen/drivers/char/xhci-dbc.c +++ b/xen/drivers/char/xhci-dbc.c @@ -20,6 +20,7 @@ */ #include +#include #include #include #include @@ -1051,13 +1052,21 @@ static struct uart_driver dbc_uart_driver = { }; /* Those are accessed via DMA. */ -static struct xhci_trb evt_trb[DBC_TRB_RING_CAP]; -static struct xhci_trb out_trb[DBC_TRB_RING_CAP]; -static struct xhci_trb in_trb[DBC_TRB_RING_CAP]; -static struct xhci_erst_segment erst __aligned(16); -static struct xhci_dbc_ctx ctx __aligned(16); -static uint8_t out_wrk_buf[DBC_WORK_RING_CAP]; -static struct xhci_string_descriptor str_buf[DBC_STRINGS_COUNT]; +struct dbc_dma_bufs { +struct xhci_trb evt_trb[DBC_TRB_RING_CAP]; +struct xhci_trb out_trb[DBC_TRB_RING_CAP]; +struct xhci_trb in_trb[DBC_TRB_RING_CAP]; +uint8_t out_wrk_buf[DBC_WORK_RING_CAP]; +struct xhci_erst_segment erst __aligned(16); +struct xhci_dbc_ctx ctx __aligned(16); +struct xhci_string_descriptor str_buf[DBC_STRINGS_COUNT]; +/* + * Don't place anything else on this page - it will be + * DMA-reachable by the USB controller. + */ +}; +static struct dbc_dma_bufs __section(".bss.page_aligned") __aligned(PAGE_SIZE) +dbc_dma_bufs; static int __init xhci_parse_dbgp(const char *opt_dbgp) { @@ -1103,16 +1112,22 @@ void __init xhci_dbc_uart_init(void) if ( !dbc->enable ) return; -dbc->dbc_ctx = -dbc->dbc_erst = -dbc->dbc_ering.trb = evt_trb; -dbc->dbc_oring.trb = out_trb; -dbc->dbc_iring.trb = in_trb; -dbc->dbc_owork.buf = out_wrk_buf; -dbc->dbc_str = str_buf; +dbc->dbc_ctx = _dma_bufs.ctx; +dbc->dbc_erst = _dma_bufs.erst; +dbc->dbc_ering.trb = dbc_dma_bufs.evt_trb; +dbc->dbc_oring.trb = dbc_dma_bufs.out_trb; +dbc->dbc_iring.trb = dbc_dma_bufs.in_trb; +dbc->dbc_owork.buf = dbc_dma_bufs.out_wrk_buf; +dbc->dbc_str = dbc_dma_bufs.str_buf; if ( dbc_open(dbc) ) +{ +iommu_add_extra_reserved_device_memory( +PFN_DOWN(virt_to_maddr(_dma_bufs)), +PFN_UP(sizeof(dbc_dma_bufs)), +uart->dbc.sbdf); serial_register_uart(SERHND_XHCI, _uart_driver, _uart); +} } #ifdef DBC_DEBUG -- git-series 0.9.1
[PATCH v6 10/10] drivers/char: use smp barriers in xhci driver
All (interesting) data is in plain WB cached memory, and the few BAR register that are configured have a UC mapping, which orders properly WRT other writes on x86. Suggested-by: Andrew Cooper Signed-off-by: Marek Marczykowski-Górecki --- New in v6 --- xen/drivers/char/xhci-dbc.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c index 829f1d1d910f..03df4d82a623 100644 --- a/xen/drivers/char/xhci-dbc.c +++ b/xen/drivers/char/xhci-dbc.c @@ -666,7 +666,7 @@ static void dbc_pop_events(struct dbc *dbc) BUILD_BUG_ON((1 << XHCI_TRB_SHIFT) != sizeof(struct xhci_trb)); -rmb(); +smp_rmb(); while ( xhci_trb_cyc(event) == er->cyc ) { @@ -710,7 +710,7 @@ static void dbc_pop_events(struct dbc *dbc) } erdp = er->dma + (er->deq << XHCI_TRB_SHIFT); -wmb(); +smp_wmb(); writeq(erdp, >erdp); } @@ -847,9 +847,9 @@ static void dbc_enable_dbc(struct dbc *dbc) { struct dbc_reg *reg = dbc->dbc_reg; -wmb(); +smp_wmb(); writel(readl(>ctrl) | (1U << DBC_CTRL_DCE), >ctrl); -wmb(); +smp_wmb(); while ( (readl(>ctrl) & (1U << DBC_CTRL_DCE)) == 0 ) cpu_relax(); @@ -858,9 +858,9 @@ static void dbc_enable_dbc(struct dbc *dbc) if ( !dbc->open ) dbc_reset_debug_port(dbc); -wmb(); +smp_wmb(); writel(readl(>portsc) | (1U << DBC_PSC_PED), >portsc); -wmb(); +smp_wmb(); while ( (readl(>ctrl) & (1U << DBC_CTRL_DCR)) == 0 ) cpu_relax(); @@ -871,7 +871,7 @@ static void dbc_disable_dbc(struct dbc *dbc) struct dbc_reg *reg = dbc->dbc_reg; writel(readl(>portsc) & ~(1U << DBC_PSC_PED), >portsc); -wmb(); +smp_wmb(); writel(readl(>ctrl) & ~(1U << DBC_CTRL_DCE), >ctrl); while ( readl(>ctrl) & (1U << DBC_CTRL_DCE) ) @@ -1032,7 +1032,7 @@ static bool dbc_ensure_running(struct dbc *dbc) { writel(ctrl | (1U << DBC_CTRL_DRC), >ctrl); writel(readl(>portsc) | (1U << DBC_PSC_PED), >portsc); -wmb(); +smp_wmb(); dbc_ring_doorbell(dbc, dbc->dbc_iring.db); dbc_ring_doorbell(dbc, dbc->dbc_oring.db); } @@ -1074,7 +1074,7 @@ static void dbc_flush(struct dbc *dbc, struct xhci_trb_ring *trb, } } -wmb(); +smp_wmb(); dbc_ring_doorbell(dbc, trb->db); } @@ -1101,7 +1101,7 @@ static void dbc_enqueue_in(struct dbc *dbc, struct xhci_trb_ring *trb, dbc_push_trb(dbc, trb, wrk->dma + wrk->enq, dbc_work_ring_space_to_end(wrk)); -wmb(); +smp_wmb(); writel(db, >db); } -- git-series 0.9.1
[PATCH v6 09/10] drivers/char: fix handling cable re-plug in XHCI console driver
When cable is unplugged, dbc_ensure_running() correctly detects this situation (DBC_CTRL_DCR flag is clear), and prevent sending data immediately to the device. It gets only queued in work ring buffers. When cable is plugged in again, subsequent dbc_flush() will send the buffered data. But there is a corner case, where no subsequent data was buffered in the work buffer, but a TRB was still pending. Ring the doorbell to let the controller re-send them. For console output it is rare corner case (TRB is pending for a very short time), but for console input it is very normal case (there is always one pending TRB for input). Extract doorbell ringing into separate function to avoid duplication. Signed-off-by: Marek Marczykowski-Górecki --- Changes in v6: - keep barriers consistent --- xen/drivers/char/xhci-dbc.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c index 9f7e1dd60a78..829f1d1d910f 100644 --- a/xen/drivers/char/xhci-dbc.c +++ b/xen/drivers/char/xhci-dbc.c @@ -554,6 +554,15 @@ static unsigned int dbc_work_ring_space_to_end(const struct dbc_work_ring *ring) return ring->deq - ring->enq; } +static void dbc_ring_doorbell(struct dbc *dbc, int doorbell) +{ +uint32_t __iomem *db_reg = >dbc_reg->db; +uint32_t db = (readl(db_reg) & ~DBC_DOORBELL_TARGET_MASK) | + (doorbell << DBC_DOORBELL_TARGET_SHIFT); + +writel(db, db_reg); +} + static void dbc_push_trb(struct dbc *dbc, struct xhci_trb_ring *ring, uint64_t dma, uint64_t len) { @@ -1024,6 +1033,8 @@ static bool dbc_ensure_running(struct dbc *dbc) writel(ctrl | (1U << DBC_CTRL_DRC), >ctrl); writel(readl(>portsc) | (1U << DBC_PSC_PED), >portsc); wmb(); +dbc_ring_doorbell(dbc, dbc->dbc_iring.db); +dbc_ring_doorbell(dbc, dbc->dbc_oring.db); } return true; @@ -1041,10 +1052,6 @@ static bool dbc_ensure_running(struct dbc *dbc) static void dbc_flush(struct dbc *dbc, struct xhci_trb_ring *trb, struct dbc_work_ring *wrk) { -struct dbc_reg *reg = dbc->dbc_reg; -uint32_t db = (readl(>db) & ~DBC_DOORBELL_TARGET_MASK) | - (trb->db << DBC_DOORBELL_TARGET_SHIFT); - if ( xhci_trb_ring_full(trb) ) return; @@ -1068,7 +1075,7 @@ static void dbc_flush(struct dbc *dbc, struct xhci_trb_ring *trb, } wmb(); -writel(db, >db); +dbc_ring_doorbell(dbc, trb->db); } /** -- git-series 0.9.1
[PATCH v6 08/10] drivers/char: allow driving the rest of XHCI by a domain while Xen uses DbC
That's possible, because the capability was designed specifically to allow separate driver handle it, in parallel to unmodified xhci driver (separate set of registers, pretending the port is "disconnected" for the main xhci driver etc). It works with Linux dom0, although requires an awful hack - re-enabling bus mastering behind dom0's backs. Linux driver does similar thing - see drivers/usb/early/xhci-dbc.c:xdbc_handle_events(). When controller sharing is enabled in kconfig (option marked as experimental), dom0 is allowed to use the controller even if Xen uses it for debug console. Additionally, option `dbgp=xhci,share=` is available to either prevent even dom0 from using it (`no` value), or allow any domain using it (`any` value). In any case, to avoid Linux messing with the DbC, mark this MMIO area as read-only. This might cause issues for Linux's driver (if it tries to write something on the same page - like anoter xcap), but makes Xen's use safe. In practice, as of Linux 5.18, it seems to work without issues. Signed-off-by: Marek Marczykowski-Górecki Reviewed-by: Jan Beulich --- Changes in v5: - drop CONFIG_XHCI_SHARE - make XHCI_SHARE_HWDOM = 0 - use parse_boolean - add comment about mmio_ro_ranges - fix doc Changes in v4: - minor fix for cmdline parsing - make sharing opt-in build time, with option marked as EXPERIMENTAL - change cmdline syntax to share=|hwdom - make share=hwdom default (if enabled build-time) Changes in v3: - adjust for xhci-dbc rename - adjust for dbc_ensure_running() split - wrap long lines - add runtime option for sharing USB controller --- docs/misc/xen-command-line.pandoc | 14 ++- xen/drivers/char/xhci-dbc.c | 129 +-- 2 files changed, 134 insertions(+), 9 deletions(-) diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index fb4d80c590f3..1c755563c40d 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -726,7 +726,7 @@ Available alternatives, with their meaning, are: ### dbgp > `= ehci[ | @pci:. ]` -> `= xhci[ | @pci:. ]` +> `= xhci[ | @pci:. ][,share=|hwdom]` Specify the USB controller to use, either by instance number (when going over the PCI busses sequentially) or by PCI device (must be on segment 0). @@ -734,6 +734,18 @@ over the PCI busses sequentially) or by PCI device (must be on segment 0). Use `ehci` for EHCI debug port, use `xhci` for XHCI debug capability. XHCI driver will wait indefinitely for the debug host to connect - make sure the cable is connected. +The `share` option for xhci controls who else can use the controller: +* `no`: use the controller exclusively for console, even hardware domain + (dom0) cannot use it +* `hwdom`: hardware domain may use the controller too, ports not used for debug + console will be available for normal devices; this is the default +* `yes`: the controller can be assigned to any domain; it is not safe to assign + the controller to untrusted domain + +Choosing `share=hwdom` (the default) or `share=yes` allows a domain to reset the +controller, which may cause small portion of the console output to be lost. + +The `share=yes` configuration is not security supported. ### debug_stack_lines > `= ` diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c index 557c5fc785ce..9f7e1dd60a78 100644 --- a/xen/drivers/char/xhci-dbc.c +++ b/xen/drivers/char/xhci-dbc.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -232,6 +233,12 @@ struct dbc_work_ring { uint64_t dma; }; +enum xhci_share { +XHCI_SHARE_HWDOM = 0, +XHCI_SHARE_NONE, +XHCI_SHARE_ANY +}; + struct dbc { struct dbc_reg __iomem *dbc_reg; struct xhci_dbc_ctx *dbc_ctx; @@ -250,6 +257,7 @@ struct dbc { bool enable; /* whether dbgp=xhci was set at all */ bool open; +enum xhci_share share; unsigned int xhc_num; /* look for n-th xhc */ }; @@ -952,13 +960,56 @@ static bool __init dbc_open(struct dbc *dbc) } /* - * Ensure DbC is still running, handle events, and possibly re-enable if cable - * was re-plugged. Returns true if DbC is operational. + * Ensure DbC is still running, handle events, and possibly + * re-enable/re-configure if cable was re-plugged or controller was reset. + * Returns true if DbC is operational. */ static bool dbc_ensure_running(struct dbc *dbc) { struct dbc_reg *reg = dbc->dbc_reg; uint32_t ctrl; +uint16_t cmd; + +if ( dbc->share != XHCI_SHARE_NONE ) +{ +/* + * Re-enable memory decoding and later bus mastering, if dom0 (or + * other) disabled it in the meantime. + */ +cmd = pci_conf_read16(dbc->sbdf, PCI_COMMAND); +if ( !(cmd & PCI_COMMAND_MEMORY) ) +{ +cmd |= PCI_COMMAND_MEMORY; +pci_conf_write16(dbc->sbdf, PCI_COMMAND, cmd); +} + +/* + * FIXME: Make Linux coordinate XHCI reset, so the
[PATCH v6 07/10] drivers/char: add RX support to the XHCI driver
Add another work ring buffer for received data, and point IN TRB at it. Ensure there is always at least one pending IN TRB, so the controller has a way to send incoming data to the driver. Note that both "success" and "short packet" completion codes are okay - in fact it will be "short packet" most of the time, as the TRB length is about maximum size, not required size. Signed-off-by: Marek Marczykowski-Górecki Acked-by: Jan Beulich --- Changes in v4: - adjust return types - add some const New patch in v3 --- docs/misc/xen-command-line.pandoc | 6 +- xen/drivers/char/xhci-dbc.c | 129 +++- 2 files changed, 132 insertions(+), 3 deletions(-) diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index f6bdae9ca5f4..fb4d80c590f3 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -731,9 +731,9 @@ Available alternatives, with their meaning, are: Specify the USB controller to use, either by instance number (when going over the PCI busses sequentially) or by PCI device (must be on segment 0). -Use `ehci` for EHCI debug port, use `xhci` for XHCI debug capability (output -only). XHCI driver will wait indefinitely for the debug host to connect - make -sure the cable is connected. +Use `ehci` for EHCI debug port, use `xhci` for XHCI debug capability. +XHCI driver will wait indefinitely for the debug host to connect - make sure +the cable is connected. ### debug_stack_lines > `= ` diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c index fc9745f7c2ac..557c5fc785ce 100644 --- a/xen/drivers/char/xhci-dbc.c +++ b/xen/drivers/char/xhci-dbc.c @@ -111,6 +111,7 @@ enum { enum { XHCI_TRB_CC_SUCCESS = 1, XHCI_TRB_CC_TRB_ERR = 5, +XHCI_TRB_CC_SHORT_PACKET = 13, }; /* DbC endpoint types */ @@ -239,6 +240,7 @@ struct dbc { struct xhci_trb_ring dbc_oring; struct xhci_trb_ring dbc_iring; struct dbc_work_ring dbc_owork; +struct dbc_work_ring dbc_iwork; struct xhci_string_descriptor *dbc_str; pci_sbdf_t sbdf; @@ -444,6 +446,16 @@ static void xhci_trb_norm_set_ioc(struct xhci_trb *trb) trb->ctrl |= 0x20; } +static uint64_t xhci_trb_norm_buf(const struct xhci_trb *trb) +{ +return trb->params; +} + +static uint32_t xhci_trb_norm_len(const struct xhci_trb *trb) +{ +return trb->status & 0x1; +} + /** * Fields for Transfer Event TRBs (see section 6.4.2.1). Note that event * TRBs are read-only from software @@ -453,6 +465,17 @@ static uint64_t xhci_trb_tfre_ptr(const struct xhci_trb *trb) return trb->params; } +static uint32_t xhci_trb_tfre_cc(const struct xhci_trb *trb) +{ +return trb->status >> 24; +} + +/* Amount of data _not_ transferred */ +static uint32_t xhci_trb_tfre_len(const struct xhci_trb *trb) +{ +return trb->status & 0x1; +} + /* Fields for link TRBs (section 6.4.4.1) */ static void xhci_trb_link_set_rsp(struct xhci_trb *trb, uint64_t rsp) { @@ -494,6 +517,14 @@ static bool xhci_trb_ring_full(const struct xhci_trb_ring *ring) return ((ring->enq + 1) & (DBC_TRB_RING_CAP - 1)) == ring->deq; } +static unsigned int xhci_trb_ring_size(const struct xhci_trb_ring *ring) +{ +if ( ring->enq >= ring->deq ) +return ring->enq - ring->deq; + +return DBC_TRB_RING_CAP - ring->deq + ring->enq; +} + static bool dbc_work_ring_full(const struct dbc_work_ring *ring) { return ((ring->enq + 1) & (DBC_WORK_RING_CAP - 1)) == ring->deq; @@ -507,6 +538,14 @@ static unsigned int dbc_work_ring_size(const struct dbc_work_ring *ring) return DBC_WORK_RING_CAP - ring->deq + ring->enq; } +static unsigned int dbc_work_ring_space_to_end(const struct dbc_work_ring *ring) +{ +if ( ring->enq >= ring->deq ) +return DBC_WORK_RING_CAP - ring->enq; + +return ring->deq - ring->enq; +} + static void dbc_push_trb(struct dbc *dbc, struct xhci_trb_ring *ring, uint64_t dma, uint64_t len) { @@ -567,6 +606,31 @@ static unsigned int dbc_push_work(struct dbc *dbc, struct dbc_work_ring *ring, return i; } +static void dbc_rx_trb(struct dbc *dbc, struct xhci_trb *trb, + uint64_t not_transferred) +{ +struct dbc_work_ring *ring = >dbc_iwork; +unsigned int rx_len; +unsigned int end, start = ring->enq; + +if ( xhci_trb_type(trb) != XHCI_TRB_NORM ) +/* Can be Link TRB for example. */ +return; + +ASSERT(xhci_trb_norm_buf(trb) == ring->dma + ring->enq); +ASSERT(xhci_trb_norm_len(trb) >= not_transferred); +rx_len = xhci_trb_norm_len(trb) - not_transferred; + +/* It can hit the ring end, but should not wrap around. */ +ASSERT(ring->enq + rx_len <= DBC_WORK_RING_CAP); +ring->enq = (ring->enq + rx_len) & (DBC_WORK_RING_CAP - 1); + +end = ring->enq; + +if ( end > start ) +cache_flush(>buf[start], end - start); +} + /* * Note that if IN transfer support is added, then this
[PATCH v6 04/10] IOMMU/VT-d: wire common device reserved memory API
Re-use rmrr= parameter handling code to handle common device reserved memory. Signed-off-by: Marek Marczykowski-Górecki --- Changes in v3: - make MAX_USER_RMRR_PAGES applicable only to user-configured RMRR --- xen/drivers/passthrough/vtd/dmar.c | 201 +- 1 file changed, 119 insertions(+), 82 deletions(-) diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c index 367304c8739c..3df5f6b69719 100644 --- a/xen/drivers/passthrough/vtd/dmar.c +++ b/xen/drivers/passthrough/vtd/dmar.c @@ -861,111 +861,139 @@ static struct user_rmrr __initdata user_rmrrs[MAX_USER_RMRR]; /* Macro for RMRR inclusive range formatting. */ #define ERMRRU_FMT "[%lx-%lx]" -#define ERMRRU_ARG(eru) eru.base_pfn, eru.end_pfn +#define ERMRRU_ARG base_pfn, end_pfn + +static int __init add_one_user_rmrr(unsigned long base_pfn, +unsigned long end_pfn, +unsigned int dev_count, +uint32_t *sbdf); static int __init add_user_rmrr(void) { +unsigned int i; +int ret; + +for ( i = 0; i < nr_rmrr; i++ ) +{ +ret = add_one_user_rmrr(user_rmrrs[i].base_pfn, +user_rmrrs[i].end_pfn, +user_rmrrs[i].dev_count, +user_rmrrs[i].sbdf); +if ( ret < 0 ) +return ret; +} +return 0; +} + +/* Returns 1 on success, 0 when ignoring and < 0 on error. */ +static int __init add_one_user_rmrr(unsigned long base_pfn, +unsigned long end_pfn, +unsigned int dev_count, +uint32_t *sbdf) +{ struct acpi_rmrr_unit *rmrr, *rmrru; -unsigned int idx, seg, i; -unsigned long base, end; +unsigned int idx, seg; +unsigned long base_iter; bool overlap; -for ( i = 0; i < nr_rmrr; i++ ) +if ( iommu_verbose ) +printk(XENLOG_DEBUG VTDPREFIX + "Adding RMRR for %d device ([0]: %#x) range "ERMRRU_FMT"\n", + dev_count, sbdf[0], ERMRRU_ARG); + +if ( base_pfn > end_pfn ) { -base = user_rmrrs[i].base_pfn; -end = user_rmrrs[i].end_pfn; +printk(XENLOG_ERR VTDPREFIX + "Invalid RMRR Range "ERMRRU_FMT"\n", + ERMRRU_ARG); +return 0; +} -if ( base > end ) +overlap = false; +list_for_each_entry(rmrru, _rmrr_units, list) +{ +if ( pfn_to_paddr(base_pfn) <= rmrru->end_address && + rmrru->base_address <= pfn_to_paddr(end_pfn) ) { printk(XENLOG_ERR VTDPREFIX - "Invalid RMRR Range "ERMRRU_FMT"\n", - ERMRRU_ARG(user_rmrrs[i])); -continue; + "Overlapping RMRRs: "ERMRRU_FMT" and [%lx-%lx]\n", + ERMRRU_ARG, + paddr_to_pfn(rmrru->base_address), + paddr_to_pfn(rmrru->end_address)); +overlap = true; +break; } +} +/* Don't add overlapping RMRR. */ +if ( overlap ) +return 0; -if ( (end - base) >= MAX_USER_RMRR_PAGES ) +base_iter = base_pfn; +do +{ +if ( !mfn_valid(_mfn(base_iter)) ) { printk(XENLOG_ERR VTDPREFIX - "RMRR range "ERMRRU_FMT" exceeds "\ - __stringify(MAX_USER_RMRR_PAGES)" pages\n", - ERMRRU_ARG(user_rmrrs[i])); -continue; + "Invalid pfn in RMRR range "ERMRRU_FMT"\n", + ERMRRU_ARG); +break; } +} while ( base_iter++ < end_pfn ); -overlap = false; -list_for_each_entry(rmrru, _rmrr_units, list) -{ -if ( pfn_to_paddr(base) <= rmrru->end_address && - rmrru->base_address <= pfn_to_paddr(end) ) -{ -printk(XENLOG_ERR VTDPREFIX - "Overlapping RMRRs: "ERMRRU_FMT" and [%lx-%lx]\n", - ERMRRU_ARG(user_rmrrs[i]), - paddr_to_pfn(rmrru->base_address), - paddr_to_pfn(rmrru->end_address)); -overlap = true; -break; -} -} -/* Don't add overlapping RMRR. */ -if ( overlap ) -continue; +/* Invalid pfn in range as the loop ended before end_pfn was reached. */ +if ( base_iter <= end_pfn ) +return 0; -do -{ -if ( !mfn_valid(_mfn(base)) ) -{ -printk(XENLOG_ERR VTDPREFIX - "Invalid pfn in RMRR range "ERMRRU_FMT"\n", - ERMRRU_ARG(user_rmrrs[i])); -break; -} -} while ( base++ < end ); +rmrr = xzalloc(struct acpi_rmrr_unit); +if ( !rmrr ) +return -ENOMEM;
[PATCH v6 03/10] IOMMU: add common API for device reserved memory
Add API similar to rmrr= and ivmd= arguments, but in a common code. This will allow drivers to register reserved memory regardless of the IOMMU vendor. The direct reason for this API is xhci-dbc console driver (aka xue), that needs to use DMA. But future change may unify command line arguments for user-supplied reserved memory, and it may be useful for other drivers in the future too. This commit just introduces an API, subsequent patches will plug it in appropriate places. The reserved memory ranges needs to be saved locally, because at the point when they are collected, Xen doesn't know yet which IOMMU driver will be used. Signed-off-by: Marek Marczykowski-Górecki Acked-by: Jan Beulich --- Changes in v5: - fix indentation, keep full "reserved_device_memory" for consistency with iommu_get_reserved_device_memory Changes in v4: - mark functions as __init - use pci_sbdf_t type Changes in v3: - adjust code style --- xen/drivers/passthrough/iommu.c | 46 ++- xen/include/xen/iommu.h | 14 ++- 2 files changed, 60 insertions(+) diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index 134cdb47e0dc..5e2a720d29b9 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -669,6 +669,52 @@ bool_t iommu_has_feature(struct domain *d, enum iommu_feature feature) return is_iommu_enabled(d) && test_bit(feature, dom_iommu(d)->features); } +#define MAX_EXTRA_RESERVED_RANGES 20 +struct extra_reserved_range { +unsigned long start; +unsigned long nr; +pci_sbdf_t sbdf; +}; +static unsigned int __initdata nr_extra_reserved_ranges; +static struct extra_reserved_range __initdata +extra_reserved_ranges[MAX_EXTRA_RESERVED_RANGES]; + +int __init iommu_add_extra_reserved_device_memory(unsigned long start, + unsigned long nr, + pci_sbdf_t sbdf) +{ +unsigned int idx; + +if ( nr_extra_reserved_ranges >= MAX_EXTRA_RESERVED_RANGES ) +return -ENOMEM; + +idx = nr_extra_reserved_ranges++; +extra_reserved_ranges[idx].start = start; +extra_reserved_ranges[idx].nr = nr; +extra_reserved_ranges[idx].sbdf = sbdf; + +return 0; +} + +int __init iommu_get_extra_reserved_device_memory(iommu_grdm_t *func, + void *ctxt) +{ +unsigned int idx; +int ret; + +for ( idx = 0; idx < nr_extra_reserved_ranges; idx++ ) +{ +ret = func(extra_reserved_ranges[idx].start, + extra_reserved_ranges[idx].nr, + extra_reserved_ranges[idx].sbdf.sbdf, + ctxt); +if ( ret < 0 ) +return ret; +} + +return 0; +} + /* * Local variables: * mode: C diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index 1240d7762d99..4f22fc1bed55 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -304,6 +304,20 @@ struct iommu_ops { #endif }; +/* + * To be called by Xen internally, to register extra RMRR/IVMD ranges. + * Needs to be called before IOMMU initialization. + */ +extern int iommu_add_extra_reserved_device_memory(unsigned long start, + unsigned long nr, + pci_sbdf_t sbdf); +/* + * To be called by specific IOMMU driver during initialization, + * to fetch ranges registered with iommu_add_extra_reserved_device_memory(). + */ +extern int iommu_get_extra_reserved_device_memory(iommu_grdm_t *func, + void *ctxt); + #include #ifndef iommu_call -- git-series 0.9.1
[PATCH v6 05/10] IOMMU/AMD: wire common device reserved memory API
Register common device reserved memory similar to how ivmd= parameter is handled. Signed-off-by: Marek Marczykowski-Górecki Acked-by: Jan Beulich --- Changes in v3: - use variable initializer - use pfn_to_paddr() --- xen/drivers/passthrough/amd/iommu_acpi.c | 21 + 1 file changed, 21 insertions(+) diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c index ac6835225bae..3b577c9b390c 100644 --- a/xen/drivers/passthrough/amd/iommu_acpi.c +++ b/xen/drivers/passthrough/amd/iommu_acpi.c @@ -1078,6 +1078,25 @@ static inline bool_t is_ivmd_block(u8 type) type == ACPI_IVRS_TYPE_MEMORY_IOMMU); } +static int __init cf_check add_one_extra_ivmd(unsigned long start, + unsigned long nr, + uint32_t id, void *ctxt) +{ +struct acpi_ivrs_memory ivmd = { +.header = { +.length = sizeof(ivmd), +.flags = ACPI_IVMD_UNITY | ACPI_IVMD_READ | ACPI_IVMD_WRITE, +.device_id = id, +.type = ACPI_IVRS_TYPE_MEMORY_ONE, +}, +}; + +ivmd.start_address = pfn_to_paddr(start); +ivmd.memory_length = pfn_to_paddr(nr); + +return parse_ivmd_block(); +} + static int __init cf_check parse_ivrs_table(struct acpi_table_header *table) { const struct acpi_ivrs_header *ivrs_block; @@ -1121,6 +1140,8 @@ static int __init cf_check parse_ivrs_table(struct acpi_table_header *table) AMD_IOMMU_DEBUG("IVMD: %u command line provided entries\n", nr_ivmd); for ( i = 0; !error && i < nr_ivmd; ++i ) error = parse_ivmd_block(user_ivmds + i); +if ( !error ) +error = iommu_get_extra_reserved_device_memory(add_one_extra_ivmd, NULL); /* Each IO-APIC must have been mentioned in the table. */ for ( apic = 0; !error && iommu_intremap && apic < nr_ioapics; ++apic ) -- git-series 0.9.1
[PATCH v6 00/10] Add Xue - console over USB 3 Debug Capability
This is integration of https://github.com/connojd/xue into mainline Xen. This patch series includes several patches that I made in the process, some are very loosely related. The driver developed by Connor supports console via USB3 debug capability. The capability is designed to operate mostly independently of normal XHCI driver, so this patch series allows dom0 to drive standard USB3 controller part, while Xen uses DbC for console output. Changes since RFC: - move the driver to xue.c, remove non-Xen parts, remove now unneeded abstraction - adjust for Xen code style - build for x86 only - drop patch hidding the device from dom0 Changes since v1: - drop ehci patch - already applied - adjust for review comments from Jan (see changelogs in individual patches) Changes since v2: - add runtime option to share (or not) the controller with dom0 or other domains - add RX support - several smaller changes according to review comments Changes since v3: - put controller sharing behind experimental kconfig option - several other changes according to review comments Changes since v4: - drop first 4 patches - already applied to staging - split dbgp=xhci into dbc=xhci Changes since v5: - roll dbc=xhci back into dbgp=xhci, but make it work together with dbgp=ehci Cc: Andrew Cooper Cc: George Dunlap Cc: Jan Beulich Cc: Julien Grall Cc: Stefano Stabellini Cc: Wei Liu Cc: "Roger Pau Monné" Cc: Paul Durrant Cc: Kevin Tian Cc: Connor Davis Marek Marczykowski-Górecki (10): drivers/char: allow using both dbgp=xhci and dbgp=ehci console: support multiple serial console simultaneously IOMMU: add common API for device reserved memory IOMMU/VT-d: wire common device reserved memory API IOMMU/AMD: wire common device reserved memory API drivers/char: mark DMA buffers as reserved for the XHCI drivers/char: add RX support to the XHCI driver drivers/char: allow driving the rest of XHCI by a domain while Xen uses DbC drivers/char: fix handling cable re-plug in XHCI console driver drivers/char: use smp barriers in xhci driver docs/misc/xen-command-line.pandoc| 30 +- xen/drivers/char/Kconfig | 11 +- xen/drivers/char/console.c | 98 -- xen/drivers/char/ehci-dbgp.c | 15 +- xen/drivers/char/serial.c| 6 +- xen/drivers/char/xhci-dbc.c | 364 +--- xen/drivers/passthrough/amd/iommu_acpi.c | 21 +- xen/drivers/passthrough/iommu.c | 46 +++- xen/drivers/passthrough/vtd/dmar.c | 201 +++-- xen/include/xen/iommu.h | 14 +- xen/include/xen/serial.h | 4 +- 11 files changed, 652 insertions(+), 158 deletions(-) base-commit: e997d055929665b12246e89eb092dc79c65de9a4 -- git-series 0.9.1
[PATCH v6 02/10] console: support multiple serial console simultaneously
Previously only one serial console was supported at the same time. Using console=com1,dbgp,vga silently ignored all but last serial console (in this case: only dbgp and vga were active). Fix this by storing not a single sercon_handle, but an array of them, up to MAX_SERCONS entries. The value of MAX_SERCONS can be chosen in kconfig, the default (4) is arbitrary, inspired by the number of SERHND_IDX values. Make console_steal() aware of multiple consoles too. It can now either steal output from specific console (for gdbstub), or from all of them at once (for console suspend). Signed-off-by: Marek Marczykowski-Górecki --- Changes in v4: - use unsigned int for loop counters - other minor changes Changes in v3: - adjust console_steal() for multiple consoles too - add MAX_SERCONS to kconfig - add warning about sync_console impact - add warning if too many consoles are configured - log issue with PCI spec parsing --- docs/misc/xen-command-line.pandoc | 4 +- xen/drivers/char/Kconfig | 11 - xen/drivers/char/console.c| 98 xen/include/xen/serial.h | 1 +- 4 files changed, 92 insertions(+), 22 deletions(-) diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index c8b07042f58e..f6bdae9ca5f4 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -435,6 +435,9 @@ only available when used together with `pv-in-pvh`. `none` indicates that Xen should not use a console. This option only makes sense on its own. +Specifying more than one serial console will increase console latency, +especially when `sync_console` option is used. + ### console_timestamps > `= none | date | datems | boot | raw` @@ -2405,6 +2408,7 @@ vulnerabilities. Flag to force synchronous console output. Useful for debugging, but not suitable for production environments due to incurred overhead. +If multiple consoles are configured, the incurred overhead is even bigger. ### tboot (x86) > `= 0x` diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig index 06350c387371..7b5ff0c414ec 100644 --- a/xen/drivers/char/Kconfig +++ b/xen/drivers/char/Kconfig @@ -85,6 +85,17 @@ config SERIAL_TX_BUFSIZE Default value is 16384 (16kiB). +config MAX_SERCONS + int "Maximum number of serial consoles active at once" + default 4 + help + Controls how many serial consoles can be active at once. Configuring more + using `console=` parameter will be ignored. + When multiple consoles are configured, overhead of `sync_console` option + is even bigger. + + Default value is 4. + config XHCI bool "XHCI DbC UART driver" depends on X86 diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c index e8468c121ad0..60d42284f606 100644 --- a/xen/drivers/char/console.c +++ b/xen/drivers/char/console.c @@ -113,7 +113,9 @@ static char *__read_mostly conring = _conring; static uint32_t __read_mostly conring_size = _CONRING_SIZE; static uint32_t conringc, conringp; -static int __read_mostly sercon_handle = -1; +#define MAX_SERCONS CONFIG_MAX_SERCONS +static int __read_mostly sercon_handle[MAX_SERCONS]; +static unsigned int __read_mostly nr_sercon_handle = 0; #ifdef CONFIG_X86 /* Tristate: 0 disabled, 1 user enabled, -1 default enabled */ @@ -393,32 +395,61 @@ long read_console_ring(struct xen_sysctl_readconsole *op) static char serial_rx_ring[SERIAL_RX_SIZE]; static unsigned int serial_rx_cons, serial_rx_prod; -static void (*serial_steal_fn)(const char *, size_t nr) = early_puts; +/* The last entry means "steal from all consoles" */ +static void (*serial_steal_fn[])(const char *, size_t nr) = { +[MAX_SERCONS] = early_puts, +}; +/* + * Redirect console *handle* output to *fn*. Use SERHND_STEAL_ALL as *handle* to + * redirect all the consoles. + */ int console_steal(int handle, void (*fn)(const char *, size_t nr)) { -if ( (handle == -1) || (handle != sercon_handle) ) -return 0; +unsigned int i; + +if ( handle == -1 ) +return -ENOENT; +if ( serial_steal_fn[MAX_SERCONS] != NULL ) +return -EBUSY; +if ( handle == SERHND_STEAL_ALL ) +{ +serial_steal_fn[MAX_SERCONS] = fn; +return MAX_SERCONS; +} +for ( i = 0; i < nr_sercon_handle; i++ ) +if ( handle == sercon_handle[i] ) +break; +if ( i == nr_sercon_handle ) +return -ENOENT; -if ( serial_steal_fn != NULL ) +if ( serial_steal_fn[i] != NULL ) return -EBUSY; -serial_steal_fn = fn; -return 1; +serial_steal_fn[i] = fn; +return i; } void console_giveback(int id) { -if ( id == 1 ) -serial_steal_fn = NULL; +if ( id >= 0 && id <= MAX_SERCONS ) +serial_steal_fn[id] = NULL; } void console_serial_puts(const char *s, size_t nr) { -if ( serial_steal_fn != NULL ) -serial_steal_fn(s, nr); +
[PATCH v6 01/10] drivers/char: allow using both dbgp=xhci and dbgp=ehci
This allows configuring EHCI and XHCI consoles separately, simultaneously. This changes string_param() to custom_param() in both ehci and xhci drivers. Both drivers parse only values applicable to them. While at it, drop unnecessary memset() of a static variable. Suggested-by: Jan Beulich Signed-off-by: Marek Marczykowski-Górecki --- Changes in v6: - keep dbgp=xhci, but use custom_param() to parse multiple values separately - modify ehci-dbgp to use custom_param() too - change console=dbc to console=xhci, as 'dbc' doesn't appear in any other option anymore - update comment in serial.h new in v5 --- docs/misc/xen-command-line.pandoc | 6 -- xen/drivers/char/ehci-dbgp.c | 15 +-- xen/drivers/char/serial.c | 6 ++ xen/drivers/char/xhci-dbc.c | 30 -- xen/include/xen/serial.h | 3 ++- 5 files changed, 45 insertions(+), 15 deletions(-) diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index 9a79385a3712..c8b07042f58e 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -409,7 +409,7 @@ The following are examples of correct specifications: Specify the size of the console ring buffer. ### console -> `= List of [ vga | com1[H,L] | com2[H,L] | pv | dbgp | none ]` +> `= List of [ vga | com1[H,L] | com2[H,L] | pv | dbgp | xhci | none ]` > Default: `console=com1,vga` @@ -428,7 +428,9 @@ cleared. This allows a single port to be shared by two subsystems `pv` indicates that Xen should use Xen's PV console. This option is only available when used together with `pv-in-pvh`. -`dbgp` indicates that Xen should use a USB debug port. +`dbgp` indicates that Xen should use a USB2 debug port. + +`xhci` indicates that Xen should use a USB3 debug port. `none` indicates that Xen should not use a console. This option only makes sense on its own. diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c index 92c588ec0aa3..b1794ed64c7b 100644 --- a/xen/drivers/char/ehci-dbgp.c +++ b/xen/drivers/char/ehci-dbgp.c @@ -1464,7 +1464,18 @@ static struct uart_driver __read_mostly ehci_dbgp_driver = { static struct ehci_dbgp ehci_dbgp = { .state = dbgp_unsafe, .phys_port = 1 }; static char __initdata opt_dbgp[30]; -string_param("dbgp", opt_dbgp); + +static int __init parse_ehci_dbgp(const char *opt) +{ +if ( strncmp(opt, "ehci", 4) ) +return 0; + +strlcpy(opt_dbgp, opt, sizeof(opt_dbgp)); + +return 0; +} + +custom_param("dbgp", parse_ehci_dbgp); void __init ehci_dbgp_init(void) { @@ -1472,7 +1483,7 @@ void __init ehci_dbgp_init(void) u32 debug_port, offset, bar_val; const char *e; -if ( strncmp(opt_dbgp, "ehci", 4) ) +if ( !opt_dbgp[0] ) return; if ( isdigit(opt_dbgp[4]) || !opt_dbgp[4] ) diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c index 47899222cef8..9d9445039232 100644 --- a/xen/drivers/char/serial.c +++ b/xen/drivers/char/serial.c @@ -311,6 +311,12 @@ int __init serial_parse_handle(const char *conf) goto common; } +if ( !strncmp(conf, "xhci", 4) && (!conf[4] || conf[4] == ',') ) +{ +handle = SERHND_XHCI; +goto common; +} + if ( !strncmp(conf, "dtuart", 6) ) { handle = SERHND_DTUART; diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c index ca7d4a62139e..8da76282259a 100644 --- a/xen/drivers/char/xhci-dbc.c +++ b/xen/drivers/char/xhci-dbc.c @@ -245,6 +245,7 @@ struct dbc { uint64_t xhc_dbc_offset; void __iomem *xhc_mmio; +bool enable; /* whether dbgp=xhci was set at all */ bool open; unsigned int xhc_num; /* look for n-th xhc */ }; @@ -1058,20 +1059,14 @@ static struct xhci_dbc_ctx ctx __aligned(16); static uint8_t out_wrk_buf[DBC_WORK_RING_CAP]; static struct xhci_string_descriptor str_buf[DBC_STRINGS_COUNT]; -static char __initdata opt_dbgp[30]; - -string_param("dbgp", opt_dbgp); - -void __init xhci_dbc_uart_init(void) +static int __init xhci_parse_dbgp(const char *opt_dbgp) { struct dbc_uart *uart = _uart; struct dbc *dbc = >dbc; const char *e; if ( strncmp(opt_dbgp, "xhci", 4) ) -return; - -memset(dbc, 0, sizeof(*dbc)); +return 0; if ( isdigit(opt_dbgp[4]) ) { @@ -1087,12 +1082,27 @@ void __init xhci_dbc_uart_init(void) printk(XENLOG_ERR "Invalid dbgp= PCI device spec: '%s'\n", opt_dbgp + 8); -return; +return -EINVAL; } dbc->sbdf = PCI_SBDF(0, bus, slot, func); } +dbc->enable = true; + +return 0; +} + +custom_param("dbgp", xhci_parse_dbgp); + +void __init xhci_dbc_uart_init(void) +{ +struct dbc_uart *uart = _uart; +struct dbc *dbc = >dbc; + +if ( !dbc->enable ) +return; + dbc->dbc_ctx = dbc->dbc_erst = dbc->dbc_ering.trb = evt_trb; @@
[qemu-mainline test] 172928: regressions - FAIL
flight 172928 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/172928/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172123 build-i386-libvirt6 libvirt-buildfail REGR. vs. 172123 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 172123 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 172123 test-amd64-amd64-pygrub 21 guest-start/debian.repeat fail REGR. vs. 172123 test-amd64-i386-xl-vhd 21 guest-start/debian.repeat fail REGR. vs. 172123 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 172123 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 172123 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 172123 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 172123 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 172123 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: qemuu7dd9d7e0bd29abf590d1ac235c0a00606ef81153 baseline version: qemuu2480f3bbd03814b0651a1f74959f5c6631ee5819 Last test of basis 172123 2022-08-03 18:10:07 Z 29 days Failing
Re: [RFC PATCH 00/30] Code tagging framework and applications
On 9/1/22 7:04 PM, Roman Gushchin wrote: > On Thu, Sep 01, 2022 at 08:17:47PM -0400, Kent Overstreet wrote: >> On Thu, Sep 01, 2022 at 03:53:57PM -0700, Roman Gushchin wrote: >>> I'd suggest to run something like iperf on a fast hardware. And maybe some >>> io_uring stuff too. These are two places which were historically most >>> sensitive >>> to the (kernel) memory accounting speed. >> >> I'm getting wildly inconsistent results with iperf. >> >> io_uring-echo-server and rust_echo_bench gets me: >> Benchmarking: 127.0.0.1:12345 >> 50 clients, running 512 bytes, 60 sec. >> >> Without alloc tagging: 120547 request/sec >> With:116748 request/sec >> >> https://github.com/frevib/io_uring-echo-server >> https://github.com/haraldh/rust_echo_bench >> >> How's that look to you? Close enough? :) > > Yes, this looks good (a bit too good). > > I'm not that familiar with io_uring, Jens and Pavel should have a better idea > what and how to run (I know they've workarounded the kernel memory accounting > because of the performance in the past, this is why I suspect it might be an > issue here as well). io_uring isn't alloc+free intensive on a per request basis anymore, it would not be a good benchmark if the goal is to check for regressions in that area. -- Jens Axboe
Re: [for-4.17 3/3] automation: Add a new job for testing boot time cpupools on arm64
> On 2 Sep 2022, at 08:09, Michal Orzel wrote: > > Add a new test job qemu-smoke-arm64-gcc-boot-cpupools that will execute > script qemu-smoke-arm64.sh to test boot time cpupools feature. > Enable CONFIG_BOOT_TIME_CPUPOOLS for the arm64 build and add a new test > case in qemu-smoke-arm64.sh that if selected will: > - create a device tree cpupool node with cpu@1 > - assign created cpupool to domU0 > - add a check in dom0 xen.start to see if domU is assigned a Pool-1 > > Take the opportunity to refactor the qemu-smoke-arm64.sh script as > follows: > - use domU_check to store the test's commands to be run from domU > - use dom0_check to store the test's commands to be run from dom0 > - use fdtput instead of sed to perform dtb modifications > - use more meaningful messages for "passed" variable. This way we can > grep for messages reported either by domU or dom0 and get rid of > assumption that tests can only be run from domU > > Signed-off-by: Michal Orzel Reviewed-by: Luca Fancellu
Re: [for-4.17 2/3] automation: qemu-smoke-arm64: Silence ifconfig error messages
> On 2 Sep 2022, at 08:09, Michal Orzel wrote: > > During the ping test, dom1 tries to assign an ip to eth0 in a loop. > Before setting up the network interface by dom0, this results in > printing the following error message several times: > (XEN) DOM1: ifconfig: SIOCSIFADDR: No such device > > Silence this by redirecting stderr/stdout to /dev/null as we do not > care about the output and we should not pollute the log file. > > Signed-off-by: Michal Orzel Reviewed-by: Luca Fancellu
Re: [for-4.17 1/3] automation: qemu-alpine-arm64-gcc: Use kernel 5.19
> On 2 Sep 2022, at 08:09, Michal Orzel wrote: > > After qemu-smoke-arm64 was changed to use kernel 5.19 we end up having > two kernel configurations. This is something not needed and maintaining > a single kernel version is always easier. Modify qemu-alpine-arm64-gcc > to use kernel 5.19 and remove kernel 5.9 from tests-artifacts. > > Signed-off-by: Michal Orzel Hi Michal, I’m not an expert of gitlab-ci, but the changes looks ok to me: Reviewed-by: Luca Fancellu
[ovmf test] 172933: regressions - FAIL
flight 172933 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/172933/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 172136 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172136 version targeted for testing: ovmf ec87181192f013f4f7ff916b2a39ff2c87b079f3 baseline version: ovmf 444260d45ec2a84e8f8c192b3539a3cd5591d009 Last test of basis 172136 2022-08-04 06:43:42 Z 29 days Failing since172151 2022-08-05 02:40:28 Z 28 days 224 attempts Testing same since 172926 2022-09-02 02:30:44 Z0 days2 attempts People who touched revisions under test: Abdul Lateef Attar Abner Chang Ard Biesheuvel Bob Feng Chasel Chiu Chen, Xiao X Czajkowski, Maciej Dimitrije Pavlov Dun Tan Edward Pickup Foster Nong Gregx Yeh Guo Dong Igor Kulchytskyy James Lu Jeff Brasen Jiaxin Wu Jose Marinho KasimX Liu Kavya Konstantin Aladyshev Kun Qin Liming Gao Liu, Zhiguang Maciej Czajkowski Michael D Kinney Michael Kubacki Pierre Gondois Ray Ni Rebecca Cran Rebecca Cran Sainadh Nagolu Sami Mujawar Shengfengx Xue Wu, Jiaxin Xiao X Chen Yuanhao Xie Zhiguang Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 1457 lines long.)
Re: [PATCH v2 1/3] xen-blkback: Advertise feature-persistent as user requested
On Fri, Sep 02, 2022 at 09:53:22AM +, Pratyush Yadav wrote: > On 31/08/22 04:58PM, SeongJae Park wrote: > > The advertisement of the persistent grants feature (writing > > 'feature-persistent' to xenbus) should mean not the decision for using > > the feature but only the availability of the feature. However, commit > > aac8a70db24b ("xen-blkback: add a parameter for disabling of persistent > > grants") made a field of blkback, which was a place for saving only the > > negotiation result, to be used for yet another purpose: caching of the > > 'feature_persistent' parameter value. As a result, the advertisement, > > which should follow only the parameter value, becomes inconsistent. > > > > This commit fixes the misuse of the semantic by making blkback saves the > > parameter value in a separate place and advertises the support based on > > only the saved value. > > > > Fixes: aac8a70db24b ("xen-blkback: add a parameter for disabling of > > persistent grants") > > Cc: # 5.10.x > > Suggested-by: Juergen Gross > > Signed-off-by: SeongJae Park > > --- > > drivers/block/xen-blkback/common.h | 3 +++ > > drivers/block/xen-blkback/xenbus.c | 6 -- > > 2 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/block/xen-blkback/common.h > > b/drivers/block/xen-blkback/common.h > > index bda5c815e441..a28473470e66 100644 > > --- a/drivers/block/xen-blkback/common.h > > +++ b/drivers/block/xen-blkback/common.h > > @@ -226,6 +226,9 @@ struct xen_vbd { > > sector_tsize; > > unsigned intflush_support:1; > > unsigned intdiscard_secure:1; > > + /* Connect-time cached feature_persistent parameter value */ > > + unsigned intfeature_gnt_persistent_parm:1; > > Continuing over from the previous version: > > > > If feature_gnt_persistent_parm is always going to be equal to > > > feature_persistent, then why introduce it at all? Why not just use > > > feature_persistent directly? This way you avoid adding an extra flag > > > whose purpose is not immediately clear, and you also avoid all the > > > mess with setting this flag at the right time. > > > > Mainly because the parameter should read twice (once for > > advertisement, and once later just before the negotitation, for > > checking if we advertised or not), and the user might change the > > parameter value between the two reads. > > > > For the detailed available sequence of the race, you could refer to the > > prior conversation[1]. > > > > [1] https://lore.kernel.org/linux-block/20200922111259.GJ19254@Air-de-Roger/ > > Okay, I see. Thanks for the pointer. But still, I think it would be > better to not maintain two copies of the value. How about doing: > > blkif->vbd.feature_gnt_persistent = > xenbus_read_unsigned(dev->nodename, "feature-persistent", 0) && > xenbus_read_unsigned(dev->otherend, "feature-persistent", 0); > > This makes it quite clear that we only enable persistent grants if > _both_ ends support it. We can do the same for blkfront. Currently, the feature-persistent xenstore entry is written to from connect() which is called after connect_ring(). So it's not available like this. Perhaps it's possible to delay the decision whether to use persistent grants until connect(). > > > + /* Persistent grants feature negotiation result */ > > unsigned intfeature_gnt_persistent:1; > > unsigned intoverflow_max_grants:1; > > }; > > diff --git a/drivers/block/xen-blkback/xenbus.c > > b/drivers/block/xen-blkback/xenbus.c > > index ee7ad2fb432d..c0227dfa4688 100644 > > --- a/drivers/block/xen-blkback/xenbus.c > > +++ b/drivers/block/xen-blkback/xenbus.c > > @@ -907,7 +907,7 @@ static void connect(struct backend_info *be) > > xen_blkbk_barrier(xbt, be, be->blkif->vbd.flush_support); > > > > err = xenbus_printf(xbt, dev->nodename, "feature-persistent", "%u", > > - be->blkif->vbd.feature_gnt_persistent); > > + be->blkif->vbd.feature_gnt_persistent_parm); > > if (err) { > > xenbus_dev_fatal(dev, err, "writing %s/feature-persistent", > > dev->nodename); > > @@ -1085,7 +1085,9 @@ static int connect_ring(struct backend_info *be) > > return -ENOSYS; > > } > > > > - blkif->vbd.feature_gnt_persistent = feature_persistent && > > + blkif->vbd.feature_gnt_persistent_parm = feature_persistent; > > + blkif->vbd.feature_gnt_persistent = > > + blkif->vbd.feature_gnt_persistent_parm && > > xenbus_read_unsigned(dev->otherend, "feature-persistent", > > 0); > > > > blkif->vbd.overflow_max_grants = 0; > > -- > > 2.25.1 > > > > -- > Amazon Development Center Germany GmbH > Krausenstr. 38 > 10117 Berlin > Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss > Eingetragen
Re: [PATCH v2 1/3] xen-blkback: Advertise feature-persistent as user requested
On 02.09.22 11:53, Pratyush Yadav wrote: On 31/08/22 04:58PM, SeongJae Park wrote: The advertisement of the persistent grants feature (writing 'feature-persistent' to xenbus) should mean not the decision for using the feature but only the availability of the feature. However, commit aac8a70db24b ("xen-blkback: add a parameter for disabling of persistent grants") made a field of blkback, which was a place for saving only the negotiation result, to be used for yet another purpose: caching of the 'feature_persistent' parameter value. As a result, the advertisement, which should follow only the parameter value, becomes inconsistent. This commit fixes the misuse of the semantic by making blkback saves the parameter value in a separate place and advertises the support based on only the saved value. Fixes: aac8a70db24b ("xen-blkback: add a parameter for disabling of persistent grants") Cc: # 5.10.x Suggested-by: Juergen Gross Signed-off-by: SeongJae Park --- drivers/block/xen-blkback/common.h | 3 +++ drivers/block/xen-blkback/xenbus.c | 6 -- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h index bda5c815e441..a28473470e66 100644 --- a/drivers/block/xen-blkback/common.h +++ b/drivers/block/xen-blkback/common.h @@ -226,6 +226,9 @@ struct xen_vbd { sector_tsize; unsigned intflush_support:1; unsigned intdiscard_secure:1; + /* Connect-time cached feature_persistent parameter value */ + unsigned intfeature_gnt_persistent_parm:1; Continuing over from the previous version: If feature_gnt_persistent_parm is always going to be equal to feature_persistent, then why introduce it at all? Why not just use feature_persistent directly? This way you avoid adding an extra flag whose purpose is not immediately clear, and you also avoid all the mess with setting this flag at the right time. Mainly because the parameter should read twice (once for advertisement, and once later just before the negotitation, for checking if we advertised or not), and the user might change the parameter value between the two reads. For the detailed available sequence of the race, you could refer to the prior conversation[1]. [1] https://lore.kernel.org/linux-block/20200922111259.GJ19254@Air-de-Roger/ Okay, I see. Thanks for the pointer. But still, I think it would be better to not maintain two copies of the value. How about doing: blkif->vbd.feature_gnt_persistent = xenbus_read_unsigned(dev->nodename, "feature-persistent", 0) && xenbus_read_unsigned(dev->otherend, "feature-persistent", 0); This makes it quite clear that we only enable persistent grants if _both_ ends support it. We can do the same for blkfront. I prefer it as is, as it will not rely on nobody having modified the Xenstore node (which would in theory be possible). Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v2 0/3] xen-blk{front, back}: Fix the broken semantic and flow of feature-persistent
On Wed, Aug 31, 2022 at 04:58:21PM +, SeongJae Park wrote: > Changes from v1 > (https://lore.kernel.org/xen-devel/20220825161511.94922-1...@kernel.org/) > - Fix the wrong feature_persistent caching position of blkfront > - Set blkfront's feature_persistent field setting with simple '&&' > instead of 'if' (Pratyush Yadav) > > This patchset fixes misuse of the 'feature-persistent' advertisement > semantic (patches 1 and 2), and the wrong timing of the > 'feature_persistent' value caching, which made persistent grants feature > always disabled. > > SeongJae Park (3): > xen-blkback: Advertise feature-persistent as user requested > xen-blkfront: Advertise feature-persistent as user requested > xen-blkfront: Cache feature_persistent value before advertisement > > drivers/block/xen-blkback/common.h | 3 +++ > drivers/block/xen-blkback/xenbus.c | 6 -- > drivers/block/xen-blkfront.c | 20 > 3 files changed, 19 insertions(+), 10 deletions(-) > > -- > 2.25.1 > I've tested this patch series in the following ways: * Only applied the blkback patch but not the blkfront patches * Only applied the blkfront patches but not the blkback patch * Applied both All scenarios worked, so Tested-by: Maximilian Heyne Actually I also wanted to test changing feature_persistent and try reconnecting but I don't know how this is done. If anyone has a pointer here, I could test that as well. Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
[xen-unstable test] 172925: tolerable FAIL
flight 172925 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/172925/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a build-amd64-libvirt 6 libvirt-buildfail like 172910 build-i386-libvirt6 libvirt-buildfail like 172910 build-arm64-libvirt 6 libvirt-buildfail like 172910 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 172910 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 172910 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 172910 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 172910 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 172910 build-armhf-libvirt 6 libvirt-buildfail like 172910 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 172910 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 172910 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 172910 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 172910 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen e997d055929665b12246e89eb092dc79c65de9a4 baseline version: xen
Re: [PATCH v2 1/3] xen-blkback: Advertise feature-persistent as user requested
On 31/08/22 04:58PM, SeongJae Park wrote: > The advertisement of the persistent grants feature (writing > 'feature-persistent' to xenbus) should mean not the decision for using > the feature but only the availability of the feature. However, commit > aac8a70db24b ("xen-blkback: add a parameter for disabling of persistent > grants") made a field of blkback, which was a place for saving only the > negotiation result, to be used for yet another purpose: caching of the > 'feature_persistent' parameter value. As a result, the advertisement, > which should follow only the parameter value, becomes inconsistent. > > This commit fixes the misuse of the semantic by making blkback saves the > parameter value in a separate place and advertises the support based on > only the saved value. > > Fixes: aac8a70db24b ("xen-blkback: add a parameter for disabling of > persistent grants") > Cc: # 5.10.x > Suggested-by: Juergen Gross > Signed-off-by: SeongJae Park > --- > drivers/block/xen-blkback/common.h | 3 +++ > drivers/block/xen-blkback/xenbus.c | 6 -- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/block/xen-blkback/common.h > b/drivers/block/xen-blkback/common.h > index bda5c815e441..a28473470e66 100644 > --- a/drivers/block/xen-blkback/common.h > +++ b/drivers/block/xen-blkback/common.h > @@ -226,6 +226,9 @@ struct xen_vbd { > sector_tsize; > unsigned intflush_support:1; > unsigned intdiscard_secure:1; > + /* Connect-time cached feature_persistent parameter value */ > + unsigned intfeature_gnt_persistent_parm:1; Continuing over from the previous version: > > If feature_gnt_persistent_parm is always going to be equal to > > feature_persistent, then why introduce it at all? Why not just use > > feature_persistent directly? This way you avoid adding an extra flag > > whose purpose is not immediately clear, and you also avoid all the > > mess with setting this flag at the right time. > > Mainly because the parameter should read twice (once for > advertisement, and once later just before the negotitation, for > checking if we advertised or not), and the user might change the > parameter value between the two reads. > > For the detailed available sequence of the race, you could refer to the > prior conversation[1]. > > [1] https://lore.kernel.org/linux-block/20200922111259.GJ19254@Air-de-Roger/ Okay, I see. Thanks for the pointer. But still, I think it would be better to not maintain two copies of the value. How about doing: blkif->vbd.feature_gnt_persistent = xenbus_read_unsigned(dev->nodename, "feature-persistent", 0) && xenbus_read_unsigned(dev->otherend, "feature-persistent", 0); This makes it quite clear that we only enable persistent grants if _both_ ends support it. We can do the same for blkfront. > + /* Persistent grants feature negotiation result */ > unsigned intfeature_gnt_persistent:1; > unsigned intoverflow_max_grants:1; > }; > diff --git a/drivers/block/xen-blkback/xenbus.c > b/drivers/block/xen-blkback/xenbus.c > index ee7ad2fb432d..c0227dfa4688 100644 > --- a/drivers/block/xen-blkback/xenbus.c > +++ b/drivers/block/xen-blkback/xenbus.c > @@ -907,7 +907,7 @@ static void connect(struct backend_info *be) > xen_blkbk_barrier(xbt, be, be->blkif->vbd.flush_support); > > err = xenbus_printf(xbt, dev->nodename, "feature-persistent", "%u", > - be->blkif->vbd.feature_gnt_persistent); > + be->blkif->vbd.feature_gnt_persistent_parm); > if (err) { > xenbus_dev_fatal(dev, err, "writing %s/feature-persistent", > dev->nodename); > @@ -1085,7 +1085,9 @@ static int connect_ring(struct backend_info *be) > return -ENOSYS; > } > > - blkif->vbd.feature_gnt_persistent = feature_persistent && > + blkif->vbd.feature_gnt_persistent_parm = feature_persistent; > + blkif->vbd.feature_gnt_persistent = > + blkif->vbd.feature_gnt_persistent_parm && > xenbus_read_unsigned(dev->otherend, "feature-persistent", 0); > > blkif->vbd.overflow_max_grants = 0; > -- > 2.25.1 > -- Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
RE: [PATCH 2/2] xen/arm: Handle reserved heap pages in boot and heap allocator
Hi Julien, > -Original Message- > From: Julien Grall > Sent: 2022年9月2日 16:05 > To: Wei Chen ; Stefano Stabellini > > Cc: Henry Wang ; xen-devel@lists.xenproject.org; > Bertrand Marquis ; Volodymyr Babchuk > > Subject: Re: [PATCH 2/2] xen/arm: Handle reserved heap pages in boot and > heap allocator > > Hi Wei, > > On 02/09/2022 04:07, Wei Chen wrote: > > > > > >> -Original Message- > >> From: Xen-devel On Behalf Of > Wei > >> Chen > >> Sent: 2022年9月2日 11:03 > >> To: Stefano Stabellini ; Julien Grall > >> > >> Cc: Henry Wang ; xen-devel@lists.xenproject.org; > >> Bertrand Marquis ; Volodymyr Babchuk > >> > >> Subject: RE: [PATCH 2/2] xen/arm: Handle reserved heap pages in boot > and > >> heap allocator > >> > >> Hi Julien and Stefano, > >> > >>> -Original Message- > >>> From: Stefano Stabellini > >>> Sent: 2022年9月2日 9:51 > >>> To: Julien Grall > >>> Cc: Stefano Stabellini ; Henry Wang > >>> ; xen-devel@lists.xenproject.org; Bertrand Marquis > >>> ; Wei Chen ; Volodymyr > >> Babchuk > >>> > >>> Subject: Re: [PATCH 2/2] xen/arm: Handle reserved heap pages in boot > and > >>> heap allocator > >>> > >>> On Thu, 1 Sep 2022, Julien Grall wrote: > Hi Stefano, > >>> > >> > >>> In any case, I think we can postpone to after the release. > > > > Maybe we can add some notes to say that this feature is still > > experimental in EFI + DTS boot? > > Why EFI + DTS only? Regardless the discussion about how to properly > checking the region, I think this wants to be a tech preview. > I had thought something like uboot + dts will not have reserved memory regions like EFI runtime services. But I forgot uboot also will have some address to load Xen and DTB. In this case, Xen still need to check relocation addresses with static heap. So you're right, I agree with you. Cheers, Wei Chen. > Cheers, > > -- > Julien Grall
[libvirt test] 172930: regressions - FAIL
flight 172930 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/172930/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 151777 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a version targeted for testing: libvirt c8b796aba31b2c97a1a56867062b3bdd0d81923f baseline version: libvirt 2c846fa6bcc11929c9fb857a22430fb9945654ad Last test of basis 151777 2020-07-10 04:19:19 Z 784 days Failing since151818 2020-07-11 04:18:52 Z 783 days 765 attempts Testing same since 172930 2022-09-02 04:20:43 Z0 days1 attempts People who touched revisions under test: Adolfo Jayme Barrientos Aleksandr Alekseev Aleksei Zakharov Amneesh Singh Andika Triwidada Andrea Bolognani Andrew Melnychenko Ani Sinha Balázs Meskó Barrett Schonefeld Bastian Germann Bastien Orivel BiaoXiang Ye Bihong Yu Binfeng Wu Bjoern Walk Boris Fiuczynski Brad Laue Brian Turek Bruno Haible Carlos Bilbao Chris Mayo Christian Borntraeger Christian Ehrhardt Christian Kirbach Christian Schoenebeck Christophe de Dinechin Christophe Fergeau Claudio Fontana Cole Robinson Collin Walling Cornelia Huck Cédric Bosdonnat Côme Borsoi Daniel Henrique Barboza Daniel Letai Daniel P. Berrange Daniel P. Berrangé Dario Faggioli David Michael Didik Supriadi dinglimin Divya Garg Dmitrii Shcherbakov Dmytro Linkin Eiichi Tsukata Emilio Herrera Eric Farman Erik Skultety Eugenio Pérez Fabian Affolter Fabian Freyer Fabiano Fidêncio Fangge Jin Farhan Ali Fedora Weblate Translation Florian Schmidt Franck Ridel Gavi Teitz gongwei Guoyi Tu Göran Uddeborg Halil Pasic Han Han Hao Wang Haonan Wang Hela Basa Helmut Grohne Hiroki Narukawa Hyman Huang(黄勇) Ian Wienand Ioanna Alifieraki Ivan Teterevkov Jakob Meng Jamie Strandboge Jamie Strandboge Jan Kuparinen jason lee Jean-Baptiste Holcroft Jia Zhou Jianan Gao Jim Fehlig Jin Yan Jing Qi Jinsheng Zhang Jiri Denemark Joachim Falk John Ferlan John Levon John Levon Jonathan Watt Jonathon Jongsma Julio Faracco Justin Gatzen Ján Tomko Kashyap Chamarthy Kevin Locke Kim InSoo Koichi Murase Kristina Hanicova Laine Stump Laszlo Ersek Lee Yarwood Lei Yang Lena Voytek Liang Yan Liang Yan Liao Pingfang Lin Ma Lin Ma Lin Ma Liu Yiding Lubomir Rintel Ludek Janda Luke Yue Luyao Zhong luzhipeng Marc Hartmayer Marc-André Lureau Marek Marczykowski-Górecki Mark Mielke Markus Schade Martin Kletzander Martin Pitt Masayoshi Mizuma Matej Cepl Matt Coleman Matt Coleman Mauro Matteo Cascella Max Goodhart Maxim Nestratov Meina Li Michal Privoznik Michał Smyk Milo Casagrande minglei.liu Moshe Levi Moteen Shah Moteen Shah Muha Aliss Nathan Neal Gompa Nick Chevsky Nick Shyrokovskiy Nickys Music Group Nico Pache Nicolas Lécureuil Nicolas Lécureuil Nikolay Shirokovskiy Nikolay Shirokovskiy Nikolay Shirokovskiy Niteesh Dubey Olaf Hering Oleksandr Tyshchenko
Re: Enable audio virtualization in Xen
On Thu, Sep 1, 2022 at 11:58 AM SHARMA, JYOTIRMOY wrote: > [AMD Official Use Only - General] > > Hi all, > Hello Jyotirmoy. [sorry for the possible format issues] > > > Forgot to mention that I am able to play audio from HVM guest with Pulse > Audio as back end. > good. > Here is the corresponding HVM configuration: > > > > vsnd = [[ 'card, backend=Domain-0, buffer-size=65536, short-name=VCard, > long-name=Virtual sound card, sample-rates=44100, sample-formats=s16_le', > 'pcm, name=dev1', 'stream, unique-id=pulse, type=P' ]] > > > > I have used xen front end and snd_be (along with libxenbe) as back end as > suggested by Christopher earlier in this thread. > > > > Only when I change unique-id=alsa, audio is not working from HVM guest. > > > > If anyone has tried ALSA back end (instead of PA), please let me know what > I am missing. > We use snd_be over pulse and it works fine. I am not too familiar with all that sound's internals. But I would suggest looking at the following thread where Oleksandr Grytsov (the author of the snd_be) mentioned the need of HW parameters matching for alsa [1]. I made an experiment with alsa (however I am not 100% sure whether it is correct), nevertheless details are below. I changed guest config to use alsa: vsnd = [[ 'card, backend=DomD, buffer-size=65536, short-name=VCard, long-name=Virtual sound card, sample-rates=8000;11025;16000;22050;32000;44100;48000, sample-formats=s16_le', 'pcm, name=dev1', 'stream, unique-id=alsa, type=P' ]] And checked that snd_be started using alsa: 03.07.22 03:37:17.185 | SndFrontend | DBG - Parse stream id: alsa 03.07.22 03:37:17.186 | SndFrontend | DBG - Create pcm device, type: ALSA, device: , propName: , propValue: ... With that command running in DomU I heard the audio in headphones: root@salvator-x-h3-4x2g-xt-domu:~# cat /dev/urandom | aplay -f S16_LE -c 2 -D hw:0,0 Playing raw data 'stdin' : Signed 16 bit Little Endian, Rate 8000 Hz, Stereo This is the output of "aplay -l" in both domains just in case: root@salvator-x-h3-4x2g-xt-domd:~# aplay -l List of PLAYBACK Hardware Devices card 0: rcarsound [rcar-sound], device 0: rsnd-dai.0-ak4613-hifi ak4613-hifi-0 [] Subdevices: 0/1 Subdevice #0: subdevice #0 root@salvator-x-h3-4x2g-xt-domu:~# aplay -l List of PLAYBACK Hardware Devices card 0: vsnd [], device 0: dev1 [Virtual card PCM] Subdevices: 1/1 Subdevice #0: subdevice #0 Please check your HW params as suggested at [1]. [1] https://lore.kernel.org/xen-devel/cacvf2ow7ghcldkqyi8w1s7-fbux8zcawanfza07xhvg-ioa...@mail.gmail.com/ > > > Regards, > > Jyotirmoy > > > -- Regards, Oleksandr Tyshchenko
RE: [PATCH 1/2] docs, xen/arm: Introduce reserved heap memory
Hi Julien, > -Original Message- > Subject: Re: [PATCH 1/2] docs, xen/arm: Introduce reserved heap memory > > Hi Henry, > > On 02/09/2022 02:28, Henry Wang wrote: > >> This is technically a change in behavior for Xen (we would panic rather > >> than continue). I am happy with the proposal. However, this doesn't seem > >> to be explained in the commit message. > >> > >> That said, I think this should be split in a separate patch along with > >> the ones below (including the prototype changes). > > > > According to Michal's comment, I've removed the return type and function > > prototype change in my local v2. So this patch itself is fine. My question > now > > would be, do maintainers think this change of behavior with processing the > > chosen node be helpful? > > Yes. I think it is saner to stop booting early rather than seen random > behavior afterwards. Cool, I will then add the patch to this series. > > > Do we prefer an instant panic or current behavior? > > I think we should leave that up to the caller. Today, this is a panic() > but we may decide differently in the future. Agreed. Kind regards, Henry > > Cheers, > > -- > Julien Grall
[linux-5.4 test] 172924: regressions - FAIL
flight 172924 linux-5.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/172924/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 172128 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 172128 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 172128 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 172128 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-debianhvm-amd64 20 guest-start/debianhvm.repeat fail pass in 172914 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 18 guest-start/debian.repeat fail in 172914 blocked in 172128 test-armhf-armhf-xl-multivcpu 18 guest-start/debian.repeat fail in 172914 like 172108 test-armhf-armhf-xl-multivcpu 15 migrate-support-check fail in 172914 never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-check fail in 172914 never pass test-armhf-armhf-xl-credit1 15 migrate-support-check fail in 172914 never pass test-armhf-armhf-xl-credit1 16 saverestore-support-check fail in 172914 never pass test-armhf-armhf-xl-vhd 14 migrate-support-check fail in 172914 never pass test-armhf-armhf-xl-vhd 15 saverestore-support-check fail in 172914 never pass test-armhf-armhf-xl-credit2 18 guest-start/debian.repeatfail like 172108 test-armhf-armhf-xl-vhd 13 guest-start fail like 172108 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 172108 test-armhf-armhf-xl-credit1 14 guest-start fail like 172128 test-armhf-armhf-xl-multivcpu 14 guest-start fail like 172128 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 172128 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 172128 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 172128 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 172128 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 172128 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 172128 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 172128 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeatfail like 172128 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 172128 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-check
RE: [PATCH 2/2] xen/arm: Handle reserved heap pages in boot and heap allocator
Hi Julien, > -Original Message- > From: Julien Grall > > Thanks for the clarification. I checked the code and found the xenheap_* > > variables are: > > xenheap_virt_start, xenheap_virt_end, xenheap_mfn_start, > > xenheap_mfn_end, xenheap_base_pdx. > > > > For clarification, do we need to change all of them to directmap_*? > > Good question. :))) Thanks for your patience! > > > > > A pure renaming should be easy (and I guess also safe), but maybe I am > > overthinking because arm32 also uses xenheap_virt_end, > xenheap_mfn_start > > and xenheap_mfn_end. These variables refer to the real xenheap, I am not > > sure renaming these would reduce the readability for arm32. > > So on arm32, only the xenheap is direct mapped today. So I think it > would be fine to switch the name to "directmap_*". For extra clarify we > could add an alias for arm32 between "xenheap_*" and "directmap_*". Sounds good to me, I think I will try to add a separate patch for purely renaming all above-mentioned variables, then another patch for creating the alias for arm32. Hope you would fine with this plan. Kind regards, Henry > > Cheers, > > -- > Julien Grall
Re: [PATCH v3 6/7] xen/arm: introduce xen-evtchn dom0less property
Hi Stefano, On 02/09/2022 03:20, Stefano Stabellini wrote: diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h index 430a1ef445..5579c875d2 100644 --- a/xen/include/xen/device_tree.h +++ b/xen/include/xen/device_tree.h @@ -82,6 +82,7 @@ struct dt_device_node { dt_phandle phandle; char *full_name; domid_t used_by; /* By default it's used by dom0 */ +bool_t static_evtchn_created; I can see why you want to add the boolean in dt_device_node. However, I dislike this approach because this feels an abuse of dt_device_node and the field is only used at boot. So this seems to be a bit of a waste to include it in the structure (even if we are re-using padding today). I don't have a solution that is has trivial as this approach. However, at minimum we should document this is a HACK and should be remove if we need space in the structure. I would move static_evtchn_created just above (or below) "bool is_protected". It would still re-use the padding and it would be closer to another more similar field of the struct. The only other option that I can think of would be to use port_is_valid, instead of static_evtchn_created, to check that the port has already been allocated, but we wouldn't be able to tell if it is a static evtchn or simply unavailable for other reasons You don't need to know the event channel was statically allocated. If you have access to the event channel, then you can easily find out what is the remote port. and it would require more device tree parsing. The parsing is indeed the big cons. Hence, why I hadn't suggest it. Cheers, -- Julien Grall
Re: [PATCH v6 1/9] xen/arm: introduce static shared memory
Hi Penny, On 02/09/2022 04:26, Penny Zheng wrote: Do you think I shall further point out that right now, this part feature is not implemented in codes? I have made a couple of suggestion for the code. But I think the binding would look a bit odd without the host physical address. We would now have: < [size] [guest address]> I think it would be more natural if we had <[guest address] [size]> And <[guest address] [size] [host physical address]> Ok, about the binding order change, do you prefer it in v7 or 4.17-post, since it may also need a few code tweak. The binding will become stable as soon as we release 4.17. So this would need to be fixed before releasing. Cheers, -- Julien Grall
Re: [PATCH 2/2] xen/arm: Handle reserved heap pages in boot and heap allocator
On 02/09/2022 04:30, Henry Wang wrote: Hi Julien, Hi Henry, -Original Message- From: Julien Grall This code is now becoming quite confusing to understanding. This loop is meant to map the xenheap. If I follow your documentation, it would mean that only the reserved region should be mapped. Yes I think this is the same question that I raised in the scissors line of the commit message of this patch. Sorry I didn't notice the comment after the scissors line. This is the same question :) What I intend to do is still mapping the whole RAM because of the xenheap_* variables that you mentioned in... More confusingly, xenheap_* variables will cover the full RAM. ...here. But only adding the reserved region to the boot allocator so the reserved region can become the heap later on. I am wondering if we have a more clear way to do that, any suggestions? I think your code is correct. It only needs some renaming of the existing variable (maybe to directmap_*?) to make clear the area is used to access the RAM easily. Thanks for the clarification. I checked the code and found the xenheap_* variables are: xenheap_virt_start, xenheap_virt_end, xenheap_mfn_start, xenheap_mfn_end, xenheap_base_pdx. For clarification, do we need to change all of them to directmap_*? Good question. A pure renaming should be easy (and I guess also safe), but maybe I am overthinking because arm32 also uses xenheap_virt_end, xenheap_mfn_start and xenheap_mfn_end. These variables refer to the real xenheap, I am not sure renaming these would reduce the readability for arm32. So on arm32, only the xenheap is direct mapped today. So I think it would be fine to switch the name to "directmap_*". For extra clarify we could add an alias for arm32 between "xenheap_*" and "directmap_*". Cheers, -- Julien Grall
Re: [PATCH 2/2] xen/arm: Handle reserved heap pages in boot and heap allocator
Hi Wei, On 02/09/2022 04:07, Wei Chen wrote: -Original Message- From: Xen-devel On Behalf Of Wei Chen Sent: 2022年9月2日 11:03 To: Stefano Stabellini ; Julien Grall Cc: Henry Wang ; xen-devel@lists.xenproject.org; Bertrand Marquis ; Volodymyr Babchuk Subject: RE: [PATCH 2/2] xen/arm: Handle reserved heap pages in boot and heap allocator Hi Julien and Stefano, -Original Message- From: Stefano Stabellini Sent: 2022年9月2日 9:51 To: Julien Grall Cc: Stefano Stabellini ; Henry Wang ; xen-devel@lists.xenproject.org; Bertrand Marquis ; Wei Chen ; Volodymyr Babchuk Subject: Re: [PATCH 2/2] xen/arm: Handle reserved heap pages in boot and heap allocator On Thu, 1 Sep 2022, Julien Grall wrote: Hi Stefano, In any case, I think we can postpone to after the release. Maybe we can add some notes to say that this feature is still experimental in EFI + DTS boot? Why EFI + DTS only? Regardless the discussion about how to properly checking the region, I think this wants to be a tech preview. Cheers, -- Julien Grall
Re: [PATCH 1/2] docs, xen/arm: Introduce reserved heap memory
Hi Henry, On 02/09/2022 02:28, Henry Wang wrote: This is technically a change in behavior for Xen (we would panic rather than continue). I am happy with the proposal. However, this doesn't seem to be explained in the commit message. That said, I think this should be split in a separate patch along with the ones below (including the prototype changes). According to Michal's comment, I've removed the return type and function prototype change in my local v2. So this patch itself is fine. My question now would be, do maintainers think this change of behavior with processing the chosen node be helpful? Yes. I think it is saner to stop booting early rather than seen random behavior afterwards. Do we prefer an instant panic or current behavior? I think we should leave that up to the caller. Today, this is a panic() but we may decide differently in the future. Cheers, -- Julien Grall
RE: [PATCH v11 6/6] xen: retrieve reserved pages on populate_physmap
Hi Julien > -Original Message- > From: Julien Grall > Sent: Thursday, September 1, 2022 9:53 PM > To: Penny Zheng ; xen-devel@lists.xenproject.org > Cc: Wei Chen ; Andrew Cooper > ; George Dunlap ; > Jan Beulich ; Stefano Stabellini ; > Wei Liu > Subject: Re: [PATCH v11 6/6] xen: retrieve reserved pages on > populate_physmap > > Hi Penny, > > On 31/08/2022 03:40, Penny Zheng wrote: > > +/* > > + * Acquire a page from reserved page list(resv_page_list), when > > +populating > > + * memory for static domain on runtime. > > + */ > > +mfn_t acquire_reserved_page(struct domain *d, unsigned int memflags) > > +{ > > +struct page_info *page; > > + > > +ASSERT_ALLOC_CONTEXT(); > > + > > +/* Acquire a page from reserved page list(resv_page_list). */ > > +spin_lock(>page_alloc_lock); > > +page = page_list_remove_head(>resv_page_list); > > +spin_unlock(>page_alloc_lock); > > +if ( unlikely(!page) ) > > +return INVALID_MFN; > > + > > +if ( !prepare_staticmem_pages(page, 1, memflags) ) > > +goto fail; > > + > > +if ( assign_domstatic_pages(d, page, 1, memflags) ) > > +goto fail_assign; > > + > > +return page_to_mfn(page); > > + > > + fail_assign: > > +unprepare_staticmem_pages(page, 1, false); > > Looking at assign_domstatic_pages(). It will already call > unprepare_staticmem_pages() in one of the error path. It doesn't look like > the latter can be called twice on a page. > > To be honest, I find a bit odd that assign_domstatic_pages() is calling > unprepare_staticmem_pages() because the former doesn't call the "prepare" > function. > > AFAICT, this is an issue introduced in this patch. So I would remove the call > from assign_domstatic_pages() and then let the caller calls > unprepare_staticmem_pages() (this would need to be added in > acquire_domstatic_pages()). > True, true, thanks for pointing out!! > Also, I think it would be good to explain why we don't need to scrub. > Something like: > > "The page was never accessible by the domain. So scrubbing can be skipped". > Ok, I'll add in-code comment > Cheers, > > -- > Julien Grall
[for-4.17 3/3] automation: Add a new job for testing boot time cpupools on arm64
Add a new test job qemu-smoke-arm64-gcc-boot-cpupools that will execute script qemu-smoke-arm64.sh to test boot time cpupools feature. Enable CONFIG_BOOT_TIME_CPUPOOLS for the arm64 build and add a new test case in qemu-smoke-arm64.sh that if selected will: - create a device tree cpupool node with cpu@1 - assign created cpupool to domU0 - add a check in dom0 xen.start to see if domU is assigned a Pool-1 Take the opportunity to refactor the qemu-smoke-arm64.sh script as follows: - use domU_check to store the test's commands to be run from domU - use dom0_check to store the test's commands to be run from dom0 - use fdtput instead of sed to perform dtb modifications - use more meaningful messages for "passed" variable. This way we can grep for messages reported either by domU or dom0 and get rid of assumption that tests can only be run from domU Signed-off-by: Michal Orzel --- automation/gitlab-ci/test.yaml | 19 +++ automation/scripts/build | 3 ++- automation/scripts/qemu-smoke-arm64.sh | 33 +++--- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index 07209820b474..d899b3bdbf7a 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -100,6 +100,25 @@ qemu-smoke-arm64-gcc-staticmem: tags: - arm64 +qemu-smoke-arm64-gcc-boot-cpupools: + extends: .test-jobs-common + variables: +CONTAINER: debian:unstable-arm64v8 + script: +- ./automation/scripts/qemu-smoke-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log + needs: +- alpine-3.12-gcc-arm64 +- alpine-3.12-arm64-rootfs-export +- kernel-5.19-arm64-export +- qemu-system-aarch64-6.0.0-arm64-export + artifacts: +paths: + - smoke.serial + - '*.log' +when: always + tags: +- arm64 + qemu-smoke-arm32-gcc: extends: .test-jobs-common variables: diff --git a/automation/scripts/build b/automation/scripts/build index 2b9f2d2b541a..2f15ab3198e6 100755 --- a/automation/scripts/build +++ b/automation/scripts/build @@ -19,7 +19,8 @@ else echo " CONFIG_EXPERT=y CONFIG_UNSUPPORTED=y -CONFIG_STATIC_MEMORY=y" > xen/.config +CONFIG_STATIC_MEMORY=y +CONFIG_BOOT_TIME_CPUPOOLS=y" > xen/.config make -j$(nproc) -C xen olddefconfig else make -j$(nproc) -C xen defconfig diff --git a/automation/scripts/qemu-smoke-arm64.sh b/automation/scripts/qemu-smoke-arm64.sh index 7ac96027760d..c2184850293c 100755 --- a/automation/scripts/qemu-smoke-arm64.sh +++ b/automation/scripts/qemu-smoke-arm64.sh @@ -4,20 +4,22 @@ set -ex test_variant=$1 -passed="passed" -check=" +if [ -z "${test_variant}" ]; then +passed="ping test passed" +domU_check=" until ifconfig eth0 192.168.0.2 &> /dev/null && ping -c 10 192.168.0.1; do sleep 30 done echo \"${passed}\" " +fi if [[ "${test_variant}" == "static-mem" ]]; then # Memory range that is statically allocated to DOM1 domu_base="5000" domu_size="1000" passed="${test_variant} test passed" -check=" +domU_check=" current=\$(hexdump -e '16/1 \"%02x\"' /proc/device-tree/memory@${domu_base}/reg 2>/dev/null) expected=$(printf \"%016x%016x\" 0x${domu_base} 0x${domu_size}) if [[ \"\${expected}\" == \"\${current}\" ]]; then @@ -42,11 +44,23 @@ curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom -cpu cortex-a57 -machine type=virt \ -m 1024 -smp 2 -display none \ -machine dumpdtb=binaries/virt-gicv2.dtb -# XXX disable pl061 to avoid Linux crash -dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts -sed 's/compatible = "arm,pl061.*/status = "disabled";/g' binaries/virt-gicv2.dts > binaries/virt-gicv2-edited.dts -dtc -I dts -O dtb binaries/virt-gicv2-edited.dts > binaries/virt-gicv2.dtb +# XXX disable pl061 to avoid Linux crash +fdtput binaries/virt-gicv2.dtb -p -t s /pl061@903 status disabled + +if [[ "${test_variant}" == "boot-cpupools" ]]; then +# Create cpupool node and assign it to domU0 +cpu_phandle="$(fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle)" +cpupool_phandle="0xff" +fdtput binaries/virt-gicv2.dtb -p -t s /chosen/cpupool compatible xen,cpupool +fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool cpupool-cpus $cpu_phandle +fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool phandle $cpupool_phandle +fdtput binaries/virt-gicv2.dtb -p -t x /chosen/domU0 domain-cpupool $cpupool_phandle + +# Check if domU0 (id=1) is assigned to Pool-1 +passed="${test_variant} test passed" +dom0_check="if xl list -c 1 | grep -q Pool-1; then echo ${passed}; fi" +fi # Busybox mkdir -p initrd @@ -66,7 +80,7 @@ echo "#!/bin/sh mount -t proc proc /proc mount -t sysfs sysfs /sys mount -t devtmpfs devtmpfs /dev -${check} +${domU_check} /bin/sh" > initrd/init chmod +x initrd/init cd initrd @@ -98,6 +112,7 @@ ifconfig
[for-4.17 2/3] automation: qemu-smoke-arm64: Silence ifconfig error messages
During the ping test, dom1 tries to assign an ip to eth0 in a loop. Before setting up the network interface by dom0, this results in printing the following error message several times: (XEN) DOM1: ifconfig: SIOCSIFADDR: No such device Silence this by redirecting stderr/stdout to /dev/null as we do not care about the output and we should not pollute the log file. Signed-off-by: Michal Orzel --- automation/scripts/qemu-smoke-arm64.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/automation/scripts/qemu-smoke-arm64.sh b/automation/scripts/qemu-smoke-arm64.sh index c80d9b2aee00..7ac96027760d 100755 --- a/automation/scripts/qemu-smoke-arm64.sh +++ b/automation/scripts/qemu-smoke-arm64.sh @@ -6,7 +6,7 @@ test_variant=$1 passed="passed" check=" -until ifconfig eth0 192.168.0.2 && ping -c 10 192.168.0.1; do +until ifconfig eth0 192.168.0.2 &> /dev/null && ping -c 10 192.168.0.1; do sleep 30 done echo \"${passed}\" -- 2.25.1
[for-4.17 1/3] automation: qemu-alpine-arm64-gcc: Use kernel 5.19
After qemu-smoke-arm64 was changed to use kernel 5.19 we end up having two kernel configurations. This is something not needed and maintaining a single kernel version is always easier. Modify qemu-alpine-arm64-gcc to use kernel 5.19 and remove kernel 5.9 from tests-artifacts. Signed-off-by: Michal Orzel --- automation/gitlab-ci/build.yaml | 11 -- automation/gitlab-ci/test.yaml| 2 +- .../kernel/5.9.9-arm64v8.dockerfile | 34 --- 3 files changed, 1 insertion(+), 46 deletions(-) delete mode 100644 automation/tests-artifacts/kernel/5.9.9-arm64v8.dockerfile diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml index d2f75a090c0f..720ce6e07ba0 100644 --- a/automation/gitlab-ci/build.yaml +++ b/automation/gitlab-ci/build.yaml @@ -586,17 +586,6 @@ alpine-3.12-arm64-rootfs-export: tags: - arm64 -kernel-5.9.9-arm64-export: - extends: .test-jobs-artifact-common - image: registry.gitlab.com/xen-project/xen/tests-artifacts/kernel:5.9.9-arm64v8 - script: -- mkdir binaries && cp /Image binaries/Image - artifacts: -paths: - - binaries/Image - tags: -- arm64 - kernel-5.19-arm64-export: extends: .test-jobs-artifact-common image: registry.gitlab.com/xen-project/xen/tests-artifacts/kernel:5.19-arm64v8 diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index 2eb6c3866e2c..07209820b474 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -34,7 +34,7 @@ qemu-alpine-arm64-gcc: needs: - alpine-3.12-gcc-arm64 - alpine-3.12-arm64-rootfs-export -- kernel-5.9.9-arm64-export +- kernel-5.19-arm64-export - qemu-system-aarch64-6.0.0-arm64-export artifacts: paths: diff --git a/automation/tests-artifacts/kernel/5.9.9-arm64v8.dockerfile b/automation/tests-artifacts/kernel/5.9.9-arm64v8.dockerfile deleted file mode 100644 index 053d65a3454e.. --- a/automation/tests-artifacts/kernel/5.9.9-arm64v8.dockerfile +++ /dev/null @@ -1,34 +0,0 @@ -FROM arm64v8/debian:unstable -LABEL maintainer.name="The Xen Project" \ - maintainer.email="xen-devel@lists.xenproject.org" - -ENV DEBIAN_FRONTEND=noninteractive -ENV LINUX_VERSION=5.9.9 -ENV USER root - -RUN mkdir /build -WORKDIR /build - -# build depends -RUN apt-get update && \ -apt-get --quiet --yes install \ -build-essential \ -libssl-dev \ -bc \ -curl \ -flex \ -bison \ -&& \ -\ -# Build the kernel -curl -fsSLO https://cdn.kernel.org/pub/linux/kernel/v5.x/linux-"$LINUX_VERSION".tar.xz && \ -tar xvJf linux-"$LINUX_VERSION".tar.xz && \ -cd linux-"$LINUX_VERSION" && \ -make defconfig && \ -make -j$(nproc) Image.gz && \ -cp arch/arm64/boot/Image / && \ -cd /build && \ -rm -rf linux-"$LINUX_VERSION"* && \ -apt-get autoremove -y && \ -apt-get clean && \ -rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/* -- 2.25.1
[for-4.17 0/3] GitLab CI cleanup and boot time cpupools test
This patch series performs a small cleanup before the release and adds a test for validating boot time cpupools feature introduced in 4.17. Notes for the release manager: Benefits: - improved dom0less test coverage - tested feature that is introduced in 4.17 Risks: - CI pipeline failure Michal Orzel (3): automation: qemu-alpine-arm64-gcc: Use kernel 5.19 automation: qemu-smoke-arm64: Silence ifconfig error messages automation: Add a new job for testing boot time cpupools on arm64 automation/gitlab-ci/build.yaml | 11 -- automation/gitlab-ci/test.yaml| 21 ++- automation/scripts/build | 3 +- automation/scripts/qemu-smoke-arm64.sh| 35 +-- .../kernel/5.9.9-arm64v8.dockerfile | 34 -- 5 files changed, 47 insertions(+), 57 deletions(-) delete mode 100644 automation/tests-artifacts/kernel/5.9.9-arm64v8.dockerfile -- 2.25.1
Re: [PATCH] xen/grants: prevent integer overflow in gnttab_dma_alloc_pages()
On 01.09.22 17:35, Dan Carpenter wrote: The change from kcalloc() to kvmalloc() means that arg->nr_pages might now be large enough that the "args->nr_pages << PAGE_SHIFT" can result in an integer overflow. Fixes: b3f7931f5c61 ("xen/gntdev: switch from kcalloc() to kvcalloc()") Signed-off-by: Dan Carpenter Reviewed-by: Juergen Gross Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature