Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path
On 2022-05-10, Steven Rostedt wrote: >> As already mentioned in the other reply, panic() sometimes stops the >> other CPUs using NMI, for example, see kdump_nmi_shootdown_cpus(). >> >> Another situation is when the CPU using the lock ends in some >> infinite loop because something went wrong. The system is in >> an unpredictable state during panic(). >> >> I am not sure if this is possible with the code under gsmi_dev.lock >> but such things really happen during panic() in other subsystems. >> Using trylock in the panic() code path is a good practice. > > I believe that Peter Zijlstra had a special spin lock for NMIs or > early printk, where it would not block if the lock was held on the > same CPU. That is, if an NMI happened and paniced while this lock was > held on the same CPU, it would not deadlock. But it would block if the > lock was held on another CPU. Yes. And starting with 5.19 it will be carrying the name that _you_ came up with (cpu_sync): printk_cpu_sync_get_irqsave() printk_cpu_sync_put_irqrestore() John
[ovmf test] 170316: regressions - FAIL
flight 170316 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170316/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 9dd964f5e5c5595a1acd5eb438fb088327db86fa baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 71 days 926 attempts Testing same since 170313 2022-05-11 01:56:56 Z0 days3 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6294 lines long.)
[ovmf test] 170315: regressions - FAIL
flight 170315 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170315/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 9dd964f5e5c5595a1acd5eb438fb088327db86fa baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 71 days 925 attempts Testing same since 170313 2022-05-11 01:56:56 Z0 days2 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6294 lines long.)
Re: [PATCH v3 4/6] xen: Switch to byteswap
On Tue, 10 May 2022, Julien Grall wrote: > > It is not reasonable to say "this unrelated thing is broken, and you > > need to fix it first to get your series in". Requests like that are, > > I'm sure, part of what Bertrand raised in the community call as > > unnecessary fiction getting work submitted. > > To be honest, you put the contributor in this situation. I would have been > perfectly happy if we keep *cpup* around as there would be only a place to > fix. > > With this approach, you are effectively going to increase the work later one > because now we would have to chase all the open-coded version of *cpup* and > check which one is not safe. Without disagreeing with Julien or Andrew, I am actually happy to see an effort to make the review process faster. We have lot of room for improvement and spotting opportunities to do so is the first step toward improving the process. I have actually been thinking about how to make things faster in cases like this and I have a suggesion below. In this case all of the options are OK. Whether we fix the alignment problem as part of this patch or soon after it doesn't make much of a difference. It is more important that we don't get bogged down in a long discussion. Coding is faster and more fun. It would take less time for Julien (or Andrew) to write the code than to explain to the contributor how to do it. English is a good language for an architectural discussion, but simply replying with the example code in C would be much faster in cases like this one. So my suggestion is that it would be best if the reviewer (Julien in this case) replied directly with the code snipper he wants added. Just an example without looking too closely: --- Please do this instead so that alignment also gets fixed: return be16_to_cpu(*(const __packed uint16_t *)p); --- Alternatively, I also think that taking this patch as is without alignment fix (either using be16_to_cpu or be16_to_cpup) is fine. The alignment could be fixed afterwards. The key is that I think it should be one of the maintainers to write the follow-up fix. Both of you are very fast coders and would certainly finish the patch before finishing the explanation on what needs to be done, which then would need to be understood and implemented by the contributor (opportunity for misunderstandings), and verified again by the reviewer on v2. That would take an order of magnitude more of collective time and effort. Of course this doesn't apply to all cases, but it should apply to quite a few. In short, less English, more C ;-)
[ovmf test] 170313: regressions - FAIL
flight 170313 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170313/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 9dd964f5e5c5595a1acd5eb438fb088327db86fa baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 71 days 924 attempts Testing same since 170313 2022-05-11 01:56:56 Z0 days1 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6294 lines long.)
[xen-unstable test] 170307: tolerable FAIL - PUSHED
flight 170307 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/170307/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 170293 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 170293 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 170293 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 170293 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 170293 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 170293 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 170293 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 170293 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 170293 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 170293 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 170293 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 170293 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-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-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 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-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 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 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-arm64-arm64-xl-vhd 15 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-libvirt 15 migrate-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-libvirt-qcow2 14 migrate-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-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 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-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: xen 407b13a71e324aba76b11e5f66f59ce4a304a088 baseline version: xen
[PATCH v3 7/9] xen/x86: use paddr_t for addresses in NUMA node structure
NUMA node structure "struct node" is using u64 as node memory range. In order to make other architectures can reuse this NUMA node relative code, we replace the u64 to paddr_t. And use pfn_to_paddr and paddr_to_pfn to replace explicit shift operations. The relate PRIx64 in print messages have been replaced by PRIpaddr at the same time. And some being-phased-out types like u64 in the lines we have touched also have been converted to uint64_t or unsigned long. Tested-by: Jiamei Xie Signed-off-by: Wei Chen --- v2 -> v3: 1. Use uint64_t for size in acpi_scan_nodes, make it be consistent with numa_emulation. 2. Add Tb. v1 -> v2: 1. Drop useless cast. 2. Use initializers of the variables. 3. Replace u64 by uint64_t. 4. Use unsigned long for start_pfn and end_pfn. --- xen/arch/x86/include/asm/numa.h | 8 xen/arch/x86/numa.c | 32 +++- xen/arch/x86/srat.c | 25 + 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/xen/arch/x86/include/asm/numa.h b/xen/arch/x86/include/asm/numa.h index 5d8385f2e1..c32ccffde3 100644 --- a/xen/arch/x86/include/asm/numa.h +++ b/xen/arch/x86/include/asm/numa.h @@ -18,7 +18,7 @@ extern cpumask_t node_to_cpumask[]; #define node_to_cpumask(node)(node_to_cpumask[node]) struct node { - u64 start,end; + paddr_t start, end; }; extern int compute_hash_shift(struct node *nodes, int numnodes, @@ -38,7 +38,7 @@ extern void numa_set_node(int cpu, nodeid_t node); extern nodeid_t setup_node(unsigned int pxm); extern void srat_detect_node(int cpu); -extern void setup_node_bootmem(nodeid_t nodeid, u64 start, u64 end); +extern void setup_node_bootmem(nodeid_t nodeid, paddr_t start, paddr_t end); extern nodeid_t apicid_to_node[]; extern void init_cpu_to_node(void); @@ -76,9 +76,9 @@ static inline __attribute__((pure)) nodeid_t phys_to_nid(paddr_t addr) NODE_DATA(nid)->node_spanned_pages) #define arch_want_default_dmazone() (num_online_nodes() > 1) -extern int valid_numa_range(u64 start, u64 end, nodeid_t node); +extern int valid_numa_range(paddr_t start, paddr_t end, nodeid_t node); -void srat_parse_regions(u64 addr); +void srat_parse_regions(paddr_t addr); extern u8 __node_distance(nodeid_t a, nodeid_t b); unsigned int arch_get_dma_bitsize(void); diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c index 680b7d9002..627ae8aa95 100644 --- a/xen/arch/x86/numa.c +++ b/xen/arch/x86/numa.c @@ -162,12 +162,10 @@ int __init compute_hash_shift(struct node *nodes, int numnodes, return shift; } /* initialize NODE_DATA given nodeid and start/end */ -void __init setup_node_bootmem(nodeid_t nodeid, u64 start, u64 end) -{ -unsigned long start_pfn, end_pfn; - -start_pfn = start >> PAGE_SHIFT; -end_pfn = end >> PAGE_SHIFT; +void __init setup_node_bootmem(nodeid_t nodeid, paddr_t start, paddr_t end) +{ +unsigned long start_pfn = paddr_to_pfn(start); +unsigned long end_pfn = paddr_to_pfn(end); NODE_DATA(nodeid)->node_start_pfn = start_pfn; NODE_DATA(nodeid)->node_spanned_pages = end_pfn - start_pfn; @@ -198,11 +196,12 @@ void __init numa_init_array(void) static int numa_fake __initdata = 0; /* Numa emulation */ -static int __init numa_emulation(u64 start_pfn, u64 end_pfn) +static int __init numa_emulation(unsigned long start_pfn, + unsigned long end_pfn) { int i; struct node nodes[MAX_NUMNODES]; -u64 sz = ((end_pfn - start_pfn)< 1 ) @@ -218,9 +217,9 @@ static int __init numa_emulation(u64 start_pfn, u64 end_pfn) memset(,0,sizeof(nodes)); for ( i = 0; i < numa_fake; i++ ) { -nodes[i].start = (start_pfnflags & ACPI_SRAT_MEM_HOT_PLUGGABLE) != !test_bit(i, memblk_hotplug); - printk("%sSRAT: PXM %u (%"PRIx64"-%"PRIx64") overlaps with itself (%"PRIx64"-%"PRIx64")\n", + printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with itself (%"PRIpaddr"-%"PRIpaddr")\n", mismatch ? KERN_ERR : KERN_WARNING, pxm, start, end, node_memblk_range[i].start, node_memblk_range[i].end); if (mismatch) { @@ -327,7 +327,7 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma) } } else { printk(KERN_ERR - "SRAT: PXM %u (%"PRIx64"-%"PRIx64") overlaps with PXM %u (%"PRIx64"-%"PRIx64")\n", +
[PATCH v3 8/9] xen/x86: add detection of memory interleaves for different nodes
One NUMA node may contain several memory blocks. In current Xen code, Xen will maintain a node memory range for each node to cover all its memory blocks. But here comes the problem, in the gap of one node's two memory blocks, if there are some memory blocks don't belong to this node (remote memory blocks). This node's memory range will be expanded to cover these remote memory blocks. One node's memory range contains other nodes' memory, this is obviously not very reasonable. This means current NUMA code only can support node has no interleaved memory blocks. However, on a physical machine, the addresses of multiple nodes can be interleaved. So in this patch, we add code to detect memory interleaves of different nodes. NUMA initialization will be failed and error messages will be printed when Xen detect such hardware configuration. Signed-off-by: Wei Chen Tested-by: Jiamei Xie --- v2 -> v3: 1. Merge the check code from a separate function to conflicting_memblks. This will reduce the loop times of node memory blocks. 2. Use an enumeration to indicate conflict check status. 3. Use a pointer to get conflict memory block id. v1 -> v2: 1. Update the description to say we're after is no memory interleaves of different nodes. 2. Only update node range when it passes the interleave check. 3. Don't use full upper-case for "node". --- xen/arch/x86/srat.c | 115 +--- 1 file changed, 86 insertions(+), 29 deletions(-) diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c index 8ffe43bdfe..53835ae3eb 100644 --- a/xen/arch/x86/srat.c +++ b/xen/arch/x86/srat.c @@ -42,6 +42,12 @@ static struct node node_memblk_range[NR_NODE_MEMBLKS]; static nodeid_t memblk_nodeid[NR_NODE_MEMBLKS]; static __initdata DECLARE_BITMAP(memblk_hotplug, NR_NODE_MEMBLKS); +enum conflicts { + NO_CONFLICT = 0, + ERR_OVERLAP, + ERR_INTERLEAVE, +}; + static inline bool node_found(unsigned idx, unsigned pxm) { return ((pxm2node[idx].pxm == pxm) && @@ -119,20 +125,43 @@ int valid_numa_range(paddr_t start, paddr_t end, nodeid_t node) return 0; } -static __init int conflicting_memblks(paddr_t start, paddr_t end) +static enum conflicts __init +conflicting_memblks(nodeid_t nid, paddr_t start, paddr_t end, + paddr_t nd_start, paddr_t nd_end, int *mblkid) { int i; + /* +* Scan all recorded nodes' memory blocks to check conflicts: +* Overlap or interleave. +*/ for (i = 0; i < num_node_memblks; i++) { struct node *nd = _memblk_range[i]; + *mblkid = i; + + /* Skip 0 bytes node memory block. */ if (nd->start == nd->end) continue; + /* +* Use memblk range to check memblk overlaps, include the +* self-overlap case. +*/ if (nd->end > start && nd->start < end) - return i; + return ERR_OVERLAP; if (nd->end == end && nd->start == start) - return i; + return ERR_OVERLAP; + /* +* Use node memory range to check whether new range contains +* memory from other nodes - interleave check. We just need +* to check full contains situation. Because overlaps have +* been checked above. +*/ + if (nid != memblk_nodeid[i] && + (nd_start < nd->start && nd->end < nd_end)) + return ERR_INTERLEAVE; } - return -1; + + return NO_CONFLICT; } static __init void cutoff_node(int i, paddr_t start, paddr_t end) @@ -275,6 +304,9 @@ acpi_numa_processor_affinity_init(const struct acpi_srat_cpu_affinity *pa) void __init acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma) { + enum conflicts status; + struct node *nd; + paddr_t nd_start, nd_end; paddr_t start, end; unsigned pxm; nodeid_t node; @@ -310,42 +342,67 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma) bad_srat(); return; } + + /* +* For the node that already has some memory blocks, we will +* expand the node memory range temporarily to check memory +* interleaves with other nodes. We will not use this node +* temp memory range to check overlaps, because it will mask +* the overlaps in same node. +* +* Node with 0 bytes memory doesn't need this expandsion. +*/ + nd_start = start; + nd_end = end; + nd = [node]; + if (nd->start != nd->end) { + if (nd_start > nd->start) + nd_start = nd->start; + + if (nd_end < end) + nd_end = nd->end; + } + /* It
[PATCH v3 9/9] xen/x86: use INFO level for node's without memory log message
In previous code, Xen was using KERN_WARNING for log message when Xen found a node without memory. Xen will print this warning message, and said that this may be an BIOS Bug or mis-configured hardware. But actually, this warning is bogus, because in an NUMA setting, nodes can only have processors, and with 0 bytes memory. So it is unreasonable to warn about BIOS or hardware corruption based on the detection of node with 0 bytes memory. So in this patch, we remove the warning messages, but just keep an info message to info users that there is one or more nodes with 0 bytes memory in the system. Signed-off-by: Wei Chen --- v2 -> v3: new commit. --- xen/arch/x86/srat.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c index 53835ae3eb..acaebad2a2 100644 --- a/xen/arch/x86/srat.c +++ b/xen/arch/x86/srat.c @@ -549,8 +549,7 @@ int __init acpi_scan_nodes(paddr_t start, paddr_t end) uint64_t size = nodes[i].end - nodes[i].start; if ( size == 0 ) - printk(KERN_WARNING "SRAT: Node %u has no memory. " - "BIOS Bug or mis-configured hardware?\n", i); + printk(KERN_INFO "SRAT: Node %u has no memory.\n", i); setup_node_bootmem(i, nodes[i].start, nodes[i].end); } -- 2.25.1
[PATCH v3 6/9] xen/arm: use !CONFIG_NUMA to keep fake NUMA API
We have introduced CONFIG_NUMA in a previous patch. And this option is enabled only on x86 at the current stage. In a follow up patch, we will enable this option for Arm. But we still want users to be able to disable the CONFIG_NUMA via Kconfig. In this case, keep the fake NUMA API, will make Arm code still able to work with NUMA aware memory allocation and scheduler. Signed-off-by: Wei Chen Tested-by: Jiamei Xie Reviewed-by: Stefano Stabellini --- v2 -> v3: Add Tb. v1 -> v2: No change. --- xen/arch/arm/include/asm/numa.h | 5 + 1 file changed, 5 insertions(+) diff --git a/xen/arch/arm/include/asm/numa.h b/xen/arch/arm/include/asm/numa.h index e4c4d89192..268a9db055 100644 --- a/xen/arch/arm/include/asm/numa.h +++ b/xen/arch/arm/include/asm/numa.h @@ -5,6 +5,8 @@ typedef u8 nodeid_t; +#ifndef CONFIG_NUMA + /* Fake one node for now. See also node_online_map. */ #define cpu_to_node(cpu) 0 #define node_to_cpumask(node) (cpu_online_map) @@ -24,6 +26,9 @@ extern mfn_t first_valid_mfn; #define node_spanned_pages(nid) (max_page - mfn_x(first_valid_mfn)) #define node_start_pfn(nid) (mfn_x(first_valid_mfn)) #define __node_distance(a, b) (20) + +#endif + #define arch_want_default_dmazone() (false) #endif /* __ARCH_ARM_NUMA_H */ -- 2.25.1
[PATCH v3 5/9] xen: decouple NUMA from ACPI in Kconfig
In current Xen code only implements x86 ACPI-based NUMA support. So in Xen Kconfig system, NUMA equals to ACPI_NUMA. x86 selects NUMA by default, and CONFIG_ACPI_NUMA is hardcode in config.h. In a follow-up patch, we will introduce support for NUMA using the device tree. That means we will have two NUMA implementations, so in this patch we decouple NUMA from ACPI based NUMA in Kconfig. Make NUMA as a common feature, that device tree based NUMA also can select it. Signed-off-by: Wei Chen Tested-by: Jiamei Xie Reviewed-by: Jan Beulich --- v2 -> v3: Add Tb. v1 -> v2: No change. --- xen/arch/x86/Kconfig | 2 +- xen/arch/x86/include/asm/config.h | 1 - xen/common/Kconfig| 3 +++ xen/drivers/acpi/Kconfig | 3 ++- xen/drivers/acpi/Makefile | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index 06d6fbc864..1e31edc99f 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -6,6 +6,7 @@ config X86 def_bool y select ACPI select ACPI_LEGACY_TABLES_LOOKUP + select ACPI_NUMA select ALTERNATIVE_CALL select ARCH_SUPPORTS_INT128 select CORE_PARKING @@ -26,7 +27,6 @@ config X86 select HAS_UBSAN select HAS_VPCI if HVM select NEEDS_LIBELF - select NUMA config ARCH_DEFCONFIG string diff --git a/xen/arch/x86/include/asm/config.h b/xen/arch/x86/include/asm/config.h index de20642524..07bcd15831 100644 --- a/xen/arch/x86/include/asm/config.h +++ b/xen/arch/x86/include/asm/config.h @@ -31,7 +31,6 @@ /* Intel P4 currently has largest cache line (L2 line size is 128 bytes). */ #define CONFIG_X86_L1_CACHE_SHIFT 7 -#define CONFIG_ACPI_NUMA 1 #define CONFIG_ACPI_SRAT 1 #define CONFIG_ACPI_CSTATE 1 diff --git a/xen/common/Kconfig b/xen/common/Kconfig index d921c74d61..d65add3fc6 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -70,6 +70,9 @@ config MEM_ACCESS config NEEDS_LIBELF bool +config NUMA + bool + config STATIC_MEMORY bool "Static Allocation Support (UNSUPPORTED)" if UNSUPPORTED depends on ARM diff --git a/xen/drivers/acpi/Kconfig b/xen/drivers/acpi/Kconfig index b64d3731fb..e3f3d8f4b1 100644 --- a/xen/drivers/acpi/Kconfig +++ b/xen/drivers/acpi/Kconfig @@ -5,5 +5,6 @@ config ACPI config ACPI_LEGACY_TABLES_LOOKUP bool -config NUMA +config ACPI_NUMA bool + select NUMA diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile index 4f8e97228e..2fc5230253 100644 --- a/xen/drivers/acpi/Makefile +++ b/xen/drivers/acpi/Makefile @@ -3,7 +3,7 @@ obj-y += utilities/ obj-$(CONFIG_X86) += apei/ obj-bin-y += tables.init.o -obj-$(CONFIG_NUMA) += numa.o +obj-$(CONFIG_ACPI_NUMA) += numa.o obj-y += osl.o obj-$(CONFIG_HAS_CPUFREQ) += pmstat.o -- 2.25.1
[PATCH v3 4/9] xen: introduce an arch helper for default dma zone status
In current code, when Xen is running in a multiple nodes NUMA system, it will set dma_bitsize in end_boot_allocator to reserve some low address memory as DMA zone. There are some x86 implications in the implementation. Because on x86, memory starts from 0. On a multiple-nodes NUMA system, if a single node contains the majority or all of the DMA memory, x86 prefers to give out memory from non-local allocations rather than exhausting the DMA memory ranges. Hence x86 uses dma_bitsize to set aside some largely arbitrary amount of memory for DMA zone. The allocations from DMA zone would happen only after exhausting all other nodes' memory. But the implications are not shared across all architectures. For example, Arm cannot guarantee the availability of memory below a certain boundary for DMA limited-capability devices either. But currently, Arm doesn't need a reserved DMA zone in Xen. Because there is no DMA device in Xen. And for guests, Xen Arm only allows Dom0 to have DMA operations without IOMMU. Xen will try to allocate memory under 4GB or memory range that is limited by dma_bitsize for Dom0 in boot time. For DomU, even Xen can passthrough devices to DomU without IOMMU, but Xen Arm doesn't guarantee their DMA operations. So, Xen Arm doesn't need a reserved DMA zone to provide DMA memory for guests. In this patch, we introduce an arch_want_default_dmazone helper for different architectures to determine whether they need to set dma_bitsize for DMA zone reservation or not. At the same time, when x86 Xen is built with CONFIG_PV=n could probably leverage this new helper to actually not trigger DMA zone reservation. Signed-off-by: Wei Chen Tested-by: Jiamei Xie --- v2 -> v3: 1. Add Tb. 2. Rename arch_have_default_dmazone to arch_want_default_dmazone. v1 -> v2: 1. Extend the description of Arm's workaround for reserve DMA allocations to avoid the same discussion every time. 2. Use a macro to define arch_have_default_dmazone, because it's little hard to make x86 version to static inline. Use a macro will also avoid add __init for this function. 3. Change arch_have_default_dmazone return value from unsigned int to bool. 4. Un-addressed comment: make arch_have_default_dmazone of x86 to be static inline. Because, if we move arch_have_default_dmazone to x86/asm/numa.h, it depends on nodemask.h to provide num_online_nodes. But nodemask.h needs numa.h to provide MAX_NUMANODES. This will cause a loop dependency. And this function can only be used in end_boot_allocator, in Xen initialization. So I think, compared to the changes introduced by inline, it doesn't mean much. --- xen/arch/arm/include/asm/numa.h | 1 + xen/arch/x86/include/asm/numa.h | 1 + xen/common/page_alloc.c | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/xen/arch/arm/include/asm/numa.h b/xen/arch/arm/include/asm/numa.h index 31a6de4e23..e4c4d89192 100644 --- a/xen/arch/arm/include/asm/numa.h +++ b/xen/arch/arm/include/asm/numa.h @@ -24,6 +24,7 @@ extern mfn_t first_valid_mfn; #define node_spanned_pages(nid) (max_page - mfn_x(first_valid_mfn)) #define node_start_pfn(nid) (mfn_x(first_valid_mfn)) #define __node_distance(a, b) (20) +#define arch_want_default_dmazone() (false) #endif /* __ARCH_ARM_NUMA_H */ /* diff --git a/xen/arch/x86/include/asm/numa.h b/xen/arch/x86/include/asm/numa.h index bada2c0bb9..5d8385f2e1 100644 --- a/xen/arch/x86/include/asm/numa.h +++ b/xen/arch/x86/include/asm/numa.h @@ -74,6 +74,7 @@ static inline __attribute__((pure)) nodeid_t phys_to_nid(paddr_t addr) #define node_spanned_pages(nid)(NODE_DATA(nid)->node_spanned_pages) #define node_end_pfn(nid) (NODE_DATA(nid)->node_start_pfn + \ NODE_DATA(nid)->node_spanned_pages) +#define arch_want_default_dmazone() (num_online_nodes() > 1) extern int valid_numa_range(u64 start, u64 end, nodeid_t node); diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index 319029140f..b3bddc719b 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -1889,7 +1889,7 @@ void __init end_boot_allocator(void) } nr_bootmem_regions = 0; -if ( !dma_bitsize && (num_online_nodes() > 1) ) +if ( !dma_bitsize && arch_want_default_dmazone() ) dma_bitsize = arch_get_dma_bitsize(); printk("Domain heap initialised"); -- 2.25.1
[PATCH v3 1/9] xen/arm: Print a 64-bit number in hex from early uart
Current putn function that is using for early print only can print low 32-bit of AArch64 register. This will lose some important messages while debugging with early console. For example: (XEN) Bringing up CPU5 - CPU 00010100 booting - Will be truncated to (XEN) Bringing up CPU5 - CPU 0100 booting - In this patch, we increased the print loops and shift bits to make putn print 64-bit number. Signed-off-by: Wei Chen Tested-by: Jiamei Xie Acked-by: Julien Grall --- v2->v3: Add Tb from Jiamei. --- xen/arch/arm/arm64/head.S | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 1fd35a8390..109ae7de0c 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -872,17 +872,19 @@ puts: ret ENDPROC(puts) -/* Print a 32-bit number in hex. Specific to the PL011 UART. +/* + * Print a 64-bit number in hex. * x0: Number to print. * x23: Early UART base address - * Clobbers x0-x3 */ + * Clobbers x0-x3 + */ putn: adr x1, hex -mov x3, #8 +mov x3, #16 1: early_uart_ready x23, 2 -and x2, x0, #0xf000/* Mask off the top nybble */ -lsr x2, x2, #28 +and x2, x0, #(0xf<<60) /* Mask off the top nybble */ +lsr x2, x2, #60 ldrb w2, [x1, x2] /* Convert to a char */ early_uart_transmit x23, w2 lsl x0, x0, #4 /* Roll it through one nybble at a time */ -- 2.25.1
[PATCH v3 0/9] Device tree based NUMA support for Arm - Part#1
(The Arm device tree based NUMA support patch set contains 35 patches. In order to make stuff easier for reviewers, I split them into 3 parts: 1. Preparation. I have re-sorted the patch series. And moved independent patches to the head of the series. 2. Move generically usable code from x86 to common. 3. Add new code to support Arm. This series only contains the first part patches.) Xen memory allocation and scheduler modules are NUMA aware. But actually, on x86 has implemented the architecture APIs to support NUMA. Arm was providing a set of fake architecture APIs to make it compatible with NUMA awared memory allocation and scheduler. Arm system was working well as a single node NUMA system with these fake APIs, because we didn't have multiple nodes NUMA system on Arm. But in recent years, more and more Arm devices support multiple nodes NUMA system. So now we have a new problem. When Xen is running on these Arm devices, Xen still treat them as single node SMP systems. The NUMA affinity capability of Xen memory allocation and scheduler becomes meaningless. Because they rely on input data that does not reflect real NUMA layout. Xen still think the access time for all of the memory is the same for all CPUs. However, Xen may allocate memory to a VM from different NUMA nodes with different access speeds. This difference can be amplified in workloads inside VM, causing performance instability and timeouts. So in this patch series, we implement a set of NUMA API to use device tree to describe the NUMA layout. We reuse most of the code of x86 NUMA to create and maintain the mapping between memory and CPU, create the matrix between any two NUMA nodes. Except ACPI and some x86 specified code, we have moved other code to common. In next stage, when we implement ACPI based NUMA for Arm64, we may move the ACPI NUMA code to common too, but in current stage, we keep it as x86 only. This patch serires has been tested and booted well on one Arm64 NUMA machine and one HPE x86 NUMA machine. --- Part1 v2->v3: 1. Rework EFI stub patch: 1.1. Add existed file check, if exists a regular stub files, the common/stub files' links will be ignored. 1.2. Keep stub.c in x86/efi to include common/efi/stub.c 1.3. Restore efi_compat_xxx stub functions to x86/efi.c. Other architectures will not use efi_compat_xxx. 1.4. Remove ARM_EFI dependency from ARM_64. 1.5. Add comment for adding stub.o to EFIOBJ-y. 1.6. Merge patch#2 and patch#3 to one patch. 2. Rename arch_have_default_dmazone to arch_want_default_dmazone 3. Use uint64_t for size in acpi_scan_nodes, make it be consistent with numa_emulation. 4. Merge the interleaves checking code from a separate function to conflicting_memblks. 5. Use INFO level for node's without memory log message. 6. Move "xen/x86: Use ASSERT instead of VIRTUAL_BUG_ON for phys_to_nid" to part#2. Part1 v1->v2: 1. Move independent patches from later to early of this series. 2. Drop the copy of EFI stub.c from Arm. Share common codes of x86 EFI stub for Arm. 3. Use CONFIG_ARM_EFI to replace CONFIG_EFI and remove help text and make CONFIG_ARM_EFI invisible. 4. Use ASSERT to replace VIRTUAL_BUG_ON in phys_to_nid. 5. Move MAX_NUMNODES from xen/numa.h to asm/numa.h for x86. 6. Extend the description of Arm's workaround for reserve DMA allocations to avoid the same discussion every time for arch_have_default_dmazone. 7. Update commit messages. Wei Chen (9): xen/arm: Print a 64-bit number in hex from early uart xen: reuse x86 EFI stub functions for Arm xen/arm: Keep memory nodes in device tree when Xen boots from EFI xen: introduce an arch helper for default dma zone status xen: decouple NUMA from ACPI in Kconfig xen/arm: use !CONFIG_NUMA to keep fake NUMA API xen/x86: use paddr_t for addresses in NUMA node structure xen/x86: add detection of memory interleaves for different nodes xen/x86: use INFO level for node's without memory log message xen/arch/arm/Kconfig | 4 + xen/arch/arm/Makefile | 2 +- xen/arch/arm/arm64/head.S | 12 +-- xen/arch/arm/bootfdt.c| 8 +- xen/arch/arm/efi/Makefile | 8 ++ xen/arch/arm/efi/efi-boot.h | 25 -- xen/arch/arm/include/asm/numa.h | 6 ++ xen/arch/x86/Kconfig | 2 +- xen/arch/x86/efi/stub.c | 32 +-- xen/arch/x86/include/asm/config.h | 1 - xen/arch/x86/include/asm/numa.h | 9 +- xen/arch/x86/numa.c | 32 --- xen/arch/x86/srat.c | 137 +- xen/common/Kconfig| 3 + xen/common/efi/efi-common.mk | 3 +- xen/common/efi/stub.c | 32 +++ xen/common/page_alloc.c | 2 +- xen/drivers/acpi/Kconfig | 3 +- xen/drivers/acpi/Makefile | 2 +- 19 files changed, 193 insertions(+), 130 deletions(-) create mode 100644 xen/common/efi/stub.c -- 2.25.1
[PATCH v3 2/9] xen: reuse x86 EFI stub functions for Arm
x86 is using compiler feature testing to decide EFI build enable or not. When EFI build is disabled, x86 will use an efi/stub.c file to replace efi/runtime.c for build objects. Following this idea, we introduce a stub file for Arm, but use CONFIG_ARM_EFI to decide EFI build enable or not. And the most functions in x86 EFI stub.c can be reused for other architectures, like Arm. So we move them to common and keep the x86 specific function in x86/efi/stub.c. To avoid the symbol link conflict error when linking common stub files to x86/efi. We add a regular file check in efi stub files' link script. Depends on this check we can bypass the link behaviors for existed stub files in x86/efi. As there is no Arm specific EFI stub function for Arm in current stage, Arm still can use the existed symbol link method for EFI stub files. Signed-off-by: Wei Chen Tested-by: Jiamei Xie --- v2 -> v3: 1. Add existed file check, if a regular stub files, the common/stub files' link will be ignored. 2. Keep stub.c in x86/efi to include common/efi/stub.c 3. Restore efi_compat_xxx stub functions to x86/efi.c. Other architectures will not use efi_compat_xxx. 4. Remove ARM_EFI dependency from ARM_64. 5. Add comment for adding stub.o to EFIOBJ-y. 6. Merge patch#2 and patch#3 to one patch. v1 -> v2: 1. Drop the copy of stub.c from Arm EFI. 2. Share common codes of x86 EFI stub for other architectures. 3. Use CONFIG_ARM_EFI to replace CONFIG_EFI 4. Remove help text and make CONFIG_ARM_EFI invisible. 5. Merge one following patch: xen/arm: introduce a stub file for non-EFI architectures 6. Use the common stub.c instead of creating new one. --- xen/arch/arm/Kconfig | 4 xen/arch/arm/Makefile| 2 +- xen/arch/arm/efi/Makefile| 8 xen/arch/x86/efi/stub.c | 32 +--- xen/common/efi/efi-common.mk | 3 ++- xen/common/efi/stub.c| 32 6 files changed, 48 insertions(+), 33 deletions(-) create mode 100644 xen/common/efi/stub.c diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index ecfa6822e4..8a16d43bd5 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -6,6 +6,7 @@ config ARM_64 def_bool y depends on !ARM_32 select 64BIT + select ARM_EFI select HAS_FAST_MULTIPLY config ARM @@ -33,6 +34,9 @@ config ACPI Advanced Configuration and Power Interface (ACPI) support for Xen is an alternative to device tree on ARM64. +config ARM_EFI + bool + config GICV3 bool "GICv3 driver" depends on ARM_64 && !NEW_VGIC diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 1d862351d1..bb7a6151c1 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -1,6 +1,5 @@ obj-$(CONFIG_ARM_32) += arm32/ obj-$(CONFIG_ARM_64) += arm64/ -obj-$(CONFIG_ARM_64) += efi/ obj-$(CONFIG_ACPI) += acpi/ obj-$(CONFIG_HAS_PCI) += pci/ ifneq ($(CONFIG_NO_PLAT),y) @@ -20,6 +19,7 @@ obj-y += domain.o obj-y += domain_build.init.o obj-y += domctl.o obj-$(CONFIG_EARLY_PRINTK) += early_printk.o +obj-y += efi/ obj-y += gic.o obj-y += gic-v2.o obj-$(CONFIG_GICV3) += gic-v3.o diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile index 4313c39066..dffe72e589 100644 --- a/xen/arch/arm/efi/Makefile +++ b/xen/arch/arm/efi/Makefile @@ -1,4 +1,12 @@ include $(srctree)/common/efi/efi-common.mk +ifeq ($(CONFIG_ARM_EFI),y) obj-y += $(EFIOBJ-y) obj-$(CONFIG_ACPI) += efi-dom0.init.o +else +# Add stub.o to EFIOBJ-y to re-use the clean-files in +# efi-common.mk. Otherwise the link of stub.c in arm/efi +# will not be cleaned in "make clean". +EFIOBJ-y += stub.o +obj-y += stub.o +endif diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c index 9984932626..f2365bc041 100644 --- a/xen/arch/x86/efi/stub.c +++ b/xen/arch/x86/efi/stub.c @@ -1,7 +1,5 @@ #include -#include #include -#include #include #include #include @@ -10,6 +8,7 @@ #include #include #include +#include "../../../common/efi/stub.c" /* * Here we are in EFI stub. EFI calls are not supported due to lack @@ -45,11 +44,6 @@ void __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle, unreachable(); } -bool efi_enabled(unsigned int feature) -{ -return false; -} - void __init efi_init_memory(void) { } bool efi_boot_mem_unused(unsigned long *start, unsigned long *end) @@ -62,32 +56,8 @@ bool efi_boot_mem_unused(unsigned long *start, unsigned long *end) void efi_update_l4_pgtable(unsigned int l4idx, l4_pgentry_t l4e) { } -bool efi_rs_using_pgtables(void) -{ -return false; -} - -unsigned long efi_get_time(void) -{ -BUG(); -return 0; -} - -void efi_halt_system(void) { } -void efi_reset_system(bool warm) { } - -int efi_get_info(uint32_t idx, union xenpf_efi_info *info) -{ -return -ENOSYS; -} - int efi_compat_get_info(uint32_t idx, union compat_pf_efi_info *) __attribute__((__alias__("efi_get_info"))); -int
[PATCH v3 3/9] xen/arm: Keep memory nodes in device tree when Xen boots from EFI
In current code, when Xen is booting from EFI, it will delete all memory nodes in device tree. This would work well in current stage, because Xen can get memory map from EFI system table. However, EFI system table cannot completely replace memory nodes of device tree. EFI system table doesn't contain memory NUMA information. Xen depends on ACPI SRAT or device tree memory nodes to parse memory blocks' NUMA mapping. So in EFI + DTB boot, Xen doesn't have any method to get numa-node-id for memory blocks any more. This makes device tree based NUMA support become impossible for Xen in EFI + DTB boot. So in this patch, we will keep memory nodes in device tree for NUMA code to parse memory numa-node-id later. As a side effect, if we still parse boot memory information in early_scan_node, bootmem.info will calculate memory ranges in memory nodes twice. So we have to prevent early_scan_node to parse memory nodes in EFI boot. Signed-off-by: Wei Chen Tested-by: Jiamei Xie Reviewed-by: Stefano Stabellini --- v2 -> v3: 1. Add Rb. v1 -> v2: 1. Move this patch from later to early of this series. 2. Refine commit message. --- xen/arch/arm/bootfdt.c | 8 +++- xen/arch/arm/efi/efi-boot.h | 25 - 2 files changed, 7 insertions(+), 26 deletions(-) diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index 29671c8df0..ec81a45de9 100644 --- a/xen/arch/arm/bootfdt.c +++ b/xen/arch/arm/bootfdt.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -367,7 +368,12 @@ static int __init early_scan_node(const void *fdt, { int rc = 0; -if ( device_tree_node_matches(fdt, node, "memory") ) +/* + * If Xen has been booted via UEFI, the memory banks are + * populated. So we should skip the parsing. + */ +if ( !efi_enabled(EFI_BOOT) && + device_tree_node_matches(fdt, node, "memory") ) rc = process_memory_node(fdt, node, name, depth, address_cells, size_cells, ); else if ( depth == 1 && !dt_node_cmp(name, "reserved-memory") ) diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h index e452b687d8..59d93c24a1 100644 --- a/xen/arch/arm/efi/efi-boot.h +++ b/xen/arch/arm/efi/efi-boot.h @@ -231,33 +231,8 @@ EFI_STATUS __init fdt_add_uefi_nodes(EFI_SYSTEM_TABLE *sys_table, int status; u32 fdt_val32; u64 fdt_val64; -int prev; int num_rsv; -/* - * Delete any memory nodes present. The EFI memory map is the only - * memory description provided to Xen. - */ -prev = 0; -for (;;) -{ -const char *type; -int len; - -node = fdt_next_node(fdt, prev, NULL); -if ( node < 0 ) -break; - -type = fdt_getprop(fdt, node, "device_type", ); -if ( type && strncmp(type, "memory", len) == 0 ) -{ -fdt_del_node(fdt, node); -continue; -} - -prev = node; -} - /* * Delete all memory reserve map entries. When booting via UEFI, * kernel will use the UEFI memory map to find reserved regions. -- 2.25.1
[ovmf test] 170311: regressions - FAIL
flight 170311 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170311/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 923 attempts Testing same since 170272 2022-05-09 15:12:57 Z1 days 27 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
[linux-linus test] 170306: tolerable FAIL - PUSHED
flight 170306 linux-linus real [real] flight 170312 linux-linus real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/170306/ http://logs.test-lab.xenproject.org/osstest/logs/170312/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-multivcpu 20 guest-localmigrate/x10 fail pass in 170312-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 170273 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 170273 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 170273 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 170273 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 170273 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 170273 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 170273 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 170273 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 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-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-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-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 15 migrate-support-checkfail never pass test-armhf-armhf-xl 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-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-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 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: linuxfeb9c5e19e913b53cb536a7aa7c9f20107bb51ec baseline version: linux9be9ed2612b5aedb52a2c240edb1630b6b743cb6 Last test of basis 170273 2022-05-09 16:11:07 Z1 days Testing same since 170306 2022-05-10 18:39:54 Z0 days1 attempts People who touched revisions under test: Linus Torvalds Michael S. Tsirkin Shunsuke Mie jobs: build-amd64-xsm pass
[ovmf test] 170310: regressions - FAIL
flight 170310 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170310/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 922 attempts Testing same since 170272 2022-05-09 15:12:57 Z1 days 26 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
[ovmf test] 170309: regressions - FAIL
flight 170309 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170309/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 921 attempts Testing same since 170272 2022-05-09 15:12:57 Z1 days 25 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
Re: [PATCH 01/30] x86/crash,reboot: Avoid re-disabling VMX in all CPUs on crash/restart
On 09/05/2022 12:52, Sean Christopherson wrote: > I find the shortlog to be very confusing, the bug has nothing to do with > disabling > VMX and I distinctly remember wrapping VMXOFF with exception fixup to prevent > doom > if VMX is already disabled :-). The issue is really that > nmi_shootdown_cpus() doesn't > play nice with being called twice. > Hey Sean, OK - I agree with you, the issue is really about the double list addition. > [...] > > Call stacks for the two callers would be very, very helpful. > [...] > This feels like were just adding more duct tape to the mess. nmi_shootdown() > is > still unsafe for more than one caller, and it takes a _lot_ of staring and > searching > to understand that crash_smp_send_stop() is invoked iff CONFIG_KEXEC_CORE=y, > i.e. > that it will call smp_ops.crash_stop_other_cpus() and not just > smp_send_stop(). > > Rather than shared a flag between two relatively unrelated functions, what if > we > instead disabling virtualization in crash_nmi_callback() and then turn the > reboot > call into a nop if an NMI shootdown has already occurred? That will also add > a > bit of documentation about multiple shootdowns not working. > > And I believe there's also a lurking bug in > native_machine_emergency_restart() that > can be fixed with cleanup. SVM can also block INIT and so should be disabled > during > an emergency reboot. > > The attached patches are compile tested only. If they seem sane, I'll post an > official mini series. Thanks Sean, it makes sense - my patch is more a "band-aid" whereas yours fixes it in a more generic way. Confess I found the logic of your patch complex, but as you said, it requires a *lot* of code analysis to understand these multiple shutdown patches, the problem is complicated by nature heh I've tested your patch 0001 and it works well for all cases [0], so go ahead and submit the miniseries, feel free to add: Reported-and-tested-by: Guilherme G. Piccoli I've read patch 0002 and it makes sense to me as well, a good proactive bug fix =) With that said, I'll of course drop this one from V2 of this series. Cheers, Guilherme [0] A summary of my tests and the code paths that the panic shutdown take depending on some conditions: New function that disables VMX/SVM: cpu_crash_disable_virtualization() [should be executed in every online CPU on shutdown) The panic path triggers the following call stacks depending on kdump and post_notifiers: (1) kexec/kdump + !crash_kexec_post_notifiers ->machine_crash_shutdown() .crash_shutdown() --native_machine_crash_shutdown() [all custom handlers except Xen PV call the native generic function] crash_smp_send_stop() --kdump_nmi_shootdown_cpus() nmi_shootdown_cpus(kdump_nmi_callback) --crash_nmi_callback() kdump_nmi_callback() --cpu_crash_disable_virtualization() (2) kexec/kdump + crash_kexec_post_notifiers ->crash_smp_send_stop() kdump_nmi_shootdown_cpus() --nmi_shootdown_cpus(kdump_nmi_callback) crash_nmi_callback() --kdump_nmi_callback() cpu_crash_disable_virtualization() After this path, will execute machine_crash_shutdown() but crash_smp_send_stop() is guarded against double execution. Also, emergency restart calls emergency_vmx_disable_all() . (3) !kexec/kdump + crash_kexec_post_notifiers Same as (2) (4) !kexec/kdump + !crash_kexec_post_notifiers -> smp_send_stop() native_stop_other_cpus() --apic_send_IPI_allbutself(REBOOT_VECTOR) sysvec_reboot --cpu_emergency_vmxoff() If not: --register_stop_handler() apic_send_IPI_allbutself(NMI_VECTOR) --smp_stop_nmi_callback() cpu_emergency_vmxoff() After that, emergency_vmx_disable_all() gets called in the emergency restart path as well.
[ovmf test] 170308: regressions - FAIL
flight 170308 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170308/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 920 attempts Testing same since 170272 2022-05-09 15:12:57 Z1 days 24 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
[ovmf test] 170305: regressions - FAIL
flight 170305 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170305/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 919 attempts Testing same since 170272 2022-05-09 15:12:57 Z1 days 23 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
[xen-unstable-smoke test] 170300: tolerable all pass - PUSHED
flight 170300 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/170300/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-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-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 407b13a71e324aba76b11e5f66f59ce4a304a088 baseline version: xen 0badfb110fa33ca9ffd3bdc3a5200cded03e6106 Last test of basis 170280 2022-05-09 23:01:42 Z0 days Testing same since 170300 2022-05-10 15:03:18 Z0 days1 attempts People who touched revisions under test: Alex Bennée Julien Grall Luca Fancellu jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt 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 Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 0badfb110f..407b13a71e 407b13a71e324aba76b11e5f66f59ce4a304a088 -> smoke
Re: [PATCH v8 01/27] notifier: Add atomic_notifier_call_chain_is_empty()
On Tue, May 10, 2022 at 1:33 AM Dmitry Osipenko wrote: > > Add atomic_notifier_call_chain_is_empty() that returns true if given > atomic call chain is empty. It would be good to mention a use case for it. > Reviewed-by: Michał Mirosław > Signed-off-by: Dmitry Osipenko > --- > include/linux/notifier.h | 2 ++ > kernel/notifier.c| 13 + > 2 files changed, 15 insertions(+) > > diff --git a/include/linux/notifier.h b/include/linux/notifier.h > index 87069b8459af..95e2440037de 100644 > --- a/include/linux/notifier.h > +++ b/include/linux/notifier.h > @@ -173,6 +173,8 @@ extern int blocking_notifier_call_chain_robust(struct > blocking_notifier_head *nh > extern int raw_notifier_call_chain_robust(struct raw_notifier_head *nh, > unsigned long val_up, unsigned long val_down, void *v); > > +extern bool atomic_notifier_call_chain_is_empty(struct atomic_notifier_head > *nh); > + > #define NOTIFY_DONE0x /* Don't care */ > #define NOTIFY_OK 0x0001 /* Suits me */ > #define NOTIFY_STOP_MASK 0x8000 /* Don't call further */ > diff --git a/kernel/notifier.c b/kernel/notifier.c > index ba005ebf4730..aaf5b56452a6 100644 > --- a/kernel/notifier.c > +++ b/kernel/notifier.c > @@ -204,6 +204,19 @@ int atomic_notifier_call_chain(struct > atomic_notifier_head *nh, > EXPORT_SYMBOL_GPL(atomic_notifier_call_chain); > NOKPROBE_SYMBOL(atomic_notifier_call_chain); > > +/** > + * atomicnotifier_call_chain_is_empty - Check whether notifier chain is > empty > + * @nh: Pointer to head of the blocking notifier chain > + * > + * Checks whether notifier chain is empty. > + * > + * Returns true is notifier chain is empty, false otherwise. > + */ > +bool atomic_notifier_call_chain_is_empty(struct atomic_notifier_head *nh) > +{ > + return !rcu_access_pointer(nh->head); > +} > + > /* > * Blocking notifier chain routines. All access to the chain is > * synchronized by an rwsem. > -- > 2.35.1 >
[ovmf test] 170304: regressions - FAIL
flight 170304 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170304/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 918 attempts Testing same since 170272 2022-05-09 15:12:57 Z1 days 22 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
Re: [PATCH 23/30] printk: kmsg_dump: Introduce helper to inform number of dumpers
On Wed, 27 Apr 2022 19:49:17 -0300 "Guilherme G. Piccoli" wrote: > Currently we don't have a way to check if there are dumpers set, > except counting the list members maybe. This patch introduces a very > simple helper to provide this information, by just keeping track of > registered/unregistered kmsg dumpers. It's going to be used on the > panic path in the subsequent patch. FYI, it is considered "bad form" to reference in the change log "this patch". We know this is a patch. The change log should just talk about what is being done. So can you reword your change logs (you do this is almost every patch). Here's what I would reword the above to be: Currently we don't have a way to check if there are dumpers set, except perhaps by counting the list members. Introduce a very simple helper to provide this information, by just keeping track of registered/unregistered kmsg dumpers. This will simplify the refactoring of the panic path. -- Steve
Re: [PATCH 18/30] notifier: Show function names on notifier routines if DEBUG_NOTIFIERS is set
On Thu, 28 Apr 2022 09:01:13 +0800 Xiaoming Ni wrote: > > +#ifdef CONFIG_DEBUG_NOTIFIERS > > + { > > + char sym_name[KSYM_NAME_LEN]; > > + > > + pr_info("notifiers: registered %s()\n", > > + notifier_name(n, sym_name)); > > + } > > Duplicate Code. > > Is it better to use __func__ and %pS? > > pr_info("%s: %pS\n", __func__, n->notifier_call); > > > > +#endif Also, don't sprinkle #ifdef in C code. Instead: if (IS_ENABLED(CONFIG_DEBUG_NOTIFIERS)) pr_info("notifers: regsiter %ps()\n", n->notifer_call); Or define a print macro at the start of the C file that is a nop if it is not defined, and use the macro. -- Steve
Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path
On Tue, 10 May 2022 13:38:39 +0200 Petr Mladek wrote: > As already mentioned in the other reply, panic() sometimes stops > the other CPUs using NMI, for example, see kdump_nmi_shootdown_cpus(). > > Another situation is when the CPU using the lock ends in some > infinite loop because something went wrong. The system is in > an unpredictable state during panic(). > > I am not sure if this is possible with the code under gsmi_dev.lock > but such things really happen during panic() in other subsystems. > Using trylock in the panic() code path is a good practice. I believe that Peter Zijlstra had a special spin lock for NMIs or early printk, where it would not block if the lock was held on the same CPU. That is, if an NMI happened and paniced while this lock was held on the same CPU, it would not deadlock. But it would block if the lock was held on another CPU. -- Steve
[ovmf test] 170303: regressions - FAIL
flight 170303 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170303/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 917 attempts Testing same since 170272 2022-05-09 15:12:57 Z1 days 21 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
[xen-unstable test] 170293: tolerable FAIL - PUSHED
flight 170293 xen-unstable real [real] flight 170302 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/170293/ http://logs.test-lab.xenproject.org/osstest/logs/170302/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-arm64-arm64-xl-seattle 8 xen-bootfail pass in 170302-retest Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-seattle 15 migrate-support-check fail in 170302 never pass test-arm64-arm64-xl-seattle 16 saverestore-support-check fail in 170302 never pass test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 170281 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 170281 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 170281 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 170281 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 170281 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 170281 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 170281 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 170281 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 170281 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 170281 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 170281 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 170281 test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-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-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 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-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-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-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-libvirt-qcow2 14 migrate-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-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 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 v6 4/7] xen/arm: configure dom0less domain for enabling xenstore after boot
Hi Rahul, On 10/05/2022 17:30, Rahul Singh wrote: +rc = evtchn_alloc_unbound(); +if ( rc ) +{ +printk("Failed allocating event channel for domain\n"); +return rc; +} + +d->arch.hvm.params[HVM_PARAM_STORE_EVTCHN] = alloc.port; + +return 0; +} + static int __init construct_domU(struct domain *d, const struct dt_device_node *node) { @@ -3214,6 +3243,14 @@ static int __init construct_domU(struct domain *d, if ( rc < 0 ) return rc; +if ( kinfo.dom0less_enhanced ) I think we need to do something like this to fix the error. if ( hardware_domain && kinfo.dom0less_enhanced ) { } Is there any use case to use "dom0less_enhanced" without dom0 (or a domain servicing Xenstored)? If not, then I would consider to forbid this case and return an error. Cheers, -- Julien Grall
Re: [PATCH v6 4/7] xen/arm: configure dom0less domain for enabling xenstore after boot
Hi Stefano, > On 5 May 2022, at 1:16 am, Stefano Stabellini wrote: > > From: Luca Miccio > > Export evtchn_alloc_unbound and make it __must_check. > > If "xen,enhanced" is enabled, then add to dom0less domains: > > - the hypervisor node in device tree > - the xenstore event channel > > The xenstore event channel is also used for the first notification to > let the guest know that xenstore has become available. > > Signed-off-by: Luca Miccio > Signed-off-by: Stefano Stabellini > Reviewed-by: Bertrand Marquis > CC: Julien Grall > CC: Volodymyr Babchuk > CC: Bertrand Marquis > CC: jbeul...@suse.com > > --- > Changes in v5: > - merge with "xen: export evtchn_alloc_unbound" > - __must_check > > Changes in v3: > - use evtchn_alloc_unbound > > Changes in v2: > - set HVM_PARAM_STORE_PFN to ~0ULL at domain creation > - in alloc_xenstore_evtchn do not call _evtchn_alloc_unbound > > xen: export evtchn_alloc_unbound > > It will be used during dom0less domains construction. > > Signed-off-by: Stefano Stabellini > --- > xen/arch/arm/domain_build.c | 37 + > xen/common/event_channel.c | 2 +- > xen/include/xen/event.h | 3 +++ > 3 files changed, 41 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 016f56a99f..bb430f2189 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -27,6 +27,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -2810,6 +2811,8 @@ static int __init prepare_dtb_domU(struct domain *d, > struct kernel_info *kinfo) > int ret; > > kinfo->phandle_gic = GUEST_PHANDLE_GIC; > +kinfo->gnttab_start = GUEST_GNTTAB_BASE; > +kinfo->gnttab_size = GUEST_GNTTAB_SIZE; > > addrcells = GUEST_ROOT_ADDRESS_CELLS; > sizecells = GUEST_ROOT_SIZE_CELLS; > @@ -2884,6 +2887,13 @@ static int __init prepare_dtb_domU(struct domain *d, > struct kernel_info *kinfo) > goto err; > } > > +if ( kinfo->dom0less_enhanced ) > +{ > +ret = make_hypervisor_node(d, kinfo, addrcells, sizecells); > +if ( ret ) > +goto err; > +} > + > ret = fdt_end_node(kinfo->fdt); > if ( ret < 0 ) > goto err; > @@ -3150,6 +3160,25 @@ static int __init construct_domain(struct domain *d, > struct kernel_info *kinfo) > return 0; > } > > +static int __init alloc_xenstore_evtchn(struct domain *d) > +{ > +evtchn_alloc_unbound_t alloc; > +int rc; > + > +alloc.dom = d->domain_id; > +alloc.remote_dom = hardware_domain->domain_id; I tried to test the patch series with two dom0less domUs without dom0 and oberved the below error. This error is because there is no hardware_domain in that case. (XEN) Data Abort Trap. Syndrome=0x6 (XEN) Walking Hypervisor VA 0x0 on CPU0 via TTBR 0xf91f5000 (XEN) 0TH[0x0] = 0xf91f4f7f (XEN) 1ST[0x0] = 0xf91f1f7f (XEN) 2ND[0x0] = 0x (XEN) CPU0: Unexpected Trap: Data Abort (XEN) [ Xen-4.17-unstable arm64 debug=y Not tainted ] (XEN) CPU:0 (XEN) PC: 002e4180 domain_build.c#construct_domU+0xc90/0xd0c (XEN) LR: 002e4178 (XEN) SP: 0031e450 (XEN) CPSR: 6249 MODE:64-bit EL2h (Hypervisor, handler) (XEN) X0: X1: X2: (XEN) X3: 0005 X4: X5: 0028 (XEN) X6: 0080 X7: fefefefefefeff09 X8: 7f7f7f7f7f7f7f7f (XEN) X9: ff6f606c2c68726c X10: 7f7f7f7f7f7f7f7f X11: 0101010101010101 (XEN) X12: 0008 X13: 002ca000 X14: 002cb000 (XEN) X15: 6db6db6db6db6db7 X16: fff8 X17: 0001 (XEN) X18: 0180 X19: 800763a34000 X20: (XEN) X21: 0031e4c0 X22: 0031e4d0 X23: 0003 (XEN) X24: 0031fdfc X25: 0084 X26: 0021 (XEN) X27: 00300d08 X28: 003fe97f FP: 0031e450 (XEN) (XEN) VTCR_EL2: 800d3590 (XEN) VTTBR_EL2: 0083e3a67000 (XEN) (XEN) SCTLR_EL2: 30cd183d (XEN)HCR_EL2: 8039 (XEN) TTBR0_EL2: f91f5000 (XEN) (XEN)ESR_EL2: 9606 (XEN) HPFAR_EL2: (XEN)FAR_EL2: (XEN) (XEN) Xen stack trace from sp=0031e450: (XEN)0031fda0 002e5484 800763ff2390 002f1ae0 (XEN)002c98c8 0031fde0 0003 0031fdfc (XEN)0084 0021 00300d08 003fe97f (XEN)00c20100 0031e4e0 040f (XEN)00220001 0010 0203 0001 (XEN)000c 0001 800763a34000 (XEN)800763a0c000 0001 a000 (XEN)
Re: Proposal: use disk sequence numbers to avoid races in blkback
On Tue, May 10, 2022 at 12:57:48PM +0200, Roger Pau Monné wrote: > On Thu, May 05, 2022 at 08:30:17PM -0400, Demi Marie Obenour wrote: > > Proposal: Check disk sequence numbers in blkback > > > > > > Currently, adding block devices to a domain is racy. libxl writes the > > major and minor number of the device to XenStore, but it does not keep > > the block device open until blkback has opened it. This creates a race > > condition, as it is possible for the device to be destroyed and another > > device allocated with the same major and minor numbers. Loop devices > > are the most obvious example, since /dev/loop0 can be reused again and > > again, but the same problem can also happen with device-mapper devices. > > If the major and minor numbers are reused before blkback has attached to > > the device, blkback will pass the wrong device to the domain, with > > obvious security consequences. > > > > Other programs on Linux have the same problem, and a solution was > > committed upstream in the form of disk sequence numbers. A disk > > sequence number, or diskseq, is a 64-bit unsigned monotonically > > increasing counter. The combination of a major and minor number and a > > disk sequence number uniquely identifies a block device for the entire > > uptime of the system. > > Seems fine to me, this is just an extra check to make sure the block > device opened by blkback is the one that user space intended. I would > see diskseq as a kind of checksum. Ideally, diskseq would be the primary means of identifying a device, but that isn’t an option without more substantial changes, sadly. > > I propose that blkback check for an unsigned 64-bit hexadecimal XenStore > > entry named “diskseq”. If the entry exists, blkback checks that the > > number stored there matches the disk sequence number of the device. If > > it does not exist, the check is skipped. If reading the entry fails for > > any other reason, the entry is malformed, or if the sequence number is > > wrong, blkback refuses to export the device. > > > > The toolstack changes are more involved for two reasons: > > > > 1. To ensure that loop devices are not leaked if the toolstack crashes, > >they must be created with the delete-on-close flag set. This > >requires that the toolstack hold the device open until blkback has > >acquired a handle to it. > > Does this work with loop devices? I would expect that you need to > issue a losetup call to detach the device. That is what the autoclear flag is for. It will cause the device to be destroyed by the kernel as soon as the last handle to it has been closed. This is why the toolstack needs to hold a file descriptor to the device. > Even more, the loop device is created by the block script, but there's > also a window between the block script execution and the toolstack > knowing about the device, which could also allow for a leak? For this to work, either the toolstack or block script will need to open the file and perform loop(4) ioctls to assign the file descriptor to a loop device. This cannot be done by a shell script, so I plan on using a C program to perform these tasks. In Qubes OS, I expect this program to replace the block script entirely, as performance is critical and flexibility less so. For upstream, I recommend having the block script be a script that calls this C program. > > 2. For block devices that are opened by path, the toolstack needs to > >ensure that the device it has opened is actually the device it > >intended to open. This requires device-specific verification of the > >open file descriptor. This is not needed for regular files, as the > >LOOP_CONFIGURE ioctl is called on an existing loop device and sets > >its backing file. > > > > The first is fairly easy in C. It can be accomplished by means of a > > XenStore watch on the “status” entry. Once that watch fires, blkback > > has opened the device, so the toolstack can safely close its file > > descriptor. > > Does the toolstack really need to close the device? What harm does it > do to keep the handle open until the domain is destroyed? This would cause no harm, but it also would not help either, so I do not see any advantages to doing it. > What about disk hotplug? Which entity will keep the device opened in > that case? Is xl block-attach going to block until the device > switches to the connected state? Whichever program opens the file will need to do this. This could be the program that is using libxl or the block script that libxl invokes. I am not familiar with xl block-attach as Qubes OS uses a custom wrapper around libvirt. > > The second is significantly more difficult. It requires the block > > script to be aware of at least device-mapper devices and LVM2 logical > > volumes. The general technique is common to all block devices: obtain > > the sequence number (via the BLKGETDISKSEQ() ioctl) and its major
Re: [PATCH v3 1/6] xen: implement byteswap
On 10/05/2022 13:10, Lin Liu (刘林) wrote: On 10/05/2022 11:15, Lin Liu wrote: swab() is massively over complicated and can be simplified by builtins. NIT: "by builtins" -> "by re-implementing using compiler builtins". The compilers provide builtin function to swap bytes. * gcc: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fonlinedocs%2Fgcc%2FOther-Builtins.htmldata=05%7C01%7Clin.liu%40citrix.com%7Ce0b3d98d7f8d47b8fe8708da3275afcd%7C335836de42ef43a2b145348c2ee9ca5b%7C0%7C0%7C63788294067911%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=HDTF1LDJcD2PLSCuM%2FjIz%2FWGf1CrYk0e%2FLox22%2FXnvQ%3Dreserved=0 * clang: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fclang.llvm.org%2Fdocs%2FLanguageExtensions.htmldata=05%7C01%7Clin.liu%40citrix.com%7Ce0b3d98d7f8d47b8fe8708da3275afcd%7C335836de42ef43a2b145348c2ee9ca5b%7C0%7C0%7C63788294067911%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=EvWcLMi2vtT9haQVo%2F9uXmjBh2zVLUzZAgU57i%2FFMNo%3Dreserved=0 This patch simplify swab() with builtins and fallback for old compilers. Signed-off-by: Lin Liu --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Bertrand Marquis Cc: Volodymyr Babchuk Cc: Andrew Cooper Cc: George Dunlap Cc: Jan Beulich Cc: Wei Liu Cc: "Roger Pau Monné" Changes in v3: - Check __has_builtin instead of GNUC version Changes in v2: - Add fallback for compilers without __builtin_bswap - Implement with plain C instead of macros --- xen/arch/arm/include/asm/byteorder.h | 14 ++- xen/arch/x86/include/asm/byteorder.h | 34 ++--- xen/include/xen/byteorder.h | 56 xen/include/xen/byteswap.h | 44 ++ xen/include/xen/compiler.h | 12 ++ 5 files changed, 120 insertions(+), 40 deletions(-) create mode 100644 xen/include/xen/byteorder.h create mode 100644 xen/include/xen/byteswap.h diff --git a/xen/arch/arm/include/asm/byteorder.h b/xen/arch/arm/include/asm/byteorder.h index 9c712c4788..622eeaba07 100644 --- a/xen/arch/arm/include/asm/byteorder.h +++ b/xen/arch/arm/include/asm/byteorder.h @@ -1,16 +1,10 @@ #ifndef __ASM_ARM_BYTEORDER_H__ #define __ASM_ARM_BYTEORDER_H__ -#define __BYTEORDER_HAS_U64__ +#ifndef __BYTE_ORDER__ + #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ +#endif -#include +#include #endif /* __ASM_ARM_BYTEORDER_H__ */ -/* - * Local variables: - * mode: C - * c-file-style: "BSD" - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ This change looks unrelated to this patch. Can you explain it? Do you mean following code block? Yes, it is unrelated, I removed it as I found some files does not include such field. So in general we try to avoid unrelated change within a same patch. In this case, the emacs magic should be present in the files rather than the other way around. Cheers, -- Julien Grall
Re: [PATCH 14/30] panic: Properly identify the panic event to the notifiers' callbacks
On 10/05/2022 12:16, Petr Mladek wrote: > [...] > Hmm, this looks like a hack. PANIC_UNUSED will never be used. > All notifiers will be always called with PANIC_NOTIFIER. > > The @val parameter is normally used when the same notifier_list > is used in different situations. > > But you are going to use it when the same notifier is used > in more lists. This is normally distinguished by the @nh > (atomic_notifier_head) parameter. > > IMHO, it is a bad idea. First, it would confuse people because > it does not follow the original design of the parameters. > Second, the related code must be touched anyway when > the notifier is moved into another list so it does not > help much. > > Or do I miss anything, please? > > Best Regards, > Petr Hi Petr, thanks for the review. I'm not strong attached to this patch, so we could drop it and refactor the code of next patches to use the @nh as identification - but personally, I feel this parameter could be used to identify the list that called such function, in other words, what is the event that triggered the callback. Some notifiers are even declared with this parameter called "ev", like the event that triggers the notifier. You mentioned 2 cases: (a) Same notifier_list used in different situations; (b) Same *notifier callback* used in different lists; Mine is case (b), right? Can you show me an example of case (a)? You can see in the following patches (or grep the kernel) that people are using this identification parameter to determine which kind of OOPS trigger the callback to condition the execution of the function to specific cases. IIUIC, this is more or less what I'm doing, but extending the idea for panic notifiers. Again, as a personal preference, it makes sense to me using id's VS comparing pointers to differentiate events/callers. Cheers, Guilherme
[ovmf test] 170301: regressions - FAIL
flight 170301 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170301/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 916 attempts Testing same since 170272 2022-05-09 15:12:57 Z1 days 20 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
[PATCH v3 1/2] ns16550: reject IRQ above nr_irqs_gsi
Intel LPSS has INTERRUPT_LINE set to 0xff by default, that is declared by the PCI Local Bus Specification Revision 3.0 (from 2004) as "unknown"/"no connection". Fallback to poll mode in this case. Signed-off-by: Marek Marczykowski-Górecki --- Changes in v3: - change back to checking 0xff explicitly - adjust commit message, include spec reference - change warning to match the above Changes in v2: - add log message - extend commit message - code style fix --- xen/drivers/char/ns16550.c | 9 + 1 file changed, 9 insertions(+) diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index fb75cee4a13a..b4434ad815e1 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -1238,6 +1238,15 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx) pci_conf_read8(PCI_SBDF(0, b, d, f), PCI_INTERRUPT_LINE) : 0; +if ( uart->irq == 0xff ) +{ +printk(XENLOG_WARNING + "ns16550: %02x:%02x.%u has no legacy IRQ %d, " + "falling back to a poll mode\n", + b, d, f, uart->irq); +uart->irq = 0; +} + return 0; } } -- 2.35.1
[PATCH v3 2/2] ns16550: Add more device IDs for Intel LPSS UART
This is purely based on the spec: - Intel 500 Series PCH: 635218-006 - Intel 600 Series PCH: 691222-001, 648364-003 This is tested only on TGL-LP added initially, but according to the spec, they should behave the same. Signed-off-by: Marek Marczykowski-Górecki Acked-by: Andrew Cooper --- Changes in v2: - new patch, adding more IDs to the patch that went in already --- xen/drivers/char/ns16550.c | 80 +- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index b4434ad815e1..72283c106514 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -1077,12 +1077,90 @@ static const struct ns16550_config __initconst uart_config[] = .dev_id = 0x0358, .param = param_exar_xr17v358 }, -/* Intel Corp. TGL-LP LPSS PCI */ +/* Intel Corp. TGL-LP LPSS PCI UART #0 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0xa0a8, +.param = param_intel_lpss +}, +/* Intel Corp. TGL-LP LPSS PCI UART #1 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0xa0a9, +.param = param_intel_lpss +}, +/* Intel Corp. TGL-LP LPSS PCI UART #2 */ { .vendor_id = PCI_VENDOR_ID_INTEL, .dev_id = 0xa0c7, .param = param_intel_lpss }, +/* Intel Corp. TGL-H LPSS PCI UART #0 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x43a8, +.param = param_intel_lpss +}, +/* Intel Corp. TGL-H LPSS PCI UART #1 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x43a9, +.param = param_intel_lpss +}, +/* Intel Corp. TGL-H LPSS PCI UART #2 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x43a7, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-P LPSS PCI UART #0 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x51a8, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-P LPSS PCI UART #1 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x51a9, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-P LPSS PCI UART #2 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x51c7, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-P LPSS PCI UART #3 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x51da, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-S LPSS PCI UART #0 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x7aa8, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-S LPSS PCI UART #1 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x7aa9, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-S LPSS PCI UART #2 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x7afe, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-S LPSS PCI UART #3 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x7adc, +.param = param_intel_lpss +}, }; static int __init -- 2.35.1
Re: [PATCH v2 1/2] ns16550: reject IRQ above nr_irqs_gsi
On Tue, May 10, 2022 at 05:46:12PM +0200, Roger Pau Monné wrote: > On Tue, May 10, 2022 at 02:30:41PM +, Andrew Cooper wrote: > > On 10/05/2022 12:55, Marek Marczykowski-Górecki wrote: > > > Intel LPSS has INTERRUPT_LINE set to 0xff by default, that can't > > > possibly work. While a proper IRQ configuration may be useful, > > > validating value retrieved from the hardware is still necessary. If it > > > fails, use the device in poll mode, instead of crashing down the line > > > (at smp_initr_init()). Currently it's > > > x86-specific, as the surrounding code is guarded with CONFIG_X86 anyway. > > > > > > Signed-off-by: Marek Marczykowski-Górecki > > > > > > > This isn't invalid per say. It explicitly means unknown/no connection > > and is used in practice to mean "never generate line interrupts, even > > when MSI is disabled". It's part of PCI 3.0 if the internet is to be > > believed, but ISTR is mandatory for SRIOV devices where the use of line > > interrupts is prohibited by the spec. > > > > Also, there are systems where nr_irq_gsi is greater than 0xff. > > > > I'd recommend handling 0xff specially as "no legacy irq", and not > > involving nr_irq_gsi at all. > > I've finally found the reference for it in (one) PCI specification. > It's in the PCI Local Bus Specification Revision 3.0 (from 2004) as a > footnote, so for the reference I'm going to paste it here: > > Interrupt Line > > The Interrupt Line register is an eight-bit register used to > communicate interrupt line routing information. The register is > read/write and must be implemented by any device (or device function) > that uses an interrupt pin. POST software will write the routing > information into this register as it initializes and configures the > system. The value in this register tells which input of the system > interrupt controller(s) the device's interrupt pin is connected to. > The device itself does not use this value, rather it is used by device > drivers and operating systems. Device drivers and operating systems > can use this information to determine priority and vector information. > Values in this register are system architecture specific. [43] > > [...] > > [43] For x86 based PCs, the values in this register correspond to IRQ > numbers (0-15) of the standard dual 8259 configuration. The value 255 > is defined as meaning "unknown" or "no connection" to the interrupt > controller. Values between 15 and 254 are reserved. > > That note however is completely missed on the newer PCI Express > specifications. > > Marek, can you please adjust the code as suggested by Andrew and add > the reference to the commit message? Sure. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab signature.asc Description: PGP signature
Re: [PATCH v2 1/2] ns16550: reject IRQ above nr_irqs_gsi
On Tue, May 10, 2022 at 02:30:41PM +, Andrew Cooper wrote: > On 10/05/2022 12:55, Marek Marczykowski-Górecki wrote: > > Intel LPSS has INTERRUPT_LINE set to 0xff by default, that can't > > possibly work. While a proper IRQ configuration may be useful, > > validating value retrieved from the hardware is still necessary. If it > > fails, use the device in poll mode, instead of crashing down the line > > (at smp_initr_init()). Currently it's > > x86-specific, as the surrounding code is guarded with CONFIG_X86 anyway. > > > > Signed-off-by: Marek Marczykowski-Górecki > > This isn't invalid per say. It explicitly means unknown/no connection > and is used in practice to mean "never generate line interrupts, even > when MSI is disabled". It's part of PCI 3.0 if the internet is to be > believed, but ISTR is mandatory for SRIOV devices where the use of line > interrupts is prohibited by the spec. > > Also, there are systems where nr_irq_gsi is greater than 0xff. > > I'd recommend handling 0xff specially as "no legacy irq", and not > involving nr_irq_gsi at all. I've finally found the reference for it in (one) PCI specification. It's in the PCI Local Bus Specification Revision 3.0 (from 2004) as a footnote, so for the reference I'm going to paste it here: Interrupt Line The Interrupt Line register is an eight-bit register used to communicate interrupt line routing information. The register is read/write and must be implemented by any device (or device function) that uses an interrupt pin. POST software will write the routing information into this register as it initializes and configures the system. The value in this register tells which input of the system interrupt controller(s) the device's interrupt pin is connected to. The device itself does not use this value, rather it is used by device drivers and operating systems. Device drivers and operating systems can use this information to determine priority and vector information. Values in this register are system architecture specific. [43] [...] [43] For x86 based PCs, the values in this register correspond to IRQ numbers (0-15) of the standard dual 8259 configuration. The value 255 is defined as meaning "unknown" or "no connection" to the interrupt controller. Values between 15 and 254 are reserved. That note however is completely missed on the newer PCI Express specifications. Marek, can you please adjust the code as suggested by Andrew and add the reference to the commit message? Thanks, Roger.
Re: [PATCH v2] xen/arm: p2m don't fall over on FEAT_LPA enabled hw
Julien Grall writes: > Hi Alex, > > On 10/05/2022 15:47, Alex Bennée wrote: >> Julien Grall writes: >>> On 10/05/2022 15:03, Alex Bennée wrote: Julien Grall writes: > Hi Alex, > > On 28/04/2022 11:34, Alex Bennée wrote: >> When we introduced FEAT_LPA to QEMU's -cpu max we discovered older >> kernels had a bug where the physical address was copied directly from >> ID_AA64MMFR0_EL1.PARange field. The early cpu_init code of Xen commits >> the same error by blindly copying across the max supported range. >> Unsurprisingly when the page tables aren't set up for these greater >> ranges hilarity ensues and the hypervisor crashes fairly early on in >> the boot-up sequence. This happens when we write to the control >> register in enable_mmu(). >> Attempt to fix this the same way as the Linux kernel does by gating >> PARange to the maximum the hypervisor can handle. I also had to fix up >> code in p2m which panics when it sees an "invalid" entry in PARange. >> Signed-off-by: Alex Bennée >> Cc: Richard Henderson >> Cc: Stefano Stabellini >> Cc: Julien Grall >> Cc: Volodymyr Babchuk >> Cc: Bertrand Marquis > > Acked-by: Julien Grall Will you pick this up via your tree or do I need to do something else to get it upstreamed? I guess it needs to go on master and last stable? >>> >>> We only have one tree in Xen where committers (such as Stefano and I) >>> will commit patches regularly to staging. Osstest will then push to >>> master once the testing passed. >>> >>> I have done that now. Interestingly, git am wasn't able to apply this >>> patch. I had to do with: >>> >>> 42sh> git am --show-current-patch=diff | patch -p1 >>> patching file xen/arch/arm/arm64/head.S >>> Hunk #1 succeeded at 474 (offset 1 line). >>> patching file xen/arch/arm/p2m.c >>> Hunk #1 succeeded at 32 with fuzz 2. >>> Hunk #2 succeeded at 2023 (offset -7 lines). >>> Hunk #3 succeeded at 2031 (offset -7 lines). >>> Hunk #4 succeeded at 2062 (offset -7 lines). >>> >>> Which branch did you use for sending the patch? >> 0941d6cb23 from RELEASE-4.16.0 > > This would explain why. Patch sent to xen-devel should be based on > staging (or master). Ahh at the time I wasn't sure if there was another regression in master so I was basing of stable. I'll re-base of master next time ;-) -- Alex Bennée
Re: [PATCH v4 17/21] AMD/IOMMU: replace all-contiguous page tables by superpage mappings
On Mon, Apr 25, 2022 at 10:43:16AM +0200, Jan Beulich wrote: > When a page table ends up with all contiguous entries (including all > identical attributes), it can be replaced by a superpage entry at the > next higher level. The page table itself can then be scheduled for > freeing. > > Signed-off-by: Jan Beulich > --- > Unlike the freeing of all-empty page tables, this causes quite a bit of > back and forth for PV domains, due to their mapping/unmapping of pages > when they get converted to/from being page tables. It may therefore be > worth considering to delay re-coalescing a little, to avoid doing so > when the superpage would otherwise get split again pretty soon. But I > think this would better be the subject of a separate change anyway. > > Of course this could also be helped by more "aware" kernel side > behavior: They could avoid immediately mapping freed page tables > writable again, in anticipation of re-using that same page for another > page table elsewhere. > --- > v4: Re-base over changes earlier in the series. > v3: New. > > --- a/xen/drivers/passthrough/amd/iommu_map.c > +++ b/xen/drivers/passthrough/amd/iommu_map.c > @@ -81,7 +81,8 @@ static union amd_iommu_pte set_iommu_pte > unsigned long dfn, > unsigned long next_mfn, > unsigned int level, > - bool iw, bool ir) > + bool iw, bool ir, > + bool *contig) > { > union amd_iommu_pte *table, *pde, old; > > @@ -94,11 +95,15 @@ static union amd_iommu_pte set_iommu_pte > old.iw != iw || old.ir != ir ) > { > set_iommu_pde_present(pde, next_mfn, 0, iw, ir); > -pt_update_contig_markers(>raw, pfn_to_pde_idx(dfn, level), > - level, PTE_kind_leaf); > +*contig = pt_update_contig_markers(>raw, > + pfn_to_pde_idx(dfn, level), > + level, PTE_kind_leaf); > } > else > +{ > old.pr = false; /* signal "no change" to the caller */ > +*contig = false; So we assume that any caller getting contig == true must have acted and coalesced the page table? Might be worth a comment, to note that the function assumes that a previous return of contig == true will have coalesced the page table and hence a "no change" PTE write is not expected to happen on a contig page table. Thanks, Roger.
[PATCH V1] libxl/arm: Insert "xen,dev-domid" property to virtio-mmio device node
From: Oleksandr Tyshchenko Use specific binding for the virtio devices for which the restricted memory access using Xen grant mappings need to be enabled. Based on device-tree binding from Linux: Documentation/devicetree/bindings/arm/xen,dev-domid.yaml Signed-off-by: Oleksandr Tyshchenko --- !!! This patch is based on non upstreamed yet “Virtio support for toolstack on Arm” V8 series which is on review now: https://lore.kernel.org/xen-devel/1651598763-12162-1-git-send-email-olekst...@gmail.com/ New device-tree binding (commit #5) is a part of solution to restrict memory access under Xen using xen-grant DMA-mapping layer (which is also on review): https://lore.kernel.org/xen-devel/1651947548-4055-1-git-send-email-olekst...@gmail.com/ Changes RFC -> V1: - update commit description - rebase according to the recent changes to "libxl: Introduce basic virtio-mmio support on Arm" --- tools/libs/light/libxl_arm.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 37403a2..27ff328 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -862,7 +862,8 @@ static int make_vpci_node(libxl__gc *gc, void *fdt, static int make_virtio_mmio_node(libxl__gc *gc, void *fdt, - uint64_t base, uint32_t irq) + uint64_t base, uint32_t irq, + uint32_t backend_domid) { int res; gic_interrupt intr; @@ -887,6 +888,14 @@ static int make_virtio_mmio_node(libxl__gc *gc, void *fdt, res = fdt_property(fdt, "dma-coherent", NULL, 0); if (res) return res; +if (backend_domid != LIBXL_TOOLSTACK_DOMID) { +uint32_t domid[1]; + +domid[0] = cpu_to_fdt32(backend_domid); +res = fdt_property(fdt, "xen,dev-domid", domid, sizeof(domid)); +if (res) return res; +} + res = fdt_end_node(fdt); if (res) return res; @@ -1205,7 +1214,8 @@ next_resize: libxl_device_disk *disk = _config->disks[i]; if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) -FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq) ); +FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq, + disk->backend_domid) ); } if (pfdt) -- 2.7.4
Re: [PATCH 15/30] bus: brcmstb_gisb: Clean-up panic/die notifiers
On Wed 2022-04-27 19:49:09, Guilherme G. Piccoli wrote: > This patch improves the panic/die notifiers in this driver by > making use of a passed "id" instead of comparing pointer > address; also, it removes an useless prototype declaration > and unnecessary header inclusion. > > This is part of a panic notifiers refactor - this notifier in > the future will be moved to a new list, that encompass the > information notifiers only. > > --- a/drivers/bus/brcmstb_gisb.c > +++ b/drivers/bus/brcmstb_gisb.c > @@ -347,25 +346,14 @@ static irqreturn_t brcmstb_gisb_bp_handler(int irq, > void *dev_id) > /* > * Dump out gisb errors on die or panic. > */ > -static int dump_gisb_error(struct notifier_block *self, unsigned long v, > -void *p); > - > -static struct notifier_block gisb_die_notifier = { > - .notifier_call = dump_gisb_error, > -}; > - > -static struct notifier_block gisb_panic_notifier = { > - .notifier_call = dump_gisb_error, > -}; > - > static int dump_gisb_error(struct notifier_block *self, unsigned long v, > void *p) > { > struct brcmstb_gisb_arb_device *gdev; > - const char *reason = "panic"; > + const char *reason = "die"; > > - if (self == _die_notifier) > - reason = "die"; > + if (v == PANIC_NOTIFIER) > + reason = "panic"; IMHO, the check of the @self parameter was the proper solution. "gisb_die_notifier" list uses @val from enum die_val. "gisb_panic_notifier" list uses @val from enum panic_notifier_val. These are unrelated types. It might easily break when someone defines the same constant also in enum die_val. Best Regards, Petr
Re: [PATCH v4 1/2] xen: sync xs_wire.h header with upstream xen
Hi Stefano, > On 5 May 2022, at 01:23, Stefano Stabellini wrote: > > From: Stefano Stabellini > > Sync the xs_wire.h header file in Linux with the one in Xen. > > Signed-off-by: Stefano Stabellini > --- > include/xen/interface/io/xs_wire.h | 34 +++--- > 1 file changed, 31 insertions(+), 3 deletions(-) > > diff --git a/include/xen/interface/io/xs_wire.h > b/include/xen/interface/io/xs_wire.h > index d40a44f09b16..04dca77abc45 100644 > --- a/include/xen/interface/io/xs_wire.h > +++ b/include/xen/interface/io/xs_wire.h > @@ -10,7 +10,8 @@ > > enum xsd_sockmsg_type > { > -XS_DEBUG, > +XS_CONTROL, > +#define XS_DEBUG XS_CONTROL > XS_DIRECTORY, > XS_READ, > XS_GET_PERMS, > @@ -30,8 +31,13 @@ enum xsd_sockmsg_type > XS_IS_DOMAIN_INTRODUCED, > XS_RESUME, > XS_SET_TARGET, > -XS_RESTRICT, > -XS_RESET_WATCHES, > +/* XS_RESTRICT has been removed */ > +XS_RESET_WATCHES = XS_SET_TARGET + 2, > +XS_DIRECTORY_PART, > + > +XS_TYPE_COUNT, /* Number of valid types. */ > + > +XS_INVALID = 0x /* Guaranteed to remain an invalid type */ > }; > I’ve checked and seems that here there is this missing? @@ -59,8 +71,10 @@ static struct xsd_errors xsd_errors[] __attribute__((unused)) = { XSD_ERROR(EROFS), XSD_ERROR(EBUSY), XSD_ERROR(EAGAIN), -XSD_ERROR(EISCONN) +XSD_ERROR(EISCONN), +XSD_ERROR(E2BIG) }; > #define XS_WRITE_NONE "NONE" > @@ -87,9 +93,31 @@ struct xenstore_domain_interface { > char rsp[XENSTORE_RING_SIZE]; /* Replies and async watch events. */ > XENSTORE_RING_IDX req_cons, req_prod; > XENSTORE_RING_IDX rsp_cons, rsp_prod; > +uint32_t server_features; /* Bitmap of features supported by the server > */ > +uint32_t connection; > +uint32_t error; > }; > > /* Violating this is very bad. See docs/misc/xenstore.txt. */ > #define XENSTORE_PAYLOAD_MAX 4096 > > +/* Violating these just gets you an error back */ > +#define XENSTORE_ABS_PATH_MAX 3072 > +#define XENSTORE_REL_PATH_MAX 2048 > + > +/* The ability to reconnect a ring */ > +#define XENSTORE_SERVER_FEATURE_RECONNECTION 1 > +/* The presence of the "error" field in the ring page */ > +#define XENSTORE_SERVER_FEATURE_ERROR2 > + > +/* Valid values for the connection field */ > +#define XENSTORE_CONNECTED 0 /* the steady-state */ > +#define XENSTORE_RECONNECT 1 /* guest has initiated a reconnect */ > + > +/* Valid values for the error field */ > +#define XENSTORE_ERROR_NONE0 /* No error */ > +#define XENSTORE_ERROR_COMM1 /* Communication problem */ > +#define XENSTORE_ERROR_RINGIDX 2 /* Invalid ring index */ > +#define XENSTORE_ERROR_PROTO 3 /* Protocol violation (payload too long) */ > + > #endif /* _XS_WIRE_H */ > -- > 2.25.1 > >
Re: [PATCH 14/30] panic: Properly identify the panic event to the notifiers' callbacks
On Wed 2022-04-27 19:49:08, Guilherme G. Piccoli wrote: > The notifiers infrastructure provides a way to pass an "id" to the > callbacks to determine what kind of event happened, i.e., what is > the reason behind they getting called. > > The panic notifier currently pass 0, but this is soon to be > used in a multi-targeted notifier, so let's pass a meaningful > "id" over there. > > Signed-off-by: Guilherme G. Piccoli > --- > include/linux/panic_notifier.h | 5 + > kernel/panic.c | 2 +- > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/include/linux/panic_notifier.h b/include/linux/panic_notifier.h > index 41e32483d7a7..07dced83a783 100644 > --- a/include/linux/panic_notifier.h > +++ b/include/linux/panic_notifier.h > @@ -9,4 +9,9 @@ extern struct atomic_notifier_head panic_notifier_list; > > extern bool crash_kexec_post_notifiers; > > +enum panic_notifier_val { > + PANIC_UNUSED, > + PANIC_NOTIFIER = 0xDEAD, > +}; Hmm, this looks like a hack. PANIC_UNUSED will never be used. All notifiers will be always called with PANIC_NOTIFIER. The @val parameter is normally used when the same notifier_list is used in different situations. But you are going to use it when the same notifier is used in more lists. This is normally distinguished by the @nh (atomic_notifier_head) parameter. IMHO, it is a bad idea. First, it would confuse people because it does not follow the original design of the parameters. Second, the related code must be touched anyway when the notifier is moved into another list so it does not help much. Or do I miss anything, please? Best Regards, Petr
[ovmf test] 170299: regressions - FAIL
flight 170299 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170299/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 915 attempts Testing same since 170272 2022-05-09 15:12:57 Z0 days 19 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
Re: [PATCH v2] xen/arm: p2m don't fall over on FEAT_LPA enabled hw
Hi Alex, On 10/05/2022 15:47, Alex Bennée wrote: Julien Grall writes: On 10/05/2022 15:03, Alex Bennée wrote: Julien Grall writes: Hi Alex, On 28/04/2022 11:34, Alex Bennée wrote: When we introduced FEAT_LPA to QEMU's -cpu max we discovered older kernels had a bug where the physical address was copied directly from ID_AA64MMFR0_EL1.PARange field. The early cpu_init code of Xen commits the same error by blindly copying across the max supported range. Unsurprisingly when the page tables aren't set up for these greater ranges hilarity ensues and the hypervisor crashes fairly early on in the boot-up sequence. This happens when we write to the control register in enable_mmu(). Attempt to fix this the same way as the Linux kernel does by gating PARange to the maximum the hypervisor can handle. I also had to fix up code in p2m which panics when it sees an "invalid" entry in PARange. Signed-off-by: Alex Bennée Cc: Richard Henderson Cc: Stefano Stabellini Cc: Julien Grall Cc: Volodymyr Babchuk Cc: Bertrand Marquis Acked-by: Julien Grall Will you pick this up via your tree or do I need to do something else to get it upstreamed? I guess it needs to go on master and last stable? We only have one tree in Xen where committers (such as Stefano and I) will commit patches regularly to staging. Osstest will then push to master once the testing passed. I have done that now. Interestingly, git am wasn't able to apply this patch. I had to do with: 42sh> git am --show-current-patch=diff | patch -p1 patching file xen/arch/arm/arm64/head.S Hunk #1 succeeded at 474 (offset 1 line). patching file xen/arch/arm/p2m.c Hunk #1 succeeded at 32 with fuzz 2. Hunk #2 succeeded at 2023 (offset -7 lines). Hunk #3 succeeded at 2031 (offset -7 lines). Hunk #4 succeeded at 2062 (offset -7 lines). Which branch did you use for sending the patch? 0941d6cb23 from RELEASE-4.16.0 This would explain why. Patch sent to xen-devel should be based on staging (or master). Cheers, -- Julien Grall
Re: [PATCH v2] xen/arm: p2m don't fall over on FEAT_LPA enabled hw
Julien Grall writes: > Hi Alex, > > On 10/05/2022 15:03, Alex Bennée wrote: >> Julien Grall writes: >> >>> Hi Alex, >>> >>> On 28/04/2022 11:34, Alex Bennée wrote: When we introduced FEAT_LPA to QEMU's -cpu max we discovered older kernels had a bug where the physical address was copied directly from ID_AA64MMFR0_EL1.PARange field. The early cpu_init code of Xen commits the same error by blindly copying across the max supported range. Unsurprisingly when the page tables aren't set up for these greater ranges hilarity ensues and the hypervisor crashes fairly early on in the boot-up sequence. This happens when we write to the control register in enable_mmu(). Attempt to fix this the same way as the Linux kernel does by gating PARange to the maximum the hypervisor can handle. I also had to fix up code in p2m which panics when it sees an "invalid" entry in PARange. Signed-off-by: Alex Bennée Cc: Richard Henderson Cc: Stefano Stabellini Cc: Julien Grall Cc: Volodymyr Babchuk Cc: Bertrand Marquis >>> >>> Acked-by: Julien Grall >> Will you pick this up via your tree or do I need to do something >> else to >> get it upstreamed? I guess it needs to go on master and last stable? > > We only have one tree in Xen where committers (such as Stefano and I) > will commit patches regularly to staging. Osstest will then push to > master once the testing passed. > > I have done that now. Interestingly, git am wasn't able to apply this > patch. I had to do with: > > 42sh> git am --show-current-patch=diff | patch -p1 > patching file xen/arch/arm/arm64/head.S > Hunk #1 succeeded at 474 (offset 1 line). > patching file xen/arch/arm/p2m.c > Hunk #1 succeeded at 32 with fuzz 2. > Hunk #2 succeeded at 2023 (offset -7 lines). > Hunk #3 succeeded at 2031 (offset -7 lines). > Hunk #4 succeeded at 2062 (offset -7 lines). > > Which branch did you use for sending the patch? 0941d6cb23 from RELEASE-4.16.0 > Regarding stable, I will add the patch in my backport candidate list > and send a list to Stefano when we prepare the backports. > > Cheers, -- Alex Bennée
Re: [PATCH v3 4/6] xen: Switch to byteswap
On Tue, May 10, 2022 at 06:15:22AM -0400, Lin Liu wrote: > Update to use byteswap to swap bytes. > > No functional change. > > Signed-off-by: Lin Liu FYI, this patch breaks build of stubdomain: In file included from /var/tmp/git.xen.lU52/stubdom/include/../../xen/common/unxz.c:124, from xg_dom_decompress_unsafe_xz.c:40: /var/tmp/git.xen.lU52/stubdom/include/../../xen/common/xz/dec_stream.c: In function ‘dec_stream_header’: /var/tmp/git.xen.lU52/stubdom/include/../../xen/common/xz/private.h:31:21: error: implicit declaration of function ‘le32_to_cpu’; did you mean ‘le32_to_cpup’? [-Werror=implicit-function-declaration] 31 | #define get_le32(p) le32_to_cpu(*(const uint32_t *)(p)) | ^~~ /var/tmp/git.xen.lU52/stubdom/include/../../xen/common/xz/dec_stream.c:393:7: note: in expansion of macro ‘get_le32’ 393 |!= get_le32(s->temp.buf + HEADER_MAGIC_SIZE + 2)) | ^~~~ cc1: all warnings being treated as errors make[2]: *** [/var/tmp/git.xen.lU52/stubdom/libs-x86_64/guest/../../../tools/Rules.mk:150: xg_dom_decompress_unsafe_xz.o] Error 1 make[1]: *** [Makefile:367: libs-x86_64/guest/libxenguest.a] Error 2 make: *** [Makefile:73: build-stubdom] Error 2 Cheers, -- Anthony PERARD
Re: [PATCH v2 2/2] ns16550: Add more device IDs for Intel LPSS UART
On 10/05/2022 12:55, Marek Marczykowski-Górecki wrote: > This is purely based on the spec: > - Intel 500 Series PCH: 635218-006 > - Intel 600 Series PCH: 691222-001, 648364-003 > > This is tested only on TGL-LP added initially, but according to the > spec, they should behave the same. > > Signed-off-by: Marek Marczykowski-Górecki Acked-by: Andrew Cooper
Re: [PATCH v2 1/2] ns16550: reject IRQ above nr_irqs_gsi
On 10/05/2022 12:55, Marek Marczykowski-Górecki wrote: > Intel LPSS has INTERRUPT_LINE set to 0xff by default, that can't > possibly work. While a proper IRQ configuration may be useful, > validating value retrieved from the hardware is still necessary. If it > fails, use the device in poll mode, instead of crashing down the line > (at smp_initr_init()). Currently it's > x86-specific, as the surrounding code is guarded with CONFIG_X86 anyway. > > Signed-off-by: Marek Marczykowski-Górecki This isn't invalid per say. It explicitly means unknown/no connection and is used in practice to mean "never generate line interrupts, even when MSI is disabled". It's part of PCI 3.0 if the internet is to be believed, but ISTR is mandatory for SRIOV devices where the use of line interrupts is prohibited by the spec. Also, there are systems where nr_irq_gsi is greater than 0xff. I'd recommend handling 0xff specially as "no legacy irq", and not involving nr_irq_gsi at all. ~Andrew
Re: [PATCH v4 16/21] VT-d: free all-empty page tables
On Mon, Apr 25, 2022 at 10:42:50AM +0200, Jan Beulich wrote: > When a page table ends up with no present entries left, it can be > replaced by a non-present entry at the next higher level. The page table > itself can then be scheduled for freeing. > > Note that while its output isn't used there yet, > pt_update_contig_markers() right away needs to be called in all places > where entries get updated, not just the one where entries get cleared. > > Note further that while pt_update_contig_markers() updates perhaps > several PTEs within the table, since these are changes to "avail" bits > only I do not think that cache flushing would be needed afterwards. Such > cache flushing (of entire pages, unless adding yet more logic to me more > selective) would be quite noticable performance-wise (very prominent > during Dom0 boot). > > Signed-off-by: Jan Beulich > --- > v4: Re-base over changes earlier in the series. > v3: Properly bound loop. Re-base over changes earlier in the series. > v2: New. > --- > The hang during boot on my Latitude E6410 (see the respective code > comment) was pretty close after iommu_enable_translation(). No errors, > no watchdog would kick in, just sometimes the first few pixel lines of > the next log message's (XEN) prefix would have made it out to the screen > (and there's no serial there). It's been a lot of experimenting until I > figured the workaround (which I consider ugly, but halfway acceptable). > I've been trying hard to make sure the workaround wouldn't be masking a > real issue, yet I'm still wary of it possibly doing so ... My best guess > at this point is that on these old IOMMUs the ignored bits 52...61 > aren't really ignored for present entries, but also aren't "reserved" > enough to trigger faults. This guess is from having tried to set other > bits in this range (unconditionally, and with the workaround here in > place), which yielded the same behavior. Should we take Kevin's Reviewed-by as a heads up that bits 52..61 on some? IOMMUs are not usable? Would be good if we could get a more formal response I think. > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -43,6 +43,9 @@ > #include "vtd.h" > #include "../ats.h" > > +#define CONTIG_MASK DMA_PTE_CONTIG_MASK > +#include > + > /* dom_io is used as a sentinel for quarantined devices */ > #define QUARANTINE_SKIP(d, pgd_maddr) ((d) == dom_io && !(pgd_maddr)) > #define DEVICE_DOMID(d, pdev) ((d) != dom_io ? (d)->domain_id \ > @@ -405,6 +408,9 @@ static uint64_t addr_to_dma_page_maddr(s > > write_atomic(>val, new_pte.val); > iommu_sync_cache(pte, sizeof(struct dma_pte)); > +pt_update_contig_markers(>val, > + address_level_offset(addr, level), I think (unless previous patches in the series have changed this) there already is an 'offset' local variable that you could use. > + level, PTE_kind_table); > } > > if ( --level == target ) > @@ -837,9 +843,31 @@ static int dma_pte_clear_one(struct doma > > old = *pte; > dma_clear_pte(*pte); > +iommu_sync_cache(pte, sizeof(*pte)); > + > +while ( pt_update_contig_markers(>val, > + address_level_offset(addr, level), > + level, PTE_kind_null) && > +++level < min_pt_levels ) > +{ > +struct page_info *pg = maddr_to_page(pg_maddr); > + > +unmap_vtd_domain_page(page); > + > +pg_maddr = addr_to_dma_page_maddr(domain, addr, level, flush_flags, > + false); > +BUG_ON(pg_maddr < PAGE_SIZE); > + > +page = map_vtd_domain_page(pg_maddr); > +pte = [address_level_offset(addr, level)]; > +dma_clear_pte(*pte); > +iommu_sync_cache(pte, sizeof(*pte)); > + > +*flush_flags |= IOMMU_FLUSHF_all; > +iommu_queue_free_pgtable(hd, pg); > +} I think I'm setting myself for trouble, but do we need to sync cache the lower lever entries if higher level ones are to be changed. IOW, would it be fine to just flush the highest level modified PTE? As the lower lever ones won't be reachable anyway. > spin_unlock(>arch.mapping_lock); > -iommu_sync_cache(pte, sizeof(struct dma_pte)); > > unmap_vtd_domain_page(page); > > @@ -2182,8 +2210,21 @@ static int __must_check cf_check intel_i > } > > *pte = new; > - > iommu_sync_cache(pte, sizeof(struct dma_pte)); > + > +/* > + * While the (ab)use of PTE_kind_table here allows to save some work in > + * the function, the main motivation for it is that it avoids a so far > + * unexplained hang during boot (while preparing Dom0) on a Westmere > + * based laptop. > + */ > +pt_update_contig_markers(>val, > + address_level_offset(dfn_to_daddr(dfn), level), > +
Re: [PATCH 11/30] um: Improve panic notifiers consistency and ordering
On Wed 2022-04-27 19:49:05, Guilherme G. Piccoli wrote: > Currently the panic notifiers from user mode linux don't follow > the convention for most of the other notifiers present in the > kernel (indentation, priority setting, numeric return). > More important, the priorities could be improved, since it's a > special case (userspace), hence we could run the notifiers earlier; > user mode linux shouldn't care much with other panic notifiers but > the ordering among the mconsole and arch notifier is important, > given that the arch one effectively triggers a core dump. It is not clear to me why user mode linux should not care about the other notifiers. It might be because I do not know much about the user mode linux. Is the because they always create core dump or are never running in a hypervisor or ...? AFAIK, the notifiers do many different things. For example, there is a notifier that disables RCU watchdog, print some extra information. Why none of them make sense here? > This patch fixes that by running the mconsole notifier as the first > panic notifier, followed by the architecture one (that coredumps). > Also, we remove a useless header inclusion. Best Regards, Petr
Re: [XEN PATCH] docs: fix path to code in migration doc
On 10/05/2022 15:05, Anthony PERARD wrote: > Signed-off-by: Anthony PERARD Acked-by: Andrew Cooper
Re: [PATCH 10/30] alpha: Clean-up the panic notifier code
On Mon 2022-05-09 11:13:17, Guilherme G. Piccoli wrote: > On 27/04/2022 19:49, Guilherme G. Piccoli wrote: > > The alpha panic notifier has some code issues, not following > > the conventions of other notifiers. Also, it might halt the > > machine but still it is set to run as early as possible, which > > doesn't seem to be a good idea. Yeah, it is pretty strange behavior. I looked into the history. This notifier was added into the alpha code in 2.4.0-test2pre2. In this historic code, the default panic() code either rebooted after a timeout or ended in a infinite loop. There was not crasdump at that times. The notifier allowed to change the behavior. There were 3 notifiers: + mips and mips64 ended with blinking in panic() + alpha did __halt() in this srm case They both still do this. I guess that it is some historic behavior that people using these architectures are used to. Anyway, it makes sense to do this as the last notifier after dumping other information. Reviewed-by: Petr Mladek Best Regards, Petr
Re: [PATCH v2] xen/arm: p2m don't fall over on FEAT_LPA enabled hw
Hi Alex, On 10/05/2022 15:03, Alex Bennée wrote: Julien Grall writes: Hi Alex, On 28/04/2022 11:34, Alex Bennée wrote: When we introduced FEAT_LPA to QEMU's -cpu max we discovered older kernels had a bug where the physical address was copied directly from ID_AA64MMFR0_EL1.PARange field. The early cpu_init code of Xen commits the same error by blindly copying across the max supported range. Unsurprisingly when the page tables aren't set up for these greater ranges hilarity ensues and the hypervisor crashes fairly early on in the boot-up sequence. This happens when we write to the control register in enable_mmu(). Attempt to fix this the same way as the Linux kernel does by gating PARange to the maximum the hypervisor can handle. I also had to fix up code in p2m which panics when it sees an "invalid" entry in PARange. Signed-off-by: Alex Bennée Cc: Richard Henderson Cc: Stefano Stabellini Cc: Julien Grall Cc: Volodymyr Babchuk Cc: Bertrand Marquis Acked-by: Julien Grall Will you pick this up via your tree or do I need to do something else to get it upstreamed? I guess it needs to go on master and last stable? We only have one tree in Xen where committers (such as Stefano and I) will commit patches regularly to staging. Osstest will then push to master once the testing passed. I have done that now. Interestingly, git am wasn't able to apply this patch. I had to do with: 42sh> git am --show-current-patch=diff | patch -p1 patching file xen/arch/arm/arm64/head.S Hunk #1 succeeded at 474 (offset 1 line). patching file xen/arch/arm/p2m.c Hunk #1 succeeded at 32 with fuzz 2. Hunk #2 succeeded at 2023 (offset -7 lines). Hunk #3 succeeded at 2031 (offset -7 lines). Hunk #4 succeeded at 2062 (offset -7 lines). Which branch did you use for sending the patch? Regarding stable, I will add the patch in my backport candidate list and send a list to Stefano when we prepare the backports. Cheers, -- Julien Grall
Re: [PATCH 08/30] powerpc/setup: Refactor/untangle panic notifiers
On 10/05/2022 10:53, Michael Ellerman wrote: > "Guilherme G. Piccoli" writes: >> On 05/05/2022 15:55, Hari Bathini wrote: >>> [...] >>> The change looks good. I have tested it on an LPAR (ppc64). >>> >>> Reviewed-by: Hari Bathini >>> >> >> Hi Michael. do you think it's possible to add this one to powerpc/next >> (or something like that), or do you prefer a V2 with his tag? > > Ah sorry, I assumed it was going in as part of the whole series. I guess > I misread the cover letter. > > So you want me to take this patch on its own via the powerpc tree? > > cheers Hi Michael, thanks for the prompt response! You didn't misread, that was the plan heh But some maintainers start to take patches and merge in their trees, and in the end, it seems to make sense - almost half of this series are fixes or clean-ups, that are not really necessary to get merged altogether. So, if you can take this one, I'd appreciate - it'll make V2 a bit smaller =) Cheers, Guilherme
[XEN PATCH] docs: fix path to code in migration doc
Signed-off-by: Anthony PERARD --- docs/features/migration.pandoc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/features/migration.pandoc b/docs/features/migration.pandoc index 719925818e..5334536d48 100644 --- a/docs/features/migration.pandoc +++ b/docs/features/migration.pandoc @@ -54,10 +54,10 @@ legacy stream into a migration v2 stream. * `docs/specs/libxc-migration-stream.pandoc` * `docs/specs/libxl-migration-stream.pandoc` * `libxc` -* `tools/libxc/xc_sr_*.[hc]` +* `tools/libs/guest/xg_sr_*.[hc]` * `libxl` -* `tools/libxl/libxl_stream_{read,write}.c` -* `tools/libxl/libxl_convert_callout.c` +* `tools/libs/light/libxl_stream_{read,write}.c` +* `tools/libs/light/libxl_convert_callout.c` * Scripts * `tools/python/xen/migration/*.py` * `tools/python/scripts/convert-legacy-stream` -- Anthony PERARD
Re: [PATCH v2] xen/arm: p2m don't fall over on FEAT_LPA enabled hw
Julien Grall writes: > Hi Alex, > > On 28/04/2022 11:34, Alex Bennée wrote: >> When we introduced FEAT_LPA to QEMU's -cpu max we discovered older >> kernels had a bug where the physical address was copied directly from >> ID_AA64MMFR0_EL1.PARange field. The early cpu_init code of Xen commits >> the same error by blindly copying across the max supported range. >> Unsurprisingly when the page tables aren't set up for these greater >> ranges hilarity ensues and the hypervisor crashes fairly early on in >> the boot-up sequence. This happens when we write to the control >> register in enable_mmu(). >> Attempt to fix this the same way as the Linux kernel does by gating >> PARange to the maximum the hypervisor can handle. I also had to fix up >> code in p2m which panics when it sees an "invalid" entry in PARange. >> Signed-off-by: Alex Bennée >> Cc: Richard Henderson >> Cc: Stefano Stabellini >> Cc: Julien Grall >> Cc: Volodymyr Babchuk >> Cc: Bertrand Marquis > > Acked-by: Julien Grall Will you pick this up via your tree or do I need to do something else to get it upstreamed? I guess it needs to go on master and last stable? -- Alex Bennée
Re: [PATCH 08/30] powerpc/setup: Refactor/untangle panic notifiers
"Guilherme G. Piccoli" writes: > On 05/05/2022 15:55, Hari Bathini wrote: >> [...] >> The change looks good. I have tested it on an LPAR (ppc64). >> >> Reviewed-by: Hari Bathini >> > > Hi Michael. do you think it's possible to add this one to powerpc/next > (or something like that), or do you prefer a V2 with his tag? Ah sorry, I assumed it was going in as part of the whole series. I guess I misread the cover letter. So you want me to take this patch on its own via the powerpc tree? cheers
[ovmf test] 170298: regressions - FAIL
flight 170298 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170298/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 914 attempts Testing same since 170272 2022-05-09 15:12:57 Z0 days 18 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
[PATCH 4/4] x86: kprobes: Prohibit probing on instruction which has emulate prefix
From: Masami Hiramatsu commit 004e8dce9c5595697951f7cd0e9f66b35c92265e upstream Prohibit probing on instruction which has XEN_EMULATE_PREFIX or KVM's emulate prefix. Since that prefix is a marker for Xen and KVM, if we modify the marker by kprobe's int3, that doesn't work as expected. Signed-off-by: Masami Hiramatsu Signed-off-by: Peter Zijlstra (Intel) Cc: Juergen Gross Cc: x...@kernel.org Cc: Boris Ostrovsky Cc: Ingo Molnar Cc: Stefano Stabellini Cc: Andrew Cooper Cc: Borislav Petkov Cc: xen-devel@lists.xenproject.org Cc: Randy Dunlap Cc: Josh Poimboeuf Link: https://lkml.kernel.org/r/156777566048.25081.6296162369492175325.stgit@devnote2 Signed-off-by: Maximilian Heyne Cc: sta...@vger.kernel.org # 5.4.x --- arch/x86/kernel/kprobes/core.c | 4 1 file changed, 4 insertions(+) diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c index c205d77d57da..3700dc94847c 100644 --- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -358,6 +358,10 @@ int __copy_instruction(u8 *dest, u8 *src, u8 *real, struct insn *insn) kernel_insn_init(insn, dest, MAX_INSN_SIZE); insn_get_length(insn); + /* We can not probe force emulate prefixed instruction */ + if (insn_has_emulate_prefix(insn)) + return 0; + /* Another subsystem puts a breakpoint, failed to recover */ if (insn->opcode.bytes[0] == BREAKPOINT_INSTRUCTION) return 0; -- 2.32.0 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 2/4] x86: xen: kvm: Gather the definition of emulate prefixes
From: Masami Hiramatsu commit b3dc0695fa40c3b280230fb6fb7fb7a94ce28bf4 upstream Gather the emulate prefixes, which forcibly make the following instruction emulated on virtualization, in one place. Suggested-by: Peter Zijlstra Signed-off-by: Masami Hiramatsu Signed-off-by: Peter Zijlstra (Intel) Cc: Juergen Gross Cc: x...@kernel.org Cc: Ingo Molnar Cc: Boris Ostrovsky Cc: Andrew Cooper Cc: Stefano Stabellini Cc: Borislav Petkov Cc: xen-devel@lists.xenproject.org Cc: Randy Dunlap Cc: Josh Poimboeuf Link: https://lkml.kernel.org/r/156777563917.25081.7286628561790289995.stgit@devnote2 Signed-off-by: Maximilian Heyne Cc: sta...@vger.kernel.org # 5.4.x --- arch/x86/include/asm/emulate_prefix.h | 14 ++ arch/x86/include/asm/xen/interface.h | 11 --- arch/x86/kvm/x86.c| 4 +++- 3 files changed, 21 insertions(+), 8 deletions(-) create mode 100644 arch/x86/include/asm/emulate_prefix.h diff --git a/arch/x86/include/asm/emulate_prefix.h b/arch/x86/include/asm/emulate_prefix.h new file mode 100644 index ..70f5b98a5286 --- /dev/null +++ b/arch/x86/include/asm/emulate_prefix.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_X86_EMULATE_PREFIX_H +#define _ASM_X86_EMULATE_PREFIX_H + +/* + * Virt escape sequences to trigger instruction emulation; + * ideally these would decode to 'whole' instruction and not destroy + * the instruction stream; sadly this is not true for the 'kvm' one :/ + */ + +#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */ +#define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */ + +#endif diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h index 62ca03ef5c65..9139b3e86316 100644 --- a/arch/x86/include/asm/xen/interface.h +++ b/arch/x86/include/asm/xen/interface.h @@ -379,12 +379,9 @@ struct xen_pmu_arch { * Prefix forces emulation of some non-trapping instructions. * Currently only CPUID. */ -#ifdef __ASSEMBLY__ -#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ; -#define XEN_CPUID XEN_EMULATE_PREFIX cpuid -#else -#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; " -#define XEN_CPUID XEN_EMULATE_PREFIX "cpuid" -#endif +#include + +#define XEN_EMULATE_PREFIX __ASM_FORM(.byte __XEN_EMULATE_PREFIX ;) +#define XEN_CPUID XEN_EMULATE_PREFIX __ASM_FORM(cpuid) #endif /* _ASM_X86_XEN_INTERFACE_H */ diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 1f7dfa5aa42d..6dd77e426889 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -68,6 +68,7 @@ #include #include #include +#include #include #define CREATE_TRACE_POINTS @@ -5583,6 +5584,7 @@ EXPORT_SYMBOL_GPL(kvm_write_guest_virt_system); int handle_ud(struct kvm_vcpu *vcpu) { + static const char kvm_emulate_prefix[] = { __KVM_EMULATE_PREFIX }; int emul_type = EMULTYPE_TRAP_UD; char sig[5]; /* ud2; .ascii "kvm" */ struct x86_exception e; @@ -5590,7 +5592,7 @@ int handle_ud(struct kvm_vcpu *vcpu) if (force_emulation_prefix && kvm_read_guest_virt(vcpu, kvm_get_linear_rip(vcpu), sig, sizeof(sig), ) == 0 && - memcmp(sig, "\xf\xbkvm", sizeof(sig)) == 0) { + memcmp(sig, kvm_emulate_prefix, sizeof(sig)) == 0) { kvm_rip_write(vcpu, kvm_rip_read(vcpu) + sizeof(sig)); emul_type = EMULTYPE_TRAP_UD_FORCED; } -- 2.32.0 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 0/4] x86: decode Xen/KVM emulate prefixes
This is a backport of a patch series for 5.4.x. The patch series allows the x86 decoder to decode the Xen and KVM emulate prefixes. In particular this solves the following issue that appeared when commit db6c6a0df840 ("objtool: Fix noreturn detection for ignored functions") was backported to 5.4.69: arch/x86/xen/enlighten_pv.o: warning: objtool: xen_cpuid()+0x25: can't find jump dest instruction at .text+0x9c Also now that this decoding is possible, also backport the commit which prevents kprobes on probing such prefixed instructions. This was also part of the original series. The series applied mostly cleanly on 5.4.192 except for a contextual problem in the 3rd patch ("x86: xen: insn: Decode Xen and KVM emulate-prefix signature"). Masami Hiramatsu (4): x86/asm: Allow to pass macros to __ASM_FORM() x86: xen: kvm: Gather the definition of emulate prefixes x86: xen: insn: Decode Xen and KVM emulate-prefix signature x86: kprobes: Prohibit probing on instruction which has emulate prefix arch/x86/include/asm/asm.h | 8 +++-- arch/x86/include/asm/emulate_prefix.h | 14 + arch/x86/include/asm/insn.h | 6 arch/x86/include/asm/xen/interface.h| 11 +++ arch/x86/kernel/kprobes/core.c | 4 +++ arch/x86/kvm/x86.c | 4 ++- arch/x86/lib/insn.c | 34 + tools/arch/x86/include/asm/emulate_prefix.h | 14 + tools/arch/x86/include/asm/insn.h | 6 tools/arch/x86/lib/insn.c | 34 + tools/objtool/sync-check.sh | 3 +- tools/perf/check-headers.sh | 3 +- 12 files changed, 128 insertions(+), 13 deletions(-) create mode 100644 arch/x86/include/asm/emulate_prefix.h create mode 100644 tools/arch/x86/include/asm/emulate_prefix.h base-commit: 1d72b776f6dc973211f5d153453cf8955fb3d70a -- 2.32.0 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 1/4] x86/asm: Allow to pass macros to __ASM_FORM()
From: Masami Hiramatsu commit f7919fd943abf0c77aed4441ea9897a323d132f5 upstream Use __stringify() at __ASM_FORM() so that user can pass code including macros to __ASM_FORM(). Signed-off-by: Masami Hiramatsu Signed-off-by: Peter Zijlstra (Intel) Cc: Juergen Gross Cc: x...@kernel.org Cc: Boris Ostrovsky Cc: Ingo Molnar Cc: Stefano Stabellini Cc: Andrew Cooper Cc: Borislav Petkov Cc: xen-devel@lists.xenproject.org Cc: Randy Dunlap Cc: Josh Poimboeuf Link: https://lkml.kernel.org/r/156777562873.25081.2288083344657460959.stgit@devnote2 Signed-off-by: Maximilian Heyne Cc: sta...@vger.kernel.org # 5.4.x --- arch/x86/include/asm/asm.h | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h index 3ff577c0b102..1b563f9167ea 100644 --- a/arch/x86/include/asm/asm.h +++ b/arch/x86/include/asm/asm.h @@ -7,9 +7,11 @@ # define __ASM_FORM_RAW(x) x # define __ASM_FORM_COMMA(x) x, #else -# define __ASM_FORM(x) " " #x " " -# define __ASM_FORM_RAW(x) #x -# define __ASM_FORM_COMMA(x) " " #x "," +#include + +# define __ASM_FORM(x) " " __stringify(x) " " +# define __ASM_FORM_RAW(x) __stringify(x) +# define __ASM_FORM_COMMA(x) " " __stringify(x) "," #endif #ifndef __x86_64__ -- 2.32.0 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 3/4] x86: xen: insn: Decode Xen and KVM emulate-prefix signature
From: Masami Hiramatsu commit 4d65adfcd1196818659d3bd9b42dccab291e1751 upstream Decode Xen and KVM's emulate-prefix signature by x86 insn decoder. It is called "prefix" but actually not x86 instruction prefix, so this adds insn.emulate_prefix_size field instead of reusing insn.prefixes. If x86 decoder finds a special sequence of instructions of XEN_EMULATE_PREFIX and 'ud2a; .ascii "kvm"', it just counts the length, set insn.emulate_prefix_size and fold it with the next instruction. In other words, the signature and the next instruction is treated as a single instruction. Signed-off-by: Masami Hiramatsu Signed-off-by: Peter Zijlstra (Intel) Acked-by: Josh Poimboeuf Cc: Juergen Gross Cc: x...@kernel.org Cc: Boris Ostrovsky Cc: Ingo Molnar Cc: Stefano Stabellini Cc: Andrew Cooper Cc: Borislav Petkov Cc: xen-devel@lists.xenproject.org Cc: Randy Dunlap Link: https://lkml.kernel.org/r/156777564986.25081.4964537658500952557.stgit@devnote2 [mheyne: resolved contextual conflict in tools/objtools/sync-check.sh] Signed-off-by: Maximilian Heyne Cc: sta...@vger.kernel.org # 5.4.x --- arch/x86/include/asm/insn.h | 6 arch/x86/lib/insn.c | 34 + tools/arch/x86/include/asm/emulate_prefix.h | 14 + tools/arch/x86/include/asm/insn.h | 6 tools/arch/x86/lib/insn.c | 34 + tools/objtool/sync-check.sh | 3 +- tools/perf/check-headers.sh | 3 +- 7 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 tools/arch/x86/include/asm/emulate_prefix.h diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h index a51ffeea6d87..a8c3d284fa46 100644 --- a/arch/x86/include/asm/insn.h +++ b/arch/x86/include/asm/insn.h @@ -45,6 +45,7 @@ struct insn { struct insn_field immediate2; /* for 64bit imm or seg16 */ }; + int emulate_prefix_size; insn_attr_t attr; unsigned char opnd_bytes; unsigned char addr_bytes; @@ -128,6 +129,11 @@ static inline int insn_is_evex(struct insn *insn) return (insn->vex_prefix.nbytes == 4); } +static inline int insn_has_emulate_prefix(struct insn *insn) +{ + return !!insn->emulate_prefix_size; +} + /* Ensure this instruction is decoded completely */ static inline int insn_complete(struct insn *insn) { diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c index 0b5862ba6a75..404279563891 100644 --- a/arch/x86/lib/insn.c +++ b/arch/x86/lib/insn.c @@ -13,6 +13,8 @@ #include #include +#include + /* Verify next sizeof(t) bytes can be on the same instruction */ #define validate_next(t, insn, n) \ ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) @@ -58,6 +60,36 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64) insn->addr_bytes = 4; } +static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX }; +static const insn_byte_t kvm_prefix[] = { __KVM_EMULATE_PREFIX }; + +static int __insn_get_emulate_prefix(struct insn *insn, +const insn_byte_t *prefix, size_t len) +{ + size_t i; + + for (i = 0; i < len; i++) { + if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i]) + goto err_out; + } + + insn->emulate_prefix_size = len; + insn->next_byte += len; + + return 1; + +err_out: + return 0; +} + +static void insn_get_emulate_prefix(struct insn *insn) +{ + if (__insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix))) + return; + + __insn_get_emulate_prefix(insn, kvm_prefix, sizeof(kvm_prefix)); +} + /** * insn_get_prefixes - scan x86 instruction prefix bytes * @insn: insn containing instruction @@ -76,6 +108,8 @@ void insn_get_prefixes(struct insn *insn) if (prefixes->got) return; + insn_get_emulate_prefix(insn); + nb = 0; lb = 0; b = peek_next(insn_byte_t, insn); diff --git a/tools/arch/x86/include/asm/emulate_prefix.h b/tools/arch/x86/include/asm/emulate_prefix.h new file mode 100644 index ..70f5b98a5286 --- /dev/null +++ b/tools/arch/x86/include/asm/emulate_prefix.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_X86_EMULATE_PREFIX_H +#define _ASM_X86_EMULATE_PREFIX_H + +/* + * Virt escape sequences to trigger instruction emulation; + * ideally these would decode to 'whole' instruction and not destroy + * the instruction stream; sadly this is not true for the 'kvm' one :/ + */ + +#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */ +#define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */ + +#endif diff --git a/tools/arch/x86/include/asm/insn.h b/tools/arch/x86/include/asm/insn.h index d7f0ae8f3c44..52c6262e6bfd 100644 --- a/tools/arch/x86/include/asm/insn.h +++
Re: [PATCH v4 15/21] AMD/IOMMU: free all-empty page tables
On Mon, Apr 25, 2022 at 10:42:19AM +0200, Jan Beulich wrote: > When a page table ends up with no present entries left, it can be > replaced by a non-present entry at the next higher level. The page table > itself can then be scheduled for freeing. > > Note that while its output isn't used there yet, > pt_update_contig_markers() right away needs to be called in all places > where entries get updated, not just the one where entries get cleared. > > Signed-off-by: Jan Beulich Reviewed-by: Roger Pau Monné Some comments below. > --- > v4: Re-base over changes earlier in the series. > v3: Re-base over changes earlier in the series. > v2: New. > > --- a/xen/drivers/passthrough/amd/iommu_map.c > +++ b/xen/drivers/passthrough/amd/iommu_map.c > @@ -21,6 +21,9 @@ > > #include "iommu.h" > > +#define CONTIG_MASK IOMMU_PTE_CONTIG_MASK > +#include > + > /* Given pfn and page table level, return pde index */ > static unsigned int pfn_to_pde_idx(unsigned long pfn, unsigned int level) > { > @@ -33,16 +36,20 @@ static unsigned int pfn_to_pde_idx(unsig > > static union amd_iommu_pte clear_iommu_pte_present(unsigned long l1_mfn, > unsigned long dfn, > - unsigned int level) > + unsigned int level, > + bool *free) > { > union amd_iommu_pte *table, *pte, old; > +unsigned int idx = pfn_to_pde_idx(dfn, level); > > table = map_domain_page(_mfn(l1_mfn)); > -pte = [pfn_to_pde_idx(dfn, level)]; > +pte = [idx]; > old = *pte; > > write_atomic(>raw, 0); > > +*free = pt_update_contig_markers(>raw, idx, level, PTE_kind_null); > + > unmap_domain_page(table); > > return old; > @@ -85,7 +92,11 @@ static union amd_iommu_pte set_iommu_pte > if ( !old.pr || old.next_level || > old.mfn != next_mfn || > old.iw != iw || old.ir != ir ) > +{ > set_iommu_pde_present(pde, next_mfn, 0, iw, ir); > +pt_update_contig_markers(>raw, pfn_to_pde_idx(dfn, level), > + level, PTE_kind_leaf); It would be better to call pt_update_contig_markers inside of set_iommu_pde_present, but that would imply changing the parameters passed to the function. It's cumbersome (and error prone) to have to pair calls to set_iommu_pde_present() with pt_update_contig_markers(). > +} > else > old.pr = false; /* signal "no change" to the caller */ > > @@ -322,6 +333,9 @@ static int iommu_pde_from_dfn(struct dom > smp_wmb(); > set_iommu_pde_present(pde, next_table_mfn, next_level, true, >true); > +pt_update_contig_markers(_table_vaddr->raw, > + pfn_to_pde_idx(dfn, level), > + level, PTE_kind_table); > > *flush_flags |= IOMMU_FLUSHF_modified; > } > @@ -347,6 +361,9 @@ static int iommu_pde_from_dfn(struct dom > next_table_mfn = mfn_x(page_to_mfn(table)); > set_iommu_pde_present(pde, next_table_mfn, next_level, true, >true); > +pt_update_contig_markers(_table_vaddr->raw, > + pfn_to_pde_idx(dfn, level), > + level, PTE_kind_table); > } > else /* should never reach here */ > { > @@ -474,8 +491,24 @@ int cf_check amd_iommu_unmap_page( > > if ( pt_mfn ) > { > +bool free; > + > /* Mark PTE as 'page not present'. */ > -old = clear_iommu_pte_present(pt_mfn, dfn_x(dfn), level); > +old = clear_iommu_pte_present(pt_mfn, dfn_x(dfn), level, ); > + > +while ( unlikely(free) && ++level < hd->arch.amd.paging_mode ) > +{ > +struct page_info *pg = mfn_to_page(_mfn(pt_mfn)); > + > +if ( iommu_pde_from_dfn(d, dfn_x(dfn), level, _mfn, > +flush_flags, false) ) > +BUG(); > +BUG_ON(!pt_mfn); > + > +clear_iommu_pte_present(pt_mfn, dfn_x(dfn), level, ); Not sure it's worth initializing free to false (at definition and before each call to clear_iommu_pte_present), just in case we manage to return early from clear_iommu_pte_present without having updated 'free'. Thanks, Roger.
[ovmf test] 170297: regressions - FAIL
flight 170297 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170297/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 913 attempts Testing same since 170272 2022-05-09 15:12:57 Z0 days 17 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path
On 10/05/2022 08:38, Petr Mladek wrote: > [...] > I see two more alternative solutions: > > 1st variant is a trick already used in console write() callbacks. > They do trylock() when oops_in_progress is set. They remember > the result to prevent double unlock when printing Oops messages and > the system will try to continue working. For example: > > pl011_console_write(struct console *co, const char *s, unsigned int count) > { > [...] > int locked = 1; > [...] > if (uap->port.sysrq) > locked = 0; > else if (oops_in_progress) > locked = spin_trylock(>port.lock); > else > spin_lock(>port.lock); > > [...] > > if (locked) > spin_unlock(>port.lock); > } > > > 2nd variant is to check panic_cpu variable. It is used in printk.c. > We might move the function to panic.h: > > static bool panic_in_progress(void) > { > return unlikely(atomic_read(_cpu) != PANIC_CPU_INVALID); > } > > and then do: > > if (panic_in_progress()) { > ... Thanks for the review Petr! I feel alternative two is way better, it checks for panic - the oops_in_progress isn't really enough, since we can call panic() directly, not necessarily through an oops path, correct? For me, we could stick with the lock check, but I'll defer to Evan - I didn't work the V2 patch yet, what do you prefer Evan? > [...] > As already mentioned in the other reply, panic() sometimes stops > the other CPUs using NMI, for example, see kdump_nmi_shootdown_cpus(). > > Another situation is when the CPU using the lock ends in some > infinite loop because something went wrong. The system is in > an unpredictable state during panic(). > > I am not sure if this is possible with the code under gsmi_dev.lock > but such things really happen during panic() in other subsystems. > Using trylock in the panic() code path is a good practice. > > Best Regards, > Petr Makes total sense, thanks for confirming! Cheers, Guilherme
Re: [PATCH 05/30] misc/pvpanic: Convert regular spinlock into trylock on panic path
On 10/05/2022 09:14, Petr Mladek wrote: > [...] >> With that said, it's dangerous to use regular spinlocks in such path, >> as introduced by commit b3c0f8774668 ("misc/pvpanic: probe multiple >> instances"). >> This patch fixes that by replacing regular spinlocks with the trylock >> safer approach. > > It seems that the lock is used just to manipulating a list. A super > safe solution would be to use the rcu API: rcu_add_rcu() and > list_del_rcu() under rcu_read_lock(). The spin lock will not be > needed and the list will always be valid. > > The advantage would be that it will always call members that > were successfully added earlier. That said, I am not familiar > with pvpanic and am not sure if it is worth it. > >> It also fixes an old comment (about a long gone framebuffer code) and >> the notifier priority - we should execute hypervisor notifiers early, >> deferring this way the panic action to the hypervisor, as expected by >> the users that are setting up pvpanic. > > This should be done in a separate patch. It changes the behavior. > Also there might be a discussion whether it really should be > the maximal priority. > > Best Regards, > Petr Thanks for the review Petr. Patch was already merged - my goal was to be concise, i.e., a patch per driver / module, so the patch kinda fixes whatever I think is wrong with the driver with regards panic handling. Do you think it worth to remove this patch from Greg's branch just to split it in 2? Personally I think it's not worth, but opinions are welcome. About the RCU part, this one really could be a new patch, a good improvement patch - it makes sense to me, we can think about that after the fixes I guess. Cheers, Guilherme
Re: [PATCH v2] xen/evtchn: Add design for static event channel signaling
Hi Rahul, On 04/05/2022 18:34, Rahul Singh wrote: This patch introduces a new feature to support the signaling between two domains in dom0less system. Signed-off-by: Rahul Singh --- v2 changes: - switch to the one-subnode-per-evtchn under xen,domain" compatible node. - Add more detail about event-channel --- docs/designs/dom0less-evtchn.md | 126 Answering here to also keep the history. On IRC, Bertrand was asking whether we merge design proposal. We have merged proposal in the past (e.g. non-cooperative migration) and I would be ready to do it again as it is easier to find them afterwards. However, I wonder whether it would be better to turn this proposal to a binding change in misc/arm/device-tree/. Any thoughts? 1 file changed, 126 insertions(+) create mode 100644 docs/designs/dom0less-evtchn.md diff --git a/docs/designs/dom0less-evtchn.md b/docs/designs/dom0less-evtchn.md new file mode 100644 index 00..62ec8a4009 --- /dev/null +++ b/docs/designs/dom0less-evtchn.md @@ -0,0 +1,126 @@ +# Signaling support between two domUs on dom0less system + +## Current state: Draft version + +## Proposer(s): Rahul Singh, Bertrand Marquis + +## Problem Statement: + +Dom0less guests would benefit from a statically-defined memory sharing and +signally system for communication. One that would be immediately available at +boot without any need for dynamic configurations. + +In embedded a great variety of guest operating system kernels exist, many of +which don't have support for xenstore, grant table, or other complex drivers. I am not sure I would consider event channel FIFO a "trival" drivers :). +Some of them are small kernel-space applications (often called "baremetal", +not to be confused with the term "baremetal" used in the data center which +means "without hypervisors") or RTOSes. Additionally, for safety reasons, users +often need to be able to configure the full system statically so that it can +be verified statically. + +Event channels are very simple and can be added even to baremetal applications. +This proposal introduces a way to define them statically to make them suitable +for dom0less embedded deployments. + +## Proposal: + +Event channels are the basic primitive provided by Xen for event notifications. +An event channel is a logical connection between 2 domains (more specifically +between dom1,port1, and dom2,port2). Each event has a pending and a masked bit. +The pending bit indicates the event has been raised. The masked bit is used by +the domain to prevent the delivery of that specific event. Xen only performs a +0 → 1 transition on the pending bits and does not touch the mask bit. The NIT: I think → is not an ascii character. Can you use "->"? +domain may toggle masked bits in the masked bit field and should clear the +pending bit when an event has been processed + +Events are received by a domain via an interrupt from Xen to the domain, +indicating when an event arrives (setting the bit). Further notifications are +blocked until the bit is cleared again. Events are delivered asynchronously to +a domain and are enqueued when the domain is not running. +More information about FIFO based event channel can be found at: I think the explanation is fine for a design proposal. If you want to use it as documentation, then I would suggest to clarify there are two different ABI for event channel: FIFO and 2L. 2L is the easiest one to implement and for embedded we may want to steer the users towards it. +https://xenbits.xen.org/people/dvrabel/event-channels-H.pdf It is quite unfortunate that this wasn't merged in docs/. Oh well, no action for you here. + +The event channel communication will be established statically between two +domains (dom0 and domU also) before unpausing the domains after domain creation. +Event channel connection information between domains will be passed to XEN via NIT: above you are using "Xen". So s/XEN/Xen/ for consistency. +the device tree node. The event channel will be created and established +beforehand in XEN before the domain started. The domain doesn’t need to do any Same here. NIT: I think "beforehand" and "before" is redundant. +operation to establish a connection. Domain only needs hypercall +EVTCHNOP_send(local port) to send notifications to the remote guest. + +There is no need to describe the static event channel info in the domU device +tree. Static event channels are only useful in fully static configurations, +and in those configurations the domU device tree dynamically generated by Xen +is not needed. + +Under the "xen,domain" compatible node, there need to be sub-nodes with +compatible "xen,evtchn" that describe the event channel connection between two +domains(dom0 and domU also). Below you provided an example between two domUs. Can you provide one between dom0 and a domU? + +The event channel sub-node has the following properties: + +- compatible + +
Re: [PATCH 05/30] misc/pvpanic: Convert regular spinlock into trylock on panic path
On Wed 2022-04-27 19:48:59, Guilherme G. Piccoli wrote: > The pvpanic driver relies on panic notifiers to execute a callback > on panic event. Such function is executed in atomic context - the > panic function disables local IRQs, preemption and all other CPUs > that aren't running the panic code. > > With that said, it's dangerous to use regular spinlocks in such path, > as introduced by commit b3c0f8774668 ("misc/pvpanic: probe multiple > instances"). > This patch fixes that by replacing regular spinlocks with the trylock > safer approach. It seems that the lock is used just to manipulating a list. A super safe solution would be to use the rcu API: rcu_add_rcu() and list_del_rcu() under rcu_read_lock(). The spin lock will not be needed and the list will always be valid. The advantage would be that it will always call members that were successfully added earlier. That said, I am not familiar with pvpanic and am not sure if it is worth it. > It also fixes an old comment (about a long gone framebuffer code) and > the notifier priority - we should execute hypervisor notifiers early, > deferring this way the panic action to the hypervisor, as expected by > the users that are setting up pvpanic. This should be done in a separate patch. It changes the behavior. Also there might be a discussion whether it really should be the maximal priority. Best Regards, Petr
[ovmf test] 170296: regressions - FAIL
flight 170296 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170296/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 912 attempts Testing same since 170272 2022-05-09 15:12:57 Z0 days 16 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
Re: [PATCH v2] xen/evtchn: Add design for static event channel signaling
Hi, > On 9 May 2022, at 21:50, Stefano Stabellini wrote: > > On Wed, 4 May 2022, Rahul Singh wrote: >> This patch introduces a new feature to support the signaling between >> two domains in dom0less system. >> >> Signed-off-by: Rahul Singh Reviewed-by: Bertrand Marquis Cheers Bertrand
[qemu-mainline test] 170286: tolerable FAIL - PUSHED
flight 170286 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/170286/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 170275 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 170275 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 170275 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 170275 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 170275 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 170275 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 170275 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 170275 test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-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-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-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-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-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-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-armhf-armhf-libvirt-raw 14 migrate-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-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 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-libvirt-qcow2 14 migrate-support-checkfail never pass version targeted for testing: qemuu178bacb66d98d9ee7a702b9f2a4dfcd88b72a9ab baseline version: qemuu7e314198157bf38ae7fdd5a000b8795db015d582 Last test of basis 170275 2022-05-09 17:08:38 Z0 days Testing same since 170286 2022-05-10 02:27:31 Z0 days1 attempts People who touched revisions under test: Gavin Shan Igor Mammedov Leif Lindholm
[PATCH v2 2/2] ns16550: Add more device IDs for Intel LPSS UART
This is purely based on the spec: - Intel 500 Series PCH: 635218-006 - Intel 600 Series PCH: 691222-001, 648364-003 This is tested only on TGL-LP added initially, but according to the spec, they should behave the same. Signed-off-by: Marek Marczykowski-Górecki --- Changes in v2: - new patch, adding more IDs to the patch that went in already --- xen/drivers/char/ns16550.c | 80 +- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index 0c6f6ec43de1..b4486a4e8768 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -1077,12 +1077,90 @@ static const struct ns16550_config __initconst uart_config[] = .dev_id = 0x0358, .param = param_exar_xr17v358 }, -/* Intel Corp. TGL-LP LPSS PCI */ +/* Intel Corp. TGL-LP LPSS PCI UART #0 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0xa0a8, +.param = param_intel_lpss +}, +/* Intel Corp. TGL-LP LPSS PCI UART #1 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0xa0a9, +.param = param_intel_lpss +}, +/* Intel Corp. TGL-LP LPSS PCI UART #2 */ { .vendor_id = PCI_VENDOR_ID_INTEL, .dev_id = 0xa0c7, .param = param_intel_lpss }, +/* Intel Corp. TGL-H LPSS PCI UART #0 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x43a8, +.param = param_intel_lpss +}, +/* Intel Corp. TGL-H LPSS PCI UART #1 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x43a9, +.param = param_intel_lpss +}, +/* Intel Corp. TGL-H LPSS PCI UART #2 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x43a7, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-P LPSS PCI UART #0 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x51a8, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-P LPSS PCI UART #1 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x51a9, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-P LPSS PCI UART #2 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x51c7, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-P LPSS PCI UART #3 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x51da, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-S LPSS PCI UART #0 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x7aa8, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-S LPSS PCI UART #1 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x7aa9, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-S LPSS PCI UART #2 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x7afe, +.param = param_intel_lpss +}, +/* Intel Corp. ADL-S LPSS PCI UART #3 */ +{ +.vendor_id = PCI_VENDOR_ID_INTEL, +.dev_id = 0x7adc, +.param = param_intel_lpss +}, }; static int __init -- 2.35.1
[PATCH v2 1/2] ns16550: reject IRQ above nr_irqs_gsi
Intel LPSS has INTERRUPT_LINE set to 0xff by default, that can't possibly work. While a proper IRQ configuration may be useful, validating value retrieved from the hardware is still necessary. If it fails, use the device in poll mode, instead of crashing down the line (at smp_initr_init()). Currently it's x86-specific, as the surrounding code is guarded with CONFIG_X86 anyway. Signed-off-by: Marek Marczykowski-Górecki --- Changes in v2: - add log message - extend commit message - code style fix --- xen/drivers/char/ns16550.c | 9 + 1 file changed, 9 insertions(+) diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index fb75cee4a13a..0c6f6ec43de1 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -1238,6 +1238,15 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx) pci_conf_read8(PCI_SBDF(0, b, d, f), PCI_INTERRUPT_LINE) : 0; +if ( uart->irq >= nr_irqs_gsi ) +{ +printk(XENLOG_WARNING + "ns16550: %02x:%02x.%u reports invalid IRQ %d, " + "falling back to a poll mode\n", + b, d, f, uart->irq); +uart->irq = 0; +} + return 0; } } -- 2.35.1
Re: [PATCH v3] xen/build: Add cppcheck and cppcheck-html make rules
Hi Julien, > On 10 May 2022, at 12:50, Julien Grall wrote: > > > > On 10/05/2022 12:49, Bertrand Marquis wrote: >> Hi > > Hi Bertrand, > >>> On 9 May 2022, at 19:22, Julien Grall wrote: >>> >>> Hi, >>> >>> On 26/04/2022 13:38, Bertrand Marquis wrote: diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h index 852b5f3c24..ef37cfa16f 100644 --- a/xen/arch/arm/include/asm/processor.h +++ b/xen/arch/arm/include/asm/processor.h @@ -219,9 +219,11 @@ SCTLR_Axx_ELx_A | SCTLR_Axx_ELx_C |\ SCTLR_Axx_ELx_WXN | SCTLR_Axx_ELx_EE) +#ifndef CPPCHECK >>> >>> Can you add a comment explaining why you need this check? >> Sure, would the following be ok ? >> Cppcheck preprocessor is wrongly throwing the error here so disable this >> check for cppcheck runs > > That's fine with me. I think my ack is technically sufficient here and > Stefano tested the patch. > > So will do the modification and commit it in a bit. Thanks a lot :-) Cheers Bertrand > > Cheers, > > -- > Julien Grall
Re: [PATCH v3] xen/build: Add cppcheck and cppcheck-html make rules
On 10/05/2022 12:49, Bertrand Marquis wrote: Hi Hi Bertrand, On 9 May 2022, at 19:22, Julien Grall wrote: Hi, On 26/04/2022 13:38, Bertrand Marquis wrote: diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h index 852b5f3c24..ef37cfa16f 100644 --- a/xen/arch/arm/include/asm/processor.h +++ b/xen/arch/arm/include/asm/processor.h @@ -219,9 +219,11 @@ SCTLR_Axx_ELx_A| SCTLR_Axx_ELx_C |\ SCTLR_Axx_ELx_WXN | SCTLR_Axx_ELx_EE) +#ifndef CPPCHECK Can you add a comment explaining why you need this check? Sure, would the following be ok ? Cppcheck preprocessor is wrongly throwing the error here so disable this check for cppcheck runs That's fine with me. I think my ack is technically sufficient here and Stefano tested the patch. So will do the modification and commit it in a bit. Cheers, -- Julien Grall
Re: [PATCH v3] xen/build: Add cppcheck and cppcheck-html make rules
Hi > On 9 May 2022, at 19:22, Julien Grall wrote: > > Hi, > > On 26/04/2022 13:38, Bertrand Marquis wrote: >> diff --git a/xen/arch/arm/include/asm/processor.h >> b/xen/arch/arm/include/asm/processor.h >> index 852b5f3c24..ef37cfa16f 100644 >> --- a/xen/arch/arm/include/asm/processor.h >> +++ b/xen/arch/arm/include/asm/processor.h >> @@ -219,9 +219,11 @@ >> SCTLR_Axx_ELx_A| SCTLR_Axx_ELx_C |\ >> SCTLR_Axx_ELx_WXN | SCTLR_Axx_ELx_EE) >> +#ifndef CPPCHECK > > Can you add a comment explaining why you need this check? Sure, would the following be ok ? Cppcheck preprocessor is wrongly throwing the error here so disable this check for cppcheck runs > > With that: > > Acked-by: Julien Grall > Thanks Bertrand > Cheers, > > -- > Julien Grall >
Re: [PATCH v3 4/6] xen: Switch to byteswap
Hi, On 10/05/2022 12:34, Andrew Cooper wrote: On 10/05/2022 12:17, Julien Grall wrote: diff --git a/xen/include/xen/unaligned.h b/xen/include/xen/unaligned.h index 0a2b16d05d..16b2e6f5f0 100644 --- a/xen/include/xen/unaligned.h +++ b/xen/include/xen/unaligned.h @@ -20,62 +20,62 @@ static inline uint16_t get_unaligned_be16(const void *p) { - return be16_to_cpup(p); + return be16_to_cpu(*(const uint16_t *)p) I haven't checked the existing implementation of be16_to_cpup(). It's a plain dereference, just like this. AFAICT, it wasn't unaligned safe before, either. Well, technically an architecture could provide an override for the copy. I agree that arm32 is already bogus but... It should be reasonably easy to fix in a followup patch. Just memcpy() to/from the void pointer to a stack variable of the appropriate type. ... I disagree that it should be fixed in a follow-up patch. It should be fixed now as this is where the badness is spread to any architecture. No. That is an inappropriate request to make. Lin's patch does not alter the broken-ness of unaligned on arm32, and does improve the aspect of the hypervisor that it pertains to. It therefore stands on its own merit. I am not sure sure why switching from *cpup* improves things... and as usual you haven't answered to the clarification questions. Your choices are to either fix it yourself (after all, you are the maintainer who cares about this unrelated bug), or you ask Lin kindly if he has time to look into fixing the unrelated bug after this series is complete. Or 3) keep *cpup* so there is only one place to fix it. It is not reasonable to say "this unrelated thing is broken, and you need to fix it first to get your series in". Requests like that are, I'm sure, part of what Bertrand raised in the community call as unnecessary fiction getting work submitted. To be honest, you put the contributor in this situation. I would have been perfectly happy if we keep *cpup* around as there would be only a place to fix. With this approach, you are effectively going to increase the work later one because now we would have to chase all the open-coded version of *cpup* and check which one is not safe. Cheers, -- Julien Grall
Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path
On Tue 2022-05-03 16:12:09, Guilherme G. Piccoli wrote: > On 03/05/2022 15:03, Evan Green wrote: > > [...] > > gsmi_shutdown_reason() is a common function called in other scenarios > > as well, like reboot and thermal trip, where it may still make sense > > to wait to acquire a spinlock. Maybe we should add a parameter to > > gsmi_shutdown_reason() so that you can get your change on panic, but > > we don't convert other callbacks into try-fail scenarios causing us to > > miss logs. > > > > Hi Evan, thanks for your feedback, much appreciated! > What I've done in other cases like this was to have a helper checking > the spinlock in the panic notifier - if we can acquire that, go ahead > but if not, bail out. For a proper example of an implementation, check > patch 13 of the series: > https://lore.kernel.org/lkml/20220427224924.592546-14-gpicc...@igalia.com/ . > > Do you agree with that, or prefer really a parameter in > gsmi_shutdown_reason() ? I'll follow your choice =) I see two more alternative solutions: 1st variant is a trick already used in console write() callbacks. They do trylock() when oops_in_progress is set. They remember the result to prevent double unlock when printing Oops messages and the system will try to continue working. For example: pl011_console_write(struct console *co, const char *s, unsigned int count) { [...] int locked = 1; [...] if (uap->port.sysrq) locked = 0; else if (oops_in_progress) locked = spin_trylock(>port.lock); else spin_lock(>port.lock); [...] if (locked) spin_unlock(>port.lock); } 2nd variant is to check panic_cpu variable. It is used in printk.c. We might move the function to panic.h: static bool panic_in_progress(void) { return unlikely(atomic_read(_cpu) != PANIC_CPU_INVALID); } and then do: if (panic_in_progress()) { ... > > Though thinking more about it, is this really a Good Change (TM)? The > > spinlock itself already disables interrupts, meaning the only case > > where this change makes a difference is if the panic happens from > > within the function that grabbed the spinlock (in which case the > > callback is also likely to panic), or in an NMI that panics within > > that window. As already mentioned in the other reply, panic() sometimes stops the other CPUs using NMI, for example, see kdump_nmi_shootdown_cpus(). Another situation is when the CPU using the lock ends in some infinite loop because something went wrong. The system is in an unpredictable state during panic(). I am not sure if this is possible with the code under gsmi_dev.lock but such things really happen during panic() in other subsystems. Using trylock in the panic() code path is a good practice. Best Regards, Petr
Re: [PATCH v3 4/6] xen: Switch to byteswap
On 10/05/2022 12:17, Julien Grall wrote: >> >>> diff --git a/xen/include/xen/unaligned.h b/xen/include/xen/unaligned.h index 0a2b16d05d..16b2e6f5f0 100644 --- a/xen/include/xen/unaligned.h +++ b/xen/include/xen/unaligned.h @@ -20,62 +20,62 @@ static inline uint16_t get_unaligned_be16(const void *p) { - return be16_to_cpup(p); + return be16_to_cpu(*(const uint16_t *)p) >>> >>> I haven't checked the existing implementation of be16_to_cpup(). >> >> It's a plain dereference, just like this. AFAICT, it wasn't unaligned >> safe before, either. > > Well, technically an architecture could provide an override for the > copy. I agree that arm32 is already bogus but... > >> >> It should be reasonably easy to fix in a followup patch. Just memcpy() >> to/from the void pointer to a stack variable of the appropriate type. > ... I disagree that it should be fixed in a follow-up patch. It should > be fixed now as this is where the badness is spread to any architecture. No. That is an inappropriate request to make. Lin's patch does not alter the broken-ness of unaligned on arm32, and does improve the aspect of the hypervisor that it pertains to. It therefore stands on its own merit. Your choices are to either fix it yourself (after all, you are the maintainer who cares about this unrelated bug), or you ask Lin kindly if he has time to look into fixing the unrelated bug after this series is complete. It is not reasonable to say "this unrelated thing is broken, and you need to fix it first to get your series in". Requests like that are, I'm sure, part of what Bertrand raised in the community call as unnecessary fiction getting work submitted. ~Andrew
Re: OPNSense running in domU has no network connectivity on 5.15.29+
On Tue, May 03, 2022 at 12:36:43PM -0700, Colton Reeder wrote: > Hello, > > I am running the FreeBSD-based router OS OPNSense in a domU. I > recently upgraded my dom0 kernel from 5.15.26 to 5.15.32 and with the > new kernel, OPNSense had no connectivity. I downloaded from kernel.org > 5.15.26-32, built and installed each version and booted them > consecutively until I found the version that no longer worked. It > turned out to be 5.15.29. > > I looked through the change log of 5.15.29 and found two commits for > xen-netback > > commit 2708ceb4e5cc84ef179bad25a2d7890573ef78be commit > fe39ab30dcc204e321c2670cc1cf55904af35d01 > > I reverted these changes (a revert of a revert, yes) in 5.15.32, > built and installed. Now the network works. Now I dont know enough to > know thats for sure the right fix. Could you try if reverting only one of those fixes your issue? > Maybe I have a config issue, I dont > know, but reverting that change fixes the problem. What should I do? > I was asked to provide xenstore -ls https://pastebin.com/hHPWgrEy It's better to post the output of `xenstore-ls -fp`, as that's way easier to read. So it's only OPNSense that's affected, other VMs run fine? Do you get any output from Linux dmesg? >From the output of xenstore that you pasted, you do have another guests that seems to be running fine regarding network, are there any differences in the configuration file? FWIW, it seems like the netback instances are stuck in state 2. I think we need the guest config file, plus the output from `xl -vvv create config_file.cfg` Thanks, Roger.
Re: [PATCH v3 4/6] xen: Switch to byteswap
Hi, On 10/05/2022 12:09, Andrew Cooper wrote: On 10/05/2022 11:51, Julien Grall wrote: On 10/05/2022 11:15, Lin Liu wrote: diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index 4aae281e89..70d3be3be6 100644 --- a/xen/common/device_tree.c +++ b/xen/common/device_tree.c @@ -171,7 +171,7 @@ bool_t dt_property_read_u32(const struct dt_device_node *np, if ( !val || len < sizeof(*out_value) ) return 0; - *out_value = be32_to_cpup(val); + *out_value = be32_to_cpu(*val); This code has been taken from Linux and I would rather prefer to keep the *cpup* helpers to avoid any changes when backporting. I specifically requested that this be de-obfuscated. Hiding indirection is a fantastic way to introduce bugs, and we've had XSAs in the past because of it (admittedly in libxl, but still...). Care providing a link to those XSAs? But I don't really see what's the problem here, this is no better no worth than passing pointer to other functions... This file is already Xen style, not Linux, so won't be taking backports directly, and the resulting compiler diagnostic will make it obvious what is going on. be32_to_cpu(*val) works fine on older versions of Xen too. In this case, the cost of changing is well worth the improvements and simplifications gained. See the 0/6 diffstat and see that the compiler can make better optimisations when it can see the builtin. I take your point... However, the commit message provides virtually zero justification into why we should switch to be32_to_cpup(). So to me, the changes so far looks unwanted. diff --git a/xen/include/xen/unaligned.h b/xen/include/xen/unaligned.h index 0a2b16d05d..16b2e6f5f0 100644 --- a/xen/include/xen/unaligned.h +++ b/xen/include/xen/unaligned.h @@ -20,62 +20,62 @@ static inline uint16_t get_unaligned_be16(const void *p) { - return be16_to_cpup(p); + return be16_to_cpu(*(const uint16_t *)p) I haven't checked the existing implementation of be16_to_cpup(). It's a plain dereference, just like this. AFAICT, it wasn't unaligned safe before, either. Well, technically an architecture could provide an override for the copy. I agree that arm32 is already bogus but... It should be reasonably easy to fix in a followup patch. Just memcpy() to/from the void pointer to a stack variable of the appropriate type. ... I disagree that it should be fixed in a follow-up patch. It should be fixed now as this is where the badness is spread to any architecture. Cheers, -- Julien Grall
[libvirt test] 170287: regressions - FAIL
flight 170287 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/170287/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: 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 build-armhf-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 8cb37bac33c39c9632f91d13734f0d7a6918f313 baseline version: libvirt 2c846fa6bcc11929c9fb857a22430fb9945654ad Last test of basis 151777 2020-07-10 04:19:19 Z 669 days Failing since151818 2020-07-11 04:18:52 Z 668 days 650 attempts Testing same since 170287 2022-05-10 04:22:10 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 Chris Mayo Christian Borntraeger Christian Ehrhardt Christian Kirbach Christian Schoenebeck 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é Didik Supriadi dinglimin Divya Garg Dmitrii Shcherbakov Dmytro Linkin Eiichi Tsukata Emilio Herrera Eric Farman Erik Skultety Fabian Affolter Fabian Freyer Fabiano Fidêncio Fangge Jin Farhan Ali Fedora Weblate Translation 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 Luke Yue Luyao Zhong luzhipeng Marc Hartmayer Marc-André Lureau Marek Marczykowski-Górecki Markus Schade Martin Kletzander Martin Pitt Masayoshi Mizuma Matej Cepl Matt Coleman Matt Coleman Mauro Matteo Cascella Maxim Nestratov Meina Li Michal Privoznik Michał Smyk Milo Casagrande 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 Olesya Gerasimenko Or Ozeri Orion Poplawski Pany Paolo Bonzini Patrick Magauran Paulo de Rezende Pinatti Pavel Hrdina Peng Liang Peter Krempa Pino Toscano Pino
Re: [PATCH v3 1/6] xen: implement byteswap
Hi, On 10/05/2022 11:15, Lin Liu wrote: swab() is massively over complicated and can be simplified by builtins. NIT: "by builtins" -> "by re-implementing using compiler builtins". The compilers provide builtin function to swap bytes. * gcc: https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html * clang: https://clang.llvm.org/docs/LanguageExtensions.html This patch simplify swab() with builtins and fallback for old compilers. Signed-off-by: Lin Liu --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Bertrand Marquis Cc: Volodymyr Babchuk Cc: Andrew Cooper Cc: George Dunlap Cc: Jan Beulich Cc: Wei Liu Cc: "Roger Pau Monné" Changes in v3: - Check __has_builtin instead of GNUC version Changes in v2: - Add fallback for compilers without __builtin_bswap - Implement with plain C instead of macros --- xen/arch/arm/include/asm/byteorder.h | 14 ++- xen/arch/x86/include/asm/byteorder.h | 34 ++--- xen/include/xen/byteorder.h | 56 xen/include/xen/byteswap.h | 44 ++ xen/include/xen/compiler.h | 12 ++ 5 files changed, 120 insertions(+), 40 deletions(-) create mode 100644 xen/include/xen/byteorder.h create mode 100644 xen/include/xen/byteswap.h diff --git a/xen/arch/arm/include/asm/byteorder.h b/xen/arch/arm/include/asm/byteorder.h index 9c712c4788..622eeaba07 100644 --- a/xen/arch/arm/include/asm/byteorder.h +++ b/xen/arch/arm/include/asm/byteorder.h @@ -1,16 +1,10 @@ #ifndef __ASM_ARM_BYTEORDER_H__ #define __ASM_ARM_BYTEORDER_H__ -#define __BYTEORDER_HAS_U64__ +#ifndef __BYTE_ORDER__ + #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ +#endif -#include +#include #endif /* __ASM_ARM_BYTEORDER_H__ */ -/* - * Local variables: - * mode: C - * c-file-style: "BSD" - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ This change looks unrelated to this patch. Can you explain it? Cheers, -- Julien Grall
Re: [PATCH v3 5/6] byteorder: Remove byteorder
On 10/05/2022 11:15, Lin Liu wrote: > include/xen/byteswap.h has simplify the interface, just clean > the old interface > > No functional change > > Signed-off-by: Lin Liu > --- > Cc: Andrew Cooper > Cc: George Dunlap > Cc: Jan Beulich > Cc: Julien Grall > Cc: Stefano Stabellini > Cc: Wei Liu > --- > xen/include/xen/byteorder/big_endian.h| 102 > xen/include/xen/byteorder/generic.h | 68 > xen/include/xen/byteorder/little_endian.h | 102 > xen/include/xen/byteorder/swab.h | 183 -- > 4 files changed, 455 deletions(-) Reviewed-by: Andrew Cooper Good riddance.
Re: [PATCH v3 4/6] xen: Switch to byteswap
On 10/05/2022 11:51, Julien Grall wrote: > On 10/05/2022 11:15, Lin Liu wrote: >> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c >> index 4aae281e89..70d3be3be6 100644 >> --- a/xen/common/device_tree.c >> +++ b/xen/common/device_tree.c >> @@ -171,7 +171,7 @@ bool_t dt_property_read_u32(const struct >> dt_device_node *np, >> if ( !val || len < sizeof(*out_value) ) >> return 0; >> - *out_value = be32_to_cpup(val); >> + *out_value = be32_to_cpu(*val); > > This code has been taken from Linux and I would rather prefer to keep > the *cpup* helpers to avoid any changes when backporting. I specifically requested that this be de-obfuscated. Hiding indirection is a fantastic way to introduce bugs, and we've had XSAs in the past because of it (admittedly in libxl, but still...). This file is already Xen style, not Linux, so won't be taking backports directly, and the resulting compiler diagnostic will make it obvious what is going on. be32_to_cpu(*val) works fine on older versions of Xen too. In this case, the cost of changing is well worth the improvements and simplifications gained. See the 0/6 diffstat and see that the compiler can make better optimisations when it can see the builtin. > >> diff --git a/xen/include/xen/unaligned.h b/xen/include/xen/unaligned.h >> index 0a2b16d05d..16b2e6f5f0 100644 >> --- a/xen/include/xen/unaligned.h >> +++ b/xen/include/xen/unaligned.h >> @@ -20,62 +20,62 @@ >> static inline uint16_t get_unaligned_be16(const void *p) >> { >> - return be16_to_cpup(p); >> + return be16_to_cpu(*(const uint16_t *)p) > > I haven't checked the existing implementation of be16_to_cpup(). It's a plain dereference, just like this. AFAICT, it wasn't unaligned safe before, either. It should be reasonably easy to fix in a followup patch. Just memcpy() to/from the void pointer to a stack variable of the appropriate type. ~Andrew
[ovmf test] 170295: regressions - FAIL
flight 170295 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/170295/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 168254 build-amd64 6 xen-buildfail REGR. vs. 168254 build-i3866 xen-buildfail REGR. vs. 168254 build-i386-xsm6 xen-buildfail REGR. vs. 168254 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 0e31124877cc8bc0140a03ad3196f0d58b2fd966 baseline version: ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70 Last test of basis 168254 2022-02-28 10:41:46 Z 71 days Failing since168258 2022-03-01 01:55:31 Z 70 days 911 attempts Testing same since 170272 2022-05-09 15:12:57 Z0 days 15 attempts People who touched revisions under test: Abdul Lateef Attar Abdul Lateef Attar via groups.io Abner Chang Akihiko Odaki Anthony PERARD Bo Chang Ke Bob Feng Chao Li Chao, Zhuoran Chen Lin Z Chen, Christine Chen, Lin Z Corvin Köhne Dandan Bi Dun Tan duntan Feng, Bob C Gerd Hoffmann Gua Guo Guo Dong Guomin Jiang Hao A Wu Heng Luo Hua Ma Huang, Li-Xia Jagadeesh Ujja Jake Garver Jake Garver via groups.io Jason Jason Lou Jiewen Yao Ke, Bo-ChangX Ken Lautner Kenneth Lautner Kuo, Ted Laszlo Ersek Lean Sheng Tan Leif Lindholm Li, Yi1 Li, Zhihao Liming Gao Liu Liu Yun Liu Yun Y Lixia Huang Lou, Yun Ma, Hua Mara Sophie Grosch Mara Sophie Grosch via groups.io Matt DeVillier Michael D Kinney Michael Kubacki Michael Kubacki Min Xu Oliver Steffen Patrick Rudolph Peter Grehan Purna Chandra Rao Bandaru Ray Ni Rebecca Cran Rebecca Cran Sami Mujawar Sean Rhodes Sean Rhodes sean@starlabs.systems Sebastien Boeuf Sunny Wang Tan, Dun Ted Kuo Wenyi Xie wenyi,xie via groups.io Xiaolu.Jiang Xie, Yuanhao Yi Li yi1 li Yu Pu Yuanhao Xie Yuwei Chen Zhihao Li Zhuoran Chao jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 6200 lines long.)
Re: [PATCH v3 3/6] arm64/find_next_bit: Remove ext2_swab()
Hi, On 10/05/2022 11:15, Lin Liu wrote: ext2 has nothing to do with this logic. This code was a verbatim copy from Linux. Looking at the history, I am not sure there was even a connection with the ext2 filesystem (I guess this is what you mean?). So I would drop this and simply say that we could use the new helpers. Clean up the code with xen/byteswap.h which now has an unsigned long helper. No functional change. Signed-off-by: Lin Liu Other that what I wrote above: Acked-by: Julien Grall Cheers, -- Julien Grall
Re: Proposal: use disk sequence numbers to avoid races in blkback
On Thu, May 05, 2022 at 08:30:17PM -0400, Demi Marie Obenour wrote: > Proposal: Check disk sequence numbers in blkback > > > Currently, adding block devices to a domain is racy. libxl writes the > major and minor number of the device to XenStore, but it does not keep > the block device open until blkback has opened it. This creates a race > condition, as it is possible for the device to be destroyed and another > device allocated with the same major and minor numbers. Loop devices > are the most obvious example, since /dev/loop0 can be reused again and > again, but the same problem can also happen with device-mapper devices. > If the major and minor numbers are reused before blkback has attached to > the device, blkback will pass the wrong device to the domain, with > obvious security consequences. > > Other programs on Linux have the same problem, and a solution was > committed upstream in the form of disk sequence numbers. A disk > sequence number, or diskseq, is a 64-bit unsigned monotonically > increasing counter. The combination of a major and minor number and a > disk sequence number uniquely identifies a block device for the entire > uptime of the system. Seems fine to me, this is just an extra check to make sure the block device opened by blkback is the one that user space intended. I would see diskseq as a kind of checksum. > I propose that blkback check for an unsigned 64-bit hexadecimal XenStore > entry named “diskseq”. If the entry exists, blkback checks that the > number stored there matches the disk sequence number of the device. If > it does not exist, the check is skipped. If reading the entry fails for > any other reason, the entry is malformed, or if the sequence number is > wrong, blkback refuses to export the device. > > The toolstack changes are more involved for two reasons: > > 1. To ensure that loop devices are not leaked if the toolstack crashes, >they must be created with the delete-on-close flag set. This >requires that the toolstack hold the device open until blkback has >acquired a handle to it. Does this work with loop devices? I would expect that you need to issue a losetup call to detach the device. Even more, the loop device is created by the block script, but there's also a window between the block script execution and the toolstack knowing about the device, which could also allow for a leak? > > 2. For block devices that are opened by path, the toolstack needs to >ensure that the device it has opened is actually the device it >intended to open. This requires device-specific verification of the >open file descriptor. This is not needed for regular files, as the >LOOP_CONFIGURE ioctl is called on an existing loop device and sets >its backing file. > > The first is fairly easy in C. It can be accomplished by means of a > XenStore watch on the “status” entry. Once that watch fires, blkback > has opened the device, so the toolstack can safely close its file > descriptor. Does the toolstack really need to close the device? What harm does it do to keep the handle open until the domain is destroyed? What about disk hotplug? Which entity will keep the device opened in that case? Is xl block-attach going to block until the device switches to the connected state? > The second is significantly more difficult. It requires the block > script to be aware of at least device-mapper devices and LVM2 logical > volumes. The general technique is common to all block devices: obtain > the sequence number (via the BLKGETDISKSEQ() ioctl) and its major and > minor numbers (via fstat()). Then open /sys/dev/block/MAJOR:MINOR to > get a directory file descriptor, and use openat(2) and read(2) to get > various sysfs attributes. Finally, read the diskseq sysfs attribute and > check that it matches the sequence number from BLKGETDISKSEQ(). > Alternatively, one can use device-specific methods, such as > device-mapper ioctls. > > Device-mapper devices can be detected via the ‘dm/name’ sysfs attribute, > which must match the name under ‘/dev/mapper/’. If the name is of the > form ‘/dev/X/Y’, and the ‘dm/uuid’ attribute starts with the literal > string “LVM-”, then the expected ‘dm/name’ attribute should be found by > doubling all ‘-’ characters in X and Y, and then joining X and Y with > another ‘-’. This accounts for LVM2 logical volumes. Alternatively, > one can use device-mapper ioctls to both check if a device is a > device-mapper device, and to obtain its name and UUID. I plan on going > with the latter route. Likely a stupid remark, but needs obviously needs to be kept to Linux only. Thanks, Roger.
Re: [PATCH v3 3/6] arm64/find_next_bit: Remove ext2_swab()
On 10/05/2022 11:15, Lin Liu wrote: > ext2 has nothing to do with this logic. Clean up the code with > xen/byteswap.h which now has an unsigned long helper. > > No functional change. > > Signed-off-by: Lin Liu Reviewed-by: Andrew Cooper
Re: [PATCH v3 2/6] crypto/vmac: Simplify code with byteswap
On 10/05/2022 11:15, Lin Liu wrote: > This file has its own implementation of swap bytes. Clean up > the code with xen/byteswap.h. > > No functional change. > > Signed-off-by: Lin Liu > Acked-by: Jan Beulich Reviewed-by: Andrew Cooper
Re: [PATCH v3 1/6] xen: implement byteswap
On 10/05/2022 11:15, Lin Liu wrote: > swab() is massively over complicated and can be simplified by builtins. > The compilers provide builtin function to swap bytes. > * gcc: https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html > * clang: https://clang.llvm.org/docs/LanguageExtensions.html > This patch simplify swab() with builtins and fallback for old compilers. Arguably, this patch introduces a new byteswapping infrastructure in terms of compiler builtins and bswapXX(), so the swab() infrastructure can be retired. > diff --git a/xen/arch/arm/include/asm/byteorder.h > b/xen/arch/arm/include/asm/byteorder.h > index 9c712c4788..622eeaba07 100644 > --- a/xen/arch/arm/include/asm/byteorder.h > +++ b/xen/arch/arm/include/asm/byteorder.h > @@ -1,16 +1,10 @@ > #ifndef __ASM_ARM_BYTEORDER_H__ > #define __ASM_ARM_BYTEORDER_H__ > > -#define __BYTEORDER_HAS_U64__ > +#ifndef __BYTE_ORDER__ > + #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ > +#endif This won't actually do what you want on GCC 4.5 or older. You also want #ifndef __ORDER_LITTLE_ENDIAN__ # define __ORDER_LITTLE_ENDIAN__ 1234 #endif #ifndef __ORDER_BIG_ENDIAN__ # define __ORDER_BIG_ENDIAN__ 4321 #endif in compiler.h to cope with older GCC. Otherwise, LGTM. Reviewed-by: Andrew Cooper I can fix this on commit if its the only issue issue. Otherwise, please correct it when posting v4. ~Andrew
Re: [PATCH v3 4/6] xen: Switch to byteswap
Hi, On 10/05/2022 11:15, Lin Liu wrote: Update to use byteswap to swap bytes. No functional change. Signed-off-by: Lin Liu --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper Cc: George Dunlap Cc: Jan Beulich Cc: Wei Liu Changes in v3: - Update xen/common/device_tree.c to use be32_to_cpu - Keep const in type cast in unaligned.h --- xen/common/device_tree.c | 44 +++--- xen/common/libelf/libelf-private.h | 6 ++-- xen/common/xz/private.h| 2 +- xen/include/xen/unaligned.h| 24 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index 4aae281e89..70d3be3be6 100644 --- a/xen/common/device_tree.c +++ b/xen/common/device_tree.c @@ -171,7 +171,7 @@ bool_t dt_property_read_u32(const struct dt_device_node *np, if ( !val || len < sizeof(*out_value) ) return 0; -*out_value = be32_to_cpup(val); +*out_value = be32_to_cpu(*val); This code has been taken from Linux and I would rather prefer to keep the *cpup* helpers to avoid any changes when backporting. diff --git a/xen/include/xen/unaligned.h b/xen/include/xen/unaligned.h index 0a2b16d05d..16b2e6f5f0 100644 --- a/xen/include/xen/unaligned.h +++ b/xen/include/xen/unaligned.h @@ -20,62 +20,62 @@ static inline uint16_t get_unaligned_be16(const void *p) { - return be16_to_cpup(p); + return be16_to_cpu(*(const uint16_t *)p) I haven't checked the existing implementation of be16_to_cpup(). However, this new approach would allow the compiler to use a single load instruction to read the 16-bit value from memory. So this change may break on platform where unaligned access is forbidden (such as arm32). } static inline void put_unaligned_be16(uint16_t val, void *p) { - *(__force __be16*)p = cpu_to_be16(val); + *(__be16 *)p = cpu_to_be16(val); Why did you drop the __force? } Cheers, -- Julien Grall
[PATCH v3 2/6] crypto/vmac: Simplify code with byteswap
This file has its own implementation of swap bytes. Clean up the code with xen/byteswap.h. No functional change. Signed-off-by: Lin Liu Acked-by: Jan Beulich --- Cc: Andrew Cooper Cc: George Dunlap Cc: Jan Beulich Cc: Julien Grall Cc: Stefano Stabellini Cc: Wei Liu --- xen/crypto/vmac.c | 76 ++- 1 file changed, 3 insertions(+), 73 deletions(-) diff --git a/xen/crypto/vmac.c b/xen/crypto/vmac.c index 294dd16a52..acb4e015f5 100644 --- a/xen/crypto/vmac.c +++ b/xen/crypto/vmac.c @@ -8,6 +8,7 @@ /* start for Xen */ #include +#include #include #include #include @@ -50,7 +51,6 @@ const uint64_t mpoly = UINT64_C(0x1fff1fff); /* Poly key mask */ * MUL64: 64x64->128-bit multiplication * PMUL64: assumes top bits cleared on inputs * ADD128: 128x128->128-bit addition - * GET_REVERSED_64: load and byte-reverse 64-bit word * --- */ /* --- */ @@ -68,22 +68,6 @@ const uint64_t mpoly = UINT64_C(0x1fff1fff); /* Poly key mask */ #define PMUL64 MUL64 -#define GET_REVERSED_64(p)\ -({uint64_t x; \ - asm ("bswapq %0" : "=r" (x) : "0"(*(uint64_t *)(p))); x;}) - -/* --- */ -#elif (__GNUC__ && __i386__) -/* --- */ - -#define GET_REVERSED_64(p)\ -({ uint64_t x;\ -uint32_t *tp = (uint32_t *)(p); \ -asm ("bswap %%edx\n\t" \ - "bswap %%eax" \ -: "=A"(x) \ -: "a"(tp[1]), "d"(tp[0]));\ -x; }) /* --- */ #elif (__GNUC__ && __ppc64__) @@ -103,37 +87,6 @@ const uint64_t mpoly = UINT64_C(0x1fff1fff); /* Poly key mask */ #define PMUL64 MUL64 -#define GET_REVERSED_64(p)\ -({ uint32_t hi, lo, *_p = (uint32_t *)(p);\ - asm volatile ("lwbrx %0, %1, %2" : "=r"(lo) : "b%"(0), "r"(_p) ); \ - asm volatile ("lwbrx %0, %1, %2" : "=r"(hi) : "b%"(4), "r"(_p) ); \ - ((uint64_t)hi << 32) | (uint64_t)lo; } ) - -/* --- */ -#elif (__GNUC__ && (__ppc__ || __PPC__)) -/* --- */ - -#define GET_REVERSED_64(p)\ -({ uint32_t hi, lo, *_p = (uint32_t *)(p);\ - asm volatile ("lwbrx %0, %1, %2" : "=r"(lo) : "b%"(0), "r"(_p) ); \ - asm volatile ("lwbrx %0, %1, %2" : "=r"(hi) : "b%"(4), "r"(_p) ); \ - ((uint64_t)hi << 32) | (uint64_t)lo; } ) - -/* --- */ -#elif (__GNUC__ && (__ARMEL__ || __ARM__)) -/* --- */ - -#define bswap32(v)\ -({ uint32_t tmp,out; \ -asm volatile( \ -"eor%1, %2, %2, ror #16\n"\ -"bic%1, %1, #0x00ff\n"\ -"mov%0, %2, ror #8\n" \ -"eor%0, %0, %1, lsr #8" \ -: "=r" (out), "=" (tmp) \ -: "r" (v)); \ -out;}) - /* --- */ #elif _MSC_VER /* --- */ @@ -154,11 +107,6 @@ const uint64_t mpoly = UINT64_C(0x1fff1fff); /* Poly key mask */ (rh) += (ih) + ((rl) < (_il)); \ } -#if _MSC_VER >= 1300 -#define GET_REVERSED_64(p) _byteswap_uint64(*(uint64_t *)(p)) -#pragma intrinsic(_byteswap_uint64) -#endif - #if _MSC_VER >= 1400 && \ (!defined(__INTEL_COMPILER) || __INTEL_COMPILER >= 1000) #define MUL32(i1,i2)(__emulu((uint32_t)(i1),(uint32_t)(i2))) @@ -219,24 +167,6 @@ const uint64_t mpoly = UINT64_C(0x1fff1fff); /* Poly key mask */ } #endif -#ifndef
[PATCH v3 1/6] xen: implement byteswap
swab() is massively over complicated and can be simplified by builtins. The compilers provide builtin function to swap bytes. * gcc: https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html * clang: https://clang.llvm.org/docs/LanguageExtensions.html This patch simplify swab() with builtins and fallback for old compilers. Signed-off-by: Lin Liu --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Bertrand Marquis Cc: Volodymyr Babchuk Cc: Andrew Cooper Cc: George Dunlap Cc: Jan Beulich Cc: Wei Liu Cc: "Roger Pau Monné" Changes in v3: - Check __has_builtin instead of GNUC version Changes in v2: - Add fallback for compilers without __builtin_bswap - Implement with plain C instead of macros --- xen/arch/arm/include/asm/byteorder.h | 14 ++- xen/arch/x86/include/asm/byteorder.h | 34 ++--- xen/include/xen/byteorder.h | 56 xen/include/xen/byteswap.h | 44 ++ xen/include/xen/compiler.h | 12 ++ 5 files changed, 120 insertions(+), 40 deletions(-) create mode 100644 xen/include/xen/byteorder.h create mode 100644 xen/include/xen/byteswap.h diff --git a/xen/arch/arm/include/asm/byteorder.h b/xen/arch/arm/include/asm/byteorder.h index 9c712c4788..622eeaba07 100644 --- a/xen/arch/arm/include/asm/byteorder.h +++ b/xen/arch/arm/include/asm/byteorder.h @@ -1,16 +1,10 @@ #ifndef __ASM_ARM_BYTEORDER_H__ #define __ASM_ARM_BYTEORDER_H__ -#define __BYTEORDER_HAS_U64__ +#ifndef __BYTE_ORDER__ + #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ +#endif -#include +#include #endif /* __ASM_ARM_BYTEORDER_H__ */ -/* - * Local variables: - * mode: C - * c-file-style: "BSD" - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ diff --git a/xen/arch/x86/include/asm/byteorder.h b/xen/arch/x86/include/asm/byteorder.h index 1f77e502a5..82aadee7bd 100644 --- a/xen/arch/x86/include/asm/byteorder.h +++ b/xen/arch/x86/include/asm/byteorder.h @@ -1,36 +1,10 @@ #ifndef __ASM_X86_BYTEORDER_H__ #define __ASM_X86_BYTEORDER_H__ -#include -#include +#ifndef __BYTE_ORDER__ + #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ +#endif -static inline __attribute_const__ __u32 ___arch__swab32(__u32 x) -{ -asm("bswap %0" : "=r" (x) : "0" (x)); -return x; -} - -static inline __attribute_const__ __u64 ___arch__swab64(__u64 val) -{ -union { -struct { __u32 a,b; } s; -__u64 u; -} v; -v.u = val; -asm("bswapl %0 ; bswapl %1 ; xchgl %0,%1" -: "=r" (v.s.a), "=r" (v.s.b) -: "0" (v.s.a), "1" (v.s.b)); -return v.u; -} - -/* Do not define swab16. Gcc is smart enough to recognize "C" version and - convert it into rotation or exhange. */ - -#define __arch__swab64(x) ___arch__swab64(x) -#define __arch__swab32(x) ___arch__swab32(x) - -#define __BYTEORDER_HAS_U64__ - -#include +#include #endif /* __ASM_X86_BYTEORDER_H__ */ diff --git a/xen/include/xen/byteorder.h b/xen/include/xen/byteorder.h new file mode 100644 index 00..2ec434e6a6 --- /dev/null +++ b/xen/include/xen/byteorder.h @@ -0,0 +1,56 @@ +#ifndef __XEN_BYTEORDER_H__ +#define __XEN_BYTEORDER_H__ + +#include + +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ + +# ifndef __LITTLE_ENDIAN +# define __LITTLE_ENDIAN 1234 +# endif + +# ifndef __LITTLE_ENDIAN_BITFIELD +# define __LITTLE_ENDIAN_BITFIELD +# endif + +# define cpu_to_le64(x) (x) +# define le64_to_cpu(x) (x) +# define cpu_to_le32(x) (x) +# define le32_to_cpu(x) (x) +# define cpu_to_le16(x) (x) +# define le16_to_cpu(x) (x) +# define cpu_to_be64(x) bswap64(x) +# define be64_to_cpu(x) bswap64(x) +# define cpu_to_be32(x) bswap32(x) +# define be32_to_cpu(x) bswap32(x) +# define cpu_to_be16(x) bswap16(x) +# define be16_to_cpu(x) bswap16(x) + +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ + +# ifndef __BIG_ENDIAN +# define __BIG_ENDIAN 4321 +# endif + +# ifndef __BIG_ENDIAN_BITFIELD +# define __BIG_ENDIAN_BITFIELD +# endif + +# define cpu_to_le64(x) bswap64(x) +# define le64_to_cpu(x) bswap64(x) +# define cpu_to_le32(x) bswap32(x) +# define le32_to_cpu(x) bswap32(x) +# define cpu_to_le16(x) bswap16(x) +# define le16_to_cpu(x) bswap16(x) +# define cpu_to_be64(x) (x) +# define be64_to_cpu(x) (x) +# define cpu_to_be32(x) (x) +# define be32_to_cpu(x) (x) +# define cpu_to_be16(x) (x) +# define be16_to_cpu(x) (x) + +#else +# error "Unknown Endianness" +#endif /* __BYTE_ORDER__ */ + +#endif /* __XEN_BYTEORDER_H__ */ diff --git a/xen/include/xen/byteswap.h b/xen/include/xen/byteswap.h new file mode 100644 index 00..0dd5567557 --- /dev/null +++ b/xen/include/xen/byteswap.h @@ -0,0 +1,44 @@ +#ifndef __XEN_BYTESWAP_H__ +#define __XEN_BYTESWAP_H__ + +#include +#include + +#if !__has_builtin(__builtin_bswap16) +static always_inline uint16_t __builtin_bswap16(uint16_t val) +{ +return ((val & 0x00FF) << 8) | ((val & 0xFF00) >> 8); +} +#endif + +#if !__has_builtin(__builtin_bswap32) +static always_inline uint32_t __builtin_bswap32(uint32_t val) +{